[Cast] Fix bandwidth estimator

Currently, the bandwidth estimator always reports 0kbps, even though we
are successfully sending packets.

Turns out, the LoopingFileSender is erroneously using a second
SenderPacketRouter that is separate from the SenderSession's instance,
so it has no packets to report. This feature is now exposed on the
SenderSession, since bandwidth estimation is a helpful signal for
embedders to throttle capture.

Bug: b/183996401
Change-Id: I932085dfe00df29d8ae79a184ee10b1d3fe5594d
Reviewed-on: https://chromium-review.googlesource.com/c/openscreen/+/2792913
Reviewed-by: Brandon Tolsch <btolsch@chromium.org>
Commit-Queue: Jordan Bayles <jophba@chromium.org>

7 files changed, 33 insertions, 9 deletions

diff --git a/cast/standalone_sender/looping_file_cast_agent.cc b/cast/standalone_sender/looping_file_cast_agent.cc
index bb88226f..e5d75815 100644
--- a/cast/standalone_sender/looping_file_cast_agent.cc
+++ b/cast/standalone_sender/looping_file_cast_agent.cc
@@ -278,7 +278,12 @@ void LoopingFileCastAgent::CreateAndStartSession() {
   OSP_DCHECK(!message_port_.client_sender_id().empty());
 
   AudioCaptureConfig audio_config;
+  // Opus does best at 192kbps, so we cap that here.
+  audio_config.bit_rate = 192 * 1000;
   VideoCaptureConfig video_config;
+  // The video config is allowed to use whatever is left over after audio.
+  video_config.max_bit_rate =
+      connection_settings_->max_bitrate - audio_config.bit_rate;
   // Use default display resolution of 1080P.
   video_config.resolutions.emplace_back(DisplayResolution{});
 
@@ -300,8 +305,8 @@ void LoopingFileCastAgent::OnNegotiated(
   }
 
   file_sender_ = std::make_unique<LoopingFileSender>(
-      environment_.get(), connection_settings_->path_to_file.c_str(), senders,
-      connection_settings_->max_bitrate);
+      environment_.get(), connection_settings_->path_to_file.c_str(), session,
+      std::move(senders), connection_settings_->max_bitrate);
 }
 
 void LoopingFileCastAgent::OnError(const SenderSession* session, Error error) {
diff --git a/cast/standalone_sender/looping_file_sender.cc b/cast/standalone_sender/looping_file_sender.cc
index 67aff23b..9fae8439 100644
--- a/cast/standalone_sender/looping_file_sender.cc
+++ b/cast/standalone_sender/looping_file_sender.cc
@@ -11,11 +11,12 @@ namespace cast {
 
 LoopingFileSender::LoopingFileSender(Environment* environment,
                                      const char* path,
+                                     const SenderSession* session,
                                      SenderSession::ConfiguredSenders senders,
                                      int max_bitrate)
     : env_(environment),
       path_(path),
-      packet_router_(env_),
+      session_(session),
       max_bitrate_(max_bitrate),
       audio_encoder_(senders.audio_sender->config().channels,
                      StreamingOpusEncoder::kDefaultCastAudioFramesPerSecond,
@@ -51,7 +52,7 @@ void LoopingFileSender::UpdateEncoderBitrates() {
 }
 
 void LoopingFileSender::ControlForNetworkCongestion() {
-  bandwidth_estimate_ = packet_router_.ComputeNetworkBandwidth();
+  bandwidth_estimate_ = session_->GetEstimatedNetworkBandwidth();
   if (bandwidth_estimate_ > 0) {
     // Don't ever try to use *all* of the network bandwidth! However, don't go
     // below the absolute minimum requirement either.
diff --git a/cast/standalone_sender/looping_file_sender.h b/cast/standalone_sender/looping_file_sender.h
index 4f6c5d90..e55a4a7e 100644
--- a/cast/standalone_sender/looping_file_sender.h
+++ b/cast/standalone_sender/looping_file_sender.h
@@ -24,6 +24,7 @@ class LoopingFileSender final : public SimulatedAudioCapturer::Client,
  public:
   LoopingFileSender(Environment* environment,
                     const char* path,
+                    const SenderSession* session,
                     SenderSession::ConfiguredSenders senders,
                     int max_bitrate);
 
@@ -59,11 +60,12 @@ class LoopingFileSender final : public SimulatedAudioCapturer::Client,
   // The path to the media file to stream over and over.
   const char* const path_;
 
-  // The packet router allows both the Audio Sender and the Video Sender to
-  // share the same UDP socket.
-  SenderPacketRouter packet_router_;
+  // Session to query for bandwidth information.
+  const SenderSession* session_;
+
+  // User provided maximum bitrate (from command line argument).
+  const int max_bitrate_;
 
-  const int max_bitrate_;  // Passed by the user on the command line.
   int bandwidth_estimate_ = 0;
   int bandwidth_being_utilized_;
 
diff --git a/cast/streaming/sender_packet_router.h b/cast/streaming/sender_packet_router.h
index a6c161a9..c86cb2a5 100644
--- a/cast/streaming/sender_packet_router.h
+++ b/cast/streaming/sender_packet_router.h
@@ -146,7 +146,7 @@ class SenderPacketRouter : public BandwidthEstimator,
   // a burst-send.
   void ScheduleNextBurst();
 
-  // Performs a burst-send of packets. This is called whevener the Alarm fires.
+  // Performs a burst-send of packets. This is called whenever the Alarm fires.
   void SendBurstOfPackets();
 
   // Send an RTCP packet from each Sender that has one ready, and return the
diff --git a/cast/streaming/sender_session.cc b/cast/streaming/sender_session.cc
index 329da305..0c90e937 100644
--- a/cast/streaming/sender_session.cc
+++ b/cast/streaming/sender_session.cc
@@ -187,6 +187,10 @@ Error SenderSession::Negotiate(std::vector<AudioCaptureConfig> audio_configs,
       [this](ReceiverMessage message) { OnAnswer(message); });
 }
 
+int SenderSession::GetEstimatedNetworkBandwidth() const {
+  return packet_router_.ComputeNetworkBandwidth();
+}
+
 void SenderSession::OnAnswer(ReceiverMessage message) {
   OSP_LOG_WARN << "Message sn: " << message.sequence_number
                << ", current: " << current_sequence_number_;
diff --git a/cast/streaming/sender_session.h b/cast/streaming/sender_session.h
index 86ab4d5a..61864d0b 100644
--- a/cast/streaming/sender_session.h
+++ b/cast/streaming/sender_session.h
@@ -98,6 +98,11 @@ class SenderSession final {
   Error Negotiate(std::vector<AudioCaptureConfig> audio_configs,
                   std::vector<VideoCaptureConfig> video_configs);
 
+  // Get the current network usage (in bits per second). This includes all
+  // senders managed by this session, and is a best guess based on receiver
+  // feedback. Embedders may use this information to throttle capture devices.
+  int GetEstimatedNetworkBandwidth() const;
+
  private:
   // We store the current negotiation, so that when we get an answer from the
   // receiver we can line up the selected streams with the original
diff --git a/cast/streaming/sender_session_unittest.cc b/cast/streaming/sender_session_unittest.cc
index 2ea2073b..3667309f 100644
--- a/cast/streaming/sender_session_unittest.cc
+++ b/cast/streaming/sender_session_unittest.cc
@@ -454,5 +454,12 @@ TEST_F(SenderSessionTest, DoesNotCrashOnMessagePortError) {
   message_port_->ReceiveError(Error(Error::Code::kUnknownError));
 }
 
+TEST_F(SenderSessionTest, ReportsZeroBandwidthWhenNoPacketsSent) {
+  // TODO(issuetracker.google.com/183996645): As part of end to end testing,
+  // we need to ensure that we are providing reasonable network bandwidth
+  // measurements.
+  EXPECT_EQ(0, session_->GetEstimatedNetworkBandwidth());
+}
+
 }  // namespace cast
 }  // namespace openscreen