From 0a5e12b07d920cc57cba9e1555ea295cece0ab5d Mon Sep 17 00:00:00 2001 From: Per Kjellander Date: Fri, 18 Mar 2022 15:13:52 +0100 Subject: [PATCH] Reland "Apply lower bound of delay based estimate in AimdRateControl::ClampBitrate" This reverts commit 865d94e45258c9c8876ea4cbdd5dade510cb7d93. First patch is the same as original cl. Second patch includes a fix to ensure the clamped bitrate does not increase to 85% of the last network estimate. Bug: none Change-Id: Idf1b2af3fb60c0d392c48c1b6c0d8526f900f9d6 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/256016 Reviewed-by: Diep Bui Commit-Queue: Per Kjellander Cr-Commit-Position: refs/heads/main@{#36270} --- .../aimd_rate_control.cc | 12 +++++---- .../aimd_rate_control_unittest.cc | 27 ++++++++++++++++++- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/modules/remote_bitrate_estimator/aimd_rate_control.cc b/modules/remote_bitrate_estimator/aimd_rate_control.cc index 1714dd115a..3297df9c83 100644 --- a/modules/remote_bitrate_estimator/aimd_rate_control.cc +++ b/modules/remote_bitrate_estimator/aimd_rate_control.cc @@ -338,11 +338,6 @@ void AimdRateControl::ChangeBitrate(const RateControlInput& input, decreased_bitrate = beta_ * link_capacity_.estimate(); } } - if (estimate_bounded_backoff_ && network_estimate_) { - decreased_bitrate = std::max( - decreased_bitrate, network_estimate_->link_capacity_lower * beta_); - } - // Avoid increasing the rate when over-using. if (decreased_bitrate < current_bitrate_) { new_bitrate = decreased_bitrate; @@ -386,6 +381,13 @@ DataRate AimdRateControl::ClampBitrate(DataRate new_bitrate) const { } new_bitrate = std::min(upper_bound, new_bitrate); } + if (estimate_bounded_backoff_ && network_estimate_ && + network_estimate_->link_capacity_lower.IsFinite() && + new_bitrate < current_bitrate_) { + new_bitrate = std::min( + current_bitrate_, + std::max(new_bitrate, network_estimate_->link_capacity_lower * beta_)); + } new_bitrate = std::max(new_bitrate, min_configured_bitrate_); return new_bitrate; } diff --git a/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc b/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc index 5d9c328e06..1fe5799545 100644 --- a/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc +++ b/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc @@ -255,7 +255,7 @@ TEST(AimdRateControlTest, SetEstimateIncreaseBweInAlr) { 2 * kInitialBitrateBps); } -TEST(AimdRateControlTest, SetEstimateClampedByNetworkEstimate) { +TEST(AimdRateControlTest, SetEstimateUpperLimitedByNetworkEstimate) { auto states = CreateAimdRateControlStates(/*send_side=*/true); NetworkStateEstimate network_estimate; network_estimate.link_capacity_upper = DataRate::KilobitsPerSec(400); @@ -265,6 +265,31 @@ TEST(AimdRateControlTest, SetEstimateClampedByNetworkEstimate) { network_estimate.link_capacity_upper); } +TEST(AimdRateControlTest, SetEstimateLowerLimitedByNetworkEstimate) { + auto states = CreateAimdRateControlStates(/*send_side=*/true); + NetworkStateEstimate network_estimate; + network_estimate.link_capacity_lower = DataRate::KilobitsPerSec(400); + states.aimd_rate_control->SetNetworkStateEstimate(network_estimate); + SetEstimate(states, 100'000); + // 0.85 is default backoff factor. (`beta_`) + EXPECT_EQ(states.aimd_rate_control->LatestEstimate(), + network_estimate.link_capacity_lower * 0.85); +} + +TEST(AimdRateControlTest, + SetEstimateIgnoredIfLowerThanNetworkEstimateAndCurrent) { + auto states = CreateAimdRateControlStates(/*send_side=*/true); + SetEstimate(states, 200'000); + ASSERT_EQ(states.aimd_rate_control->LatestEstimate().kbps(), 200); + NetworkStateEstimate network_estimate; + network_estimate.link_capacity_lower = DataRate::KilobitsPerSec(400); + states.aimd_rate_control->SetNetworkStateEstimate(network_estimate); + // Ignore the next SetEstimate, since the estimate is lower than 85% of + // the network estimate. + SetEstimate(states, 100'000); + EXPECT_EQ(states.aimd_rate_control->LatestEstimate().kbps(), 200); +} + TEST(AimdRateControlTest, SetEstimateIgnoresNetworkEstimatesLowerThanCurrent) { test::ScopedFieldTrials override_field_trials( "WebRTC-Bwe-EstimateBoundedIncrease/"