From b5dedfc8562389f42121d4a44af32cfd802600bc Mon Sep 17 00:00:00 2001 From: Per K Date: Fri, 25 Aug 2023 14:18:47 +0200 Subject: [PATCH] In AimdRateControl, add trial to use current bitrate as min upper limit This is to ensure that a bad NetworkState estimate can not decrease BWE unless an delay BW overuse has been detected. Bug: webrtc:10489 Change-Id: Ic3a516345999eeba814200c2e295a19b347b2eb6 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/317800 Commit-Queue: Per Kjellander Reviewed-by: Diep Bui Auto-Submit: Per Kjellander Cr-Commit-Position: refs/heads/main@{#40628} --- .../aimd_rate_control.cc | 8 +++-- .../aimd_rate_control.h | 2 ++ .../aimd_rate_control_unittest.cc | 30 +++++++++++++++++++ 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/modules/remote_bitrate_estimator/aimd_rate_control.cc b/modules/remote_bitrate_estimator/aimd_rate_control.cc index b9abb785a5..686ebcf026 100644 --- a/modules/remote_bitrate_estimator/aimd_rate_control.cc +++ b/modules/remote_bitrate_estimator/aimd_rate_control.cc @@ -83,7 +83,8 @@ AimdRateControl::AimdRateControl(const FieldTrialsView& key_value_config, initial_backoff_interval_("initial_backoff_interval"), link_capacity_fix_("link_capacity_fix") { ParseFieldTrial( - {&disable_estimate_bounded_increase_}, + {&disable_estimate_bounded_increase_, + &use_current_estimate_as_min_upper_bound_}, key_value_config.Lookup("WebRTC-Bwe-EstimateBoundedIncrease")); // E.g // WebRTC-BweAimdRateControlConfig/initial_backoff_interval:100ms/ @@ -350,7 +351,10 @@ void AimdRateControl::ChangeBitrate(const RateControlInput& input, DataRate AimdRateControl::ClampBitrate(DataRate new_bitrate) const { if (!disable_estimate_bounded_increase_ && network_estimate_ && network_estimate_->link_capacity_upper.IsFinite()) { - DataRate upper_bound = network_estimate_->link_capacity_upper; + DataRate upper_bound = + use_current_estimate_as_min_upper_bound_ + ? std::max(network_estimate_->link_capacity_upper, current_bitrate_) + : network_estimate_->link_capacity_upper; new_bitrate = std::min(upper_bound, new_bitrate); } if (network_estimate_ && network_estimate_->link_capacity_lower.IsFinite() && diff --git a/modules/remote_bitrate_estimator/aimd_rate_control.h b/modules/remote_bitrate_estimator/aimd_rate_control.h index 91d5a3edb8..95459e3d6a 100644 --- a/modules/remote_bitrate_estimator/aimd_rate_control.h +++ b/modules/remote_bitrate_estimator/aimd_rate_control.h @@ -105,6 +105,8 @@ class AimdRateControl { const bool no_bitrate_increase_in_alr_; // If "Disabled", estimated link capacity is not used as upper bound. FieldTrialFlag disable_estimate_bounded_increase_{"Disabled"}; + FieldTrialParameter use_current_estimate_as_min_upper_bound_{"c_upper", + false}; absl::optional last_decrease_; FieldTrialOptional initial_backoff_interval_; FieldTrialFlag link_capacity_fix_; diff --git a/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc b/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc index 8a5ac849a3..5b8b0caffe 100644 --- a/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc +++ b/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc @@ -212,6 +212,36 @@ TEST(AimdRateControlTest, SetEstimateUpperLimitedByNetworkEstimate) { network_estimate.link_capacity_upper); } +TEST(AimdRateControlTest, + SetEstimateUpperLimitedByCurrentBitrateIfNetworkEstimateIsLow) { + AimdRateControl aimd_rate_control( + ExplicitKeyValueConfig( + "WebRTC-Bwe-EstimateBoundedIncrease/c_upper:true/"), + /*send_side=*/true); + aimd_rate_control.SetEstimate(DataRate::BitsPerSec(500'000), kInitialTime); + ASSERT_EQ(aimd_rate_control.LatestEstimate(), DataRate::BitsPerSec(500'000)); + + NetworkStateEstimate network_estimate; + network_estimate.link_capacity_upper = DataRate::BitsPerSec(300'000); + aimd_rate_control.SetNetworkStateEstimate(network_estimate); + aimd_rate_control.SetEstimate(DataRate::BitsPerSec(700'000), kInitialTime); + EXPECT_EQ(aimd_rate_control.LatestEstimate(), DataRate::BitsPerSec(500'000)); +} + +TEST(AimdRateControlTest, + SetEstimateDefaultNotUpperLimitedByCurrentBitrateIfNetworkEstimateIsLow) { + AimdRateControl aimd_rate_control(ExplicitKeyValueConfig(""), + /*send_side=*/true); + aimd_rate_control.SetEstimate(DataRate::BitsPerSec(500'000), kInitialTime); + ASSERT_EQ(aimd_rate_control.LatestEstimate(), DataRate::BitsPerSec(500'000)); + + NetworkStateEstimate network_estimate; + network_estimate.link_capacity_upper = DataRate::BitsPerSec(300'000); + aimd_rate_control.SetNetworkStateEstimate(network_estimate); + aimd_rate_control.SetEstimate(DataRate::BitsPerSec(700'000), kInitialTime); + EXPECT_EQ(aimd_rate_control.LatestEstimate(), DataRate::BitsPerSec(300'000)); +} + TEST(AimdRateControlTest, SetEstimateLowerLimitedByNetworkEstimate) { AimdRateControl aimd_rate_control(ExplicitKeyValueConfig(""), /*send_side=*/true);