diff --git a/pc/BUILD.gn b/pc/BUILD.gn index 7fcb046904..ba0586755e 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -1416,6 +1416,7 @@ rtc_library("video_track_source") { "../media:rtc_media_base", "../rtc_base:checks", "../rtc_base:rtc_base_approved", + "../rtc_base/system:no_unique_address", "../rtc_base/system:rtc_export", ] absl_deps = [ "//third_party/abseil-cpp/absl/types:optional" ] diff --git a/pc/audio_rtp_receiver.cc b/pc/audio_rtp_receiver.cc index 7890d9b1e0..a49b7ce48e 100644 --- a/pc/audio_rtp_receiver.cc +++ b/pc/audio_rtp_receiver.cc @@ -61,7 +61,6 @@ AudioRtpReceiver::AudioRtpReceiver( AudioRtpReceiver::~AudioRtpReceiver() { RTC_DCHECK_RUN_ON(&signaling_thread_checker_); - RTC_DCHECK(stopped_); RTC_DCHECK(!media_channel_); track_->GetSource()->UnregisterAudioObserver(this); @@ -85,6 +84,10 @@ void AudioRtpReceiver::OnChanged() { void AudioRtpReceiver::SetOutputVolume_w(double volume) { RTC_DCHECK_GE(volume, 0.0); RTC_DCHECK_LE(volume, 10.0); + + if (!media_channel_) + return; + ssrc_ ? media_channel_->SetOutputVolume(*ssrc_, volume) : media_channel_->SetDefaultOutputVolume(volume); } @@ -94,13 +97,10 @@ void AudioRtpReceiver::OnSetVolume(double volume) { RTC_DCHECK_GE(volume, 0); RTC_DCHECK_LE(volume, 10); - // Update the cached_volume_ even when stopped_, to allow clients to set the + // 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; - if (stopped_) - return; - // 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. @@ -160,10 +160,7 @@ AudioRtpReceiver::GetFrameDecryptor() const { void AudioRtpReceiver::Stop() { RTC_DCHECK_RUN_ON(&signaling_thread_checker_); // TODO(deadbeef): Need to do more here to fully stop receiving packets. - if (!stopped_) { - source_->SetState(MediaSourceInterface::kEnded); - stopped_ = true; - } + source_->SetState(MediaSourceInterface::kEnded); worker_thread_->Invoke(RTC_FROM_HERE, [&]() { RTC_DCHECK_RUN_ON(worker_thread_); @@ -183,22 +180,17 @@ void AudioRtpReceiver::StopAndEndTrack() { void AudioRtpReceiver::RestartMediaChannel(absl::optional ssrc) { RTC_DCHECK_RUN_ON(&signaling_thread_checker_); - bool ok = worker_thread_->Invoke( - RTC_FROM_HERE, [&, enabled = cached_track_enabled_, - volume = cached_volume_, was_stopped = stopped_]() { + MediaSourceInterface::SourceState state = source_->state(); + worker_thread_->Invoke( + RTC_FROM_HERE, + [&, enabled = cached_track_enabled_, volume = cached_volume_]() { RTC_DCHECK_RUN_ON(worker_thread_); - if (!media_channel_) { - RTC_DCHECK(was_stopped); - return false; // Can't restart. - } + if (!media_channel_) + return; // Can't restart. - if (!was_stopped && ssrc_ == ssrc) { - // Already running with that ssrc. - RTC_DCHECK(worker_thread_safety_->alive()); - return true; - } - - if (!was_stopped) { + if (state != MediaSourceInterface::kInitializing) { + if (ssrc_ == ssrc) + return; source_->Stop(media_channel_, ssrc_); } @@ -209,13 +201,8 @@ void AudioRtpReceiver::RestartMediaChannel(absl::optional ssrc) { } Reconfigure(enabled, volume); - return true; }); - - if (!ok) - return; - - stopped_ = false; + source_->SetState(MediaSourceInterface::kLive); } void AudioRtpReceiver::SetupMediaChannel(uint32_t ssrc) { @@ -335,9 +322,6 @@ void AudioRtpReceiver::SetMediaChannel(cricket::MediaChannel* media_channel) { RTC_DCHECK(media_channel == nullptr || media_channel->media_type() == media_type()); - if (stopped_ && !media_channel) - return; - worker_thread_->Invoke(RTC_FROM_HERE, [&] { RTC_DCHECK_RUN_ON(worker_thread_); SetMediaChannel_w(media_channel); diff --git a/pc/audio_rtp_receiver.h b/pc/audio_rtp_receiver.h index aef497db76..978c550dfe 100644 --- a/pc/audio_rtp_receiver.h +++ b/pc/audio_rtp_receiver.h @@ -133,7 +133,6 @@ class AudioRtpReceiver : public ObserverInterface, RTC_GUARDED_BY(&signaling_thread_checker_); bool cached_track_enabled_ RTC_GUARDED_BY(&signaling_thread_checker_); double cached_volume_ RTC_GUARDED_BY(&signaling_thread_checker_) = 1.0; - bool stopped_ RTC_GUARDED_BY(&signaling_thread_checker_) = true; RtpReceiverObserverInterface* observer_ RTC_GUARDED_BY(&signaling_thread_checker_) = nullptr; bool received_first_packet_ RTC_GUARDED_BY(&signaling_thread_checker_) = diff --git a/pc/audio_rtp_receiver_unittest.cc b/pc/audio_rtp_receiver_unittest.cc index 1865115b2e..763677b046 100644 --- a/pc/audio_rtp_receiver_unittest.cc +++ b/pc/audio_rtp_receiver_unittest.cc @@ -63,6 +63,7 @@ TEST_F(AudioRtpReceiverTest, SetOutputVolumeIsCalled) { receiver_->track(); receiver_->track()->set_enabled(true); receiver_->SetMediaChannel(&media_channel_); + EXPECT_CALL(media_channel_, SetDefaultRawAudioSink(_)).Times(0); receiver_->SetupMediaChannel(kSsrc); EXPECT_CALL(media_channel_, SetOutputVolume(kSsrc, kVolume)) diff --git a/pc/remote_audio_source.cc b/pc/remote_audio_source.cc index dc890e737c..78a35f32a8 100644 --- a/pc/remote_audio_source.cc +++ b/pc/remote_audio_source.cc @@ -55,7 +55,7 @@ RemoteAudioSource::RemoteAudioSource( : main_thread_(rtc::Thread::Current()), worker_thread_(worker_thread), on_audio_channel_gone_action_(on_audio_channel_gone_action), - state_(MediaSourceInterface::kLive) { + state_(MediaSourceInterface::kInitializing) { RTC_DCHECK(main_thread_); RTC_DCHECK(worker_thread_); } @@ -134,11 +134,6 @@ void RemoteAudioSource::AddSink(AudioTrackSinkInterface* sink) { RTC_DCHECK_RUN_ON(main_thread_); RTC_DCHECK(sink); - if (state_ != MediaSourceInterface::kLive) { - RTC_LOG(LS_ERROR) << "Can't register sink as the source isn't live."; - return; - } - MutexLock lock(&sink_lock_); RTC_DCHECK(!absl::c_linear_search(sinks_, sink)); sinks_.push_back(sink); diff --git a/pc/video_rtp_receiver.cc b/pc/video_rtp_receiver.cc index 8db4d9f02f..c5ead9e773 100644 --- a/pc/video_rtp_receiver.cc +++ b/pc/video_rtp_receiver.cc @@ -49,12 +49,11 @@ VideoRtpReceiver::VideoRtpReceiver( attachment_id_(GenerateUniqueId()) { RTC_DCHECK(worker_thread_); SetStreams(streams); - RTC_DCHECK_EQ(source_->state(), MediaSourceInterface::kLive); + RTC_DCHECK_EQ(source_->state(), MediaSourceInterface::kInitializing); } VideoRtpReceiver::~VideoRtpReceiver() { RTC_DCHECK_RUN_ON(&signaling_thread_checker_); - RTC_DCHECK(stopped_); RTC_DCHECK(!media_channel_); } @@ -116,10 +115,7 @@ void VideoRtpReceiver::Stop() { RTC_DCHECK_RUN_ON(&signaling_thread_checker_); // TODO(deadbeef): Need to do more here to fully stop receiving packets. - if (!stopped_) { - source_->SetState(MediaSourceInterface::kEnded); - stopped_ = true; - } + source_->SetState(MediaSourceInterface::kEnded); worker_thread_->Invoke(RTC_FROM_HERE, [&] { RTC_DCHECK_RUN_ON(worker_thread_); @@ -140,33 +136,29 @@ void VideoRtpReceiver::StopAndEndTrack() { void VideoRtpReceiver::RestartMediaChannel(absl::optional ssrc) { RTC_DCHECK_RUN_ON(&signaling_thread_checker_); - // `stopped_` will be `true` on construction. RestartMediaChannel - // can in this case function like "ensure started" and flip `stopped_` - // to false. + MediaSourceInterface::SourceState state = source_->state(); // TODO(tommi): Can we restart the media channel without blocking? - bool ok = worker_thread_->Invoke(RTC_FROM_HERE, [&, was_stopped = - stopped_] { + worker_thread_->Invoke(RTC_FROM_HERE, [&] { RTC_DCHECK_RUN_ON(worker_thread_); if (!media_channel_) { // Ignore further negotiations if we've already been stopped and don't // have an associated media channel. - RTC_DCHECK(was_stopped); - return false; // Can't restart. + return; // Can't restart. } - if (!was_stopped && ssrc_ == ssrc) { - // Already running with that ssrc. - return true; - } + const bool encoded_sink_enabled = saved_encoded_sink_enabled_; - // Disconnect from the previous ssrc. - if (!was_stopped) { + if (state != MediaSourceInterface::kInitializing) { + if (ssrc == ssrc_) + return; + + // Disconnect from a previous ssrc. SetSink(nullptr); - } - bool encoded_sink_enabled = saved_encoded_sink_enabled_; - SetEncodedSinkEnabled(false); + if (encoded_sink_enabled) + SetEncodedSinkEnabled(false); + } // Set up the new ssrc. ssrc_ = std::move(ssrc); @@ -187,14 +179,8 @@ void VideoRtpReceiver::RestartMediaChannel(absl::optional ssrc) { media_channel_->SetBaseMinimumPlayoutDelayMs(*ssrc_, delay_.GetMs()); } - - return true; }); - - if (!ok) - return; - - stopped_ = false; + source_->SetState(MediaSourceInterface::kLive); } // RTC_RUN_ON(worker_thread_) @@ -288,9 +274,6 @@ void VideoRtpReceiver::SetMediaChannel(cricket::MediaChannel* media_channel) { RTC_DCHECK(media_channel == nullptr || media_channel->media_type() == media_type()); - if (stopped_ && !media_channel) - return; - worker_thread_->Invoke(RTC_FROM_HERE, [&] { RTC_DCHECK_RUN_ON(worker_thread_); SetMediaChannel_w(media_channel); diff --git a/pc/video_rtp_receiver.h b/pc/video_rtp_receiver.h index b5381860b3..681f423e29 100644 --- a/pc/video_rtp_receiver.h +++ b/pc/video_rtp_receiver.h @@ -141,8 +141,6 @@ class VideoRtpReceiver : public RtpReceiverInternal { rtc::Thread* const worker_thread_; const std::string id_; - // See documentation for `stopped_` below for when a valid media channel - // has been assigned and when this pointer will be null. cricket::VideoMediaChannel* media_channel_ RTC_GUARDED_BY(worker_thread_) = nullptr; absl::optional ssrc_ RTC_GUARDED_BY(worker_thread_); @@ -152,15 +150,6 @@ class VideoRtpReceiver : public RtpReceiverInternal { const rtc::scoped_refptr> track_; std::vector> streams_ RTC_GUARDED_BY(&signaling_thread_checker_); - // `stopped` is state that's used on the signaling thread to indicate whether - // a valid `media_channel_` has been assigned and configured. When an instance - // of VideoRtpReceiver is initially created, `stopped_` is true and will - // remain true until either `SetupMediaChannel` or - // `SetupUnsignaledMediaChannel` is called after assigning a media channel. - // After that, `stopped_` will remain false until `Stop()` is called. - // Note, for checking the state of the class on the worker thread, - // check `media_channel_` instead, as that's the main worker thread state. - bool stopped_ RTC_GUARDED_BY(&signaling_thread_checker_) = true; RtpReceiverObserverInterface* observer_ RTC_GUARDED_BY(&signaling_thread_checker_) = nullptr; bool received_first_packet_ RTC_GUARDED_BY(&signaling_thread_checker_) = diff --git a/pc/video_rtp_receiver_unittest.cc b/pc/video_rtp_receiver_unittest.cc index 3a8099d30f..42ff2611ef 100644 --- a/pc/video_rtp_receiver_unittest.cc +++ b/pc/video_rtp_receiver_unittest.cc @@ -169,7 +169,6 @@ TEST_F(VideoRtpReceiverTest, BroadcastsEncodedFramesWhenEnabled) { TEST_F(VideoRtpReceiverTest, EnablesEncodedOutputOnChannelRestart) { InSequence s; - EXPECT_CALL(channel_, ClearRecordableEncodedFrameCallback(0)); MockVideoSink sink; Source()->AddEncodedSink(&sink); EXPECT_CALL(channel_, SetRecordableEncodedFrameCallback(4711, _)); diff --git a/pc/video_track_source.cc b/pc/video_track_source.cc index d15eaaf43c..64e99cc064 100644 --- a/pc/video_track_source.cc +++ b/pc/video_track_source.cc @@ -15,11 +15,12 @@ namespace webrtc { VideoTrackSource::VideoTrackSource(bool remote) - : state_(kLive), remote_(remote) { + : state_(kInitializing), remote_(remote) { worker_thread_checker_.Detach(); } void VideoTrackSource::SetState(SourceState new_state) { + RTC_DCHECK_RUN_ON(&signaling_thread_checker_); if (state_ != new_state) { state_ = new_state; FireOnChanged(); diff --git a/pc/video_track_source.h b/pc/video_track_source.h index 4a29381c4c..3f568f642b 100644 --- a/pc/video_track_source.h +++ b/pc/video_track_source.h @@ -20,6 +20,7 @@ #include "api/video/video_sink_interface.h" #include "api/video/video_source_interface.h" #include "media/base/media_channel.h" +#include "rtc_base/system/no_unique_address.h" #include "rtc_base/system/rtc_export.h" namespace webrtc { @@ -31,7 +32,10 @@ class RTC_EXPORT VideoTrackSource : public Notifier { explicit VideoTrackSource(bool remote); void SetState(SourceState new_state); - SourceState state() const override { return state_; } + SourceState state() const override { + RTC_DCHECK_RUN_ON(&signaling_thread_checker_); + return state_; + } bool remote() const override { return remote_; } bool is_screencast() const override { return false; } @@ -56,8 +60,9 @@ class RTC_EXPORT VideoTrackSource : public Notifier { virtual rtc::VideoSourceInterface* source() = 0; private: - SequenceChecker worker_thread_checker_; - SourceState state_; + RTC_NO_UNIQUE_ADDRESS SequenceChecker worker_thread_checker_; + RTC_NO_UNIQUE_ADDRESS SequenceChecker signaling_thread_checker_; + SourceState state_ RTC_GUARDED_BY(&signaling_thread_checker_); const bool remote_; };