From 25f35a8fa56180b262402c39c97e7f87fd4d1b44 Mon Sep 17 00:00:00 2001 From: Konrad Hofbauer Date: Wed, 10 Apr 2019 12:38:06 +0200 Subject: [PATCH] Add FieldTrial to only send probes on OnMaxTotalAllocatedBitrate() if currently sent bitrate is application-limited. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: chromium:951299 Change-Id: Ibc1ebd74eaa4a019dc290c11b606796c5be21d0f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/131126 Commit-Queue: Konrad Hofbauer Reviewed-by: Sebastian Jansson Reviewed-by: Erik Språng Reviewed-by: Christoffer Rodbro Cr-Commit-Position: refs/heads/master@{#27539} --- .../goog_cc/probe_controller.cc | 14 +++++++++- .../goog_cc/probe_controller.h | 1 + .../goog_cc/probe_controller_unittest.cc | 26 +++++++++++++++++++ 3 files changed, 40 insertions(+), 1 deletion(-) diff --git a/modules/congestion_controller/goog_cc/probe_controller.cc b/modules/congestion_controller/goog_cc/probe_controller.cc index 3ff96c47f5..f0ad41397a 100644 --- a/modules/congestion_controller/goog_cc/probe_controller.cc +++ b/modules/congestion_controller/goog_cc/probe_controller.cc @@ -73,6 +73,10 @@ constexpr char kBweRapidRecoveryExperiment[] = // Never probe higher than configured by OnMaxTotalAllocatedBitrate(). constexpr char kCappedProbingFieldTrialName[] = "WebRTC-BweCappedProbing"; +// Only do allocation probing when in ALR (but not when network-limited). +constexpr char kAllocProbingOnlyInAlrFieldTrialName[] = + "WebRTC-BweAllocProbingOnlyInAlr"; + void MaybeLogProbeClusterCreated(RtcEventLog* event_log, const ProbeClusterConfig& probe) { RTC_DCHECK(event_log); @@ -121,6 +125,9 @@ ProbeController::ProbeController(const WebRtcKeyValueConfig* key_value_config, limit_probes_with_allocateable_rate_( key_value_config->Lookup(kCappedProbingFieldTrialName) .find("Disabled") != 0), + allocation_probing_only_in_alr_( + key_value_config->Lookup(kAllocProbingOnlyInAlrFieldTrialName) + .find("Enabled") == 0), event_log_(event_log), config_(ProbeControllerConfig(key_value_config)) { Reset(0); @@ -181,11 +188,16 @@ std::vector ProbeController::SetBitrates( std::vector ProbeController::OnMaxTotalAllocatedBitrate( int64_t max_total_allocated_bitrate, int64_t at_time_ms) { + const bool in_alr = alr_start_time_ms_.has_value(); + const bool allow_allocation_probe = + allocation_probing_only_in_alr_ ? in_alr : true; + if (state_ == State::kProbingComplete && max_total_allocated_bitrate != max_total_allocated_bitrate_ && estimated_bitrate_bps_ != 0 && (max_bitrate_bps_ <= 0 || estimated_bitrate_bps_ < max_bitrate_bps_) && - estimated_bitrate_bps_ < max_total_allocated_bitrate) { + estimated_bitrate_bps_ < max_total_allocated_bitrate && + allow_allocation_probe) { max_total_allocated_bitrate_ = max_total_allocated_bitrate; if (!config_.first_allocation_probe_scale) diff --git a/modules/congestion_controller/goog_cc/probe_controller.h b/modules/congestion_controller/goog_cc/probe_controller.h index ebf8c94339..71ce001f1d 100644 --- a/modules/congestion_controller/goog_cc/probe_controller.h +++ b/modules/congestion_controller/goog_cc/probe_controller.h @@ -134,6 +134,7 @@ class ProbeController { const bool in_rapid_recovery_experiment_; const bool limit_probes_with_allocateable_rate_; + const bool allocation_probing_only_in_alr_; // For WebRTC.BWE.MidCallProbing.* metric. bool mid_call_probing_waiting_for_result_; int64_t mid_call_probing_bitrate_bps_; diff --git a/modules/congestion_controller/goog_cc/probe_controller_unittest.cc b/modules/congestion_controller/goog_cc/probe_controller_unittest.cc index df806c21fd..e1335c13a7 100644 --- a/modules/congestion_controller/goog_cc/probe_controller_unittest.cc +++ b/modules/congestion_controller/goog_cc/probe_controller_unittest.cc @@ -95,6 +95,32 @@ TEST_F(ProbeControllerTest, InitiatesProbingOnMaxBitrateIncrease) { EXPECT_EQ(probes[0].target_data_rate.bps(), kMaxBitrateBps + 100); } +TEST_F(ProbeControllerTest, ProbesOnMaxBitrateIncreaseOnlyWhenInAlr) { + test::ScopedFieldTrials trials("WebRTC-BweAllocProbingOnlyInAlr/Enabled/"); + probe_controller_.reset( + new ProbeController(&field_trial_config_, &mock_rtc_event_log)); + auto probes = probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, + kMaxBitrateBps, NowMs()); + probes = probe_controller_->SetEstimatedBitrate(kMaxBitrateBps - 1, NowMs()); + + // Wait long enough to time out exponential probing. + clock_.AdvanceTimeMilliseconds(kExponentialProbingTimeoutMs); + probes = probe_controller_->Process(NowMs()); + EXPECT_EQ(probes.size(), 0u); + + // Probe when in alr. + probe_controller_->SetAlrStartTimeMs(clock_.TimeInMilliseconds()); + probes = probe_controller_->OnMaxTotalAllocatedBitrate(kMaxBitrateBps + 1, + NowMs()); + EXPECT_EQ(probes.size(), 2u); + + // Do not probe when not in alr. + probe_controller_->SetAlrStartTimeMs(absl::nullopt); + probes = probe_controller_->OnMaxTotalAllocatedBitrate(kMaxBitrateBps + 2, + NowMs()); + EXPECT_TRUE(probes.empty()); +} + TEST_F(ProbeControllerTest, InitiatesProbingOnMaxBitrateIncreaseAtMaxBitrate) { auto probes = probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, kMaxBitrateBps, NowMs());