From 416d5db75d8e4a6d746c6e823e64f705696f57a4 Mon Sep 17 00:00:00 2001 From: Per Kjellander Date: Wed, 17 Apr 2019 12:53:23 +0200 Subject: [PATCH] Add field trial to AimdRateController to only increase while not in ALR The idea is that when ALR is detected, the encoder can not produce the bitrate needed for the delay based estimator to detect overuse and thus the delay based estimator should not be allowed to increase further. Likewise, if ALR is not detected, the delay based estimator is allowed to increase the BWE to ensure that there is no region where the BWE can get stuck. BUG=webrtc:10542 Change-Id: Ic94b708461c9077fd09132ee4ecb6279ffcd5f99 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/133190 Commit-Queue: Per Kjellander Reviewed-by: Sebastian Jansson Cr-Commit-Position: refs/heads/master@{#27661} --- .../goog_cc/delay_based_bwe.cc | 3 +- .../aimd_rate_control.cc | 67 +++++++++++++------ .../aimd_rate_control.h | 7 ++ .../aimd_rate_control_unittest.cc | 63 ++++++++++++++++- 4 files changed, 117 insertions(+), 23 deletions(-) diff --git a/modules/congestion_controller/goog_cc/delay_based_bwe.cc b/modules/congestion_controller/goog_cc/delay_based_bwe.cc index 665c11dc2b..b63b84a13a 100644 --- a/modules/congestion_controller/goog_cc/delay_based_bwe.cc +++ b/modules/congestion_controller/goog_cc/delay_based_bwe.cc @@ -90,7 +90,7 @@ DelayBasedBwe::DelayBasedBwe(const WebRtcKeyValueConfig* key_value_config, delay_detector_(), last_seen_packet_(Timestamp::MinusInfinity()), uma_recorded_(false), - rate_control_(key_value_config), + rate_control_(key_value_config, /*send_side=*/true), trendline_window_size_( key_value_config->Lookup(kBweWindowSizeInPacketsExperiment) .find("Enabled") == 0 @@ -160,6 +160,7 @@ DelayBasedBwe::Result DelayBasedBwe::IncomingPacketFeedbackVector( // against building very large network queues. return Result(); } + rate_control_.SetInApplicationLimitedRegion(in_alr); rate_control_.SetNetworkStateEstimate(network_estimate); return MaybeUpdateEstimate(acked_bitrate, probe_bitrate, std::move(network_estimate), diff --git a/modules/remote_bitrate_estimator/aimd_rate_control.cc b/modules/remote_bitrate_estimator/aimd_rate_control.cc index 90cda33e35..ffe118a50c 100644 --- a/modules/remote_bitrate_estimator/aimd_rate_control.cc +++ b/modules/remote_bitrate_estimator/aimd_rate_control.cc @@ -32,7 +32,7 @@ namespace { constexpr TimeDelta kDefaultRtt = TimeDelta::Millis<200>(); constexpr double kDefaultBackoffFactor = 0.85; -const char kBweBackOffFactorExperiment[] = "WebRTC-BweBackOffFactor"; +constexpr char kBweBackOffFactorExperiment[] = "WebRTC-BweBackOffFactor"; bool IsEnabled(const WebRtcKeyValueConfig& field_trials, absl::string_view key) { @@ -62,6 +62,10 @@ double ReadBackoffFactor(const WebRtcKeyValueConfig& key_value_config) { } // namespace AimdRateControl::AimdRateControl(const WebRtcKeyValueConfig* key_value_config) + : AimdRateControl(key_value_config, /* send_side =*/false) {} + +AimdRateControl::AimdRateControl(const WebRtcKeyValueConfig* key_value_config, + bool send_side) : min_configured_bitrate_(congestion_controller::GetMinBitrate()), max_configured_bitrate_(DataRate::kbps(30000)), current_bitrate_(max_configured_bitrate_), @@ -75,8 +79,13 @@ AimdRateControl::AimdRateControl(const WebRtcKeyValueConfig* key_value_config) beta_(IsEnabled(*key_value_config, kBweBackOffFactorExperiment) ? ReadBackoffFactor(*key_value_config) : kDefaultBackoffFactor), + in_alr_(false), rtt_(kDefaultRtt), + send_side_(send_side), in_experiment_(!AdaptiveThresholdExperimentIsDisabled(*key_value_config)), + no_bitrate_increase_in_alr_( + IsEnabled(*key_value_config, + "WebRTC-DontIncreaseDelayBasedBweInAlr")), smoothing_experiment_( IsEnabled(*key_value_config, "WebRTC-Audio-BandwidthSmoothing")), initial_backoff_interval_("initial_backoff_interval"), @@ -192,6 +201,10 @@ DataRate AimdRateControl::Update(const RateControlInput* input, return current_bitrate_; } +void AimdRateControl::SetInApplicationLimitedRegion(bool in_alr) { + in_alr_ = in_alr; +} + void AimdRateControl::SetEstimate(DataRate bitrate, Timestamp at_time) { bitrate_is_initialized_ = true; DataRate prev_bitrate = current_bitrate_; @@ -265,19 +278,25 @@ DataRate AimdRateControl::ChangeBitrate(DataRate new_bitrate, if (estimated_throughput > link_capacity_.UpperBound()) link_capacity_.Reset(); - 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 increase. - DataRate additive_increase = - AdditiveRateIncrease(at_time, time_last_bitrate_change_); - new_bitrate += additive_increase; - } else { - // If we don't have an estimate of the link capacity, use faster ramp up - // to discover the capacity. - DataRate multiplicative_increase = MultiplicativeRateIncrease( - at_time, time_last_bitrate_change_, new_bitrate); - new_bitrate += multiplicative_increase; + // 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. + if (!(send_side_ && in_alr_ && no_bitrate_increase_in_alr_)) { + 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 + // increase. + DataRate additive_increase = + AdditiveRateIncrease(at_time, time_last_bitrate_change_); + new_bitrate += additive_increase; + } else { + // If we don't have an estimate of the link capacity, use faster ramp + // up to discover the capacity. + DataRate multiplicative_increase = MultiplicativeRateIncrease( + at_time, time_last_bitrate_change_, new_bitrate); + new_bitrate += multiplicative_increase; + } } time_last_bitrate_change_ = at_time; @@ -352,13 +371,21 @@ DataRate AimdRateControl::ChangeBitrate(DataRate new_bitrate, DataRate AimdRateControl::ClampBitrate(DataRate new_bitrate, DataRate estimated_throughput) const { - // Don't change the bit rate if the send side is too far off. - // We allow a bit more lag at very low rates to not too easily get stuck if - // the encoder produces uneven outputs. - const DataRate max_bitrate = 1.5 * estimated_throughput + DataRate::kbps(10); - if (new_bitrate > current_bitrate_ && new_bitrate > max_bitrate) { - new_bitrate = std::max(current_bitrate_, max_bitrate); + // Allow the estimate to increase as long as alr is not detected to ensure + // that there is no BWE values that can make the estimate stuck at a too + // low bitrate. If an encoder can not produce the bitrate necessary to + // fully use the capacity, alr will sooner or later trigger. + if (!(send_side_ && no_bitrate_increase_in_alr_)) { + // Don't change the bit rate if the send side is too far off. + // We allow a bit more lag at very low rates to not too easily get stuck if + // the encoder produces uneven outputs. + const DataRate max_bitrate = + 1.5 * estimated_throughput + DataRate::kbps(10); + if (new_bitrate > current_bitrate_ && new_bitrate > max_bitrate) { + new_bitrate = std::max(current_bitrate_, max_bitrate); + } } + if (network_estimate_ && capacity_limit_deviation_factor_) { DataRate upper_bound = network_estimate_->link_capacity + network_estimate_->link_capacity_std_dev * diff --git a/modules/remote_bitrate_estimator/aimd_rate_control.h b/modules/remote_bitrate_estimator/aimd_rate_control.h index 4b51c9462d..d1a1caa12a 100644 --- a/modules/remote_bitrate_estimator/aimd_rate_control.h +++ b/modules/remote_bitrate_estimator/aimd_rate_control.h @@ -31,6 +31,7 @@ namespace webrtc { class AimdRateControl { public: explicit AimdRateControl(const WebRtcKeyValueConfig* key_value_config); + AimdRateControl(const WebRtcKeyValueConfig* key_value_config, bool send_side); ~AimdRateControl(); // Returns true if the target bitrate has been initialized. This happens @@ -53,6 +54,7 @@ class AimdRateControl { DataRate LatestEstimate() const; void SetRtt(TimeDelta rtt); DataRate Update(const RateControlInput* input, Timestamp at_time); + void SetInApplicationLimitedRegion(bool in_alr); void SetEstimate(DataRate bitrate, Timestamp at_time); void SetNetworkStateEstimate( const absl::optional& estimate); @@ -98,8 +100,13 @@ class AimdRateControl { Timestamp time_first_throughput_estimate_; bool bitrate_is_initialized_; double beta_; + bool in_alr_; TimeDelta rtt_; + const bool send_side_; const bool in_experiment_; + // 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_; const bool smoothing_experiment_; absl::optional last_decrease_; FieldTrialOptional initial_backoff_interval_; diff --git a/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc b/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc index 0c93ca86c4..caaafa88b4 100644 --- a/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc +++ b/modules/remote_bitrate_estimator/aimd_rate_control_unittest.cc @@ -36,9 +36,10 @@ struct AimdRateControlStates { FieldTrialBasedConfig field_trials; }; -AimdRateControlStates CreateAimdRateControlStates() { +AimdRateControlStates CreateAimdRateControlStates(bool send_side = false) { AimdRateControlStates states; - states.aimd_rate_control.reset(new AimdRateControl(&states.field_trials)); + states.aimd_rate_control.reset( + new AimdRateControl(&states.field_trials, send_side)); states.simulated_clock.reset(new SimulatedClock(kClockInitialTime)); return states; } @@ -278,4 +279,62 @@ TEST(AimdRateControlTest, SendingRateBoundedWhenThroughputNotEstimated) { kInitialBitrateBps * 1.5 + 10000); } +TEST(AimdRateControlTest, EstimateDoesNotIncreaseInAlr) { + // 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. + test::ScopedFieldTrials override_field_trials( + "WebRTC-DontIncreaseDelayBasedBweInAlr/Enabled/"); + auto states = CreateAimdRateControlStates(/*send_side=*/true); + constexpr int kInitialBitrateBps = 123000; + SetEstimate(states, kInitialBitrateBps); + states.aimd_rate_control->SetInApplicationLimitedRegion(true); + UpdateRateControl(states, BandwidthUsage::kBwNormal, kInitialBitrateBps, + states.simulated_clock->TimeInMilliseconds()); + ASSERT_EQ(states.aimd_rate_control->LatestEstimate().bps(), + kInitialBitrateBps); + + for (int i = 0; i < 100; ++i) { + UpdateRateControl(states, BandwidthUsage::kBwNormal, absl::nullopt, + states.simulated_clock->TimeInMilliseconds()); + states.simulated_clock->AdvanceTimeMilliseconds(100); + } + EXPECT_EQ(states.aimd_rate_control->LatestEstimate().bps(), + kInitialBitrateBps); +} + +TEST(AimdRateControlTest, SetEstimateIncreaseBweInAlr) { + test::ScopedFieldTrials override_field_trials( + "WebRTC-DontIncreaseDelayBasedBweInAlr/Enabled/"); + auto states = CreateAimdRateControlStates(/*send_side=*/true); + constexpr int kInitialBitrateBps = 123000; + SetEstimate(states, kInitialBitrateBps); + states.aimd_rate_control->SetInApplicationLimitedRegion(true); + ASSERT_EQ(states.aimd_rate_control->LatestEstimate().bps(), + kInitialBitrateBps); + SetEstimate(states, 2 * kInitialBitrateBps); + EXPECT_EQ(states.aimd_rate_control->LatestEstimate().bps(), + 2 * kInitialBitrateBps); +} + +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. + test::ScopedFieldTrials override_field_trials( + "WebRTC-DontIncreaseDelayBasedBweInAlr/Enabled/"); + auto states = CreateAimdRateControlStates(/*send_side=*/true); + constexpr int kInitialBitrateBps = 123000; + SetEstimate(states, kInitialBitrateBps); + states.aimd_rate_control->SetInApplicationLimitedRegion(false); + UpdateRateControl(states, BandwidthUsage::kBwNormal, kInitialBitrateBps, + states.simulated_clock->TimeInMilliseconds()); + for (int i = 0; i < 100; ++i) { + UpdateRateControl(states, BandwidthUsage::kBwNormal, absl::nullopt, + states.simulated_clock->TimeInMilliseconds()); + states.simulated_clock->AdvanceTimeMilliseconds(100); + } + EXPECT_GT(states.aimd_rate_control->LatestEstimate().bps(), + kInitialBitrateBps); +} + } // namespace webrtc