Reland of Change VideoTrack implementation to invoke VideoTrackSourceInterface::AddOrUpdateSink on wt

Added documentation of thread expectations for video tracks and sources to the API.

Originally landed as patchset #2 id:20001 of https://codereview.webrtc.org/2964863002/.

Patchset 1 is the originall cl.
Patschet 2 is modified so that VideoTrackInterface::AddSink and RemoveSink have a default implementation.

BUG=none

Review-Url: https://codereview.webrtc.org/2989113002
Cr-Commit-Position: refs/heads/master@{#19195}

webrtc/api/mediastreaminterface.h

@@ -111,6 +111,11 @@ class MediaStreamTrackInterface : public rtc::RefCountInterface,
 
 // VideoTrackSourceInterface is a reference counted source used for
 // VideoTracks. The same source can be used by multiple VideoTracks.
+// VideoTrackSourceInterface is designed to be invoked on the signaling thread
+// except for rtc::VideoSourceInterface<VideoFrame> methods that will be invoked
+// on the worker thread via a VideoTrack. A custom implementation of a source
+// can inherit AdaptedVideoTrackSource instead of directly implementing this
+// interface.
 class VideoTrackSourceInterface
     : public MediaSourceInterface,
       public rtc::VideoSourceInterface<VideoFrame> {
@@ -145,6 +150,12 @@ class VideoTrackSourceInterface
   virtual ~VideoTrackSourceInterface() {}
 };
 
+// VideoTrackInterface is designed to be invoked on the signaling thread except
+// for rtc::VideoSourceInterface<VideoFrame> methods that must be invoked
+// on the worker thread.
+// PeerConnectionFactory::CreateVideoTrack can be used for creating a VideoTrack
+// that ensures thread safety and that all methods are called on the right
+// thread.
 class VideoTrackInterface
     : public MediaStreamTrackInterface,
       public rtc::VideoSourceInterface<VideoFrame> {

webrtc/ortc/ortcfactory.cc

@@ -479,7 +479,8 @@ rtc::scoped_refptr<VideoTrackInterface> OrtcFactory::CreateVideoTrack(
     const std::string& id,
     VideoTrackSourceInterface* source) {
   RTC_DCHECK_RUN_ON(signaling_thread_);
-  rtc::scoped_refptr<VideoTrackInterface> track(VideoTrack::Create(id, source));
+  rtc::scoped_refptr<VideoTrackInterface> track(
+      VideoTrack::Create(id, source, worker_thread_.get()));
   return VideoTrackProxy::Create(signaling_thread_, worker_thread_.get(),
                                  track);
 }

webrtc/pc/mediastream_unittest.cc

@@ -56,8 +56,8 @@ class MediaStreamTest: public testing::Test {
     stream_ = MediaStream::Create(kStreamLabel1);
     ASSERT_TRUE(stream_.get() != NULL);
 
-    video_track_ =
+    video_track_ = VideoTrack::Create(
-        VideoTrack::Create(kVideoTrackId, FakeVideoTrackSource::Create());
+        kVideoTrackId, FakeVideoTrackSource::Create(), rtc::Thread::Current());
     ASSERT_TRUE(video_track_.get() != NULL);
     EXPECT_EQ(MediaStreamTrackInterface::kLive, video_track_->state());
 

webrtc/pc/peerconnectionfactory.cc

@@ -292,7 +292,7 @@ rtc::scoped_refptr<VideoTrackInterface> PeerConnectionFactory::CreateVideoTrack(
     VideoTrackSourceInterface* source) {
   RTC_DCHECK(signaling_thread_->IsCurrent());
   rtc::scoped_refptr<VideoTrackInterface> track(
-      VideoTrack::Create(id, source));
+      VideoTrack::Create(id, source, worker_thread_));
   return VideoTrackProxy::Create(signaling_thread_, worker_thread_, track);
 }
 

webrtc/pc/peerconnectioninterface_unittest.cc

@@ -460,7 +460,8 @@ rtc::scoped_refptr<StreamCollection> CreateStreamCollection(
       // Add a local video track.
       rtc::scoped_refptr<webrtc::VideoTrackInterface> video_track(
           webrtc::VideoTrack::Create(kVideoTracks[i * tracks_per_stream + j],
-                                     webrtc::FakeVideoTrackSource::Create()));
+                                     webrtc::FakeVideoTrackSource::Create(),
+                                     rtc::Thread::Current()));
       stream->AddTrack(video_track);
     }
 
@@ -1150,7 +1151,8 @@ class PeerConnectionInterfaceTest : public testing::Test {
                      MediaStreamInterface* stream) {
     rtc::scoped_refptr<webrtc::VideoTrackInterface> video_track(
         webrtc::VideoTrack::Create(track_id,
-                                   webrtc::FakeVideoTrackSource::Create()));
+                                   webrtc::FakeVideoTrackSource::Create(),
+                                   rtc::Thread::Current()));
     ASSERT_TRUE(stream->AddTrack(video_track));
   }
 

webrtc/pc/rtcstatscollector_unittest.cc

@@ -218,6 +218,11 @@ class FakeVideoTrackForStats
   std::string kind() const override {
     return MediaStreamTrackInterface::kVideoKind;
   }
+
+  void AddOrUpdateSink(rtc::VideoSinkInterface<VideoFrame>* sink,
+                       const rtc::VideoSinkWants& wants) override{};
+  void RemoveSink(rtc::VideoSinkInterface<VideoFrame>* sink) override{};
+
   VideoTrackSourceInterface* GetSource() const override { return nullptr; }
 };
 

webrtc/pc/rtpreceiver.cc

@@ -155,7 +155,8 @@ VideoRtpReceiver::VideoRtpReceiver(const std::string& track_id,
               track_id,
               VideoTrackSourceProxy::Create(rtc::Thread::Current(),
                                             worker_thread,
-                                            source_)))) {
+                                            source_),
+              worker_thread))) {
   source_->SetState(MediaSourceInterface::kLive);
   if (!channel_) {
     LOG(LS_ERROR)

webrtc/pc/rtpsenderreceiver_unittest.cc

@@ -124,7 +124,8 @@ class RtpSenderReceiverTest : public testing::Test,
   void AddVideoTrack(bool is_screencast) {
     rtc::scoped_refptr<VideoTrackSourceInterface> source(
         FakeVideoTrackSource::Create(is_screencast));
-    video_track_ = VideoTrack::Create(kVideoTrackId, source);
+    video_track_ =
+        VideoTrack::Create(kVideoTrackId, source, rtc::Thread::Current());
     EXPECT_TRUE(local_stream_->AddTrack(video_track_));
   }
 

webrtc/pc/statscollector_unittest.cc

@@ -545,7 +545,8 @@ class StatsCollectorTest : public testing::Test {
   void AddOutgoingVideoTrackStats() {
     stream_ = webrtc::MediaStream::Create("streamlabel");
     track_ = webrtc::VideoTrack::Create(kLocalTrackId,
-                                        webrtc::FakeVideoTrackSource::Create());
+                                        webrtc::FakeVideoTrackSource::Create(),
+                                        rtc::Thread::Current());
     stream_->AddTrack(track_);
     EXPECT_CALL(session_, GetLocalTrackIdBySsrc(kSsrcOfTrack, _))
         .WillRepeatedly(DoAll(SetArgPointee<1>(kLocalTrackId), Return(true)));
@@ -555,7 +556,8 @@ class StatsCollectorTest : public testing::Test {
   void AddIncomingVideoTrackStats() {
     stream_ = webrtc::MediaStream::Create("streamlabel");
     track_ = webrtc::VideoTrack::Create(kRemoteTrackId,
-                                        webrtc::FakeVideoTrackSource::Create());
+                                        webrtc::FakeVideoTrackSource::Create(),
+                                        rtc::Thread::Current());
     stream_->AddTrack(track_);
     EXPECT_CALL(session_, GetRemoteTrackIdBySsrc(kSsrcOfTrack, _))
         .WillRepeatedly(DoAll(SetArgPointee<1>(kRemoteTrackId), Return(true)));

webrtc/pc/trackmediainfomap_unittest.cc

@@ -85,10 +85,12 @@ class TrackMediaInfoMapTest : public testing::Test {
         remote_audio_track_(AudioTrack::Create("RemoteAudioTrack", nullptr)),
         local_video_track_(
             VideoTrack::Create("LocalVideoTrack",
-                               FakeVideoTrackSource::Create(false))),
+                               FakeVideoTrackSource::Create(false),
+                               rtc::Thread::Current())),
         remote_video_track_(
             VideoTrack::Create("RemoteVideoTrack",
-                               FakeVideoTrackSource::Create(false))) {}
+                               FakeVideoTrackSource::Create(false),
+                               rtc::Thread::Current())) {}
 
   ~TrackMediaInfoMapTest() {
     // If we have a map the ownership has been passed to the map, only delete if

webrtc/pc/videotrack.cc

@@ -15,11 +15,12 @@
 namespace webrtc {
 
 VideoTrack::VideoTrack(const std::string& label,
-                       VideoTrackSourceInterface* video_source)
+                       VideoTrackSourceInterface* video_source,
+                       rtc::Thread* worker_thread)
     : MediaStreamTrack<VideoTrackInterface>(label),
+      worker_thread_(worker_thread),
       video_source_(video_source),
       content_hint_(ContentHint::kNone) {
-  worker_thread_checker_.DetachFromThread();
   video_source_->RegisterObserver(this);
 }
 
@@ -35,7 +36,7 @@ std::string VideoTrack::kind() const {
 // thread.
 void VideoTrack::AddOrUpdateSink(rtc::VideoSinkInterface<VideoFrame>* sink,
                                  const rtc::VideoSinkWants& wants) {
-  RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
+  RTC_DCHECK(worker_thread_->IsCurrent());
   VideoSourceBase::AddOrUpdateSink(sink, wants);
   rtc::VideoSinkWants modified_wants = wants;
   modified_wants.black_frames = !enabled();
@@ -43,7 +44,7 @@ void VideoTrack::AddOrUpdateSink(rtc::VideoSinkInterface<VideoFrame>* sink,
 }
 
 void VideoTrack::RemoveSink(rtc::VideoSinkInterface<VideoFrame>* sink) {
-  RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
+  RTC_DCHECK(worker_thread_->IsCurrent());
   VideoSourceBase::RemoveSink(sink);
   video_source_->RemoveSink(sink);
 }
@@ -63,13 +64,14 @@ void VideoTrack::set_content_hint(ContentHint hint) {
 
 bool VideoTrack::set_enabled(bool enable) {
   RTC_DCHECK(signaling_thread_checker_.CalledOnValidThread());
-  for (auto& sink_pair : sink_pairs()) {
+  worker_thread_->Invoke<void>(RTC_FROM_HERE, [enable, this] {
-    rtc::VideoSinkWants modified_wants = sink_pair.wants;
+    RTC_DCHECK(worker_thread_->IsCurrent());
-    modified_wants.black_frames = !enable;
+    for (auto& sink_pair : sink_pairs()) {
-    // video_source_ is a proxy object, marshalling the call to the
+      rtc::VideoSinkWants modified_wants = sink_pair.wants;
-    // worker thread.
+      modified_wants.black_frames = !enable;
-    video_source_->AddOrUpdateSink(sink_pair.sink, modified_wants);
+      video_source_->AddOrUpdateSink(sink_pair.sink, modified_wants);
-  }
+    }
+  });
   return MediaStreamTrack<VideoTrackInterface>::set_enabled(enable);
 }
 
@@ -84,9 +86,10 @@ void VideoTrack::OnChanged() {
 
 rtc::scoped_refptr<VideoTrack> VideoTrack::Create(
     const std::string& id,
-    VideoTrackSourceInterface* source) {
+    VideoTrackSourceInterface* source,
+    rtc::Thread* worker_thread) {
   rtc::RefCountedObject<VideoTrack>* track =
-      new rtc::RefCountedObject<VideoTrack>(id, source);
+      new rtc::RefCountedObject<VideoTrack>(id, source, worker_thread);
   return track;
 }
 

webrtc/pc/videotrack.h

@@ -27,7 +27,8 @@ class VideoTrack : public MediaStreamTrack<VideoTrackInterface>,
  public:
   static rtc::scoped_refptr<VideoTrack> Create(
       const std::string& label,
-      VideoTrackSourceInterface* source);
+      VideoTrackSourceInterface* source,
+      rtc::Thread* worker_thread);
 
   void AddOrUpdateSink(rtc::VideoSinkInterface<VideoFrame>* sink,
                        const rtc::VideoSinkWants& wants) override;
@@ -42,15 +43,17 @@ class VideoTrack : public MediaStreamTrack<VideoTrackInterface>,
   std::string kind() const override;
 
  protected:
-  VideoTrack(const std::string& id, VideoTrackSourceInterface* video_source);
+  VideoTrack(const std::string& id,
+             VideoTrackSourceInterface* video_source,
+             rtc::Thread* worker_thread);
   ~VideoTrack();
 
  private:
   // Implements ObserverInterface. Observes |video_source_| state.
   void OnChanged() override;
 
+  rtc::Thread* const worker_thread_;
   rtc::ThreadChecker signaling_thread_checker_;
-  rtc::ThreadChecker worker_thread_checker_;
   rtc::scoped_refptr<VideoTrackSourceInterface> video_source_;
   ContentHint content_hint_ GUARDED_BY(signaling_thread_checker_);
 };

webrtc/pc/videotrack_unittest.cc

@@ -31,7 +31,8 @@ class VideoTrackTest : public testing::Test {
     static const char kVideoTrackId[] = "track_id";
     video_track_source_ = new rtc::RefCountedObject<VideoTrackSource>(
         &capturer_, true /* remote */);
-    video_track_ = VideoTrack::Create(kVideoTrackId, video_track_source_);
+    video_track_ = VideoTrack::Create(kVideoTrackId, video_track_source_,
+                                      rtc::Thread::Current());
     capturer_.Start(
         cricket::VideoFormat(640, 480, cricket::VideoFormat::FpsToInterval(30),
                              cricket::FOURCC_I420));