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}

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),

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()) {
+      // Do not increase the delay based estimate in alr since the estimator
-        // The link_capacity estimate is reset if the measured throughput
+      // will not be able to get transport feedback necessary to detect if
-        // is too far from the estimate. We can therefore assume that our target
+      // the new estimate is correct.
-        // rate is reasonably close to link capacity and use additive increase.
+      if (!(send_side_ && in_alr_ && no_bitrate_increase_in_alr_)) {
-        DataRate additive_increase =
+        if (link_capacity_.has_estimate()) {
-            AdditiveRateIncrease(at_time, time_last_bitrate_change_);
+          // The link_capacity estimate is reset if the measured throughput
-        new_bitrate += additive_increase;
+          // is too far from the estimate. We can therefore assume that our
-      } else {
+          // target rate is reasonably close to link capacity and use additive
-        // If we don't have an estimate of the link capacity, use faster ramp up
+          // increase.
-        // to discover the capacity.
+          DataRate additive_increase =
-        DataRate multiplicative_increase = MultiplicativeRateIncrease(
+              AdditiveRateIncrease(at_time, time_last_bitrate_change_);
-            at_time, time_last_bitrate_change_, new_bitrate);
+          new_bitrate += additive_increase;
-        new_bitrate += multiplicative_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.
+  // Allow the estimate to increase as long as alr is not detected to ensure
-  // We allow a bit more lag at very low rates to not too easily get stuck if
+  // that there is no BWE values that can make the estimate stuck at a too
-  // the encoder produces uneven outputs.
+  // low bitrate. If an encoder can not produce the bitrate necessary to
-  const DataRate max_bitrate = 1.5 * estimated_throughput + DataRate::kbps(10);
+  // fully use the capacity, alr will sooner or later trigger.
-  if (new_bitrate > current_bitrate_ && new_bitrate > max_bitrate) {
+  if (!(send_side_ && no_bitrate_increase_in_alr_)) {
-    new_bitrate = std::max(current_bitrate_, max_bitrate);
+    // 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 *

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<NetworkStateEstimate>& 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<DataRate> last_decrease_;
   FieldTrialOptional<TimeDelta> initial_backoff_interval_;

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