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