Probing: Add support for exponential startup probing

webrtc/modules/congestion_controller/delay_based_bwe.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.
  */
 
 #include "webrtc/modules/congestion_controller/delay_based_bwe.h"
 
 #include <math.h>
 
 #include <algorithm>
 
 #include "webrtc/base/checks.h"
 #include "webrtc/base/constructormagic.h"
 #include "webrtc/base/logging.h"
 #include "webrtc/base/thread_annotations.h"
+#include "webrtc/modules/congestion_controller/include/congestion_controller.h"
 #include "webrtc/modules/pacing/paced_sender.h"
 #include "webrtc/modules/remote_bitrate_estimator/include/remote_bitrate_estimator.h"
 #include "webrtc/system_wrappers/include/critical_section_wrapper.h"
 #include "webrtc/system_wrappers/include/metrics.h"
 #include "webrtc/typedefs.h"
 
 namespace {
 constexpr int kTimestampGroupLengthMs = 5;
 constexpr int kAbsSendTimeFraction = 18;
 constexpr int kAbsSendTimeInterArrivalUpshift = 8;
 constexpr int kInterArrivalShift =
     kAbsSendTimeFraction + kAbsSendTimeInterArrivalUpshift;
 constexpr double kTimestampToMs =
     1000.0 / static_cast<double>(1 << kInterArrivalShift);
 
 // This ssrc is used to fulfill the current API but will be removed
 // after the API has been changed.
 constexpr uint32_t kFixedSsrc = 0;
 }  // namespace
 
 namespace webrtc {
 
-DelayBasedBwe::DelayBasedBwe(RemoteBitrateObserver* observer, Clock* clock)
+DelayBasedBwe::DelayBasedBwe(CongestionController* controller, Clock* clock)
     : clock_(clock),
-      observer_(observer),
+      controller_(controller),
       inter_arrival_(),
       estimator_(),
       detector_(OverUseDetectorOptions()),
       incoming_bitrate_(kBitrateWindowMs, 8000),
-      first_packet_time_ms_(-1),
       last_update_ms_(-1),
       last_seen_packet_ms_(-1),
       uma_recorded_(false) {
-  RTC_DCHECK(observer_);
+  RTC_DCHECK(controller_);
   network_thread_.DetachFromThread();
 }
 
 void DelayBasedBwe::IncomingPacketFeedbackVector(
     const std::vector<PacketInfo>& packet_feedback_vector) {
   RTC_DCHECK(network_thread_.CalledOnValidThread());
   if (!uma_recorded_) {
     RTC_LOGGED_HISTOGRAM_ENUMERATION(kBweTypeHistogram,
                                      BweNames::kSendSideTransportSeqNum,
                                      BweNames::kBweNamesMax);
     uma_recorded_ = true;
   }
   for (const auto& packet_info : packet_feedback_vector) {
     IncomingPacketInfo(packet_info);
   }
 }
 
 void DelayBasedBwe::IncomingPacketInfo(const PacketInfo& info) {
   int64_t now_ms = clock_->TimeInMilliseconds();
 
-  if (first_packet_time_ms_ == -1)
-    first_packet_time_ms_ = now_ms;
-
   incoming_bitrate_.Update(info.payload_size, info.arrival_time_ms);
-  bool update_estimate = false;
-  uint32_t target_bitrate_bps = 0;
+  bool delay_based_bwe_changed = false;
+  int target_bitrate_bps = 0;
   {
     rtc::CritScope lock(&crit_);
 
     // Reset if the stream has timed out.
     if (last_seen_packet_ms_ == -1 ||
         now_ms - last_seen_packet_ms_ > kStreamTimeOutMs) {
       inter_arrival_.reset(new InterArrival(
           (kTimestampGroupLengthMs << kInterArrivalShift) / 1000,
           kTimestampToMs, true));
       estimator_.reset(new OveruseEstimator(OverUseDetectorOptions()));
     }
     last_seen_packet_ms_ = now_ms;
 
-    if (info.probe_cluster_id != PacketInfo::kNotAProbe) {
-      ProbingResult probe_result =
-          probe_bitrate_estimator_.PacketFeedback(info);
-      if (probe_result.valid()) {
-        remote_rate_.SetEstimate(probe_result.bps, probe_result.timestamp);
-        update_estimate = true;
-      }
-    }
-
     uint32_t send_time_24bits =
         static_cast<uint32_t>(((static_cast<uint64_t>(info.send_time_ms)
                                 << kAbsSendTimeFraction) +
                                500) /
                               1000) &
         0x00FFFFFF;
     // Shift up send time to use the full 32 bits that inter_arrival works with,
     // so wrapping works properly.
     uint32_t timestamp = send_time_24bits << kAbsSendTimeInterArrivalUpshift;
 
     uint32_t ts_delta = 0;
     int64_t t_delta = 0;
     int size_delta = 0;
     if (inter_arrival_->ComputeDeltas(timestamp, info.arrival_time_ms, now_ms,
                                       info.payload_size, &ts_delta, &t_delta,
                                       &size_delta)) {
       double ts_delta_ms = (1000.0 * ts_delta) / (1 << kInterArrivalShift);
       estimator_->Update(t_delta, ts_delta_ms, size_delta, detector_.State());
       detector_.Detect(estimator_->offset(), ts_delta_ms,
                        estimator_->num_of_deltas(), info.arrival_time_ms);
     }
 
-    if (!update_estimate) {
-      // Check if it's time for a periodic update or if we should update because
-      // of an over-use.
-      if (last_update_ms_ == -1 ||
-          now_ms - last_update_ms_ > remote_rate_.GetFeedbackInterval()) {
-        update_estimate = true;
-      } else if (detector_.State() == kBwOverusing) {
-        rtc::Optional<uint32_t> incoming_rate =
-            incoming_bitrate_.Rate(info.arrival_time_ms);
-        if (incoming_rate &&
-            remote_rate_.TimeToReduceFurther(now_ms, *incoming_rate)) {
-          update_estimate = true;
-        }
+    int probing_bps = 0;
+    if (info.probe_cluster_id != PacketInfo::kNotAProbe &&
+        controller_->SenderState() ==
+            PacedSender::State::kWaitForProbingResult) {
+      // Without a valid estimate, we wait for two clusters and then later on
+      // wait on a single cluster to update probing bitrate.
+      if (!remote_rate_.ValidEstimate()) {
+        probing_bps =
+            probe_bitrate_estimator_.HandleProbeAndEstimateBitrate(info, 2);

[philipel, 2016/08/16 14:44:43]

Seems like probe_bitrate_estimator.h/.cc haven't been uploaded, can you upload a
patch with them included?

[Irfan, 2016/08/16 16:04:17]

https://codereview.chromium.org/2239143002/ is the CL which has changes in
probe_bitrate_estimator.

+      } else {
+        probing_bps =
+            probe_bitrate_estimator_.HandleProbeAndEstimateBitrate(info, 1);
       }
     }
 
-    if (update_estimate) {
-      // The first overuse should immediately trigger a new estimate.
-      // We also have to update the estimate immediately if we are overusing
-      // and the target bitrate is too high compared to what we are receiving.
-      const RateControlInput input(detector_.State(),
-                                   incoming_bitrate_.Rate(info.arrival_time_ms),
-                                   estimator_->var_noise());
-      remote_rate_.Update(&input, now_ms);
-      target_bitrate_bps = remote_rate_.UpdateBandwidthEstimate(now_ms);
-      update_estimate = remote_rate_.ValidEstimate();
+    // Overuse

[stefan-webrtc, 2016/08/16 11:27:04]

End the comments with "."

[Irfan, 2016/08/16 18:12:47]

Will make these full sentence comments and add full stop.

+    if (detector_.State() == kBwOverusing) {
+      rtc::Optional<uint32_t> incoming_rate =
+          incoming_bitrate_.Rate(info.arrival_time_ms);
+      if (incoming_rate &&
+          remote_rate_.TimeToReduceFurther(now_ms, *incoming_rate)) {
+        delay_based_bwe_changed =
+            UpdateEstimate(info.arrival_time_ms, now_ms, &target_bitrate_bps);
+      }
+      // Probe results with overuse
+      if (probing_bps) {
+        // Stop probing
+        controller_->OnProbingBitrateMeasured(0);

[stefan-webrtc, 2016/08/16 11:27:04]

This looks a bit hacky. It's not really that we measured a zero bitrate on a
probe. Should we have a separate method to stop probing, or a parameter to say
so explicitly?

[Irfan, 2016/08/16 18:12:47]

Will make it a well-defined name instead. Works for you ?

+      }
+      // No overuse, but probing measured a bitrate
+    } else if (probing_bps > 0) {
+      controller_->OnProbingBitrateMeasured(probing_bps);
+      remote_rate_.SetEstimate(probing_bps, info.arrival_time_ms);
+      target_bitrate_bps = probing_bps;
+      delay_based_bwe_changed = true;
+      // Been a while since we updated

[stefan-webrtc, 2016/08/16 11:27:04]

Move this inside the else if below instead to make it clear that it doesn't
belong to this block.

[Irfan, 2016/08/16 18:12:47]

Done.

+    } else if (last_update_ms_ == -1 ||
+               now_ms - last_update_ms_ > remote_rate_.GetFeedbackInterval()) {
+      delay_based_bwe_changed =
+          UpdateEstimate(info.arrival_time_ms, now_ms, &target_bitrate_bps);
     }
   }
 
-  if (update_estimate) {
+  if (delay_based_bwe_changed) {
     last_update_ms_ = now_ms;
-    observer_->OnReceiveBitrateChanged({kFixedSsrc}, target_bitrate_bps);
+    controller_->OnDelayBasedBweChanged(target_bitrate_bps);
   }
 }
 
+bool DelayBasedBwe::UpdateEstimate(int64_t arrival_time_ms,
+                                   int64_t now_ms,
+                                   int* target_bitrate_bps) {
+  rtc::CritScope lock(&crit_);
+  // The first overuse should immediately trigger a new estimate.
+  // We also have to update the estimate immediately if we are overusing
+  // and the target bitrate is too high compared to what we are receiving.
+  const RateControlInput input(detector_.State(),
+                               incoming_bitrate_.Rate(arrival_time_ms),
+                               estimator_->var_noise());
+  remote_rate_.Update(&input, now_ms);
+  *target_bitrate_bps = remote_rate_.UpdateBandwidthEstimate(now_ms);
+  return remote_rate_.ValidEstimate();
+}
+
 void DelayBasedBwe::Process() {}
 
 int64_t DelayBasedBwe::TimeUntilNextProcess() {
   const int64_t kDisabledModuleTime = 1000;
   return kDisabledModuleTime;
 }
 
 void DelayBasedBwe::OnRttUpdate(int64_t avg_rtt_ms, int64_t max_rtt_ms) {
   rtc::CritScope lock(&crit_);
   remote_rate_.SetRtt(avg_rtt_ms);
@@ skipping 18 matching lines @@
   return true;
 }
 
 void DelayBasedBwe::SetMinBitrate(int min_bitrate_bps) {
   // Called from both the configuration thread and the network thread. Shouldn't
   // be called from the network thread in the future.
   rtc::CritScope lock(&crit_);
   remote_rate_.SetMinBitrate(min_bitrate_bps);
 }
 }  // namespace webrtc