Revert of Change VideoTrack implementation to invoke VideoTrackSourceInterface::AddOrUpdateSink on wt (patchset #2 id:20001 of https://codereview.webrtc.org/2964863002/ )

Reason for revert:
It breaks a downstream project.

Original issue's description:
> Change VideoTrack implementation to invoke VideoTrackSourceInterface::AddOrUpdateSink on the worker thread.
>
> Added documentation of thread expectations for video tracks and sources to the API.
>
> BUG=None
>
> Review-Url: https://codereview.webrtc.org/2964863002
> Cr-Commit-Position: refs/heads/master@{#18938}
> Committed: f1377f7222

TBR=deadbeef@webrtc.org,noahric@chromium.org,yujo@chromium.org,perkj@webrtc.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG=None

Review-Url: https://codereview.webrtc.org/2979493003
Cr-Commit-Position: refs/heads/master@{#18942}
This commit is contained in:
mbonadei 2017-07-10 02:40:49 -07:00 committed by Commit Bot
parent bffe597e69
commit 539d104e3e
13 changed files with 35 additions and 61 deletions

View File

@ -111,11 +111,6 @@ 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> {
@ -150,12 +145,6 @@ 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> {
@ -165,6 +154,12 @@ class VideoTrackInterface
// See https://crbug.com/653531 and https://github.com/WICG/mst-content-hint.
enum class ContentHint { kNone, kFluid, kDetailed };
// Register a video sink for this track. Used to connect the track to the
// underlying video engine.
void AddOrUpdateSink(rtc::VideoSinkInterface<VideoFrame>* sink,
const rtc::VideoSinkWants& wants) override {}
void RemoveSink(rtc::VideoSinkInterface<VideoFrame>* sink) override {}
virtual VideoTrackSourceInterface* GetSource() const = 0;
virtual ContentHint content_hint() const { return ContentHint::kNone; }

View File

@ -479,8 +479,7 @@ 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, worker_thread_.get()));
rtc::scoped_refptr<VideoTrackInterface> track(VideoTrack::Create(id, source));
return VideoTrackProxy::Create(signaling_thread_, worker_thread_.get(),
track);
}

View File

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

View File

@ -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, worker_thread_));
VideoTrack::Create(id, source));
return VideoTrackProxy::Create(signaling_thread_, worker_thread_, track);
}

View File

@ -460,8 +460,7 @@ 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(),
rtc::Thread::Current()));
webrtc::FakeVideoTrackSource::Create()));
stream->AddTrack(video_track);
}
@ -1140,8 +1139,7 @@ class PeerConnectionInterfaceTest : public testing::Test {
MediaStreamInterface* stream) {
rtc::scoped_refptr<webrtc::VideoTrackInterface> video_track(
webrtc::VideoTrack::Create(track_id,
webrtc::FakeVideoTrackSource::Create(),
rtc::Thread::Current()));
webrtc::FakeVideoTrackSource::Create()));
ASSERT_TRUE(stream->AddTrack(video_track));
}

View File

@ -218,11 +218,6 @@ 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; }
};

View File

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

View File

@ -124,8 +124,7 @@ 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, rtc::Thread::Current());
video_track_ = VideoTrack::Create(kVideoTrackId, source);
EXPECT_TRUE(local_stream_->AddTrack(video_track_));
}

View File

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

View File

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

View File

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

View File

@ -27,8 +27,7 @@ class VideoTrack : public MediaStreamTrack<VideoTrackInterface>,
public:
static rtc::scoped_refptr<VideoTrack> Create(
const std::string& label,
VideoTrackSourceInterface* source,
rtc::Thread* worker_thread);
VideoTrackSourceInterface* source);
void AddOrUpdateSink(rtc::VideoSinkInterface<VideoFrame>* sink,
const rtc::VideoSinkWants& wants) override;
@ -43,17 +42,15 @@ class VideoTrack : public MediaStreamTrack<VideoTrackInterface>,
std::string kind() const override;
protected:
VideoTrack(const std::string& id,
VideoTrackSourceInterface* video_source,
rtc::Thread* worker_thread);
VideoTrack(const std::string& id, VideoTrackSourceInterface* video_source);
~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_);
};

View File

@ -31,8 +31,7 @@ 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_,
rtc::Thread::Current());
video_track_ = VideoTrack::Create(kVideoTrackId, video_track_source_);
capturer_.Start(
cricket::VideoFormat(640, 480, cricket::VideoFormat::FpsToInterval(30),
cricket::FOURCC_I420));