From 2b7d96959916306cb267899a219cbd20ec24b841 Mon Sep 17 00:00:00 2001 From: Markus Handell Date: Thu, 14 May 2020 15:03:40 +0200 Subject: [PATCH] RtpVideoSender: remove lock recursions. This change removes lock recursions and adds thread annotations. Bug: webrtc:11567 Change-Id: Ib3341a96ab896a3c10696182d6a30c133988dc44 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/175102 Reviewed-by: Stefan Holmer Commit-Queue: Markus Handell Cr-Commit-Position: refs/heads/master@{#31262} --- call/rtp_video_sender.cc | 13 +++++++-- call/rtp_video_sender.h | 61 ++++++++++++++++++++++++---------------- 2 files changed, 48 insertions(+), 26 deletions(-) diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc index 8c31a848aa..b6cb054488 100644 --- a/call/rtp_video_sender.cc +++ b/call/rtp_video_sender.cc @@ -461,11 +461,16 @@ void RtpVideoSender::SetActive(bool active) { if (active_ == active) return; const std::vector active_modules(rtp_streams_.size(), active); - SetActiveModules(active_modules); + SetActiveModulesLocked(active_modules); } void RtpVideoSender::SetActiveModules(const std::vector active_modules) { rtc::CritScope lock(&crit_); + return SetActiveModulesLocked(active_modules); +} + +void RtpVideoSender::SetActiveModulesLocked( + const std::vector active_modules) { RTC_DCHECK_EQ(rtp_streams_.size(), active_modules.size()); active_ = false; for (size_t i = 0; i < active_modules.size(); ++i) { @@ -481,6 +486,10 @@ void RtpVideoSender::SetActiveModules(const std::vector active_modules) { bool RtpVideoSender::IsActive() { rtc::CritScope lock(&crit_); + return IsActiveLocked(); +} + +bool RtpVideoSender::IsActiveLocked() { return active_ && !rtp_streams_.empty(); } @@ -565,7 +574,7 @@ EncodedImageCallback::Result RtpVideoSender::OnEncodedImage( void RtpVideoSender::OnBitrateAllocationUpdated( const VideoBitrateAllocation& bitrate) { rtc::CritScope lock(&crit_); - if (IsActive()) { + if (IsActiveLocked()) { if (rtp_streams_.size() == 1) { // If spatial scalability is enabled, it is covered by a single stream. rtp_streams_[0].rtp_rtcp->SetVideoBitrateAllocation(bitrate); diff --git a/call/rtp_video_sender.h b/call/rtp_video_sender.h index f7d8c763d2..58bb7f412e 100644 --- a/call/rtp_video_sender.h +++ b/call/rtp_video_sender.h @@ -96,61 +96,74 @@ class RtpVideoSender : public RtpVideoSenderInterface, // |module_process_thread| was created (libjingle's worker thread). // TODO(perkj): Replace the use of |module_process_thread| with a TaskQueue, // maybe |worker_queue|. - void RegisterProcessThread(ProcessThread* module_process_thread) override; - void DeRegisterProcessThread() override; + void RegisterProcessThread(ProcessThread* module_process_thread) + RTC_LOCKS_EXCLUDED(crit_) override; + void DeRegisterProcessThread() RTC_LOCKS_EXCLUDED(crit_) override; // RtpVideoSender will only route packets if being active, all packets will be // dropped otherwise. - void SetActive(bool active) override; + void SetActive(bool active) RTC_LOCKS_EXCLUDED(crit_) override; // Sets the sending status of the rtp modules and appropriately sets the // payload router to active if any rtp modules are active. - void SetActiveModules(const std::vector active_modules) override; - bool IsActive() override; + void SetActiveModules(const std::vector active_modules) + RTC_LOCKS_EXCLUDED(crit_) override; + bool IsActive() RTC_LOCKS_EXCLUDED(crit_) override; - void OnNetworkAvailability(bool network_available) override; - std::map GetRtpStates() const override; - std::map GetRtpPayloadStates() const override; + void OnNetworkAvailability(bool network_available) + RTC_LOCKS_EXCLUDED(crit_) override; + std::map GetRtpStates() const + RTC_LOCKS_EXCLUDED(crit_) override; + std::map GetRtpPayloadStates() const + RTC_LOCKS_EXCLUDED(crit_) override; - void DeliverRtcp(const uint8_t* packet, size_t length) override; + void DeliverRtcp(const uint8_t* packet, size_t length) + RTC_LOCKS_EXCLUDED(crit_) override; // Implements webrtc::VCMProtectionCallback. int ProtectionRequest(const FecProtectionParams* delta_params, const FecProtectionParams* key_params, uint32_t* sent_video_rate_bps, uint32_t* sent_nack_rate_bps, - uint32_t* sent_fec_rate_bps) override; + uint32_t* sent_fec_rate_bps) + RTC_LOCKS_EXCLUDED(crit_) override; // Implements FecControllerOverride. - void SetFecAllowed(bool fec_allowed) override; + void SetFecAllowed(bool fec_allowed) RTC_LOCKS_EXCLUDED(crit_) override; // Implements EncodedImageCallback. // Returns 0 if the packet was routed / sent, -1 otherwise. EncodedImageCallback::Result OnEncodedImage( const EncodedImage& encoded_image, const CodecSpecificInfo* codec_specific_info, - const RTPFragmentationHeader* fragmentation) override; + const RTPFragmentationHeader* fragmentation) + RTC_LOCKS_EXCLUDED(crit_) override; - void OnBitrateAllocationUpdated( - const VideoBitrateAllocation& bitrate) override; + void OnBitrateAllocationUpdated(const VideoBitrateAllocation& bitrate) + RTC_LOCKS_EXCLUDED(crit_) override; - void OnTransportOverheadChanged( - size_t transport_overhead_bytes_per_packet) override; - void OnBitrateUpdated(BitrateAllocationUpdate update, int framerate) override; - uint32_t GetPayloadBitrateBps() const override; - uint32_t GetProtectionBitrateBps() const override; - void SetEncodingData(size_t width, - size_t height, - size_t num_temporal_layers) override; + void OnTransportOverheadChanged(size_t transport_overhead_bytes_per_packet) + RTC_LOCKS_EXCLUDED(crit_) override; + void OnBitrateUpdated(BitrateAllocationUpdate update, int framerate) + RTC_LOCKS_EXCLUDED(crit_) override; + uint32_t GetPayloadBitrateBps() const RTC_LOCKS_EXCLUDED(crit_) override; + uint32_t GetProtectionBitrateBps() const RTC_LOCKS_EXCLUDED(crit_) override; + void SetEncodingData(size_t width, size_t height, size_t num_temporal_layers) + RTC_LOCKS_EXCLUDED(crit_) override; std::vector GetSentRtpPacketInfos( uint32_t ssrc, - rtc::ArrayView sequence_numbers) const override; + rtc::ArrayView sequence_numbers) const + RTC_LOCKS_EXCLUDED(crit_) override; // From StreamFeedbackObserver. void OnPacketFeedbackVector( - std::vector packet_feedback_vector) override; + std::vector packet_feedback_vector) + RTC_LOCKS_EXCLUDED(crit_) override; private: + bool IsActiveLocked() RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_); + void SetActiveModulesLocked(const std::vector active_modules) + RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_); void UpdateModuleSendingState() RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_); void ConfigureProtection(); void ConfigureSsrcs();