From 5a4ce031019bce349ad65e76aa5da6d0f8e5989e Mon Sep 17 00:00:00 2001
From: Per K <perkj@webrtc.org>
Date: Tue, 19 Mar 2024 10:02:04 +0100
Subject: [PATCH] Stop exponential probing if 2xmax allocated bitrate lower
 than BWE.

Without this, if max allocated bitrate is lowered while exponential probing is ongoing, a new probe can be sent at a rate of the new low max allocated bitrate which may cause BWE to decrease.

Bug: webrtc:14928
Change-Id: Id8e314740c2403d3b801d28f634dbc718f77c16e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/343384
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41929}
---
 .../goog_cc/probe_controller.cc               |  8 +++++-
 .../goog_cc/probe_controller_unittest.cc      | 26 +++++++++++++++++++
 2 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/modules/congestion_controller/goog_cc/probe_controller.cc b/modules/congestion_controller/goog_cc/probe_controller.cc
index 31727051a8..6c3667b9df 100644
--- a/modules/congestion_controller/goog_cc/probe_controller.cc
+++ b/modules/congestion_controller/goog_cc/probe_controller.cc
@@ -294,7 +294,13 @@ std::vector<ProbeClusterConfig> ProbeController::SetEstimatedBitrate(
 
   if (state_ == State::kWaitingForProbingResult) {
     // Continue probing if probing results indicate channel has greater
-    // capacity.
+    // capacity unless we already reached the needed bitrate.
+    if (bitrate > max_bitrate_ ||
+        (!max_total_allocated_bitrate_.IsZero() &&
+         bitrate > 2 * max_total_allocated_bitrate_)) {
+      // No need to continue probing.
+      min_bitrate_to_probe_further_ = DataRate::PlusInfinity();
+    }
     DataRate network_state_estimate_probe_further_limit =
         config_.network_state_estimate_probing_interval->IsFinite() &&
                 network_estimate_
diff --git a/modules/congestion_controller/goog_cc/probe_controller_unittest.cc b/modules/congestion_controller/goog_cc/probe_controller_unittest.cc
index 6e34a2962d..fb5fc2df7f 100644
--- a/modules/congestion_controller/goog_cc/probe_controller_unittest.cc
+++ b/modules/congestion_controller/goog_cc/probe_controller_unittest.cc
@@ -23,8 +23,10 @@
 #include "test/gmock.h"
 #include "test/gtest.h"
 
+using ::testing::Gt;
 using ::testing::IsEmpty;
 using ::testing::NiceMock;
+using ::testing::SizeIs;
 
 namespace webrtc {
 namespace test {
@@ -325,6 +327,30 @@ TEST(ProbeControllerTest, TestExponentialProbing) {
   EXPECT_EQ(probes[0].target_data_rate.bps(), 2 * 1800);
 }
 
+TEST(ProbeControllerTest, ExponentialProbingStopIfMaxAllocatedBitrateLow) {
+  ProbeControllerFixture fixture;
+  std::unique_ptr<ProbeController> probe_controller =
+      fixture.CreateController();
+  ASSERT_THAT(
+      probe_controller->OnNetworkAvailability({.network_available = true}),
+      IsEmpty());
+  auto probes = probe_controller->SetBitrates(
+      kMinBitrate, kStartBitrate, kMaxBitrate, fixture.CurrentTime());
+  ASSERT_THAT(probes, SizeIs(Gt(0)));
+
+  // Repeated probe normally is sent when estimated bitrate climbs above
+  // 0.7 * 6 * kStartBitrate = 1260. But since allocated bitrate i slow, expect
+  // exponential probing to stop.
+  probes = probe_controller->OnMaxTotalAllocatedBitrate(kStartBitrate,
+                                                        fixture.CurrentTime());
+  EXPECT_THAT(probes, IsEmpty());
+
+  probes = probe_controller->SetEstimatedBitrate(
+      DataRate::BitsPerSec(1800), BandwidthLimitedCause::kDelayBasedLimited,
+      fixture.CurrentTime());
+  EXPECT_THAT(probes, IsEmpty());
+}
+
 TEST(ProbeControllerTest, TestExponentialProbingTimeout) {
   ProbeControllerFixture fixture;
   std::unique_ptr<ProbeController> probe_controller =