From 865d94e45258c9c8876ea4cbdd5dade510cb7d93 Mon Sep 17 00:00:00 2001 From: Per Kjellander Date: Fri, 18 Mar 2022 10:49:28 +0000 Subject: [PATCH] Revert "Apply lower bound of delay based estimate in AimdRateControl::ClampBitrate" This reverts commit e66e6a845b50f212ebb60234446cfc9db897879c. Reason for revert: May cause BWE to increase on delay increase if link capacity estimate is too high. Original change's description: > Apply lower bound of delay based estimate in AimdRateControl::ClampBitrate > > This move the functionality of applying the lower bound of a network estimate to AimdRateControl::ClampBitrate instead of ChangeBitrate. > The purpose is to be able to also clamp probe estimates set by AimdRateControl::SetEstimate as well. > > Bug: none > Change-Id: I6a4d64d2e98bb99da06010e2edaf20dc42880e37 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/255823 > Reviewed-by: Per Kjellander > Commit-Queue: Per Kjellander > Reviewed-by: Diep Bui > Cr-Commit-Position: refs/heads/main@{#36219} Bug: none Change-Id: I8c65b1461160dbf3d35e50ef2cc6f9bc305c2b15 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/256011 Bot-Commit: rubber-stamper@appspot.gserviceaccount.com Commit-Queue: Per Kjellander Reviewed-by: Diep Bui Cr-Commit-Position: refs/heads/main@{#36248} --- .../remote_bitrate_estimator/aimd_rate_control.cc | 11 +++++------ .../aimd_rate_control_unittest.cc | 13 +------------ 2 files changed, 6 insertions(+), 18 deletions(-) diff --git a/modules/remote_bitrate_estimator/aimd_rate_control.cc b/modules/remote_bitrate_estimator/aimd_rate_control.cc index 3a4ba85a0f..1714dd115a 100644 --- a/modules/remote_bitrate_estimator/aimd_rate_control.cc +++ b/modules/remote_bitrate_estimator/aimd_rate_control.cc @@ -338,6 +338,11 @@ 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; @@ -381,12 +386,6 @@ 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::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 e217a1a7bc..5d9c328e06 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, SetEstimateUpperLimitedByNetworkEstimate) { +TEST(AimdRateControlTest, SetEstimateClampedByNetworkEstimate) { auto states = CreateAimdRateControlStates(/*send_side=*/true); NetworkStateEstimate network_estimate; network_estimate.link_capacity_upper = DataRate::KilobitsPerSec(400); @@ -265,17 +265,6 @@ TEST(AimdRateControlTest, SetEstimateUpperLimitedByNetworkEstimate) { 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, SetEstimateIgnoresNetworkEstimatesLowerThanCurrent) { test::ScopedFieldTrials override_field_trials( "WebRTC-Bwe-EstimateBoundedIncrease/"