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 c008997a2c..7cfa9d9126 100644 --- a/modules/congestion_controller/goog_cc/goog_cc_network_control.cc +++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.cc @@ -118,8 +118,7 @@ GoogCcNetworkController::GoogCcNetworkController(NetworkControllerConfig config, config.stream_based_config.min_total_allocated_bitrate.value_or( DataRate::Zero())), max_padding_rate_(config.stream_based_config.max_padding_rate.value_or( - DataRate::Zero())), - max_total_allocated_bitrate_(DataRate::Zero()) { + DataRate::Zero())) { RTC_DCHECK(config.constraints.at_time.IsFinite()); ParseFieldTrial( {&safe_reset_on_route_change_, &safe_reset_acknowledged_rate_}, @@ -192,8 +191,6 @@ NetworkControlUpdate GoogCcNetworkController::OnProcessInterval( *total_bitrate, msg.at_time); update.probe_cluster_configs.insert(update.probe_cluster_configs.end(), probes.begin(), probes.end()); - - max_total_allocated_bitrate_ = *total_bitrate; } initial_config_.reset(); } @@ -287,17 +284,12 @@ NetworkControlUpdate GoogCcNetworkController::OnStreamsConfig( if (msg.requests_alr_probing) { probe_controller_->EnablePeriodicAlrProbing(*msg.requests_alr_probing); } - if (msg.max_total_allocated_bitrate && - *msg.max_total_allocated_bitrate != max_total_allocated_bitrate_) { - if (rate_control_settings_.TriggerProbeOnMaxAllocatedBitrateChange()) { - update.probe_cluster_configs = - probe_controller_->OnMaxTotalAllocatedBitrate( - *msg.max_total_allocated_bitrate, msg.at_time); - } else { - probe_controller_->SetMaxBitrate(*msg.max_total_allocated_bitrate); - } - max_total_allocated_bitrate_ = *msg.max_total_allocated_bitrate; + if (msg.max_total_allocated_bitrate) { + update.probe_cluster_configs = + probe_controller_->OnMaxTotalAllocatedBitrate( + *msg.max_total_allocated_bitrate, msg.at_time); } + bool pacing_changed = false; if (msg.pacing_factor && *msg.pacing_factor != pacing_factor_) { pacing_factor_ = *msg.pacing_factor; diff --git a/modules/congestion_controller/goog_cc/goog_cc_network_control.h b/modules/congestion_controller/goog_cc/goog_cc_network_control.h index 884b572740..1b9f962732 100644 --- a/modules/congestion_controller/goog_cc/goog_cc_network_control.h +++ b/modules/congestion_controller/goog_cc/goog_cc_network_control.h @@ -138,7 +138,6 @@ class GoogCcNetworkController : public NetworkControllerInterface { double pacing_factor_; DataRate min_total_allocated_bitrate_; DataRate max_padding_rate_; - DataRate max_total_allocated_bitrate_; bool previously_in_alr_ = false; diff --git a/modules/congestion_controller/goog_cc/probe_controller.cc b/modules/congestion_controller/goog_cc/probe_controller.cc index 1b98dcd872..6f5fe09c8f 100644 --- a/modules/congestion_controller/goog_cc/probe_controller.cc +++ b/modules/congestion_controller/goog_cc/probe_controller.cc @@ -97,6 +97,7 @@ ProbeControllerConfig::ProbeControllerConfig( network_state_probe_duration("network_state_probe_duration", TimeDelta::Millis(15)), + probe_on_max_allocated_bitrate_change("probe_max_allocation", true), first_allocation_probe_scale("alloc_p1", 1), second_allocation_probe_scale("alloc_p2", 2), allocation_allow_further_probing("alloc_probe_further", false), @@ -111,7 +112,8 @@ ProbeControllerConfig::ProbeControllerConfig( ParseFieldTrial( {&first_exponential_probe_scale, &second_exponential_probe_scale, &further_exponential_probe_scale, &further_probe_threshold, - &alr_probing_interval, &alr_probe_scale, &first_allocation_probe_scale, + &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, @@ -210,7 +212,8 @@ std::vector ProbeController::OnMaxTotalAllocatedBitrate( const bool in_alr = alr_start_time_.has_value(); const bool allow_allocation_probe = in_alr; - if (state_ == State::kProbingComplete && + if (config_.probe_on_max_allocated_bitrate_change && + state_ == State::kProbingComplete && max_total_allocated_bitrate != max_total_allocated_bitrate_ && estimated_bitrate_ < max_bitrate_ && estimated_bitrate_ < max_total_allocated_bitrate && @@ -364,10 +367,6 @@ std::vector ProbeController::RequestProbe( return std::vector(); } -void ProbeController::SetMaxBitrate(DataRate max_bitrate) { - max_bitrate_ = max_bitrate; -} - void ProbeController::SetNetworkStateEstimate( webrtc::NetworkStateEstimate estimate) { network_estimate_ = estimate; @@ -463,8 +462,12 @@ std::vector ProbeController::InitiateProbing( DataRate network_estimate = network_estimate_ ? network_estimate_->link_capacity_upper : DataRate::PlusInfinity(); + DataRate max_probe_rate = + max_total_allocated_bitrate_.IsZero() + ? max_bitrate_ + : 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_bitrate_) { + config_.skip_if_estimate_larger_than_fraction_of_max * max_probe_rate) { return {}; } } diff --git a/modules/congestion_controller/goog_cc/probe_controller.h b/modules/congestion_controller/goog_cc/probe_controller.h index 4bffc14439..6538b0eecd 100644 --- a/modules/congestion_controller/goog_cc/probe_controller.h +++ b/modules/congestion_controller/goog_cc/probe_controller.h @@ -62,6 +62,7 @@ struct ProbeControllerConfig { FieldTrialParameter network_state_probe_duration; // Configures the probes emitted by changed to the allocated bitrate. + FieldTrialParameter probe_on_max_allocated_bitrate_change; FieldTrialOptional first_allocation_probe_scale; FieldTrialOptional second_allocation_probe_scale; FieldTrialFlag allocation_allow_further_probing; @@ -119,8 +120,6 @@ class ProbeController { ABSL_MUST_USE_RESULT std::vector RequestProbe( Timestamp at_time); - // Sets a new maximum probing bitrate, without generating a new probe cluster. - void SetMaxBitrate(DataRate max_bitrate); void SetNetworkStateEstimate(webrtc::NetworkStateEstimate estimate); // Resets the ProbeController to a state equivalent to as if it was just diff --git a/modules/congestion_controller/goog_cc/probe_controller_unittest.cc b/modules/congestion_controller/goog_cc/probe_controller_unittest.cc index d7b580c3a0..120aa7a7eb 100644 --- a/modules/congestion_controller/goog_cc/probe_controller_unittest.cc +++ b/modules/congestion_controller/goog_cc/probe_controller_unittest.cc @@ -148,7 +148,7 @@ TEST(ProbeControllerTest, InitiatesProbingOnMaxBitrateIncrease) { EXPECT_EQ(probes[0].target_data_rate.bps(), kMaxBitrate.bps() + 100); } -TEST(ProbeControllerTest, ProbesOnMaxBitrateIncreaseOnlyWhenInAlr) { +TEST(ProbeControllerTest, ProbesOnMaxAllocatedBitrateIncreaseOnlyWhenInAlr) { ProbeControllerFixture fixture; std::unique_ptr probe_controller = fixture.CreateController(); @@ -176,6 +176,30 @@ TEST(ProbeControllerTest, ProbesOnMaxBitrateIncreaseOnlyWhenInAlr) { EXPECT_TRUE(probes.empty()); } +TEST(ProbeControllerTest, CanDisableProbingOnMaxTotalAllocatedBitrateIncrease) { + ProbeControllerFixture fixture( + "WebRTC-Bwe-ProbingConfiguration/" + "probe_max_allocation:false/"); + std::unique_ptr probe_controller = + fixture.CreateController(); + + auto probes = probe_controller->SetBitrates( + kMinBitrate, kStartBitrate, kMaxBitrate, fixture.CurrentTime()); + probes = probe_controller->SetEstimatedBitrate( + kMaxBitrate - DataRate::BitsPerSec(1), + /*bwe_limited_due_to_packet_loss=*/false, fixture.CurrentTime()); + fixture.AdvanceTime(kExponentialProbingTimeout); + probes = probe_controller->Process(fixture.CurrentTime()); + ASSERT_TRUE(probes.empty()); + probe_controller->SetAlrStartTimeMs(fixture.CurrentTime().ms()); + + // Do no probe, since probe_max_allocation:false. + probe_controller->SetAlrStartTimeMs(fixture.CurrentTime().ms()); + probes = probe_controller->OnMaxTotalAllocatedBitrate( + kMaxBitrate + DataRate::BitsPerSec(1), fixture.CurrentTime()); + EXPECT_TRUE(probes.empty()); +} + TEST(ProbeControllerTest, InitiatesProbingOnMaxBitrateIncreaseAtMaxBitrate) { ProbeControllerFixture fixture; std::unique_ptr probe_controller = @@ -787,8 +811,39 @@ TEST(ProbeControllerTest, SkipAlrProbeIfEstimateLargerThanMaxProbe) { EXPECT_TRUE(probes.empty()); // But if the max rate increase, A new probe is sent. - probe_controller->SetMaxBitrate(2 * kMaxBitrate); + probes = probe_controller->SetBitrates( + kMinBitrate, kStartBitrate, 2 * kMaxBitrate, fixture.CurrentTime()); + EXPECT_FALSE(probes.empty()); +} + +TEST(ProbeControllerTest, + SkipAlrProbeIfEstimateLargerThanFractionOfMaxAllocated) { + ProbeControllerFixture fixture( + "WebRTC-Bwe-ProbingConfiguration/" + "skip_if_est_larger_than_fraction_of_max:1.0/"); + std::unique_ptr probe_controller = + fixture.CreateController(); + probe_controller->EnablePeriodicAlrProbing(true); + auto probes = probe_controller->SetBitrates( + kMinBitrate, kStartBitrate, kMaxBitrate, fixture.CurrentTime()); + ASSERT_FALSE(probes.empty()); + probes = probe_controller->SetEstimatedBitrate( + kMaxBitrate / 2, /*bwe_limited_due_to_packet_loss=*/false, + fixture.CurrentTime()); + + fixture.AdvanceTime(TimeDelta::Seconds(10)); + probe_controller->SetAlrStartTimeMs(fixture.CurrentTime().ms()); + probes = probe_controller->OnMaxTotalAllocatedBitrate(kMaxBitrate / 2, + fixture.CurrentTime()); + // No probes since total allocated is not higher than the current estimate. + EXPECT_TRUE(probes.empty()); + fixture.AdvanceTime(TimeDelta::Seconds(2)); probes = probe_controller->Process(fixture.CurrentTime()); + EXPECT_TRUE(probes.empty()); + + // But if the max allocated increase, A new probe is sent. + probes = probe_controller->OnMaxTotalAllocatedBitrate( + kMaxBitrate / 2 + DataRate::BitsPerSec(1), fixture.CurrentTime()); EXPECT_FALSE(probes.empty()); }