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}
diff --git a/webrtc/pc/mediastream_unittest.cc b/webrtc/pc/mediastream_unittest.cc
index 01a547b..1a96a56 100644
--- a/webrtc/pc/mediastream_unittest.cc
+++ b/webrtc/pc/mediastream_unittest.cc
@@ -56,8 +56,8 @@
stream_ = MediaStream::Create(kStreamLabel1);
ASSERT_TRUE(stream_.get() != NULL);
- video_track_ =
- VideoTrack::Create(kVideoTrackId, FakeVideoTrackSource::Create());
+ video_track_ = VideoTrack::Create(
+ kVideoTrackId, FakeVideoTrackSource::Create(), rtc::Thread::Current());
ASSERT_TRUE(video_track_.get() != NULL);
EXPECT_EQ(MediaStreamTrackInterface::kLive, video_track_->state());
diff --git a/webrtc/pc/peerconnectionfactory.cc b/webrtc/pc/peerconnectionfactory.cc
index 15e36f0..be083c2 100644
--- a/webrtc/pc/peerconnectionfactory.cc
+++ b/webrtc/pc/peerconnectionfactory.cc
@@ -292,7 +292,7 @@
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);
}
diff --git a/webrtc/pc/peerconnectioninterface_unittest.cc b/webrtc/pc/peerconnectioninterface_unittest.cc
index 114ffd7..0e52c20 100644
--- a/webrtc/pc/peerconnectioninterface_unittest.cc
+++ b/webrtc/pc/peerconnectioninterface_unittest.cc
@@ -460,7 +460,8 @@
// 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 @@
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));
}
diff --git a/webrtc/pc/rtcstatscollector_unittest.cc b/webrtc/pc/rtcstatscollector_unittest.cc
index 8dc4a97..f6591c0 100644
--- a/webrtc/pc/rtcstatscollector_unittest.cc
+++ b/webrtc/pc/rtcstatscollector_unittest.cc
@@ -218,6 +218,11 @@
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; }
};
diff --git a/webrtc/pc/rtpreceiver.cc b/webrtc/pc/rtpreceiver.cc
index f3eef5a..c6a2128 100644
--- a/webrtc/pc/rtpreceiver.cc
+++ b/webrtc/pc/rtpreceiver.cc
@@ -155,7 +155,8 @@
track_id,
VideoTrackSourceProxy::Create(rtc::Thread::Current(),
worker_thread,
- source_)))) {
+ source_),
+ worker_thread))) {
source_->SetState(MediaSourceInterface::kLive);
if (!channel_) {
LOG(LS_ERROR)
diff --git a/webrtc/pc/rtpsenderreceiver_unittest.cc b/webrtc/pc/rtpsenderreceiver_unittest.cc
index bf413e4..9779eb6 100644
--- a/webrtc/pc/rtpsenderreceiver_unittest.cc
+++ b/webrtc/pc/rtpsenderreceiver_unittest.cc
@@ -124,7 +124,8 @@
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_));
}
diff --git a/webrtc/pc/statscollector_unittest.cc b/webrtc/pc/statscollector_unittest.cc
index 9b62b81..0fad6ad 100644
--- a/webrtc/pc/statscollector_unittest.cc
+++ b/webrtc/pc/statscollector_unittest.cc
@@ -545,7 +545,8 @@
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 @@
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)));
diff --git a/webrtc/pc/trackmediainfomap_unittest.cc b/webrtc/pc/trackmediainfomap_unittest.cc
index b380cc4..f0a3f1a 100644
--- a/webrtc/pc/trackmediainfomap_unittest.cc
+++ b/webrtc/pc/trackmediainfomap_unittest.cc
@@ -85,10 +85,12 @@
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
diff --git a/webrtc/pc/videotrack.cc b/webrtc/pc/videotrack.cc
index 494d728..f106460 100644
--- a/webrtc/pc/videotrack.cc
+++ b/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 @@
// 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::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 @@
bool VideoTrack::set_enabled(bool enable) {
RTC_DCHECK(signaling_thread_checker_.CalledOnValidThread());
- for (auto& sink_pair : sink_pairs()) {
- rtc::VideoSinkWants modified_wants = sink_pair.wants;
- modified_wants.black_frames = !enable;
- // video_source_ is a proxy object, marshalling the call to the
- // worker thread.
- video_source_->AddOrUpdateSink(sink_pair.sink, modified_wants);
- }
+ worker_thread_->Invoke<void>(RTC_FROM_HERE, [enable, this] {
+ RTC_DCHECK(worker_thread_->IsCurrent());
+ for (auto& sink_pair : sink_pairs()) {
+ rtc::VideoSinkWants modified_wants = sink_pair.wants;
+ modified_wants.black_frames = !enable;
+ video_source_->AddOrUpdateSink(sink_pair.sink, modified_wants);
+ }
+ });
return MediaStreamTrack<VideoTrackInterface>::set_enabled(enable);
}
@@ -84,9 +86,10 @@
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;
}
diff --git a/webrtc/pc/videotrack.h b/webrtc/pc/videotrack.h
index f251797..6e20e97 100644
--- a/webrtc/pc/videotrack.h
+++ b/webrtc/pc/videotrack.h
@@ -27,7 +27,8 @@
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 @@
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_);
};
diff --git a/webrtc/pc/videotrack_unittest.cc b/webrtc/pc/videotrack_unittest.cc
index d033efe63..1f569eb 100644
--- a/webrtc/pc/videotrack_unittest.cc
+++ b/webrtc/pc/videotrack_unittest.cc
@@ -31,7 +31,8 @@
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));
