From 08848ea04c1c3cdbafe51bba0fc91d32d8296e51 Mon Sep 17 00:00:00 2001 From: Per K Date: Mon, 25 Mar 2024 15:57:00 +0100 Subject: [PATCH] Reland "Stop exponential probing if 2xmax allocated bitrate lower than BWE." This reverts commit 802dd5bdbd97d880761059c7362c9e843211e32d. First patch set is pure reland. New patch set adds field trial flag. Original change's description: 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: I35c341bbaced800d9931657d62c73a17a3279b7c Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/344440 Auto-Submit: Per Kjellander Reviewed-by: Diep Bui Commit-Queue: Per Kjellander Cr-Commit-Position: refs/heads/main@{#41965} --- .../goog_cc/probe_controller.cc | 44 +++++++++++++------ .../goog_cc/probe_controller.h | 1 + .../goog_cc/probe_controller_unittest.cc | 27 ++++++++++++ 3 files changed, 58 insertions(+), 14 deletions(-) diff --git a/modules/congestion_controller/goog_cc/probe_controller.cc b/modules/congestion_controller/goog_cc/probe_controller.cc index 31727051a8..3fc8677e87 100644 --- a/modules/congestion_controller/goog_cc/probe_controller.cc +++ b/modules/congestion_controller/goog_cc/probe_controller.cc @@ -88,6 +88,7 @@ ProbeControllerConfig::ProbeControllerConfig( second_exponential_probe_scale("p2", 6.0), further_exponential_probe_scale("step_size", 2), further_probe_threshold("further_probe_threshold", 0.7), + abort_further_probe_if_max_lower_than_current("abort_further", false), alr_probing_interval("alr_interval", TimeDelta::Seconds(5)), alr_probe_scale("alr_scale", 2), network_state_estimate_probing_interval("network_state_interval", @@ -112,19 +113,27 @@ ProbeControllerConfig::ProbeControllerConfig( skip_if_estimate_larger_than_fraction_of_max( "skip_if_est_larger_than_fraction_of_max", 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_probe_limit_by_current_scale, - &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")); + ParseFieldTrial({&first_exponential_probe_scale, + &second_exponential_probe_scale, + &further_exponential_probe_scale, + &further_probe_threshold, + &abort_further_probe_if_max_lower_than_current, + &alr_probing_interval, + &alr_probe_scale, + &probe_on_max_allocated_bitrate_change, + &first_allocation_probe_scale, + &second_allocation_probe_scale, + &allocation_probe_limit_by_current_scale, + &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( @@ -294,7 +303,14 @@ std::vector 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 (config_.abort_further_probe_if_max_lower_than_current && + (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.h b/modules/congestion_controller/goog_cc/probe_controller.h index 25f02aee69..cec6157851 100644 --- a/modules/congestion_controller/goog_cc/probe_controller.h +++ b/modules/congestion_controller/goog_cc/probe_controller.h @@ -42,6 +42,7 @@ struct ProbeControllerConfig { FieldTrialOptional second_exponential_probe_scale; FieldTrialParameter further_exponential_probe_scale; FieldTrialParameter further_probe_threshold; + FieldTrialParameter abort_further_probe_if_max_lower_than_current; // Configures how often we send ALR probes and how big they are. FieldTrialParameter alr_probing_interval; diff --git a/modules/congestion_controller/goog_cc/probe_controller_unittest.cc b/modules/congestion_controller/goog_cc/probe_controller_unittest.cc index 6e34a2962d..aa62c476d5 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,31 @@ TEST(ProbeControllerTest, TestExponentialProbing) { EXPECT_EQ(probes[0].target_data_rate.bps(), 2 * 1800); } +TEST(ProbeControllerTest, ExponentialProbingStopIfMaxAllocatedBitrateLow) { + ProbeControllerFixture fixture( + "WebRTC-Bwe-ProbingConfiguration/abort_further:true/"); + std::unique_ptr 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 probe_controller =