From 6b0aea07ab927b6297ec0c1af27d4bbe6caa7f61 Mon Sep 17 00:00:00 2001 From: Per Kjellander Date: Fri, 11 Nov 2022 12:01:43 +0100 Subject: [PATCH] Reland "Continue probing if networkstat estimate increase"" Patchset 1 contrains the original cl. Later patchsets contain fix. Original description: 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: I8d4e1968020f9f8de18e12a4a0322a87f1a8fd2f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/283082 Reviewed-by: Diep Bui Commit-Queue: Per Kjellander Cr-Commit-Position: refs/heads/main@{#38612} --- .../goog_cc/probe_controller.cc | 57 ++++++++++++------- .../goog_cc/probe_controller_unittest.cc | 39 +++++++++++++ 2 files changed, 75 insertions(+), 21 deletions(-) diff --git a/modules/congestion_controller/goog_cc/probe_controller.cc b/modules/congestion_controller/goog_cc/probe_controller.cc index 6f5fe09c8f..aac957622f 100644 --- a/modules/congestion_controller/goog_cc/probe_controller.cc +++ b/modules/congestion_controller/goog_cc/probe_controller.cc @@ -298,21 +298,27 @@ std::vector ProbeController::SetEstimatedBitrate( bitrate.kbps()); mid_call_probing_waiting_for_result_ = false; } - std::vector pending_probes; if (state_ == State::kWaitingForProbingResult) { // Continue probing if probing results indicate channel has greater // capacity. + DataRate network_state_estimate_probe_further_limit = + config_.network_state_estimate_probing_interval->IsFinite() && + network_estimate_ + ? network_estimate_->link_capacity_upper * + config_.further_probe_threshold + : DataRate::PlusInfinity(); RTC_LOG(LS_INFO) << "Measured bitrate: " << bitrate << " Minimum to probe further: " - << min_bitrate_to_probe_further_; + << min_bitrate_to_probe_further_ << " upper limit: " + << network_state_estimate_probe_further_limit; - if (bitrate > min_bitrate_to_probe_further_) { - pending_probes = InitiateProbing( + if (bitrate > min_bitrate_to_probe_further_ && + bitrate <= network_state_estimate_probe_further_limit) { + return InitiateProbing( at_time, {config_.further_exponential_probe_scale * bitrate}, true); } } - - return pending_probes; + return {}; } void ProbeController::EnablePeriodicAlrProbing(bool enable) { @@ -468,25 +474,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 +492,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; @@ -526,8 +538,11 @@ std::vector ProbeController::InitiateProbing( time_last_probing_initiated_ = now; if (probe_further) { state_ = State::kWaitingForProbingResult; + // Dont expect probe results to be larger than a fraction of the actual + // probe rate. 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..c060664cb7 100644 --- a/modules/congestion_controller/goog_cc/probe_controller_unittest.cc +++ b/modules/congestion_controller/goog_cc/probe_controller_unittest.cc @@ -789,6 +789,45 @@ 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()); + + // But no more probes if estimate is close to the link capacity. + probes = probe_controller->SetEstimatedBitrate( + state_estimate.link_capacity_upper * 0.9, + /*bwe_limited_due_to_packet_loss=*/false, fixture.CurrentTime()); + EXPECT_TRUE(probes.empty()); +} + TEST(ProbeControllerTest, SkipAlrProbeIfEstimateLargerThanMaxProbe) { ProbeControllerFixture fixture( "WebRTC-Bwe-ProbingConfiguration/"