From 5c7760a3a2a72b905705e003f62b60eabfda7ff2 Mon Sep 17 00:00:00 2001 From: pbos Date: Fri, 10 Mar 2017 11:23:12 -0800 Subject: [PATCH] Support removing b=AS bandwidth constraints. In code this is represented as setting -1 to max_bandwidth_bps, but this value was being ignored by webrtcvideoengine2.cc, so previous restrictions would still apply. BUG=webrtc:6202 TEST=Setting "unlimited" for Bandwidth in Chromium in https://webrtc.github.io/samples/src/content/peerconnection/bandwidth/. R=deadbeef@webrtc.org,stefan@webrtc.org Review-Url: https://codereview.webrtc.org/2740783006 Cr-Commit-Position: refs/heads/master@{#17175} --- webrtc/media/engine/webrtcvideoengine2.cc | 15 +++++++++++++-- .../media/engine/webrtcvideoengine2_unittest.cc | 7 +++---- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/webrtc/media/engine/webrtcvideoengine2.cc b/webrtc/media/engine/webrtcvideoengine2.cc index 6723b1b89f..f5682edfe6 100644 --- a/webrtc/media/engine/webrtcvideoengine2.cc +++ b/webrtc/media/engine/webrtcvideoengine2.cc @@ -715,8 +715,10 @@ bool WebRtcVideoChannel2::GetChangedSendParameters( // Handle max bitrate. if (params.max_bandwidth_bps != send_params_.max_bandwidth_bps && - params.max_bandwidth_bps >= 0) { - // 0 uncaps max bitrate (-1). + params.max_bandwidth_bps >= -1) { + // 0 or -1 uncaps max bitrate. + // TODO(pbos): Reconsider how 0 should be treated. It is not mentioned as a + // special value and might very well be used for stopping sending. changed_params->max_bandwidth_bps = rtc::Optional( params.max_bandwidth_bps == 0 ? -1 : params.max_bandwidth_bps); } @@ -760,6 +762,15 @@ bool WebRtcVideoChannel2::SetSendParameters(const VideoSendParameters& params) { } if (changed_params.codec || changed_params.max_bandwidth_bps) { + if (params.max_bandwidth_bps == -1) { + // Unset the global max bitrate (max_bitrate_bps) if max_bandwidth_bps is + // -1, which corresponds to no "b=AS" attribute in SDP. Note that the + // global max bitrate may be set below in GetBitrateConfigForCodec, from + // the codec max bitrate. + // TODO(pbos): This should be reconsidered (codec max bitrate should + // probably not affect global call max bitrate). + bitrate_config_.max_bitrate_bps = -1; + } if (send_codec_) { // TODO(holmer): Changing the codec parameters shouldn't necessarily mean // that we change the min/max of bandwidth estimation. Reevaluate this. diff --git a/webrtc/media/engine/webrtcvideoengine2_unittest.cc b/webrtc/media/engine/webrtcvideoengine2_unittest.cc index bf5537f161..47f912b196 100644 --- a/webrtc/media/engine/webrtcvideoengine2_unittest.cc +++ b/webrtc/media/engine/webrtcvideoengine2_unittest.cc @@ -2728,8 +2728,8 @@ TEST_F(WebRtcVideoChannel2Test, SetMaxSendBandwidthShouldBeRemovable) { send_parameters_.max_bandwidth_bps = 300000; EXPECT_TRUE(channel_->SetSendParameters(send_parameters_)); EXPECT_EQ(300000, fake_call_->GetConfig().bitrate_config.max_bitrate_bps); - // <= 0 means disable (infinite) max bitrate. - send_parameters_.max_bandwidth_bps = 0; + // -1 means to disable max bitrate (set infinite). + send_parameters_.max_bandwidth_bps = -1; EXPECT_TRUE(channel_->SetSendParameters(send_parameters_)); EXPECT_EQ(-1, fake_call_->GetConfig().bitrate_config.max_bitrate_bps) << "Setting zero max bitrate did not reset start bitrate."; @@ -3867,8 +3867,7 @@ TEST_F(WebRtcVideoChannel2Test, CanSentMaxBitrateForExistingStream) { // - 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. + // max_bandwidth_bps = -1 - remove the bandwidth limit SetAndExpectMaxBitrate(1000, 0, 1000); SetAndExpectMaxBitrate(1000, 800, 800);