Return a long timeout value from TimeUntilNextProcess when the PacedSender is paused

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 3421d3a817cddb71e457bb3276310f2197b7ff46..bae924f61c504a4fdbf57053b36cbf0647c7d968 100644
--- a/webrtc/modules/pacing/paced_sender.cc
+++ b/webrtc/modules/pacing/paced_sender.cc
@@ -21,6 +21,7 @@
 #include "webrtc/modules/include/module_common_types.h"
 #include "webrtc/modules/pacing/alr_detector.h"
 #include "webrtc/modules/pacing/bitrate_prober.h"
+#include "webrtc/modules/utility/include/process_thread.h"
 #include "webrtc/system_wrappers/include/clock.h"
 #include "webrtc/system_wrappers/include/critical_section_wrapper.h"
 #include "webrtc/system_wrappers/include/field_trial.h"
@@ -275,14 +276,22 @@ void PacedSender::CreateProbeCluster(int bitrate_bps) {
 
 void PacedSender::Pause() {
   LOG(LS_INFO) << "PacedSender paused.";
-  CriticalSectionScoped cs(critsect_.get());
-  paused_ = true;
+  {
+    CriticalSectionScoped cs(critsect_.get());
+    paused_ = true;
+  }
+  if (process_thread_)
+    process_thread_->WakeUp(this);

[stefan-webrtc, 2017/03/15 12:43:18]

Is there a point in waking up the thread if we're being paused? In that case I
assume there's nothing to send?

[tommi, 2017/03/15 13:42:56]

Yes, I added a comment that explains why:

  // Tell the process thread to call our TimeUntilNextProcess() method to get
  // a new (longer) estimate for when to call Process().

 }
 
 void PacedSender::Resume() {
   LOG(LS_INFO) << "PacedSender resumed.";
-  CriticalSectionScoped cs(critsect_.get());
-  paused_ = false;
+  {
+    CriticalSectionScoped cs(critsect_.get());
+    paused_ = false;
+  }
+  if (process_thread_)
+    process_thread_->WakeUp(this);
 }
 
 void PacedSender::SetProbingEnabled(bool enabled) {
@@ -373,6 +382,9 @@ int64_t PacedSender::AverageQueueTimeMs() {
 
 int64_t PacedSender::TimeUntilNextProcess() {
   CriticalSectionScoped cs(critsect_.get());
+  if (paused_)
+    return 1000 * 60 * 60;
+
   if (prober_->IsProbing()) {
     int64_t ret = prober_->TimeUntilNextProbe(clock_->TimeInMilliseconds());
     if (ret > 0 || (ret == 0 && !probing_send_failure_))
@@ -457,6 +469,11 @@ void PacedSender::Process() {
   alr_detector_->OnBytesSent(bytes_sent, now_us / 1000);
 }
 
+void PacedSender::ProcessThreadAttached(ProcessThread* process_thread) {

[stefan-webrtc, 2017/03/15 12:43:18]

We're never detaching because the thread is assumed to outlive the object?

[tommi, 2017/03/15 13:42:56]

This method is called when the module is registered with the thread,
unregistered (process_thread == nullptr) or when the process thread itself
stops.  Here's the documentation:

  // This method is called when the module is attached to a *running* process
  // thread or detached from one.  In the case of detaching, |process_thread|
  // will be nullptr.
  //
  // This method will be called in the following cases:
  //
  // * Non-null process_thread:
  //   * ProcessThread::RegisterModule() is called while the thread is running.
  //   * ProcessThread::Start() is called and RegisterModule has previously
  //     been called.  The thread will be started immediately after notifying
  //     all modules.
  //
  // * Null process_thread:
  //   * ProcessThread::DeRegisterModule() is called while the thread is
  //     running.
  //   * ProcessThread::Stop() was called and the thread has been stopped.
  //
  // NOTE: This method is not called from the worker thread itself, but from
  //       the thread that registers/deregisters the module or calls Start/Stop.

[stefan-webrtc, 2017/03/15 13:51:17]

I see, thanks

+  LOG(LS_INFO) << "ProcessThreadAttached 0x" << std::hex << process_thread;
+  process_thread_ = process_thread;
+}
+
 bool PacedSender::SendPacket(const paced_sender::Packet& packet,
                              const PacedPacketInfo& pacing_info) {
   if (paused_)