Skip to content

Commit

Permalink
Allow WMPI to drive compositor updates for invisible video tags.
Browse files Browse the repository at this point in the history
Invisible tags don't get compositor updates, so we need to let them
drive those updates (within reason). This allows VideoFrameCompositor
instances without a VideoFrameProvider::Client to update at up to
250Hz, which should be enough for everyone.

This is done by introducing GetCurrentFrameAndMaybeBackgroundRender()
which is called by WebMediaPlayerImpl for grabbing frames.  If we
have no client, have a callback, and are background rendering, this
allows WebMediaPlayerImpl::paint() calls to drive frame acquisition.

Also fixes bug with media_blink_unittests which isn't caught by the
try bots for some reason...

BUG=485783
TEST=new unittest, ro.me plays great.

Review URL: https://codereview.chromium.org/1132213002

Cr-Commit-Position: refs/heads/master@{#329201}
  • Loading branch information
dalecurtis authored and Commit bot committed May 11, 2015
1 parent d5fcf98 commit 44ce4de
Show file tree
Hide file tree
Showing 5 changed files with 112 additions and 15 deletions.
7 changes: 7 additions & 0 deletions media/blink/run_all_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// found in the LICENSE file.

#include "base/bind.h"
#include "base/message_loop/message_loop.h"
#include "base/test/launcher/unit_test_launcher.h"
#include "base/test/test_suite.h"
#include "build/build_config.h"
Expand Down Expand Up @@ -81,6 +82,12 @@ void BlinkMediaTestSuite::Initialize() {
gin::V8Initializer::LoadV8Snapshot();
#endif

// Dummy task runner is initialized here because the blink::initialize creates
// IsolateHolder which needs the current task runner handle. There should be
// no task posted to this task runner.
scoped_ptr<base::MessageLoop> message_loop;
if (!base::MessageLoop::current())
message_loop.reset(new base::MessageLoop());
blink::initialize(blink_platform_support_.get());
}

Expand Down
24 changes: 24 additions & 0 deletions media/blink/video_frame_compositor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,29 @@ void VideoFrameCompositor::PaintFrameUsingOldRenderingPath(
client_->DidReceiveFrame();
}

scoped_refptr<VideoFrame>
VideoFrameCompositor::GetCurrentFrameAndUpdateIfStale() {
DCHECK(compositor_task_runner_->BelongsToCurrentThread());
if (client_ || !rendering_ || !is_background_rendering_)
return current_frame_;

DCHECK(!last_background_render_.is_null());

const base::TimeTicks now = tick_clock_->NowTicks();
const base::TimeDelta interval = now - last_background_render_;

// Cap updates to 250Hz which should be more than enough for everyone.
if (interval < base::TimeDelta::FromMilliseconds(4))
return current_frame_;

// Update the interval based on the time between calls and call background
// render which will give this information to the client.
last_interval_ = interval;
BackgroundRender();

return current_frame_;
}

bool VideoFrameCompositor::ProcessNewFrame(
const scoped_refptr<VideoFrame>& frame) {
DCHECK(compositor_task_runner_->BelongsToCurrentThread());
Expand All @@ -191,6 +214,7 @@ bool VideoFrameCompositor::ProcessNewFrame(
void VideoFrameCompositor::BackgroundRender() {
DCHECK(compositor_task_runner_->BelongsToCurrentThread());
const base::TimeTicks now = tick_clock_->NowTicks();
last_background_render_ = now;
CallRender(now, now + last_interval_, true);
}

Expand Down
14 changes: 14 additions & 0 deletions media/blink/video_frame_compositor.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,19 @@ class MEDIA_EXPORT VideoFrameCompositor
void PaintFrameUsingOldRenderingPath(
const scoped_refptr<VideoFrame>& frame) override;

// Returns |current_frame_| if |client_| is set. If no |client_| is set,
// |is_background_rendering_| is true, and |callback_| is set, it requests a
// new frame from |callback_|, using the elapsed time between calls to this
// function as the render interval; defaulting to 16.6ms if no prior calls
// have been made. A cap of 250Hz (4ms) is in place to prevent clients from
// accidentally (or intentionally) spamming the rendering pipeline.
//
// This method is primarily to facilitate canvas and WebGL based applications
// where the <video> tag is invisible (possibly not even in the DOM) and thus
// does not receive a |client_|. In this case, frame acquisition is driven by
// the frequency of canvas or WebGL paints requested via JavaScript.
scoped_refptr<VideoFrame> GetCurrentFrameAndUpdateIfStale();

void set_tick_clock_for_testing(scoped_ptr<base::TickClock> tick_clock) {
tick_clock_ = tick_clock.Pass();
}
Expand Down Expand Up @@ -146,6 +159,7 @@ class MEDIA_EXPORT VideoFrameCompositor
bool rendered_last_frame_;
bool is_background_rendering_;
base::TimeDelta last_interval_;
base::TimeTicks last_background_render_;

// These values are updated and read from the media and compositor threads.
base::Lock lock_;
Expand Down
78 changes: 65 additions & 13 deletions media/blink/video_frame_compositor_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,12 @@ class VideoFrameCompositorTest : public testing::Test,
compositor_->SetVideoFrameProviderClient(nullptr);
}

scoped_refptr<VideoFrame> CreateOpaqueFrame() {
gfx::Size size(8, 8);
return VideoFrame::CreateFrame(VideoFrame::YV12, size, gfx::Rect(size),
size, base::TimeDelta());
}

VideoFrameCompositor* compositor() { return compositor_.get(); }
int did_receive_frame_count() { return did_receive_frame_count_; }
int natural_size_changed_count() { return natural_size_changed_count_; }
Expand Down Expand Up @@ -212,8 +218,7 @@ TEST_F(VideoFrameCompositorTest, NaturalSizeChanged) {

TEST_F(VideoFrameCompositorTest, OpacityChanged) {
gfx::Size size(8, 8);
scoped_refptr<VideoFrame> opaque_frame = VideoFrame::CreateFrame(
VideoFrame::YV12, size, gfx::Rect(size), size, base::TimeDelta());
scoped_refptr<VideoFrame> opaque_frame = CreateOpaqueFrame();
scoped_refptr<VideoFrame> not_opaque_frame = VideoFrame::CreateFrame(
VideoFrame::YV12A, size, gfx::Rect(size), size, base::TimeDelta());

Expand Down Expand Up @@ -275,9 +280,7 @@ TEST_F(VideoFrameCompositorTest, OpacityChanged) {
}

TEST_F(VideoFrameCompositorTest, VideoRendererSinkFrameDropped) {
gfx::Size size(8, 8);
scoped_refptr<VideoFrame> opaque_frame = VideoFrame::CreateFrame(
VideoFrame::YV12, size, gfx::Rect(size), size, base::TimeDelta());
scoped_refptr<VideoFrame> opaque_frame = CreateOpaqueFrame();

EXPECT_CALL(*this, Render(_, _, _)).WillRepeatedly(Return(opaque_frame));
StartVideoRendererSink();
Expand Down Expand Up @@ -320,20 +323,14 @@ TEST_F(VideoFrameCompositorTest, VideoLayerShutdownWhileRendering) {
}

TEST_F(VideoFrameCompositorTest, StartFiresBackgroundRender) {
gfx::Size size(8, 8);
scoped_refptr<VideoFrame> opaque_frame = VideoFrame::CreateFrame(
VideoFrame::YV12, size, gfx::Rect(size), size, base::TimeDelta());

scoped_refptr<VideoFrame> opaque_frame = CreateOpaqueFrame();
EXPECT_CALL(*this, Render(_, _, true)).WillRepeatedly(Return(opaque_frame));
StartVideoRendererSink();
StopVideoRendererSink(true);
}

TEST_F(VideoFrameCompositorTest, BackgroundRenderTicks) {
gfx::Size size(8, 8);
scoped_refptr<VideoFrame> opaque_frame = VideoFrame::CreateFrame(
VideoFrame::YV12, size, gfx::Rect(size), size, base::TimeDelta());

scoped_refptr<VideoFrame> opaque_frame = CreateOpaqueFrame();
compositor_->set_background_rendering_for_testing(true);

base::RunLoop run_loop;
Expand All @@ -353,4 +350,59 @@ TEST_F(VideoFrameCompositorTest, BackgroundRenderTicks) {
StopVideoRendererSink(true);
}

TEST_F(VideoFrameCompositorTest, GetCurrentFrameAndUpdateIfStale) {
scoped_refptr<VideoFrame> opaque_frame_1 = CreateOpaqueFrame();
scoped_refptr<VideoFrame> opaque_frame_2 = CreateOpaqueFrame();
compositor_->set_background_rendering_for_testing(true);

// |current_frame_| should be null at this point since we don't have a client
// or a callback.
ASSERT_FALSE(compositor()->GetCurrentFrameAndUpdateIfStale());

// Starting the video renderer should return a single frame.
EXPECT_CALL(*this, Render(_, _, true)).WillOnce(Return(opaque_frame_1));
StartVideoRendererSink();

// Since we have a client, this call should not call background render, even
// if a lot of time has elapsed between calls.
tick_clock_->Advance(base::TimeDelta::FromSeconds(1));
ASSERT_EQ(opaque_frame_1, compositor()->GetCurrentFrameAndUpdateIfStale());

// An update current frame call should stop background rendering.
EXPECT_CALL(*this, Render(_, _, false)).WillOnce(Return(opaque_frame_2));
EXPECT_TRUE(
compositor()->UpdateCurrentFrame(base::TimeTicks(), base::TimeTicks()));

// This call should still not call background render.
ASSERT_EQ(opaque_frame_2, compositor()->GetCurrentFrameAndUpdateIfStale());

testing::Mock::VerifyAndClearExpectations(this);

// Clear our client, which means no mock function calls for Client.
compositor()->SetVideoFrameProviderClient(nullptr);

// This call should still not call background render, because we aren't in the
// background rendering state yet.
ASSERT_EQ(opaque_frame_2, compositor()->GetCurrentFrameAndUpdateIfStale());

// Wait for background rendering to tick again.
base::RunLoop run_loop;
EXPECT_CALL(*this, Render(_, _, true))
.WillOnce(
DoAll(RunClosure(run_loop.QuitClosure()), Return(opaque_frame_1)))
.WillOnce(Return(opaque_frame_2));
run_loop.Run();

// This call should still not call background render, because not enough time
// has elapsed since the last background render call.
ASSERT_EQ(opaque_frame_1, compositor()->GetCurrentFrameAndUpdateIfStale());

// Advancing the tick clock should allow a new frame to be requested.
tick_clock_->Advance(base::TimeDelta::FromMilliseconds(10));
ASSERT_EQ(opaque_frame_2, compositor()->GetCurrentFrameAndUpdateIfStale());

// Background rendering should tick another render callback.
StopVideoRendererSink(false);
}

} // namespace media
4 changes: 2 additions & 2 deletions media/blink/webmediaplayer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -987,15 +987,15 @@ static void GetCurrentFrameAndSignal(
scoped_refptr<VideoFrame>* video_frame_out,
base::WaitableEvent* event) {
TRACE_EVENT0("media", "GetCurrentFrameAndSignal");
*video_frame_out = compositor->GetCurrentFrame();
*video_frame_out = compositor->GetCurrentFrameAndUpdateIfStale();
event->Signal();
}

scoped_refptr<VideoFrame>
WebMediaPlayerImpl::GetCurrentFrameFromCompositor() {
TRACE_EVENT0("media", "WebMediaPlayerImpl::GetCurrentFrameFromCompositor");
if (compositor_task_runner_->BelongsToCurrentThread())
return compositor_->GetCurrentFrame();
return compositor_->GetCurrentFrameAndUpdateIfStale();

// Use a posted task and waitable event instead of a lock otherwise
// WebGL/Canvas can see different content than what the compositor is seeing.
Expand Down

0 comments on commit 44ce4de

Please sign in to comment.