Change return type of CongestionController::pacer() method.

webrtc/modules/congestion_controller/congestion_controller.cc

 /*
  *  Copyright (c) 2012 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.
  */
 
 #include "webrtc/modules/congestion_controller/include/congestion_controller.h"
 
 #include <algorithm>
 #include <memory>
 #include <vector>
 
 #include "webrtc/base/checks.h"
 #include "webrtc/base/logging.h"
 #include "webrtc/base/rate_limiter.h"
 #include "webrtc/base/socket.h"
 #include "webrtc/modules/bitrate_controller/include/bitrate_controller.h"
 #include "webrtc/modules/congestion_controller/probe_controller.h"
 #include "webrtc/modules/remote_bitrate_estimator/include/bwe_defines.h"
 #include "webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_abs_send_time.h"
 #include "webrtc/modules/remote_bitrate_estimator/remote_bitrate_estimator_single_stream.h"
+#include "webrtc/modules/utility/include/process_thread.h"
 #include "webrtc/system_wrappers/include/critical_section_wrapper.h"
 
 namespace webrtc {
 namespace {
 
 static const uint32_t kTimeOffsetSwitchThreshold = 30;
 static const int64_t kRetransmitWindowSizeMs = 500;
 
 // Makes sure that the bitrate and the min, max values are in valid range.
 static void ClampBitrates(int* bitrate_bps,
@@ skipping 130 matching lines @@
 CongestionController::CongestionController(
     Clock* clock,
     Observer* observer,
     RemoteBitrateObserver* remote_bitrate_observer,
     RtcEventLog* event_log,
     PacketRouter* packet_router,
     std::unique_ptr<PacedSender> pacer)
     : clock_(clock),
       observer_(observer),
       packet_router_(packet_router),
+      pacer_thread_(ProcessThread::Create("PacerThread")),

[perkj_webrtc, 2017/01/02 09:18:58]

There are upstreams projects that use CongestionController but not the pacer
that does not want an extra thread. 
Why is the thread created here when its not used in this class? Can it be
created and owned by the pacer itself? 

[nisse-webrtc, 2017/01/09 13:43:46]

I can try moving it to the pacer itself. One obvious obstacle is the calls to
RegisterModule and DeRegisterModule with &remote_estimator_proxy_.

Or is it bad to have the remote_estimator_proxy and pacer share the same thread?
If they do, it makes some sense to me to let the CongestionController own it.

       pacer_(std::move(pacer)),
       remote_bitrate_estimator_(
           new WrappingBitrateEstimator(remote_bitrate_observer, clock_)),
       // Constructed last as this object calls the provided callback on
       // construction.
       bitrate_controller_(
           BitrateController::CreateBitrateController(clock_, event_log)),
       probe_controller_(new ProbeController(pacer_.get(), clock_)),
       retransmission_rate_limiter_(
           new RateLimiter(clock, kRetransmitWindowSizeMs)),
       remote_estimator_proxy_(clock_, packet_router_),
       transport_feedback_adapter_(clock_, bitrate_controller_.get()),
       min_bitrate_bps_(congestion_controller::GetMinBitrateBps()),
       max_bitrate_bps_(0),
       last_reported_bitrate_bps_(0),
       last_reported_fraction_loss_(0),
       last_reported_rtt_(0),
       network_state_(kNetworkUp) {
   transport_feedback_adapter_.InitBwe();
   transport_feedback_adapter_.SetMinBitrate(min_bitrate_bps_);
+
+  pacer_thread_->RegisterModule(pacer_.get());
+  pacer_thread_->RegisterModule(&remote_estimator_proxy_);
+  pacer_thread_->Start();
+
 }
 
-CongestionController::~CongestionController() {}
+CongestionController::~CongestionController() {
+  pacer_thread_->Stop();
+  pacer_thread_->DeRegisterModule(pacer_.get());
+  pacer_thread_->DeRegisterModule(&remote_estimator_proxy_);
+}
 
 void CongestionController::SetBweBitrates(int min_bitrate_bps,
                                           int start_bitrate_bps,
                                           int max_bitrate_bps) {
   ClampBitrates(&start_bitrate_bps, &min_bitrate_bps, &max_bitrate_bps);
   bitrate_controller_->SetBitrates(start_bitrate_bps,
                                    min_bitrate_bps,
                                    max_bitrate_bps);
 
   probe_controller_->SetBitrates(min_bitrate_bps, start_bitrate_bps,
@@ skipping 151 matching lines @@
 bool CongestionController::IsSendQueueFull() const {
   return pacer_->ExpectedQueueTimeMs() > PacedSender::kMaxQueueLengthMs;
 }
 
 bool CongestionController::IsNetworkDown() const {
   rtc::CritScope cs(&critsect_);
   return network_state_ == kNetworkDown;
 }
 
 }  // namespace webrtc