Revert "Remove stopped_ from AudioRtpReceiver and VideoRtpReceiver."

This reverts commit bb57e2d7aa9b36843233d1394422f03d12d9c31f.

Reason for revert: Speculative revert to see if this is causing
breakage in Chromium

Bug: chromium:13665

Original change's description:
> Remove `stopped_` from AudioRtpReceiver and VideoRtpReceiver.
>
> This simplifies the logic in these classes a bit, which makes upcoming
> change easier. The `stopped_` flag in these classes was essentially
> the same thing as `media_channel_ == nullptr`, which is what's
> consistently used now for the same checks.
>
> Bug: webrtc:13540
> Change-Id: Ib60bfad9f28d5ddee8a8d5170c3f2a7ef017a5ca
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/250163
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#35907}

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: webrtc:13540
Change-Id: I67fb2c26b6931b80e3aab749443122d62a82855d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/251141
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Auto-Submit: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35938}

pc/BUILD.gn

@@ -1092,7 +1092,6 @@ 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" ]

pc/audio_rtp_receiver.cc

@@ -61,6 +61,7 @@ AudioRtpReceiver::AudioRtpReceiver(
 
 AudioRtpReceiver::~AudioRtpReceiver() {
   RTC_DCHECK_RUN_ON(&signaling_thread_checker_);
+  RTC_DCHECK(stopped_);
   RTC_DCHECK(!media_channel_);
 
   track_->GetSource()->UnregisterAudioObserver(this);
@@ -84,10 +85,6 @@ 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);
 }
@@ -97,10 +94,13 @@ 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,7 +160,10 @@ 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.
-  source_->SetState(MediaSourceInterface::kEnded);
+  if (!stopped_) {
+    source_->SetState(MediaSourceInterface::kEnded);
+    stopped_ = true;
+  }
 
   worker_thread_->Invoke<void>(RTC_FROM_HERE, [&]() {
     RTC_DCHECK_RUN_ON(worker_thread_);
@@ -180,17 +183,22 @@ void AudioRtpReceiver::StopAndEndTrack() {
 
 void AudioRtpReceiver::RestartMediaChannel(absl::optional<uint32_t> ssrc) {
   RTC_DCHECK_RUN_ON(&signaling_thread_checker_);
-  MediaSourceInterface::SourceState state = source_->state();
-  worker_thread_->Invoke<void>(
-      RTC_FROM_HERE,
-      [&, enabled = cached_track_enabled_, volume = cached_volume_]() {
+  bool ok = worker_thread_->Invoke<bool>(
+      RTC_FROM_HERE, [&, enabled = cached_track_enabled_,
+                      volume = cached_volume_, was_stopped = stopped_]() {
         RTC_DCHECK_RUN_ON(worker_thread_);
-        if (!media_channel_)
-          return;  // Can't restart.
+        if (!media_channel_) {
+          RTC_DCHECK(was_stopped);
+          return false;  // Can't restart.
+        }
 
-        if (state != MediaSourceInterface::kInitializing) {
-          if (ssrc_ == ssrc)
-            return;
+        if (!was_stopped && ssrc_ == ssrc) {
+          // Already running with that ssrc.
+          RTC_DCHECK(worker_thread_safety_->alive());
+          return true;
+        }
+
+        if (!was_stopped) {
           source_->Stop(media_channel_, ssrc_);
         }
 
@@ -201,8 +209,13 @@ void AudioRtpReceiver::RestartMediaChannel(absl::optional<uint32_t> ssrc) {
         }
 
         Reconfigure(enabled, volume);
+        return true;
       });
-  source_->SetState(MediaSourceInterface::kLive);
+
+  if (!ok)
+    return;
+
+  stopped_ = false;
 }
 
 void AudioRtpReceiver::SetupMediaChannel(uint32_t ssrc) {
@@ -322,6 +335,9 @@ 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<void>(RTC_FROM_HERE, [&] {
     RTC_DCHECK_RUN_ON(worker_thread_);
     SetMediaChannel_w(media_channel);

pc/audio_rtp_receiver.h

@@ -133,6 +133,7 @@ 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_) =

pc/audio_rtp_receiver_unittest.cc

@@ -63,7 +63,6 @@ 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))

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::kInitializing) {
+      state_(MediaSourceInterface::kLive) {
   RTC_DCHECK(main_thread_);
   RTC_DCHECK(worker_thread_);
 }

pc/video_rtp_receiver.cc

@@ -49,11 +49,12 @@ VideoRtpReceiver::VideoRtpReceiver(
       attachment_id_(GenerateUniqueId()) {
   RTC_DCHECK(worker_thread_);
   SetStreams(streams);
-  RTC_DCHECK_EQ(source_->state(), MediaSourceInterface::kInitializing);
+  RTC_DCHECK_EQ(source_->state(), MediaSourceInterface::kLive);
 }
 
 VideoRtpReceiver::~VideoRtpReceiver() {
   RTC_DCHECK_RUN_ON(&signaling_thread_checker_);
+  RTC_DCHECK(stopped_);
   RTC_DCHECK(!media_channel_);
 }
 
@@ -115,7 +116,10 @@ void VideoRtpReceiver::Stop() {
   RTC_DCHECK_RUN_ON(&signaling_thread_checker_);
   // TODO(deadbeef): Need to do more here to fully stop receiving packets.
 
-  source_->SetState(MediaSourceInterface::kEnded);
+  if (!stopped_) {
+    source_->SetState(MediaSourceInterface::kEnded);
+    stopped_ = true;
+  }
 
   worker_thread_->Invoke<void>(RTC_FROM_HERE, [&] {
     RTC_DCHECK_RUN_ON(worker_thread_);
@@ -136,30 +140,34 @@ void VideoRtpReceiver::StopAndEndTrack() {
 void VideoRtpReceiver::RestartMediaChannel(absl::optional<uint32_t> ssrc) {
   RTC_DCHECK_RUN_ON(&signaling_thread_checker_);
 
-  MediaSourceInterface::SourceState state = source_->state();
+  // `stopped_` will be `true` on construction. RestartMediaChannel
+  // can in this case function like "ensure started" and flip `stopped_`
+  // to false.
 
   // TODO(tommi): Can we restart the media channel without blocking?
-  worker_thread_->Invoke<void>(RTC_FROM_HERE, [&] {
+  bool ok = worker_thread_->Invoke<bool>(RTC_FROM_HERE, [&, was_stopped =
+                                                                stopped_] {
     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.
-      return;  // Can't restart.
+      RTC_DCHECK(was_stopped);
+      return false;  // Can't restart.
     }
 
-    const bool encoded_sink_enabled = saved_encoded_sink_enabled_;
+    if (!was_stopped && ssrc_ == ssrc) {
+      // Already running with that ssrc.
+      return true;
+    }
 
-    if (state != MediaSourceInterface::kInitializing) {
-      if (ssrc == ssrc_)
-        return;
-
-      // Disconnect from a previous ssrc.
+    // Disconnect from the previous ssrc.
+    if (!was_stopped) {
       SetSink(nullptr);
-
-      if (encoded_sink_enabled)
-        SetEncodedSinkEnabled(false);
     }
 
+    bool encoded_sink_enabled = saved_encoded_sink_enabled_;
+    SetEncodedSinkEnabled(false);
+
     // Set up the new ssrc.
     ssrc_ = std::move(ssrc);
     SetSink(source_->sink());
@@ -179,8 +187,14 @@ void VideoRtpReceiver::RestartMediaChannel(absl::optional<uint32_t> ssrc) {
 
       media_channel_->SetBaseMinimumPlayoutDelayMs(*ssrc_, delay_.GetMs());
     }
+
+    return true;
   });
-  source_->SetState(MediaSourceInterface::kLive);
+
+  if (!ok)
+    return;
+
+  stopped_ = false;
 }
 
 // RTC_RUN_ON(worker_thread_)
@@ -274,6 +288,9 @@ 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<void>(RTC_FROM_HERE, [&] {
     RTC_DCHECK_RUN_ON(worker_thread_);
     SetMediaChannel_w(media_channel);

pc/video_rtp_receiver.h

@@ -141,6 +141,8 @@ 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<uint32_t> ssrc_ RTC_GUARDED_BY(worker_thread_);
@@ -150,6 +152,15 @@ class VideoRtpReceiver : public RtpReceiverInternal {
   const rtc::scoped_refptr<VideoTrackProxyWithInternal<VideoTrack>> track_;
   std::vector<rtc::scoped_refptr<MediaStreamInterface>> 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_) =

pc/video_rtp_receiver_unittest.cc

@@ -169,6 +169,7 @@ 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, _));

pc/video_track_source.cc

@@ -15,12 +15,11 @@
 namespace webrtc {
 
 VideoTrackSource::VideoTrackSource(bool remote)
-    : state_(kInitializing), remote_(remote) {
+    : state_(kLive), 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();

pc/video_track_source.h

@@ -20,7 +20,6 @@
 #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 {
@@ -32,10 +31,7 @@ class RTC_EXPORT VideoTrackSource : public Notifier<VideoTrackSourceInterface> {
   explicit VideoTrackSource(bool remote);
   void SetState(SourceState new_state);
 
-  SourceState state() const override {
-    RTC_DCHECK_RUN_ON(&signaling_thread_checker_);
-    return state_;
-  }
+  SourceState state() const override { return state_; }
   bool remote() const override { return remote_; }
 
   bool is_screencast() const override { return false; }
@@ -60,9 +56,8 @@ class RTC_EXPORT VideoTrackSource : public Notifier<VideoTrackSourceInterface> {
   virtual rtc::VideoSourceInterface<VideoFrame>* source() = 0;
 
  private:
-  RTC_NO_UNIQUE_ADDRESS SequenceChecker worker_thread_checker_;
-  RTC_NO_UNIQUE_ADDRESS SequenceChecker signaling_thread_checker_;
-  SourceState state_ RTC_GUARDED_BY(&signaling_thread_checker_);
+  SequenceChecker worker_thread_checker_;
+  SourceState state_;
   const bool remote_;
 };