Add probing to recover faster from large bitrate drops.

webrtc/modules/congestion_controller/delay_based_bwe.cc

Index: webrtc/modules/congestion_controller/delay_based_bwe.cc
diff --git a/webrtc/modules/congestion_controller/delay_based_bwe.cc b/webrtc/modules/congestion_controller/delay_based_bwe.cc
index c50f0d4b45e8fd1c9bd071f3f68082e30cc9647a..fd55d1ac23ef9b710d28d04859ca82b1ea782fe5 100644
--- a/webrtc/modules/congestion_controller/delay_based_bwe.cc
+++ b/webrtc/modules/congestion_controller/delay_based_bwe.cc
@@ -46,6 +46,13 @@ constexpr double kDefaultTrendlineThresholdGain = 4.0;
 
 constexpr int kMaxConsecutiveFailedLookups = 5;
 
+// If the bitrate drops to a factor |kBitrateDropThreshold| or lower
+// and we recover within |kBitrateDropTimeLimitMs|, then we'll send
+// a probe at a fraction |kBitrateDropProbeFraction| of the original bitrate.
+constexpr double kBitrateDropThreshold = 0.5;
+constexpr int kBitrateDropTimeLimitMs = 3000;
+constexpr double kBitrateDropProbeFraction = 0.85;
+
 const char kBweSparseUpdateExperiment[] = "WebRTC-BweSparseUpdateExperiment";
 
 bool BweSparseUpdateExperimentIsEnabled() {
@@ -57,6 +64,28 @@ bool BweSparseUpdateExperimentIsEnabled() {
 
 namespace webrtc {
 
+DelayBasedBwe::Result::Result()
+    : updated(false),
+      probe(false),
+      target_bitrate_bps(0),
+      suggested_probe_bps() {}
+
+DelayBasedBwe::Result::Result(bool probe, uint32_t target_bitrate_bps)
+    : updated(true),
+      probe(probe),
+      target_bitrate_bps(target_bitrate_bps),
+      suggested_probe_bps() {}
+
+DelayBasedBwe::Result::Result(bool probe,
+                              uint32_t target_bitrate_bps,
+                              uint32_t suggested_probe_bps)
+    : updated(true),
+      probe(probe),
+      target_bitrate_bps(target_bitrate_bps),
+      suggested_probe_bps(suggested_probe_bps) {}
+
+DelayBasedBwe::Result::~Result() {}
+
 DelayBasedBwe::DelayBasedBwe(RtcEventLog* event_log, const Clock* clock)
     : event_log_(event_log),
       clock_(clock),
@@ -102,6 +131,8 @@ DelayBasedBwe::Result DelayBasedBwe::IncomingPacketFeedbackVector(
   }
   bool overusing = false;
   bool delayed_feedback = true;
+  bool request_probe = 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 +140,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) {
+      request_probe = 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 +157,7 @@ DelayBasedBwe::Result DelayBasedBwe::IncomingPacketFeedbackVector(
     }
   } else {
     consecutive_delayed_feedbacks_ = 0;
-    return MaybeUpdateEstimate(overusing, acked_bitrate_bps);
+    return MaybeUpdateEstimate(overusing, acked_bitrate_bps, request_probe);
   }
   return Result();
 }
@@ -189,7 +226,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 request_probe) {
   Result result;
   int64_t now_ms = clock_->TimeInMilliseconds();
 
@@ -223,6 +261,13 @@ DelayBasedBwe::Result DelayBasedBwe::MaybeUpdateEstimate(
     } else {
       result.updated = UpdateEstimate(now_ms, acked_bitrate_bps, overusing,
                                       &result.target_bitrate_bps);
+      if (request_probe &&
+          result.target_bitrate_bps <
+              kBitrateDropProbeFraction * bitrate_before_last_large_drop_ &&

[philipel, 2017/07/25 11:26:04]

I think a slightly clearer way of doing this check is to replace 
"result.target_bitrate_bps < 
    kBitrateDropProbeFraction * bitrate_before_last_large_drop_"
with
"time_of_last_drop_ms_ == now_ms"

[terelius, 2017/07/27 21:02:45]

It is not the same thing. However, it seems a bit redundant to check that we're
not probing at a lower bitrate that the current target both here and in the
ProbeController. The main difference is that the ProbeController gets its
estimate from the BitrateController. One the other hand, maybe it is safer and
clearer to do the similar checks in both places? WDYT?

[terelius, 2017/07/28 17:14:47]

Moved the entire drop detection into the ProbeController as discussed offline.

+          now_ms - time_of_last_large_drop_ms_ < kBitrateDropTimeLimitMs) {
+        result.suggested_probe_bps = static_cast<rtc::Optional<uint32_t>>(

[philipel, 2017/07/25 11:26:04]

result.suggested_probe_bps.emplace 

[terelius, 2017/07/27 21:02:45]

Done.

+            kBitrateDropProbeFraction * bitrate_before_last_large_drop_);
+      }
     }
   }
   if (result.updated) {
@@ -250,6 +295,10 @@ bool DelayBasedBwe::UpdateEstimate(int64_t now_ms,
       acked_bitrate_bps, 0);
   uint32_t prev_target_bitrate_bps = rate_control_.LatestEstimate();
   *target_bitrate_bps = rate_control_.Update(&input, now_ms);
+  if (*target_bitrate_bps < kBitrateDropThreshold * prev_target_bitrate_bps) {
+    time_of_last_large_drop_ms_ = now_ms;
+    bitrate_before_last_large_drop_ = prev_target_bitrate_bps;
+  }
   return rate_control_.ValidEstimate() &&
          prev_target_bitrate_bps != *target_bitrate_bps;
 }