From c33c0fcbf7ca125671f3a9d68f4e57168ea13a54 Mon Sep 17 00:00:00 2001 From: Sebastian Jansson Date: Thu, 22 Feb 2018 11:10:18 +0100 Subject: [PATCH] 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 Commit-Queue: Sebastian Jansson Cr-Commit-Position: refs/heads/master@{#22166} --- call/BUILD.gn | 1 + call/call.cc | 22 ++----------------- call/rtp_transport_controller_send.cc | 21 +++++++++++------- call/rtp_transport_controller_send.h | 5 +++-- .../rtp_transport_controller_send_interface.h | 2 -- 5 files changed, 19 insertions(+), 32 deletions(-) diff --git a/call/BUILD.gn b/call/BUILD.gn index 3871928242..9b9296ebd5 100644 --- a/call/BUILD.gn +++ b/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", ] diff --git a/call/call.cc b/call/call.cc index 0322a8624d..323763abc2 100644 --- a/call/call.cc +++ b/call/call.cc @@ -258,7 +258,6 @@ class Call : public webrtc::Call, const int num_cpu_cores_; const std::unique_ptr module_process_thread_; - const std::unique_ptr pacer_thread_; const std::unique_ptr call_stats_; const std::unique_ptr 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()); diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc index 5e054a85b0..ed0054772a 100644 --- a/call/rtp_transport_controller_send.cc +++ b/call/rtp_transport_controller_send.cc @@ -10,6 +10,7 @@ #include #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_; } diff --git a/call/rtp_transport_controller_send.h b/call/rtp_transport_controller_send.h index ec71614464..371f626f7f 100644 --- a/call/rtp_transport_controller_send.h +++ b/call/rtp_transport_controller_send.h @@ -12,6 +12,7 @@ #define CALL_RTP_TRANSPORT_CONTROLLER_SEND_H_ #include +#include #include #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 network_routes_; + const std::unique_ptr process_thread_; RTC_DISALLOW_COPY_AND_ASSIGN(RtpTransportControllerSend); }; diff --git a/call/rtp_transport_controller_send_interface.h b/call/rtp_transport_controller_send_interface.h index 736689e3d7..0bf0fd6058 100644 --- a/call/rtp_transport_controller_send_interface.h +++ b/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(