From fac0ff022bbe4f01fd37ac8b5416b1e30c5be295 Mon Sep 17 00:00:00 2001
From: noahric <noahric@chromium.org>
Date: Fri, 9 Sep 2016 10:27:15 -0700
Subject: [PATCH] Change SimulcastEncoderAdapter to allow a 0 for SetRates.

0 means "pause", so forcing it to the min bitrate means we'll never
allow pausing for internal source encoders.

BUG=

Review-Url: https://codereview.webrtc.org/2304603002
Cr-Commit-Position: refs/heads/master@{#14168}
---
 .../codecs/vp8/simulcast_encoder_adapter.cc   | 16 ++++++++-----
 .../vp8/simulcast_encoder_adapter_unittest.cc | 24 +++++++++++++++++++
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc b/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc
index 486968286d..586a6491fa 100644
--- a/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc
+++ b/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc
@@ -363,12 +363,16 @@ int SimulcastEncoderAdapter::SetRates(uint32_t new_bitrate_kbit,
   if (codec_.maxBitrate > 0 && new_bitrate_kbit > codec_.maxBitrate) {
     new_bitrate_kbit = codec_.maxBitrate;
   }
-  if (new_bitrate_kbit < codec_.minBitrate) {
-    new_bitrate_kbit = codec_.minBitrate;
-  }
-  if (codec_.numberOfSimulcastStreams > 0 &&
-      new_bitrate_kbit < codec_.simulcastStream[0].minBitrate) {
-    new_bitrate_kbit = codec_.simulcastStream[0].minBitrate;
+  if (new_bitrate_kbit > 0) {
+    // Make sure the bitrate fits the configured min bitrates. 0 is a special
+    // value that means paused, though, so leave it alone.
+    if (new_bitrate_kbit < codec_.minBitrate) {
+      new_bitrate_kbit = codec_.minBitrate;
+    }
+    if (codec_.numberOfSimulcastStreams > 0 &&
+        new_bitrate_kbit < codec_.simulcastStream[0].minBitrate) {
+      new_bitrate_kbit = codec_.simulcastStream[0].minBitrate;
+    }
   }
   codec_.maxFramerate = new_framerate;
 
diff --git a/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter_unittest.cc b/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter_unittest.cc
index d14d1a4317..47d2322d0b 100644
--- a/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter_unittest.cc
+++ b/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter_unittest.cc
@@ -133,6 +133,7 @@ class MockVideoEncoder : public VideoEncoder {
   int32_t Release() /* override */ { return 0; }
 
   int32_t SetRates(uint32_t newBitRate, uint32_t frameRate) /* override */ {
+    last_set_bitrate_ = static_cast<int32_t>(newBitRate);
     return 0;
   }
 
@@ -159,11 +160,14 @@ class MockVideoEncoder : public VideoEncoder {
   void set_supports_native_handle(bool enabled) {
     supports_native_handle_ = enabled;
   }
+  int32_t last_set_bitrate() const { return last_set_bitrate_; }
 
   MOCK_CONST_METHOD0(ImplementationName, const char*());
 
  private:
   bool supports_native_handle_ = false;
+  int32_t last_set_bitrate_ = -1;
+
   VideoCodec codec_;
   EncodedImageCallback* callback_;
 };
@@ -424,6 +428,26 @@ TEST_F(TestSimulcastEncoderAdapterFake, SupportsNativeHandleForSingleStreams) {
   EXPECT_FALSE(adapter_->SupportsNativeHandle());
 }
 
+TEST_F(TestSimulcastEncoderAdapterFake, SetRatesUnderMinBitrate) {
+  TestVp8Simulcast::DefaultSettings(
+      &codec_, static_cast<const int*>(kTestTemporalLayerProfile));
+  codec_.minBitrate = 50;
+  codec_.numberOfSimulcastStreams = 1;
+  EXPECT_EQ(0, adapter_->InitEncode(&codec_, 1, 1200));
+
+  // Above min should be respected.
+  adapter_->SetRates(100, 30);
+  EXPECT_EQ(100, helper_->factory()->encoders()[0]->last_set_bitrate());
+
+  // Below min but non-zero should be replaced with the min bitrate.
+  adapter_->SetRates(15, 30);
+  EXPECT_EQ(50, helper_->factory()->encoders()[0]->last_set_bitrate());
+
+  // Zero should be passed on as is, since it means "pause".
+  adapter_->SetRates(0, 30);
+  EXPECT_EQ(0, helper_->factory()->encoders()[0]->last_set_bitrate());
+}
+
 TEST_F(TestSimulcastEncoderAdapterFake, SupportsImplementationName) {
   EXPECT_STREQ("SimulcastEncoderAdapter", adapter_->ImplementationName());
   TestVp8Simulcast::DefaultSettings(