From 7703f23b60c493744aba84977aec459cbc33f016 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Mon, 14 Sep 2020 11:03:13 +0200 Subject: [PATCH] Adds ability to delay pacer start until media is added. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This avoids the pacer thread waking up at 5ms interval if a PeerConnection is created without actually using media. The TaskQueuePacedSender solves the problem too, this CL is mostly a safeguard in case we still find issues when turning it on... Can be turned off by setting field trial "WebRTC-LazyPacerStart" to "Disabled". Bug: webrtc:10809 Change-Id: I8501106e608eccb14487576f24bdceaf3f324d80 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/183982 Reviewed-by: Sebastian Jansson Reviewed-by: Tommi Commit-Queue: Erik Språng Cr-Commit-Position: refs/heads/master@{#32101} --- call/call.cc | 26 +++++++++---------- call/rtp_transport_controller_send.cc | 16 ++++++++---- call/rtp_transport_controller_send.h | 2 ++ .../rtp_transport_controller_send_interface.h | 2 ++ call/rtp_video_sender_unittest.cc | 1 + .../test/mock_rtp_transport_controller_send.h | 1 + 6 files changed, 30 insertions(+), 18 deletions(-) diff --git a/call/call.cc b/call/call.cc index ace83bee9f..f53f779737 100644 --- a/call/call.cc +++ b/call/call.cc @@ -306,7 +306,9 @@ class Call final : public webrtc::Call, void UpdateHistograms(); void UpdateAggregateNetworkState(); - void RegisterRateObserver(); + // Ensure that necessary process threads are started, and any required + // callbacks have been registered. + void EnsureStarted() RTC_EXCLUSIVE_LOCKS_REQUIRED(worker_thread_); rtc::TaskQueue* send_transport_queue() const { return transport_send_ptr_->GetWorkerQueue(); @@ -433,8 +435,7 @@ class Call final : public webrtc::Call, // last ensures that it is destroyed first and any running tasks are finished. std::unique_ptr transport_send_; - bool is_target_rate_observer_registered_ RTC_GUARDED_BY(worker_thread_) = - false; + bool is_started_ RTC_GUARDED_BY(worker_thread_) = false; RTC_DISALLOW_COPY_AND_ASSIGN(Call); }; @@ -655,19 +656,18 @@ Call::~Call() { UpdateHistograms(); } -void Call::RegisterRateObserver() { - RTC_DCHECK_RUN_ON(worker_thread_); - - if (is_target_rate_observer_registered_) +void Call::EnsureStarted() { + if (is_started_) { return; - - is_target_rate_observer_registered_ = true; + } + is_started_ = true; // This call seems to kick off a number of things, so probably better left // off being kicked off on request rather than in the ctor. transport_send_ptr_->RegisterTargetTransferRateObserver(this); module_process_thread_->EnsureStarted(); + transport_send_ptr_->EnsureStarted(); } void Call::SetClientBitratePreferences(const BitrateSettings& preferences) { @@ -762,7 +762,7 @@ webrtc::AudioSendStream* Call::CreateAudioSendStream( TRACE_EVENT0("webrtc", "Call::CreateAudioSendStream"); RTC_DCHECK_RUN_ON(worker_thread_); - RegisterRateObserver(); + EnsureStarted(); // Stream config is logged in AudioSendStream::ConfigureStream, as it may // change during the stream's lifetime. @@ -822,7 +822,7 @@ webrtc::AudioReceiveStream* Call::CreateAudioReceiveStream( const webrtc::AudioReceiveStream::Config& config) { TRACE_EVENT0("webrtc", "Call::CreateAudioReceiveStream"); RTC_DCHECK_RUN_ON(worker_thread_); - RegisterRateObserver(); + EnsureStarted(); event_log_->Log(std::make_unique( CreateRtcLogStreamConfig(config))); AudioReceiveStream* receive_stream = new AudioReceiveStream( @@ -877,7 +877,7 @@ webrtc::VideoSendStream* Call::CreateVideoSendStream( TRACE_EVENT0("webrtc", "Call::CreateVideoSendStream"); RTC_DCHECK_RUN_ON(worker_thread_); - RegisterRateObserver(); + EnsureStarted(); video_send_delay_stats_->AddSsrcs(config); for (size_t ssrc_index = 0; ssrc_index < config.rtp.ssrcs.size(); @@ -976,7 +976,7 @@ webrtc::VideoReceiveStream* Call::CreateVideoReceiveStream( receive_side_cc_.SetSendPeriodicFeedback( SendPeriodicFeedback(configuration.rtp.extensions)); - RegisterRateObserver(); + EnsureStarted(); TaskQueueBase* current = GetCurrentTaskQueueOrThread(); RTC_CHECK(current); diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc index 9baf164a60..d508a1064e 100644 --- a/call/rtp_transport_controller_send.cc +++ b/call/rtp_transport_controller_send.cc @@ -82,6 +82,7 @@ RtpTransportControllerSend::RtpTransportControllerSend( : clock_(clock), event_log_(event_log), bitrate_configurator_(bitrate_config), + process_thread_started_(false), process_thread_(std::move(process_thread)), use_task_queue_pacer_(IsEnabled(trials, "WebRTC-TaskQueuePacer")), process_thread_pacer_(use_task_queue_pacer_ @@ -130,15 +131,13 @@ RtpTransportControllerSend::RtpTransportControllerSend( pacer()->SetPacingRates( DataRate::BitsPerSec(bitrate_config.start_bitrate_bps), DataRate::Zero()); - if (!use_task_queue_pacer_) { - process_thread_->Start(); + if (!absl::StartsWith(trials->Lookup("WebRTC-LazyPacerStart"), "Disabled")) { + EnsureStarted(); } } RtpTransportControllerSend::~RtpTransportControllerSend() { - if (!use_task_queue_pacer_) { - process_thread_->Stop(); - } + process_thread_->Stop(); } RtpVideoSenderInterface* RtpTransportControllerSend::CreateRtpVideoSender( @@ -491,6 +490,13 @@ void RtpTransportControllerSend::IncludeOverheadInPacedSender() { pacer()->SetIncludeOverhead(); } +void RtpTransportControllerSend::EnsureStarted() { + if (!use_task_queue_pacer_ && !process_thread_started_) { + process_thread_started_ = true; + process_thread_->Start(); + } +} + void RtpTransportControllerSend::OnReceivedEstimatedBitrate(uint32_t bitrate) { RemoteBitrateReport msg; msg.receive_time = Timestamp::Millis(clock_->TimeInMilliseconds()); diff --git a/call/rtp_transport_controller_send.h b/call/rtp_transport_controller_send.h index e7310334cf..7025b03312 100644 --- a/call/rtp_transport_controller_send.h +++ b/call/rtp_transport_controller_send.h @@ -110,6 +110,7 @@ class RtpTransportControllerSend final void AccountForAudioPacketsInPacedSender(bool account_for_audio) override; void IncludeOverheadInPacedSender() override; + void EnsureStarted() override; // Implements RtcpBandwidthObserver interface void OnReceivedEstimatedBitrate(uint32_t bitrate) override; @@ -151,6 +152,7 @@ class RtpTransportControllerSend final std::vector> video_rtp_senders_; RtpBitrateConfigurator bitrate_configurator_; std::map network_routes_; + bool process_thread_started_; const std::unique_ptr process_thread_; const bool use_task_queue_pacer_; std::unique_ptr process_thread_pacer_; diff --git a/call/rtp_transport_controller_send_interface.h b/call/rtp_transport_controller_send_interface.h index 3c24c018e2..602908e2a4 100644 --- a/call/rtp_transport_controller_send_interface.h +++ b/call/rtp_transport_controller_send_interface.h @@ -154,6 +154,8 @@ class RtpTransportControllerSendInterface { virtual void AccountForAudioPacketsInPacedSender(bool account_for_audio) = 0; virtual void IncludeOverheadInPacedSender() = 0; + + virtual void EnsureStarted() = 0; }; } // namespace webrtc diff --git a/call/rtp_video_sender_unittest.cc b/call/rtp_video_sender_unittest.cc index f998c7ffd8..af0b5032f3 100644 --- a/call/rtp_video_sender_unittest.cc +++ b/call/rtp_video_sender_unittest.cc @@ -143,6 +143,7 @@ class RtpVideoSenderTestFixture { VideoEncoderConfig::ContentType::kRealtimeVideo), retransmission_rate_limiter_(time_controller_.GetClock(), kRetransmitWindowSizeMs) { + transport_controller_.EnsureStarted(); std::map suspended_ssrcs; router_ = std::make_unique( time_controller_.GetClock(), suspended_ssrcs, suspended_payload_states, diff --git a/call/test/mock_rtp_transport_controller_send.h b/call/test/mock_rtp_transport_controller_send.h index 308c087a40..b468aa6cb2 100644 --- a/call/test/mock_rtp_transport_controller_send.h +++ b/call/test/mock_rtp_transport_controller_send.h @@ -99,6 +99,7 @@ class MockRtpTransportControllerSend MOCK_METHOD(void, AccountForAudioPacketsInPacedSender, (bool), (override)); MOCK_METHOD(void, IncludeOverheadInPacedSender, (), (override)); MOCK_METHOD(void, OnReceivedPacket, (const ReceivedPacket&), (override)); + MOCK_METHOD(void, EnsureStarted, (), (override)); }; } // namespace webrtc #endif // CALL_TEST_MOCK_RTP_TRANSPORT_CONTROLLER_SEND_H_