Delete FrameInput method and FrameInputWrapper class. Added VideoTrackInterface::GetSink method, for use by VideoRtpReceiver. This lets us delete FrameInput. I realized this change doesn't depend on VideoSinkInterface changes, so this is a more standalone version of cl https://codereview.webrtc.org/1664773002/ BUG=webrtc:5426 Review URL: https://codereview.webrtc.org/1660103003 Cr-Commit-Position: refs/heads/master@{#11498}
diff --git a/talk/app/webrtc/mediastreaminterface.h b/talk/app/webrtc/mediastreaminterface.h index 8d6feb0..2d2dc9a 100644 --- a/talk/app/webrtc/mediastreaminterface.h +++ b/talk/app/webrtc/mediastreaminterface.h
@@ -147,6 +147,26 @@ virtual VideoSourceInterface* GetSource() const = 0; + // Return the track input sink. I.e., frames sent to this sink are + // propagated to all renderers registered with the track. The + // returned sink must not change between calls. Currently, this + // method is used for remote tracks (VideoRtpReceiver); further + // refactoring is planned for this path, it's unclear if this method + // belongs here long term. + + // We do this instead of simply implementing the + // VideoSourceInterface directly, because if we did the latter, we'd + // need an OnFrame method in VideoTrackProxy, with a thread jump on + // each call. + + // TODO(nisse): It has a default implementation so that mock + // objects, in particular, chrome's MockWebRtcVideoTrack, doesn't + // need to know about it. Consider removing the implementation (or + // this comment) after refactoring dust settles. + virtual rtc::VideoSinkInterface<cricket::VideoFrame>* GetSink() { + return nullptr; + }; + protected: virtual ~VideoTrackInterface() {} };
diff --git a/talk/app/webrtc/mediastreamtrackproxy.h b/talk/app/webrtc/mediastreamtrackproxy.h index b485905..e99910e 100644 --- a/talk/app/webrtc/mediastreamtrackproxy.h +++ b/talk/app/webrtc/mediastreamtrackproxy.h
@@ -66,6 +66,7 @@ PROXY_METHOD1(void, AddRenderer, VideoRendererInterface*) PROXY_METHOD1(void, RemoveRenderer, VideoRendererInterface*) PROXY_CONSTMETHOD0(VideoSourceInterface*, GetSource) + PROXY_METHOD0(rtc::VideoSinkInterface<cricket::VideoFrame>*, GetSink) PROXY_METHOD1(void, RegisterObserver, ObserverInterface*) PROXY_METHOD1(void, UnregisterObserver, ObserverInterface*)
diff --git a/talk/app/webrtc/rtpreceiver.cc b/talk/app/webrtc/rtpreceiver.cc index 9540f36..34efe5c 100644 --- a/talk/app/webrtc/rtpreceiver.cc +++ b/talk/app/webrtc/rtpreceiver.cc
@@ -86,8 +86,7 @@ uint32_t ssrc, VideoProviderInterface* provider) : id_(track->id()), track_(track), ssrc_(ssrc), provider_(provider) { - RTC_DCHECK(track_->GetSource()->remote()); - provider_->SetVideoPlayout(ssrc_, true, track_->GetSource()->FrameInput()); + provider_->SetVideoPlayout(ssrc_, true, track_->GetSink()); } VideoRtpReceiver::~VideoRtpReceiver() {
diff --git a/talk/app/webrtc/rtpsenderreceiver_unittest.cc b/talk/app/webrtc/rtpsenderreceiver_unittest.cc index b9ef881..bcd9ea0 100644 --- a/talk/app/webrtc/rtpsenderreceiver_unittest.cc +++ b/talk/app/webrtc/rtpsenderreceiver_unittest.cc
@@ -181,7 +181,7 @@ AddVideoTrack(true); EXPECT_CALL(video_provider_, SetVideoPlayout(kVideoSsrc, true, - video_track_->GetSource()->FrameInput())); + video_track_->GetSink())); video_rtp_receiver_ = new VideoRtpReceiver(stream_->GetVideoTracks()[0], kVideoSsrc, &video_provider_); }
diff --git a/talk/app/webrtc/videosource.cc b/talk/app/webrtc/videosource.cc index 018bad7..08c9717 100644 --- a/talk/app/webrtc/videosource.cc +++ b/talk/app/webrtc/videosource.cc
@@ -294,34 +294,6 @@ return all_valid; } -class FrameInputWrapper : public cricket::VideoRenderer { - public: - explicit FrameInputWrapper(cricket::VideoCapturer* capturer) - : capturer_(capturer) { - ASSERT(capturer_ != NULL); - } - - virtual ~FrameInputWrapper() {} - - // VideoRenderer implementation. - bool RenderFrame(const cricket::VideoFrame* frame) override { - if (!capturer_->IsRunning()) { - return true; - } - - // This signal will be made on media engine render thread. The clients - // of this signal should have no assumptions on what thread this signal - // come from. - capturer_->SignalVideoFrame(capturer_, frame); - return true; - } - - private: - cricket::VideoCapturer* capturer_; - - RTC_DISALLOW_COPY_AND_ASSIGN(FrameInputWrapper); -}; - } // anonymous namespace namespace webrtc { @@ -418,15 +390,6 @@ // Initialize hasn't succeeded until a successful state change has occurred. } -cricket::VideoRenderer* VideoSource::FrameInput() { - // Defer creation of frame_input_ until it's needed, e.g. the local video - // sources will never need it. - if (!frame_input_) { - frame_input_.reset(new FrameInputWrapper(video_capturer_.get())); - } - return frame_input_.get(); -} - void VideoSource::Stop() { channel_manager_->StopVideoCapture(video_capturer_.get(), format_); }
diff --git a/talk/app/webrtc/videosource.h b/talk/app/webrtc/videosource.h index 24a9f99..bae5d30 100644 --- a/talk/app/webrtc/videosource.h +++ b/talk/app/webrtc/videosource.h
@@ -74,7 +74,6 @@ bool remote() const override { return remote_; } virtual const cricket::VideoOptions* options() const { return &options_; } - virtual cricket::VideoRenderer* FrameInput(); virtual cricket::VideoCapturer* GetVideoCapturer() { return video_capturer_.get();
diff --git a/talk/app/webrtc/videosource_unittest.cc b/talk/app/webrtc/videosource_unittest.cc index 7d8a731..bb91127 100644 --- a/talk/app/webrtc/videosource_unittest.cc +++ b/talk/app/webrtc/videosource_unittest.cc
@@ -222,11 +222,6 @@ EXPECT_EQ_WAIT(MediaSourceInterface::kLive, state_observer_->state(), kMaxWaitMs); - cricket::VideoRenderer* frameinput = source_->FrameInput(); - cricket::WebRtcVideoFrame test_frame; - frameinput->RenderFrame(&test_frame); - EXPECT_EQ(1, renderer_.num_rendered_frames()); - source_->GetVideoCapturer()->Stop(); EXPECT_EQ_WAIT(MediaSourceInterface::kEnded, state_observer_->state(), kMaxWaitMs);
diff --git a/talk/app/webrtc/videosourceinterface.h b/talk/app/webrtc/videosourceinterface.h index 6ae45cb..d74bf3b 100644 --- a/talk/app/webrtc/videosourceinterface.h +++ b/talk/app/webrtc/videosourceinterface.h
@@ -55,7 +55,9 @@ virtual void RemoveSink( rtc::VideoSinkInterface<cricket::VideoFrame>* output) = 0; virtual const cricket::VideoOptions* options() const = 0; - virtual cricket::VideoRenderer* FrameInput() = 0; + // TODO(nisse): Dummy implementation. Delete as soon as chrome's + // MockVideoSource is updated. + virtual cricket::VideoRenderer* FrameInput() { return nullptr; } protected: virtual ~VideoSourceInterface() {}
diff --git a/talk/app/webrtc/videosourceproxy.h b/talk/app/webrtc/videosourceproxy.h index c847dc6..01abaf6 100644 --- a/talk/app/webrtc/videosourceproxy.h +++ b/talk/app/webrtc/videosourceproxy.h
@@ -45,7 +45,6 @@ PROXY_METHOD1(void, AddSink, rtc::VideoSinkInterface<cricket::VideoFrame>*) PROXY_METHOD1(void, RemoveSink, rtc::VideoSinkInterface<cricket::VideoFrame>*) PROXY_CONSTMETHOD0(const cricket::VideoOptions*, options) - PROXY_METHOD0(cricket::VideoRenderer*, FrameInput) PROXY_METHOD1(void, RegisterObserver, ObserverInterface*) PROXY_METHOD1(void, UnregisterObserver, ObserverInterface*) END_PROXY()
diff --git a/talk/app/webrtc/videotrack.cc b/talk/app/webrtc/videotrack.cc index f138240..c649275 100644 --- a/talk/app/webrtc/videotrack.cc +++ b/talk/app/webrtc/videotrack.cc
@@ -58,6 +58,10 @@ renderers_.RemoveRenderer(renderer); } +rtc::VideoSinkInterface<cricket::VideoFrame>* VideoTrack::GetSink() { + return &renderers_; +} + bool VideoTrack::set_enabled(bool enable) { renderers_.SetEnabled(enable); return MediaStreamTrack<VideoTrackInterface>::set_enabled(enable);
diff --git a/talk/app/webrtc/videotrack.h b/talk/app/webrtc/videotrack.h index 67a2163..b321c42 100644 --- a/talk/app/webrtc/videotrack.h +++ b/talk/app/webrtc/videotrack.h
@@ -47,6 +47,7 @@ virtual VideoSourceInterface* GetSource() const { return video_source_.get(); } + rtc::VideoSinkInterface<cricket::VideoFrame>* GetSink() override; virtual bool set_enabled(bool enable); virtual std::string kind() const;
diff --git a/talk/app/webrtc/videotrack_unittest.cc b/talk/app/webrtc/videotrack_unittest.cc index a205a28..88552a5 100644 --- a/talk/app/webrtc/videotrack_unittest.cc +++ b/talk/app/webrtc/videotrack_unittest.cc
@@ -77,13 +77,13 @@ rtc::scoped_ptr<FakeVideoTrackRenderer> renderer_1( new FakeVideoTrackRenderer(video_track_.get())); - cricket::VideoRenderer* renderer_input = - video_track_->GetSource()->FrameInput(); + rtc::VideoSinkInterface<cricket::VideoFrame>* renderer_input = + video_track_->GetSink(); ASSERT_FALSE(renderer_input == NULL); cricket::WebRtcVideoFrame frame; frame.InitToBlack(123, 123, 0); - renderer_input->RenderFrame(&frame); + renderer_input->OnFrame(frame); EXPECT_EQ(1, renderer_1->num_rendered_frames()); EXPECT_EQ(123, renderer_1->width()); @@ -93,7 +93,7 @@ rtc::scoped_ptr<FakeVideoTrackRenderer> renderer_2( new FakeVideoTrackRenderer(video_track_.get())); - renderer_input->RenderFrame(&frame); + renderer_input->OnFrame(frame); EXPECT_EQ(123, renderer_1->width()); EXPECT_EQ(123, renderer_1->height()); @@ -104,7 +104,7 @@ EXPECT_EQ(1, renderer_2->num_rendered_frames()); video_track_->RemoveRenderer(renderer_1.get()); - renderer_input->RenderFrame(&frame); + renderer_input->OnFrame(frame); EXPECT_EQ(2, renderer_1->num_rendered_frames()); EXPECT_EQ(2, renderer_2->num_rendered_frames()); @@ -115,8 +115,8 @@ rtc::scoped_ptr<FakeVideoTrackRenderer> renderer( new FakeVideoTrackRenderer(video_track_.get())); - cricket::VideoRenderer* renderer_input = - video_track_->GetSource()->FrameInput(); + rtc::VideoSinkInterface<cricket::VideoFrame>* renderer_input = + video_track_->GetSink(); ASSERT_FALSE(renderer_input == NULL); cricket::WebRtcVideoFrame frame; @@ -124,21 +124,21 @@ // Make it not all-black frame.GetUPlane()[0] = 0; - renderer_input->RenderFrame(&frame); + renderer_input->OnFrame(frame); EXPECT_EQ(1, renderer->num_rendered_frames()); EXPECT_FALSE(renderer->black_frame()); EXPECT_EQ(100, renderer->width()); EXPECT_EQ(200, renderer->height()); video_track_->set_enabled(false); - renderer_input->RenderFrame(&frame); + renderer_input->OnFrame(frame); EXPECT_EQ(2, renderer->num_rendered_frames()); EXPECT_TRUE(renderer->black_frame()); EXPECT_EQ(100, renderer->width()); EXPECT_EQ(200, renderer->height()); video_track_->set_enabled(true); - renderer_input->RenderFrame(&frame); + renderer_input->OnFrame(frame); EXPECT_EQ(3, renderer->num_rendered_frames()); EXPECT_FALSE(renderer->black_frame()); EXPECT_EQ(100, renderer->width());