From 803e3ff2986c9237027c1b08387b3157c9a2a3be Mon Sep 17 00:00:00 2001 From: Sebastian Jansson Date: Mon, 6 Aug 2018 19:14:37 +0200 Subject: [PATCH] Removes unused reserved bitrate in BitrateController. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This removes the reserved bitrate feature as it is not currently used. This saves debugging time as there is less code to investigate. This also makes it easier to compare the code with the task queue based version which lacks this feature. Bug: webrtc:9586 Change-Id: I207624ceb8d203c88c3d01bfc753d60523f99fe4 Reviewed-on: https://webrtc-review.googlesource.com/92622 Reviewed-by: Björn Terelius Commit-Queue: Sebastian Jansson Cr-Commit-Position: refs/heads/master@{#24357} --- .../bitrate_controller_impl.cc | 18 +------ .../bitrate_controller_impl.h | 4 -- .../bitrate_controller_unittest.cc | 50 ------------------- .../include/bitrate_controller.h | 2 - .../include/mock/mock_bitrate_controller.h | 1 - 5 files changed, 2 insertions(+), 73 deletions(-) diff --git a/modules/bitrate_controller/bitrate_controller_impl.cc b/modules/bitrate_controller/bitrate_controller_impl.cc index 38f97fb7e2..9b816f67dc 100644 --- a/modules/bitrate_controller/bitrate_controller_impl.cc +++ b/modules/bitrate_controller/bitrate_controller_impl.cc @@ -63,11 +63,9 @@ BitrateControllerImpl::BitrateControllerImpl(const Clock* clock, last_bitrate_update_ms_(clock_->TimeInMilliseconds()), event_log_(event_log), bandwidth_estimation_(event_log), - reserved_bitrate_bps_(0), last_bitrate_bps_(0), last_fraction_loss_(0), - last_rtt_ms_(0), - last_reserved_bitrate_bps_(0) { + last_rtt_ms_(0) { // This calls the observer_ if set, which means that the observer provided by // the user must be ready to accept a bitrate update when it constructs the // controller. We do this to avoid having to keep synchronized initial values @@ -119,14 +117,6 @@ void BitrateControllerImpl::ResetBitrates(int bitrate_bps, MaybeTriggerOnNetworkChanged(); } -void BitrateControllerImpl::SetReservedBitrate(uint32_t reserved_bitrate_bps) { - { - rtc::CritScope cs(&critsect_); - reserved_bitrate_bps_ = reserved_bitrate_bps; - } - MaybeTriggerOnNetworkChanged(); -} - // This is called upon reception of REMB or TMMBR. void BitrateControllerImpl::OnReceivedEstimatedBitrate(uint32_t bitrate) { { @@ -248,18 +238,15 @@ bool BitrateControllerImpl::GetNetworkParameters(uint32_t* bitrate, int current_bitrate; bandwidth_estimation_.CurrentEstimate(¤t_bitrate, fraction_loss, rtt); *bitrate = current_bitrate; - *bitrate -= std::min(*bitrate, reserved_bitrate_bps_); *bitrate = std::max(*bitrate, bandwidth_estimation_.GetMinBitrate()); bool new_bitrate = false; if (*bitrate != last_bitrate_bps_ || *fraction_loss != last_fraction_loss_ || - *rtt != last_rtt_ms_ || - last_reserved_bitrate_bps_ != reserved_bitrate_bps_) { + *rtt != last_rtt_ms_) { last_bitrate_bps_ = *bitrate; last_fraction_loss_ = *fraction_loss; last_rtt_ms_ = *rtt; - last_reserved_bitrate_bps_ = reserved_bitrate_bps_; new_bitrate = true; } @@ -280,7 +267,6 @@ bool BitrateControllerImpl::AvailableBandwidth(uint32_t* bandwidth) const { int64_t rtt; bandwidth_estimation_.CurrentEstimate(&bitrate, &fraction_loss, &rtt); if (bitrate > 0) { - bitrate = bitrate - std::min(bitrate, reserved_bitrate_bps_); bitrate = std::max(bitrate, bandwidth_estimation_.GetMinBitrate()); *bandwidth = bitrate; return true; diff --git a/modules/bitrate_controller/bitrate_controller_impl.h b/modules/bitrate_controller/bitrate_controller_impl.h index e315c5f9fd..ee220b6f11 100644 --- a/modules/bitrate_controller/bitrate_controller_impl.h +++ b/modules/bitrate_controller/bitrate_controller_impl.h @@ -54,8 +54,6 @@ class BitrateControllerImpl : public BitrateController { int min_bitrate_bps, int max_bitrate_bps) override; - void SetReservedBitrate(uint32_t reserved_bitrate_bps) override; - // Returns true if the parameters have changed since the last call. bool GetNetworkParameters(uint32_t* bitrate, uint8_t* fraction_loss, @@ -94,12 +92,10 @@ class BitrateControllerImpl : public BitrateController { std::map ssrc_to_last_received_extended_high_seq_num_ RTC_GUARDED_BY(critsect_); SendSideBandwidthEstimation bandwidth_estimation_ RTC_GUARDED_BY(critsect_); - uint32_t reserved_bitrate_bps_ RTC_GUARDED_BY(critsect_); uint32_t last_bitrate_bps_ RTC_GUARDED_BY(critsect_); uint8_t last_fraction_loss_ RTC_GUARDED_BY(critsect_); int64_t last_rtt_ms_ RTC_GUARDED_BY(critsect_); - uint32_t last_reserved_bitrate_bps_ RTC_GUARDED_BY(critsect_); RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(BitrateControllerImpl); }; diff --git a/modules/bitrate_controller/bitrate_controller_unittest.cc b/modules/bitrate_controller/bitrate_controller_unittest.cc index ce930d10d4..96f8a1ef26 100644 --- a/modules/bitrate_controller/bitrate_controller_unittest.cc +++ b/modules/bitrate_controller/bitrate_controller_unittest.cc @@ -352,56 +352,6 @@ TEST_F(BitrateControllerTest, OneBitrateObserverMultipleReportBlocks) { report_blocks.clear(); } -TEST_F(BitrateControllerTest, SetReservedBitrate) { - // Receive successively lower REMBs, verify the reserved bitrate is deducted. - controller_->SetReservedBitrate(0); - bandwidth_observer_->OnReceivedEstimatedBitrate(400000); - EXPECT_EQ(200000, bitrate_observer_.last_bitrate_); - controller_->SetReservedBitrate(50000); - bandwidth_observer_->OnReceivedEstimatedBitrate(400000); - EXPECT_EQ(150000, bitrate_observer_.last_bitrate_); - - controller_->SetReservedBitrate(0); - bandwidth_observer_->OnReceivedEstimatedBitrate(250000); - EXPECT_EQ(200000, bitrate_observer_.last_bitrate_); - controller_->SetReservedBitrate(50000); - bandwidth_observer_->OnReceivedEstimatedBitrate(250000); - EXPECT_EQ(150000, bitrate_observer_.last_bitrate_); - - controller_->SetReservedBitrate(0); - bandwidth_observer_->OnReceivedEstimatedBitrate(200000); - EXPECT_EQ(200000, bitrate_observer_.last_bitrate_); - controller_->SetReservedBitrate(30000); - bandwidth_observer_->OnReceivedEstimatedBitrate(200000); - EXPECT_EQ(170000, bitrate_observer_.last_bitrate_); - - controller_->SetReservedBitrate(0); - bandwidth_observer_->OnReceivedEstimatedBitrate(160000); - EXPECT_EQ(160000, bitrate_observer_.last_bitrate_); - controller_->SetReservedBitrate(30000); - bandwidth_observer_->OnReceivedEstimatedBitrate(160000); - EXPECT_EQ(130000, bitrate_observer_.last_bitrate_); - - controller_->SetReservedBitrate(0); - bandwidth_observer_->OnReceivedEstimatedBitrate(120000); - EXPECT_EQ(120000, bitrate_observer_.last_bitrate_); - controller_->SetReservedBitrate(10000); - bandwidth_observer_->OnReceivedEstimatedBitrate(120000); - EXPECT_EQ(110000, bitrate_observer_.last_bitrate_); - - controller_->SetReservedBitrate(0); - bandwidth_observer_->OnReceivedEstimatedBitrate(120000); - EXPECT_EQ(120000, bitrate_observer_.last_bitrate_); - controller_->SetReservedBitrate(50000); - bandwidth_observer_->OnReceivedEstimatedBitrate(120000); - // Limited by min bitrate. - EXPECT_EQ(100000, bitrate_observer_.last_bitrate_); - - controller_->SetReservedBitrate(10000); - bandwidth_observer_->OnReceivedEstimatedBitrate(1); - EXPECT_EQ(100000, bitrate_observer_.last_bitrate_); -} - TEST_F(BitrateControllerTest, TimeoutsWithoutFeedback) { { webrtc::test::ScopedFieldTrials override_field_trials( diff --git a/modules/bitrate_controller/include/bitrate_controller.h b/modules/bitrate_controller/include/bitrate_controller.h index 6e692d2c24..f67600d190 100644 --- a/modules/bitrate_controller/include/bitrate_controller.h +++ b/modules/bitrate_controller/include/bitrate_controller.h @@ -92,8 +92,6 @@ class BitrateController : public Module, public RtcpBandwidthObserver { // this bandwidth excludes packet headers. virtual bool AvailableBandwidth(uint32_t* bandwidth) const = 0; - virtual void SetReservedBitrate(uint32_t reserved_bitrate_bps) = 0; - virtual bool GetNetworkParameters(uint32_t* bitrate, uint8_t* fraction_loss, int64_t* rtt) = 0; diff --git a/modules/bitrate_controller/include/mock/mock_bitrate_controller.h b/modules/bitrate_controller/include/mock/mock_bitrate_controller.h index 4207b7498f..d33f07b216 100644 --- a/modules/bitrate_controller/include/mock/mock_bitrate_controller.h +++ b/modules/bitrate_controller/include/mock/mock_bitrate_controller.h @@ -41,7 +41,6 @@ class MockBitrateController : public BitrateController { MOCK_METHOD1(UpdateProbeBitrate, void(uint32_t bitrate_bps)); MOCK_METHOD1(SetEventLog, void(RtcEventLog* event_log)); MOCK_CONST_METHOD1(AvailableBandwidth, bool(uint32_t* bandwidth)); - MOCK_METHOD1(SetReservedBitrate, void(uint32_t reserved_bitrate_bps)); MOCK_METHOD3(GetNetworkParameters, bool(uint32_t* bitrate, uint8_t* fraction_loss, int64_t* rtt));