From 7c682e0c3512485bda285670e5a88f9bc41809be Mon Sep 17 00:00:00 2001
From: Seth Hampson <shampson@webrtc.org>
Date: Fri, 4 May 2018 16:28:15 -0700
Subject: [PATCH] Update to allow the application to set a low max bitrate.

A bug surfaced when setting a low max bitrate with
30kbps hard-coded min bitrate value then a DCHECK was hit in the
VideoCodecInitializer, expecting the max bitrate to be higher than the
min bitrate. This change allows the application to set a max bitrate
below 30kbps, and adjusts the min bitrate to the value set for the
max bitrate.

RtpSender: :setParameters. If the value set was lower than the
Bug: webrtc:9141
Change-Id: I9b43ee7814b1a2caba00bc9614fc66d4438d66d8
Reviewed-on: https://webrtc-review.googlesource.com/74641
Commit-Queue: Seth Hampson <shampson@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23179}
---
 media/engine/webrtcvideoengine.cc          |  6 ++++-
 media/engine/webrtcvideoengine_unittest.cc | 31 +++++++++++++++++++++-
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/media/engine/webrtcvideoengine.cc b/media/engine/webrtcvideoengine.cc
index a1f15e18c5..5932dbf048 100644
--- a/media/engine/webrtcvideoengine.cc
+++ b/media/engine/webrtcvideoengine.cc
@@ -2688,7 +2688,11 @@ std::vector<webrtc::VideoStream> EncoderStreamFactory::CreateEncoderStreams(
   layer.width = width;
   layer.height = height;
   layer.max_framerate = max_framerate_;
-  layer.min_bitrate_bps = GetMinVideoBitrateBps();
+  // The min bitrate is hardcoded, but the max_bitrate_bps is set by the
+  // application. In the case that the application sets a max bitrate
+  // that's lower than the min bitrate, we adjust it down (see
+  // bugs.webrtc.org/9141).
+  layer.min_bitrate_bps = std::min(GetMinVideoBitrateBps(), max_bitrate_bps);
   layer.target_bitrate_bps = layer.max_bitrate_bps = max_bitrate_bps;
   layer.max_qp = max_qp_;
   layer.bitrate_priority = encoder_config.bitrate_priority;
diff --git a/media/engine/webrtcvideoengine_unittest.cc b/media/engine/webrtcvideoengine_unittest.cc
index 54f64707b4..31f56568e7 100644
--- a/media/engine/webrtcvideoengine_unittest.cc
+++ b/media/engine/webrtcvideoengine_unittest.cc
@@ -5157,7 +5157,7 @@ TEST_F(WebRtcVideoChannelTest,
   EXPECT_EQ(kSsrcs3[1], recv_stream1->GetConfig().rtp.remote_ssrc);
 }
 
-TEST_F(WebRtcVideoChannelTest, CanSentMaxBitrateForExistingStream) {
+TEST_F(WebRtcVideoChannelTest, CanSetMaxBitrateForExistingStream) {
   AddSendStream();
 
   FakeVideoCapturerWithTaskQueue capturer;
@@ -5199,6 +5199,35 @@ TEST_F(WebRtcVideoChannelTest, CannotSetMaxBitrateForNonexistentStream) {
       channel_->SetRtpSendParameters(last_ssrc_, nonexistent_parameters).ok());
 }
 
+TEST_F(WebRtcVideoChannelTest,
+       SetLowMaxBitrateOverwritesVideoStreamMinBitrate) {
+  AddSendStream();
+  webrtc::RtpParameters parameters = channel_->GetRtpSendParameters(last_ssrc_);
+  EXPECT_EQ(1UL, parameters.encodings.size());
+  EXPECT_FALSE(parameters.encodings[0].max_bitrate_bps.has_value());
+  EXPECT_TRUE(channel_->SetRtpSendParameters(last_ssrc_, parameters).ok());
+
+  // Note that this is testing the behavior of the FakeVideoSendStream, which
+  // also calls to CreateEncoderStreams to get the VideoStreams, so essentially
+  // we are just testing the behavior of
+  // EncoderStreamFactory::CreateEncoderStreams.
+  std::vector<webrtc::VideoStream> video_streams =
+      fake_call_->GetVideoSendStreams().front()->GetVideoStreams();
+  ASSERT_EQ(1UL, video_streams.size());
+  EXPECT_EQ(kMinVideoBitrateBps, video_streams[0].min_bitrate_bps);
+
+  // Set a low max bitrate & check that VideoStream.min_bitrate_bps is limited
+  // by this amount.
+  parameters = channel_->GetRtpSendParameters(last_ssrc_);
+  int low_max_bitrate_bps = kMinVideoBitrateBps - 1000;
+  parameters.encodings[0].max_bitrate_bps = low_max_bitrate_bps;
+  EXPECT_TRUE(channel_->SetRtpSendParameters(last_ssrc_, parameters).ok());
+
+  video_streams = fake_call_->GetVideoSendStreams().front()->GetVideoStreams();
+  ASSERT_EQ(1UL, video_streams.size());
+  EXPECT_GE(low_max_bitrate_bps, video_streams[0].min_bitrate_bps);
+}
+
 TEST_F(WebRtcVideoChannelTest,
        CannotSetRtpSendParametersWithIncorrectNumberOfEncodings) {
   AddSendStream();