From 6d191800301ba9fbdc8791e08872aa0270ee5389 Mon Sep 17 00:00:00 2001 From: Anastasia Koloskova Date: Mon, 11 Jun 2018 11:44:29 +0200 Subject: [PATCH] Fix increase in send rate when not receiving feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Store the last known throughput estimate and use that if we're lacking a new measurement. Bug: webrtc:9363 Change-Id: Ib7a9a495b446bd0f5799382cc095ccff894e0c2b Reviewed-on: https://webrtc-review.googlesource.com/81749 Commit-Queue: Anastasia Koloskova Reviewed-by: Sebastian Jansson Reviewed-by: Björn Terelius Cr-Commit-Position: refs/heads/master@{#23565} --- .../aimd_rate_control.cc | 7 +++++- .../aimd_rate_control.h | 1 + .../aimd_rate_control_unittest.cc | 24 ++++++++++++++++++- 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/modules/remote_bitrate_estimator/aimd_rate_control.cc b/modules/remote_bitrate_estimator/aimd_rate_control.cc index 19bfbb9a6d..52a41e41f2 100644 --- a/modules/remote_bitrate_estimator/aimd_rate_control.cc +++ b/modules/remote_bitrate_estimator/aimd_rate_control.cc @@ -58,6 +58,7 @@ AimdRateControl::AimdRateControl() : min_configured_bitrate_bps_(congestion_controller::GetMinBitrateBps()), max_configured_bitrate_bps_(30000000), current_bitrate_bps_(max_configured_bitrate_bps_), + latest_incoming_bitrate_bps_(current_bitrate_bps_), avg_max_bitrate_kbps_(-1.0f), var_max_bitrate_kbps_(0.4f), rate_control_state_(kRcHold), @@ -79,6 +80,7 @@ AimdRateControl::~AimdRateControl() {} void AimdRateControl::SetStartBitrate(int start_bitrate_bps) { current_bitrate_bps_ = start_bitrate_bps; + latest_incoming_bitrate_bps_ = current_bitrate_bps_; bitrate_is_initialized_ = true; } @@ -132,6 +134,7 @@ uint32_t AimdRateControl::Update(const RateControlInput* input, // Set the initial bit rate value to what we're receiving the first half // second. + // TODO(bugs.webrtc.org/9379): The comment above doesn't match to the code. if (!bitrate_is_initialized_) { const int64_t kInitializationTimeMs = 5000; RTC_DCHECK_LE(kBitrateWindowMs, kInitializationTimeMs); @@ -187,7 +190,9 @@ uint32_t AimdRateControl::ChangeBitrate(uint32_t new_bitrate_bps, const RateControlInput& input, int64_t now_ms) { uint32_t incoming_bitrate_bps = - input.incoming_bitrate.value_or(current_bitrate_bps_); + input.incoming_bitrate.value_or(latest_incoming_bitrate_bps_); + if (input.incoming_bitrate) + latest_incoming_bitrate_bps_ = *input.incoming_bitrate; // An over-use should always trigger us to reduce the bitrate, even though // we have not yet established our first estimate. By acting on the over-use, diff --git a/modules/remote_bitrate_estimator/aimd_rate_control.h b/modules/remote_bitrate_estimator/aimd_rate_control.h index a9ecb67c34..9791c1d46c 100644 --- a/modules/remote_bitrate_estimator/aimd_rate_control.h +++ b/modules/remote_bitrate_estimator/aimd_rate_control.h @@ -75,6 +75,7 @@ class AimdRateControl { uint32_t min_configured_bitrate_bps_; uint32_t max_configured_bitrate_bps_; uint32_t current_bitrate_bps_; + uint32_t latest_incoming_bitrate_bps_; float avg_max_bitrate_kbps_; float var_max_bitrate_kbps_; RateControlState rate_control_state_; diff --git a/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc b/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc index f6844b0e94..de88682f67 100644 --- a/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc +++ b/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc @@ -43,7 +43,7 @@ AimdRateControlStates CreateAimdRateControlStates() { void UpdateRateControl(const AimdRateControlStates& states, const BandwidthUsage& bandwidth_usage, - int bitrate, + rtc::Optional bitrate, int64_t now_ms) { RateControlInput input(bandwidth_usage, bitrate, now_ms); states.aimd_rate_control->Update(&input, now_ms); @@ -247,4 +247,26 @@ TEST(AimdRateControlTest, BandwidthPeriodIsNotAboveMaxNoSmoothingExp) { states.aimd_rate_control->GetExpectedBandwidthPeriodMs()); } +TEST(AimdRateControlTest, SendingRateBoundedWhenThroughputNotEstimated) { + auto states = CreateAimdRateControlStates(); + constexpr int kInitialBitrateBps = 123000; + UpdateRateControl(states, BandwidthUsage::kBwNormal, kInitialBitrateBps, + states.simulated_clock->TimeInMilliseconds()); + // AimdRateControl sets the initial bit rate to what it receives after + // five seconds has passed. + // TODO(bugs.webrtc.org/9379): The comment in the AimdRateControl does not + // match the constant. + constexpr int kInitializationTimeMs = 5000; + states.simulated_clock->AdvanceTimeMilliseconds(kInitializationTimeMs + 1); + UpdateRateControl(states, BandwidthUsage::kBwNormal, kInitialBitrateBps, + states.simulated_clock->TimeInMilliseconds()); + for (int i = 0; i < 100; ++i) { + UpdateRateControl(states, BandwidthUsage::kBwNormal, rtc::nullopt, + states.simulated_clock->TimeInMilliseconds()); + states.simulated_clock->AdvanceTimeMilliseconds(100); + } + EXPECT_LE(states.aimd_rate_control->LatestEstimate(), + kInitialBitrateBps * 1.5 + 10000); +} + } // namespace webrtc