From b2ecc3dc49dc3b8aac1ef439ae5d6e75cfa7b2db Mon Sep 17 00:00:00 2001 From: Sebastian Jansson Date: Fri, 13 Jul 2018 17:22:01 +0200 Subject: [PATCH] Removes old probe controller. This removes the old version of Probe Controller. The new controller is slightly different, therefore the legacy SendSideCongestionController is changed to accommodate the new function. Most notably, the functionality is changed so that probes are now sent only from the OnProcess call and not immediately on changing a parameter. The lock previously owned and used by ProbeController is moved to SendSideCongestionController. This should not change any behavior. Bug: webrtc:8415 Change-Id: I3c69ddeb04aeeae1234a2a5e116fb677f36b4ae4 Reviewed-on: https://webrtc-review.googlesource.com/86541 Commit-Queue: Sebastian Jansson Reviewed-by: Philip Eliasson Cr-Commit-Position: refs/heads/master@{#23973} --- modules/congestion_controller/BUILD.gn | 4 +- .../congestion_controller/goog_cc/BUILD.gn | 27 +- .../goog_cc/probe_controller.cc | 3 +- .../goog_cc/probe_controller.h | 3 - .../include/send_side_congestion_controller.h | 6 +- .../congestion_controller/probe_controller.cc | 325 ------------------ .../congestion_controller/probe_controller.h | 99 ------ .../probe_controller_unittest.cc | 289 ---------------- .../send_side_congestion_controller.cc | 71 +++- 9 files changed, 90 insertions(+), 737 deletions(-) delete mode 100644 modules/congestion_controller/probe_controller.cc delete mode 100644 modules/congestion_controller/probe_controller.h delete mode 100644 modules/congestion_controller/probe_controller_unittest.cc diff --git a/modules/congestion_controller/BUILD.gn b/modules/congestion_controller/BUILD.gn index e281bee7bd..175730deb8 100644 --- a/modules/congestion_controller/BUILD.gn +++ b/modules/congestion_controller/BUILD.gn @@ -24,8 +24,6 @@ rtc_static_library("congestion_controller") { "include/receive_side_congestion_controller.h", "include/send_side_congestion_controller.h", "include/send_side_congestion_controller_interface.h", - "probe_controller.cc", - "probe_controller.h", "receive_side_congestion_controller.cc", "send_side_congestion_controller.cc", ] @@ -51,6 +49,7 @@ rtc_static_library("congestion_controller") { "../rtp_rtcp:rtp_rtcp_format", "goog_cc:delay_based_bwe", "goog_cc:estimators", + "goog_cc:probe_controller", "//third_party/abseil-cpp/absl/memory", ] @@ -96,7 +95,6 @@ if (rtc_include_tests) { testonly = true sources = [ - "probe_controller_unittest.cc", "receive_side_congestion_controller_unittest.cc", "send_side_congestion_controller_unittest.cc", "transport_feedback_adapter_unittest.cc", diff --git a/modules/congestion_controller/goog_cc/BUILD.gn b/modules/congestion_controller/goog_cc/BUILD.gn index 05f2212f4a..e802f3bd2d 100644 --- a/modules/congestion_controller/goog_cc/BUILD.gn +++ b/modules/congestion_controller/goog_cc/BUILD.gn @@ -23,8 +23,6 @@ rtc_static_library("goog_cc") { "goog_cc_network_control.cc", "goog_cc_network_control.h", "include/goog_cc_factory.h", - "probe_controller.cc", - "probe_controller.h", ] if (!build_with_chromium && is_clang) { @@ -36,6 +34,7 @@ rtc_static_library("goog_cc") { ":alr_detector", ":delay_based_bwe", ":estimators", + ":probe_controller", "../..:module_api", "../../..:webrtc_common", "../../../:typedefs", @@ -146,6 +145,29 @@ rtc_source_set("delay_based_bwe") { } } +rtc_source_set("probe_controller") { + sources = [ + "probe_controller.cc", + "probe_controller.h", + ] + + deps = [ + "../../../api/transport:network_control", + "../../../logging:rtc_event_log_api", + "../../../logging:rtc_event_pacing", + "../../../rtc_base:checks", + "../../../rtc_base:rtc_base_approved", + "../../../system_wrappers:field_trial_api", + "../../../system_wrappers:metrics_api", + "//third_party/abseil-cpp/absl/types:optional", + ] + + if (!build_with_chromium && is_clang) { + # Suppress warnings from the Chromium Clang plugin (bugs.webrtc.org/163). + suppressed_configs += [ "//build/config/clang:find_bad_constructs" ] + } +} + if (rtc_include_tests) { rtc_source_set("test_goog_cc_printer") { testonly = true @@ -177,6 +199,7 @@ if (rtc_include_tests) { ":delay_based_bwe", ":estimators", ":goog_cc", + ":probe_controller", "../../../api/transport:network_control", "../../../api/transport:network_control_test", "../../../rtc_base:checks", diff --git a/modules/congestion_controller/goog_cc/probe_controller.cc b/modules/congestion_controller/goog_cc/probe_controller.cc index fbc0620a21..37763b4e6e 100644 --- a/modules/congestion_controller/goog_cc/probe_controller.cc +++ b/modules/congestion_controller/goog_cc/probe_controller.cc @@ -19,7 +19,6 @@ #include "system_wrappers/include/metrics.h" namespace webrtc { -namespace webrtc_cc { namespace { // The minimum number probing packets used. @@ -142,6 +141,7 @@ void ProbeController::OnMaxTotalAllocatedBitrate( estimated_bitrate_bps_ != 0 && (max_bitrate_bps_ <= 0 || estimated_bitrate_bps_ < max_bitrate_bps_) && estimated_bitrate_bps_ < max_total_allocated_bitrate) { + max_total_allocated_bitrate_ = max_total_allocated_bitrate; InitiateProbing(at_time_ms, {max_total_allocated_bitrate}, false); } } @@ -334,5 +334,4 @@ void ProbeController::InitiateProbing( } } -} // namespace webrtc_cc } // namespace webrtc diff --git a/modules/congestion_controller/goog_cc/probe_controller.h b/modules/congestion_controller/goog_cc/probe_controller.h index c8946e8221..9f8e8ad5b5 100644 --- a/modules/congestion_controller/goog_cc/probe_controller.h +++ b/modules/congestion_controller/goog_cc/probe_controller.h @@ -24,8 +24,6 @@ namespace webrtc { class Clock; -namespace webrtc_cc { - // This class controls initiation of probing to estimate initial channel // capacity. There is also support for probing during a session when max // bitrate is adjusted by an application. @@ -104,7 +102,6 @@ class ProbeController { RTC_DISALLOW_COPY_AND_ASSIGN(ProbeController); }; -} // namespace webrtc_cc } // namespace webrtc #endif // MODULES_CONGESTION_CONTROLLER_GOOG_CC_PROBE_CONTROLLER_H_ diff --git a/modules/congestion_controller/include/send_side_congestion_controller.h b/modules/congestion_controller/include/send_side_congestion_controller.h index 98efb19d88..7e5022eda0 100644 --- a/modules/congestion_controller/include/send_side_congestion_controller.h +++ b/modules/congestion_controller/include/send_side_congestion_controller.h @@ -131,6 +131,7 @@ class SendSideCongestionController uint8_t fraction_loss, int64_t rtt); void LimitOutstandingBytes(size_t num_outstanding_bytes); + void SendPendingProbes() RTC_EXCLUSIVE_LOCKS_REQUIRED(&probe_lock_); const Clock* const clock_; rtc::CriticalSection observer_lock_; Observer* observer_ RTC_GUARDED_BY(observer_lock_); @@ -138,7 +139,10 @@ class SendSideCongestionController PacedSender* const pacer_; const std::unique_ptr bitrate_controller_; std::unique_ptr acknowledged_bitrate_estimator_; - const std::unique_ptr probe_controller_; + rtc::CriticalSection probe_lock_; + const std::unique_ptr probe_controller_ + RTC_GUARDED_BY(probe_lock_); + const std::unique_ptr retransmission_rate_limiter_; TransportFeedbackAdapter transport_feedback_adapter_; rtc::CriticalSection network_state_lock_; diff --git a/modules/congestion_controller/probe_controller.cc b/modules/congestion_controller/probe_controller.cc deleted file mode 100644 index 674fa4231f..0000000000 --- a/modules/congestion_controller/probe_controller.cc +++ /dev/null @@ -1,325 +0,0 @@ -/* - * Copyright (c) 2016 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#include "modules/congestion_controller/probe_controller.h" - -#include -#include - -#include "rtc_base/logging.h" -#include "rtc_base/numerics/safe_conversions.h" -#include "system_wrappers/include/field_trial.h" -#include "system_wrappers/include/metrics.h" - -namespace webrtc { - -namespace { -// Maximum waiting time from the time of initiating probing to getting -// the measured results back. -constexpr int64_t kMaxWaitingTimeForProbingResultMs = 1000; - -// Value of |min_bitrate_to_probe_further_bps_| that indicates -// further probing is disabled. -constexpr int kExponentialProbingDisabled = 0; - -// Default probing bitrate limit. Applied only when the application didn't -// specify max bitrate. -constexpr int64_t kDefaultMaxProbingBitrateBps = 5000000; - -// Interval between probes when ALR periodic probing is enabled. -constexpr int64_t kAlrPeriodicProbingIntervalMs = 5000; - -// Minimum probe bitrate percentage to probe further for repeated probes, -// relative to the previous probe. For example, if 1Mbps probe results in -// 80kbps, then we'll probe again at 1.6Mbps. In that case second probe won't be -// sent if we get 600kbps from the first one. -constexpr int kRepeatedProbeMinPercentage = 70; - -// If the bitrate drops to a factor |kBitrateDropThreshold| or lower -// and we recover within |kBitrateDropTimeoutMs|, then we'll send -// a probe at a fraction |kProbeFractionAfterDrop| of the original bitrate. -constexpr double kBitrateDropThreshold = 0.66; -constexpr int kBitrateDropTimeoutMs = 5000; -constexpr double kProbeFractionAfterDrop = 0.85; - -// Timeout for probing after leaving ALR. If the bitrate drops significantly, -// (as determined by the delay based estimator) and we leave ALR, then we will -// send a probe if we recover within |kLeftAlrTimeoutMs| ms. -constexpr int kAlrEndedTimeoutMs = 3000; - -// The expected uncertainty of probe result (as a fraction of the target probe -// This is a limit on how often probing can be done when there is a BW -// drop detected in ALR. -constexpr int64_t kMinTimeBetweenAlrProbesMs = 5000; - -// bitrate). Used to avoid probing if the probe bitrate is close to our current -// estimate. -constexpr double kProbeUncertainty = 0.05; - -// Use probing to recover faster after large bitrate estimate drops. -constexpr char kBweRapidRecoveryExperiment[] = - "WebRTC-BweRapidRecoveryExperiment"; - -} // namespace - -ProbeController::ProbeController(PacedSender* pacer, const Clock* clock) - : pacer_(pacer), clock_(clock), enable_periodic_alr_probing_(false) { - Reset(); - in_rapid_recovery_experiment_ = webrtc::field_trial::FindFullName( - kBweRapidRecoveryExperiment) == "Enabled"; -} - -void ProbeController::SetBitrates(int64_t min_bitrate_bps, - int64_t start_bitrate_bps, - int64_t max_bitrate_bps) { - rtc::CritScope cs(&critsect_); - - if (start_bitrate_bps > 0) { - start_bitrate_bps_ = start_bitrate_bps; - estimated_bitrate_bps_ = start_bitrate_bps; - } else if (start_bitrate_bps_ == 0) { - start_bitrate_bps_ = min_bitrate_bps; - } - - // The reason we use the variable |old_max_bitrate_pbs| is because we - // need to set |max_bitrate_bps_| before we call InitiateProbing. - int64_t old_max_bitrate_bps = max_bitrate_bps_; - max_bitrate_bps_ = max_bitrate_bps; - - switch (state_) { - case State::kInit: - if (network_state_ == kNetworkUp) - InitiateExponentialProbing(); - break; - - case State::kWaitingForProbingResult: - break; - - case State::kProbingComplete: - // If the new max bitrate is higher than the old max bitrate and the - // estimate is lower than the new max bitrate then initiate probing. - if (estimated_bitrate_bps_ != 0 && - old_max_bitrate_bps < max_bitrate_bps_ && - estimated_bitrate_bps_ < max_bitrate_bps_) { - // The assumption is that if we jump more than 20% in the bandwidth - // estimate or if the bandwidth estimate is within 90% of the new - // max bitrate then the probing attempt was successful. - mid_call_probing_succcess_threshold_ = - std::min(estimated_bitrate_bps_ * 1.2, max_bitrate_bps_ * 0.9); - mid_call_probing_waiting_for_result_ = true; - mid_call_probing_bitrate_bps_ = max_bitrate_bps_; - - RTC_HISTOGRAM_COUNTS_10000("WebRTC.BWE.MidCallProbing.Initiated", - max_bitrate_bps_ / 1000); - - InitiateProbing(clock_->TimeInMilliseconds(), {max_bitrate_bps}, false); - } - break; - } -} - -void ProbeController::OnMaxTotalAllocatedBitrate( - int64_t max_total_allocated_bitrate) { - rtc::CritScope cs(&critsect_); - // TODO(philipel): Should |max_total_allocated_bitrate| be used as a limit for - // ALR probing? - 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) { - max_total_allocated_bitrate_ = max_total_allocated_bitrate; - InitiateProbing(clock_->TimeInMilliseconds(), {max_total_allocated_bitrate}, - false); - } -} - -void ProbeController::OnNetworkStateChanged(NetworkState network_state) { - rtc::CritScope cs(&critsect_); - network_state_ = network_state; - - if (network_state_ == kNetworkDown && - state_ == State::kWaitingForProbingResult) { - state_ = State::kProbingComplete; - min_bitrate_to_probe_further_bps_ = kExponentialProbingDisabled; - } - - if (network_state_ == kNetworkUp && state_ == State::kInit) - InitiateExponentialProbing(); -} - -void ProbeController::InitiateExponentialProbing() { - RTC_DCHECK(network_state_ == kNetworkUp); - RTC_DCHECK(state_ == State::kInit); - RTC_DCHECK_GT(start_bitrate_bps_, 0); - - // When probing at 1.8 Mbps ( 6x 300), this represents a threshold of - // 1.2 Mbps to continue probing. - InitiateProbing(clock_->TimeInMilliseconds(), - {3 * start_bitrate_bps_, 6 * start_bitrate_bps_}, true); -} - -void ProbeController::SetEstimatedBitrate(int64_t bitrate_bps) { - rtc::CritScope cs(&critsect_); - int64_t now_ms = clock_->TimeInMilliseconds(); - - if (mid_call_probing_waiting_for_result_ && - bitrate_bps >= mid_call_probing_succcess_threshold_) { - RTC_HISTOGRAM_COUNTS_10000("WebRTC.BWE.MidCallProbing.Success", - mid_call_probing_bitrate_bps_ / 1000); - RTC_HISTOGRAM_COUNTS_10000("WebRTC.BWE.MidCallProbing.ProbedKbps", - bitrate_bps / 1000); - mid_call_probing_waiting_for_result_ = false; - } - - if (state_ == State::kWaitingForProbingResult) { - // Continue probing if probing results indicate channel has greater - // capacity. - RTC_LOG(LS_INFO) << "Measured bitrate: " << bitrate_bps - << " Minimum to probe further: " - << min_bitrate_to_probe_further_bps_; - - if (min_bitrate_to_probe_further_bps_ != kExponentialProbingDisabled && - bitrate_bps > min_bitrate_to_probe_further_bps_) { - // Double the probing bitrate. - InitiateProbing(now_ms, {2 * bitrate_bps}, true); - } - } - - if (bitrate_bps < kBitrateDropThreshold * estimated_bitrate_bps_) { - time_of_last_large_drop_ms_ = now_ms; - bitrate_before_last_large_drop_bps_ = estimated_bitrate_bps_; - } - - estimated_bitrate_bps_ = bitrate_bps; -} - -void ProbeController::EnablePeriodicAlrProbing(bool enable) { - rtc::CritScope cs(&critsect_); - enable_periodic_alr_probing_ = enable; -} - -void ProbeController::SetAlrEndedTimeMs(int64_t alr_end_time_ms) { - rtc::CritScope cs(&critsect_); - alr_end_time_ms_.emplace(alr_end_time_ms); -} - -void ProbeController::RequestProbe() { - int64_t now_ms = clock_->TimeInMilliseconds(); - rtc::CritScope cs(&critsect_); - // Called once we have returned to normal state after a large drop in - // estimated bandwidth. The current response is to initiate a single probe - // session (if not already probing) at the previous bitrate. - // - // If the probe session fails, the assumption is that this drop was a - // real one from a competing flow or a network change. - bool in_alr = pacer_->GetApplicationLimitedRegionStartTime().has_value(); - bool alr_ended_recently = - (alr_end_time_ms_.has_value() && - now_ms - alr_end_time_ms_.value() < kAlrEndedTimeoutMs); - if (in_alr || alr_ended_recently || in_rapid_recovery_experiment_) { - if (state_ == State::kProbingComplete) { - uint32_t suggested_probe_bps = - kProbeFractionAfterDrop * bitrate_before_last_large_drop_bps_; - uint32_t min_expected_probe_result_bps = - (1 - kProbeUncertainty) * suggested_probe_bps; - int64_t time_since_drop_ms = now_ms - time_of_last_large_drop_ms_; - int64_t time_since_probe_ms = now_ms - last_bwe_drop_probing_time_ms_; - if (min_expected_probe_result_bps > estimated_bitrate_bps_ && - time_since_drop_ms < kBitrateDropTimeoutMs && - time_since_probe_ms > kMinTimeBetweenAlrProbesMs) { - RTC_LOG(LS_INFO) << "Detected big bandwidth drop, start probing."; - // Track how often we probe in response to bandwidth drop in ALR. - RTC_HISTOGRAM_COUNTS_10000( - "WebRTC.BWE.BweDropProbingIntervalInS", - (now_ms - last_bwe_drop_probing_time_ms_) / 1000); - InitiateProbing(now_ms, {suggested_probe_bps}, false); - last_bwe_drop_probing_time_ms_ = now_ms; - } - } - } -} - -void ProbeController::Reset() { - rtc::CritScope cs(&critsect_); - network_state_ = kNetworkUp; - state_ = State::kInit; - min_bitrate_to_probe_further_bps_ = kExponentialProbingDisabled; - time_last_probing_initiated_ms_ = 0; - estimated_bitrate_bps_ = 0; - start_bitrate_bps_ = 0; - max_bitrate_bps_ = 0; - int64_t now_ms = clock_->TimeInMilliseconds(); - last_bwe_drop_probing_time_ms_ = now_ms; - alr_end_time_ms_.reset(); - mid_call_probing_waiting_for_result_ = false; - time_of_last_large_drop_ms_ = now_ms; - bitrate_before_last_large_drop_bps_ = 0; - max_total_allocated_bitrate_ = 0; -} - -void ProbeController::Process() { - rtc::CritScope cs(&critsect_); - - int64_t now_ms = clock_->TimeInMilliseconds(); - - if (now_ms - time_last_probing_initiated_ms_ > - kMaxWaitingTimeForProbingResultMs) { - mid_call_probing_waiting_for_result_ = false; - - if (state_ == State::kWaitingForProbingResult) { - RTC_LOG(LS_INFO) << "kWaitingForProbingResult: timeout"; - state_ = State::kProbingComplete; - min_bitrate_to_probe_further_bps_ = kExponentialProbingDisabled; - } - } - - if (state_ != State::kProbingComplete || !enable_periodic_alr_probing_) - return; - - // Probe bandwidth periodically when in ALR state. - absl::optional alr_start_time = - pacer_->GetApplicationLimitedRegionStartTime(); - if (alr_start_time && estimated_bitrate_bps_ > 0) { - int64_t next_probe_time_ms = - std::max(*alr_start_time, time_last_probing_initiated_ms_) + - kAlrPeriodicProbingIntervalMs; - if (now_ms >= next_probe_time_ms) { - InitiateProbing(now_ms, {estimated_bitrate_bps_ * 2}, true); - } - } -} - -void ProbeController::InitiateProbing( - int64_t now_ms, - std::initializer_list bitrates_to_probe, - bool probe_further) { - for (int64_t bitrate : bitrates_to_probe) { - RTC_DCHECK_GT(bitrate, 0); - int64_t max_probe_bitrate_bps = - max_bitrate_bps_ > 0 ? max_bitrate_bps_ : kDefaultMaxProbingBitrateBps; - if (bitrate > max_probe_bitrate_bps) { - bitrate = max_probe_bitrate_bps; - probe_further = false; - } - pacer_->CreateProbeCluster(rtc::dchecked_cast(bitrate)); - } - time_last_probing_initiated_ms_ = now_ms; - if (probe_further) { - state_ = State::kWaitingForProbingResult; - min_bitrate_to_probe_further_bps_ = - (*(bitrates_to_probe.end() - 1)) * kRepeatedProbeMinPercentage / 100; - } else { - state_ = State::kProbingComplete; - min_bitrate_to_probe_further_bps_ = kExponentialProbingDisabled; - } -} - -} // namespace webrtc diff --git a/modules/congestion_controller/probe_controller.h b/modules/congestion_controller/probe_controller.h deleted file mode 100644 index 5bc16bc0a0..0000000000 --- a/modules/congestion_controller/probe_controller.h +++ /dev/null @@ -1,99 +0,0 @@ -/* - * Copyright (c) 2016 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ - -#ifndef MODULES_CONGESTION_CONTROLLER_PROBE_CONTROLLER_H_ -#define MODULES_CONGESTION_CONTROLLER_PROBE_CONTROLLER_H_ - -#include - -#include "common_types.h" // NOLINT(build/include) -#include "modules/pacing/paced_sender.h" -#include "rtc_base/criticalsection.h" - -namespace webrtc { - -class Clock; - -// This class controls initiation of probing to estimate initial channel -// capacity. There is also support for probing during a session when max -// bitrate is adjusted by an application. -class ProbeController { - public: - ProbeController(PacedSender* pacer, const Clock* clock); - - void SetBitrates(int64_t min_bitrate_bps, - int64_t start_bitrate_bps, - int64_t max_bitrate_bps); - - // The total bitrate, as opposed to the max bitrate, is the sum of the - // configured bitrates for all active streams. - void OnMaxTotalAllocatedBitrate(int64_t max_total_allocated_bitrate); - - void OnNetworkStateChanged(NetworkState state); - - void SetEstimatedBitrate(int64_t bitrate_bps); - - void EnablePeriodicAlrProbing(bool enable); - - void SetAlrEndedTimeMs(int64_t alr_end_time); - - void RequestProbe(); - - // Resets the ProbeController to a state equivalent to as if it was just - // created EXCEPT for |enable_periodic_alr_probing_|. - void Reset(); - - void Process(); - - private: - enum class State { - // Initial state where no probing has been triggered yet. - kInit, - // Waiting for probing results to continue further probing. - kWaitingForProbingResult, - // Probing is complete. - kProbingComplete, - }; - - void InitiateExponentialProbing() RTC_EXCLUSIVE_LOCKS_REQUIRED(critsect_); - void InitiateProbing(int64_t now_ms, - std::initializer_list bitrates_to_probe, - bool probe_further) - RTC_EXCLUSIVE_LOCKS_REQUIRED(critsect_); - - rtc::CriticalSection critsect_; - PacedSender* const pacer_; - const Clock* const clock_; - NetworkState network_state_ RTC_GUARDED_BY(critsect_); - State state_ RTC_GUARDED_BY(critsect_); - int64_t min_bitrate_to_probe_further_bps_ RTC_GUARDED_BY(critsect_); - int64_t time_last_probing_initiated_ms_ RTC_GUARDED_BY(critsect_); - int64_t estimated_bitrate_bps_ RTC_GUARDED_BY(critsect_); - int64_t start_bitrate_bps_ RTC_GUARDED_BY(critsect_); - int64_t max_bitrate_bps_ RTC_GUARDED_BY(critsect_); - int64_t last_bwe_drop_probing_time_ms_ RTC_GUARDED_BY(critsect_); - absl::optional alr_end_time_ms_ RTC_GUARDED_BY(critsect_); - bool enable_periodic_alr_probing_ RTC_GUARDED_BY(critsect_); - int64_t time_of_last_large_drop_ms_ RTC_GUARDED_BY(critsect_); - int64_t bitrate_before_last_large_drop_bps_ RTC_GUARDED_BY(critsect_); - int64_t max_total_allocated_bitrate_ RTC_GUARDED_BY(critsect_); - - bool in_rapid_recovery_experiment_ RTC_GUARDED_BY(critsect_); - // For WebRTC.BWE.MidCallProbing.* metric. - bool mid_call_probing_waiting_for_result_ RTC_GUARDED_BY(&critsect_); - int64_t mid_call_probing_bitrate_bps_ RTC_GUARDED_BY(&critsect_); - int64_t mid_call_probing_succcess_threshold_ RTC_GUARDED_BY(&critsect_); - - RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(ProbeController); -}; - -} // namespace webrtc - -#endif // MODULES_CONGESTION_CONTROLLER_PROBE_CONTROLLER_H_ diff --git a/modules/congestion_controller/probe_controller_unittest.cc b/modules/congestion_controller/probe_controller_unittest.cc deleted file mode 100644 index 14f53b73b8..0000000000 --- a/modules/congestion_controller/probe_controller_unittest.cc +++ /dev/null @@ -1,289 +0,0 @@ -/* - * Copyright (c) 2016 The WebRTC project authors. All Rights Reserved. - * - * Use of this source code is governed by a BSD-style license - * that can be found in the LICENSE file in the root of the source - * tree. An additional intellectual property rights grant can be found - * in the file PATENTS. All contributing project authors may - * be found in the AUTHORS file in the root of the source tree. - */ -#include - -#include "modules/congestion_controller/probe_controller.h" -#include "modules/pacing/mock/mock_paced_sender.h" -#include "rtc_base/logging.h" -#include "system_wrappers/include/clock.h" -#include "test/gmock.h" -#include "test/gtest.h" - -using testing::_; -using testing::AtLeast; -using testing::NiceMock; -using testing::Return; - -namespace webrtc { -namespace test { - -namespace { - -constexpr int kMinBitrateBps = 100; -constexpr int kStartBitrateBps = 300; -constexpr int kMaxBitrateBps = 10000; - -constexpr int kExponentialProbingTimeoutMs = 5000; - -constexpr int kAlrProbeInterval = 5000; -constexpr int kAlrEndedTimeoutMs = 3000; -constexpr int kBitrateDropTimeoutMs = 5000; - -} // namespace - -class LegacyProbeControllerTest : public ::testing::Test { - protected: - LegacyProbeControllerTest() : clock_(100000000L) { - probe_controller_.reset(new ProbeController(&pacer_, &clock_)); - } - ~LegacyProbeControllerTest() override {} - - const int64_t kMbpsMultiplier = 1000000; - SimulatedClock clock_; - NiceMock pacer_; - std::unique_ptr probe_controller_; -}; - -TEST_F(LegacyProbeControllerTest, InitiatesProbingAtStart) { - EXPECT_CALL(pacer_, CreateProbeCluster(_)).Times(AtLeast(2)); - probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, - kMaxBitrateBps); -} - -TEST_F(LegacyProbeControllerTest, ProbeOnlyWhenNetworkIsUp) { - probe_controller_->OnNetworkStateChanged(kNetworkDown); - EXPECT_CALL(pacer_, CreateProbeCluster(_)).Times(0); - probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, - kMaxBitrateBps); - - testing::Mock::VerifyAndClearExpectations(&pacer_); - EXPECT_CALL(pacer_, CreateProbeCluster(_)).Times(AtLeast(2)); - probe_controller_->OnNetworkStateChanged(kNetworkUp); -} - -TEST_F(LegacyProbeControllerTest, InitiatesProbingOnMaxBitrateIncrease) { - EXPECT_CALL(pacer_, CreateProbeCluster(_)).Times(AtLeast(2)); - probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, - kMaxBitrateBps); - // Long enough to time out exponential probing. - clock_.AdvanceTimeMilliseconds(kExponentialProbingTimeoutMs); - probe_controller_->SetEstimatedBitrate(kStartBitrateBps); - probe_controller_->Process(); - - EXPECT_CALL(pacer_, CreateProbeCluster(kMaxBitrateBps + 100)); - probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, - kMaxBitrateBps + 100); -} - -TEST_F(LegacyProbeControllerTest, - InitiatesProbingOnMaxBitrateIncreaseAtMaxBitrate) { - EXPECT_CALL(pacer_, CreateProbeCluster(_)).Times(AtLeast(2)); - probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, - kMaxBitrateBps); - // Long enough to time out exponential probing. - clock_.AdvanceTimeMilliseconds(kExponentialProbingTimeoutMs); - probe_controller_->SetEstimatedBitrate(kStartBitrateBps); - probe_controller_->Process(); - - probe_controller_->SetEstimatedBitrate(kMaxBitrateBps); - EXPECT_CALL(pacer_, CreateProbeCluster(kMaxBitrateBps + 100)); - probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, - kMaxBitrateBps + 100); -} - -TEST_F(LegacyProbeControllerTest, TestExponentialProbing) { - probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, - kMaxBitrateBps); - - // Repeated probe should only be sent when estimated bitrate climbs above - // 0.7 * 6 * kStartBitrateBps = 1260. - EXPECT_CALL(pacer_, CreateProbeCluster(_)).Times(0); - probe_controller_->SetEstimatedBitrate(1000); - testing::Mock::VerifyAndClearExpectations(&pacer_); - - EXPECT_CALL(pacer_, CreateProbeCluster(2 * 1800)); - probe_controller_->SetEstimatedBitrate(1800); -} - -TEST_F(LegacyProbeControllerTest, TestExponentialProbingTimeout) { - probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, - kMaxBitrateBps); - - // Advance far enough to cause a time out in waiting for probing result. - clock_.AdvanceTimeMilliseconds(kExponentialProbingTimeoutMs); - probe_controller_->Process(); - - EXPECT_CALL(pacer_, CreateProbeCluster(_)).Times(0); - probe_controller_->SetEstimatedBitrate(1800); -} - -TEST_F(LegacyProbeControllerTest, RequestProbeInAlr) { - EXPECT_CALL(pacer_, CreateProbeCluster(_)).Times(2); - probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, - kMaxBitrateBps); - probe_controller_->SetEstimatedBitrate(500); - testing::Mock::VerifyAndClearExpectations(&pacer_); - EXPECT_CALL(pacer_, CreateProbeCluster(0.85 * 500)).Times(1); - EXPECT_CALL(pacer_, GetApplicationLimitedRegionStartTime()) - .WillRepeatedly(Return(clock_.TimeInMilliseconds())); - clock_.AdvanceTimeMilliseconds(kAlrProbeInterval + 1); - probe_controller_->Process(); - probe_controller_->SetEstimatedBitrate(250); - probe_controller_->RequestProbe(); -} - -TEST_F(LegacyProbeControllerTest, RequestProbeWhenAlrEndedRecently) { - EXPECT_CALL(pacer_, CreateProbeCluster(_)).Times(2); - probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, - kMaxBitrateBps); - probe_controller_->SetEstimatedBitrate(500); - testing::Mock::VerifyAndClearExpectations(&pacer_); - EXPECT_CALL(pacer_, CreateProbeCluster(0.85 * 500)).Times(1); - EXPECT_CALL(pacer_, GetApplicationLimitedRegionStartTime()) - .WillRepeatedly(Return(absl::nullopt)); - clock_.AdvanceTimeMilliseconds(kAlrProbeInterval + 1); - probe_controller_->Process(); - probe_controller_->SetEstimatedBitrate(250); - probe_controller_->SetAlrEndedTimeMs(clock_.TimeInMilliseconds()); - clock_.AdvanceTimeMilliseconds(kAlrEndedTimeoutMs - 1); - probe_controller_->RequestProbe(); -} - -TEST_F(LegacyProbeControllerTest, RequestProbeWhenAlrNotEndedRecently) { - EXPECT_CALL(pacer_, CreateProbeCluster(_)).Times(2); - probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, - kMaxBitrateBps); - probe_controller_->SetEstimatedBitrate(500); - testing::Mock::VerifyAndClearExpectations(&pacer_); - EXPECT_CALL(pacer_, CreateProbeCluster(_)).Times(0); - EXPECT_CALL(pacer_, GetApplicationLimitedRegionStartTime()) - .WillRepeatedly(Return(absl::nullopt)); - clock_.AdvanceTimeMilliseconds(kAlrProbeInterval + 1); - probe_controller_->Process(); - probe_controller_->SetEstimatedBitrate(250); - probe_controller_->SetAlrEndedTimeMs(clock_.TimeInMilliseconds()); - clock_.AdvanceTimeMilliseconds(kAlrEndedTimeoutMs + 1); - probe_controller_->RequestProbe(); -} - -TEST_F(LegacyProbeControllerTest, RequestProbeWhenBweDropNotRecent) { - EXPECT_CALL(pacer_, CreateProbeCluster(_)).Times(2); - probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, - kMaxBitrateBps); - probe_controller_->SetEstimatedBitrate(500); - testing::Mock::VerifyAndClearExpectations(&pacer_); - EXPECT_CALL(pacer_, CreateProbeCluster(_)).Times(0); - EXPECT_CALL(pacer_, GetApplicationLimitedRegionStartTime()) - .WillRepeatedly(Return(clock_.TimeInMilliseconds())); - clock_.AdvanceTimeMilliseconds(kAlrProbeInterval + 1); - probe_controller_->Process(); - probe_controller_->SetEstimatedBitrate(250); - clock_.AdvanceTimeMilliseconds(kBitrateDropTimeoutMs + 1); - probe_controller_->RequestProbe(); -} - -TEST_F(LegacyProbeControllerTest, PeriodicProbing) { - EXPECT_CALL(pacer_, CreateProbeCluster(_)).Times(2); - probe_controller_->EnablePeriodicAlrProbing(true); - probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, - kMaxBitrateBps); - probe_controller_->SetEstimatedBitrate(500); - testing::Mock::VerifyAndClearExpectations(&pacer_); - - int64_t start_time = clock_.TimeInMilliseconds(); - - // Expect the controller to send a new probe after 5s has passed. - EXPECT_CALL(pacer_, CreateProbeCluster(1000)).Times(1); - EXPECT_CALL(pacer_, GetApplicationLimitedRegionStartTime()) - .WillRepeatedly(Return(start_time)); - clock_.AdvanceTimeMilliseconds(5000); - probe_controller_->Process(); - probe_controller_->SetEstimatedBitrate(500); - testing::Mock::VerifyAndClearExpectations(&pacer_); - - // The following probe should be sent at 10s into ALR. - EXPECT_CALL(pacer_, CreateProbeCluster(_)).Times(0); - EXPECT_CALL(pacer_, GetApplicationLimitedRegionStartTime()) - .WillRepeatedly(Return(start_time)); - clock_.AdvanceTimeMilliseconds(4000); - probe_controller_->Process(); - probe_controller_->SetEstimatedBitrate(500); - testing::Mock::VerifyAndClearExpectations(&pacer_); - - EXPECT_CALL(pacer_, CreateProbeCluster(_)).Times(1); - EXPECT_CALL(pacer_, GetApplicationLimitedRegionStartTime()) - .WillRepeatedly(Return(start_time)); - clock_.AdvanceTimeMilliseconds(1000); - probe_controller_->Process(); - probe_controller_->SetEstimatedBitrate(500); - testing::Mock::VerifyAndClearExpectations(&pacer_); -} - -TEST_F(LegacyProbeControllerTest, PeriodicProbingAfterReset) { - testing::StrictMock local_pacer; - probe_controller_.reset(new ProbeController(&local_pacer, &clock_)); - int64_t alr_start_time = clock_.TimeInMilliseconds(); - EXPECT_CALL(local_pacer, GetApplicationLimitedRegionStartTime()) - .WillRepeatedly(Return(alr_start_time)); - - EXPECT_CALL(local_pacer, CreateProbeCluster(_)).Times(2); - probe_controller_->EnablePeriodicAlrProbing(true); - probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, - kMaxBitrateBps); - probe_controller_->Reset(); - - clock_.AdvanceTimeMilliseconds(10000); - probe_controller_->Process(); - - EXPECT_CALL(local_pacer, CreateProbeCluster(_)).Times(2); - probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, - kMaxBitrateBps); - - // Make sure we use |kStartBitrateBps| as the estimated bitrate - // until SetEstimatedBitrate is called with an updated estimate. - clock_.AdvanceTimeMilliseconds(10000); - EXPECT_CALL(local_pacer, CreateProbeCluster(kStartBitrateBps * 2)); - probe_controller_->Process(); -} - -TEST_F(LegacyProbeControllerTest, TestExponentialProbingOverflow) { - probe_controller_->SetBitrates(kMinBitrateBps, 10 * kMbpsMultiplier, - 100 * kMbpsMultiplier); - - // Verify that probe bitrate is capped at the specified max bitrate - EXPECT_CALL(pacer_, CreateProbeCluster(100 * kMbpsMultiplier)); - probe_controller_->SetEstimatedBitrate(60 * kMbpsMultiplier); - testing::Mock::VerifyAndClearExpectations(&pacer_); - - // Verify that repeated probes aren't sent. - EXPECT_CALL(pacer_, CreateProbeCluster(_)).Times(0); - probe_controller_->SetEstimatedBitrate(100 * kMbpsMultiplier); -} - -TEST_F(LegacyProbeControllerTest, TotalBitrateProbing) { - probe_controller_->SetBitrates(kMinBitrateBps, 1 * kMbpsMultiplier, - 2 * kMbpsMultiplier); - - EXPECT_CALL(pacer_, CreateProbeCluster(1 * kMbpsMultiplier)); - probe_controller_->SetEstimatedBitrate(500000); - probe_controller_->OnMaxTotalAllocatedBitrate(1 * kMbpsMultiplier); -} - -TEST_F(LegacyProbeControllerTest, TotalBitrateNoProbing) { - probe_controller_->SetBitrates(kMinBitrateBps, 1 * kMbpsMultiplier, - 2 * kMbpsMultiplier); - - EXPECT_CALL(pacer_, CreateProbeCluster(_)).Times(0); - probe_controller_->SetEstimatedBitrate(500000); - probe_controller_->OnMaxTotalAllocatedBitrate(250000); -} - -} // namespace test -} // namespace webrtc diff --git a/modules/congestion_controller/send_side_congestion_controller.cc b/modules/congestion_controller/send_side_congestion_controller.cc index 8ede6ed128..ddf7043efd 100644 --- a/modules/congestion_controller/send_side_congestion_controller.cc +++ b/modules/congestion_controller/send_side_congestion_controller.cc @@ -19,7 +19,7 @@ #include "absl/memory/memory.h" #include "modules/bitrate_controller/include/bitrate_controller.h" #include "modules/congestion_controller/goog_cc/acknowledged_bitrate_estimator.h" -#include "modules/congestion_controller/probe_controller.h" +#include "modules/congestion_controller/goog_cc/probe_controller.h" #include "modules/remote_bitrate_estimator/include/bwe_defines.h" #include "rtc_base/checks.h" #include "rtc_base/format_macros.h" @@ -117,7 +117,7 @@ SendSideCongestionController::SendSideCongestionController( BitrateController::CreateBitrateController(clock_, event_log)), acknowledged_bitrate_estimator_( absl::make_unique()), - probe_controller_(new ProbeController(pacer_, clock_)), + probe_controller_(new ProbeController()), retransmission_rate_limiter_( new RateLimiter(clock, kRetransmitWindowSizeMs)), transport_feedback_adapter_(clock_), @@ -177,8 +177,13 @@ void SendSideCongestionController::SetBweBitrates(int min_bitrate_bps, bitrate_controller_->SetBitrates(start_bitrate_bps, min_bitrate_bps, max_bitrate_bps); - probe_controller_->SetBitrates(min_bitrate_bps, start_bitrate_bps, - max_bitrate_bps); + { + rtc::CritScope cs(&probe_lock_); + probe_controller_->SetBitrates(min_bitrate_bps, start_bitrate_bps, + max_bitrate_bps, + clock_->TimeInMilliseconds()); + SendPendingProbes(); + } { rtc::CritScope cs(&bwe_lock_); @@ -195,7 +200,11 @@ void SendSideCongestionController::SetAllocatedSendBitrateLimits( int64_t max_padding_bitrate_bps, int64_t max_total_bitrate_bps) { pacer_->SetSendBitrateLimits(min_send_bitrate_bps, max_padding_bitrate_bps); - probe_controller_->OnMaxTotalAllocatedBitrate(max_total_bitrate_bps); + + rtc::CritScope cs(&probe_lock_); + probe_controller_->OnMaxTotalAllocatedBitrate(max_total_bitrate_bps, + clock_->TimeInMilliseconds()); + SendPendingProbes(); } // TODO(holmer): Split this up and use SetBweBitrates in combination with @@ -222,9 +231,14 @@ void SendSideCongestionController::OnNetworkRouteChanged( delay_based_bwe_->SetStartBitrate(bitrate_bps); delay_based_bwe_->SetMinBitrate(min_bitrate_bps); } - - probe_controller_->Reset(); - probe_controller_->SetBitrates(min_bitrate_bps, bitrate_bps, max_bitrate_bps); + { + rtc::CritScope cs(&probe_lock_); + probe_controller_->Reset(clock_->TimeInMilliseconds()); + probe_controller_->SetBitrates(min_bitrate_bps, bitrate_bps, + max_bitrate_bps, + clock_->TimeInMilliseconds()); + SendPendingProbes(); + } MaybeTriggerOnNetworkChanged(); } @@ -255,6 +269,7 @@ void SendSideCongestionController::SetPerPacketFeedbackAvailable( bool available) {} void SendSideCongestionController::EnablePeriodicAlrProbing(bool enable) { + rtc::CritScope cs(&probe_lock_); probe_controller_->EnablePeriodicAlrProbing(enable); } @@ -279,7 +294,15 @@ void SendSideCongestionController::SignalNetworkState(NetworkState state) { pause_pacer_ = state == kNetworkDown; network_state_ = state; } - probe_controller_->OnNetworkStateChanged(state); + + { + rtc::CritScope cs(&probe_lock_); + NetworkAvailability msg; + msg.at_time = Timestamp::ms(clock_->TimeInMilliseconds()); + msg.network_available = state == kNetworkUp; + probe_controller_->OnNetworkAvailability(msg); + SendPendingProbes(); + } MaybeTriggerOnNetworkChanged(); } @@ -311,6 +334,12 @@ int64_t SendSideCongestionController::TimeUntilNextProcess() { return bitrate_controller_->TimeUntilNextProcess(); } +void SendSideCongestionController::SendPendingProbes() { + for (auto probe_config : probe_controller_->GetAndResetPendingProbes()) { + pacer_->CreateProbeCluster(probe_config.target_data_rate.bps()); + } +} + void SendSideCongestionController::Process() { bool pause_pacer; // TODO(holmer): Once this class is running on a task queue we should @@ -327,7 +356,14 @@ void SendSideCongestionController::Process() { pacer_paused_ = false; } bitrate_controller_->Process(); - probe_controller_->Process(); + + { + rtc::CritScope cs(&probe_lock_); + probe_controller_->SetAlrStartTimeMs( + pacer_->GetApplicationLimitedRegionStartTime()); + probe_controller_->Process(clock_->TimeInMilliseconds()); + SendPendingProbes(); + } MaybeTriggerOnNetworkChanged(); } @@ -357,6 +393,7 @@ void SendSideCongestionController::OnTransportFeedback( if (was_in_alr_ && !currently_in_alr) { int64_t now_ms = rtc::TimeMillis(); acknowledged_bitrate_estimator_->SetAlrEndedTimeMs(now_ms); + rtc::CritScope cs(&probe_lock_); probe_controller_->SetAlrEndedTimeMs(now_ms); } was_in_alr_ = currently_in_alr; @@ -375,8 +412,12 @@ void SendSideCongestionController::OnTransportFeedback( // Update the estimate in the ProbeController, in case we want to probe. MaybeTriggerOnNetworkChanged(); } - if (result.recovered_from_overuse) - probe_controller_->RequestProbe(); + if (result.recovered_from_overuse) { + rtc::CritScope cs(&probe_lock_); + probe_controller_->SetAlrStartTimeMs( + pacer_->GetApplicationLimitedRegionStartTime()); + probe_controller_->RequestProbe(clock_->TimeInMilliseconds()); + } if (in_cwnd_experiment_) LimitOutstandingBytes(transport_feedback_adapter_.GetOutstandingBytes()); } @@ -429,7 +470,11 @@ void SendSideCongestionController::MaybeTriggerOnNetworkChanged() { &bitrate_bps, &fraction_loss, &rtt); if (estimate_changed) { pacer_->SetEstimatedBitrate(bitrate_bps); - probe_controller_->SetEstimatedBitrate(bitrate_bps); + { + rtc::CritScope cs(&probe_lock_); + probe_controller_->SetEstimatedBitrate(bitrate_bps, + clock_->TimeInMilliseconds()); + } retransmission_rate_limiter_->SetMaxRate(bitrate_bps); }