From 7c612c30745f418b6959256800e60bbaa1bb0fb6 Mon Sep 17 00:00:00 2001 From: Per K Date: Tue, 17 Oct 2023 10:04:55 +0200 Subject: [PATCH] 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 Reviewed-by: Diep Bui Cr-Commit-Position: refs/heads/main@{#40949} --- .../goog_cc/goog_cc_network_control.cc | 30 +++---- .../goog_cc/probe_controller.cc | 78 +++++++------------ .../goog_cc/probe_controller.h | 11 +-- .../goog_cc/probe_controller_unittest.cc | 46 ++++------- 4 files changed, 58 insertions(+), 107 deletions(-) diff --git a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc index e84d188e88..381345fc4e 100644 --- a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc +++ b/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()); diff --git a/modules/congestion_controller/goog_cc/probe_controller.cc b/modules/congestion_controller/goog_cc/probe_controller.cc index 6139704fb5..f0211d75e0 100644 --- a/modules/congestion_controller/goog_cc/probe_controller.cc +++ b/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, - ¬_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 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() && diff --git a/modules/congestion_controller/goog_cc/probe_controller.h b/modules/congestion_controller/goog_cc/probe_controller.h index c8653b8d69..feec81f2dc 100644 --- a/modules/congestion_controller/goog_cc/probe_controller.h +++ b/modules/congestion_controller/goog_cc/probe_controller.h @@ -71,14 +71,10 @@ struct ProbeControllerConfig { FieldTrialParameter min_probe_packets_sent; // The minimum probing duration. FieldTrialParameter min_probe_duration; - // Periodically probe when bandwidth estimate is loss limited. - FieldTrialParameter limit_probe_target_rate_to_loss_bwe; FieldTrialParameter 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 skip_if_estimate_larger_than_fraction_of_max; - // Do not send probes if either overusing/underusing network or high rtt. - FieldTrialParameter 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 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. diff --git a/modules/congestion_controller/goog_cc/probe_controller_unittest.cc b/modules/congestion_controller/goog_cc/probe_controller_unittest.cc index 6c02b3e2b5..87c48ffd25 100644 --- a/modules/congestion_controller/goog_cc/probe_controller_unittest.cc +++ b/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 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 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 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 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 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 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 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 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 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 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 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 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 probe_controller = fixture.CreateController();