From dbe511aa9eea169722b239f82174850df2185eff Mon Sep 17 00:00:00 2001 From: emircan Date: Fri, 25 Mar 2016 17:46:01 -0700 Subject: Request frame for new sinks from MediaStreamVideoTrack This CL adds a series of RequstFrame() calls propagated from MediaStreamVideoTrack when a new sink is added. MediaStreamVideoTrack -> MediaStreamVideoCapturerSource which is a MediaStreamVideoSource -> media::VideoCapturerSource This addresses the issues when a new sink is added but doesn't receive a frame because it wasn't added on time. BUG=597876, 587789, 486274 Review URL: https://codereview.chromium.org/1829193003 Cr-Commit-Position: refs/heads/master@{#383421} --- content/renderer/media/canvas_capture_handler.cc | 19 ++++++++++++++++ content/renderer/media/canvas_capture_handler.h | 3 +++ .../media/media_stream_video_capturer_source.cc | 4 ++++ .../media/media_stream_video_capturer_source.h | 3 +++ .../media_stream_video_capturer_source_unittest.cc | 2 ++ content/renderer/media/media_stream_video_source.h | 3 +++ content/renderer/media/media_stream_video_track.cc | 3 +++ .../media/media_stream_video_track_unittest.cc | 26 ++++++++++++++++++++++ .../media/mock_media_stream_video_source.cc | 20 +++++++++++++++-- .../media/mock_media_stream_video_source.h | 6 +++++ media/base/video_capturer_source.h | 10 +++++++++ 11 files changed, 97 insertions(+), 2 deletions(-) diff --git a/content/renderer/media/canvas_capture_handler.cc b/content/renderer/media/canvas_capture_handler.cc index e0d6089e..0220238 100644 --- a/content/renderer/media/canvas_capture_handler.cc +++ b/content/renderer/media/canvas_capture_handler.cc @@ -85,6 +85,10 @@ class VideoCapturerSource : public media::VideoCapturerSource { canvas_handler_->StartVideoCapture(params, frame_callback, running_callback); } + void RequestRefreshFrame() override { + DCHECK(main_render_thread_checker_.CalledOnValidThread()); + canvas_handler_->RequestRefreshFrame(); + } void StopCapture() override { DCHECK(main_render_thread_checker_.CalledOnValidThread()); if (canvas_handler_.get()) @@ -194,6 +198,19 @@ void CanvasCaptureHandler::StartVideoCapture( running_callback.Run(true); } +void CanvasCaptureHandler::RequestRefreshFrame() { + DVLOG(3) << __FUNCTION__; + DCHECK(main_render_thread_checker_.CalledOnValidThread()); + if (last_frame_ && delegate_) { + io_task_runner_->PostTask( + FROM_HERE, + base::Bind(&CanvasCaptureHandler::CanvasCaptureHandlerDelegate:: + SendNewFrameOnIOThread, + delegate_->GetWeakPtrForIOThread(), last_frame_, + base::TimeTicks::Now())); + } +} + void CanvasCaptureHandler::StopVideoCapture() { DVLOG(3) << __FUNCTION__; DCHECK(main_render_thread_checker_.CalledOnValidThread()); @@ -202,6 +219,7 @@ void CanvasCaptureHandler::StopVideoCapture() { } void CanvasCaptureHandler::CreateNewFrame(const SkImage* image) { + DVLOG(4) << __FUNCTION__; DCHECK(main_render_thread_checker_.CalledOnValidThread()); DCHECK(image); @@ -243,6 +261,7 @@ void CanvasCaptureHandler::CreateNewFrame(const SkImage* image) { CopyAlphaChannelIntoVideoFrame(temp_data_.data(), video_frame); } + last_frame_ = video_frame; io_task_runner_->PostTask( FROM_HERE, base::Bind(&CanvasCaptureHandler::CanvasCaptureHandlerDelegate:: diff --git a/content/renderer/media/canvas_capture_handler.h b/content/renderer/media/canvas_capture_handler.h index dff93b4..5424994 100644 --- a/content/renderer/media/canvas_capture_handler.h +++ b/content/renderer/media/canvas_capture_handler.h @@ -53,6 +53,7 @@ class CONTENT_EXPORT CanvasCaptureHandler final const media::VideoCapturerSource::VideoCaptureDeliverFrameCB& new_frame_callback, const media::VideoCapturerSource::RunningCallback& running_callback); + void RequestRefreshFrame(); void StopVideoCapture(); blink::WebSize GetSourceSize() const { return size_; } @@ -86,6 +87,8 @@ class CONTENT_EXPORT CanvasCaptureHandler final SkImageInfo image_info_; media::VideoFramePool frame_pool_; + scoped_refptr last_frame_; + const scoped_refptr io_task_runner_; scoped_ptr delegate_; diff --git a/content/renderer/media/media_stream_video_capturer_source.cc b/content/renderer/media/media_stream_video_capturer_source.cc index ea2fb04..405e26a 100644 --- a/content/renderer/media/media_stream_video_capturer_source.cc +++ b/content/renderer/media/media_stream_video_capturer_source.cc @@ -398,6 +398,10 @@ MediaStreamVideoCapturerSource::MediaStreamVideoCapturerSource( MediaStreamVideoCapturerSource::~MediaStreamVideoCapturerSource() { } +void MediaStreamVideoCapturerSource::RequestRefreshFrame() { + source_->RequestRefreshFrame(); +} + void MediaStreamVideoCapturerSource::GetCurrentSupportedFormats( int max_requested_width, int max_requested_height, diff --git a/content/renderer/media/media_stream_video_capturer_source.h b/content/renderer/media/media_stream_video_capturer_source.h index 9dc7b1f..bc7b015 100644 --- a/content/renderer/media/media_stream_video_capturer_source.h +++ b/content/renderer/media/media_stream_video_capturer_source.h @@ -31,6 +31,9 @@ class CONTENT_EXPORT MediaStreamVideoCapturerSource const StreamDeviceInfo& device_info); ~MediaStreamVideoCapturerSource() override; + // Implements MediaStreamVideoSource. + void RequestRefreshFrame() override; + private: friend class CanvasCaptureHandlerTest; friend class MediaStreamVideoCapturerSourceTest; diff --git a/content/renderer/media/media_stream_video_capturer_source_unittest.cc b/content/renderer/media/media_stream_video_capturer_source_unittest.cc index 08168c5..fe24e66 100644 --- a/content/renderer/media/media_stream_video_capturer_source_unittest.cc +++ b/content/renderer/media/media_stream_video_capturer_source_unittest.cc @@ -36,6 +36,7 @@ class MockVideoCapturerSource : public media::VideoCapturerSource { Invoke(this, &MockVideoCapturerSource::EnumerateDeviceFormats))); } + MOCK_METHOD0(RequestRefreshFrame, void()); MOCK_METHOD4(GetCurrentSupportedFormats, void(int max_requested_width, int max_requested_height, @@ -363,6 +364,7 @@ TEST_F(MediaStreamVideoCapturerSourceTest, CaptureTimeAndMetadataPlumbing) { EXPECT_CALL(mock_delegate(), StartCapture(_, _, _)) .WillOnce(testing::DoAll(testing::SaveArg<1>(&deliver_frame_cb), testing::SaveArg<2>(&running_cb))); + EXPECT_CALL(mock_delegate(), RequestRefreshFrame()); EXPECT_CALL(mock_delegate(), StopCapture()); blink::WebMediaStreamTrack track = StartSource(); running_cb.Run(true); diff --git a/content/renderer/media/media_stream_video_source.h b/content/renderer/media/media_stream_video_source.h index 027ee38..e2e24f0 100644 --- a/content/renderer/media/media_stream_video_source.h +++ b/content/renderer/media/media_stream_video_source.h @@ -84,6 +84,9 @@ class CONTENT_EXPORT MediaStreamVideoSource // Return true if |name| is a constraint supported by MediaStreamVideoSource. static bool IsConstraintSupported(const std::string& name); + // Request underlying source to capture a new frame. + virtual void RequestRefreshFrame() {} + // Returns the task runner where video frames will be delivered on. base::SingleThreadTaskRunner* io_task_runner() const; diff --git a/content/renderer/media/media_stream_video_track.cc b/content/renderer/media/media_stream_video_track.cc index cae744b..00e75fa 100644 --- a/content/renderer/media/media_stream_video_track.cc +++ b/content/renderer/media/media_stream_video_track.cc @@ -247,6 +247,9 @@ void MediaStreamVideoTrack::AddSink( DCHECK(std::find(sinks_.begin(), sinks_.end(), sink) == sinks_.end()); sinks_.push_back(sink); frame_deliverer_->AddCallback(sink, callback); + // Request source to deliver a frame because a new sink is added. + if (source_) + source_->RequestRefreshFrame(); } void MediaStreamVideoTrack::RemoveSink(MediaStreamVideoSink* sink) { diff --git a/content/renderer/media/media_stream_video_track_unittest.cc b/content/renderer/media/media_stream_video_track_unittest.cc index 8de755e..1b5071b 100644 --- a/content/renderer/media/media_stream_video_track_unittest.cc +++ b/content/renderer/media/media_stream_video_track_unittest.cc @@ -82,6 +82,16 @@ class MediaStreamVideoTrackTest : public ::testing::Test { return track; } + void UpdateVideoSourceToRespondToRequestRefreshFrame() { + blink_source_.reset(); + mock_source_ = new MockMediaStreamVideoSource(false, true); + blink_source_.initialize(base::UTF8ToUTF16("dummy_source_id"), + blink::WebMediaStreamSource::TypeVideo, + base::UTF8ToUTF16("dummy_source_name"), + false /* remote */, true /* readonly */); + blink_source_.setExtraData(mock_source_); + } + MockMediaStreamVideoSource* mock_source() { return mock_source_; } const blink::WebMediaStreamSource& blink_source() const { return blink_source_; @@ -232,4 +242,20 @@ TEST_F(MediaStreamVideoTrackTest, StopLastTrack) { MediaStreamVideoSink::RemoveFromVideoTrack(&sink2, track2); } +TEST_F(MediaStreamVideoTrackTest, CheckTrackRequestsFrame) { + UpdateVideoSourceToRespondToRequestRefreshFrame(); + blink::WebMediaStreamTrack track = CreateTrack(); + + // Add sink and expect to get a frame. + MockMediaStreamVideoSink sink; + base::RunLoop run_loop; + base::Closure quit_closure = run_loop.QuitClosure(); + EXPECT_CALL(sink, OnVideoFrame()).WillOnce(RunClosure(quit_closure)); + MediaStreamVideoSink::AddToVideoTrack(&sink, sink.GetDeliverFrameCB(), track); + run_loop.Run(); + EXPECT_EQ(1, sink.number_of_frames()); + + MediaStreamVideoSink::RemoveFromVideoTrack(&sink, track); +} + } // namespace content diff --git a/content/renderer/media/mock_media_stream_video_source.cc b/content/renderer/media/mock_media_stream_video_source.cc index f80e2c5..3cfa3c0 100644 --- a/content/renderer/media/mock_media_stream_video_source.cc +++ b/content/renderer/media/mock_media_stream_video_source.cc @@ -12,7 +12,13 @@ namespace content { MockMediaStreamVideoSource::MockMediaStreamVideoSource( bool manual_get_supported_formats) + : MockMediaStreamVideoSource(manual_get_supported_formats, false) {} + +MockMediaStreamVideoSource::MockMediaStreamVideoSource( + bool manual_get_supported_formats, + bool respond_to_request_refresh_frame) : manual_get_supported_formats_(manual_get_supported_formats), + respond_to_request_refresh_frame_(respond_to_request_refresh_frame), max_requested_height_(0), max_requested_width_(0), max_requested_frame_rate_(0.0), @@ -20,8 +26,7 @@ MockMediaStreamVideoSource::MockMediaStreamVideoSource( supported_formats_.push_back(media::VideoCaptureFormat( gfx::Size(MediaStreamVideoSource::kDefaultWidth, MediaStreamVideoSource::kDefaultHeight), - MediaStreamVideoSource::kDefaultFrameRate, - media::PIXEL_FORMAT_I420)); + MediaStreamVideoSource::kDefaultFrameRate, media::PIXEL_FORMAT_I420)); } MockMediaStreamVideoSource::~MockMediaStreamVideoSource() {} @@ -43,6 +48,17 @@ void MockMediaStreamVideoSource::CompleteGetSupportedFormats() { base::ResetAndReturn(&formats_callback_).Run(supported_formats_); } +void MockMediaStreamVideoSource::RequestRefreshFrame() { + DCHECK(!frame_callback_.is_null()); + if (respond_to_request_refresh_frame_) { + const scoped_refptr frame = + media::VideoFrame::CreateColorFrame(format_.frame_size, 0, 0, 0, + base::TimeDelta()); + io_task_runner()->PostTask( + FROM_HERE, base::Bind(frame_callback_, frame, base::TimeTicks())); + } +} + void MockMediaStreamVideoSource::GetCurrentSupportedFormats( int max_requested_height, int max_requested_width, diff --git a/content/renderer/media/mock_media_stream_video_source.h b/content/renderer/media/mock_media_stream_video_source.h index e5e4d7e..4a44e03 100644 --- a/content/renderer/media/mock_media_stream_video_source.h +++ b/content/renderer/media/mock_media_stream_video_source.h @@ -15,6 +15,8 @@ namespace content { class MockMediaStreamVideoSource : public MediaStreamVideoSource { public: explicit MockMediaStreamVideoSource(bool manual_get_supported_formats); + MockMediaStreamVideoSource(bool manual_get_supported_formats, + bool respond_to_request_refresh_frame); virtual ~MockMediaStreamVideoSource(); MOCK_METHOD1(DoSetMutedState, void(bool muted_state)); @@ -50,6 +52,9 @@ class MockMediaStreamVideoSource : public MediaStreamVideoSource { DoSetMutedState(muted_state); } + // Implements MediaStreamVideoSource. + void RequestRefreshFrame() override; + protected: // Implements MediaStreamVideoSource. void GetCurrentSupportedFormats( @@ -67,6 +72,7 @@ class MockMediaStreamVideoSource : public MediaStreamVideoSource { media::VideoCaptureFormat format_; media::VideoCaptureFormats supported_formats_; bool manual_get_supported_formats_; + bool respond_to_request_refresh_frame_; int max_requested_height_; int max_requested_width_; double max_requested_frame_rate_; diff --git a/media/base/video_capturer_source.h b/media/base/video_capturer_source.h index a99faf8..ddec90c 100644 --- a/media/base/video_capturer_source.h +++ b/media/base/video_capturer_source.h @@ -76,6 +76,16 @@ class MEDIA_EXPORT VideoCapturerSource { const VideoCaptureDeliverFrameCB& new_frame_callback, const RunningCallback& running_callback) = 0; + // Asks source to send a refresh frame. In cases where source does not provide + // a continuous rate of new frames (e.g. canvas capture, screen capture where + // the screen's content has not changed in a while), consumers may request a + // "refresh frame" to be delivered. For instance, this would be needed when + // a new sink is added to a MediaStreamTrack. + // The default implementation is a no-op and implementations are not required + // to honor this request. If they decide to and capturing is started + // successfully, then |new_frame_callback| should be called with a frame. + virtual void RequestRefreshFrame() {} + // Stops capturing frames and clears all callbacks including the // SupportedFormatsCallback callback. Note that queued frame callbacks // may still occur after this call, so the caller must take care to -- cgit v1.1