Continue probing if networkstat estimate increase

This fixes an issue where continues probing stops if networkstate estimate is low when a probe is sent, but increase as a consequence of the probe.


Bug: webrtc:14392
Change-Id: Id1d703f7efc824a6a6f8d899c367660291bd88c8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/282941
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38606}

modules/congestion_controller/goog_cc/probe_controller.cc

@@ -468,25 +468,13 @@ std::vector<ProbeClusterConfig> ProbeController::InitiateProbing(
             : std::min(max_total_allocated_bitrate_, max_bitrate_);
     if (std::min(network_estimate, estimated_bitrate_) >
         config_.skip_if_estimate_larger_than_fraction_of_max * max_probe_rate) {
+      state_ = State::kProbingComplete;
+      min_bitrate_to_probe_further_ = DataRate::PlusInfinity();
       return {};
     }
   }
 
   DataRate max_probe_bitrate = max_bitrate_;
-  if (bwe_limited_due_to_packet_loss_ &&
-      config_.limit_probe_target_rate_to_loss_bwe) {
-    max_probe_bitrate = std::min(estimated_bitrate_, max_bitrate_);
-  }
-  if (config_.network_state_estimate_probing_interval->IsFinite() &&
-      network_estimate_ && network_estimate_->link_capacity_upper.IsFinite()) {
-    if (network_estimate_->link_capacity_upper.IsZero()) {
-      RTC_LOG(LS_INFO) << "Not sending probe, Network state estimate is zero";
-      return {};
-    }
-    max_probe_bitrate =
-        std::min(max_probe_bitrate, network_estimate_->link_capacity_upper *
-                                        config_.network_state_probe_scale);
-  }
   if (max_total_allocated_bitrate_ > DataRate::Zero()) {
     // If a max allocated bitrate has been configured, allow probing up to 2x
     // that rate. This allows some overhead to account for bursty streams,
@@ -498,10 +486,28 @@ std::vector<ProbeClusterConfig> ProbeController::InitiateProbing(
         std::min(max_probe_bitrate, max_total_allocated_bitrate_ * 2);
   }
 
+  DataRate estimate_capped_bitrate = DataRate::PlusInfinity();
+  if (bwe_limited_due_to_packet_loss_ &&
+      config_.limit_probe_target_rate_to_loss_bwe) {
+    estimate_capped_bitrate = std::min(estimated_bitrate_, max_probe_bitrate);
+  }
+  if (config_.network_state_estimate_probing_interval->IsFinite() &&
+      network_estimate_ && network_estimate_->link_capacity_upper.IsFinite()) {
+    if (network_estimate_->link_capacity_upper.IsZero()) {
+      RTC_LOG(LS_INFO) << "Not sending probe, Network state estimate is zero";
+      return {};
+    }
+    estimate_capped_bitrate =
+        std::min({estimate_capped_bitrate, max_probe_bitrate,
+                  network_estimate_->link_capacity_upper *
+                      config_.network_state_probe_scale});
+  }
+
   std::vector<ProbeClusterConfig> pending_probes;
   for (DataRate bitrate : bitrates_to_probe) {
     RTC_DCHECK(!bitrate.IsZero());
 
+    bitrate = std::min(bitrate, estimate_capped_bitrate);
     if (bitrate > max_probe_bitrate) {
       bitrate = max_probe_bitrate;
       probe_further = false;
@@ -527,7 +533,8 @@ std::vector<ProbeClusterConfig> ProbeController::InitiateProbing(
   if (probe_further) {
     state_ = State::kWaitingForProbingResult;
     min_bitrate_to_probe_further_ =
-        (*(bitrates_to_probe.end() - 1)) * config_.further_probe_threshold;
+        std::min(estimate_capped_bitrate, (*(bitrates_to_probe.end() - 1))) *
+        config_.further_probe_threshold;
   } else {
     state_ = State::kProbingComplete;
     min_bitrate_to_probe_further_ = DataRate::PlusInfinity();

modules/congestion_controller/goog_cc/probe_controller_unittest.cc

@@ -789,6 +789,42 @@ TEST(ProbeControllerTest, ProbeFurtherWhenDelayBasedLimited) {
   EXPECT_EQ(probes[0].target_data_rate, state_estimate.link_capacity_upper);
 }
 
+TEST(ProbeControllerTest,
+     ProbeFurtherIfNetworkStateEstimateIncreaseAfterProbeSent) {
+  ProbeControllerFixture fixture(
+      "WebRTC-Bwe-ProbingConfiguration/"
+      "network_state_interval:5s,limit_probe_target_rate_to_loss_bwe:true/");
+  std::unique_ptr<ProbeController> probe_controller =
+      fixture.CreateController();
+
+  auto probes = probe_controller->SetBitrates(
+      kMinBitrate, kStartBitrate, kMaxBitrate, fixture.CurrentTime());
+  ASSERT_FALSE(probes.empty());
+
+  NetworkStateEstimate state_estimate;
+  state_estimate.link_capacity_upper = 1.2 * probes[0].target_data_rate / 2;
+  probe_controller->SetNetworkStateEstimate(state_estimate);
+
+  // No immediate further probing since probe result is low.
+  probes = probe_controller->SetEstimatedBitrate(
+      probes[0].target_data_rate / 2, /*bwe_limited_due_to_packet_loss=*/false,
+      fixture.CurrentTime());
+  ASSERT_TRUE(probes.empty());
+
+  fixture.AdvanceTime(TimeDelta::Seconds(5));
+  probes = probe_controller->Process(fixture.CurrentTime());
+  ASSERT_FALSE(probes.empty());
+  EXPECT_LE(probes[0].target_data_rate, state_estimate.link_capacity_upper);
+  // If the network state estimate increase above the threshold to probe
+  // further, and the probe suceeed, expect a new probe.
+  state_estimate.link_capacity_upper = 3 * kStartBitrate;
+  probe_controller->SetNetworkStateEstimate(state_estimate);
+  probes = probe_controller->SetEstimatedBitrate(
+      probes[0].target_data_rate, /*bwe_limited_due_to_packet_loss=*/false,
+      fixture.CurrentTime());
+  EXPECT_FALSE(probes.empty());
+}
+
 TEST(ProbeControllerTest, SkipAlrProbeIfEstimateLargerThanMaxProbe) {
   ProbeControllerFixture fixture(
       "WebRTC-Bwe-ProbingConfiguration/"