From 4b9798024f3a940c0c6cd6327c086bf987485a5f Mon Sep 17 00:00:00 2001 From: zstein Date: Fri, 2 Jun 2017 14:37:37 -0700 Subject: [PATCH] Relanding: Adds PeerConnectionInterface::UpdateCallBitrate to give clients more control of the bandwidth estimator. PeerConnection implements this method by passing a BitrateConfigMask to its associated Call, which is combined with the existing BitrateConfig and passed on to the SendSideCongestionController as necessary. The existing BitrateConfig generally comes from the x-google-{min,start,max}-bitrate params in the SDP. BUG=webrtc:7395 Review-Url: https://codereview.webrtc.org/2888303005 Cr-Original-Commit-Position: refs/heads/master@{#18417} Committed: https://chromium.googlesource.com/external/webrtc/+/9641c133277da0d04b78782b32391996607afa80 Review-Url: https://codereview.webrtc.org/2888303005 Cr-Commit-Position: refs/heads/master@{#18421} --- webrtc/api/peerconnectioninterface.h | 17 ++ webrtc/api/peerconnectionproxy.h | 1 + webrtc/call/call.cc | 124 +++++++-- webrtc/call/call.h | 32 ++- webrtc/call/call_unittest.cc | 240 +++++++++++++++++- webrtc/media/engine/fakewebrtccall.cc | 5 + webrtc/media/engine/fakewebrtccall.h | 2 + webrtc/pc/peerconnection.cc | 48 ++++ webrtc/pc/peerconnection.h | 2 + webrtc/pc/peerconnectioninterface_unittest.cc | 63 +++++ 10 files changed, 497 insertions(+), 37 deletions(-) diff --git a/webrtc/api/peerconnectioninterface.h b/webrtc/api/peerconnectioninterface.h index 8c5a64bf1e..8ff7e1e469 100644 --- a/webrtc/api/peerconnectioninterface.h +++ b/webrtc/api/peerconnectioninterface.h @@ -729,6 +729,23 @@ class PeerConnectionInterface : public rtc::RefCountInterface { // destroyed, RegisterUMAOberver(nullptr) should be called. virtual void RegisterUMAObserver(UMAObserver* observer) = 0; + // 0 <= min <= current <= max should hold for set parameters. + struct BitrateParameters { + rtc::Optional min_bitrate_bps; + rtc::Optional current_bitrate_bps; + rtc::Optional max_bitrate_bps; + }; + + // SetBitrate limits the bandwidth allocated for all RTP streams sent by + // this PeerConnection. Other limitations might affect these limits and + // are respected (for example "b=AS" in SDP). + // + // Setting |current_bitrate_bps| will reset the current bitrate estimate + // to the provided value. + virtual RTCError SetBitrate(const BitrateParameters& bitrate) { + return RTCError::OK(); + } + // Returns the current SignalingState. virtual SignalingState signaling_state() = 0; virtual IceConnectionState ice_connection_state() = 0; diff --git a/webrtc/api/peerconnectionproxy.h b/webrtc/api/peerconnectionproxy.h index 2375dd4dce..c68bee6afd 100644 --- a/webrtc/api/peerconnectionproxy.h +++ b/webrtc/api/peerconnectionproxy.h @@ -100,6 +100,7 @@ BEGIN_SIGNALING_PROXY_MAP(PeerConnection) RemoveIceCandidates, const std::vector&); PROXY_METHOD1(void, RegisterUMAObserver, UMAObserver*) + PROXY_METHOD1(RTCError, SetBitrate, const BitrateParameters&); PROXY_METHOD0(SignalingState, signaling_state) PROXY_METHOD0(IceConnectionState, ice_connection_state) PROXY_METHOD0(IceGatheringState, ice_gathering_state) diff --git a/webrtc/call/call.cc b/webrtc/call/call.cc index 6a8cd14e7a..f31e11479b 100644 --- a/webrtc/call/call.cc +++ b/webrtc/call/call.cc @@ -200,6 +200,9 @@ class Call : public webrtc::Call, void SetBitrateConfig( const webrtc::Call::Config::BitrateConfig& bitrate_config) override; + void SetBitrateConfigMask( + const webrtc::Call::Config::BitrateConfigMask& bitrate_config) override; + void SignalChannelNetworkState(MediaType media, NetworkState state) override; void OnTransportOverheadChanged(MediaType media, @@ -246,6 +249,10 @@ class Call : public webrtc::Call, void UpdateHistograms(); void UpdateAggregateNetworkState(); + // Applies update to the BitrateConfig cached in |config_|, restarting + // bandwidth estimation from |new_start| if set. + void UpdateCurrentBitrateConfig(const rtc::Optional& new_start); + Clock* const clock_; const int num_cpu_cores_; @@ -341,6 +348,14 @@ class Call : public webrtc::Call, // and deleted before any other members. rtc::TaskQueue worker_queue_; + // The config mask set by SetBitrateConfigMask. + // 0 <= min <= start <= max + Config::BitrateConfigMask bitrate_config_mask_; + + // The config set by SetBitrateConfig. + // min >= 0, start != 0, max == -1 || max > 0 + Config::BitrateConfig base_bitrate_config_; + RTC_DISALLOW_COPY_AND_ASSIGN(Call); }; } // namespace internal @@ -396,8 +411,9 @@ Call::Call(const Call::Config& config, receive_side_cc_(clock_, transport_send->packet_router()), video_send_delay_stats_(new SendDelayStats(clock_)), start_ms_(clock_->TimeInMilliseconds()), - worker_queue_("call_worker_queue") { - RTC_DCHECK_RUN_ON(&configuration_thread_checker_); + worker_queue_("call_worker_queue"), + base_bitrate_config_(config.bitrate_config) { + RTC_DCHECK(&configuration_thread_checker_); RTC_DCHECK(config.event_log != nullptr); RTC_DCHECK_GE(config.bitrate_config.min_bitrate_bps, 0); RTC_DCHECK_GE(config.bitrate_config.start_bitrate_bps, @@ -905,28 +921,94 @@ void Call::SetBitrateConfig( TRACE_EVENT0("webrtc", "Call::SetBitrateConfig"); RTC_DCHECK_RUN_ON(&configuration_thread_checker_); RTC_DCHECK_GE(bitrate_config.min_bitrate_bps, 0); - if (bitrate_config.max_bitrate_bps != -1) + RTC_DCHECK_NE(bitrate_config.start_bitrate_bps, 0); + if (bitrate_config.max_bitrate_bps != -1) { RTC_DCHECK_GT(bitrate_config.max_bitrate_bps, 0); - if (config_.bitrate_config.min_bitrate_bps == - bitrate_config.min_bitrate_bps && - (bitrate_config.start_bitrate_bps <= 0 || - config_.bitrate_config.start_bitrate_bps == - bitrate_config.start_bitrate_bps) && - config_.bitrate_config.max_bitrate_bps == - bitrate_config.max_bitrate_bps) { - // Nothing new to set, early abort to avoid encoder reconfigurations. + } + + rtc::Optional new_start; + // Only update the "start" bitrate if it's set, and different from the old + // value. In practice, this value comes from the x-google-start-bitrate codec + // parameter in SDP, and setting the same remote description twice shouldn't + // restart bandwidth estimation. + if (bitrate_config.start_bitrate_bps != -1 && + bitrate_config.start_bitrate_bps != + base_bitrate_config_.start_bitrate_bps) { + new_start.emplace(bitrate_config.start_bitrate_bps); + } + base_bitrate_config_ = bitrate_config; + UpdateCurrentBitrateConfig(new_start); +} + +void Call::SetBitrateConfigMask( + const webrtc::Call::Config::BitrateConfigMask& mask) { + TRACE_EVENT0("webrtc", "Call::SetBitrateConfigMask"); + RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread()); + + bitrate_config_mask_ = mask; + UpdateCurrentBitrateConfig(mask.start_bitrate_bps); +} + +namespace { + +static int MinPositive(int a, int b) { + if (a <= 0) { + return b; + } + if (b <= 0) { + return a; + } + return std::min(a, b); +} + +} // namespace + +void Call::UpdateCurrentBitrateConfig(const rtc::Optional& new_start) { + Config::BitrateConfig updated; + updated.min_bitrate_bps = + std::max(bitrate_config_mask_.min_bitrate_bps.value_or(0), + base_bitrate_config_.min_bitrate_bps); + + updated.max_bitrate_bps = + MinPositive(bitrate_config_mask_.max_bitrate_bps.value_or(-1), + base_bitrate_config_.max_bitrate_bps); + + // If the combined min ends up greater than the combined max, the max takes + // priority. + if (updated.max_bitrate_bps != -1 && + updated.min_bitrate_bps > updated.max_bitrate_bps) { + updated.min_bitrate_bps = updated.max_bitrate_bps; + } + + // If there is nothing to update (min/max unchanged, no new bandwidth + // estimation start value), return early. + if (updated.min_bitrate_bps == config_.bitrate_config.min_bitrate_bps && + updated.max_bitrate_bps == config_.bitrate_config.max_bitrate_bps && + !new_start) { + LOG(LS_VERBOSE) << "WebRTC.Call.UpdateCurrentBitrateConfig: " + << "nothing to update"; return; } - config_.bitrate_config.min_bitrate_bps = bitrate_config.min_bitrate_bps; - // Start bitrate of -1 means we should keep the old bitrate, which there is - // no point in remembering for the future. - if (bitrate_config.start_bitrate_bps > 0) - config_.bitrate_config.start_bitrate_bps = bitrate_config.start_bitrate_bps; - config_.bitrate_config.max_bitrate_bps = bitrate_config.max_bitrate_bps; - RTC_DCHECK_NE(bitrate_config.start_bitrate_bps, 0); - transport_send_->send_side_cc()->SetBweBitrates( - bitrate_config.min_bitrate_bps, bitrate_config.start_bitrate_bps, - bitrate_config.max_bitrate_bps); + + if (new_start) { + // Clamp start by min and max. + updated.start_bitrate_bps = MinPositive( + std::max(*new_start, updated.min_bitrate_bps), updated.max_bitrate_bps); + } else { + updated.start_bitrate_bps = -1; + } + + LOG(INFO) << "WebRTC.Call.UpdateCurrentBitrateConfig: " + << "calling SetBweBitrates with args (" << updated.min_bitrate_bps + << ", " << updated.start_bitrate_bps << ", " + << updated.max_bitrate_bps << ")"; + transport_send_->send_side_cc()->SetBweBitrates(updated.min_bitrate_bps, + updated.start_bitrate_bps, + updated.max_bitrate_bps); + if (!new_start) { + updated.start_bitrate_bps = config_.bitrate_config.start_bitrate_bps; + } + config_.bitrate_config = updated; } void Call::SignalChannelNetworkState(MediaType media, NetworkState state) { diff --git a/webrtc/call/call.h b/webrtc/call/call.h index f67b6907e5..06479890ce 100644 --- a/webrtc/call/call.h +++ b/webrtc/call/call.h @@ -14,6 +14,7 @@ #include #include +#include "webrtc/api/rtcerror.h" #include "webrtc/base/networkroute.h" #include "webrtc/base/platform_file.h" #include "webrtc/base/socket.h" @@ -68,13 +69,24 @@ class Call { static const int kDefaultStartBitrateBps; // Bitrate config used until valid bitrate estimates are calculated. Also - // used to cap total bitrate used. + // used to cap total bitrate used. This comes from the remote connection. struct BitrateConfig { int min_bitrate_bps = 0; int start_bitrate_bps = kDefaultStartBitrateBps; int max_bitrate_bps = -1; } bitrate_config; + // The local client's bitrate preferences. The actual configuration used + // is a combination of this and |bitrate_config|. The combination is + // currently more complicated than a simple mask operation (see + // SetBitrateConfig and SetBitrateConfigMask). Assumes that 0 <= min <= + // start <= max holds for set parameters. + struct BitrateConfigMask { + rtc::Optional min_bitrate_bps; + rtc::Optional start_bitrate_bps; + rtc::Optional max_bitrate_bps; + }; + // AudioState which is possibly shared between multiple calls. // TODO(solenberg): Change this to a shared_ptr once we can use C++11. rtc::scoped_refptr audio_state; @@ -141,14 +153,22 @@ class Call { // pacing delay, etc. virtual Stats GetStats() const = 0; - // TODO(pbos): Like BitrateConfig above this is currently per-stream instead - // of maximum for entire Call. This should be fixed along with the above. - // Specifying a start bitrate (>0) will currently reset the current bitrate - // estimate. This is due to how the 'x-google-start-bitrate' flag is currently - // implemented. + // The greater min and smaller max set by this and SetBitrateConfigMask will + // be used. The latest non-negative start value from either call will be used. + // Specifying a start bitrate (>0) will reset the current bitrate estimate. + // This is due to how the 'x-google-start-bitrate' flag is currently + // implemented. Passing -1 leaves the start bitrate unchanged. Behavior is not + // guaranteed for other negative values or 0. virtual void SetBitrateConfig( const Config::BitrateConfig& bitrate_config) = 0; + // The greater min and smaller max set by this and SetBitrateConfig will be + // used. The latest non-negative start value form either call will be used. + // Specifying a start bitrate will reset the current bitrate estimate. + // Assumes 0 <= min <= start <= max holds for set parameters. + virtual void SetBitrateConfigMask( + const Config::BitrateConfigMask& bitrate_mask) = 0; + // TODO(skvlad): When the unbundled case with multiple streams for the same // media type going over different networks is supported, track the state // for each stream separately. Right now it's global per media type. diff --git a/webrtc/call/call_unittest.cc b/webrtc/call/call_unittest.cc index 06ee2005b1..8f0a340e72 100644 --- a/webrtc/call/call_unittest.cc +++ b/webrtc/call/call_unittest.cc @@ -320,14 +320,16 @@ TEST(CallTest, MultipleFlexfecReceiveStreamsProtectingSingleVideoStream) { namespace { struct CallBitrateHelper { - CallBitrateHelper() : CallBitrateHelper(Call::Config(&event_log_)) {} + CallBitrateHelper() : CallBitrateHelper(Call::Config::BitrateConfig()) {} - explicit CallBitrateHelper(const Call::Config& config) - : mock_cc_(Clock::GetRealTimeClock(), &event_log_, &packet_router_), - call_(Call::Create( - config, - rtc::MakeUnique(&packet_router_, - &mock_cc_))) {} + explicit CallBitrateHelper(const Call::Config::BitrateConfig& bitrate_config) + : mock_cc_(Clock::GetRealTimeClock(), &event_log_, &packet_router_) { + Call::Config config(&event_log_); + config.bitrate_config = bitrate_config; + call_.reset( + Call::Create(config, rtc::MakeUnique( + &packet_router_, &mock_cc_))); + } webrtc::Call* operator->() { return call_.get(); } testing::NiceMock& mock_cc() { @@ -364,7 +366,7 @@ TEST(CallBitrateTest, SetBitrateConfigWithDifferentMinCallsSetBweBitrates) { call->SetBitrateConfig(bitrate_config); bitrate_config.min_bitrate_bps = 11; - EXPECT_CALL(call.mock_cc(), SetBweBitrates(11, 20, 30)); + EXPECT_CALL(call.mock_cc(), SetBweBitrates(11, -1, 30)); call->SetBitrateConfig(bitrate_config); } @@ -392,13 +394,12 @@ TEST(CallBitrateTest, SetBitrateConfigWithDifferentMaxCallsSetBweBitrates) { call->SetBitrateConfig(bitrate_config); bitrate_config.max_bitrate_bps = 31; - EXPECT_CALL(call.mock_cc(), SetBweBitrates(10, 20, 31)); + EXPECT_CALL(call.mock_cc(), SetBweBitrates(10, -1, 31)); call->SetBitrateConfig(bitrate_config); } TEST(CallBitrateTest, SetBitrateConfigWithSameConfigElidesSecondCall) { CallBitrateHelper call; - Call::Config::BitrateConfig bitrate_config; bitrate_config.min_bitrate_bps = 1; bitrate_config.start_bitrate_bps = 2; @@ -489,5 +490,224 @@ TEST(CallTest, RecreatingAudioStreamWithSameSsrcReusesRtpState) { rtp_state2.last_timestamp_time_ms); EXPECT_EQ(rtp_state1.media_has_been_sent, rtp_state2.media_has_been_sent); } +TEST(CallBitrateTest, BiggerMaskMinUsed) { + CallBitrateHelper call; + Call::Config::BitrateConfigMask mask; + mask.min_bitrate_bps = rtc::Optional(1234); + + EXPECT_CALL(call.mock_cc(), + SetBweBitrates(*mask.min_bitrate_bps, testing::_, testing::_)); + call->SetBitrateConfigMask(mask); +} + +TEST(CallBitrateTest, BiggerConfigMinUsed) { + CallBitrateHelper call; + Call::Config::BitrateConfigMask mask; + mask.min_bitrate_bps = rtc::Optional(1000); + EXPECT_CALL(call.mock_cc(), SetBweBitrates(1000, testing::_, testing::_)); + call->SetBitrateConfigMask(mask); + + Call::Config::BitrateConfig config; + config.min_bitrate_bps = 1234; + + EXPECT_CALL(call.mock_cc(), SetBweBitrates(1234, testing::_, testing::_)); + call->SetBitrateConfig(config); +} + +// The last call to set start should be used. +TEST(CallBitrateTest, LatestStartMaskPreferred) { + CallBitrateHelper call; + Call::Config::BitrateConfigMask mask; + mask.start_bitrate_bps = rtc::Optional(1300); + + EXPECT_CALL(call.mock_cc(), + SetBweBitrates(testing::_, *mask.start_bitrate_bps, testing::_)); + call->SetBitrateConfigMask(mask); + + Call::Config::BitrateConfig bitrate_config; + bitrate_config.start_bitrate_bps = 1200; + + EXPECT_CALL( + call.mock_cc(), + SetBweBitrates(testing::_, bitrate_config.start_bitrate_bps, testing::_)); + call->SetBitrateConfig(bitrate_config); +} + +TEST(CallBitrateTest, SmallerMaskMaxUsed) { + Call::Config::BitrateConfig bitrate_config; + bitrate_config.max_bitrate_bps = bitrate_config.start_bitrate_bps + 2000; + CallBitrateHelper call(bitrate_config); + + Call::Config::BitrateConfigMask mask; + mask.max_bitrate_bps = + rtc::Optional(bitrate_config.start_bitrate_bps + 1000); + + EXPECT_CALL(call.mock_cc(), + SetBweBitrates(testing::_, testing::_, *mask.max_bitrate_bps)); + call->SetBitrateConfigMask(mask); +} + +TEST(CallBitrateTest, SmallerConfigMaxUsed) { + Call::Config::BitrateConfig bitrate_config; + bitrate_config.max_bitrate_bps = bitrate_config.start_bitrate_bps + 1000; + CallBitrateHelper call(bitrate_config); + + Call::Config::BitrateConfigMask mask; + mask.max_bitrate_bps = + rtc::Optional(bitrate_config.start_bitrate_bps + 2000); + + // Expect no calls because nothing changes + EXPECT_CALL(call.mock_cc(), + SetBweBitrates(testing::_, testing::_, testing::_)) + .Times(0); + call->SetBitrateConfigMask(mask); +} + +TEST(CallBitrateTest, MaskStartLessThanConfigMinClamped) { + Call::Config::BitrateConfig bitrate_config; + bitrate_config.min_bitrate_bps = 2000; + CallBitrateHelper call(bitrate_config); + + Call::Config::BitrateConfigMask mask; + mask.start_bitrate_bps = rtc::Optional(1000); + + EXPECT_CALL(call.mock_cc(), SetBweBitrates(2000, 2000, testing::_)); + call->SetBitrateConfigMask(mask); +} + +TEST(CallBitrateTest, MaskStartGreaterThanConfigMaxClamped) { + Call::Config::BitrateConfig bitrate_config; + bitrate_config.start_bitrate_bps = 2000; + CallBitrateHelper call(bitrate_config); + + Call::Config::BitrateConfigMask mask; + mask.max_bitrate_bps = rtc::Optional(1000); + + EXPECT_CALL(call.mock_cc(), SetBweBitrates(testing::_, -1, 1000)); + call->SetBitrateConfigMask(mask); +} + +TEST(CallBitrateTest, MaskMinGreaterThanConfigMaxClamped) { + Call::Config::BitrateConfig bitrate_config; + bitrate_config.min_bitrate_bps = 2000; + CallBitrateHelper call(bitrate_config); + + Call::Config::BitrateConfigMask mask; + mask.max_bitrate_bps = rtc::Optional(1000); + + EXPECT_CALL(call.mock_cc(), SetBweBitrates(1000, testing::_, 1000)); + call->SetBitrateConfigMask(mask); +} + +TEST(CallBitrateTest, SettingMaskStartForcesUpdate) { + CallBitrateHelper call; + + Call::Config::BitrateConfigMask mask; + mask.start_bitrate_bps = rtc::Optional(1000); + + // SetBweBitrates should be called twice with the same params since + // start_bitrate_bps is set. + EXPECT_CALL(call.mock_cc(), SetBweBitrates(testing::_, 1000, testing::_)) + .Times(2); + call->SetBitrateConfigMask(mask); + call->SetBitrateConfigMask(mask); +} + +TEST(CallBitrateTest, SetBitrateConfigWithNoChangesDoesNotCallSetBweBitrates) { + CallBitrateHelper call; + + Call::Config::BitrateConfig config1; + config1.min_bitrate_bps = 0; + config1.start_bitrate_bps = 1000; + config1.max_bitrate_bps = -1; + + Call::Config::BitrateConfig config2; + config2.min_bitrate_bps = 0; + config2.start_bitrate_bps = -1; + config2.max_bitrate_bps = -1; + + // The second call should not call SetBweBitrates because it doesn't + // change any values. + EXPECT_CALL(call.mock_cc(), SetBweBitrates(0, 1000, -1)); + call->SetBitrateConfig(config1); + call->SetBitrateConfig(config2); +} + +// If SetBitrateConfig changes the max, but not the effective max, +// SetBweBitrates shouldn't be called, to avoid unnecessary encoder +// reconfigurations. +TEST(CallBitrateTest, SetBweBitratesNotCalledWhenEffectiveMaxUnchanged) { + CallBitrateHelper call; + + Call::Config::BitrateConfig config; + config.min_bitrate_bps = 0; + config.start_bitrate_bps = -1; + config.max_bitrate_bps = 2000; + EXPECT_CALL(call.mock_cc(), SetBweBitrates(testing::_, testing::_, 2000)); + call->SetBitrateConfig(config); + + // Reduce effective max to 1000 with the mask. + Call::Config::BitrateConfigMask mask; + mask.max_bitrate_bps = rtc::Optional(1000); + EXPECT_CALL(call.mock_cc(), SetBweBitrates(testing::_, testing::_, 1000)); + call->SetBitrateConfigMask(mask); + + // This leaves the effective max unchanged, so SetBweBitrates shouldn't be + // called again. + config.max_bitrate_bps = 1000; + call->SetBitrateConfig(config); +} + +// When the "start bitrate" mask is removed, SetBweBitrates shouldn't be called +// again, since nothing's changing. +TEST(CallBitrateTest, SetBweBitratesNotCalledWhenStartMaskRemoved) { + CallBitrateHelper call; + + Call::Config::BitrateConfigMask mask; + mask.start_bitrate_bps = rtc::Optional(1000); + EXPECT_CALL(call.mock_cc(), SetBweBitrates(0, 1000, -1)); + call->SetBitrateConfigMask(mask); + + mask.start_bitrate_bps.reset(); + call->SetBitrateConfigMask(mask); +} + +// Test that if SetBitrateConfig is called after SetBitrateConfigMask applies a +// "start" value, the SetBitrateConfig call won't apply that start value a +// second time. +TEST(CallBitrateTest, SetBitrateConfigAfterSetBitrateConfigMaskWithStart) { + CallBitrateHelper call; + + Call::Config::BitrateConfigMask mask; + mask.start_bitrate_bps = rtc::Optional(1000); + EXPECT_CALL(call.mock_cc(), SetBweBitrates(0, 1000, -1)); + call->SetBitrateConfigMask(mask); + + Call::Config::BitrateConfig config; + config.min_bitrate_bps = 0; + config.start_bitrate_bps = -1; + config.max_bitrate_bps = 5000; + // The start value isn't changing, so SetBweBitrates should be called with + // -1. + EXPECT_CALL(call.mock_cc(), SetBweBitrates(0, -1, 5000)); + call->SetBitrateConfig(config); +} + +TEST(CallBitrateTest, SetBweBitratesNotCalledWhenClampedMinUnchanged) { + Call::Config::BitrateConfig bitrate_config; + bitrate_config.start_bitrate_bps = 500; + bitrate_config.max_bitrate_bps = 1000; + CallBitrateHelper call(bitrate_config); + + // Set min to 2000; it is clamped to the max (1000). + Call::Config::BitrateConfigMask mask; + mask.min_bitrate_bps = rtc::Optional(2000); + EXPECT_CALL(call.mock_cc(), SetBweBitrates(1000, -1, 1000)); + call->SetBitrateConfigMask(mask); + + // Set min to 3000; the clamped value stays the same so nothing happens. + mask.min_bitrate_bps = rtc::Optional(3000); + call->SetBitrateConfigMask(mask); +} } // namespace webrtc diff --git a/webrtc/media/engine/fakewebrtccall.cc b/webrtc/media/engine/fakewebrtccall.cc index d323ef6806..ec8f6b8f16 100644 --- a/webrtc/media/engine/fakewebrtccall.cc +++ b/webrtc/media/engine/fakewebrtccall.cc @@ -588,6 +588,11 @@ void FakeCall::SetBitrateConfig( config_.bitrate_config = bitrate_config; } +void FakeCall::SetBitrateConfigMask( + const webrtc::Call::Config::BitrateConfigMask& mask) { + // TODO(zstein): not implemented +} + void FakeCall::SignalChannelNetworkState(webrtc::MediaType media, webrtc::NetworkState state) { switch (media) { diff --git a/webrtc/media/engine/fakewebrtccall.h b/webrtc/media/engine/fakewebrtccall.h index 77cae6b0eb..34db106c43 100644 --- a/webrtc/media/engine/fakewebrtccall.h +++ b/webrtc/media/engine/fakewebrtccall.h @@ -293,6 +293,8 @@ class FakeCall final : public webrtc::Call, public webrtc::PacketReceiver { void SetBitrateConfig( const webrtc::Call::Config::BitrateConfig& bitrate_config) override; + void SetBitrateConfigMask( + const webrtc::Call::Config::BitrateConfigMask& mask) override; void OnNetworkRouteChanged(const std::string& transport_name, const rtc::NetworkRoute& network_route) override {} void SignalChannelNetworkState(webrtc::MediaType media, diff --git a/webrtc/pc/peerconnection.cc b/webrtc/pc/peerconnection.cc index 852dd39de5..7111b1c53d 100644 --- a/webrtc/pc/peerconnection.cc +++ b/webrtc/pc/peerconnection.cc @@ -1242,6 +1242,54 @@ void PeerConnection::RegisterUMAObserver(UMAObserver* observer) { } } +RTCError PeerConnection::SetBitrate(const BitrateParameters& bitrate) { + rtc::Thread* worker_thread = factory_->worker_thread(); + if (!worker_thread->IsCurrent()) { + return worker_thread->Invoke( + RTC_FROM_HERE, rtc::Bind(&PeerConnection::SetBitrate, this, bitrate)); + } + + const bool has_min = static_cast(bitrate.min_bitrate_bps); + const bool has_current = static_cast(bitrate.current_bitrate_bps); + const bool has_max = static_cast(bitrate.max_bitrate_bps); + if (has_min && *bitrate.min_bitrate_bps < 0) { + LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, + "min_bitrate_bps <= 0"); + } + if (has_current) { + if (has_min && *bitrate.current_bitrate_bps < *bitrate.min_bitrate_bps) { + LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, + "current_bitrate_bps < min_bitrate_bps"); + } else if (*bitrate.current_bitrate_bps < 0) { + LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, + "curent_bitrate_bps < 0"); + } + } + if (has_max) { + if (has_current && + *bitrate.max_bitrate_bps < *bitrate.current_bitrate_bps) { + LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, + "max_bitrate_bps < current_bitrate_bps"); + } else if (has_min && *bitrate.max_bitrate_bps < *bitrate.min_bitrate_bps) { + LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, + "max_bitrate_bps < min_bitrate_bps"); + } else if (*bitrate.max_bitrate_bps < 0) { + LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, + "max_bitrate_bps < 0"); + } + } + + Call::Config::BitrateConfigMask mask; + mask.min_bitrate_bps = bitrate.min_bitrate_bps; + mask.start_bitrate_bps = bitrate.current_bitrate_bps; + mask.max_bitrate_bps = bitrate.max_bitrate_bps; + + RTC_DCHECK(call_.get()); + call_->SetBitrateConfigMask(mask); + + return RTCError::OK(); +} + bool PeerConnection::StartRtcEventLog(rtc::PlatformFile file, int64_t max_size_bytes) { return factory_->worker_thread()->Invoke( diff --git a/webrtc/pc/peerconnection.h b/webrtc/pc/peerconnection.h index 1e2ca3bb95..289a5d87a7 100644 --- a/webrtc/pc/peerconnection.h +++ b/webrtc/pc/peerconnection.h @@ -144,6 +144,8 @@ class PeerConnection : public PeerConnectionInterface, void RegisterUMAObserver(UMAObserver* observer) override; + RTCError SetBitrate(const BitrateParameters& bitrate) override; + bool StartRtcEventLog(rtc::PlatformFile file, int64_t max_size_bytes) override; void StopRtcEventLog() override; diff --git a/webrtc/pc/peerconnectioninterface_unittest.cc b/webrtc/pc/peerconnectioninterface_unittest.cc index d0e42621cf..f8d9ef6020 100644 --- a/webrtc/pc/peerconnectioninterface_unittest.cc +++ b/webrtc/pc/peerconnectioninterface_unittest.cc @@ -3301,6 +3301,69 @@ TEST_F(PeerConnectionInterfaceTest, EXPECT_TRUE(DoSetLocalDescription(answer.release())); } +TEST_F(PeerConnectionInterfaceTest, SetBitrateWithoutMinSucceeds) { + CreatePeerConnection(); + PeerConnectionInterface::BitrateParameters bitrate; + bitrate.current_bitrate_bps = rtc::Optional(100000); + EXPECT_TRUE(pc_->SetBitrate(bitrate).ok()); +} + +TEST_F(PeerConnectionInterfaceTest, SetBitrateNegativeMinFails) { + CreatePeerConnection(); + PeerConnectionInterface::BitrateParameters bitrate; + bitrate.min_bitrate_bps = rtc::Optional(-1); + EXPECT_FALSE(pc_->SetBitrate(bitrate).ok()); +} + +TEST_F(PeerConnectionInterfaceTest, SetBitrateCurrentLessThanMinFails) { + CreatePeerConnection(); + PeerConnectionInterface::BitrateParameters bitrate; + bitrate.min_bitrate_bps = rtc::Optional(5); + bitrate.current_bitrate_bps = rtc::Optional(3); + EXPECT_FALSE(pc_->SetBitrate(bitrate).ok()); +} + +TEST_F(PeerConnectionInterfaceTest, SetBitrateCurrentNegativeFails) { + CreatePeerConnection(); + PeerConnectionInterface::BitrateParameters bitrate; + bitrate.current_bitrate_bps = rtc::Optional(-1); + EXPECT_FALSE(pc_->SetBitrate(bitrate).ok()); +} + +TEST_F(PeerConnectionInterfaceTest, SetBitrateMaxLessThanCurrentFails) { + CreatePeerConnection(); + PeerConnectionInterface::BitrateParameters bitrate; + bitrate.current_bitrate_bps = rtc::Optional(10); + bitrate.max_bitrate_bps = rtc::Optional(8); + EXPECT_FALSE(pc_->SetBitrate(bitrate).ok()); +} + +TEST_F(PeerConnectionInterfaceTest, SetBitrateMaxLessThanMinFails) { + CreatePeerConnection(); + PeerConnectionInterface::BitrateParameters bitrate; + bitrate.min_bitrate_bps = rtc::Optional(10); + bitrate.max_bitrate_bps = rtc::Optional(8); + EXPECT_FALSE(pc_->SetBitrate(bitrate).ok()); +} + +TEST_F(PeerConnectionInterfaceTest, SetBitrateMaxNegativeFails) { + CreatePeerConnection(); + PeerConnectionInterface::BitrateParameters bitrate; + bitrate.max_bitrate_bps = rtc::Optional(-1); + EXPECT_FALSE(pc_->SetBitrate(bitrate).ok()); +} + +// The current bitrate from Call's BitrateConfigMask is currently clamped by +// Call's BitrateConfig, which comes from the SDP or a default value. This test +// checks that a call to SetBitrate with a current bitrate that will be clamped +// succeeds. +TEST_F(PeerConnectionInterfaceTest, SetBitrateCurrentLessThanImplicitMin) { + CreatePeerConnection(); + PeerConnectionInterface::BitrateParameters bitrate; + bitrate.current_bitrate_bps = rtc::Optional(1); + EXPECT_TRUE(pc_->SetBitrate(bitrate).ok()); +} + class PeerConnectionMediaConfigTest : public testing::Test { protected: void SetUp() override {