Align sender/receiver teardown in RtpTransceiver.
This makes SetChannel() consistently make 2 invokes instead of a
multiple of senders+receivers (previous minimum was 4 but could be
larger).
* Stop() doesn't hop to the worker thread.
* SetMediaChannel(), an already-required step on the worker thread for
senders and *sometimes* for receivers[1], is now consistently required
for both. This simplifies transceiver teardown and enables the next
bullet.
* Transceiver stops all senders and receivers in one go rather than
ping ponging between threads.
[1] When not required, it was done implicitly inside of Stop().
See changes in `RtpTransceiver::SetChannel`
Bug: webrtc:13540
Change-Id: Ied61636c8ef09d782bf519524fff2a31e15219a8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/249797
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36057}
diff --git a/pc/audio_rtp_receiver.cc b/pc/audio_rtp_receiver.cc
index a49b7ce..43294c7 100644
--- a/pc/audio_rtp_receiver.cc
+++ b/pc/audio_rtp_receiver.cc
@@ -25,20 +25,24 @@
namespace webrtc {
-AudioRtpReceiver::AudioRtpReceiver(rtc::Thread* worker_thread,
- std::string receiver_id,
- std::vector<std::string> stream_ids,
- bool is_unified_plan)
+AudioRtpReceiver::AudioRtpReceiver(
+ rtc::Thread* worker_thread,
+ std::string receiver_id,
+ std::vector<std::string> stream_ids,
+ bool is_unified_plan,
+ cricket::VoiceMediaChannel* voice_channel /*= nullptr*/)
: AudioRtpReceiver(worker_thread,
receiver_id,
CreateStreamsFromIds(std::move(stream_ids)),
- is_unified_plan) {}
+ is_unified_plan,
+ voice_channel) {}
AudioRtpReceiver::AudioRtpReceiver(
rtc::Thread* worker_thread,
const std::string& receiver_id,
const std::vector<rtc::scoped_refptr<MediaStreamInterface>>& streams,
- bool is_unified_plan)
+ bool is_unified_plan,
+ cricket::VoiceMediaChannel* voice_channel /*= nullptr*/)
: worker_thread_(worker_thread),
id_(receiver_id),
source_(rtc::make_ref_counted<RemoteAudioSource>(
@@ -49,7 +53,8 @@
track_(AudioTrackProxyWithInternal<AudioTrack>::Create(
rtc::Thread::Current(),
AudioTrack::Create(receiver_id, source_))),
- cached_track_enabled_(track_->enabled()),
+ media_channel_(voice_channel),
+ cached_track_enabled_(track_->internal()->enabled()),
attachment_id_(GenerateUniqueId()),
worker_thread_safety_(PendingTaskSafetyFlag::CreateDetachedInactive()) {
RTC_DCHECK(worker_thread_);
@@ -69,15 +74,15 @@
void AudioRtpReceiver::OnChanged() {
RTC_DCHECK_RUN_ON(&signaling_thread_checker_);
- if (cached_track_enabled_ != track_->enabled()) {
- cached_track_enabled_ = track_->enabled();
- worker_thread_->PostTask(ToQueuedTask(
- worker_thread_safety_,
- [this, enabled = cached_track_enabled_, volume = cached_volume_]() {
- RTC_DCHECK_RUN_ON(worker_thread_);
- Reconfigure(enabled, volume);
- }));
- }
+ const bool enabled = track_->internal()->enabled();
+ if (cached_track_enabled_ == enabled)
+ return;
+ cached_track_enabled_ = enabled;
+ worker_thread_->PostTask(
+ ToQueuedTask(worker_thread_safety_, [this, enabled]() {
+ RTC_DCHECK_RUN_ON(worker_thread_);
+ Reconfigure(enabled);
+ }));
}
// RTC_RUN_ON(worker_thread_)
@@ -97,20 +102,18 @@
RTC_DCHECK_GE(volume, 0);
RTC_DCHECK_LE(volume, 10);
- // Update the cached_volume_ even when stopped, to allow clients to set the
- // volume before starting/restarting, eg see crbug.com/1272566.
- cached_volume_ = volume;
-
- // When the track is disabled, the volume of the source, which is the
- // corresponding WebRtc Voice Engine channel will be 0. So we do not allow
- // setting the volume to the source when the track is disabled.
- if (track_->enabled()) {
- worker_thread_->PostTask(
- ToQueuedTask(worker_thread_safety_, [this, volume = cached_volume_]() {
- RTC_DCHECK_RUN_ON(worker_thread_);
- SetOutputVolume_w(volume);
- }));
- }
+ bool track_enabled = track_->internal()->enabled();
+ worker_thread_->Invoke<void>(RTC_FROM_HERE, [&]() {
+ RTC_DCHECK_RUN_ON(worker_thread_);
+ // Update the cached_volume_ even when stopped, to allow clients to set
+ // the volume before starting/restarting, eg see crbug.com/1272566.
+ cached_volume_ = volume;
+ // When the track is disabled, the volume of the source, which is the
+ // corresponding WebRtc Voice Engine channel will be 0. So we do not
+ // allow setting the volume to the source when the track is disabled.
+ if (track_enabled)
+ SetOutputVolume_w(volume);
+ });
}
rtc::scoped_refptr<DtlsTransportInterface> AudioRtpReceiver::dtls_transport()
@@ -159,52 +162,49 @@
void AudioRtpReceiver::Stop() {
RTC_DCHECK_RUN_ON(&signaling_thread_checker_);
- // TODO(deadbeef): Need to do more here to fully stop receiving packets.
source_->SetState(MediaSourceInterface::kEnded);
-
- worker_thread_->Invoke<void>(RTC_FROM_HERE, [&]() {
- RTC_DCHECK_RUN_ON(worker_thread_);
-
- if (media_channel_)
- SetOutputVolume_w(0.0);
-
- SetMediaChannel_w(nullptr);
- });
-}
-
-void AudioRtpReceiver::StopAndEndTrack() {
- RTC_DCHECK_RUN_ON(&signaling_thread_checker_);
- Stop();
track_->internal()->set_ended();
}
-void AudioRtpReceiver::RestartMediaChannel(absl::optional<uint32_t> ssrc) {
+void AudioRtpReceiver::SetSourceEnded() {
RTC_DCHECK_RUN_ON(&signaling_thread_checker_);
+ source_->SetState(MediaSourceInterface::kEnded);
+}
+
+// RTC_RUN_ON(&signaling_thread_checker_)
+void AudioRtpReceiver::RestartMediaChannel(absl::optional<uint32_t> ssrc) {
+ bool enabled = track_->internal()->enabled();
MediaSourceInterface::SourceState state = source_->state();
- worker_thread_->Invoke<void>(
- RTC_FROM_HERE,
- [&, enabled = cached_track_enabled_, volume = cached_volume_]() {
- RTC_DCHECK_RUN_ON(worker_thread_);
- if (!media_channel_)
- return; // Can't restart.
-
- if (state != MediaSourceInterface::kInitializing) {
- if (ssrc_ == ssrc)
- return;
- source_->Stop(media_channel_, ssrc_);
- }
-
- ssrc_ = std::move(ssrc);
- source_->Start(media_channel_, ssrc_);
- if (ssrc_) {
- media_channel_->SetBaseMinimumPlayoutDelayMs(*ssrc_, delay_.GetMs());
- }
-
- Reconfigure(enabled, volume);
- });
+ worker_thread_->Invoke<void>(RTC_FROM_HERE, [&]() {
+ RTC_DCHECK_RUN_ON(worker_thread_);
+ RestartMediaChannel_w(std::move(ssrc), enabled, state);
+ });
source_->SetState(MediaSourceInterface::kLive);
}
+// RTC_RUN_ON(worker_thread_)
+void AudioRtpReceiver::RestartMediaChannel_w(
+ absl::optional<uint32_t> ssrc,
+ bool track_enabled,
+ MediaSourceInterface::SourceState state) {
+ if (!media_channel_)
+ return; // Can't restart.
+
+ if (state != MediaSourceInterface::kInitializing) {
+ if (ssrc_ == ssrc)
+ return;
+ source_->Stop(media_channel_, ssrc_);
+ }
+
+ ssrc_ = std::move(ssrc);
+ source_->Start(media_channel_, ssrc_);
+ if (ssrc_) {
+ media_channel_->SetBaseMinimumPlayoutDelayMs(*ssrc_, delay_.GetMs());
+ }
+
+ Reconfigure(track_enabled);
+}
+
void AudioRtpReceiver::SetupMediaChannel(uint32_t ssrc) {
RTC_DCHECK_RUN_ON(&signaling_thread_checker_);
RestartMediaChannel(ssrc);
@@ -284,10 +284,10 @@
}
// RTC_RUN_ON(worker_thread_)
-void AudioRtpReceiver::Reconfigure(bool track_enabled, double volume) {
+void AudioRtpReceiver::Reconfigure(bool track_enabled) {
RTC_DCHECK(media_channel_);
- SetOutputVolume_w(track_enabled ? volume : 0);
+ SetOutputVolume_w(track_enabled ? cached_volume_ : 0);
if (ssrc_ && frame_decryptor_) {
// Reattach the frame decryptor if we were reconfigured.
@@ -318,18 +318,12 @@
}
void AudioRtpReceiver::SetMediaChannel(cricket::MediaChannel* media_channel) {
- RTC_DCHECK_RUN_ON(&signaling_thread_checker_);
+ RTC_DCHECK_RUN_ON(worker_thread_);
RTC_DCHECK(media_channel == nullptr ||
media_channel->media_type() == media_type());
+ if (!media_channel && media_channel_)
+ SetOutputVolume_w(0.0);
- worker_thread_->Invoke<void>(RTC_FROM_HERE, [&] {
- RTC_DCHECK_RUN_ON(worker_thread_);
- SetMediaChannel_w(media_channel);
- });
-}
-
-// RTC_RUN_ON(worker_thread_)
-void AudioRtpReceiver::SetMediaChannel_w(cricket::MediaChannel* media_channel) {
media_channel ? worker_thread_safety_->SetAlive()
: worker_thread_safety_->SetNotAlive();
media_channel_ = static_cast<cricket::VoiceMediaChannel*>(media_channel);
