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 <perkj@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27661}diff --git a/modules/congestion_controller/goog_cc/delay_based_bwe.cc b/modules/congestion_controller/goog_cc/delay_based_bwe.cc
index 665c11d..b63b84a 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 @@
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 @@
// 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 90cda33..ffe118a 100644
--- a/modules/remote_bitrate_estimator/aimd_rate_control.cc
+++ b/modules/remote_bitrate_estimator/aimd_rate_control.cc
@@ -32,7 +32,7 @@
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 @@
} // 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 @@
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 @@
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 @@
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::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 4b51c94..d1a1caa 100644
--- a/modules/remote_bitrate_estimator/aimd_rate_control.h
+++ b/modules/remote_bitrate_estimator/aimd_rate_control.h
@@ -31,6 +31,7 @@
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 @@
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<NetworkStateEstimate>& estimate);
@@ -98,8 +100,13 @@
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<DataRate> last_decrease_;
FieldTrialOptional<TimeDelta> 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 0c93ca8..caaafa8 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 @@
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 @@
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
