From 2d319df955f2105ab3d2a8370d53d14d3288c996 Mon Sep 17 00:00:00 2001 From: Tommi Date: Thu, 23 Dec 2021 22:45:50 +0100 Subject: [PATCH] Add a sequence checker and a few checks to RtpVideoSender. Moving the following TODO into a bug for tracking. // TODO(holmer): Remove mutex_ once RtpVideoSender runs on the // transport task queue. Bug: webrtc:13517 Change-Id: Ie3deb1276c2edaf9894001501ce79409f5437dd6 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/242368 Reviewed-by: Stefan Holmer Commit-Queue: Tomas Gunnarsson Cr-Commit-Position: refs/heads/main@{#35612} --- call/rtp_video_sender.cc | 10 ++++++++++ call/rtp_video_sender.h | 6 +++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc index b953705459..e90a9a6159 100644 --- a/call/rtp_video_sender.cc +++ b/call/rtp_video_sender.cc @@ -394,6 +394,7 @@ RtpVideoSender::RtpVideoSender( encoder_target_rate_bps_(0), frame_counts_(rtp_config.ssrcs.size()), frame_count_observer_(observers.frame_count_observer) { + transport_checker_.Detach(); RTC_DCHECK_EQ(rtp_config_.ssrcs.size(), rtp_streams_.size()); if (send_side_bwe_with_overhead_ && has_packet_feedback_) transport_->IncludeOverheadInPacedSender(); @@ -460,6 +461,10 @@ RtpVideoSender::RtpVideoSender( } RtpVideoSender::~RtpVideoSender() { + // TODO(bugs.webrtc.org/13517): Remove once RtpVideoSender gets deleted on the + // transport task queue. + transport_checker_.Detach(); + SetActiveModulesLocked( std::vector(rtp_streams_.size(), /*active=*/false)); transport_->GetStreamFeedbackProvider()->DeRegisterStreamFeedbackObserver( @@ -467,6 +472,7 @@ RtpVideoSender::~RtpVideoSender() { } void RtpVideoSender::SetActive(bool active) { + RTC_DCHECK_RUN_ON(&transport_checker_); MutexLock lock(&mutex_); if (active_ == active) return; @@ -475,12 +481,14 @@ void RtpVideoSender::SetActive(bool active) { } void RtpVideoSender::SetActiveModules(const std::vector active_modules) { + RTC_DCHECK_RUN_ON(&transport_checker_); MutexLock lock(&mutex_); return SetActiveModulesLocked(active_modules); } void RtpVideoSender::SetActiveModulesLocked( const std::vector active_modules) { + RTC_DCHECK_RUN_ON(&transport_checker_); RTC_DCHECK_EQ(rtp_streams_.size(), active_modules.size()); active_ = false; for (size_t i = 0; i < active_modules.size(); ++i) { @@ -514,6 +522,7 @@ void RtpVideoSender::SetActiveModulesLocked( } bool RtpVideoSender::IsActive() { + RTC_DCHECK_RUN_ON(&transport_checker_); MutexLock lock(&mutex_); return IsActiveLocked(); } @@ -622,6 +631,7 @@ EncodedImageCallback::Result RtpVideoSender::OnEncodedImage( void RtpVideoSender::OnBitrateAllocationUpdated( const VideoBitrateAllocation& bitrate) { + RTC_DCHECK_RUN_ON(&transport_checker_); MutexLock lock(&mutex_); if (IsActiveLocked()) { if (rtp_streams_.size() == 1) { diff --git a/call/rtp_video_sender.h b/call/rtp_video_sender.h index 7e5de98763..25ef20f0a9 100644 --- a/call/rtp_video_sender.h +++ b/call/rtp_video_sender.h @@ -170,7 +170,11 @@ class RtpVideoSender : public RtpVideoSenderInterface, const bool has_packet_feedback_; const bool simulate_generic_structure_; - // TODO(holmer): Remove mutex_ once RtpVideoSender runs on the + // Semantically equivalent to checking for `transport_->GetWorkerQueue()` + // but some tests need to be updated to call from the correct context. + RTC_NO_UNIQUE_ADDRESS SequenceChecker transport_checker_; + + // TODO(bugs.webrtc.org/13517): Remove mutex_ once RtpVideoSender runs on the // transport task queue. mutable Mutex mutex_; bool active_ RTC_GUARDED_BY(mutex_);