Only include overhead if using send side bandwidth estimation.

Bug: webrtc:11298
Change-Id: Ia2daf690461b55d394c1b964d6a7977a98be8be2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/166820
Reviewed-by: Oskar Sundbom <ossu@webrtc.org>
Reviewed-by: Sam Zackrisson <saza@webrtc.org>
Reviewed-by: Ali Tofigh <alito@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30382}

audio/audio_send_stream.cc

@@ -342,6 +342,8 @@ void AudioSendStream::Start() {
       config_.max_bitrate_bps != -1 &&
       (allocate_audio_without_feedback_ || TransportSeqNumId(config_) != 0)) {
     rtp_transport_->AccountForAudioPacketsInPacedSender(true);
+    if (send_side_bwe_with_overhead_)
+      rtp_transport_->IncludeOverheadInPacedSender();
     rtp_rtcp_module_->SetAsPartOfAllocation(true);
     rtc::Event thread_sync_event;
     worker_queue_->PostTask([&] {
@@ -591,7 +593,8 @@ bool AudioSendStream::SetupSendCodec(const Config& new_config) {
   }
 
   // Enable ANA if configured (currently only used by Opus).
-  if (new_config.audio_network_adaptor_config) {
+  if (new_config.audio_network_adaptor_config &&
+      TransportSeqNumId(new_config) != 0) {
     if (encoder->EnableAudioNetworkAdaptor(
             *new_config.audio_network_adaptor_config, event_log_)) {
       RTC_DLOG(LS_INFO) << "Audio network adaptor enabled on SSRC "
@@ -690,7 +693,8 @@ void AudioSendStream::ReconfigureANA(const Config& new_config) {
       config_.audio_network_adaptor_config) {
     return;
   }
-  if (new_config.audio_network_adaptor_config) {
+  if (new_config.audio_network_adaptor_config &&
+      TransportSeqNumId(new_config) != 0) {
     channel_send_->CallEncoder([&](AudioEncoder* encoder) {
       if (encoder->EnableAudioNetworkAdaptor(
               *new_config.audio_network_adaptor_config, event_log_)) {
@@ -765,6 +769,8 @@ void AudioSendStream::ReconfigureBitrateObserver(
   if (!new_config.has_dscp && new_config.min_bitrate_bps != -1 &&
       new_config.max_bitrate_bps != -1 && TransportSeqNumId(new_config) != 0) {
     rtp_transport_->AccountForAudioPacketsInPacedSender(true);
+    if (send_side_bwe_with_overhead_)
+      rtp_transport_->IncludeOverheadInPacedSender();
     rtc::Event thread_sync_event;
     worker_queue_->PostTask([&] {
       RTC_DCHECK_RUN_ON(worker_queue_);

audio/audio_send_stream_unittest.cc

@@ -490,6 +490,8 @@ TEST(AudioSendStreamTest, SendCodecAppliesAudioNetworkAdaptor) {
   const std::string kAnaConfigString = "abcde";
   const std::string kAnaReconfigString = "12345";
 
+  helper.config().rtp.extensions.push_back(RtpExtension(
+      RtpExtension::kTransportSequenceNumberUri, kTransportSequenceNumberId));
   helper.config().audio_network_adaptor_config = kAnaConfigString;
 
   EXPECT_CALL(helper.mock_encoder_factory(), MakeAudioEncoderMock(_, _, _, _))

call/rtp_transport_controller_send.cc

@@ -434,6 +434,10 @@ void RtpTransportControllerSend::AccountForAudioPacketsInPacedSender(
   pacer()->SetAccountForAudioPackets(account_for_audio);
 }
 
+void RtpTransportControllerSend::IncludeOverheadInPacedSender() {
+  pacer()->SetIncludeOverhead();
+}
+
 void RtpTransportControllerSend::OnReceivedEstimatedBitrate(uint32_t bitrate) {
   RemoteBitrateReport msg;
   msg.receive_time = Timestamp::ms(clock_->TimeInMilliseconds());

call/rtp_transport_controller_send.h

@@ -107,6 +107,7 @@ class RtpTransportControllerSend final
       size_t transport_overhead_per_packet) override;
 
   void AccountForAudioPacketsInPacedSender(bool account_for_audio) override;
+  void IncludeOverheadInPacedSender() override;
 
   // Implements RtcpBandwidthObserver interface
   void OnReceivedEstimatedBitrate(uint32_t bitrate) override;

call/rtp_transport_controller_send_interface.h

@@ -150,6 +150,7 @@ class RtpTransportControllerSendInterface {
       size_t transport_overhead_per_packet) = 0;
 
   virtual void AccountForAudioPacketsInPacedSender(bool account_for_audio) = 0;
+  virtual void IncludeOverheadInPacedSender() = 0;
 };
 
 }  // namespace webrtc

call/rtp_video_sender.cc

@@ -279,6 +279,11 @@ absl::optional<VideoCodecType> GetVideoCodecType(const RtpConfig& config) {
   }
   return PayloadStringToCodecType(config.payload_name);
 }
+bool TransportSeqNumExtensionConfigured(const RtpConfig& config_config) {
+  return absl::c_any_of(config_config.extensions, [](const RtpExtension& ext) {
+    return ext.uri == RtpExtension::kTransportSequenceNumberUri;
+  });
+}
 }  // namespace
 
 RtpVideoSender::RtpVideoSender(
@@ -301,6 +306,7 @@ RtpVideoSender::RtpVideoSender(
           "WebRTC-SubtractPacketizationOverhead")),
       use_early_loss_detection_(
           !webrtc::field_trial::IsDisabled("WebRTC-UseEarlyLossDetection")),
+      has_packet_feedback_(TransportSeqNumExtensionConfigured(rtp_config)),
       active_(false),
       module_process_thread_(nullptr),
       suspended_ssrcs_(std::move(suspended_ssrcs)),
@@ -330,6 +336,8 @@ RtpVideoSender::RtpVideoSender(
       frame_counts_(rtp_config.ssrcs.size()),
       frame_count_observer_(observers.frame_count_observer) {
   RTC_DCHECK_EQ(rtp_config_.ssrcs.size(), rtp_streams_.size());
+  if (send_side_bwe_with_overhead_ && has_packet_feedback_)
+    transport_->IncludeOverheadInPacedSender();
   module_process_thread_checker_.Detach();
   // SSRCs are assumed to be sorted in the same order as |rtp_modules|.
   for (uint32_t ssrc : rtp_config_.ssrcs) {
@@ -700,7 +708,7 @@ void RtpVideoSender::OnBitrateUpdated(BitrateAllocationUpdate update,
   DataSize max_total_packet_size = DataSize::bytes(
       rtp_config_.max_packet_size + transport_overhead_bytes_per_packet_);
   uint32_t payload_bitrate_bps = update.target_bitrate.bps();
-  if (send_side_bwe_with_overhead_) {
+  if (send_side_bwe_with_overhead_ && has_packet_feedback_) {
     DataRate overhead_rate = CalculateOverheadRate(
         update.target_bitrate, max_total_packet_size, packet_overhead);
     // TODO(srte): We probably should not accept 0 payload bitrate here.
@@ -736,7 +744,7 @@ void RtpVideoSender::OnBitrateUpdated(BitrateAllocationUpdate update,
   loss_mask_vector_.clear();
 
   uint32_t encoder_overhead_rate_bps = 0;
-  if (send_side_bwe_with_overhead_) {
+  if (send_side_bwe_with_overhead_ && has_packet_feedback_) {
     // TODO(srte): The packet size should probably be the same as in the
     // CalculateOverheadRate call above (just max_total_packet_size), it doesn't
     // make sense to use different packet rates for different overhead

call/rtp_video_sender.h

@@ -163,6 +163,7 @@ class RtpVideoSender : public RtpVideoSenderInterface,
   const bool send_side_bwe_with_overhead_;
   const bool account_for_packetization_overhead_;
   const bool use_early_loss_detection_;
+  const bool has_packet_feedback_;
 
   // TODO(holmer): Remove crit_ once RtpVideoSender runs on the
   // transport task queue.

call/test/mock_rtp_transport_controller_send.h

@@ -67,6 +67,7 @@ class MockRtpTransportControllerSend
   MOCK_METHOD1(SetClientBitratePreferences, void(const BitrateSettings&));
   MOCK_METHOD1(OnTransportOverheadChanged, void(size_t));
   MOCK_METHOD1(AccountForAudioPacketsInPacedSender, void(bool));
+  MOCK_METHOD0(IncludeOverheadInPacedSender, void());
   MOCK_METHOD1(OnReceivedPacket, void(const ReceivedPacket&));
 };
 }  // namespace webrtc

modules/audio_coding/codecs/opus/audio_encoder_opus.cc

@@ -593,6 +593,11 @@ void AudioEncoderOpusImpl::OnReceivedUplinkPacketLossFraction(
   ApplyAudioNetworkAdaptor();
 }
 
+void AudioEncoderOpusImpl::OnReceivedTargetAudioBitrate(
+    int target_audio_bitrate_bps) {
+  SetTargetBitrate(target_audio_bitrate_bps);
+}
+
 void AudioEncoderOpusImpl::OnReceivedUplinkBandwidth(
     int target_audio_bitrate_bps,
     absl::optional<int64_t> bwe_period_ms,

modules/audio_coding/codecs/opus/audio_encoder_opus.h

@@ -104,6 +104,7 @@ class AudioEncoderOpusImpl final : public AudioEncoder {
   void DisableAudioNetworkAdaptor() override;
   void OnReceivedUplinkPacketLossFraction(
       float uplink_packet_loss_fraction) override;
+  void OnReceivedTargetAudioBitrate(int target_audio_bitrate_bps) override;
   void OnReceivedUplinkBandwidth(
       int target_audio_bitrate_bps,
       absl::optional<int64_t> bwe_period_ms) override;

modules/pacing/paced_sender.cc

@@ -126,6 +126,11 @@ void PacedSender::SetAccountForAudioPackets(bool account_for_audio) {
   pacing_controller_.SetAccountForAudioPackets(account_for_audio);
 }
 
+void PacedSender::SetIncludeOverhead() {
+  rtc::CritScope cs(&critsect_);
+  pacing_controller_.SetIncludeOverhead();
+}
+
 TimeDelta PacedSender::ExpectedQueueTime() const {
   rtc::CritScope cs(&critsect_);
   return pacing_controller_.ExpectedQueueTime();

modules/pacing/paced_sender.h

@@ -97,6 +97,8 @@ class PacedSender : public Module,
   // at high priority.
   void SetAccountForAudioPackets(bool account_for_audio) override;
 
+  void SetIncludeOverhead() override;
+
   // Returns the time since the oldest queued packet was enqueued.
   TimeDelta OldestPacketWaitTime() const override;
 

modules/pacing/pacing_controller.cc

@@ -99,8 +99,6 @@ PacingController::PacingController(Clock* clock,
       pace_audio_(IsEnabled(*field_trials_, "WebRTC-Pacer-BlockAudio")),
       small_first_probe_packet_(
           IsEnabled(*field_trials_, "WebRTC-Pacer-SmallFirstProbePacket")),
-      send_side_bwe_with_overhead_(
-          IsEnabled(*field_trials_, "WebRTC-SendSideBwe-WithOverhead")),
       min_packet_limit_(kDefaultMinPacketLimit),
       last_timestamp_(clock_->CurrentTime()),
       paused_(false),
@@ -120,7 +118,8 @@ PacingController::PacingController(Clock* clock,
       congestion_window_size_(DataSize::PlusInfinity()),
       outstanding_data_(DataSize::Zero()),
       queue_time_limit(kMaxExpectedQueueLength),
-      account_for_audio_(false) {
+      account_for_audio_(false),
+      include_overhead_(false) {
   if (!drain_large_queues_) {
     RTC_LOG(LS_WARNING) << "Pacer queues will not be drained,"
                            "pushback experiment must be enabled.";
@@ -226,6 +225,11 @@ void PacingController::SetAccountForAudioPackets(bool account_for_audio) {
   account_for_audio_ = account_for_audio;
 }
 
+void PacingController::SetIncludeOverhead() {
+  include_overhead_ = true;
+  packet_queue_.SetIncludeOverhead();
+}
+
 TimeDelta PacingController::ExpectedQueueTime() const {
   RTC_DCHECK_GT(pacing_bitrate_, DataRate::Zero());
   return TimeDelta::ms(
@@ -517,10 +521,10 @@ void PacingController::ProcessPackets() {
     RTC_DCHECK(rtp_packet);
     RTC_DCHECK(rtp_packet->packet_type().has_value());
     const RtpPacketToSend::Type packet_type = *rtp_packet->packet_type();
-    const DataSize packet_size = DataSize::bytes(
-        send_side_bwe_with_overhead_
-            ? rtp_packet->size()
-            : rtp_packet->payload_size() + rtp_packet->padding_size());
+    const DataSize packet_size =
+        DataSize::bytes(include_overhead_ ? rtp_packet->size()
+                                          : rtp_packet->payload_size() +
+                                                rtp_packet->padding_size());
     packet_sender_->SendRtpPacket(std::move(rtp_packet), pacing_info);
 
     data_sent += packet_size;

modules/pacing/pacing_controller.h

@@ -107,6 +107,7 @@ class PacingController {
   // the pacer budget calculation. The audio traffic still will be injected
   // at high priority.
   void SetAccountForAudioPackets(bool account_for_audio);
+  void SetIncludeOverhead();
 
   // Returns the time since the oldest queued packet was enqueued.
   TimeDelta OldestPacketWaitTime() const;
@@ -176,7 +177,6 @@ class PacingController {
   const bool send_padding_if_silent_;
   const bool pace_audio_;
   const bool small_first_probe_packet_;
-  const bool send_side_bwe_with_overhead_;
 
   TimeDelta min_packet_limit_;
 
@@ -219,6 +219,7 @@ class PacingController {
 
   TimeDelta queue_time_limit;
   bool account_for_audio_;
+  bool include_overhead_;
 };
 }  // namespace webrtc
 

modules/pacing/round_robin_packet_queue.cc

@@ -114,8 +114,7 @@ RoundRobinPacketQueue::RoundRobinPacketQueue(
       max_size_(kMaxLeadingSize),
       queue_time_sum_(TimeDelta::Zero()),
       pause_time_sum_(TimeDelta::Zero()),
-      send_side_bwe_with_overhead_(
-          IsEnabled(field_trials, "WebRTC-SendSideBwe-WithOverhead")) {}
+      include_overhead_(false) {}
 
 RoundRobinPacketQueue::~RoundRobinPacketQueue() {
   // Make sure to release any packets owned by raw pointer in QueuedPacket.
@@ -158,7 +157,7 @@ std::unique_ptr<RtpPacketToSend> RoundRobinPacketQueue::Pop() {
   // case a "budget" will be built up for the stream sending at the lower
   // rate. To avoid building a too large budget we limit |bytes| to be within
   // kMaxLeading bytes of the stream that has sent the most amount of bytes.
-  DataSize packet_size = queued_packet.Size(send_side_bwe_with_overhead_);
+  DataSize packet_size = queued_packet.Size(include_overhead_);
   stream->size =
       std::max(stream->size + packet_size, max_size_ - kMaxLeadingSize);
   max_size_ = std::max(max_size_, stream->size);
@@ -238,6 +237,10 @@ void RoundRobinPacketQueue::SetPauseState(bool paused, Timestamp now) {
   paused_ = paused;
 }
 
+void RoundRobinPacketQueue::SetIncludeOverhead() {
+  include_overhead_ = true;
+}
+
 TimeDelta RoundRobinPacketQueue::AverageQueueTime() const {
   if (Empty())
     return TimeDelta::Zero();
@@ -279,7 +282,7 @@ void RoundRobinPacketQueue::Push(QueuedPacket packet) {
   packet.SubtractPauseTime(pause_time_sum_);
 
   size_packets_ += 1;
-  size_ += packet.Size(send_side_bwe_with_overhead_);
+  size_ += packet.Size(include_overhead_);
 
   stream->packet_queue.push(packet);
 }

modules/pacing/round_robin_packet_queue.h

@@ -52,6 +52,7 @@ class RoundRobinPacketQueue {
   TimeDelta AverageQueueTime() const;
   void UpdateQueueTime(Timestamp now);
   void SetPauseState(bool paused, Timestamp now);
+  void SetIncludeOverhead();
 
  private:
   struct QueuedPacket {
@@ -150,7 +151,7 @@ class RoundRobinPacketQueue {
   // the age of the oldest packet in the queue.
   std::multiset<Timestamp> enqueue_times_;
 
-  const bool send_side_bwe_with_overhead_;
+  bool include_overhead_;
 };
 }  // namespace webrtc
 

modules/pacing/rtp_packet_pacer.h

@@ -64,6 +64,7 @@ class RtpPacketPacer {
   // the pacer budget calculation. The audio traffic still will be injected
   // at high priority.
   virtual void SetAccountForAudioPackets(bool account_for_audio) = 0;
+  virtual void SetIncludeOverhead() = 0;
 };
 
 }  // namespace webrtc

modules/pacing/task_queue_paced_sender.cc

@@ -136,6 +136,13 @@ void TaskQueuePacedSender::SetAccountForAudioPackets(bool account_for_audio) {
   });
 }
 
+void TaskQueuePacedSender::SetIncludeOverhead() {
+  task_queue_.PostTask([this]() {
+    RTC_DCHECK_RUN_ON(&task_queue_);
+    pacing_controller_.SetIncludeOverhead();
+  });
+}
+
 void TaskQueuePacedSender::SetQueueTimeLimit(TimeDelta limit) {
   task_queue_.PostTask([this, limit]() {
     RTC_DCHECK_RUN_ON(&task_queue_);

modules/pacing/task_queue_paced_sender.h

@@ -79,6 +79,7 @@ class TaskQueuePacedSender : public RtpPacketPacer,
   // at high priority.
   void SetAccountForAudioPackets(bool account_for_audio) override;
 
+  void SetIncludeOverhead() override;
   // Returns the time since the oldest queued packet was enqueued.
   TimeDelta OldestPacketWaitTime() const override;