Add ability to throttle VideoBitrateAllocation updates.

When the bandwidth estimate is volatile, and the frame rate is high,
each new frame might trigger a new video bitrate allocation that is very
close to the previous one, during BWE rampup.
This might cause unnecessarily high RTCP traffic.

This CL throttles those updates, if the allocation fullfills all of:
* Larger or the same total bitrate as the previously sent one
* Less than 10% larger bitrate compared to the previous one
* Same layers enables as the previous one
* Less than 500ms has passed since the previous one

Additionally, a call to OnEncodedImage can cause a throttled allocation
to be sent if 500ms has passed but no new call to OnBitrateUpdated has
been seen.

Bug: webrtc:9734
Change-Id: I2a17c2e512387e273e9c22bffcebd290727dc883
Reviewed-on: https://webrtc-review.googlesource.com/100560
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#24751}
diff --git a/video/video_send_stream_impl.cc b/video/video_send_stream_impl.cc
index 5595bd9..721e84f 100644
--- a/video/video_send_stream_impl.cc
+++ b/video/video_send_stream_impl.cc
@@ -33,6 +33,11 @@
 // packet sequence number for at least last 60 seconds.
 static const int kSendSideSeqNumSetMaxSize = 5500;
 
+// Max positive size difference to treat allocations as "similar".
+static constexpr int kMaxVbaSizeDifferencePercent = 10;
+// Max time we will throttle similar video bitrate allocations.
+static constexpr int64_t kMaxVbaThrottleTimeMs = 500;
+
 // We don't do MTU discovery, so assume that we have the standard ethernet MTU.
 const size_t kPathMTU = 1500;
 
@@ -156,6 +161,18 @@
   return AlrExperimentSettings::CreateFromFieldTrial(
       AlrExperimentSettings::kStrictPacingAndProbingExperimentName);
 }
+
+bool SameStreamsEnabled(const VideoBitrateAllocation& lhs,
+                        const VideoBitrateAllocation& rhs) {
+  for (size_t si = 0; si < kMaxSpatialLayers; ++si) {
+    for (size_t ti = 0; ti < kMaxTemporalStreams; ++ti) {
+      if (lhs.HasBitrate(si, ti) != rhs.HasBitrate(si, ti)) {
+        return false;
+      }
+    }
+  }
+  return true;
+}
 }  // namespace
 
 // CheckEncoderActivityTask is used for tracking when the encoder last produced
@@ -454,7 +471,48 @@
 
 void VideoSendStreamImpl::OnBitrateAllocationUpdated(
     const VideoBitrateAllocation& allocation) {
+  if (!worker_queue_->IsCurrent()) {
+    auto ptr = weak_ptr_;
+    worker_queue_->PostTask([=] {
+      if (!ptr.get())
+        return;
+      ptr->OnBitrateAllocationUpdated(allocation);
+    });
+    return;
+  }
+
+  RTC_DCHECK_RUN_ON(worker_queue_);
+
+  int64_t now_ms = rtc::TimeMillis();
   if (encoder_target_rate_bps_ != 0) {
+    if (video_bitrate_allocation_context_) {
+      // If new allocation is within kMaxVbaSizeDifferencePercent larger than
+      // the previously sent allocation and the same streams are still enabled,
+      // it is considered "similar". We do not want send similar allocations
+      // more once per kMaxVbaThrottleTimeMs.
+      const VideoBitrateAllocation& last =
+          video_bitrate_allocation_context_->last_sent_allocation;
+      const bool is_similar =
+          allocation.get_sum_bps() >= last.get_sum_bps() &&
+          allocation.get_sum_bps() <
+              (last.get_sum_bps() * (100 + kMaxVbaSizeDifferencePercent)) /
+                  100 &&
+          SameStreamsEnabled(allocation, last);
+      if (is_similar &&
+          (now_ms - video_bitrate_allocation_context_->last_send_time_ms) <
+              kMaxVbaThrottleTimeMs) {
+        // This allocation is too similar, cache it and return.
+        video_bitrate_allocation_context_->throttled_allocation = allocation;
+        return;
+      }
+    } else {
+      video_bitrate_allocation_context_.emplace();
+    }
+
+    video_bitrate_allocation_context_->last_sent_allocation = allocation;
+    video_bitrate_allocation_context_->throttled_allocation.reset();
+    video_bitrate_allocation_context_->last_send_time_ms = now_ms;
+
     // Send bitrate allocation metadata only if encoder is not paused.
     rtp_video_sender_->OnBitrateAllocationUpdated(allocation);
   }
@@ -567,8 +625,28 @@
 
   fec_controller_->UpdateWithEncodedData(encoded_image._length,
                                          encoded_image._frameType);
-  return rtp_video_sender_->OnEncodedImage(encoded_image, codec_specific_info,
-                                           fragmentation);
+  EncodedImageCallback::Result result = rtp_video_sender_->OnEncodedImage(
+      encoded_image, codec_specific_info, fragmentation);
+
+  // Check if there's a throttled VideoBitrateAllocation that we should try
+  // sending.
+  rtc::WeakPtr<VideoSendStreamImpl> send_stream = weak_ptr_;
+  auto update_task = [send_stream]() {
+    if (send_stream) {
+      RTC_DCHECK_RUN_ON(send_stream->worker_queue_);
+      auto& context = send_stream->video_bitrate_allocation_context_;
+      if (context && context->throttled_allocation) {
+        send_stream->OnBitrateAllocationUpdated(*context->throttled_allocation);
+      }
+    }
+  };
+  if (!worker_queue_->IsCurrent()) {
+    worker_queue_->PostTask(update_task);
+  } else {
+    update_task();
+  }
+
+  return result;
 }
 
 std::map<uint32_t, RtpState> VideoSendStreamImpl::GetRtpStates() const {
diff --git a/video/video_send_stream_impl.h b/video/video_send_stream_impl.h
index 79fcc21..c87a32e 100644
--- a/video/video_send_stream_impl.h
+++ b/video/video_send_stream_impl.h
@@ -182,6 +182,16 @@
 
   std::unordered_set<uint16_t> feedback_packet_seq_num_set_;
   std::vector<bool> loss_mask_vector_;
+
+  // Context for the most recent and last sent video bitrate allocation. Used to
+  // throttle sending of similar bitrate allocations.
+  struct VbaSendContext {
+    VideoBitrateAllocation last_sent_allocation;
+    absl::optional<VideoBitrateAllocation> throttled_allocation;
+    int64_t last_send_time_ms;
+  };
+  absl::optional<VbaSendContext> video_bitrate_allocation_context_
+      RTC_GUARDED_BY(worker_queue_);
 };
 }  // namespace internal
 }  // namespace webrtc
diff --git a/video/video_send_stream_impl_unittest.cc b/video/video_send_stream_impl_unittest.cc
index 412478f..03c6244 100644
--- a/video/video_send_stream_impl_unittest.cc
+++ b/video/video_send_stream_impl_unittest.cc
@@ -17,6 +17,7 @@
 #include "modules/utility/include/process_thread.h"
 #include "modules/video_coding/fec_controller_default.h"
 #include "rtc_base/experiments/alr_experiment.h"
+#include "rtc_base/fakeclock.h"
 #include "rtc_base/task_queue_for_test.h"
 #include "test/field_trial.h"
 #include "test/gmock.h"
@@ -341,5 +342,216 @@
     vss_impl->Stop();
   });
 }
+
+TEST_F(VideoSendStreamImplTest, ForwardsVideoBitrateAllocationWhenEnabled) {
+  test_queue_.SendTask([this] {
+    EXPECT_CALL(transport_controller_, SetPacingFactor(_)).Times(0);
+    auto vss_impl = CreateVideoSendStreamImpl(
+        kDefaultInitialBitrateBps, kDefaultBitratePriority,
+        VideoEncoderConfig::ContentType::kScreen);
+    vss_impl->Start();
+    VideoBitrateAllocationObserver* const observer =
+        static_cast<VideoBitrateAllocationObserver*>(vss_impl.get());
+
+    // Populate a test instance of video bitrate allocation.
+    VideoBitrateAllocation alloc;
+    alloc.SetBitrate(0, 0, 10000);
+    alloc.SetBitrate(0, 1, 20000);
+    alloc.SetBitrate(1, 0, 30000);
+    alloc.SetBitrate(1, 1, 40000);
+
+    // Encoder starts out paused, don't forward allocation.
+    EXPECT_CALL(payload_router_, OnBitrateAllocationUpdated(alloc)).Times(0);
+    observer->OnBitrateAllocationUpdated(alloc);
+
+    // Unpause encoder, allocation should be passed through.
+    static_cast<BitrateAllocatorObserver*>(vss_impl.get())
+        ->OnBitrateUpdated(100000, 0, 0, 0);
+    EXPECT_CALL(payload_router_, OnBitrateAllocationUpdated(alloc)).Times(1);
+    observer->OnBitrateAllocationUpdated(alloc);
+
+    // Pause encoder again, and block allocations.
+    static_cast<BitrateAllocatorObserver*>(vss_impl.get())
+        ->OnBitrateUpdated(0, 0, 0, 0);
+    EXPECT_CALL(payload_router_, OnBitrateAllocationUpdated(alloc)).Times(0);
+    observer->OnBitrateAllocationUpdated(alloc);
+
+    vss_impl->Stop();
+  });
+}
+
+TEST_F(VideoSendStreamImplTest, ThrottlesVideoBitrateAllocationWhenTooSimilar) {
+  test_queue_.SendTask([this] {
+    auto vss_impl = CreateVideoSendStreamImpl(
+        kDefaultInitialBitrateBps, kDefaultBitratePriority,
+        VideoEncoderConfig::ContentType::kScreen);
+    vss_impl->Start();
+    // Unpause encoder, to allows allocations to be passed through.
+    static_cast<BitrateAllocatorObserver*>(vss_impl.get())
+        ->OnBitrateUpdated(100000, 0, 0, 0);
+    VideoBitrateAllocationObserver* const observer =
+        static_cast<VideoBitrateAllocationObserver*>(vss_impl.get());
+
+    // Populate a test instance of video bitrate allocation.
+    VideoBitrateAllocation alloc;
+    alloc.SetBitrate(0, 0, 10000);
+    alloc.SetBitrate(0, 1, 20000);
+    alloc.SetBitrate(1, 0, 30000);
+    alloc.SetBitrate(1, 1, 40000);
+
+    // Initial value.
+    EXPECT_CALL(payload_router_, OnBitrateAllocationUpdated(alloc)).Times(1);
+    observer->OnBitrateAllocationUpdated(alloc);
+
+    VideoBitrateAllocation updated_alloc = alloc;
+    // Needs 10% increase in bitrate to trigger immediate forward.
+    const uint32_t base_layer_min_update_bitrate_bps =
+        alloc.GetBitrate(0, 0) + alloc.get_sum_bps() / 10;
+
+    // Too small increase, don't forward.
+    updated_alloc.SetBitrate(0, 0, base_layer_min_update_bitrate_bps - 1);
+    EXPECT_CALL(payload_router_, OnBitrateAllocationUpdated(_)).Times(0);
+    observer->OnBitrateAllocationUpdated(updated_alloc);
+
+    // Large enough increase, do forward.
+    updated_alloc.SetBitrate(0, 0, base_layer_min_update_bitrate_bps);
+    EXPECT_CALL(payload_router_, OnBitrateAllocationUpdated(updated_alloc))
+        .Times(1);
+    observer->OnBitrateAllocationUpdated(updated_alloc);
+
+    // This is now a decrease compared to last forward allocation, forward
+    // immediately.
+    updated_alloc.SetBitrate(0, 0, base_layer_min_update_bitrate_bps - 1);
+    EXPECT_CALL(payload_router_, OnBitrateAllocationUpdated(updated_alloc))
+        .Times(1);
+    observer->OnBitrateAllocationUpdated(updated_alloc);
+
+    vss_impl->Stop();
+  });
+}
+
+TEST_F(VideoSendStreamImplTest, ForwardsVideoBitrateAllocationOnLayerChange) {
+  test_queue_.SendTask([this] {
+    auto vss_impl = CreateVideoSendStreamImpl(
+        kDefaultInitialBitrateBps, kDefaultBitratePriority,
+        VideoEncoderConfig::ContentType::kScreen);
+    vss_impl->Start();
+    // Unpause encoder, to allows allocations to be passed through.
+    static_cast<BitrateAllocatorObserver*>(vss_impl.get())
+        ->OnBitrateUpdated(100000, 0, 0, 0);
+    VideoBitrateAllocationObserver* const observer =
+        static_cast<VideoBitrateAllocationObserver*>(vss_impl.get());
+
+    // Populate a test instance of video bitrate allocation.
+    VideoBitrateAllocation alloc;
+    alloc.SetBitrate(0, 0, 10000);
+    alloc.SetBitrate(0, 1, 20000);
+    alloc.SetBitrate(1, 0, 30000);
+    alloc.SetBitrate(1, 1, 40000);
+
+    // Initial value.
+    EXPECT_CALL(payload_router_, OnBitrateAllocationUpdated(alloc)).Times(1);
+    observer->OnBitrateAllocationUpdated(alloc);
+
+    // Move some bitrate from one layer to a new one, but keep sum the same.
+    // Since layout has changed, immediately trigger forward.
+    VideoBitrateAllocation updated_alloc = alloc;
+    updated_alloc.SetBitrate(2, 0, 10000);
+    updated_alloc.SetBitrate(1, 1, alloc.GetBitrate(1, 1) - 10000);
+    EXPECT_EQ(alloc.get_sum_bps(), updated_alloc.get_sum_bps());
+    EXPECT_CALL(payload_router_, OnBitrateAllocationUpdated(updated_alloc))
+        .Times(1);
+    observer->OnBitrateAllocationUpdated(updated_alloc);
+
+    vss_impl->Stop();
+  });
+}
+
+TEST_F(VideoSendStreamImplTest, ForwardsVideoBitrateAllocationAfterTimeout) {
+  test_queue_.SendTask([this] {
+    rtc::ScopedFakeClock fake_clock;
+    fake_clock.SetTimeMicros(clock_.TimeInMicroseconds());
+
+    auto vss_impl = CreateVideoSendStreamImpl(
+        kDefaultInitialBitrateBps, kDefaultBitratePriority,
+        VideoEncoderConfig::ContentType::kScreen);
+    vss_impl->Start();
+    // Unpause encoder, to allows allocations to be passed through.
+    static_cast<BitrateAllocatorObserver*>(vss_impl.get())
+        ->OnBitrateUpdated(100000, 0, 0, 0);
+    VideoBitrateAllocationObserver* const observer =
+        static_cast<VideoBitrateAllocationObserver*>(vss_impl.get());
+
+    // Populate a test instance of video bitrate allocation.
+    VideoBitrateAllocation alloc;
+    alloc.SetBitrate(0, 0, 10000);
+    alloc.SetBitrate(0, 1, 20000);
+    alloc.SetBitrate(1, 0, 30000);
+    alloc.SetBitrate(1, 1, 40000);
+
+    EncodedImage encoded_image;
+    CodecSpecificInfo codec_specific;
+    EXPECT_CALL(payload_router_, OnEncodedImage(_, _, _))
+        .WillRepeatedly(Return(
+            EncodedImageCallback::Result(EncodedImageCallback::Result::OK)));
+
+    // Max time we will throttle similar video bitrate allocations.
+    static constexpr int64_t kMaxVbaThrottleTimeMs = 500;
+
+    {
+      // Initial value.
+      EXPECT_CALL(payload_router_, OnBitrateAllocationUpdated(alloc)).Times(1);
+      observer->OnBitrateAllocationUpdated(alloc);
+    }
+
+    {
+      // Sending same allocation again, this one should be throttled.
+      EXPECT_CALL(payload_router_, OnBitrateAllocationUpdated(alloc)).Times(0);
+      observer->OnBitrateAllocationUpdated(alloc);
+    }
+
+    fake_clock.AdvanceTimeMicros(kMaxVbaThrottleTimeMs * 1000);
+
+    {
+      // Sending similar allocation again after timeout, should forward.
+      EXPECT_CALL(payload_router_, OnBitrateAllocationUpdated(alloc)).Times(1);
+      observer->OnBitrateAllocationUpdated(alloc);
+    }
+
+    {
+      // Sending similar allocation again without timeout, throttle.
+      EXPECT_CALL(payload_router_, OnBitrateAllocationUpdated(alloc)).Times(0);
+      observer->OnBitrateAllocationUpdated(alloc);
+    }
+
+    {
+      // Send encoded image, should be a noop.
+      EXPECT_CALL(payload_router_, OnBitrateAllocationUpdated(alloc)).Times(0);
+      static_cast<EncodedImageCallback*>(vss_impl.get())
+          ->OnEncodedImage(encoded_image, &codec_specific, nullptr);
+    }
+
+    {
+      // Advance time and send encoded image, this should wake up and send
+      // cached bitrate allocation.
+      fake_clock.AdvanceTimeMicros(kMaxVbaThrottleTimeMs * 1000);
+      EXPECT_CALL(payload_router_, OnBitrateAllocationUpdated(alloc)).Times(1);
+      static_cast<EncodedImageCallback*>(vss_impl.get())
+          ->OnEncodedImage(encoded_image, &codec_specific, nullptr);
+    }
+
+    {
+      // Advance time and send encoded image, there should be no cached
+      // allocation to send.
+      fake_clock.AdvanceTimeMicros(kMaxVbaThrottleTimeMs * 1000);
+      EXPECT_CALL(payload_router_, OnBitrateAllocationUpdated(alloc)).Times(0);
+      static_cast<EncodedImageCallback*>(vss_impl.get())
+          ->OnEncodedImage(encoded_image, &codec_specific, nullptr);
+    }
+
+    vss_impl->Stop();
+  });
+}
+
 }  // namespace internal
 }  // namespace webrtc