From 2221e1cd1dd19ae16c87c14bbea92fa62d15154d Mon Sep 17 00:00:00 2001 From: honghaiz Date: Thu, 2 Jun 2016 14:48:05 -0700 Subject: [PATCH] Update the BWE when the network route changes. BUG= Review-Url: https://codereview.webrtc.org/2000063003 Cr-Commit-Position: refs/heads/master@{#13021} --- webrtc/call/call.cc | 10 ++-- .../bitrate_controller_impl.cc | 12 +++++ .../bitrate_controller_impl.h | 4 ++ .../include/bitrate_controller.h | 4 ++ .../include/mock/mock_bitrate_controller.h | 2 + .../congestion_controller.cc | 51 ++++++++++++++----- .../congestion_controller_unittest.cc | 14 +++++ .../include/congestion_controller.h | 5 ++ 8 files changed, 87 insertions(+), 15 deletions(-) diff --git a/webrtc/call/call.cc b/webrtc/call/call.cc index e3c6ec7a0c..6d12202a1d 100644 --- a/webrtc/call/call.cc +++ b/webrtc/call/call.cc @@ -626,9 +626,13 @@ void Call::OnNetworkRouteChanged(const std::string& transport_name, kv->second = network_route; LOG(LS_INFO) << "Network route changed on transport " << transport_name << ": new local network id " << network_route.local_network_id - << " new remote network id " - << network_route.remote_network_id; - // TODO(holmer): Update the BWE bitrates. + << " new remote network id " << network_route.remote_network_id + << " Reset bitrate to " + << config_.bitrate_config.start_bitrate_bps << "bps"; + congestion_controller_->ResetBweAndBitrates( + config_.bitrate_config.start_bitrate_bps, + config_.bitrate_config.min_bitrate_bps, + config_.bitrate_config.max_bitrate_bps); } } diff --git a/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc b/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc index 09652d8419..c99bd53218 100644 --- a/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc +++ b/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc @@ -138,6 +138,18 @@ void BitrateControllerImpl::SetBitrates(int start_bitrate_bps, MaybeTriggerOnNetworkChanged(); } +void BitrateControllerImpl::ResetBitrates(int bitrate_bps, + int min_bitrate_bps, + int max_bitrate_bps) { + { + rtc::CritScope cs(&critsect_); + bandwidth_estimation_ = SendSideBandwidthEstimation(); + bandwidth_estimation_.SetBitrates(bitrate_bps, min_bitrate_bps, + max_bitrate_bps); + } + MaybeTriggerOnNetworkChanged(); +} + void BitrateControllerImpl::SetReservedBitrate(uint32_t reserved_bitrate_bps) { { rtc::CritScope cs(&critsect_); diff --git a/webrtc/modules/bitrate_controller/bitrate_controller_impl.h b/webrtc/modules/bitrate_controller/bitrate_controller_impl.h index 5a61379ce0..91f0902cb2 100644 --- a/webrtc/modules/bitrate_controller/bitrate_controller_impl.h +++ b/webrtc/modules/bitrate_controller/bitrate_controller_impl.h @@ -46,6 +46,10 @@ class BitrateControllerImpl : public BitrateController { int min_bitrate_bps, int max_bitrate_bps) override; + void ResetBitrates(int bitrate_bps, + int min_bitrate_bps, + int max_bitrate_bps) override; + void UpdateDelayBasedEstimate(uint32_t bitrate_bps) override; void SetReservedBitrate(uint32_t reserved_bitrate_bps) override; diff --git a/webrtc/modules/bitrate_controller/include/bitrate_controller.h b/webrtc/modules/bitrate_controller/include/bitrate_controller.h index a61cf6a7a7..4165f066b2 100644 --- a/webrtc/modules/bitrate_controller/include/bitrate_controller.h +++ b/webrtc/modules/bitrate_controller/include/bitrate_controller.h @@ -70,6 +70,10 @@ class BitrateController : public Module { int min_bitrate_bps, int max_bitrate_bps) = 0; + virtual void ResetBitrates(int bitrate_bps, + int min_bitrate_bps, + int max_bitrate_bps) = 0; + virtual void UpdateDelayBasedEstimate(uint32_t bitrate_bps) = 0; virtual void SetEventLog(RtcEventLog* event_log) = 0; diff --git a/webrtc/modules/bitrate_controller/include/mock/mock_bitrate_controller.h b/webrtc/modules/bitrate_controller/include/mock/mock_bitrate_controller.h index da6169e748..7e9b4ece31 100644 --- a/webrtc/modules/bitrate_controller/include/mock/mock_bitrate_controller.h +++ b/webrtc/modules/bitrate_controller/include/mock/mock_bitrate_controller.h @@ -35,6 +35,8 @@ class MockBitrateController : public BitrateController { void(int start_bitrate_bps, int min_bitrate_bps, int max_bitrate_bps)); + MOCK_METHOD3(ResetBitrates, + void(int bitrate_bps, int min_bitrate_bps, int max_bitrate_bps)); MOCK_METHOD1(UpdateDelayBasedEstimate, void(uint32_t bitrate_bps)); MOCK_METHOD1(SetEventLog, void(RtcEventLog* event_log)); MOCK_CONST_METHOD1(AvailableBandwidth, bool(uint32_t* bandwidth)); diff --git a/webrtc/modules/congestion_controller/congestion_controller.cc b/webrtc/modules/congestion_controller/congestion_controller.cc index 5767448a5f..01d984fe91 100644 --- a/webrtc/modules/congestion_controller/congestion_controller.cc +++ b/webrtc/modules/congestion_controller/congestion_controller.cc @@ -32,6 +32,22 @@ namespace { static const uint32_t kTimeOffsetSwitchThreshold = 30; +// Makes sure that the bitrate and the min, max values are in valid range. +static void ClampBitrates(int* bitrate_bps, + int* min_bitrate_bps, + int* max_bitrate_bps) { + // TODO(holmer): We should make sure the default bitrates are set to 10 kbps, + // and that we don't try to set the min bitrate to 0 from any applications. + // The congestion controller should allow a min bitrate of 0. + const int kMinBitrateBps = 10000; + if (*min_bitrate_bps < kMinBitrateBps) + *min_bitrate_bps = kMinBitrateBps; + if (*max_bitrate_bps > 0) + *max_bitrate_bps = std::max(*min_bitrate_bps, *max_bitrate_bps); + if (*bitrate_bps > 0) + *bitrate_bps = std::max(*min_bitrate_bps, *bitrate_bps); +} + class WrappingBitrateEstimator : public RemoteBitrateEstimator { public: WrappingBitrateEstimator(RemoteBitrateObserver* observer, Clock* clock) @@ -212,21 +228,10 @@ void CongestionController::Init() { min_bitrate_bps_); } - void CongestionController::SetBweBitrates(int min_bitrate_bps, int start_bitrate_bps, int max_bitrate_bps) { - // TODO(holmer): We should make sure the default bitrates are set to 10 kbps, - // and that we don't try to set the min bitrate to 0 from any applications. - // The congestion controller should allow a min bitrate of 0. - const int kMinBitrateBps = 10000; - if (min_bitrate_bps < kMinBitrateBps) - min_bitrate_bps = kMinBitrateBps; - if (max_bitrate_bps > 0) - max_bitrate_bps = std::max(min_bitrate_bps, max_bitrate_bps); - if (start_bitrate_bps > 0) - start_bitrate_bps = std::max(min_bitrate_bps, start_bitrate_bps); - + ClampBitrates(&start_bitrate_bps, &min_bitrate_bps, &max_bitrate_bps); bitrate_controller_->SetBitrates(start_bitrate_bps, min_bitrate_bps, max_bitrate_bps); @@ -239,6 +244,28 @@ void CongestionController::SetBweBitrates(int min_bitrate_bps, MaybeTriggerOnNetworkChanged(); } +void CongestionController::ResetBweAndBitrates(int bitrate_bps, + int min_bitrate_bps, + int max_bitrate_bps) { + ClampBitrates(&bitrate_bps, &min_bitrate_bps, &max_bitrate_bps); + // TODO(honghaiz): Recreate this object once the bitrate controller is + // no longer exposed outside CongestionController. + bitrate_controller_->ResetBitrates(bitrate_bps, min_bitrate_bps, + max_bitrate_bps); + min_bitrate_bps_ = min_bitrate_bps; + // TODO(honghaiz): Recreate this object once the remote bitrate estimator is + // no longer exposed outside CongestionController. + if (remote_bitrate_estimator_) + remote_bitrate_estimator_->SetMinBitrate(min_bitrate_bps); + + RemoteBitrateEstimator* rbe = + new RemoteBitrateEstimatorAbsSendTime(&transport_feedback_adapter_); + transport_feedback_adapter_.SetBitrateEstimator(rbe); + rbe->SetMinBitrate(min_bitrate_bps); + // TODO(holmer): Trigger a new probe once mid-call probing is implemented. + MaybeTriggerOnNetworkChanged(); +} + BitrateController* CongestionController::GetBitrateController() const { return bitrate_controller_.get(); } diff --git a/webrtc/modules/congestion_controller/congestion_controller_unittest.cc b/webrtc/modules/congestion_controller/congestion_controller_unittest.cc index c82c75daf3..1a265824bc 100644 --- a/webrtc/modules/congestion_controller/congestion_controller_unittest.cc +++ b/webrtc/modules/congestion_controller/congestion_controller_unittest.cc @@ -124,6 +124,20 @@ TEST_F(CongestionControllerTest, SignalNetworkState) { controller_->SignalNetworkState(kNetworkDown); } +TEST_F(CongestionControllerTest, ResetBweAndBitrates) { + int new_bitrate = 200000; + EXPECT_CALL(observer_, OnNetworkChanged(new_bitrate, _, _)); + EXPECT_CALL(*pacer_, SetEstimatedBitrate(new_bitrate)); + controller_->ResetBweAndBitrates(new_bitrate, -1, -1); + + // If the bitrate is reset to -1, the new starting bitrate will be + // the minimum default bitrate 10000bps. + int min_default_bitrate = 10000; + EXPECT_CALL(observer_, OnNetworkChanged(min_default_bitrate, _, _)); + EXPECT_CALL(*pacer_, SetEstimatedBitrate(min_default_bitrate)); + controller_->ResetBweAndBitrates(-1, -1, -1); +} + TEST_F(CongestionControllerTest, SignalNetworkStateAndQueueIsFullAndEstimateChange) { // Send queue is full diff --git a/webrtc/modules/congestion_controller/include/congestion_controller.h b/webrtc/modules/congestion_controller/include/congestion_controller.h index da8719d33a..3ba2f95300 100644 --- a/webrtc/modules/congestion_controller/include/congestion_controller.h +++ b/webrtc/modules/congestion_controller/include/congestion_controller.h @@ -70,6 +70,11 @@ class CongestionController : public CallStatsObserver, public Module { virtual void SetBweBitrates(int min_bitrate_bps, int start_bitrate_bps, int max_bitrate_bps); + // Resets both the BWE state and the bitrate estimator. Note the first + // argument is the bitrate_bps. + virtual void ResetBweAndBitrates(int bitrate_bps, + int min_bitrate_bps, + int max_bitrate_bps); virtual void SignalNetworkState(NetworkState state); virtual BitrateController* GetBitrateController() const; virtual RemoteBitrateEstimator* GetRemoteBitrateEstimator(