Add probing to recover faster from large bitrate drops. A single probe at 85% of the original bitrate is sent when transitioning from underusing back to normal state. The actual sending of the probes is disabled by default, and enabled by the field trial string WebRTC-BweRapidRecoveryExperiment/Enabled/. Existing code that did probing after large drops in ALR have been restructured so that it also delays the probe until we are no longer overusing.

BUG=webrtc:8015

Review-Url: https://codereview.webrtc.org/2986563002
Cr-Commit-Position: refs/heads/master@{#19187}

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<uint32_t> acked_bitrate_bps) {
+    rtc::Optional<uint32_t> 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) {

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<uint32_t> acked_bitrate_bps);
+                             rtc::Optional<uint32_t> 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,

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() {

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<int64_t> 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_);

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<int64_t>(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<int64_t>()));
+  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<int64_t>()));
+  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<int64_t>(clock_.TimeInMilliseconds())));
+  clock_.AdvanceTimeMilliseconds(kAlrProbeInterval + 1);
+  probe_controller_->Process();
+  probe_controller_->SetEstimatedBitrate(250);
+  clock_.AdvanceTimeMilliseconds(kBitrateDropTimeoutMs + 1);
+  probe_controller_->RequestProbe();
 }
 
 TEST_F(ProbeControllerTest, PeriodicProbing) {

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<PacketFeedback>