From 755ffa7e8a26896a29851bba3a09c25152fd68a3 Mon Sep 17 00:00:00 2001 From: Per K Date: Mon, 3 Apr 2023 15:44:20 +0200 Subject: [PATCH] Remove unused field trial parameters from AimdRateControl. ratio and ignore_acked where added here https://webrtc-review.googlesource.com/c/src/+/252442 ignore_decr https://webrtc-review.googlesource.com/c/src/+/253901 Bug: none Change-Id: I85008dfa70bf57bfefb453f9f1d1929510c7b825 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298045 Reviewed-by: Diep Bui Commit-Queue: Per Kjellander Cr-Commit-Position: refs/heads/main@{#39751} --- .../aimd_rate_control.cc | 22 +-- .../aimd_rate_control.h | 10 +- .../aimd_rate_control_unittest.cc | 145 ------------------ 3 files changed, 5 insertions(+), 172 deletions(-) diff --git a/modules/remote_bitrate_estimator/aimd_rate_control.cc b/modules/remote_bitrate_estimator/aimd_rate_control.cc index 830aaa6e3c..129e82492f 100644 --- a/modules/remote_bitrate_estimator/aimd_rate_control.cc +++ b/modules/remote_bitrate_estimator/aimd_rate_control.cc @@ -83,9 +83,7 @@ AimdRateControl::AimdRateControl(const FieldTrialsView& key_value_config, initial_backoff_interval_("initial_backoff_interval"), link_capacity_fix_("link_capacity_fix") { ParseFieldTrial( - {&disable_estimate_bounded_increase_, &estimate_bounded_increase_ratio_, - &ignore_throughput_limit_if_network_estimate_, - &ignore_network_estimate_decrease_, &increase_to_network_estimate_}, + {&disable_estimate_bounded_increase_}, key_value_config.Lookup("WebRTC-Bwe-EstimateBoundedIncrease")); // E.g // WebRTC-BweAimdRateControlConfig/initial_backoff_interval:100ms/ @@ -270,12 +268,7 @@ void AimdRateControl::ChangeBitrate(const RateControlInput& input, // easily get stuck if the encoder produces uneven outputs. DataRate increase_limit = 1.5 * estimated_throughput + DataRate::KilobitsPerSec(10); - if (ignore_throughput_limit_if_network_estimate_ && network_estimate_ && - network_estimate_->link_capacity_upper.IsFinite()) { - // If we have a Network estimate, we do allow the estimate to increase. - increase_limit = network_estimate_->link_capacity_upper * - estimate_bounded_increase_ratio_.Get(); - } else if (send_side_ && in_alr_ && no_bitrate_increase_in_alr_) { + if (send_side_ && in_alr_ && no_bitrate_increase_in_alr_) { // Do not increase the delay based estimate in alr since the estimator // will not be able to get transport feedback necessary to detect if // the new estimate is correct. @@ -286,10 +279,7 @@ void AimdRateControl::ChangeBitrate(const RateControlInput& input, if (current_bitrate_ < increase_limit) { DataRate increased_bitrate = DataRate::MinusInfinity(); - if (increase_to_network_estimate_ && network_estimate_ && - network_estimate_->link_capacity_upper.IsFinite()) { - increased_bitrate = increase_limit; - } else if (link_capacity_.has_estimate()) { + if (link_capacity_.has_estimate()) { // The link_capacity estimate is reset if the measured throughput // is too far from the estimate. We can therefore assume that our // target rate is reasonably close to link capacity and use additive @@ -360,11 +350,7 @@ 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 * - estimate_bounded_increase_ratio_.Get(); - if (ignore_network_estimate_decrease_) { - upper_bound = std::max(upper_bound, current_bitrate_); - } + DataRate upper_bound = 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 4c56a2dbb1..91d5a3edb8 100644 --- a/modules/remote_bitrate_estimator/aimd_rate_control.h +++ b/modules/remote_bitrate_estimator/aimd_rate_control.h @@ -103,16 +103,8 @@ class AimdRateControl { // Allow the delay based estimate to only increase as long as application // limited region (alr) is not detected. const bool no_bitrate_increase_in_alr_; - // If false, uses estimated link capacity upper bound * - // `estimate_bounded_increase_ratio_` as upper limit for the estimate. + // If "Disabled", estimated link capacity is not used as upper bound. FieldTrialFlag disable_estimate_bounded_increase_{"Disabled"}; - FieldTrialParameter estimate_bounded_increase_ratio_{"ratio", 1.0}; - FieldTrialParameter ignore_throughput_limit_if_network_estimate_{ - "ignore_acked", false}; - FieldTrialParameter increase_to_network_estimate_{"immediate_incr", - false}; - FieldTrialParameter ignore_network_estimate_decrease_{"ignore_decr", - 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 814c14624e..8a5ac849a3 100644 --- a/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc +++ b/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc @@ -239,27 +239,6 @@ TEST(AimdRateControlTest, EXPECT_EQ(aimd_rate_control.LatestEstimate().kbps(), 200); } -TEST(AimdRateControlTest, SetEstimateIgnoresNetworkEstimatesLowerThanCurrent) { - ExplicitKeyValueConfig field_trials( - "WebRTC-Bwe-EstimateBoundedIncrease/" - "ratio:0.85,ignore_acked:true,ignore_decr:true/"); - AimdRateControl aimd_rate_control(field_trials, /*send_side=*/true); - aimd_rate_control.SetStartBitrate(DataRate::KilobitsPerSec(30)); - NetworkStateEstimate network_estimate; - network_estimate.link_capacity_upper = DataRate::KilobitsPerSec(400); - aimd_rate_control.SetNetworkStateEstimate(network_estimate); - aimd_rate_control.SetEstimate(DataRate::KilobitsPerSec(500), kInitialTime); - ASSERT_EQ(aimd_rate_control.LatestEstimate(), - network_estimate.link_capacity_upper * 0.85); - - NetworkStateEstimate lower_network_estimate; - lower_network_estimate.link_capacity_upper = DataRate::KilobitsPerSec(300); - aimd_rate_control.SetNetworkStateEstimate(lower_network_estimate); - aimd_rate_control.SetEstimate(DataRate::KilobitsPerSec(500), kInitialTime); - EXPECT_EQ(aimd_rate_control.LatestEstimate(), - network_estimate.link_capacity_upper * 0.85); -} - TEST(AimdRateControlTest, EstimateIncreaseWhileNotInAlr) { // Allow the estimate to increase as long as alr is not detected to ensure // tha BWE can not get stuck at a certain bitrate. @@ -298,128 +277,4 @@ TEST(AimdRateControlTest, EstimateNotLimitedByNetworkEstimateIfDisabled) { network_estimate.link_capacity_upper); } -TEST(AimdRateControlTest, - EstimateSlowlyIncreaseToUpperLinkCapacityEstimateIfConfigured) { - // Even if alr is detected, the delay based estimator is allowed to increase - // up to a percentage of upper link capacity. - ExplicitKeyValueConfig field_trials( - "WebRTC-Bwe-EstimateBoundedIncrease/" - "ratio:0.85,ignore_acked:true,immediate_incr:false/" - "WebRTC-DontIncreaseDelayBasedBweInAlr/Enabled/"); - AimdRateControl aimd_rate_control(field_trials, /*send_side=*/true); - Timestamp now = kInitialTime; - constexpr DataRate kInitialBitrate = DataRate::BitsPerSec(123'000); - aimd_rate_control.SetEstimate(kInitialBitrate, now); - aimd_rate_control.SetInApplicationLimitedRegion(true); - - NetworkStateEstimate network_estimate; - network_estimate.link_capacity_upper = DataRate::KilobitsPerSec(200); - aimd_rate_control.SetNetworkStateEstimate(network_estimate); - for (int i = 0; i < 10; ++i) { - aimd_rate_control.Update({BandwidthUsage::kBwNormal, absl::nullopt}, now); - now += TimeDelta::Millis(100); - EXPECT_LT(aimd_rate_control.LatestEstimate(), - network_estimate.link_capacity_upper * 0.85); - } - for (int i = 0; i < 50; ++i) { - aimd_rate_control.Update({BandwidthUsage::kBwNormal, absl::nullopt}, now); - now += TimeDelta::Millis(100); - } - EXPECT_EQ(aimd_rate_control.LatestEstimate(), - network_estimate.link_capacity_upper * 0.85); -} - -TEST(AimdRateControlTest, - EstimateImmediatelyIncreaseToUpperLinkCapacityEstimateIfConfigured) { - // Even if alr is detected, the delay based estimator is allowed to increase - // up to a percentage of upper link capacity. - ExplicitKeyValueConfig field_trials( - "WebRTC-Bwe-EstimateBoundedIncrease/" - "ratio:0.85,ignore_acked:true,immediate_incr:true/" - "WebRTC-DontIncreaseDelayBasedBweInAlr/Enabled/"); - AimdRateControl aimd_rate_control(field_trials, /*send_side=*/true); - - constexpr DataRate kInitialBitrate = DataRate::BitsPerSec(123'000); - aimd_rate_control.SetEstimate(kInitialBitrate, kInitialTime); - aimd_rate_control.SetInApplicationLimitedRegion(true); - - NetworkStateEstimate network_estimate; - network_estimate.link_capacity_upper = DataRate::KilobitsPerSec(200); - aimd_rate_control.SetNetworkStateEstimate(network_estimate); - aimd_rate_control.Update({BandwidthUsage::kBwNormal, absl::nullopt}, - kInitialTime); - EXPECT_EQ(aimd_rate_control.LatestEstimate(), - network_estimate.link_capacity_upper * 0.85); -} - -TEST(AimdRateControlTest, EstimateNotLoweredByNetworkEstimate) { - // The delay based estimator is allowed to increase up to a percentage of - // upper link capacity but does not decrease unless the delay detector - // discover an overuse. - ExplicitKeyValueConfig field_trials( - "WebRTC-Bwe-EstimateBoundedIncrease/" - "ratio:0.85,ignore_acked:true,ignore_decr:true/" - "WebRTC-DontIncreaseDelayBasedBweInAlr/Enabled/"); - AimdRateControl aimd_rate_control(field_trials, /*send_side=*/true); - constexpr DataRate kInitialBitrate = DataRate::BitsPerSec(123'000); - constexpr DataRate kEstimatedThroughput = DataRate::BitsPerSec(30'000); - Timestamp now = kInitialTime; - - aimd_rate_control.SetEstimate(kInitialBitrate, now); - - NetworkStateEstimate network_estimate; - network_estimate.link_capacity_upper = DataRate::KilobitsPerSec(200); - aimd_rate_control.SetNetworkStateEstimate(network_estimate); - for (int i = 0; i < 100; ++i) { - aimd_rate_control.Update({BandwidthUsage::kBwNormal, kEstimatedThroughput}, - now); - now += TimeDelta::Millis(100); - } - DataRate estimate_after_increase = aimd_rate_control.LatestEstimate(); - ASSERT_EQ(estimate_after_increase, - network_estimate.link_capacity_upper * 0.85); - - // A lower network estimate does not decrease the estimate immediately, - // but the estimate is not allowed to increase. - network_estimate.link_capacity_upper = DataRate::KilobitsPerSec(100); - network_estimate.link_capacity_lower = DataRate::KilobitsPerSec(80); - aimd_rate_control.SetNetworkStateEstimate(network_estimate); - for (int i = 0; i < 10; ++i) { - aimd_rate_control.Update({BandwidthUsage::kBwNormal, kEstimatedThroughput}, - now); - now += TimeDelta::Millis(100); - EXPECT_EQ(aimd_rate_control.LatestEstimate(), estimate_after_increase); - } - - // If the detector detects and overuse, BWE drops to a value relative the - // network estimate. - aimd_rate_control.Update({BandwidthUsage::kBwOverusing, kEstimatedThroughput}, - now); - EXPECT_LT(aimd_rate_control.LatestEstimate(), - network_estimate.link_capacity_lower); - EXPECT_GT(aimd_rate_control.LatestEstimate(), kEstimatedThroughput); -} - -TEST(AimdRateControlTest, EstimateDoesNotIncreaseInAlrIfNetworkEstimateNotSet) { - // When alr is detected, the delay based estimator is not allowed to increase - // bwe since there will be no feedback from the network if the new estimate - // is correct. - ExplicitKeyValueConfig field_trials( - "WebRTC-Bwe-EstimateBoundedIncrease/ratio:0.85,ignore_acked:true/" - "WebRTC-DontIncreaseDelayBasedBweInAlr/Enabled/"); - AimdRateControl aimd_rate_control(field_trials, /*send_side=*/true); - constexpr DataRate kInitialBitrate = DataRate::BitsPerSec(123'000); - Timestamp now = kInitialTime; - aimd_rate_control.SetEstimate(kInitialBitrate, now); - aimd_rate_control.SetInApplicationLimitedRegion(true); - aimd_rate_control.Update({BandwidthUsage::kBwNormal, kInitialBitrate}, now); - ASSERT_EQ(aimd_rate_control.LatestEstimate(), kInitialBitrate); - - for (int i = 0; i < 100; ++i) { - aimd_rate_control.Update({BandwidthUsage::kBwNormal, absl::nullopt}, now); - now += TimeDelta::Millis(100); - } - EXPECT_EQ(aimd_rate_control.LatestEstimate(), kInitialBitrate); -} - } // namespace webrtc