From f643aea8ac6fb6dc6c75cc5cfbb6f08b96e3b98b Mon Sep 17 00:00:00 2001 From: Tomas Gunnarsson Date: Sun, 2 Jan 2022 15:57:28 +0000 Subject: [PATCH] Updating OnDemuxerCriteria* notifications to be on the same thread. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This makes things slightly simpler for the time being as surrounding code is being refactored. This also removes a PostTask which has the effect of shrinking the window between the Pending/Complete notifications slightly since there's no additional async task for the 'complete' step. Bug: webrtc:11993 Change-Id: Ia86779b21c6f87301f37d763f89ace722e06e563 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/244081 Auto-Submit: Tomas Gunnarsson Reviewed-by: Henrik Boström Commit-Queue: Henrik Boström Cr-Commit-Position: refs/heads/main@{#35609} --- media/base/media_channel.h | 9 ++++++--- media/engine/webrtc_video_engine.cc | 7 ++----- pc/channel.cc | 7 +++++-- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/media/base/media_channel.h b/media/base/media_channel.h index 89f316e09d..e7684913e9 100644 --- a/media/base/media_channel.h +++ b/media/base/media_channel.h @@ -208,11 +208,14 @@ class MediaChannel { // Resets any cached StreamParams for an unsignaled RecvStream, and removes // any existing unsignaled streams. virtual void ResetUnsignaledRecvStream() = 0; - // Informs the media channel when the transport's demuxer criteria is updated. + // This is currently a workaround because of the demuxer state being managed + // across two separate threads. Once the state is consistently managed on + // the same thread (network), this workaround can be removed. + // These two notifications inform the media channel when the transport's + // demuxer criteria is being updated. // * OnDemuxerCriteriaUpdatePending() happens on the same thread that the // channel's streams are added and removed (worker thread). - // * OnDemuxerCriteriaUpdateComplete() happens on the thread where the demuxer - // lives (network thread). + // * OnDemuxerCriteriaUpdateComplete() happens on the same thread. // Because the demuxer is updated asynchronously, there is a window of time // where packets are arriving to the channel for streams that have already // been removed on the worker thread. It is important NOT to treat these as diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index dcfa0b52e3..7f3cf92f83 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -1593,11 +1593,8 @@ void WebRtcVideoChannel::OnDemuxerCriteriaUpdatePending() { } void WebRtcVideoChannel::OnDemuxerCriteriaUpdateComplete() { - RTC_DCHECK_RUN_ON(&network_thread_checker_); - worker_thread_->PostTask(ToQueuedTask(task_safety_, [this] { - RTC_DCHECK_RUN_ON(&thread_checker_); - ++demuxer_criteria_completed_id_; - })); + RTC_DCHECK_RUN_ON(&thread_checker_); + ++demuxer_criteria_completed_id_; } bool WebRtcVideoChannel::SetSink( diff --git a/pc/channel.cc b/pc/channel.cc index 63fa72ce90..7b6c0c8704 100644 --- a/pc/channel.cc +++ b/pc/channel.cc @@ -499,7 +499,7 @@ bool BaseChannel::RegisterRtpDemuxerSink_w() { media_channel_->OnDemuxerCriteriaUpdatePending(); // Copy demuxer criteria, since they're a worker-thread variable // and we want to pass them to the network thread - return network_thread_->Invoke( + bool ret = network_thread_->Invoke( RTC_FROM_HERE, [this, demuxer_criteria = demuxer_criteria_] { RTC_DCHECK_RUN_ON(network_thread()); RTC_DCHECK(rtp_transport_); @@ -510,9 +510,12 @@ bool BaseChannel::RegisterRtpDemuxerSink_w() { } else { previous_demuxer_criteria_ = {}; } - media_channel_->OnDemuxerCriteriaUpdateComplete(); return result; }); + + media_channel_->OnDemuxerCriteriaUpdateComplete(); + + return ret; } void BaseChannel::EnableMedia_w() {