Introduce VideoFrameScheduler{Impl}.

Introduce VideoFrameScheduler{Impl,Proxy}.

VideoFrameScheduler defines an interface for scheduling video frames to be displayed at specific a wall clock time. A single-threaded implementation that uses delayed tasks on a message loop is provided as well as a thread-safe proxy for coordinating scheduling between threads.

BUG=110814
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=264066

[acolwell GONE FROM CHROMIUM, 2014-04-14 21:17:59]

lgtm % nits

https://codereview.chromium.org/237093007/diff/20001/media/filters/video_fram...
File media/filters/video_frame_scheduler.h (right):

https://codereview.chromium.org/237093007/diff/20001/media/filters/video_fram...
media/filters/video_frame_scheduler.h:26: RESET,      // Scheduler was reset and
frames are being returned.
nit: Frames are returned in all cases so this comment seems a little misleading.
How about something along the lines of "Scheduler was reset before the frame was
scheduled for display."

https://codereview.chromium.org/237093007/diff/20001/media/filters/video_fram...
media/filters/video_frame_scheduler.h:34: // or DROPPED.
This part of the comment doesn't seem that helpful since frames are returned in
all cases right?

https://codereview.chromium.org/237093007/diff/20001/media/filters/video_fram...
File media/filters/video_frame_scheduler_impl.cc (right):

https://codereview.chromium.org/237093007/diff/20001/media/filters/video_fram...
media/filters/video_frame_scheduler_impl.cc:54: pending_frames_.top().wall_ticks
- tick_clock_->NowTicks(),
Can't this potentially result in negative delays?

https://codereview.chromium.org/237093007/diff/20001/media/filters/video_fram...
media/filters/video_frame_scheduler_impl.cc:63: PendingFrameQueue
expired_frames;
nit: This doesn't need to be a priority queue since you are extracting frames in
order.  Using a list here can avoid unnecessary rebalancing and struct copying
below.

https://codereview.chromium.org/237093007/diff/20001/media/filters/video_fram...
File media/filters/video_frame_scheduler_impl_unittest.cc (right):

https://codereview.chromium.org/237093007/diff/20001/media/filters/video_fram...
media/filters/video_frame_scheduler_impl_unittest.cc:66:
scoped_refptr<VideoFrame> frame = VideoFrame::CreateEOSFrame();
nit: I think we should DCHECK on EOS frames since they should never be passed in
and having them report as DISPLAYED or DROPPED seems wrong. Mind using
CreateBlackFrame() instead?

[scherkus (not reviewing), 2014-04-14 21:35:30]

thanks!

https://codereview.chromium.org/237093007/diff/20001/media/filters/video_fram...
File media/filters/video_frame_scheduler.h (right):

https://codereview.chromium.org/237093007/diff/20001/media/filters/video_fram...
media/filters/video_frame_scheduler.h:26: RESET,      // Scheduler was reset and
frames are being returned.
On 2014/04/14 21:17:59, acolwell wrote:
> nit: Frames are returned in all cases so this comment seems a little
misleading.
> How about something along the lines of "Scheduler was reset before the frame
was
> scheduled for display."

Done.

https://codereview.chromium.org/237093007/diff/20001/media/filters/video_fram...
media/filters/video_frame_scheduler.h:34: // or DROPPED.
On 2014/04/14 21:17:59, acolwell wrote:
> This part of the comment doesn't seem that helpful since frames are returned
in
> all cases right?

yeah -- removed

https://codereview.chromium.org/237093007/diff/20001/media/filters/video_fram...
File media/filters/video_frame_scheduler_impl.cc (right):

https://codereview.chromium.org/237093007/diff/20001/media/filters/video_fram...
media/filters/video_frame_scheduler_impl.cc:54: pending_frames_.top().wall_ticks
- tick_clock_->NowTicks(),
On 2014/04/14 21:17:59, acolwell wrote:
> Can't this potentially result in negative delays?

Yep, which results in a PostTask() instead of PostDelayedTask()

will add clarifying comment

https://codereview.chromium.org/237093007/diff/20001/media/filters/video_fram...
media/filters/video_frame_scheduler_impl.cc:63: PendingFrameQueue
expired_frames;
On 2014/04/14 21:17:59, acolwell wrote:
> nit: This doesn't need to be a priority queue since you are extracting frames
in
> order.  Using a list here can avoid unnecessary rebalancing and struct copying
> below.

Done.

https://codereview.chromium.org/237093007/diff/20001/media/filters/video_fram...
File media/filters/video_frame_scheduler_impl_unittest.cc (right):

https://codereview.chromium.org/237093007/diff/20001/media/filters/video_fram...
media/filters/video_frame_scheduler_impl_unittest.cc:66:
scoped_refptr<VideoFrame> frame = VideoFrame::CreateEOSFrame();
On 2014/04/14 21:17:59, acolwell wrote:
> nit: I think we should DCHECK on EOS frames since they should never be passed
in
> and having them report as DISPLAYED or DROPPED seems wrong. Mind using
> CreateBlackFrame() instead?

Done.

[scherkus (not reviewing), 2014-04-14 21:36:16]

The CQ bit was checked by scherkus@chromium.org

[commit-bot: I haz the power, 2014-04-14 21:36:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scherkus@chromium.org/237093007/40001

[commit-bot: I haz the power, 2014-04-14 22:00:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-14 22:00:06]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_clang_dbg
  tryserver.chromium on linux_chromium_gn_rel

[scherkus (not reviewing), 2014-04-15 00:06:27]

The CQ bit was checked by scherkus@chromium.org

[commit-bot: I haz the power, 2014-04-15 00:07:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scherkus@chromium.org/237093007/40001

[commit-bot: I haz the power, 2014-04-15 00:15:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-15 00:15:03]

Try jobs failed on following builders:
  tryserver.chromium on win_chromium_rel

[scherkus (not reviewing), 2014-04-15 00:16:38]

The CQ bit was checked by scherkus@chromium.org

[commit-bot: I haz the power, 2014-04-15 00:17:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scherkus@chromium.org/237093007/40001

[commit-bot: I haz the power, 2014-04-15 07:25:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-04-15 07:25:54]

The commit queue went berserk retrying too often for a
seemingly flaky test on builder android_dbg:
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...

[scherkus (not reviewing), 2014-04-15 16:20:39]

The CQ bit was checked by scherkus@chromium.org

[commit-bot: I haz the power, 2014-04-15 16:21:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scherkus@chromium.org/237093007/40001

[scherkus (not reviewing), 2014-04-15 19:48:39]

The CQ bit was unchecked by scherkus@chromium.org

[scherkus (not reviewing), 2014-04-15 19:48:54]

The CQ bit was checked by scherkus@chromium.org

[commit-bot: I haz the power, 2014-04-15 19:50:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scherkus@chromium.org/237093007/40001

[commit-bot: I haz the power, 2014-04-16 01:22:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/scherkus@chromium.org/237093007/40001

[commit-bot: I haz the power, 2014-04-16 02:03:44]

Change committed as 264066