Prevent busy-looping PacedSender on small packets.

webrtc/modules/pacing/bitrate_prober.cc

 /*
  *  Copyright (c) 2014 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 56 matching lines @@
     bitrates_bps[i] = kProbeBitrateMultipliers[i] * bitrate_bps;
     bitrate_log << " " << bitrates_bps[i];
     // We need one extra to get 5 deltas for the first probe.
     if (i == 0)
       probe_bitrates_.push_back(bitrates_bps[i]);
     for (int j = 0; j < kPacketsPerProbe; ++j)
       probe_bitrates_.push_back(bitrates_bps[i]);
   }
   bitrate_log << ", num packets: " << probe_bitrates_.size();
   LOG(LS_INFO) << bitrate_log.str().c_str();
+  // Set last send time to non-(-1) so TimeUntilNextProbe doesn't short circuit.
+  time_last_send_ms_ = 0;
   probing_state_ = kProbing;
 }
 
 int BitrateProber::TimeUntilNextProbe(int64_t now_ms) {
   if (probing_state_ != kDisabled && probe_bitrates_.empty()) {
     probing_state_ = kWait;
   }
-  if (probe_bitrates_.empty()) {
+  if (probe_bitrates_.empty() || time_last_send_ms_ == -1) {
     // No probe started, or waiting for next probe.

[stefan-webrtc, 2016/02/10 10:56:19]

Given line 78, is this comment still correct?

[pbos-webrtc, 2016/02/10 13:19:08]

Done.

     return -1;
   }
   int64_t elapsed_time_ms = now_ms - time_last_send_ms_;
+  // If no packets have been sent for n milliseconds, inactivate to not keep

[stefan-webrtc, 2016/02/10 10:56:19]

deactivate

[pbos-webrtc, 2016/02/10 13:19:08]

Done.

+  // spinning.
+  static const int kSendDeltaUntilConsideredInactiveMs = 5000;
+  if (elapsed_time_ms > kSendDeltaUntilConsideredInactiveMs) {
+    time_last_send_ms_ = -1;
+    packet_size_last_send_ = 0;
+    return -1;
+  }

[stefan-webrtc, 2016/02/10 10:56:19]

Shouldn't we change the state to say we're no longer probing?

[pbos-webrtc, 2016/02/10 13:19:08]

Changed state to allow a fresh probe.

   // 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_send_ > PacedSender::kMinProbePacketSize &&
-      probing_state_ == kProbing) {
+  if (packet_size_last_send_ != 0 && probing_state_ == kProbing) {
     int next_delta_ms = ComputeDeltaFromBitrate(packet_size_last_send_,
                                                 probe_bitrates_.front());
     time_until_probe_ms = next_delta_ms - elapsed_time_ms;
     // There is no point in trying to probe with less than 1 ms between packets
     // as it essentially means trying to probe at infinite bandwidth.
     const int kMinProbeDeltaMs = 1;
     // If we have waited more than 3 ms for a new packet to probe with we will
     // consider this probing session over.
     const int kMaxProbeDelayMs = 3;
     if (next_delta_ms < kMinProbeDeltaMs ||
         time_until_probe_ms < -kMaxProbeDelayMs) {
       // We currently disable probing after the first probe, as we only want
       // to probe at the beginning of a connection. We should set this to
       // kWait if we later want to probe periodically.
       probing_state_ = kWait;
       LOG(LS_INFO) << "Next delta too small, stop probing.";
       time_until_probe_ms = 0;
     }
   }
   return std::max(time_until_probe_ms, 0);
 }
 
 size_t BitrateProber::RecommendedPacketSize() const {
   return packet_size_last_send_;
 }
 
 void BitrateProber::PacketSent(int64_t now_ms, size_t packet_size) {
   assert(packet_size > 0);
+  if (packet_size < PacedSender::kMinProbePacketSize)
+    return;
   packet_size_last_send_ = packet_size;
   time_last_send_ms_ = now_ms;
   if (probing_state_ != kProbing)
     return;
   if (!probe_bitrates_.empty())
     probe_bitrates_.pop_front();
 }
 }  // namespace webrtc