From 55f6613fc02323f0b65ee6794c782c1e2aea41b8 Mon Sep 17 00:00:00 2001 From: Per K Date: Thu, 2 May 2024 10:23:32 +0000 Subject: [PATCH] Retry initial probe if it times out and BWE has not been updated. This is to avoid the case where the initial probe fail and the BWE is not updated, which can lead to a long period of low bandwidth. Bug: webrtc:14928 Change-Id: Ie8f84270507b59995d57e4ab6e2a984570191529 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/349580 Commit-Queue: Per Kjellander Reviewed-by: Diep Bui Cr-Commit-Position: refs/heads/main@{#42208} --- .../goog_cc/probe_controller.cc | 8 +++ .../goog_cc/probe_controller_unittest.cc | 49 +++++++++++++++++-- 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/modules/congestion_controller/goog_cc/probe_controller.cc b/modules/congestion_controller/goog_cc/probe_controller.cc index 36d61bf9fc..8961675f9c 100644 --- a/modules/congestion_controller/goog_cc/probe_controller.cc +++ b/modules/congestion_controller/goog_cc/probe_controller.cc @@ -476,6 +476,14 @@ std::vector ProbeController::Process(Timestamp at_time) { if (at_time - time_last_probing_initiated_ > kMaxWaitingTimeForProbingResult) { if (state_ == State::kWaitingForProbingResult) { + // If the initial probe timed out, and the estimate has not changed by + // other means, (likely because normal media packets are not being sent + // yet), then send a probe again. + if (waiting_for_initial_probe_result_ && + estimated_bitrate_ == start_bitrate_ && first_probe_to_max_bitrate_) { + UpdateState(State::kInit); + return InitiateExponentialProbing(at_time); + } RTC_LOG(LS_INFO) << "kWaitingForProbingResult: timeout"; 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 8024fec4dd..6691e87d3d 100644 --- a/modules/congestion_controller/goog_cc/probe_controller_unittest.cc +++ b/modules/congestion_controller/goog_cc/probe_controller_unittest.cc @@ -438,7 +438,7 @@ TEST(ProbeControllerTest, EXPECT_EQ(probes[0].target_data_rate.bps(), 2 * kStartBitrate.bps()); } -TEST(ProbeControllerTest, TestExponentialProbingTimeout) { +TEST(ProbeControllerTest, InitialProbingTimeout) { ProbeControllerFixture fixture; std::unique_ptr probe_controller = fixture.CreateController(); @@ -447,14 +447,57 @@ TEST(ProbeControllerTest, TestExponentialProbingTimeout) { IsEmpty()); auto probes = probe_controller->SetBitrates( kMinBitrate, kStartBitrate, kMaxBitrate, fixture.CurrentTime()); + EXPECT_THAT(probes, SizeIs(Gt(0))); // Advance far enough to cause a time out in waiting for probing result. fixture.AdvanceTime(kExponentialProbingTimeout); probes = probe_controller->Process(fixture.CurrentTime()); - + EXPECT_THAT(probes, IsEmpty()); probes = probe_controller->SetEstimatedBitrate( DataRate::BitsPerSec(1800), BandwidthLimitedCause::kDelayBasedLimited, fixture.CurrentTime()); - EXPECT_TRUE(probes.empty()); + EXPECT_THAT(probes, IsEmpty()); +} + +TEST( + ProbeControllerTest, + InitialProbingRetriedAfterTimeoutIfFirstProbeToMaxBitrateAndBweNotUpdated) { + ProbeControllerFixture fixture; + std::unique_ptr probe_controller = + fixture.CreateController(); + probe_controller->SetFirstProbeToMaxBitrate(true); + ASSERT_THAT( + probe_controller->OnNetworkAvailability({.network_available = true}), + IsEmpty()); + auto probes = probe_controller->SetBitrates( + kMinBitrate, kStartBitrate, kMaxBitrate, fixture.CurrentTime()); + EXPECT_THAT(probes, SizeIs(Gt(0))); + // Advance far enough to cause a time out in waiting for probing result. + fixture.AdvanceTime(kExponentialProbingTimeout); + probes = probe_controller->Process(fixture.CurrentTime()); + EXPECT_THAT(probes, SizeIs(Gt(0))); +} + +TEST(ProbeControllerTest, + InitialProbingNotRetriedAfterTimeoutIfFirstProbeAndBweUpdated) { + ProbeControllerFixture fixture; + std::unique_ptr probe_controller = + fixture.CreateController(); + probe_controller->SetFirstProbeToMaxBitrate(true); + ASSERT_THAT( + probe_controller->OnNetworkAvailability({.network_available = true}), + IsEmpty()); + auto probes = probe_controller->SetBitrates( + kMinBitrate, kStartBitrate, kMaxBitrate, fixture.CurrentTime()); + EXPECT_THAT(probes, SizeIs(Gt(0))); + fixture.AdvanceTime(TimeDelta::Millis(700)); + probes = probe_controller->SetEstimatedBitrate( + DataRate::BitsPerSec(180), BandwidthLimitedCause::kDelayBasedLimited, + fixture.CurrentTime()); + EXPECT_THAT(probes, IsEmpty()); + // Advance far enough to cause a time out in waiting for probing result. + fixture.AdvanceTime(kExponentialProbingTimeout); + probes = probe_controller->Process(fixture.CurrentTime()); + EXPECT_THAT(probes, IsEmpty()); } TEST(ProbeControllerTest, RequestProbeInAlr) {