diff --git a/webrtc/modules/congestion_controller/delay_based_bwe.cc b/webrtc/modules/congestion_controller/delay_based_bwe.cc index c50f0d4b45..2ef4463e91 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe.cc +++ b/webrtc/modules/congestion_controller/delay_based_bwe.cc @@ -57,6 +57,20 @@ bool BweSparseUpdateExperimentIsEnabled() { namespace webrtc { +DelayBasedBwe::Result::Result() + : updated(false), + probe(false), + target_bitrate_bps(0), + recovered_from_overuse(false) {} + +DelayBasedBwe::Result::Result(bool probe, uint32_t target_bitrate_bps) + : updated(true), + probe(probe), + target_bitrate_bps(target_bitrate_bps), + recovered_from_overuse(false) {} + +DelayBasedBwe::Result::~Result() {} + DelayBasedBwe::DelayBasedBwe(RtcEventLog* event_log, const Clock* clock) : event_log_(event_log), clock_(clock), @@ -102,6 +116,8 @@ DelayBasedBwe::Result DelayBasedBwe::IncomingPacketFeedbackVector( } bool overusing = false; bool delayed_feedback = true; + bool recovered_from_overuse = false; + BandwidthUsage prev_detector_state = detector_.State(); for (const auto& packet_feedback : packet_feedback_vector) { if (packet_feedback.send_time_ms < 0) continue; @@ -109,9 +125,15 @@ DelayBasedBwe::Result DelayBasedBwe::IncomingPacketFeedbackVector( IncomingPacketFeedback(packet_feedback); if (!in_sparse_update_experiment_) overusing |= (detector_.State() == BandwidthUsage::kBwOverusing); + if (prev_detector_state == BandwidthUsage::kBwUnderusing && + detector_.State() == BandwidthUsage::kBwNormal) { + recovered_from_overuse = true; + } + prev_detector_state = detector_.State(); } if (in_sparse_update_experiment_) overusing = (detector_.State() == BandwidthUsage::kBwOverusing); + if (delayed_feedback) { ++consecutive_delayed_feedbacks_; if (consecutive_delayed_feedbacks_ >= kMaxConsecutiveFailedLookups) { @@ -120,7 +142,8 @@ DelayBasedBwe::Result DelayBasedBwe::IncomingPacketFeedbackVector( } } else { consecutive_delayed_feedbacks_ = 0; - return MaybeUpdateEstimate(overusing, acked_bitrate_bps); + return MaybeUpdateEstimate(overusing, acked_bitrate_bps, + recovered_from_overuse); } return Result(); } @@ -189,7 +212,8 @@ void DelayBasedBwe::IncomingPacketFeedback( DelayBasedBwe::Result DelayBasedBwe::MaybeUpdateEstimate( bool overusing, - rtc::Optional acked_bitrate_bps) { + rtc::Optional acked_bitrate_bps, + bool recovered_from_overuse) { Result result; int64_t now_ms = clock_->TimeInMilliseconds(); @@ -223,6 +247,7 @@ DelayBasedBwe::Result DelayBasedBwe::MaybeUpdateEstimate( } else { result.updated = UpdateEstimate(now_ms, acked_bitrate_bps, overusing, &result.target_bitrate_bps); + result.recovered_from_overuse = recovered_from_overuse; } } if (result.updated) { diff --git a/webrtc/modules/congestion_controller/delay_based_bwe.h b/webrtc/modules/congestion_controller/delay_based_bwe.h index 429797af05..52c227f672 100644 --- a/webrtc/modules/congestion_controller/delay_based_bwe.h +++ b/webrtc/modules/congestion_controller/delay_based_bwe.h @@ -36,12 +36,13 @@ class DelayBasedBwe { static const int64_t kStreamTimeOutMs = 2000; struct Result { - Result() : updated(false), probe(false), target_bitrate_bps(0) {} - Result(bool probe, uint32_t target_bitrate_bps) - : updated(true), probe(probe), target_bitrate_bps(target_bitrate_bps) {} + Result(); + Result(bool probe, uint32_t target_bitrate_bps); + ~Result(); bool updated; bool probe; uint32_t target_bitrate_bps; + bool recovered_from_overuse; }; DelayBasedBwe(RtcEventLog* event_log, const Clock* clock); @@ -60,9 +61,9 @@ class DelayBasedBwe { private: void IncomingPacketFeedback(const PacketFeedback& packet_feedback); Result OnLongFeedbackDelay(int64_t arrival_time_ms); - Result MaybeUpdateEstimate(bool overusing, - rtc::Optional acked_bitrate_bps); + rtc::Optional acked_bitrate_bps, + bool request_probe); // Updates the current remote rate estimate and returns true if a valid // estimate exists. bool UpdateEstimate(int64_t now_ms, diff --git a/webrtc/modules/congestion_controller/probe_controller.cc b/webrtc/modules/congestion_controller/probe_controller.cc index 43889bce9a..24ce7d42c3 100644 --- a/webrtc/modules/congestion_controller/probe_controller.cc +++ b/webrtc/modules/congestion_controller/probe_controller.cc @@ -15,6 +15,7 @@ #include "webrtc/rtc_base/logging.h" #include "webrtc/rtc_base/safe_conversions.h" +#include "webrtc/system_wrappers/include/field_trial.h" #include "webrtc/system_wrappers/include/metrics.h" namespace webrtc { @@ -32,10 +33,6 @@ constexpr int kExponentialProbingDisabled = 0; // specify max bitrate. constexpr int64_t kDefaultMaxProbingBitrateBps = 5000000; -// This is a limit on how often probing can be done when there is a BW -// drop detected in ALR. -constexpr int64_t kAlrProbingIntervalMinMs = 5000; - // Interval between probes when ALR periodic probing is enabled. constexpr int64_t kAlrPeriodicProbingIntervalMs = 5000; @@ -45,11 +42,38 @@ constexpr int64_t kAlrPeriodicProbingIntervalMs = 5000; // 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, @@ -146,28 +170,9 @@ void ProbeController::SetEstimatedBitrate(int64_t bitrate_bps) { } } - // Detect a drop in estimated BW when operating in ALR and not already - // probing. The current response is to initiate a single probe session at the - // previous bitrate and immediately use the reported bitrate as the new - // bitrate. - // - // If the probe session fails, the assumption is that this drop was a - // real one from a competing flow or something else on the network and - // it ramps up from bitrate_bps. - if (state_ == State::kProbingComplete && - pacer_->GetApplicationLimitedRegionStartTime() && - bitrate_bps < 2 * estimated_bitrate_bps_ / 3 && - (now_ms - last_alr_probing_time_) > kAlrProbingIntervalMinMs) { - LOG(LS_INFO) << "Detected big BW drop in ALR, start probe."; - // Track how often we probe in response to BW drop in ALR. - RTC_HISTOGRAM_COUNTS_10000("WebRTC.BWE.AlrProbingIntervalInS", - (now_ms - last_alr_probing_time_) / 1000); - InitiateProbing(now_ms, {estimated_bitrate_bps_}, false); - last_alr_probing_time_ = now_ms; - - // TODO(isheriff): May want to track when we did ALR probing in order - // to reset |last_alr_probing_time_| if we validate that it was a - // drop due to exogenous event. + 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; @@ -178,6 +183,47 @@ void ProbeController::EnablePeriodicAlrProbing(bool enable) { 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) { + 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; @@ -187,8 +233,12 @@ void ProbeController::Reset() { estimated_bitrate_bps_ = 0; start_bitrate_bps_ = 0; max_bitrate_bps_ = 0; - last_alr_probing_time_ = clock_->TimeInMilliseconds(); + 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; } void ProbeController::Process() { diff --git a/webrtc/modules/congestion_controller/probe_controller.h b/webrtc/modules/congestion_controller/probe_controller.h index 4a3071cc59..f47bdb0ca2 100644 --- a/webrtc/modules/congestion_controller/probe_controller.h +++ b/webrtc/modules/congestion_controller/probe_controller.h @@ -38,6 +38,10 @@ class ProbeController { 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(); @@ -69,9 +73,13 @@ class ProbeController { int64_t estimated_bitrate_bps_ GUARDED_BY(critsect_); int64_t start_bitrate_bps_ GUARDED_BY(critsect_); int64_t max_bitrate_bps_ GUARDED_BY(critsect_); - int64_t last_alr_probing_time_ GUARDED_BY(critsect_); + int64_t last_bwe_drop_probing_time_ms_ GUARDED_BY(critsect_); + rtc::Optional alr_end_time_ms_ GUARDED_BY(critsect_); bool enable_periodic_alr_probing_ GUARDED_BY(critsect_); + int64_t time_of_last_large_drop_ms_ GUARDED_BY(critsect_); + int64_t bitrate_before_last_large_drop_bps_ GUARDED_BY(critsect_); + bool in_rapid_recovery_experiment_ GUARDED_BY(critsect_); // For WebRTC.BWE.MidCallProbing.* metric. bool mid_call_probing_waiting_for_result_ GUARDED_BY(&critsect_); int64_t mid_call_probing_bitrate_bps_ GUARDED_BY(&critsect_); diff --git a/webrtc/modules/congestion_controller/probe_controller_unittest.cc b/webrtc/modules/congestion_controller/probe_controller_unittest.cc index 9fc0f55088..e0a040f3a1 100644 --- a/webrtc/modules/congestion_controller/probe_controller_unittest.cc +++ b/webrtc/modules/congestion_controller/probe_controller_unittest.cc @@ -33,6 +33,8 @@ constexpr int kMaxBitrateBps = 10000; constexpr int kExponentialProbingTimeoutMs = 5000; constexpr int kAlrProbeInterval = 5000; +constexpr int kAlrEndedTimeoutMs = 3000; +constexpr int kBitrateDropTimeoutMs = 5000; } // namespace @@ -120,22 +122,71 @@ TEST_F(ProbeControllerTest, TestExponentialProbingTimeout) { probe_controller_->SetEstimatedBitrate(1800); } -TEST_F(ProbeControllerTest, ProbeAfterEstimateDropInAlr) { +TEST_F(ProbeControllerTest, RequestProbeInAlr) { EXPECT_CALL(pacer_, CreateProbeCluster(_)).Times(2); probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps, kMaxBitrateBps); probe_controller_->SetEstimatedBitrate(500); testing::Mock::VerifyAndClearExpectations(&pacer_); - - // When bandwidth estimate drops the controller should send a probe at the - // previous bitrate. - EXPECT_CALL(pacer_, CreateProbeCluster(500)).Times(1); + EXPECT_CALL(pacer_, CreateProbeCluster(0.85 * 500)).Times(1); EXPECT_CALL(pacer_, GetApplicationLimitedRegionStartTime()) .WillRepeatedly( Return(rtc::Optional(clock_.TimeInMilliseconds()))); clock_.AdvanceTimeMilliseconds(kAlrProbeInterval + 1); probe_controller_->Process(); - probe_controller_->SetEstimatedBitrate(50); + probe_controller_->SetEstimatedBitrate(250); + probe_controller_->RequestProbe(); +} + +TEST_F(ProbeControllerTest, 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(rtc::Optional())); + 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(ProbeControllerTest, 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(rtc::Optional())); + 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(ProbeControllerTest, 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(rtc::Optional(clock_.TimeInMilliseconds()))); + clock_.AdvanceTimeMilliseconds(kAlrProbeInterval + 1); + probe_controller_->Process(); + probe_controller_->SetEstimatedBitrate(250); + clock_.AdvanceTimeMilliseconds(kBitrateDropTimeoutMs + 1); + probe_controller_->RequestProbe(); } TEST_F(ProbeControllerTest, PeriodicProbing) { diff --git a/webrtc/modules/congestion_controller/send_side_congestion_controller.cc b/webrtc/modules/congestion_controller/send_side_congestion_controller.cc index 5328161e08..df9d762aa8 100644 --- a/webrtc/modules/congestion_controller/send_side_congestion_controller.cc +++ b/webrtc/modules/congestion_controller/send_side_congestion_controller.cc @@ -283,8 +283,10 @@ void SendSideCongestionController::OnTransportFeedback( bool currently_in_alr = pacer_->GetApplicationLimitedRegionStartTime().has_value(); - if (!currently_in_alr && was_in_alr_) { - acknowledged_bitrate_estimator_->SetAlrEndedTimeMs(rtc::TimeMillis()); + if (was_in_alr_ && !currently_in_alr) { + int64_t now_ms = rtc::TimeMillis(); + acknowledged_bitrate_estimator_->SetAlrEndedTimeMs(now_ms); + probe_controller_->SetAlrEndedTimeMs(now_ms); } was_in_alr_ = currently_in_alr; @@ -296,8 +298,13 @@ void SendSideCongestionController::OnTransportFeedback( result = delay_based_bwe_->IncomingPacketFeedbackVector( feedback_vector, acknowledged_bitrate_estimator_->bitrate_bps()); } - if (result.updated) + if (result.updated) { bitrate_controller_->OnDelayBasedBweResult(result); + // Update the estimate in the ProbeController, in case we want to probe. + MaybeTriggerOnNetworkChanged(); + } + if (result.recovered_from_overuse) + probe_controller_->RequestProbe(); } std::vector