diff --git a/webrtc/api/peerconnectioninterface.h b/webrtc/api/peerconnectioninterface.h index 64f5b2da6e..8c5a64bf1e 100644 --- a/webrtc/api/peerconnectioninterface.h +++ b/webrtc/api/peerconnectioninterface.h @@ -729,21 +729,6 @@ 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) = 0; - // 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 c68bee6afd..2375dd4dce 100644 --- a/webrtc/api/peerconnectionproxy.h +++ b/webrtc/api/peerconnectionproxy.h @@ -100,7 +100,6 @@ 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 f31e11479b..6a8cd14e7a 100644 --- a/webrtc/call/call.cc +++ b/webrtc/call/call.cc @@ -200,9 +200,6 @@ 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, @@ -249,10 +246,6 @@ 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_; @@ -348,14 +341,6 @@ 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 @@ -411,9 +396,8 @@ 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"), - base_bitrate_config_(config.bitrate_config) { - RTC_DCHECK(&configuration_thread_checker_); + worker_queue_("call_worker_queue") { + RTC_DCHECK_RUN_ON(&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, @@ -921,94 +905,28 @@ void Call::SetBitrateConfig( TRACE_EVENT0("webrtc", "Call::SetBitrateConfig"); RTC_DCHECK_RUN_ON(&configuration_thread_checker_); RTC_DCHECK_GE(bitrate_config.min_bitrate_bps, 0); - RTC_DCHECK_NE(bitrate_config.start_bitrate_bps, 0); - if (bitrate_config.max_bitrate_bps != -1) { + if (bitrate_config.max_bitrate_bps != -1) RTC_DCHECK_GT(bitrate_config.max_bitrate_bps, 0); - } - - 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"; + 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. return; } - - 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; + 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); } void Call::SignalChannelNetworkState(MediaType media, NetworkState state) { diff --git a/webrtc/call/call.h b/webrtc/call/call.h index 06479890ce..f67b6907e5 100644 --- a/webrtc/call/call.h +++ b/webrtc/call/call.h @@ -14,7 +14,6 @@ #include #include -#include "webrtc/api/rtcerror.h" #include "webrtc/base/networkroute.h" #include "webrtc/base/platform_file.h" #include "webrtc/base/socket.h" @@ -69,24 +68,13 @@ class Call { static const int kDefaultStartBitrateBps; // Bitrate config used until valid bitrate estimates are calculated. Also - // used to cap total bitrate used. This comes from the remote connection. + // used to cap total bitrate used. 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; @@ -153,22 +141,14 @@ class Call { // pacing delay, etc. virtual Stats GetStats() const = 0; - // 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. + // 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. 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 8f0a340e72..06ee2005b1 100644 --- a/webrtc/call/call_unittest.cc +++ b/webrtc/call/call_unittest.cc @@ -320,16 +320,14 @@ TEST(CallTest, MultipleFlexfecReceiveStreamsProtectingSingleVideoStream) { namespace { struct CallBitrateHelper { - CallBitrateHelper() : CallBitrateHelper(Call::Config::BitrateConfig()) {} + CallBitrateHelper() : CallBitrateHelper(Call::Config(&event_log_)) {} - 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_))); - } + explicit CallBitrateHelper(const Call::Config& config) + : mock_cc_(Clock::GetRealTimeClock(), &event_log_, &packet_router_), + call_(Call::Create( + config, + rtc::MakeUnique(&packet_router_, + &mock_cc_))) {} webrtc::Call* operator->() { return call_.get(); } testing::NiceMock& mock_cc() { @@ -366,7 +364,7 @@ TEST(CallBitrateTest, SetBitrateConfigWithDifferentMinCallsSetBweBitrates) { call->SetBitrateConfig(bitrate_config); bitrate_config.min_bitrate_bps = 11; - EXPECT_CALL(call.mock_cc(), SetBweBitrates(11, -1, 30)); + EXPECT_CALL(call.mock_cc(), SetBweBitrates(11, 20, 30)); call->SetBitrateConfig(bitrate_config); } @@ -394,12 +392,13 @@ TEST(CallBitrateTest, SetBitrateConfigWithDifferentMaxCallsSetBweBitrates) { call->SetBitrateConfig(bitrate_config); bitrate_config.max_bitrate_bps = 31; - EXPECT_CALL(call.mock_cc(), SetBweBitrates(10, -1, 31)); + EXPECT_CALL(call.mock_cc(), SetBweBitrates(10, 20, 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; @@ -490,224 +489,5 @@ 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 ec8f6b8f16..d323ef6806 100644 --- a/webrtc/media/engine/fakewebrtccall.cc +++ b/webrtc/media/engine/fakewebrtccall.cc @@ -588,11 +588,6 @@ 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 34db106c43..77cae6b0eb 100644 --- a/webrtc/media/engine/fakewebrtccall.h +++ b/webrtc/media/engine/fakewebrtccall.h @@ -293,8 +293,6 @@ 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 7111b1c53d..852dd39de5 100644 --- a/webrtc/pc/peerconnection.cc +++ b/webrtc/pc/peerconnection.cc @@ -1242,54 +1242,6 @@ 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 289a5d87a7..1e2ca3bb95 100644 --- a/webrtc/pc/peerconnection.h +++ b/webrtc/pc/peerconnection.h @@ -144,8 +144,6 @@ 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 f8d9ef6020..d0e42621cf 100644 --- a/webrtc/pc/peerconnectioninterface_unittest.cc +++ b/webrtc/pc/peerconnectioninterface_unittest.cc @@ -3301,69 +3301,6 @@ 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 {