Add functionality which limits the number of bytes on the network.

webrtc/modules/pacing/paced_sender.cc

Index: webrtc/modules/pacing/paced_sender.cc
diff --git a/webrtc/modules/pacing/paced_sender.cc b/webrtc/modules/pacing/paced_sender.cc
index 4a66581e783fd24b6171d5a70079a5502ac7a839..9de65a4ed7ae6cb7418ddb6ad74fb5d6f18711a3 100644
--- a/webrtc/modules/pacing/paced_sender.cc
+++ b/webrtc/modules/pacing/paced_sender.cc
@@ -28,6 +28,7 @@
 namespace {
 // Time limit in milliseconds between packet bursts.
 const int64_t kMinPacketLimitMs = 5;
+const int64_t kPausedPacketIntervalMs = 500;
 
 // Upper cap on process interval, in case process has not been called in a long
 // time.
@@ -278,9 +279,10 @@ void PacedSender::CreateProbeCluster(int bitrate_bps) {
 }
 
 void PacedSender::Pause() {
-  LOG(LS_INFO) << "PacedSender paused.";
   {
     rtc::CritScope cs(&critsect_);
+    if (!paused_)
+      LOG(LS_INFO) << "PacedSender paused.";
     paused_ = true;
   }
   // Tell the process thread to call our TimeUntilNextProcess() method to get
@@ -290,9 +292,10 @@ void PacedSender::Pause() {
 }
 
 void PacedSender::Resume() {
-  LOG(LS_INFO) << "PacedSender resumed.";
   {
     rtc::CritScope cs(&critsect_);
+    if (paused_)
+      LOG(LS_INFO) << "PacedSender resumed.";
     paused_ = false;
   }
   // Tell the process thread to call our TimeUntilNextProcess() method to
@@ -394,16 +397,18 @@ int64_t PacedSender::AverageQueueTimeMs() {
 
 int64_t PacedSender::TimeUntilNextProcess() {
   rtc::CritScope cs(&critsect_);
+  int64_t elapsed_time_us = clock_->TimeInMicroseconds() - time_last_update_us_;
+  int64_t elapsed_time_ms = (elapsed_time_us + 500) / 1000;
+  // When paused we wake up every 500 ms to send a padding packet to ensure
+  // we won't get stuck in the paused state due to no feedback being received.
   if (paused_)
-    return 1000 * 60 * 60;
+    return std::max<int64_t>(kPausedPacketIntervalMs - elapsed_time_ms, 0);
 
   if (prober_->IsProbing()) {
     int64_t ret = prober_->TimeUntilNextProbe(clock_->TimeInMilliseconds());
     if (ret > 0 || (ret == 0 && !probing_send_failure_))
       return ret;
   }
-  int64_t elapsed_time_us = clock_->TimeInMicroseconds() - time_last_update_us_;
-  int64_t elapsed_time_ms = (elapsed_time_us + 500) / 1000;
   return std::max<int64_t>(kMinPacketLimitMs - elapsed_time_ms, 0);
 }
 
@@ -411,9 +416,17 @@ void PacedSender::Process() {
   int64_t now_us = clock_->TimeInMicroseconds();
   rtc::CritScope cs(&critsect_);
   int64_t elapsed_time_ms = (now_us - time_last_update_us_ + 500) / 1000;
-  time_last_update_us_ = now_us;
   int target_bitrate_kbps = pacing_bitrate_kbps_;
-  if (!paused_ && elapsed_time_ms > 0) {
+
+  if (paused_) {
+    PacedPacketInfo pacing_info;
+    size_t bytes_sent = SendPadding(1, pacing_info);

[philipel, 2017/08/02 15:35:16]

Is there a way to completely stop the pacer, not just pause it and still have it
send padding.

[stefan-webrtc, 2017/08/04 09:50:00]

We could, but the problem is that if we lose some feedback packets, or if the
packets in flight are lost, we may never resume. We need to send something to
ensure that we sooner or later get feedback and can resume...

+    alr_detector_->OnBytesSent(bytes_sent, now_us / 1000);
+    time_last_update_us_ = now_us;
+    return;
+  }
+
+  if (elapsed_time_ms > 0) {
     size_t queue_size_bytes = packets_->SizeInBytes();
     if (queue_size_bytes > 0) {
       // Assuming equal size packets and input/output rate, the average packet
@@ -434,6 +447,8 @@ void PacedSender::Process() {
     UpdateBudgetWithElapsedTime(elapsed_time_ms);
   }
 
+  time_last_update_us_ = now_us;
+
   bool is_probing = prober_->IsProbing();
   PacedPacketInfo pacing_info;
   size_t bytes_sent = 0;
@@ -463,17 +478,12 @@ void PacedSender::Process() {
     }
   }
 
-  if (packets_->Empty() && !paused_) {
-    // We can not send padding unless a normal packet has first been sent. If we
-    // do, timestamps get messed up.
-    if (packet_counter_ > 0) {
-      int padding_needed =
-          static_cast<int>(is_probing ? (recommended_probe_size - bytes_sent)
-                                      : padding_budget_->bytes_remaining());
-
-      if (padding_needed > 0)
-        bytes_sent += SendPadding(padding_needed, pacing_info);
-    }
+  if (packets_->Empty()) {

[philipel, 2017/08/02 15:35:16]

Is this correct?

I assume you want to be able to send padding while paused, even if there are
packets in the queue?

[stefan-webrtc, 2017/08/04 09:50:00]

Why would we do that if there are packets in the queue? I don't think this is a
change in behavior?

[philipel, 2017/08/04 14:13:04]

Ah, you're right.

+    int padding_needed =
+        static_cast<int>(is_probing ? (recommended_probe_size - bytes_sent)
+                                    : padding_budget_->bytes_remaining());
+    if (padding_needed > 0)
+      bytes_sent += SendPadding(padding_needed, pacing_info);
   }
   if (is_probing) {
     probing_send_failure_ = bytes_sent == 0;
@@ -490,8 +500,6 @@ void PacedSender::ProcessThreadAttached(ProcessThread* process_thread) {
 
 bool PacedSender::SendPacket(const paced_sender::Packet& packet,
                              const PacedPacketInfo& pacing_info) {
-  if (paused_)
-    return false;

[philipel, 2017/08/02 15:35:16]

Make it into an RTC_DCHECK

[stefan-webrtc, 2017/08/04 09:50:00]

Done.

   if (media_budget_->bytes_remaining() == 0 &&
       pacing_info.probe_cluster_id == PacedPacketInfo::kNotAProbe) {
     return false;
@@ -517,6 +525,10 @@ bool PacedSender::SendPacket(const paced_sender::Packet& packet,
 
 size_t PacedSender::SendPadding(size_t padding_needed,
                                 const PacedPacketInfo& pacing_info) {
+  // We can not send padding unless a normal packet has first been sent. If we
+  // do, timestamps get messed up.
+  if (packet_counter_ == 0)

[philipel, 2017/08/02 15:35:16]

I think it's somewhat clearer to keep this check where it was, moving it here
just makes this function behave in an unexpected way.

Maybe even add a DCHECK_GT for the |packet_counter_|.

[stefan-webrtc, 2017/08/04 09:50:00]

Then the code will have to be duplicated since we're calling SendPadding from
two places now. Is that preferable?

[philipel, 2017/08/04 14:13:04]

I still think that would be preferable.

+    return 0;
   critsect_.Leave();
   size_t bytes_sent =
       packet_sender_->TimeToSendPadding(padding_needed, pacing_info);