Fixing the interaction between codec bitrate limit and "b=AS".

webrtc/media/engine/webrtcvideoengine2_unittest.cc

 /*
  *  Copyright (c) 2004 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 2372 matching lines @@
 TEST_F(WebRtcVideoChannel2Test, SetSendCodecsCapsMinAndStartBitrate) {
   SetSendCodecsShouldWorkForBitrates("-1", 0, "-100", -1, "", -1);
 }
 
 TEST_F(WebRtcVideoChannel2Test, SetSendCodecsRejectsMaxLessThanMinBitrate) {
   send_parameters_.codecs[0].params[kCodecParamMinBitrate] = "300";
   send_parameters_.codecs[0].params[kCodecParamMaxBitrate] = "200";
   EXPECT_FALSE(channel_->SetSendParameters(send_parameters_));
 }
 
+// Test that when both the codec-specific bitrate params and max_bandwidth_bps
+// are present in the same send parameters, the settings are combined correctly.
+TEST_F(WebRtcVideoChannel2Test, SetSendCodecsWithBitratesAndMaxSendBandwidth) {
+  send_parameters_.codecs[0].params[kCodecParamMinBitrate] = "100";
+  send_parameters_.codecs[0].params[kCodecParamStartBitrate] = "200";
+  send_parameters_.codecs[0].params[kCodecParamMaxBitrate] = "300";
+  send_parameters_.max_bandwidth_bps = 400000;
+  EXPECT_TRUE(channel_->SetSendParameters(send_parameters_));
+  EXPECT_EQ(100000, fake_call_->GetConfig().bitrate_config.min_bitrate_bps);
+  EXPECT_EQ(200000, fake_call_->GetConfig().bitrate_config.start_bitrate_bps);
+  // We expect the video channel to take the minimum of the maximums.
+  EXPECT_EQ(300000, fake_call_->GetConfig().bitrate_config.max_bitrate_bps);
+
+  // If we decrease max_bandwidth_bps, the codec max should still apply.
+  send_parameters_.max_bandwidth_bps = 350000;
+  EXPECT_TRUE(channel_->SetSendParameters(send_parameters_));
+  EXPECT_EQ(100000, fake_call_->GetConfig().bitrate_config.min_bitrate_bps);
+  // Since the codec isn't changing, start_bitrate_bps should be -1.
+  EXPECT_EQ(-1, fake_call_->GetConfig().bitrate_config.start_bitrate_bps);
+  // We expect the video channel to take the minimum of the maximums.
+  EXPECT_EQ(300000, fake_call_->GetConfig().bitrate_config.max_bitrate_bps);
+
+  // Now try again with the values flipped around.
+  send_parameters_.codecs[0].params[kCodecParamMaxBitrate] = "400";
+  send_parameters_.max_bandwidth_bps = 300000;
+  EXPECT_TRUE(channel_->SetSendParameters(send_parameters_));
+  EXPECT_EQ(100000, fake_call_->GetConfig().bitrate_config.min_bitrate_bps);
+  EXPECT_EQ(200000, fake_call_->GetConfig().bitrate_config.start_bitrate_bps);
+  // We expect the video channel to take the minimum of the maximums.
+  EXPECT_EQ(300000, fake_call_->GetConfig().bitrate_config.max_bitrate_bps);
+
+  // If we decrease the codec max, max_bandwidth_bps should still apply.
+  send_parameters_.codecs[0].params[kCodecParamMaxBitrate] = "350";
+  EXPECT_TRUE(channel_->SetSendParameters(send_parameters_));
+  EXPECT_EQ(100000, fake_call_->GetConfig().bitrate_config.min_bitrate_bps);
+  EXPECT_EQ(200000, fake_call_->GetConfig().bitrate_config.start_bitrate_bps);
+  // We expect the video channel to take the minimum of the maximums.
+  EXPECT_EQ(300000, fake_call_->GetConfig().bitrate_config.max_bitrate_bps);
+}
+
 TEST_F(WebRtcVideoChannel2Test,
        SetMaxSendBandwidthShouldPreserveOtherBitrates) {
-  SetSendCodecsShouldWorkForBitrates("100", 100000, "150", 150000, "200",
-                                     200000);
+  SetSendCodecsShouldWorkForBitrates("100", 100000, "150", 150000, "400",
+                                     400000);

[Taylor Brandstetter, 2016/04/21 18:44:35]

Previously, max_bandwidth_bps was allowed to increase the max above the codec
maximum. Was this intentional? This doesn't happen any more now that I'm taking
the minimum of the two maximums.

[pbos-webrtc, 2016/04/21 18:57:24]

Yep, this was intentional, since we can apply FEC above the codec target
bitrate.

[stefan-webrtc, 2016/04/27 07:24:41]

Yes, what would otherwise happen if you have two send streams, one with a max of
200 and one with a max of 400? We don't want the max_bandwidth_bps to be limited
to 200 or 400.

   send_parameters_.max_bandwidth_bps = 300000;
   EXPECT_TRUE(channel_->SetSendParameters(send_parameters_));
   EXPECT_EQ(100000, fake_call_->GetConfig().bitrate_config.min_bitrate_bps)
       << "Setting max bitrate should keep previous min bitrate.";
   EXPECT_EQ(-1, fake_call_->GetConfig().bitrate_config.start_bitrate_bps)
       << "Setting max bitrate should not reset start bitrate.";
   EXPECT_EQ(300000, fake_call_->GetConfig().bitrate_config.max_bitrate_bps);
 }
 
 TEST_F(WebRtcVideoChannel2Test, SetMaxSendBandwidthShouldBeRemovable) {
@@ skipping 1243 matching lines @@
 }
 
 // Test that we normalize send codec format size in simulcast.
 TEST_F(WebRtcVideoChannel2SimulcastTest, SetSendCodecsWithOddSizeInSimulcast) {
   cricket::VideoCodec codec(kVp8Codec270p);
   codec.width += 1;
   codec.height += 1;
   VerifySimulcastSettings(codec, 2, 2);
 }
 }  // namespace cricket