Reland "Reland "Only include overhead if using send side bandwidth estimation.""

This is a reland of 086055d0fd9b9b9efe8bcf85884324a019e9bd33

ANA was accitendly disabled even when transport sequence numbers were
negotiated due to a bug in how the audio send stream is configured. To
solve this we simply continue to always allow enabling ANA and leave it
up to the application to ensure that it's not used together with receive
side estimation.

Original change's description:
> Reland "Only include overhead if using send side bandwidth estimation."
>
> This is a reland of 8c79c6e1af354c526497082c79ccbe12af03a33e
>
> Original change's description:
> > 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}
>
> Bug: webrtc:11298
> Change-Id: I33205e869a8ae27c15ffe991f6d985973ed6d15a
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/167524
> Reviewed-by: Ali Tofigh <alito@webrtc.org>
> Reviewed-by: Sam Zackrisson <saza@webrtc.org>
> Reviewed-by: Erik Språng <sprang@webrtc.org>
> Reviewed-by: Oskar Sundbom <ossu@webrtc.org>
> Commit-Queue: Sebastian Jansson <srte@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#30390}

Bug: webrtc:11298
Change-Id: If2ad91e17ebfc85dc51edcd9607996e18c5d1f13
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/167883
Commit-Queue: Sebastian Jansson <srte@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30413}

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([&] {
@@ -765,6 +767,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(
+    const DataSize packet_size =
-        send_side_bwe_with_overhead_
+        DataSize::bytes(include_overhead_ ? rtp_packet->size()
-            ? rtp_packet->size()
+                                          : rtp_packet->payload_size() +
-            : rtp_packet->payload_size() + rtp_packet->padding_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

@@ -93,6 +93,16 @@ void RoundRobinPacketQueue::QueuedPacket::SubtractPauseTime(
   enqueue_time_ -= pause_time_sum;
 }
 
+RoundRobinPacketQueue::PriorityPacketQueue::const_iterator
+RoundRobinPacketQueue::PriorityPacketQueue::begin() const {
+  return c.begin();
+}
+
+RoundRobinPacketQueue::PriorityPacketQueue::const_iterator
+RoundRobinPacketQueue::PriorityPacketQueue::end() const {
+  return c.end();
+}
+
 RoundRobinPacketQueue::Stream::Stream() : size(DataSize::Zero()), ssrc(0) {}
 RoundRobinPacketQueue::Stream::Stream(const Stream& stream) = default;
 RoundRobinPacketQueue::Stream::~Stream() = default;
@@ -114,8 +124,7 @@ RoundRobinPacketQueue::RoundRobinPacketQueue(
       max_size_(kMaxLeadingSize),
       queue_time_sum_(TimeDelta::Zero()),
       pause_time_sum_(TimeDelta::Zero()),
-      send_side_bwe_with_overhead_(
+      include_overhead_(false) {}
-          IsEnabled(field_trials, "WebRTC-SendSideBwe-WithOverhead")) {}
 
 RoundRobinPacketQueue::~RoundRobinPacketQueue() {
   // Make sure to release any packets owned by raw pointer in QueuedPacket.
@@ -158,7 +167,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 +247,17 @@ void RoundRobinPacketQueue::SetPauseState(bool paused, Timestamp now) {
   paused_ = paused;
 }
 
+void RoundRobinPacketQueue::SetIncludeOverhead() {
+  include_overhead_ = true;
+  // We need to update the size to reflect overhead for existing packets.
+  size_ = DataSize::Zero();
+  for (const auto& stream : streams_) {
+    for (const QueuedPacket& packet : stream.second.packet_queue) {
+      size_ += packet.Size(include_overhead_);
+    }
+  }
+}
+
 TimeDelta RoundRobinPacketQueue::AverageQueueTime() const {
   if (Empty())
     return TimeDelta::Zero();
@@ -279,7 +299,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 {
@@ -89,6 +90,13 @@ class RoundRobinPacketQueue {
     RtpPacketToSend* owned_packet_;
   };
 
+  class PriorityPacketQueue : public std::priority_queue<QueuedPacket> {
+   public:
+    using const_iterator = container_type::const_iterator;
+    const_iterator begin() const;
+    const_iterator end() const;
+  };
+
   struct StreamPrioKey {
     StreamPrioKey(int priority, DataSize size)
         : priority(priority), size(size) {}
@@ -111,7 +119,8 @@ class RoundRobinPacketQueue {
 
     DataSize size;
     uint32_t ssrc;
-    std::priority_queue<QueuedPacket> packet_queue;
+
+    PriorityPacketQueue packet_queue;
 
     // Whenever a packet is inserted for this stream we check if |priority_it|
     // points to an element in |stream_priorities_|, and if it does it means
@@ -150,7 +159,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;