VideoRendererImpl: Delay report of BUFFERING_HAVE_NOTHING.

VideoRendererImpl: Delay report of BUFFERING_HAVE_NOTHING.

Currently VideoRendererImpl triggers the pipeline to underflow immediately after
it runs out of frame. This is not desired because relatively small video
decoding glitch would cause audio to stutter as well, which is more noticeable
than some dropped video frames. With this CL, VRI only declares
BUFFERING_HAVE_NOTHING after it has run out of buffers and have not painted any
frame for certain amount of time (e.g. 3s in media time). In other words, video
can still trigger pipeline underflow but won't do this until really necessary.

BUG=144683
TEST=Tested by making video decoder slower. See
https://codereview.chromium.org/464113003/

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

[DaleCurtis, 2014-08-14 19:47:27]

lgtm % up to you nit.

https://codereview.chromium.org/475863002/diff/20001/media/filters/video_rend...
File media/filters/video_renderer_impl_unittest.cc (right):

https://codereview.chromium.org/475863002/diff/20001/media/filters/video_rend...
media/filters/video_renderer_impl_unittest.cc:544: AdvanceTimeInMs(3000); //
Must match kTimeToDeclareHaveNothing.
Up to you, put this as an enum in the header file?

[acolwell GONE FROM CHROMIUM, 2014-08-14 19:49:27]

The CQ bit was checked by acolwell@chromium.org

[acolwell GONE FROM CHROMIUM, 2014-08-14 19:51:22]

Thanks for the review.

https://codereview.chromium.org/475863002/diff/20001/media/filters/video_rend...
File media/filters/video_renderer_impl_unittest.cc (right):

https://codereview.chromium.org/475863002/diff/20001/media/filters/video_rend...
media/filters/video_renderer_impl_unittest.cc:544: AdvanceTimeInMs(3000); //
Must match kTimeToDeclareHaveNothing.
On 2014/08/14 19:47:26, DaleCurtis wrote:
> Up to you, put this as an enum in the header file?

I don't think it is worth it and I don't really want other code to be tempted to
access the definition. If we need more than this instance I'll reconsider.

[commit-bot: I haz the power, 2014-08-14 19:52:44]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/acolwell@chromium.org/475863002/20001

[commit-bot: I haz the power, 2014-08-15 03:45:24]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)

[commit-bot: I haz the power, 2014-08-15 04:43:58]

Committed patchset #2 (20001) as 289786

[scherkus (not reviewing), 2014-08-22 01:41:00]

https://codereview.chromium.org/475863002/diff/20001/media/filters/video_rend...
File media/filters/video_renderer_impl.cc (right):

https://codereview.chromium.org/475863002/diff/20001/media/filters/video_rend...
media/filters/video_renderer_impl.cc:206: base::TimeDelta now =
get_time_cb_.Run();
did we mean to use *media* time instead of wall clock time?

if so ... I think we've made it impossible for that condition to evaluate to
true in the video-only case since media time is controlled via running
max_time_cb_ inside this class.

we might need to have a quick pow-wow with everyone in the office on Friday --
my brain is hurting thinking about these edge cases.