Moved pacer and congestion thread from call.

Moving the module process thread responsible for running the pacer
and the send side congestion controller to RtpTransportControllerSend
since it already owns the pacer and the congestion controller. They
are also moved to a common thread rather than using two separate
threads.

As part of the move, the remote bitrate estimator has been moved to the
common process thread in the Call class. Previously it was run on the
removed pacer thread.

Bug: webrtc:8415
Change-Id: I4322eef30d8b97b9611f33af7e560703b710d232
Reviewed-on: https://webrtc-review.googlesource.com/55700
Reviewed-by: Niels Moller <nisse@webrtc.org>
Commit-Queue: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22166}

call/BUILD.gn

@@ -97,6 +97,7 @@ rtc_source_set("rtp_sender") {
     "..:webrtc_common",
     "../modules/congestion_controller",
     "../modules/pacing",
+    "../modules/utility",
     "../rtc_base:rtc_base",
     "../rtc_base:rtc_base_approved",
   ]

call/call.cc

@@ -258,7 +258,6 @@ class Call : public webrtc::Call,
 
   const int num_cpu_cores_;
   const std::unique_ptr<ProcessThread> module_process_thread_;
-  const std::unique_ptr<ProcessThread> pacer_thread_;
   const std::unique_ptr<CallStats> call_stats_;
   const std::unique_ptr<BitrateAllocator> bitrate_allocator_;
   Call::Config config_;
@@ -405,7 +404,6 @@ Call::Call(const Call::Config& config,
     : clock_(Clock::GetRealTimeClock()),
       num_cpu_cores_(CpuInfo::DetectNumberOfCores()),
       module_process_thread_(ProcessThread::Create("ModuleProcessThread")),
-      pacer_thread_(ProcessThread::Create("PacerThread")),
       call_stats_(new CallStats(clock_)),
       bitrate_allocator_(new BitrateAllocator(this)),
       config_(config),
@@ -433,19 +431,10 @@ Call::Call(const Call::Config& config,
   call_stats_->RegisterStatsObserver(&receive_side_cc_);
   call_stats_->RegisterStatsObserver(transport_send_->GetCallStatsObserver());
 
-  // We have to attach the pacer to the pacer thread before starting the
-  // module process thread to avoid a race accessing the process thread
-  // both from the process thread and the pacer thread.
-  pacer_thread_->RegisterModule(transport_send_->GetPacerModule(),
-                                RTC_FROM_HERE);
-  pacer_thread_->RegisterModule(
+  module_process_thread_->RegisterModule(
       receive_side_cc_.GetRemoteBitrateEstimator(true), RTC_FROM_HERE);
-  pacer_thread_->Start();
-
   module_process_thread_->RegisterModule(call_stats_.get(), RTC_FROM_HERE);
   module_process_thread_->RegisterModule(&receive_side_cc_, RTC_FROM_HERE);
-  module_process_thread_->RegisterModule(transport_send_->GetModule(),
-                                         RTC_FROM_HERE);
   module_process_thread_->Start();
 }
 
@@ -458,14 +447,7 @@ Call::~Call() {
   RTC_CHECK(audio_receive_streams_.empty());
   RTC_CHECK(video_receive_streams_.empty());
 
-  // The send-side congestion controller must be de-registered prior to
-  // the pacer thread being stopped to avoid a race when accessing the
-  // pacer thread object on the module process thread at the same time as
-  // the pacer thread is stopped.
-  module_process_thread_->DeRegisterModule(transport_send_->GetModule());
-  pacer_thread_->Stop();
-  pacer_thread_->DeRegisterModule(transport_send_->GetPacerModule());
-  pacer_thread_->DeRegisterModule(
+  module_process_thread_->DeRegisterModule(
       receive_side_cc_.GetRemoteBitrateEstimator(true));
   module_process_thread_->DeRegisterModule(&receive_side_cc_);
   module_process_thread_->DeRegisterModule(call_stats_.get());

call/rtp_transport_controller_send.cc

@@ -10,6 +10,7 @@
 #include <utility>
 
 #include "call/rtp_transport_controller_send.h"
+#include "rtc_base/location.h"
 #include "rtc_base/logging.h"
 
 namespace webrtc {
@@ -20,13 +21,23 @@ RtpTransportControllerSend::RtpTransportControllerSend(
     const BitrateConstraints& bitrate_config)
     : pacer_(clock, &packet_router_, event_log),
       send_side_cc_(clock, nullptr /* observer */, event_log, &pacer_),
-      bitrate_configurator_(bitrate_config) {
+      bitrate_configurator_(bitrate_config),
+      process_thread_(ProcessThread::Create("SendControllerThread")) {
   send_side_cc_.SignalNetworkState(kNetworkDown);
   send_side_cc_.SetBweBitrates(bitrate_config.min_bitrate_bps,
                                bitrate_config.start_bitrate_bps,
                                bitrate_config.max_bitrate_bps);
+
+  process_thread_->RegisterModule(&pacer_, RTC_FROM_HERE);
+  process_thread_->RegisterModule(&send_side_cc_, RTC_FROM_HERE);
+  process_thread_->Start();
+}
+
+RtpTransportControllerSend::~RtpTransportControllerSend() {
+  process_thread_->Stop();
+  process_thread_->DeRegisterModule(&send_side_cc_);
+  process_thread_->DeRegisterModule(&pacer_);
 }
-RtpTransportControllerSend::~RtpTransportControllerSend() = default;
 
 PacketRouter* RtpTransportControllerSend::packet_router() {
   return &packet_router_;
@@ -55,18 +66,12 @@ void RtpTransportControllerSend::SetKeepAliveConfig(
     const RtpKeepAliveConfig& config) {
   keepalive_ = config;
 }
-Module* RtpTransportControllerSend::GetPacerModule() {
-  return &pacer_;
-}
 void RtpTransportControllerSend::SetPacingFactor(float pacing_factor) {
   pacer_.SetPacingFactor(pacing_factor);
 }
 void RtpTransportControllerSend::SetQueueTimeLimit(int limit_ms) {
   pacer_.SetQueueTimeLimit(limit_ms);
 }
-Module* RtpTransportControllerSend::GetModule() {
-  return &send_side_cc_;
-}
 CallStatsObserver* RtpTransportControllerSend::GetCallStatsObserver() {
   return &send_side_cc_;
 }

call/rtp_transport_controller_send.h

@@ -12,6 +12,7 @@
 #define CALL_RTP_TRANSPORT_CONTROLLER_SEND_H_
 
 #include <map>
+#include <memory>
 #include <string>
 
 #include "call/rtp_bitrate_configurator.h"
@@ -19,6 +20,7 @@
 #include "common_types.h"  // NOLINT(build/include)
 #include "modules/congestion_controller/include/send_side_congestion_controller.h"
 #include "modules/pacing/packet_router.h"
+#include "modules/utility/include/process_thread.h"
 #include "rtc_base/constructormagic.h"
 #include "rtc_base/networkroute.h"
 
@@ -46,10 +48,8 @@ class RtpTransportControllerSend : public RtpTransportControllerSendInterface {
                                      int max_padding_bitrate_bps) override;
 
   void SetKeepAliveConfig(const RtpKeepAliveConfig& config);
-  Module* GetPacerModule() override;
   void SetPacingFactor(float pacing_factor) override;
   void SetQueueTimeLimit(int limit_ms) override;
-  Module* GetModule() override;
   CallStatsObserver* GetCallStatsObserver() override;
   void RegisterPacketFeedbackObserver(
       PacketFeedbackObserver* observer) override;
@@ -81,6 +81,7 @@ class RtpTransportControllerSend : public RtpTransportControllerSendInterface {
   RtpKeepAliveConfig keepalive_;
   RtpBitrateConfigurator bitrate_configurator_;
   std::map<std::string, rtc::NetworkRoute> network_routes_;
+  const std::unique_ptr<ProcessThread> process_thread_;
 
   RTC_DISALLOW_COPY_AND_ASSIGN(RtpTransportControllerSend);
 };

call/rtp_transport_controller_send_interface.h

@@ -80,11 +80,9 @@ class RtpTransportControllerSendInterface {
   virtual void SetAllocatedSendBitrateLimits(int min_send_bitrate_bps,
                                              int max_padding_bitrate_bps) = 0;
 
-  virtual Module* GetPacerModule() = 0;
   virtual void SetPacingFactor(float pacing_factor) = 0;
   virtual void SetQueueTimeLimit(int limit_ms) = 0;
 
-  virtual Module* GetModule() = 0;
   virtual CallStatsObserver* GetCallStatsObserver() = 0;
 
   virtual void RegisterPacketFeedbackObserver(