Don't carry over frame overage unless frame is selected by cadence.

Don't carry over frame overage unless frame is selected by cadence.

The overage values should only be applied if the frame is actually
chosen by cadence.  This is most wrong when there is video at some
non zero time and audio at time zero.  If cadence is present the
first frame will have an obscene display count which carries over
and forever wrecks cadence :(

We should also not track render counts until the first frame has
reached its display period.

BUG=439548
TEST=new unittest

Committed: https://crrev.com/0080f370f5b84d094195ebd7e9fb7052524468b8
Cr-Commit-Position: refs/heads/master@{#329423}

[DaleCurtis, 2015-05-12 04:26:49]

dalecurtis@chromium.org changed reviewers:
+ miu@chromium.org

[DaleCurtis, 2015-05-12 04:26:49]

Found while trying to implement 3:2 cadence :)

[miu, 2015-05-12 04:49:47]

https://codereview.chromium.org/1124333012/diff/1/media/filters/video_rendere...
File media/filters/video_renderer_algorithm.cc (right):

https://codereview.chromium.org/1124333012/diff/1/media/filters/video_rendere...
media/filters/video_renderer_algorithm.cc:214: deadline_min -
frame_queue_.front().start_time >= render_interval_ / -2) {
Two comments:

1. The "divide by negative 2" feels a little weird.  Should this expression be
multiplied by -1 throughout (and the less-than operator used)?

2. Based on the way the comment is worded, it seems like this expression should
be:

  (frame_queue_.front().start_time - deadline_min).magnitude() < render_interval
/ 2
                                                  ^^^^^^^^^^^^

Since start_time could be before or after deadline_min...?

[DaleCurtis, 2015-05-12 06:13:20]

https://codereview.chromium.org/1124333012/diff/1/media/filters/video_rendere...
File media/filters/video_renderer_algorithm.cc (right):

https://codereview.chromium.org/1124333012/diff/1/media/filters/video_rendere...
media/filters/video_renderer_algorithm.cc:214: deadline_min -
frame_queue_.front().start_time >= render_interval_ / -2) {
On 2015/05/12 04:49:47, miu wrote:
> Two comments:
> 
> 1. The "divide by negative 2" feels a little weird.  Should this expression be
> multiplied by -1 throughout (and the less-than operator used)?
> 
> 2. Based on the way the comment is worded, it seems like this expression
should
> be:
> 
>   (frame_queue_.front().start_time - deadline_min).magnitude() <
render_interval
> / 2
>                                                   ^^^^^^^^^^^^
> 
> Since start_time could be before or after deadline_min...?

Yeah it's ugly, I've rearranged it to a clearer way to say what I wanted, which
is deadline_min >= start_time - interval / 2.

I don't want magnitude since if the interval lands weirdly we may not start time
until after start_time has passed.

[miu, 2015-05-12 08:16:55]

lgtm

[DaleCurtis, 2015-05-12 15:33:23]

The CQ bit was checked by dalecurtis@chromium.org

[commit-bot: I haz the power, 2015-05-12 15:34:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1124333012/20001

[commit-bot: I haz the power, 2015-05-12 16:57:26]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-05-12 16:58:18]

Patchset 2 (id:??) landed as
https://crrev.com/0080f370f5b84d094195ebd7e9fb7052524468b8
Cr-Commit-Position: refs/heads/master@{#329423}