admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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

Browse Source 
This fixes a problem where "b=AS" and "x-google-start-bitrate" can't
be used together. It also starts taking the minimum of "b=AS" and
"x-google-max-bitrate", instead of just letting "b=AS" win.

BUG=webrtc:5811
R=pbos@webrtc.org

Review URL: https://codereview.webrtc.org/1904063003 .

Cr-Commit-Position: refs/heads/master@{#12519}
This commit is contained in:
Taylor Brandstetter 2016-04-26 17:15:23 -07:00
parent d6b9d772c2
commit 58f2bd90f1
2 changed files with 59 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
webrtc/media/engine/webrtcvideoengine2.cc
View File

@ -781,7 +781,7 @@ bool WebRtcVideoChannel2::GetChangedSendParameters(
}
// Handle max bitrate.
if (params.max_bandwidth_bps != bitrate_config_.max_bitrate_bps &&
if (params.max_bandwidth_bps != send_params_.max_bandwidth_bps &&
params.max_bandwidth_bps >= 0) {
// 0 uncaps max bitrate (-1).
changed_params->max_bandwidth_bps = rtc::Optional<int>(
@ -816,39 +816,38 @@ bool WebRtcVideoChannel2::SetSendParameters(const VideoSendParameters& params) {
return false;
}
bool bitrate_config_changed = false;
if (changed_params.codec) {
const VideoCodecSettings& codec_settings = *changed_params.codec;
send_codec_ = rtc::Optional<VideoCodecSettings>(codec_settings);
LOG(LS_INFO) << "Using codec: " << codec_settings.codec.ToString();
// TODO(holmer): Changing the codec parameters shouldn't necessarily mean
// that we change the min/max of bandwidth estimation. Reevaluate this.
bitrate_config_ = GetBitrateConfigForCodec(codec_settings.codec);
bitrate_config_changed = true;
}
if (changed_params.rtp_header_extensions) {
send_rtp_extensions_ = *changed_params.rtp_header_extensions;
}
if (changed_params.max_bandwidth_bps) {
// TODO(pbos): Figure out whether b=AS means max bitrate for this
// WebRtcVideoChannel2 (in which case we're good), or per sender (SSRC), in
// which case this should not set a Call::BitrateConfig but rather
// reconfigure all senders.
int max_bitrate_bps = *changed_params.max_bandwidth_bps;
bitrate_config_.start_bitrate_bps = -1;
bitrate_config_.max_bitrate_bps = max_bitrate_bps;
if (max_bitrate_bps > 0 &&
bitrate_config_.min_bitrate_bps > max_bitrate_bps) {
bitrate_config_.min_bitrate_bps = max_bitrate_bps;
if (changed_params.codec || changed_params.max_bandwidth_bps) {
if (send_codec_) {
// TODO(holmer): Changing the codec parameters shouldn't necessarily mean
// that we change the min/max of bandwidth estimation. Reevaluate this.
bitrate_config_ = GetBitrateConfigForCodec(send_codec_->codec);
if (!changed_params.codec) {
// If the codec isn't changing, set the start bitrate to -1 which means
// "unchanged" so that BWE isn't affected.
bitrate_config_.start_bitrate_bps = -1;
}
}
if (params.max_bandwidth_bps >= 0) {
// Note that max_bandwidth_bps intentionally takes priority over the
// bitrate config for the codec. This allows FEC to be applied above the
// codec target bitrate.
// TODO(pbos): Figure out whether b=AS means max bitrate for this
// WebRtcVideoChannel2 (in which case we're good), or per sender (SSRC),
// in which case this should not set a Call::BitrateConfig but rather
// reconfigure all senders.
bitrate_config_.max_bitrate_bps =
params.max_bandwidth_bps == 0 ? -1 : params.max_bandwidth_bps;
}
bitrate_config_changed = true;
}
if (bitrate_config_changed) {
call_->SetBitrateConfig(bitrate_config_);
}

37
webrtc/media/engine/webrtcvideoengine2_unittest.cc
View File

@ -2390,6 +2390,43 @@ TEST_F(WebRtcVideoChannel2Test, SetSendCodecsRejectsMaxLessThanMinBitrate) {
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 max_bandwidth_bps to take priority, if set.
EXPECT_EQ(400000, fake_call_->GetConfig().bitrate_config.max_bitrate_bps);
// Decrease max_bandwidth_bps.
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);
EXPECT_EQ(350000, 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);
EXPECT_EQ(300000, fake_call_->GetConfig().bitrate_config.max_bitrate_bps);
// If we change 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);
EXPECT_EQ(300000, fake_call_->GetConfig().bitrate_config.max_bitrate_bps);
}
TEST_F(WebRtcVideoChannel2Test,
SetMaxSendBandwidthShouldPreserveOtherBitrates) {
SetSendCodecsShouldWorkForBitrates("100", 100000, "150", 150000, "200",