From f94bddf72cefe1abd30b54474d774ff08dfd181e Mon Sep 17 00:00:00 2001 From: Harald Alvestrand Date: Sun, 12 Jan 2025 13:44:44 +0000 Subject: [PATCH] Split PushNewMediaChannelAndDeleteChannel This admits to the fact that a transceiver's channel can't change, it's just either created or deleted. Bug: webrtc:42224170 Change-Id: I9a44bf0c0bace74eda6cdf1a1d6967eb8c697594 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/372380 Reviewed-by: Tomas Gunnarsson Commit-Queue: Harald Alvestrand Cr-Commit-Position: refs/heads/main@{#43713} --- pc/rtp_transceiver.cc | 41 ++++++++++++++++++++++++++--------------- pc/rtp_transceiver.h | 8 +++++--- 2 files changed, 31 insertions(+), 18 deletions(-) diff --git a/pc/rtp_transceiver.cc b/pc/rtp_transceiver.cc index 6f4b5ae20c..788b49145e 100644 --- a/pc/rtp_transceiver.cc +++ b/pc/rtp_transceiver.cc @@ -334,7 +334,7 @@ void RtpTransceiver::SetChannel( SafeTask(std::move(flag), [this]() { OnFirstPacketSent(); })); }); }); - PushNewMediaChannelAndDeleteChannel(nullptr); + PushNewMediaChannel(); RTC_DCHECK_BLOCK_COUNT_NO_MORE_THAN(2); } @@ -358,35 +358,46 @@ void RtpTransceiver::ClearChannel() { }); RTC_DCHECK_BLOCK_COUNT_NO_MORE_THAN(1); - PushNewMediaChannelAndDeleteChannel(std::move(channel_)); + DeleteChannel(); RTC_DCHECK_BLOCK_COUNT_NO_MORE_THAN(2); } -void RtpTransceiver::PushNewMediaChannelAndDeleteChannel( - std::unique_ptr channel_to_delete) { - // The clumsy combination of pushing down media channel and deleting - // the channel is due to the desire to do both things in one Invoke(). - if (!channel_to_delete && senders_.empty() && receivers_.empty()) { +void RtpTransceiver::PushNewMediaChannel() { + RTC_DCHECK(channel_); + if (senders_.empty() && receivers_.empty()) { return; } context()->worker_thread()->BlockingCall([&]() { - // Push down the new media_channel, if any, otherwise clear it. - auto* media_send_channel = - channel_ ? channel_->media_send_channel() : nullptr; + // Push down the new media_channel. + auto* media_send_channel = channel_->media_send_channel(); for (const auto& sender : senders_) { sender->internal()->SetMediaChannel(media_send_channel); } - auto* media_receive_channel = - channel_ ? channel_->media_receive_channel() : nullptr; + auto* media_receive_channel = channel_->media_receive_channel(); for (const auto& receiver : receivers_) { receiver->internal()->SetMediaChannel(media_receive_channel); } + }); +} - // Destroy the channel, if we had one, now _after_ updating the receivers - // who might have had references to the previous channel. - channel_to_delete = nullptr; +void RtpTransceiver::DeleteChannel() { + RTC_DCHECK(channel_); + // Ensure that channel_ is not reachable via transceiver, but is deleted + // only after clearing the references in senders_ and receivers_. + context()->worker_thread()->BlockingCall([&]() { + auto channel_to_delete = std::move(channel_); + // Clear the media channel reference from senders and receivers. + for (const auto& sender : senders_) { + sender->internal()->SetMediaChannel(nullptr); + } + for (const auto& receiver : receivers_) { + receiver->internal()->SetMediaChannel(nullptr); + } + // The channel is destroyed here, on the worker thread as it needs to + // be. + channel_to_delete.reset(); }); } diff --git a/pc/rtp_transceiver.h b/pc/rtp_transceiver.h index d9c38c1fce..e544cece34 100644 --- a/pc/rtp_transceiver.h +++ b/pc/rtp_transceiver.h @@ -304,10 +304,12 @@ class RtpTransceiver : public RtpTransceiverInterface { void OnFirstPacketReceived(); void OnFirstPacketSent(); void StopSendingAndReceiving(); - // Delete a channel, and ensure that references to its media channel + // Tell the senders and receivers about possibly-new media channels + // in a newly created `channel_`. + void PushNewMediaChannel(); + // Delete `channel_`, and ensure that references to its media channels // are updated before deleting it. - void PushNewMediaChannelAndDeleteChannel( - std::unique_ptr channel_to_delete); + void DeleteChannel(); // Enforce that this object is created, used and destroyed on one thread. TaskQueueBase* const thread_;