Probing: Add support for exponential startup probing

webrtc/modules/pacing/bitrate_prober.cc

Index: webrtc/modules/pacing/bitrate_prober.cc
diff --git a/webrtc/modules/pacing/bitrate_prober.cc b/webrtc/modules/pacing/bitrate_prober.cc
index 4b2a977bbebcdcab302e24c762e679ae0cd87d0e..bb8a059f69a4a1573d9170fdfcddd53cea2959a6 100644
--- a/webrtc/modules/pacing/bitrate_prober.cc
+++ b/webrtc/modules/pacing/bitrate_prober.cc
@@ -22,6 +22,14 @@ namespace {
 
 // Inactivity threshold above which probing is restarted.
 constexpr int kInactivityThresholdMs = 5000;
+// Number of deltas between probes per cluster. On the very first cluster,
+// we will need kProbeDeltasPerCluster + 1 probes, but on a cluster following
+// another, we need kProbeDeltasPerCluster probes.
+constexpr int kProbeDeltasPerCluster = 5;
+
+// Maximum waiting time from the time of sending last probe to getting
+// the measured results back.
+const int64_t kMaxWaitingTimeForProbingResultMs = 1000;
 
 int ComputeDeltaFromBitrate(size_t packet_size, uint32_t bitrate_bps) {
   RTC_CHECK_GT(bitrate_bps, 0u);
@@ -32,39 +40,94 @@ int ComputeDeltaFromBitrate(size_t packet_size, uint32_t bitrate_bps) {
 }  // namespace
 
 BitrateProber::BitrateProber()
-    : probing_state_(ProbingState::kDisabled),
+    : probing_state_(State::kInit),
       packet_size_last_sent_(0),
       time_last_probe_sent_ms_(-1),
-      next_cluster_id_(0) {
-  SetEnabled(true);
+      next_cluster_id_(0),
+      min_bitrate_to_probe_further_(0),
+      estimated_bitrate_bps_(0) {}
+
+bool BitrateProber::IsProbing() const {
+  return probing_state_ == State::kSending;
 }
 
 void BitrateProber::SetEnabled(bool enable) {
   if (enable) {
-    if (probing_state_ == ProbingState::kDisabled) {
-      probing_state_ = ProbingState::kInactive;
-      LOG(LS_INFO) << "Bandwidth probing enabled, set to inactive";
+    if (probing_state_ == State::kDisabled) {
+      probing_state_ = State::kInit;
+      LOG(LS_INFO) << "Bandwidth probing enabled";
     }
   } else {
-    probing_state_ = ProbingState::kDisabled;
+    probing_state_ = State::kDisabled;
     LOG(LS_INFO) << "Bandwidth probing disabled";
   }
 }
 
-bool BitrateProber::IsProbing() const {
-  return probing_state_ == ProbingState::kActive;
+bool BitrateProber::IsExpectingProbingResults() const {
+  // We would like to hear of probe results the moment we start sending
+  // probes.
+  return probing_state_ == State::kSending ||
+         probing_state_ == State::kWaitForResult;
 }
 
-void BitrateProber::OnIncomingPacket(size_t packet_size) {
-  // Don't initialize probing unless we have something large enough to start
-  // probing.
-  if (probing_state_ == ProbingState::kInactive &&
-      packet_size >= PacedSender::kMinProbePacketSize) {
-    probing_state_ = ProbingState::kActive;
+void BitrateProber::SetEstimatedBitrate(int bitrate_bps) {
+  switch (probing_state_) {
+    // We could have a result before we finish sending probe clusters
+    // completely.
+    case State::kSending:
+    case State::kWaitForResult:

[danilchap, 2016/08/19 17:19:34]

shouldn't you start probing from State::kComplete too?

[Irfan, 2016/08/23 05:46:57]

kComplete indicates we do not expect any results from probing and we are done
with probing.

kSending and kWaitForResults are basically states where the prober expects
feedback to decide if it should ramp up again. so it makes sense to look at
feedback only in these states.

Let me know if that is not clear.

[danilchap, 2016/08/23 17:59:29]

So it is not a coincidence it is same states as in functions just above.
May be then replace the switch with 
if (IsExpectingProbingResult()) {
 .... 
}

+      LOG(LS_INFO) << "SetEstimatedBitrate " << bitrate_bps
+                   << " min_bitrate_to_probe_further_  "
+                   << min_bitrate_to_probe_further_;
+      if (bitrate_bps > min_bitrate_to_probe_further_) {
+        // Reset before new probe session.
+        ResetState();
+        ProbeAtBitrate(2 * bitrate_bps, kProbeDeltasPerCluster + 1);
+        // A minimum of 25% gain to continue.
+        min_bitrate_to_probe_further_ = 1.25 * bitrate_bps;
+      }
+      break;
+    default:
+      break;
   }
+  estimated_bitrate_bps_ = bitrate_bps;
 }
 
-void BitrateProber::ProbeAtBitrate(uint32_t bitrate_bps, int num_packets) {
+void BitrateProber::OnIncomingPacket(int64_t now_ms, size_t packet_size) {
+  switch (probing_state_) {
+    case State::kInit:
+      // Don't initialize probing unless we have something large enough to start
+      // probing.
+      if (packet_size >= PacedSender::kMinProbePacketSize &&
+          estimated_bitrate_bps_ > 0) {
+        LOG(LS_INFO) << "kInit: initialize probing";
+        // Ensure state is initialized before starting probing.
+        ResetState();
+        ProbeAtBitrate(3 * estimated_bitrate_bps_, kProbeDeltasPerCluster + 1);
+        ProbeAtBitrate(6 * estimated_bitrate_bps_, kProbeDeltasPerCluster);
+        // When probing at 1.8 Mbps ( 6x 300), this represents a threshold of
+        // 1.2 Mbps to continue probing.
+        min_bitrate_to_probe_further_ = 4 * estimated_bitrate_bps_;
+      }
+      break;
+    case State::kWaitForResult:
+      if ((now_ms - time_last_probe_sent_ms_) >
+          kMaxWaitingTimeForProbingResultMs) {
+        probing_state_ = State::kComplete;
+        LOG(LS_INFO) << "kWaitForResult: timeout";
+      }
+      break;
+    case State::kSending:
+    case State::kComplete:
+    case State::kDisabled:
+      break;
+    default:

[danilchap, 2016/08/19 17:19:34]

remove default.
Missing state would be noticed at compile time, instead of runtime then.

[Irfan, 2016/08/23 05:46:57]

Done.

+      RTC_NOTREACHED();
+  }
+}
+
+void BitrateProber::ProbeAtBitrate(int bitrate_bps, int num_packets) {
+  RTC_DCHECK(probing_state_ != State::kDisabled);

[danilchap, 2016/08/19 17:19:34]

RTC_DCHECK_NE

[Irfan, 2016/08/23 05:46:57]

RTC_DCHECK_NE does not handle scoped enums properly. Something to look at
outside this CL.

   ProbeCluster cluster;
   cluster.max_probe_packets = num_packets;
   cluster.probe_bitrate_bps = bitrate_bps;
@@ -73,8 +136,7 @@ void BitrateProber::ProbeAtBitrate(uint32_t bitrate_bps, int num_packets) {
   LOG(LS_INFO) << "Probe cluster (bitrate:packets): ("
                << cluster.probe_bitrate_bps << ":" << cluster.max_probe_packets
                << ") ";
-  if (probing_state_ != ProbingState::kActive)
-    probing_state_ = ProbingState::kInactive;
+  probing_state_ = State::kSending;
 }
 
 void BitrateProber::ResetState() {
@@ -89,14 +151,11 @@ void BitrateProber::ResetState() {
                    clusters.front().max_probe_packets);
     clusters.pop();
   }
-  // If its enabled, reset to inactive.
-  if (probing_state_ != ProbingState::kDisabled)
-    probing_state_ = ProbingState::kInactive;
 }
 
 int BitrateProber::TimeUntilNextProbe(int64_t now_ms) {
-  // Probing is not active or probing is already complete.
-  if (probing_state_ != ProbingState::kActive || clusters_.empty())
+  // Proceed only if we are in kSending state.
+  if (probing_state_ != State::kSending)
     return -1;
   // time_last_probe_sent_ms_ of -1 indicates no probes have yet been sent.
   int64_t elapsed_time_ms;
@@ -114,7 +173,7 @@ int BitrateProber::TimeUntilNextProbe(int64_t now_ms) {
   // We will send the first probe packet immediately if no packet has been
   // sent before.
   int time_until_probe_ms = 0;
-  if (packet_size_last_sent_ != 0 && probing_state_ == ProbingState::kActive) {
+  if (packet_size_last_sent_ != 0 && probing_state_ == State::kSending) {

[danilchap, 2016/08/19 17:19:34]

probing_state == State::kSending seems no longer needed this line.

[Irfan, 2016/08/23 05:46:57]

Done.

     int next_delta_ms = ComputeDeltaFromBitrate(
         packet_size_last_sent_, clusters_.front().probe_bitrate_bps);
     time_until_probe_ms = next_delta_ms - elapsed_time_ms;
@@ -126,7 +185,7 @@ int BitrateProber::TimeUntilNextProbe(int64_t now_ms) {
     const int kMaxProbeDelayMs = 3;
     if (next_delta_ms < kMinProbeDeltaMs ||
         time_until_probe_ms < -kMaxProbeDelayMs) {
-      probing_state_ = ProbingState::kSuspended;
+      probing_state_ = State::kComplete;
       LOG(LS_INFO) << "Delta too small or missed probing accurately, suspend";
       time_until_probe_ms = 0;
     }
@@ -136,7 +195,7 @@ int BitrateProber::TimeUntilNextProbe(int64_t now_ms) {
 
 int BitrateProber::CurrentClusterId() const {
   RTC_DCHECK(!clusters_.empty());
-  RTC_DCHECK(ProbingState::kActive == probing_state_);
+  RTC_DCHECK(probing_state_ == State::kSending);

[danilchap, 2016/08/19 17:19:34]

RTC_DCHECK_EQ

[Irfan, 2016/08/23 05:46:57]

see above

   return clusters_.front().id;
 }
 
@@ -146,11 +205,11 @@ size_t BitrateProber::RecommendedPacketSize() const {
 
 void BitrateProber::PacketSent(int64_t now_ms, size_t packet_size) {
   assert(packet_size > 0);
+  RTC_DCHECK(probing_state_ == State::kSending);

[danilchap, 2016/08/19 17:19:34]

RTC_DCHECK_EQ

[Irfan, 2016/08/23 05:46:57]

see above

+
   if (packet_size < PacedSender::kMinProbePacketSize)
     return;
   packet_size_last_sent_ = packet_size;
-  if (probing_state_ != ProbingState::kActive)
-    return;
   time_last_probe_sent_ms_ = now_ms;
   if (!clusters_.empty()) {
     ProbeCluster* cluster = &clusters_.front();
@@ -158,7 +217,7 @@ void BitrateProber::PacketSent(int64_t now_ms, size_t packet_size) {
     if (cluster->sent_probe_packets == cluster->max_probe_packets)
       clusters_.pop();
     if (clusters_.empty())
-      probing_state_ = ProbingState::kSuspended;
+      probing_state_ = State::kWaitForResult;
   }
 }
 }  // namespace webrtc