Implement periodic bandwidth probing in application-limited region.

webrtc/modules/congestion_controller/probe_controller.cc

 /*
  *  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.
  */
 
@@ skipping 21 matching lines @@
 // 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 int kDefaultMaxProbingBitrateBps = 100000000;
 
 // This is a limit on how often probing can be done when there is a BW
 // drop detected in ALR region.
-constexpr int kAlrProbingIntervalLimitMs = 5000;
+constexpr int64_t kAlrProbingIntervalMinMs = 5000;
+
+// Increase periodic probes intervals up to 2 minutes.
+constexpr int64_t kAlrProbingIntervalMaxMs = 120000;
 
 }  // namespace
 
 ProbeController::ProbeController(PacedSender* pacer, Clock* clock)
     : pacer_(pacer),
       clock_(clock),
       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),
-      last_alr_probing_time_(clock_->TimeInMilliseconds()) {}
+      last_alr_probing_time_(clock_->TimeInMilliseconds()),
+      enable_periodic_probing_(false),
+      last_periodic_probing_time_(clock_->TimeInMilliseconds()) {}
 
 void ProbeController::SetBitrates(int min_bitrate_bps,
                                   int start_bitrate_bps,
                                   int max_bitrate_bps) {
   rtc::CritScope cs(&critsect_);
 
   start_bitrate_bps_ =
       start_bitrate_bps > 0 ? start_bitrate_bps : min_bitrate_bps;
 
   int 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:
       // Initiate probing when |max_bitrate_| was increased mid-call.
       if (estimated_bitrate_bps_ != 0 &&
           estimated_bitrate_bps_ < old_max_bitrate_bps &&
           max_bitrate_bps_ > old_max_bitrate_bps) {
-        InitiateProbing({max_bitrate_bps}, kExponentialProbingDisabled);
+        InitiateProbing(clock_->TimeInMilliseconds(), {max_bitrate_bps},
+                        kExponentialProbingDisabled);
       }
       break;
   }
 }
 
 void ProbeController::OnNetworkStateChanged(NetworkState network_state) {
   rtc::CritScope cs(&critsect_);
   network_state_ = network_state;
   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({3 * start_bitrate_bps_, 6 * start_bitrate_bps_},
+  InitiateProbing(clock_->TimeInMilliseconds(),
+                  {3 * start_bitrate_bps_, 6 * start_bitrate_bps_},
                   4 * start_bitrate_bps_);
 }
 
 void ProbeController::SetEstimatedBitrate(int bitrate_bps) {
   rtc::CritScope cs(&critsect_);
+  int64_t now_ms = clock_->TimeInMilliseconds();
+
   if (state_ == State::kWaitingForProbingResult) {
-    if ((clock_->TimeInMilliseconds() - time_last_probing_initiated_ms_) >
+    if ((now_ms - time_last_probing_initiated_ms_) >
         kMaxWaitingTimeForProbingResultMs) {
       LOG(LS_INFO) << "kWaitingForProbingResult: timeout";
       state_ = State::kProbingComplete;
       min_bitrate_to_probe_further_bps_ = kExponentialProbingDisabled;
     } else {
       // Continue probing if probing results indicate channel has greater
       // capacity.
       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 and expect a minimum of 25% gain to
         // continue probing.
-        InitiateProbing({2 * bitrate_bps}, 1.25 * bitrate_bps);
+        InitiateProbing(now_ms, {2 * bitrate_bps}, bitrate_bps * 5 / 4);
+      } else {
+        // Stop exponential probing.
+        state_ = State::kProbingComplete;
+        min_bitrate_to_probe_further_bps_ = kExponentialProbingDisabled;
       }
     }
-  } else {
-    // 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 (pacer_->InApplicationLimitedRegion() &&
-        bitrate_bps < 0.5 * estimated_bitrate_bps_) {
-      int64_t now_ms = clock_->TimeInMilliseconds();
-      if ((now_ms - last_alr_probing_time_) > kAlrProbingIntervalLimitMs) {
-        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({estimated_bitrate_bps_}, kExponentialProbingDisabled);
-        last_alr_probing_time_ = now_ms;
-      }
-    }
+  }
+
+  // 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() &&

[philipel, 2016/11/16 14:59:15]

I think it's cleaner to keep the InApplicationLimitedRegion function in addition
to the new GetApplicationLimitedRegionStartTime.

[Sergey Ulanov, 2016/11/21 09:06:03]

I don't think keeping InApplicationLimitedRegion() next to
GetApplicationLimitedRegionStartTime() is the right thing to do, because
separating them makes it easier to use them incorrectly. Particularly because
pacer_ runs on a different threads, and so ALR state may change between the two
calls. Another alternative here is to have GetApplicationLimitedRegionState()
that would return both the bool state and start time, e.g. as a struct:
 struct AlrState {
   bool in_alr;
   int64_t start_time;
 }

But this looks like it adds more complexity than we need here. Another option is
to return rtc::Option<int64_t> from GetApplicationLimitedRegionStartTime().
WDYT?

[philipel, 2016/11/21 12:05:42]

I see what you mean about having both InApplicationLimitedRegion and
GetApplicationLimitedRegionStartTime, so I agree it's better to only have
GetApplicationLimitedRegionStartTime.

I think using rtc::Optional is the best option in this case (we want to move
away from magic constants).

[Sergey Ulanov, 2016/11/21 19:28:18]

Done.

+      bitrate_bps < estimated_bitrate_bps_ / 2 &&
+      (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_},
+                    kExponentialProbingDisabled);
+    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.
   }
+
   estimated_bitrate_bps_ = bitrate_bps;
 }
 
+void ProbeController::EnablePeriodicProbing(bool enable) {
+  rtc::CritScope cs(&critsect_);
+  enable_periodic_probing_ = enable;
+}
+
+void ProbeController::Process() {
+  rtc::CritScope cs(&critsect_);
+
+  if (state_ != State::kProbingComplete || !enable_periodic_probing_)
+    return;
+
+  // Probe bandwidth periodically when in ALR state.
+  int64_t alr_start_time = pacer_->GetApplicationLimitedRegionStartTime();
+  if (alr_start_time > 0) {
+    int64_t now_ms = clock_->TimeInMilliseconds();
+    int64_t time_in_alr_ms = now_ms - alr_start_time;
+
+    // Calculate probing period as half of the time passed since ALR started,
+    // clamped between min and max values. This results in the interval
+    // increasing from the minimum exponentially until it reaches the maximum.
+    // With min and max values set to 5s and 2m the probes are sent at 5s, 10s,
+    // 20s, 40s, 80s, 160s and every 2 minutes after that.
+    int alr_probe_period_ms =

[philipel, 2016/11/16 14:59:15]

Probing is very cheap (but jittery) so I don't think think we need exponential
backoff. I think an interval of 5-10 seconds would work well.

Have you done any tests regarding this behavior?

[Sergey Ulanov, 2016/11/21 09:06:03]

The idea here was that the longer the sender stays in ALR the less likely it is
that it will need the extra bandwidth, but yes, it's premature to implement
exponential backoff here. Changed it to regular interval of 5 second. Will see
how well that works.

What did you mean by "jittery"?

[philipel, 2016/11/21 12:05:42]

Lets say we have a stable link that does not change, probing this link will
result in estimates that are fairly jittery, i.e. you will get different results
every time (sometimes even higher than the actual available bandwidth).

[Sergey Ulanov, 2016/11/21 19:28:18]

Right, there is a lot of noise in the probes result, but I don't think it's an
issue in ALR, it won't be noticeable to the user.

+        std::min(std::max(kAlrProbingIntervalMinMs, time_in_alr_ms / 2),
+                 kAlrProbingIntervalMaxMs);
+
+    int time_since_last_probe_ms =

[philipel, 2016/11/16 14:59:15]

I'm not sure why you added the |now_ms| argument to InitiateProbing (since it's
not used), but don't you think it's better if we calculate the time since the
last probing was initiated?

[Sergey Ulanov, 2016/11/21 09:06:03]

That was a mistake, fixed that function to use now_ms.
Done. Also simplified this code a bit.

+        std::min(time_in_alr_ms, now_ms - last_periodic_probing_time_);
+    if (time_since_last_probe_ms >= alr_probe_period_ms) {
+      InitiateProbing(now_ms, {estimated_bitrate_bps_ * 2},
+                      estimated_bitrate_bps_ * 5 / 4);
+      last_periodic_probing_time_ = now_ms;
+    }
+  }
+}
+
 void ProbeController::InitiateProbing(
+    int64_t now_ms,

[philipel, 2016/11/16 14:59:15]

Not used?

[Sergey Ulanov, 2016/11/21 09:06:03]

Done.

     std::initializer_list<int> bitrates_to_probe,
     int min_bitrate_to_probe_further_bps) {
   bool first_cluster = true;
   for (int bitrate : bitrates_to_probe) {
     int max_probe_bitrate_bps =
         max_bitrate_bps_ > 0 ? max_bitrate_bps_ : kDefaultMaxProbingBitrateBps;
     bitrate = std::min(bitrate, max_probe_bitrate_bps);
     if (first_cluster) {
       pacer_->CreateProbeCluster(bitrate, kProbeDeltasPerCluster + 1);
       first_cluster = false;
     } else {
       pacer_->CreateProbeCluster(bitrate, kProbeDeltasPerCluster);
     }
   }
   min_bitrate_to_probe_further_bps_ = min_bitrate_to_probe_further_bps;
   time_last_probing_initiated_ms_ = clock_->TimeInMilliseconds();
   if (min_bitrate_to_probe_further_bps == kExponentialProbingDisabled)
     state_ = State::kProbingComplete;
   else
     state_ = State::kWaitingForProbingResult;
 }
 
 }  // namespace webrtc