From dd7dc25a30c5841e6620d195b83058a22ffff7cd Mon Sep 17 00:00:00 2001 From: Per Kjellander Date: Thu, 10 Nov 2022 14:39:52 +0100 Subject: [PATCH] Continue probing if networkstat estimate increase This fixes an issue where continues probing stops if networkstate estimate is low when a probe is sent, but increase as a consequence of the probe. Bug: webrtc:14392 Change-Id: Id1d703f7efc824a6a6f8d899c367660291bd88c8 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/282941 Reviewed-by: Diep Bui Commit-Queue: Per Kjellander Cr-Commit-Position: refs/heads/main@{#38606} --- .../goog_cc/probe_controller.cc | 37 +++++++++++-------- .../goog_cc/probe_controller_unittest.cc | 36 ++++++++++++++++++ 2 files changed, 58 insertions(+), 15 deletions(-) diff --git a/modules/congestion_controller/goog_cc/probe_controller.cc b/modules/congestion_controller/goog_cc/probe_controller.cc index 6f5fe09c8f..ac5a9154a4 100644 --- a/modules/congestion_controller/goog_cc/probe_controller.cc +++ b/modules/congestion_controller/goog_cc/probe_controller.cc @@ -468,25 +468,13 @@ std::vector ProbeController::InitiateProbing( : 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_probe_rate) { + state_ = State::kProbingComplete; + min_bitrate_to_probe_further_ = DataRate::PlusInfinity(); return {}; } } DataRate max_probe_bitrate = max_bitrate_; - if (bwe_limited_due_to_packet_loss_ && - config_.limit_probe_target_rate_to_loss_bwe) { - max_probe_bitrate = std::min(estimated_bitrate_, max_bitrate_); - } - if (config_.network_state_estimate_probing_interval->IsFinite() && - network_estimate_ && network_estimate_->link_capacity_upper.IsFinite()) { - if (network_estimate_->link_capacity_upper.IsZero()) { - RTC_LOG(LS_INFO) << "Not sending probe, Network state estimate is zero"; - return {}; - } - max_probe_bitrate = - std::min(max_probe_bitrate, network_estimate_->link_capacity_upper * - config_.network_state_probe_scale); - } if (max_total_allocated_bitrate_ > DataRate::Zero()) { // If a max allocated bitrate has been configured, allow probing up to 2x // that rate. This allows some overhead to account for bursty streams, @@ -498,10 +486,28 @@ std::vector ProbeController::InitiateProbing( std::min(max_probe_bitrate, max_total_allocated_bitrate_ * 2); } + DataRate estimate_capped_bitrate = DataRate::PlusInfinity(); + if (bwe_limited_due_to_packet_loss_ && + config_.limit_probe_target_rate_to_loss_bwe) { + estimate_capped_bitrate = std::min(estimated_bitrate_, max_probe_bitrate); + } + if (config_.network_state_estimate_probing_interval->IsFinite() && + network_estimate_ && network_estimate_->link_capacity_upper.IsFinite()) { + if (network_estimate_->link_capacity_upper.IsZero()) { + RTC_LOG(LS_INFO) << "Not sending probe, Network state estimate is zero"; + return {}; + } + estimate_capped_bitrate = + std::min({estimate_capped_bitrate, max_probe_bitrate, + network_estimate_->link_capacity_upper * + config_.network_state_probe_scale}); + } + 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) { bitrate = max_probe_bitrate; probe_further = false; @@ -527,7 +533,8 @@ std::vector ProbeController::InitiateProbing( if (probe_further) { state_ = State::kWaitingForProbingResult; min_bitrate_to_probe_further_ = - (*(bitrates_to_probe.end() - 1)) * config_.further_probe_threshold; + std::min(estimate_capped_bitrate, (*(bitrates_to_probe.end() - 1))) * + config_.further_probe_threshold; } else { state_ = State::kProbingComplete; min_bitrate_to_probe_further_ = DataRate::PlusInfinity(); diff --git a/modules/congestion_controller/goog_cc/probe_controller_unittest.cc b/modules/congestion_controller/goog_cc/probe_controller_unittest.cc index 120aa7a7eb..5b1d79bd49 100644 --- a/modules/congestion_controller/goog_cc/probe_controller_unittest.cc +++ b/modules/congestion_controller/goog_cc/probe_controller_unittest.cc @@ -789,6 +789,42 @@ TEST(ProbeControllerTest, ProbeFurtherWhenDelayBasedLimited) { EXPECT_EQ(probes[0].target_data_rate, state_estimate.link_capacity_upper); } +TEST(ProbeControllerTest, + ProbeFurtherIfNetworkStateEstimateIncreaseAfterProbeSent) { + ProbeControllerFixture fixture( + "WebRTC-Bwe-ProbingConfiguration/" + "network_state_interval:5s,limit_probe_target_rate_to_loss_bwe:true/"); + std::unique_ptr probe_controller = + fixture.CreateController(); + + auto probes = probe_controller->SetBitrates( + kMinBitrate, kStartBitrate, kMaxBitrate, fixture.CurrentTime()); + ASSERT_FALSE(probes.empty()); + + NetworkStateEstimate state_estimate; + state_estimate.link_capacity_upper = 1.2 * probes[0].target_data_rate / 2; + probe_controller->SetNetworkStateEstimate(state_estimate); + + // No immediate further probing since probe result is low. + probes = probe_controller->SetEstimatedBitrate( + probes[0].target_data_rate / 2, /*bwe_limited_due_to_packet_loss=*/false, + fixture.CurrentTime()); + ASSERT_TRUE(probes.empty()); + + fixture.AdvanceTime(TimeDelta::Seconds(5)); + 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. + state_estimate.link_capacity_upper = 3 * kStartBitrate; + probe_controller->SetNetworkStateEstimate(state_estimate); + probes = probe_controller->SetEstimatedBitrate( + probes[0].target_data_rate, /*bwe_limited_due_to_packet_loss=*/false, + fixture.CurrentTime()); + EXPECT_FALSE(probes.empty()); +} + TEST(ProbeControllerTest, SkipAlrProbeIfEstimateLargerThanMaxProbe) { ProbeControllerFixture fixture( "WebRTC-Bwe-ProbingConfiguration/"