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 c5aa983266c4c923b7189675a68fadfbff5e508d..61565dd758a8b6d4539686e7eafd68ea434e1735 100644
--- a/webrtc/modules/pacing/bitrate_prober.cc
+++ b/webrtc/modules/pacing/bitrate_prober.cc
@@ -26,6 +26,8 @@ namespace {
 
 // Inactivity threshold above which probing is restarted.
 constexpr int kInactivityThresholdMs = 5000;
+// Number of packets per cluster of probes.
+constexpr int kPacketsPerCluster = 5;
 
 int ComputeDeltaFromBitrate(size_t packet_size, uint32_t bitrate_bps) {
   assert(bitrate_bps > 0);
@@ -36,7 +38,7 @@ int ComputeDeltaFromBitrate(size_t packet_size, uint32_t bitrate_bps) {
 }  // namespace
 
 BitrateProber::BitrateProber()
-    : probing_state_(ProbingState::kDisabled),
+    : probing_state_(State::kDisabled),
       packet_size_last_sent_(0),
       time_last_probe_sent_ms_(-1),
       next_cluster_id_(0) {
@@ -45,40 +47,30 @@ BitrateProber::BitrateProber()
 
 void BitrateProber::SetEnabled(bool enable) {
   if (enable) {
-    if (probing_state_ == ProbingState::kDisabled) {
-      probing_state_ = ProbingState::kInactive;
+    if (probing_state_ == State::kDisabled) {
+      probing_state_ = State::kInactive;
       LOG(LS_INFO) << "Bandwidth probing enabled, set to inactive";
     }
   } else {
-    probing_state_ = ProbingState::kDisabled;
+    probing_state_ = State::kDisabled;
     LOG(LS_INFO) << "Bandwidth probing disabled";
   }
 }
 
 bool BitrateProber::IsProbing() const {
-  return probing_state_ == ProbingState::kActive;
+  return probing_state_ == State::kActive;
 }
 
-void BitrateProber::OnIncomingPacket(uint32_t bitrate_bps,
-                                     size_t packet_size,
-                                     int64_t now_ms) {
-  // Don't initialize probing unless we have something large enough to start
-  // probing.
-  if (packet_size < PacedSender::kMinProbePacketSize)
-    return;
-  if (probing_state_ != ProbingState::kInactive)
-    return;
-  // Max number of packets used for probing.
-  const int kMaxNumProbes = 2;
-  const int kPacketsPerProbe = 5;
-  const float kProbeBitrateMultipliers[kMaxNumProbes] = {3, 6};
+bool BitrateProber::InitiateProbing(uint32_t bitrate_bps,
+                                    const int* multipliers,
+                                    int multipler_count) {
   std::stringstream bitrate_log;
   bitrate_log << "Start probing for bandwidth, (bitrate:packets): ";
-  for (int i = 0; i < kMaxNumProbes; ++i) {
+  for (int i = 0; i < multipler_count; ++i) {
     ProbeCluster cluster;
     // We need one extra to get 5 deltas for the first probe, therefore (i == 0)
-    cluster.max_probe_packets = kPacketsPerProbe + (i == 0 ? 1 : 0);
-    cluster.probe_bitrate_bps = kProbeBitrateMultipliers[i] * bitrate_bps;
+    cluster.max_probe_packets = kPacketsPerCluster + (i == 0 ? 1 : 0);
+    cluster.probe_bitrate_bps = multipliers[i] * bitrate_bps;
     cluster.id = next_cluster_id_++;
 
     bitrate_log << "(" << cluster.probe_bitrate_bps << ":"
@@ -87,7 +79,8 @@ void BitrateProber::OnIncomingPacket(uint32_t bitrate_bps,
     clusters_.push(cluster);
   }
   LOG(LS_INFO) << bitrate_log.str().c_str();
-  probing_state_ = ProbingState::kActive;
+  probing_state_ = State::kActive;
+  return true;
 }
 
 void BitrateProber::ResetState() {
@@ -95,13 +88,13 @@ void BitrateProber::ResetState() {
   packet_size_last_sent_ = 0;
   clusters_ = std::queue<ProbeCluster>();
   // If its enabled, reset to inactive.
-  if (probing_state_ != ProbingState::kDisabled)
-    probing_state_ = ProbingState::kInactive;
+  if (probing_state_ != State::kDisabled)
+    probing_state_ = State::kInactive;
 }
 
 int BitrateProber::TimeUntilNextProbe(int64_t now_ms) {
   // Probing is not active or probing is already complete.
-  if (probing_state_ != ProbingState::kActive || clusters_.empty())
+  if (probing_state_ != State::kActive || clusters_.empty())
     return -1;
   // time_last_probe_sent_ms_ of -1 indicates no probes have yet been sent.
   int64_t elapsed_time_ms;
@@ -119,7 +112,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::kActive) {
     int next_delta_ms = ComputeDeltaFromBitrate(
         packet_size_last_sent_, clusters_.front().probe_bitrate_bps);
     time_until_probe_ms = next_delta_ms - elapsed_time_ms;
@@ -131,7 +124,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;
     }
@@ -141,7 +134,7 @@ int BitrateProber::TimeUntilNextProbe(int64_t now_ms) {
 
 int BitrateProber::CurrentClusterId() const {
   RTC_DCHECK(!clusters_.empty());
-  RTC_DCHECK(ProbingState::kActive == probing_state_);
+  RTC_DCHECK(State::kActive == probing_state_);
   return clusters_.front().id;
 }
 
@@ -149,13 +142,14 @@ size_t BitrateProber::RecommendedPacketSize() const {
   return packet_size_last_sent_;
 }
 
-void BitrateProber::PacketSent(int64_t now_ms, size_t packet_size) {
+BitrateProber::State BitrateProber::PacketSent(int64_t now_ms,
+                                               size_t packet_size) {
   assert(packet_size > 0);
+  RTC_DCHECK(probing_state_ == State::kActive);

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

Isn't it good to notify the prober about recent packets even though it's not
active? That way it will know what's been sent before and can better recommend a
packet size on line 142.

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

We initiate probing only when there is an incoming packet > kMinProbePacketSize
and is added to queue.

The RecommendedPacketSize() is used for padding and should be triggered at the
earliest on the second probe packet (since we have atleast one in queue).

Once we initiate probing, we keep sending probes until done.

The last_packet_size_ is used to decide on delta between probes (except on first
probe when it is immediately sent). All this seems to indicate we really only
need to track last_packet_size_ on the probes themselves. 

By doing so, we will be able to have stricter constraints on the method (add
DCHECK on state we expect to be called) and calling only on probes keeps it
simpler.

Let me know if I have missed something here. I can revert this behavior if there
is a problem.

+
   if (packet_size < PacedSender::kMinProbePacketSize)
-    return;
+    return probing_state_;
   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();
@@ -163,7 +157,8 @@ 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::kComplete;
   }
+  return probing_state_;
 }
 }  // namespace webrtc