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}
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 e84d188..381345f 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 @@
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 @@
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 6139704..f0211d7 100644
--- a/modules/congestion_controller/goog_cc/probe_controller.cc
+++ b/modules/congestion_controller/goog_cc/probe_controller.cc
@@ -109,35 +109,23 @@
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 @@
}
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 c8653b8..feec81f 100644
--- a/modules/congestion_controller/goog_cc/probe_controller.h
+++ b/modules/congestion_controller/goog_cc/probe_controller.h
@@ -71,14 +71,10 @@
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 @@
// estimate.
enum class BandwidthLimitedCause {
kLossLimitedBweIncreasing = 0,
- kLossLimitedBweDecreasing = 1,
+ kLossLimitedBwe = 1,
kDelayBasedLimited = 2,
kDelayBasedLimitedDelayIncreased = 3,
kRttBasedBackOffHighRtt = 4
@@ -142,11 +138,6 @@
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.
diff --git a/modules/congestion_controller/goog_cc/probe_controller_unittest.cc b/modules/congestion_controller/goog_cc/probe_controller_unittest.cc
index 6c02b3e..87c48ff 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, 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 @@
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, 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, 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, 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, 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, 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, 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 @@
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 @@
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 @@
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, 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, 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();