Default dont probe when BWE estimators detects a limit

Cleanup field trials for not probing when BWE limited due to high RTT,
loss.

Bug: webrtc:14754, webrtc:12707
Change-Id: Ib664784e321d9284d842ea42a0dd1d8361000f20
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/323640
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#40949}

modules/congestion_controller/goog_cc/goog_cc_network_control.cc

@@ -73,22 +73,18 @@ bool IsNotDisabled(const FieldTrialsView* config, absl::string_view key) {
   return !absl::StartsWith(config->Lookup(key), "Disabled");
 }
 
-BandwidthLimitedCause GetBandwidthLimitedCause(
-    LossBasedState loss_based_state,
-    bool is_rtt_above_limit,
-    BandwidthUsage bandwidth_usage,
-    bool not_probe_if_delay_increased) {
-  if (not_probe_if_delay_increased) {
-    if (bandwidth_usage == BandwidthUsage::kBwOverusing ||
-        bandwidth_usage == BandwidthUsage::kBwUnderusing) {
-      return BandwidthLimitedCause::kDelayBasedLimitedDelayIncreased;
-    } else if (is_rtt_above_limit) {
-      return BandwidthLimitedCause::kRttBasedBackOffHighRtt;
-    }
+BandwidthLimitedCause GetBandwidthLimitedCause(LossBasedState loss_based_state,
+                                               bool is_rtt_above_limit,
+                                               BandwidthUsage bandwidth_usage) {
+  if (bandwidth_usage == BandwidthUsage::kBwOverusing ||
+      bandwidth_usage == BandwidthUsage::kBwUnderusing) {
+    return BandwidthLimitedCause::kDelayBasedLimitedDelayIncreased;
+  } else if (is_rtt_above_limit) {
+    return BandwidthLimitedCause::kRttBasedBackOffHighRtt;
   }
   switch (loss_based_state) {
     case LossBasedState::kDecreasing:
-      return BandwidthLimitedCause::kLossLimitedBweDecreasing;
+      return BandwidthLimitedCause::kLossLimitedBwe;
     case LossBasedState::kIncreasing:
       return BandwidthLimitedCause::kLossLimitedBweIncreasing;
     default:
@@ -698,11 +694,9 @@ void GoogCcNetworkController::MaybeTriggerOnNetworkChanged(
 
     auto probes = probe_controller_->SetEstimatedBitrate(
         loss_based_target_rate,
-        GetBandwidthLimitedCause(
-            bandwidth_estimation_->loss_based_state(),
-            bandwidth_estimation_->IsRttAboveLimit(),
-            delay_based_bwe_->last_state(),
-            probe_controller_->DontProbeIfDelayIncreased()),
+        GetBandwidthLimitedCause(bandwidth_estimation_->loss_based_state(),
+                                 bandwidth_estimation_->IsRttAboveLimit(),
+                                 delay_based_bwe_->last_state()),
         at_time);
     update->probe_cluster_configs.insert(update->probe_cluster_configs.end(),
                                          probes.begin(), probes.end());

modules/congestion_controller/goog_cc/probe_controller.cc

@@ -109,35 +109,23 @@ ProbeControllerConfig::ProbeControllerConfig(
       allocation_probe_max("alloc_probe_max", DataRate::PlusInfinity()),
       min_probe_packets_sent("min_probe_packets_sent", 5),
       min_probe_duration("min_probe_duration", TimeDelta::Millis(15)),
-      limit_probe_target_rate_to_loss_bwe("limit_probe_target_rate_to_loss_bwe",
-                                          false),
       loss_limited_probe_scale("loss_limited_scale", 1.5),
       skip_if_estimate_larger_than_fraction_of_max(
           "skip_if_est_larger_than_fraction_of_max",
-          0.0),
-      not_probe_if_delay_increased("not_probe_if_delay_increased", false) {
-  ParseFieldTrial({&first_exponential_probe_scale,
-                   &second_exponential_probe_scale,
-                   &further_exponential_probe_scale,
-                   &further_probe_threshold,
-                   &alr_probing_interval,
-                   &alr_probe_scale,
-                   &probe_on_max_allocated_bitrate_change,
-                   &first_allocation_probe_scale,
-                   &second_allocation_probe_scale,
-                   &allocation_allow_further_probing,
-                   &min_probe_duration,
-                   &network_state_estimate_probing_interval,
-                   &probe_if_estimate_lower_than_network_state_estimate_ratio,
-                   &estimate_lower_than_network_state_estimate_probing_interval,
-                   &network_state_probe_scale,
-                   &network_state_probe_duration,
-                   &min_probe_packets_sent,
-                   &limit_probe_target_rate_to_loss_bwe,
-                   &loss_limited_probe_scale,
-                   &skip_if_estimate_larger_than_fraction_of_max,
-                   &not_probe_if_delay_increased},
-                  key_value_config->Lookup("WebRTC-Bwe-ProbingConfiguration"));
+          0.0) {
+  ParseFieldTrial(
+      {&first_exponential_probe_scale, &second_exponential_probe_scale,
+       &further_exponential_probe_scale, &further_probe_threshold,
+       &alr_probing_interval, &alr_probe_scale,
+       &probe_on_max_allocated_bitrate_change, &first_allocation_probe_scale,
+       &second_allocation_probe_scale, &allocation_allow_further_probing,
+       &min_probe_duration, &network_state_estimate_probing_interval,
+       &probe_if_estimate_lower_than_network_state_estimate_ratio,
+       &estimate_lower_than_network_state_estimate_probing_interval,
+       &network_state_probe_scale, &network_state_probe_duration,
+       &min_probe_packets_sent, &loss_limited_probe_scale,
+       &skip_if_estimate_larger_than_fraction_of_max},
+      key_value_config->Lookup("WebRTC-Bwe-ProbingConfiguration"));
 
   // Specialized keys overriding subsets of WebRTC-Bwe-ProbingConfiguration
   ParseFieldTrial(
@@ -484,29 +472,21 @@ std::vector<ProbeClusterConfig> ProbeController::InitiateProbing(
   }
 
   DataRate estimate_capped_bitrate = DataRate::PlusInfinity();
-  if (config_.limit_probe_target_rate_to_loss_bwe) {
-    switch (bandwidth_limited_cause_) {
-      case BandwidthLimitedCause::kLossLimitedBweDecreasing:
-        // If bandwidth estimate is decreasing because of packet loss, do not
-        // send probes.
-        return {};
-      case BandwidthLimitedCause::kLossLimitedBweIncreasing:
-        estimate_capped_bitrate =
-            std::min(max_probe_bitrate,
-                     estimated_bitrate_ * config_.loss_limited_probe_scale);
-        break;
-      case BandwidthLimitedCause::kDelayBasedLimited:
-        break;
-      default:
-        break;
-    }
-  }
-  if (config_.not_probe_if_delay_increased &&
-      (bandwidth_limited_cause_ ==
-           BandwidthLimitedCause::kDelayBasedLimitedDelayIncreased ||
-       bandwidth_limited_cause_ ==
-           BandwidthLimitedCause::kRttBasedBackOffHighRtt)) {
-    return {};
+  switch (bandwidth_limited_cause_) {
+    case BandwidthLimitedCause::kRttBasedBackOffHighRtt:
+    case BandwidthLimitedCause::kDelayBasedLimitedDelayIncreased:
+    case BandwidthLimitedCause::kLossLimitedBwe:
+      RTC_LOG(LS_INFO) << "Not sending probe in bandwidth limited state.";
+      return {};
+    case BandwidthLimitedCause::kLossLimitedBweIncreasing:
+      estimate_capped_bitrate =
+          std::min(max_probe_bitrate,
+                   estimated_bitrate_ * config_.loss_limited_probe_scale);
+      break;
+    case BandwidthLimitedCause::kDelayBasedLimited:
+      break;
+    default:
+      break;
   }
 
   if (config_.network_state_estimate_probing_interval->IsFinite() &&

modules/congestion_controller/goog_cc/probe_controller.h

@@ -71,14 +71,10 @@ struct ProbeControllerConfig {
   FieldTrialParameter<int> min_probe_packets_sent;
   // The minimum probing duration.
   FieldTrialParameter<TimeDelta> min_probe_duration;
-  // Periodically probe when bandwidth estimate is loss limited.
-  FieldTrialParameter<bool> limit_probe_target_rate_to_loss_bwe;
   FieldTrialParameter<double> loss_limited_probe_scale;
   // Dont send a probe if min(estimate, network state estimate) is larger than
   // this fraction of the set max bitrate.
   FieldTrialParameter<double> skip_if_estimate_larger_than_fraction_of_max;
-  // Do not send probes if either overusing/underusing network or high rtt.
-  FieldTrialParameter<bool> not_probe_if_delay_increased;
 };
 
 // Reason that bandwidth estimate is limited. Bandwidth estimate can be limited
@@ -86,7 +82,7 @@ struct ProbeControllerConfig {
 // estimate.
 enum class BandwidthLimitedCause {
   kLossLimitedBweIncreasing = 0,
-  kLossLimitedBweDecreasing = 1,
+  kLossLimitedBwe = 1,
   kDelayBasedLimited = 2,
   kDelayBasedLimitedDelayIncreased = 3,
   kRttBasedBackOffHighRtt = 4
@@ -142,11 +138,6 @@ class ProbeController {
   ABSL_MUST_USE_RESULT std::vector<ProbeClusterConfig> Process(
       Timestamp at_time);
 
-  // Gets the value of field trial not_probe_if_delay_increased.
-  bool DontProbeIfDelayIncreased() {
-    return config_.not_probe_if_delay_increased;
-  }
-
  private:
   enum class State {
     // Initial state where no probing has been triggered yet.

modules/congestion_controller/goog_cc/probe_controller_unittest.cc

@@ -563,9 +563,7 @@ TEST(ProbeControllerTest, ConfigurableProbingFieldTrial) {
 }
 
 TEST(ProbeControllerTest, LimitAlrProbeWhenLossBasedBweLimited) {
-  ProbeControllerFixture fixture(
-      "WebRTC-Bwe-ProbingConfiguration/"
-      "limit_probe_target_rate_to_loss_bwe:true/");
+  ProbeControllerFixture fixture;
   std::unique_ptr<ProbeController> probe_controller =
       fixture.CreateController();
   probe_controller->EnablePeriodicAlrProbing(true);
@@ -627,7 +625,7 @@ TEST(ProbeControllerTest,
      LimitProbeAtUpperNetworkStateEstimateIfLossBasedLimited) {
   ProbeControllerFixture fixture(
       "WebRTC-Bwe-ProbingConfiguration/"
-      "network_state_interval:5s,limit_probe_target_rate_to_loss_bwe:true/");
+      "network_state_interval:5s/");
   std::unique_ptr<ProbeController> probe_controller =
       fixture.CreateController();
 
@@ -706,9 +704,7 @@ TEST(ProbeControllerTest, CanSetLongerProbeDurationAfterNetworkStateEstimate) {
 }
 
 TEST(ProbeControllerTest, ProbeInAlrIfLossBasedIncreasing) {
-  ProbeControllerFixture fixture(
-      "WebRTC-Bwe-ProbingConfiguration/"
-      "limit_probe_target_rate_to_loss_bwe:true/");
+  ProbeControllerFixture fixture;
   std::unique_ptr<ProbeController> probe_controller =
       fixture.CreateController();
   auto probes = probe_controller->SetBitrates(
@@ -732,9 +728,7 @@ TEST(ProbeControllerTest, ProbeInAlrIfLossBasedIncreasing) {
 }
 
 TEST(ProbeControllerTest, ProbeFurtherInAlrIfLossBasedIncreasing) {
-  ProbeControllerFixture fixture(
-      "WebRTC-Bwe-ProbingConfiguration/"
-      "limit_probe_target_rate_to_loss_bwe:true/");
+  ProbeControllerFixture fixture;
   std::unique_ptr<ProbeController> probe_controller =
       fixture.CreateController();
   auto probes = probe_controller->SetBitrates(
@@ -764,16 +758,14 @@ TEST(ProbeControllerTest, ProbeFurtherInAlrIfLossBasedIncreasing) {
 }
 
 TEST(ProbeControllerTest, NotProbeWhenInAlrIfLossBasedDecreases) {
-  ProbeControllerFixture fixture(
-      "WebRTC-Bwe-ProbingConfiguration/"
-      "network_state_interval:5s,limit_probe_target_rate_to_loss_bwe:true/");
+  ProbeControllerFixture fixture;
   std::unique_ptr<ProbeController> probe_controller =
       fixture.CreateController();
   auto probes = probe_controller->SetBitrates(
       kMinBitrate, kStartBitrate, kMaxBitrate, fixture.CurrentTime());
   probe_controller->EnablePeriodicAlrProbing(true);
   probes = probe_controller->SetEstimatedBitrate(
-      kStartBitrate, BandwidthLimitedCause::kLossLimitedBweDecreasing,
+      kStartBitrate, BandwidthLimitedCause::kLossLimitedBwe,
       fixture.CurrentTime());
 
   // Wait long enough to time out exponential probing.
@@ -789,9 +781,7 @@ TEST(ProbeControllerTest, NotProbeWhenInAlrIfLossBasedDecreases) {
 }
 
 TEST(ProbeControllerTest, NotProbeIfLossBasedIncreasingOutsideAlr) {
-  ProbeControllerFixture fixture(
-      "WebRTC-Bwe-ProbingConfiguration/"
-      "limit_probe_target_rate_to_loss_bwe:true/");
+  ProbeControllerFixture fixture;
   std::unique_ptr<ProbeController> probe_controller =
       fixture.CreateController();
   auto probes = probe_controller->SetBitrates(
@@ -815,7 +805,7 @@ TEST(ProbeControllerTest, NotProbeIfLossBasedIncreasingOutsideAlr) {
 TEST(ProbeControllerTest, ProbeFurtherWhenLossBasedIsSameAsDelayBasedEstimate) {
   ProbeControllerFixture fixture(
       "WebRTC-Bwe-ProbingConfiguration/"
-      "network_state_interval:5s,limit_probe_target_rate_to_loss_bwe:true/");
+      "network_state_interval:5s/");
   std::unique_ptr<ProbeController> probe_controller =
       fixture.CreateController();
 
@@ -896,7 +886,7 @@ TEST(ProbeControllerTest, ProbeIfEstimateLowerThanNetworkStateEstimate) {
 TEST(ProbeControllerTest, DontProbeFurtherWhenLossLimited) {
   ProbeControllerFixture fixture(
       "WebRTC-Bwe-ProbingConfiguration/"
-      "network_state_interval:5s,limit_probe_target_rate_to_loss_bwe:true/");
+      "network_state_interval:5s/");
   std::unique_ptr<ProbeController> probe_controller =
       fixture.CreateController();
 
@@ -918,15 +908,15 @@ TEST(ProbeControllerTest, DontProbeFurtherWhenLossLimited) {
   EXPECT_LT(probes[0].target_data_rate, state_estimate.link_capacity_upper);
   // Expect that no more probes are sent immediately if BWE is loss limited.
   probes = probe_controller->SetEstimatedBitrate(
-      probes[0].target_data_rate,
-      BandwidthLimitedCause::kLossLimitedBweDecreasing, fixture.CurrentTime());
+      probes[0].target_data_rate, BandwidthLimitedCause::kLossLimitedBwe,
+      fixture.CurrentTime());
   EXPECT_TRUE(probes.empty());
 }
 
 TEST(ProbeControllerTest, ProbeFurtherWhenDelayBasedLimited) {
   ProbeControllerFixture fixture(
       "WebRTC-Bwe-ProbingConfiguration/"
-      "network_state_interval:5s,limit_probe_target_rate_to_loss_bwe:true/");
+      "network_state_interval:5s/");
   std::unique_ptr<ProbeController> probe_controller =
       fixture.CreateController();
 
@@ -958,7 +948,7 @@ TEST(ProbeControllerTest,
      ProbeFurtherIfNetworkStateEstimateIncreaseAfterProbeSent) {
   ProbeControllerFixture fixture(
       "WebRTC-Bwe-ProbingConfiguration/"
-      "network_state_interval:5s,limit_probe_target_rate_to_loss_bwe:true/");
+      "network_state_interval:5s/");
   std::unique_ptr<ProbeController> probe_controller =
       fixture.CreateController();
   auto probes = probe_controller->SetBitrates(
@@ -1107,7 +1097,7 @@ TEST(ProbeControllerTest,
      ProbeNotLimitedByNetworkStateEsimateIfLowerThantCurrent) {
   ProbeControllerFixture fixture(
       "WebRTC-Bwe-ProbingConfiguration/"
-      "network_state_interval:5s,limit_probe_target_rate_to_loss_bwe:true/");
+      "network_state_interval:5s/");
   std::unique_ptr<ProbeController> probe_controller =
       fixture.CreateController();
   probe_controller->EnablePeriodicAlrProbing(true);
@@ -1133,9 +1123,7 @@ TEST(ProbeControllerTest,
 }
 
 TEST(ProbeControllerTest, DontProbeIfDelayIncreased) {
-  ProbeControllerFixture fixture(
-      "WebRTC-Bwe-ProbingConfiguration/"
-      "network_state_interval:5s,not_probe_if_delay_increased:true/");
+  ProbeControllerFixture fixture;
   std::unique_ptr<ProbeController> probe_controller =
       fixture.CreateController();
 
@@ -1162,9 +1150,7 @@ TEST(ProbeControllerTest, DontProbeIfDelayIncreased) {
 }
 
 TEST(ProbeControllerTest, DontProbeIfHighRtt) {
-  ProbeControllerFixture fixture(
-      "WebRTC-Bwe-ProbingConfiguration/"
-      "network_state_interval:5s,not_probe_if_delay_increased:true/");
+  ProbeControllerFixture fixture;
   std::unique_ptr<ProbeController> probe_controller =
       fixture.CreateController();