From 6cd201cf31dc8e50bf815139b0c9fdc83d3ba2bf Mon Sep 17 00:00:00 2001 From: "andrew@webrtc.org" Date: Tue, 25 Mar 2014 19:42:39 +0000 Subject: [PATCH] Revert 5775 "Modify bitrate controller to update bitrate based o..." This triggered an occasional TSAN failure in CallTest.ReceivesPliAndRecoversWithNack e.g.: http://build.chromium.org/p/client.webrtc/builders/Linux%20Tsan/builds/1444/steps/memory%20test%3A%20video_engine_tests/logs/stdio I managed to reproduce this locally and verified that reverting this CL corrected it. > Modify bitrate controller to update bitrate based on process call and not > only whenever a RTCP receiver block is received. > > Additionally: > Add condition to only start rampup after a receiver block is received. This was same as old behaviour but now an explicit check is needed to verify process does not ramps up before the first block. > > Fix logic around capping max bitrate increase at 8% per second. Before it was only increasing once every 1 second and each increase would be as high as 8%. If receiver blocks had a different interval before it would lose an update or waste an update slot and not ramp up as much as a 8% (e.g. if RTCP received < 1 second). > > Did not touch decrease logic, however since it can be triggered more often it > may decrease much faster and closer to the original written cap of once every > 300ms + rtt. > > Note: > rampup_tests.cc don't seem to be affected by this since there is no packet loss or REMB that go higher than expected cap. > bitrate_controller_unittests.cc are don't really simulate a clock and the process thread, but trigger update by inserting an rtcp block. > > BUG=3065 > R=stefan@webrtc.org, mflodman@webrtc.org > > Review URL: https://webrtc-codereview.appspot.com/10529004 TBR=andresp@webrtc.org Review URL: https://webrtc-codereview.appspot.com/10079005 git-svn-id: http://webrtc.googlecode.com/svn/trunk@5785 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../bitrate_controller_impl.cc | 45 ++------- .../bitrate_controller_impl.h | 13 +-- .../bitrate_controller_unittest.cc | 90 ++++++++--------- .../include/bitrate_controller.h | 6 +- .../send_side_bandwidth_estimation.cc | 96 ++++++------------- .../send_side_bandwidth_estimation.h | 19 +--- webrtc/video/video_send_stream_tests.cc | 5 +- webrtc/video_engine/vie_channel_group.cc | 20 ++-- 8 files changed, 101 insertions(+), 193 deletions(-) diff --git a/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc b/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc index e31aafb77d..d777536b96 100644 --- a/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc +++ b/webrtc/modules/bitrate_controller/bitrate_controller_impl.cc @@ -78,22 +78,13 @@ class BitrateControllerImpl::RtcpBandwidthObserverImpl }; BitrateController* BitrateController::CreateBitrateController( - Clock* clock, bool enforce_min_bitrate) { - return new BitrateControllerImpl(clock, enforce_min_bitrate); + return new BitrateControllerImpl(enforce_min_bitrate); } -BitrateControllerImpl::BitrateControllerImpl(Clock* clock, - bool enforce_min_bitrate) - : clock_(clock), - last_bitrate_update_ms_(clock_->TimeInMilliseconds()), - critsect_(CriticalSectionWrapper::CreateCriticalSection()), - enforce_min_bitrate_(enforce_min_bitrate), - last_bitrate_bps_(0), - last_fraction_loss_(0), - last_rtt_ms_(0), - last_enforce_min_bitrate_(enforce_min_bitrate_), - bitrate_observers_modified_(false) {} +BitrateControllerImpl::BitrateControllerImpl(bool enforce_min_bitrate) + : critsect_(CriticalSectionWrapper::CreateCriticalSection()), + enforce_min_bitrate_(enforce_min_bitrate) {} BitrateControllerImpl::~BitrateControllerImpl() { BitrateObserverConfList::iterator it = @@ -202,26 +193,6 @@ void BitrateControllerImpl::OnReceivedEstimatedBitrate(const uint32_t bitrate) { MaybeTriggerOnNetworkChanged(); } -int32_t BitrateControllerImpl::TimeUntilNextProcess() { - enum { kBitrateControllerUpdateIntervalMs = 25 }; - CriticalSectionScoped cs(critsect_); - int time_since_update_ms = - clock_->TimeInMilliseconds() - last_bitrate_update_ms_; - return std::max(0, kBitrateControllerUpdateIntervalMs - time_since_update_ms); -} - -int32_t BitrateControllerImpl::Process() { - if (TimeUntilNextProcess() > 0) - return 0; - { - CriticalSectionScoped cs(critsect_); - bandwidth_estimation_.UpdateEstimate(clock_->TimeInMilliseconds()); - MaybeTriggerOnNetworkChanged(); - } - last_bitrate_update_ms_ = clock_->TimeInMilliseconds(); - return 0; -} - void BitrateControllerImpl::OnReceivedRtcpReceiverReport( const uint8_t fraction_loss, const uint32_t rtt, @@ -239,12 +210,12 @@ void BitrateControllerImpl::MaybeTriggerOnNetworkChanged() { uint32_t rtt; bandwidth_estimation_.CurrentEstimate(&bitrate, &fraction_loss, &rtt); - if (bitrate_observers_modified_ || bitrate != last_bitrate_bps_ || - fraction_loss != last_fraction_loss_ || rtt != last_rtt_ms_ || + if (bitrate_observers_modified_ || bitrate != last_bitrate_ || + fraction_loss != last_fraction_loss_ || rtt != last_rtt_ || last_enforce_min_bitrate_ != enforce_min_bitrate_) { - last_bitrate_bps_ = bitrate; + last_bitrate_ = bitrate; last_fraction_loss_ = fraction_loss; - last_rtt_ms_ = rtt; + last_rtt_ = rtt; last_enforce_min_bitrate_ = enforce_min_bitrate_; bitrate_observers_modified_ = false; OnNetworkChanged(bitrate, fraction_loss, rtt); diff --git a/webrtc/modules/bitrate_controller/bitrate_controller_impl.h b/webrtc/modules/bitrate_controller/bitrate_controller_impl.h index df7d28edc5..f9d2354ca1 100644 --- a/webrtc/modules/bitrate_controller/bitrate_controller_impl.h +++ b/webrtc/modules/bitrate_controller/bitrate_controller_impl.h @@ -29,7 +29,7 @@ namespace webrtc { class BitrateControllerImpl : public BitrateController { public: - BitrateControllerImpl(Clock* clock, bool enforce_min_bitrate); + explicit BitrateControllerImpl(bool enforce_min_bitrate); virtual ~BitrateControllerImpl(); virtual bool AvailableBandwidth(uint32_t* bandwidth) const OVERRIDE; @@ -45,9 +45,6 @@ class BitrateControllerImpl : public BitrateController { virtual void EnforceMinBitrate(bool enforce_min_bitrate) OVERRIDE; - virtual int32_t TimeUntilNextProcess() OVERRIDE; - virtual int32_t Process() OVERRIDE; - private: class RtcpBandwidthObserverImpl; @@ -110,17 +107,13 @@ class BitrateControllerImpl : public BitrateController { BitrateObserverConfList::iterator FindObserverConfigurationPair( const BitrateObserver* observer) EXCLUSIVE_LOCKS_REQUIRED(*critsect_); - // Used by process thread. - Clock* clock_; - uint32_t last_bitrate_update_ms_; - CriticalSectionWrapper* critsect_; SendSideBandwidthEstimation bandwidth_estimation_ GUARDED_BY(*critsect_); BitrateObserverConfList bitrate_observers_ GUARDED_BY(*critsect_); bool enforce_min_bitrate_ GUARDED_BY(*critsect_); - uint32_t last_bitrate_bps_ GUARDED_BY(*critsect_); + uint32_t last_bitrate_ GUARDED_BY(*critsect_); uint8_t last_fraction_loss_ GUARDED_BY(*critsect_); - uint32_t last_rtt_ms_ GUARDED_BY(*critsect_); + uint32_t last_rtt_ GUARDED_BY(*critsect_); bool last_enforce_min_bitrate_ GUARDED_BY(*critsect_); bool bitrate_observers_modified_ GUARDED_BY(*critsect_); }; diff --git a/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc b/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc index edfe343775..c53928b239 100644 --- a/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc +++ b/webrtc/modules/bitrate_controller/bitrate_controller_unittest.cc @@ -57,12 +57,12 @@ class TestBitrateObserver: public BitrateObserver { class BitrateControllerTest : public ::testing::Test { protected: - BitrateControllerTest() : clock_(0), enforce_min_bitrate_(true) {} + BitrateControllerTest() : enforce_min_bitrate_(true) {} ~BitrateControllerTest() {} virtual void SetUp() { - controller_ = BitrateController::CreateBitrateController( - &clock_, enforce_min_bitrate_); + controller_ = + BitrateController::CreateBitrateController(enforce_min_bitrate_); bandwidth_observer_ = controller_->CreateRtcpBandwidthObserver(); } @@ -70,8 +70,6 @@ class BitrateControllerTest : public ::testing::Test { delete bandwidth_observer_; delete controller_; } - - webrtc::SimulatedClock clock_; bool enforce_min_bitrate_; BitrateController* controller_; RtcpBandwidthObserver* bandwidth_observer_; @@ -89,50 +87,52 @@ TEST_F(BitrateControllerTest, OneBitrateObserverOneRtcpObserver) { // Receive a high remb, test bitrate inc. bandwidth_observer_->OnReceivedEstimatedBitrate(400000); - EXPECT_EQ(200000u, bitrate_observer.last_bitrate_); - EXPECT_EQ(0, bitrate_observer.last_fraction_loss_); - EXPECT_EQ(0u, bitrate_observer.last_rtt_); - // Test bitrate increase 8% per second. + // Test start bitrate. webrtc::ReportBlockList report_blocks; report_blocks.push_back(CreateReportBlock(1, 2, 0, 1)); bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, 1); - EXPECT_EQ(217000u, bitrate_observer.last_bitrate_); + EXPECT_EQ(200000u, bitrate_observer.last_bitrate_); EXPECT_EQ(0, bitrate_observer.last_fraction_loss_); EXPECT_EQ(50u, bitrate_observer.last_rtt_); + // Test bitrate increase 8% per second. report_blocks.clear(); report_blocks.push_back(CreateReportBlock(1, 2, 0, 21)); bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, 1001); - EXPECT_EQ(235360u, bitrate_observer.last_bitrate_); + EXPECT_EQ(217000u, bitrate_observer.last_bitrate_); EXPECT_EQ(0, bitrate_observer.last_fraction_loss_); EXPECT_EQ(50u, bitrate_observer.last_rtt_); report_blocks.clear(); report_blocks.push_back(CreateReportBlock(1, 2, 0, 41)); bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, 2001); - EXPECT_EQ(255189u, bitrate_observer.last_bitrate_); + EXPECT_EQ(235360u, bitrate_observer.last_bitrate_); report_blocks.clear(); report_blocks.push_back(CreateReportBlock(1, 2, 0, 61)); bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, 3001); - EXPECT_EQ(276604u, bitrate_observer.last_bitrate_); + EXPECT_EQ(255189u, bitrate_observer.last_bitrate_); report_blocks.clear(); report_blocks.push_back(CreateReportBlock(1, 2, 0, 801)); bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, 4001); - EXPECT_EQ(299732u, bitrate_observer.last_bitrate_); + EXPECT_EQ(276604u, bitrate_observer.last_bitrate_); - // Reach max cap. report_blocks.clear(); report_blocks.push_back(CreateReportBlock(1, 2, 0, 101)); bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, 5001); - EXPECT_EQ(300000u, bitrate_observer.last_bitrate_); + EXPECT_EQ(299732u, bitrate_observer.last_bitrate_); + + report_blocks.clear(); + report_blocks.push_back(CreateReportBlock(1, 2, 0, 121)); + bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, 6001); + EXPECT_EQ(300000u, bitrate_observer.last_bitrate_); // Max cap. report_blocks.clear(); report_blocks.push_back(CreateReportBlock(1, 2, 0, 141)); bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, 7001); - EXPECT_EQ(300000u, bitrate_observer.last_bitrate_); + EXPECT_EQ(300000u, bitrate_observer.last_bitrate_); // Max cap. // Test that a low REMB trigger immediately. bandwidth_observer_->OnReceivedEstimatedBitrate(250000); @@ -154,9 +154,6 @@ TEST_F(BitrateControllerTest, OneBitrateObserverTwoRtcpObservers) { // Receive a high remb, test bitrate inc. bandwidth_observer_->OnReceivedEstimatedBitrate(400000); - EXPECT_EQ(200000u, bitrate_observer.last_bitrate_); - EXPECT_EQ(0, bitrate_observer.last_fraction_loss_); - EXPECT_EQ(0u, bitrate_observer.last_rtt_); // Test start bitrate. webrtc::ReportBlockList report_blocks; @@ -164,7 +161,7 @@ TEST_F(BitrateControllerTest, OneBitrateObserverTwoRtcpObservers) { bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, 1); second_bandwidth_observer->OnReceivedRtcpReceiverReport( report_blocks, 100, 1); - EXPECT_EQ(217000u, bitrate_observer.last_bitrate_); + EXPECT_EQ(200000u, bitrate_observer.last_bitrate_); EXPECT_EQ(0, bitrate_observer.last_fraction_loss_); EXPECT_EQ(100u, bitrate_observer.last_rtt_); @@ -174,7 +171,7 @@ TEST_F(BitrateControllerTest, OneBitrateObserverTwoRtcpObservers) { bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, 501); second_bandwidth_observer->OnReceivedRtcpReceiverReport(report_blocks, 100, 1001); - EXPECT_EQ(235360u, bitrate_observer.last_bitrate_); + EXPECT_EQ(217000u, bitrate_observer.last_bitrate_); EXPECT_EQ(0, bitrate_observer.last_fraction_loss_); EXPECT_EQ(100u, bitrate_observer.last_rtt_); @@ -183,45 +180,50 @@ TEST_F(BitrateControllerTest, OneBitrateObserverTwoRtcpObservers) { report_blocks.push_back(CreateReportBlock(1, 2, 0, 31)); second_bandwidth_observer->OnReceivedRtcpReceiverReport(report_blocks, 100, 1501); - EXPECT_EQ(235360u, bitrate_observer.last_bitrate_); + EXPECT_EQ(217000u, bitrate_observer.last_bitrate_); report_blocks.clear(); report_blocks.push_back(CreateReportBlock(1, 2, 0, 41)); bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, 2001); - EXPECT_EQ(255189u, bitrate_observer.last_bitrate_); + EXPECT_EQ(235360u, bitrate_observer.last_bitrate_); // Second report should not change estimate. report_blocks.clear(); report_blocks.push_back(CreateReportBlock(1, 2, 0, 41)); second_bandwidth_observer->OnReceivedRtcpReceiverReport(report_blocks, 100, 2001); - EXPECT_EQ(255189u, bitrate_observer.last_bitrate_); + EXPECT_EQ(235360u, bitrate_observer.last_bitrate_); // Reports from only one bandwidth observer is ok. report_blocks.clear(); report_blocks.push_back(CreateReportBlock(1, 2, 0, 61)); second_bandwidth_observer->OnReceivedRtcpReceiverReport(report_blocks, 50, 3001); - EXPECT_EQ(276604u, bitrate_observer.last_bitrate_); + EXPECT_EQ(255189u, bitrate_observer.last_bitrate_); report_blocks.clear(); report_blocks.push_back(CreateReportBlock(1, 2, 0, 81)); second_bandwidth_observer->OnReceivedRtcpReceiverReport(report_blocks, 50, 4001); + EXPECT_EQ(276604u, bitrate_observer.last_bitrate_); + + report_blocks.clear(); + report_blocks.push_back(CreateReportBlock(1, 2, 0, 101)); + second_bandwidth_observer->OnReceivedRtcpReceiverReport(report_blocks, 50, + 5001); EXPECT_EQ(299732u, bitrate_observer.last_bitrate_); - // Reach max cap. report_blocks.clear(); report_blocks.push_back(CreateReportBlock(1, 2, 0, 121)); - second_bandwidth_observer->OnReceivedRtcpReceiverReport( - report_blocks, 50, 5001); - EXPECT_EQ(300000u, bitrate_observer.last_bitrate_); + second_bandwidth_observer->OnReceivedRtcpReceiverReport(report_blocks, 50, + 6001); + EXPECT_EQ(300000u, bitrate_observer.last_bitrate_); // Max cap. report_blocks.clear(); report_blocks.push_back(CreateReportBlock(1, 2, 0, 141)); - second_bandwidth_observer->OnReceivedRtcpReceiverReport( - report_blocks, 50, 6001); - EXPECT_EQ(300000u, bitrate_observer.last_bitrate_); + second_bandwidth_observer->OnReceivedRtcpReceiverReport(report_blocks, 50, + 7001); + EXPECT_EQ(300000u, bitrate_observer.last_bitrate_); // Max cap. // Test that a low REMB trigger immediately. // We don't care which bandwidth observer that delivers the REMB. @@ -230,9 +232,8 @@ TEST_F(BitrateControllerTest, OneBitrateObserverTwoRtcpObservers) { EXPECT_EQ(0, bitrate_observer.last_fraction_loss_); EXPECT_EQ(50u, bitrate_observer.last_rtt_); - // Min cap. bandwidth_observer_->OnReceivedEstimatedBitrate(1000); - EXPECT_EQ(100000u, bitrate_observer.last_bitrate_); + EXPECT_EQ(100000u, bitrate_observer.last_bitrate_); // Min cap. controller_->RemoveBitrateObserver(&bitrate_observer); delete second_bandwidth_observer; } @@ -316,24 +317,27 @@ TEST_F(BitrateControllerTest, TwoBitrateObserversOneRtcpObserver) { controller_->SetBitrateObserver(&bitrate_observer_1, 200000, 100000, 300000); // Receive a high remb, test bitrate inc. - // Test too low start bitrate, hence lower than sum of min. bandwidth_observer_->OnReceivedEstimatedBitrate(400000); - EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_); - EXPECT_EQ(0, bitrate_observer_1.last_fraction_loss_); - EXPECT_EQ(0u, bitrate_observer_1.last_rtt_); - // Test bitrate increase 8% per second, distributed equally. + // Test too low start bitrate, hence lower than sum of min. webrtc::ReportBlockList report_blocks; report_blocks.push_back(CreateReportBlock(1, 2, 0, 1)); - bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, 1001); - EXPECT_EQ(112500u, bitrate_observer_1.last_bitrate_); + bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, 1); + EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_); EXPECT_EQ(0, bitrate_observer_1.last_fraction_loss_); EXPECT_EQ(50u, bitrate_observer_1.last_rtt_); - EXPECT_EQ(212500u, bitrate_observer_2.last_bitrate_); + EXPECT_EQ(200000u, bitrate_observer_2.last_bitrate_); EXPECT_EQ(0, bitrate_observer_2.last_fraction_loss_); EXPECT_EQ(50u, bitrate_observer_2.last_rtt_); + // Test bitrate increase 8% per second, distributed equally. + report_blocks.clear(); + report_blocks.push_back(CreateReportBlock(1, 2, 0, 21)); + bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, 1001); + EXPECT_EQ(112500u, bitrate_observer_1.last_bitrate_); + EXPECT_EQ(212500u, bitrate_observer_2.last_bitrate_); + report_blocks.clear(); report_blocks.push_back(CreateReportBlock(1, 2, 0, 41)); bandwidth_observer_->OnReceivedRtcpReceiverReport(report_blocks, 50, 2001); diff --git a/webrtc/modules/bitrate_controller/include/bitrate_controller.h b/webrtc/modules/bitrate_controller/include/bitrate_controller.h index 1d90cb9876..0f74367658 100644 --- a/webrtc/modules/bitrate_controller/include/bitrate_controller.h +++ b/webrtc/modules/bitrate_controller/include/bitrate_controller.h @@ -15,7 +15,6 @@ #ifndef WEBRTC_MODULES_BITRATE_CONTROLLER_INCLUDE_BITRATE_CONTROLLER_H_ #define WEBRTC_MODULES_BITRATE_CONTROLLER_INCLUDE_BITRATE_CONTROLLER_H_ -#include "webrtc/modules/interface/module.h" #include "webrtc/modules/rtp_rtcp/interface/rtp_rtcp_defines.h" namespace webrtc { @@ -36,7 +35,7 @@ class BitrateObserver { virtual ~BitrateObserver() {} }; -class BitrateController : public Module { +class BitrateController { /* * This class collects feedback from all streams sent to a peer (via * RTCPBandwidthObservers). It does one aggregated send side bandwidth @@ -49,8 +48,7 @@ class BitrateController : public Module { // When true, the bitrate will never be set lower than the minimum bitrate(s). // When false, the bitrate observers will be allocated rates up to their // respective minimum bitrate, satisfying one observer after the other. - static BitrateController* CreateBitrateController(Clock* clock, - bool enforce_min_bitrate); + static BitrateController* CreateBitrateController(bool enforce_min_bitrate); virtual ~BitrateController() {} virtual RtcpBandwidthObserver* CreateRtcpBandwidthObserver() = 0; diff --git a/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.cc b/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.cc index 036e04fc64..52eec8fe2d 100644 --- a/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.cc +++ b/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.cc @@ -52,20 +52,16 @@ SendSideBandwidthEstimation::SendSideBandwidthEstimation() bitrate_(0), min_bitrate_configured_(0), max_bitrate_configured_(0), - time_last_receiver_block_ms_(0), last_fraction_loss_(0), - last_round_trip_time_ms_(0), + last_round_trip_time_(0), bwe_incoming_(0), - time_last_decrease_ms_(0) {} + time_last_increase_(0), + time_last_decrease_(0) {} SendSideBandwidthEstimation::~SendSideBandwidthEstimation() {} void SendSideBandwidthEstimation::SetSendBitrate(uint32_t bitrate) { bitrate_ = bitrate; - - // Clear last sent bitrate history so the new value can be used directly - // and not capped. - min_bitrate_history_.clear(); } void SendSideBandwidthEstimation::SetMinMaxBitrate(uint32_t min_bitrate, @@ -83,7 +79,7 @@ void SendSideBandwidthEstimation::CurrentEstimate(uint32_t* bitrate, uint32_t* rtt) const { *bitrate = bitrate_; *loss = last_fraction_loss_; - *rtt = last_round_trip_time_ms_; + *rtt = last_round_trip_time_; } void SendSideBandwidthEstimation::UpdateReceiverEstimate(uint32_t bandwidth) { @@ -96,7 +92,7 @@ void SendSideBandwidthEstimation::UpdateReceiverBlock(uint8_t fraction_loss, int number_of_packets, uint32_t now_ms) { // Update RTT. - last_round_trip_time_ms_ = rtt; + last_round_trip_time_ = rtt; // Check sequence number diff and weight loss report if (number_of_packets > 0) { @@ -119,82 +115,50 @@ void SendSideBandwidthEstimation::UpdateReceiverBlock(uint8_t fraction_loss, return; } } - time_last_receiver_block_ms_ = now_ms; UpdateEstimate(now_ms); } void SendSideBandwidthEstimation::UpdateEstimate(uint32_t now_ms) { - UpdateMinHistory(now_ms); + if (last_fraction_loss_ <= 5) { + // Loss < 2%: Limit the rate increases to once a kBweIncreaseIntervalMs. + if ((now_ms - time_last_increase_) >= kBweIncreaseIntervalMs) { + time_last_increase_ = now_ms; - // Only start updating bitrate when receiving receiver blocks. - if (time_last_receiver_block_ms_ != 0) { - if (last_fraction_loss_ <= 5) { - // Loss < 2%: Increase rate by 8% of the min bitrate in the last - // kBweIncreaseIntervalMs. - // Note that by remembering the bitrate over the last second one can - // rampup up one second faster than if only allowed to start ramping - // at 8% per second rate now. E.g.: - // If sending a constant 100kbps it can rampup immediatly to 108kbps - // whenever a receiver report is received with lower packet loss. - // If instead one would do: bitrate_ *= 1.08^(delta time), it would - // take over one second since the lower packet loss to achieve 108kbps. - bitrate_ = static_cast( - min_bitrate_history_.front().second * 1.08 + 0.5); + // Increase rate by 8%. + bitrate_ = static_cast(bitrate_ * 1.08 + 0.5); // Add 1 kbps extra, just to make sure that we do not get stuck // (gives a little extra increase at low rates, negligible at higher // rates). bitrate_ += 1000; + } - } else if (last_fraction_loss_ <= 26) { - // Loss between 2% - 10%: Do nothing. + } else if (last_fraction_loss_ <= 26) { + // Loss between 2% - 10%: Do nothing. - } else { - // Loss > 10%: Limit the rate decreases to once a kBweDecreaseIntervalMs + - // rtt. - if ((now_ms - time_last_decrease_ms_) >= - static_cast(kBweDecreaseIntervalMs + - last_round_trip_time_ms_)) { - time_last_decrease_ms_ = now_ms; + } else { + // Loss > 10%: Limit the rate decreases to once a kBweDecreaseIntervalMs + + // rtt. + if ((now_ms - time_last_decrease_) >= + static_cast(kBweDecreaseIntervalMs + last_round_trip_time_)) { + time_last_decrease_ = now_ms; - // Reduce rate: - // newRate = rate * (1 - 0.5*lossRate); - // where packetLoss = 256*lossRate; - bitrate_ = static_cast( - (bitrate_ * static_cast(512 - last_fraction_loss_)) / - 512.0); + // Reduce rate: + // newRate = rate * (1 - 0.5*lossRate); + // where packetLoss = 256*lossRate; + bitrate_ = static_cast( + (bitrate_ * static_cast(512 - last_fraction_loss_)) / 512.0); - // Calculate what rate TFRC would apply in this situation and to not - // reduce further than it. - bitrate_ = std::max( - bitrate_, - CalcTfrcBps(last_round_trip_time_ms_, last_fraction_loss_)); - } + // Calculate what rate TFRC would apply in this situation and to not + // reduce further than it. + bitrate_ = std::max( + bitrate_, CalcTfrcBps(last_round_trip_time_, last_fraction_loss_)); } } + CapBitrateToThresholds(); } -void SendSideBandwidthEstimation::UpdateMinHistory(uint32_t now_ms) { - // Remove old data points from history. - // Since history precision is in ms, add one so it is able to increase - // bitrate if it is off by as little as 0.5ms. - while (!min_bitrate_history_.empty() && - now_ms - min_bitrate_history_.front().first + 1 > - kBweIncreaseIntervalMs) { - min_bitrate_history_.pop_front(); - } - - // Typical minimum sliding-window algorithm: Pop values higher than current - // bitrate before pushing it. - while (!min_bitrate_history_.empty() && - bitrate_ <= min_bitrate_history_.back().second) { - min_bitrate_history_.pop_back(); - } - - min_bitrate_history_.push_back(std::make_pair(now_ms, bitrate_)); -} - void SendSideBandwidthEstimation::CapBitrateToThresholds() { if (bwe_incoming_ > 0 && bitrate_ > bwe_incoming_) { bitrate_ = bwe_incoming_; diff --git a/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.h b/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.h index eb675d1ca6..9ba3a68078 100644 --- a/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.h +++ b/webrtc/modules/bitrate_controller/send_side_bandwidth_estimation.h @@ -13,8 +13,6 @@ #ifndef WEBRTC_MODULES_BITRATE_CONTROLLER_SEND_SIDE_BANDWIDTH_ESTIMATION_H_ #define WEBRTC_MODULES_BITRATE_CONTROLLER_SEND_SIDE_BANDWIDTH_ESTIMATION_H_ -#include - #include "webrtc/modules/rtp_rtcp/interface/rtp_rtcp_defines.h" #include "webrtc/system_wrappers/interface/critical_section_wrapper.h" @@ -26,9 +24,6 @@ class SendSideBandwidthEstimation { void CurrentEstimate(uint32_t* bitrate, uint8_t* loss, uint32_t* rtt) const; - // Call periodically to update estimate. - void UpdateEstimate(uint32_t now_ms); - // Call when we receive a RTCP message with TMMBR or REMB. void UpdateReceiverEstimate(uint32_t bandwidth); @@ -43,15 +38,9 @@ class SendSideBandwidthEstimation { void SetMinBitrate(uint32_t min_bitrate); private: + void UpdateEstimate(uint32_t now_ms); void CapBitrateToThresholds(); - // Updates history of min bitrates. - // After this method returns min_bitrate_history_.front().second contains the - // min bitrate used during last kBweIncreaseIntervalMs. - void UpdateMinHistory(uint32_t now_ms); - - std::deque > min_bitrate_history_; - // incoming filters int accumulate_lost_packets_Q8_; int accumulate_expected_packets_; @@ -60,12 +49,12 @@ class SendSideBandwidthEstimation { uint32_t min_bitrate_configured_; uint32_t max_bitrate_configured_; - uint32_t time_last_receiver_block_ms_; uint8_t last_fraction_loss_; - uint16_t last_round_trip_time_ms_; + uint16_t last_round_trip_time_; uint32_t bwe_incoming_; - uint32_t time_last_decrease_ms_; + uint32_t time_last_increase_; + uint32_t time_last_decrease_; }; } // namespace webrtc #endif // WEBRTC_MODULES_BITRATE_CONTROLLER_SEND_SIDE_BANDWIDTH_ESTIMATION_H_ diff --git a/webrtc/video/video_send_stream_tests.cc b/webrtc/video/video_send_stream_tests.cc index 656bfab3e1..5172258684 100644 --- a/webrtc/video/video_send_stream_tests.cc +++ b/webrtc/video/video_send_stream_tests.cc @@ -1123,12 +1123,9 @@ TEST_F(VideoSendStreamTest, ProducesStats) { // indicate that it should be lowered significantly. The test then observes that // the bitrate observed is sinking well below the min-transmit-bitrate threshold // to verify that the min-transmit bitrate respects incoming REMB. -// -// Note that the test starts at "high" bitrate and does not ramp up to "higher" -// bitrate since no receiver block or remb is sent in the initial phase. TEST_F(VideoSendStreamTest, MinTransmitBitrateRespectsRemb) { static const int kMinTransmitBitrateBps = 400000; - static const int kHighBitrateBps = 150000; + static const int kHighBitrateBps = 200000; static const int kRembBitrateBps = 80000; static const int kRembRespectedBitrateBps = 100000; class BitrateObserver: public test::RtpRtcpObserver, public PacketReceiver { diff --git a/webrtc/video_engine/vie_channel_group.cc b/webrtc/video_engine/vie_channel_group.cc index 7cfb445962..9ce52a220d 100644 --- a/webrtc/video_engine/vie_channel_group.cc +++ b/webrtc/video_engine/vie_channel_group.cc @@ -160,13 +160,10 @@ class WrappingBitrateEstimator : public RemoteBitrateEstimator { }; } // namespace -ChannelGroup::ChannelGroup(int engine_id, - ProcessThread* process_thread, +ChannelGroup::ChannelGroup(int engine_id, ProcessThread* process_thread, const Config* config) : remb_(new VieRemb()), - bitrate_controller_( - BitrateController::CreateBitrateController(Clock::GetRealTimeClock(), - true)), + bitrate_controller_(BitrateController::CreateBitrateController(true)), call_stats_(new CallStats()), encoder_state_feedback_(new EncoderStateFeedback()), config_(config), @@ -178,21 +175,16 @@ ChannelGroup::ChannelGroup(int engine_id, } assert(config_); // Must have a valid config pointer here. remote_bitrate_estimator_.reset( - new WrappingBitrateEstimator(engine_id, - remb_.get(), - Clock::GetRealTimeClock(), - process_thread, + new WrappingBitrateEstimator(engine_id, remb_.get(), + Clock::GetRealTimeClock(), process_thread, *config_)), - call_stats_->RegisterStatsObserver(remote_bitrate_estimator_.get()); - + call_stats_->RegisterStatsObserver(remote_bitrate_estimator_.get()); process_thread->RegisterModule(call_stats_.get()); - process_thread->RegisterModule(bitrate_controller_.get()); } ChannelGroup::~ChannelGroup() { - process_thread_->DeRegisterModule(bitrate_controller_.get()); - process_thread_->DeRegisterModule(call_stats_.get()); call_stats_->DeregisterStatsObserver(remote_bitrate_estimator_.get()); + process_thread_->DeRegisterModule(call_stats_.get()); assert(channels_.empty()); assert(!remb_->InUse()); }