diff --git a/modules/congestion_controller/goog_cc/probe_controller.cc b/modules/congestion_controller/goog_cc/probe_controller.cc index bc56ce0350..c1a39f8595 100644 --- a/modules/congestion_controller/goog_cc/probe_controller.cc +++ b/modules/congestion_controller/goog_cc/probe_controller.cc @@ -574,7 +574,6 @@ std::vector ProbeController::InitiateProbing( std::min(max_probe_bitrate, max_total_allocated_bitrate_ * 2); } - DataRate estimate_capped_bitrate = DataRate::PlusInfinity(); switch (bandwidth_limited_cause_) { case BandwidthLimitedCause::kRttBasedBackOffHighRtt: case BandwidthLimitedCause::kDelayBasedLimitedDelayIncreased: @@ -583,7 +582,7 @@ std::vector ProbeController::InitiateProbing( << static_cast(bandwidth_limited_cause_); return {}; case BandwidthLimitedCause::kLossLimitedBweIncreasing: - estimate_capped_bitrate = + max_probe_bitrate = std::min(max_probe_bitrate, estimated_bitrate_ * config_.loss_limited_probe_scale); break; @@ -599,8 +598,8 @@ std::vector ProbeController::InitiateProbing( RTC_LOG(LS_INFO) << "Not sending probe, Network state estimate is zero"; return {}; } - estimate_capped_bitrate = std::min( - {estimate_capped_bitrate, max_probe_bitrate, + max_probe_bitrate = std::min( + {max_probe_bitrate, std::max(estimated_bitrate_, network_estimate_->link_capacity_upper * config_.network_state_probe_scale)}); } @@ -608,12 +607,10 @@ std::vector ProbeController::InitiateProbing( std::vector pending_probes; for (DataRate bitrate : bitrates_to_probe) { RTC_DCHECK(!bitrate.IsZero()); - bitrate = std::min(bitrate, estimate_capped_bitrate); - if (bitrate > max_probe_bitrate) { + if (bitrate >= max_probe_bitrate) { bitrate = max_probe_bitrate; probe_further = false; } - pending_probes.push_back(CreateProbeClusterConfig(now, bitrate)); } time_last_probing_initiated_ = now; @@ -621,9 +618,8 @@ std::vector ProbeController::InitiateProbing( UpdateState(State::kWaitingForProbingResult); // Dont expect probe results to be larger than a fraction of the actual // probe rate. - min_bitrate_to_probe_further_ = - std::min(estimate_capped_bitrate, (*(bitrates_to_probe.end() - 1))) * - config_.further_probe_threshold; + min_bitrate_to_probe_further_ = pending_probes.back().target_data_rate * + config_.further_probe_threshold; } else { UpdateState(State::kProbingComplete); } diff --git a/modules/congestion_controller/goog_cc/probe_controller_unittest.cc b/modules/congestion_controller/goog_cc/probe_controller_unittest.cc index 01d4a6d5b5..bb3c79c466 100644 --- a/modules/congestion_controller/goog_cc/probe_controller_unittest.cc +++ b/modules/congestion_controller/goog_cc/probe_controller_unittest.cc @@ -988,39 +988,6 @@ TEST(ProbeControllerTest, ProbeInAlrIfLossBasedIncreasing) { EXPECT_EQ(probes.at(0).target_data_rate, 1.5 * kStartBitrate); } -TEST(ProbeControllerTest, ProbeFurtherInAlrIfLossBasedIncreasing) { - ProbeControllerFixture fixture; - 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()); - probe_controller->EnablePeriodicAlrProbing(true); - probes = probe_controller->SetEstimatedBitrate( - kStartBitrate, BandwidthLimitedCause::kLossLimitedBweIncreasing, - fixture.CurrentTime()); - - // Wait long enough to time out exponential probing. - fixture.AdvanceTime(kExponentialProbingTimeout); - probes = probe_controller->Process(fixture.CurrentTime()); - ASSERT_TRUE(probes.empty()); - - // Probe when in alr. - probe_controller->SetAlrStartTimeMs(fixture.CurrentTime().ms()); - fixture.AdvanceTime(kAlrProbeInterval + TimeDelta::Millis(1)); - probes = probe_controller->Process(fixture.CurrentTime()); - ASSERT_EQ(probes.size(), 1u); - ASSERT_EQ(probes.at(0).target_data_rate, 1.5 * kStartBitrate); - - probes = probe_controller->SetEstimatedBitrate( - 1.5 * kStartBitrate, BandwidthLimitedCause::kLossLimitedBweIncreasing, - fixture.CurrentTime()); - ASSERT_EQ(probes.size(), 1u); - EXPECT_EQ(probes[0].target_data_rate, 1.5 * 1.5 * kStartBitrate); -} - TEST(ProbeControllerTest, NotProbeWhenInAlrIfLossBasedDecreases) { ProbeControllerFixture fixture; std::unique_ptr probe_controller = @@ -1227,10 +1194,11 @@ TEST(ProbeControllerTest, ProbeFurtherWhenDelayBasedLimited) { } TEST(ProbeControllerTest, - ProbeFurtherIfNetworkStateEstimateIncreaseAfterProbeSent) { + ProbeAfterTimeoutIfNetworkStateEstimateIncreaseAfterProbeSent) { ProbeControllerFixture fixture( "WebRTC-Bwe-ProbingConfiguration/" - "network_state_interval:5s/"); + "network_state_interval:5s,est_lower_than_network_interval:3s,est_lower_" + "than_network_ratio:0.7/"); std::unique_ptr probe_controller = fixture.CreateController(); ASSERT_THAT( @@ -1252,14 +1220,18 @@ TEST(ProbeControllerTest, probes = probe_controller->Process(fixture.CurrentTime()); ASSERT_FALSE(probes.empty()); EXPECT_LE(probes[0].target_data_rate, state_estimate.link_capacity_upper); - // If the network state estimate increase above the threshold to probe - // further, and the probe suceeed, expect a new probe. + // If the network state estimate increase, even before the probe result, + // expect a new probe after `est_lower_than_network_interval` timeout. state_estimate.link_capacity_upper = 3 * kStartBitrate; probe_controller->SetNetworkStateEstimate(state_estimate); probes = probe_controller->SetEstimatedBitrate( probes[0].target_data_rate, BandwidthLimitedCause::kDelayBasedLimited, fixture.CurrentTime()); - EXPECT_FALSE(probes.empty()); + EXPECT_THAT(probes, IsEmpty()); + + fixture.AdvanceTime(TimeDelta::Seconds(3)); + probes = probe_controller->Process(fixture.CurrentTime()); + EXPECT_THAT(probes, Not(IsEmpty())); // But no more probes if estimate is close to the link capacity. probes = probe_controller->SetEstimatedBitrate( @@ -1268,6 +1240,45 @@ TEST(ProbeControllerTest, EXPECT_TRUE(probes.empty()); } +TEST(ProbeControllerTest, SkipProbeFurtherIfAlreadyProbedToMaxRate) { + ProbeControllerFixture fixture( + "WebRTC-Bwe-ProbingConfiguration/" + "network_state_interval:2s,skip_if_est_larger_than_fraction_of_max:0.9/"); + 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_FALSE(probes.empty()); + + probe_controller->SetNetworkStateEstimate( + {.link_capacity_upper = 2 * kMaxBitrate}); + + // Attempt to probe up to max rate. + probes = probe_controller->SetEstimatedBitrate( + kMaxBitrate * 0.8, BandwidthLimitedCause::kDelayBasedLimited, + fixture.CurrentTime()); + ASSERT_FALSE(probes.empty()); + EXPECT_EQ(probes[0].target_data_rate, kMaxBitrate); + + // If the probe result arrives, dont expect a new probe immediately since we + // already tried to probe at the max rate. + probes = probe_controller->SetEstimatedBitrate( + kMaxBitrate * 0.8, BandwidthLimitedCause::kDelayBasedLimited, + fixture.CurrentTime()); + EXPECT_TRUE(probes.empty()); + + fixture.AdvanceTime(TimeDelta::Millis(1000)); + probes = probe_controller->Process(fixture.CurrentTime()); + EXPECT_THAT(probes, IsEmpty()); + // But when enough time has passed, expect a new probe. + fixture.AdvanceTime(TimeDelta::Millis(1000)); + probes = probe_controller->Process(fixture.CurrentTime()); + EXPECT_THAT(probes, Not(IsEmpty())); +} + TEST(ProbeControllerTest, MaxAllocatedBitrateNotReset) { ProbeControllerFixture fixture; std::unique_ptr probe_controller =