Fix so that codec max bitrate doesn't override.
Currently the codec specific max bitrate that is set in the SDP
gets overridden by the value set with the "b=AS" attribute
(WebRtcVideoChannel::SetSendParameters). But at the
WebRtcVideoSendStream level it does the opposite - the codec
specific max bitrate value overrides the values that could be
set by RtpParameters or the "b=AS" value
(in WebRtcVideoSendStream::CreateVideoEncoderConfig). This change
updates the logic to be consistent with what happens at the
WebRtcVideoChannel level, and allows the RtpParameter max bitrate
to override the codec specific max bitrate.
Bug: webrtc:8655
Change-Id: I3f0347cb7cffcfc577484231b061ab0712453e69
Reviewed-on: https://webrtc-review.googlesource.com/88520
Commit-Queue: Stefan Holmer <stefan@webrtc.org>
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Reviewed-by: Magnus Jedvert <magjed@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23989}diff --git a/media/engine/webrtcvideoengine.cc b/media/engine/webrtcvideoengine.cc
index 3a10299..134fa40 100644
--- a/media/engine/webrtcvideoengine.cc
+++ b/media/engine/webrtcvideoengine.cc
@@ -1940,6 +1940,10 @@
encoder_config.number_of_streams = 1;
}
+ // parameters_.max_bitrate comes from the max bitrate set at the SDP
+ // (m-section) level with the attribute "b=AS." Note that we override this
+ // value below if the RtpParameters max bitrate set with
+ // RtpSender::SetParameters has a lower value.
int stream_max_bitrate = parameters_.max_bitrate_bps;
// When simulcast is enabled (when there are multiple encodings),
// encodings[i].max_bitrate_bps will be enforced by
@@ -1953,8 +1957,13 @@
parameters_.max_bitrate_bps);
}
+ // The codec max bitrate comes from the "x-google-max-bitrate" parameter
+ // attribute set in the SDP for a specific codec. As done in
+ // WebRtcVideoChannel::SetSendParameters, this value does not override the
+ // stream max_bitrate set above.
int codec_max_bitrate_kbps;
- if (codec.GetParam(kCodecParamMaxBitrate, &codec_max_bitrate_kbps)) {
+ if (codec.GetParam(kCodecParamMaxBitrate, &codec_max_bitrate_kbps) &&
+ stream_max_bitrate == -1) {
stream_max_bitrate = codec_max_bitrate_kbps * 1000;
}
encoder_config.max_bitrate_bps = stream_max_bitrate;
diff --git a/media/engine/webrtcvideoengine_unittest.cc b/media/engine/webrtcvideoengine_unittest.cc
index c03861a..4373ec2 100644
--- a/media/engine/webrtcvideoengine_unittest.cc
+++ b/media/engine/webrtcvideoengine_unittest.cc
@@ -3955,6 +3955,65 @@
stream->GetVideoStreams()[0].max_bitrate_bps);
}
+// Tests that when the codec specific max bitrate and VideoSendParameters
+// max_bandwidth_bps are used, that it sets the VideoStream's max bitrate
+// appropriately.
+TEST_F(WebRtcVideoChannelTest,
+ MaxBitratePrioritizesVideoSendParametersOverCodecMaxBitrate) {
+ 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 = -1;
+ AddSendStream();
+ ExpectSetMaxBitrate(300000);
+ ASSERT_TRUE(channel_->SetSendParameters(send_parameters_));
+
+ std::vector<FakeVideoSendStream*> video_send_streams = GetFakeSendStreams();
+ ASSERT_EQ(1u, video_send_streams.size());
+ FakeVideoSendStream* video_send_stream = video_send_streams[0];
+ ASSERT_EQ(1u, video_send_streams[0]->GetVideoStreams().size());
+ // First the max bitrate is set based upon the codec param.
+ EXPECT_EQ(300000,
+ video_send_streams[0]->GetVideoStreams()[0].max_bitrate_bps);
+
+ // The VideoSendParameters max bitrate overrides the codec's.
+ send_parameters_.max_bandwidth_bps = 500000;
+ ExpectSetMaxBitrate(send_parameters_.max_bandwidth_bps);
+ ASSERT_TRUE(channel_->SetSendParameters(send_parameters_));
+ ASSERT_EQ(1u, video_send_stream->GetVideoStreams().size());
+ EXPECT_EQ(500000, video_send_stream->GetVideoStreams()[0].max_bitrate_bps);
+}
+
+// Tests that when the codec specific max bitrate and RtpParameters
+// max_bitrate_bps are used, that it sets the VideoStream's max bitrate
+// appropriately.
+TEST_F(WebRtcVideoChannelTest,
+ MaxBitratePrioritizesRtpParametersOverCodecMaxBitrate) {
+ 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 = -1;
+ AddSendStream();
+ ExpectSetMaxBitrate(300000);
+ ASSERT_TRUE(channel_->SetSendParameters(send_parameters_));
+
+ std::vector<FakeVideoSendStream*> video_send_streams = GetFakeSendStreams();
+ ASSERT_EQ(1u, video_send_streams.size());
+ FakeVideoSendStream* video_send_stream = video_send_streams[0];
+ ASSERT_EQ(1u, video_send_stream->GetVideoStreams().size());
+ // First the max bitrate is set based upon the codec param.
+ EXPECT_EQ(300000, video_send_stream->GetVideoStreams()[0].max_bitrate_bps);
+
+ // The RtpParameter max bitrate overrides the codec's.
+ webrtc::RtpParameters parameters = channel_->GetRtpSendParameters(last_ssrc_);
+ ASSERT_EQ(1u, parameters.encodings.size());
+ parameters.encodings[0].max_bitrate_bps = 500000;
+ EXPECT_TRUE(channel_->SetRtpSendParameters(last_ssrc_, parameters).ok());
+ ASSERT_EQ(1u, video_send_stream->GetVideoStreams().size());
+ EXPECT_EQ(parameters.encodings[0].max_bitrate_bps,
+ video_send_stream->GetVideoStreams()[0].max_bitrate_bps);
+}
+
TEST_F(WebRtcVideoChannelTest,
MaxBitrateIsMinimumOfMaxSendBandwidthAndMaxEncodingBitrate) {
send_parameters_.max_bandwidth_bps = 99999;
