Enable setting the maximum bitrate limit in RtpSender.

webrtc/media/engine/webrtcvideoengine2_unittest.cc

Index: webrtc/media/engine/webrtcvideoengine2_unittest.cc
diff --git a/webrtc/media/engine/webrtcvideoengine2_unittest.cc b/webrtc/media/engine/webrtcvideoengine2_unittest.cc
index 529d77b14b3a083e4917f758bf8ceb3e345c9836..58a07c026bdca00e0798caa251f3cda4146d1568 100644
--- a/webrtc/media/engine/webrtcvideoengine2_unittest.cc
+++ b/webrtc/media/engine/webrtcvideoengine2_unittest.cc
@@ -1044,6 +1044,39 @@ class WebRtcVideoChannel2Test : public WebRtcVideoEngine2Test {
     return AddSendStream(CreateSimStreamParams("cname", ssrcs));
   }
 
+  int GetMaxEncoderBitrate(cricket::FakeVideoCapturer& capturer) {
+    EXPECT_TRUE(capturer.CaptureFrame());
+
+    std::vector<FakeVideoSendStream*> streams =
+        fake_call_->GetVideoSendStreams();
+    EXPECT_TRUE(streams.size() > 0);
+    FakeVideoSendStream* stream = streams[streams.size() - 1];
+
+    webrtc::VideoEncoderConfig encoder_config = stream->GetEncoderConfig();
+    EXPECT_EQ(1, encoder_config.streams.size());
+    return encoder_config.streams[0].max_bitrate_bps;
+  }
+
+  void VerifyEncoderBitrate(cricket::FakeVideoCapturer& capturer,

[pthatcher1, 2016/03/14 16:26:51]

Should this be called SetAndExpectMaxBandwidth?

[skvlad, 2016/03/15 21:18:18]

Done.

+                            int global_max,
+                            int stream_max,
+                            int expected_encoder_bitrate) {
+    VideoSendParameters limited_send_params = send_parameters_;
+    limited_send_params.max_bandwidth_bps = global_max;
+    EXPECT_TRUE(channel_->SetSendParameters(limited_send_params));
+    webrtc::RTCRtpParameters parameters =
+        channel_->GetRtpParameters(last_ssrc_);
+    EXPECT_EQ(1UL, parameters.encodings.size());
+    parameters.encodings[0].max_bitrate_bps = stream_max;
+    EXPECT_TRUE(channel_->SetRtpParameters(last_ssrc_, parameters));
+    // Read back the parameteres and verify they have the correct value

[Taylor Brandstetter, 2016/03/12 01:57:06]

nit: Periods at ends of comments

[skvlad, 2016/03/15 21:18:18]

Done.

+    parameters = channel_->GetRtpParameters(last_ssrc_);
+    EXPECT_EQ(1UL, parameters.encodings.size());
+    EXPECT_EQ(stream_max, parameters.encodings[0].max_bitrate_bps);
+    // Verify that the new value propagated down to the encoder
+    EXPECT_EQ(expected_encoder_bitrate, GetMaxEncoderBitrate(capturer));
+  }
+
   std::unique_ptr<FakeCall> fake_call_;
   std::unique_ptr<VideoMediaChannel> channel_;
   cricket::VideoSendParameters send_parameters_;
@@ -2981,6 +3014,63 @@ TEST_F(WebRtcVideoChannel2Test, RedRtxPacketDoesntCreateUnsignalledStream) {
   TestReceiveUnsignalledSsrcPacket(kRedRtxPayloadType, false);
 }
 
+TEST_F(WebRtcVideoChannel2Test, CanSentMaxBitrateForExistingStream) {
+  AddSendStream();
+
+  cricket::FakeVideoCapturer capturer;
+  capturer.SetScreencast(false);
+  EXPECT_TRUE(channel_->SetCapturer(last_ssrc_, &capturer));
+  cricket::VideoFormat capture_format_hd =
+      capturer.GetSupportedFormats()->front();
+  EXPECT_EQ(1280, capture_format_hd.width);
+  EXPECT_EQ(720, capture_format_hd.height);
+  EXPECT_EQ(cricket::CS_RUNNING, capturer.Start(capture_format_hd));
+  EXPECT_TRUE(channel_->SetSend(true));
+
+  int default_encoder_bitrate = GetMaxEncoderBitrate(capturer);
+  EXPECT_TRUE(default_encoder_bitrate > 1000);
+
+  // TODO(skvlad): Resolve the inconsistency between the interpretation
+  // of the global bitrate limit for audio and video:
+  // - audio: max_bandwidth_bps = 0 - fail the operation,
+  //          max_bandwidth_bps = -1 - remove the bandwidth limit
+  // - video: max_bandwidth_bps = 0 - remove the bandwidth limit,
+  //          max_bandwidth_bps = -1 - do not change the previously set
+  //                                   limit...

[pthatcher1, 2016/03/14 16:26:51]

FYI, I think we should make it:
- An Optiona<int> instead of using -1.
- Unset = remove limitation (like audio)
- 0 = don't send (like active = false)
- No way to not change, except to pass the same value as last time.

+
+  VerifyEncoderBitrate(capturer, 1000, 0, 1000);
+  VerifyEncoderBitrate(capturer, 1000, 800, 800);
+  VerifyEncoderBitrate(capturer, 600, 800, 600);
+  VerifyEncoderBitrate(capturer, 0, 800, 800);
+  VerifyEncoderBitrate(capturer, 0, 0, default_encoder_bitrate);
+
+  EXPECT_TRUE(channel_->SetCapturer(last_ssrc_, NULL));
+}
+
+TEST_F(WebRtcVideoChannel2Test, CannotSetMaxBitrateForNonexistentStream) {
+  webrtc::RTCRtpParameters nonexistent_parameters =
+      channel_->GetRtpParameters(last_ssrc_);
+  EXPECT_EQ(0, nonexistent_parameters.encodings.size());
+
+  nonexistent_parameters.encodings.push_back(
+      webrtc::RTCRtpEncodingParameters());
+  EXPECT_FALSE(channel_->SetRtpParameters(last_ssrc_, nonexistent_parameters));
+}
+
+TEST_F(WebRtcVideoChannel2Test,

[Taylor Brandstetter, 2016/03/12 01:57:06]

I'd add a "TODO: Change this when we support setting parameters for multiple
encodings". May be obvious to us, but possibly not to someone else in the
future.

[skvlad, 2016/03/15 21:18:18]

Done.

+       CannotSetRtpParametersWithIncorrectNumberOfEncodings) {
+  AddSendStream();
+  // no encodings - should fail
+  webrtc::RTCRtpParameters parameters;
+  EXPECT_FALSE(channel_->SetRtpParameters(last_ssrc_, parameters));
+  // one encoding - expected to succeed
+  parameters.encodings.push_back(webrtc::RTCRtpEncodingParameters());
+  EXPECT_TRUE(channel_->SetRtpParameters(last_ssrc_, parameters));
+  // two encodings - too many
+  parameters.encodings.push_back(webrtc::RTCRtpEncodingParameters());
+  EXPECT_FALSE(channel_->SetRtpParameters(last_ssrc_, parameters));
+}
+
 void WebRtcVideoChannel2Test::TestReceiverLocalSsrcConfiguration(
     bool receiver_first) {
   EXPECT_TRUE(channel_->SetSendParameters(send_parameters_));