FileVideoCaptureDevice: Regulate framerate without drifting

FileVideoCaptureDevice: Regulate framerate without drifting

We currently rely on PostDelayedTask with a constant frame interval to get a specified frame rate.
This CL dynamically adapts the delay to PostDelayedTask to keep the frame rate in sync.

BUG=492189
Committed: https://crrev.com/fa660905fb2f7cd1dc12ea4c3bc609c6fa85966d
Cr-Commit-Position: refs/heads/master@{#332827}

[magjed_chromium, 2015-06-04 09:17:03]

The CQ bit was checked by magjed@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-06-04 09:17:15]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1162903004/1

[magjed_chromium, 2015-06-04 09:17:45]

magjed@chromium.org changed reviewers:
+ tommi@chromium.org

[magjed_chromium, 2015-06-04 09:18:01]

tommi - Please take a look.

[commit-bot: I haz the power, 2015-06-04 10:14:30]

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

[commit-bot: I haz the power, 2015-06-04 10:14:32]

Dry run: This issue passed the CQ dry run.

[tommi (sloooow) - chröme, 2015-06-04 12:34:09]

lgtm

[magjed_chromium, 2015-06-04 13:24:52]

The CQ bit was checked by magjed@chromium.org

[commit-bot: I haz the power, 2015-06-04 13:25:29]

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

[commit-bot: I haz the power, 2015-06-04 13:58:52]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-06-04 13:59:51]

Patchset 1 (id:??) landed as
https://crrev.com/fa660905fb2f7cd1dc12ea4c3bc609c6fa85966d
Cr-Commit-Position: refs/heads/master@{#332827}

[brucedawson, 2015-06-04 20:16:45]

brucedawson@chromium.org changed reviewers:
+ brucedawson@chromium.org

[brucedawson, 2015-06-04 20:16:45]

This looks great and should be much more stable.

However it looks like the code could hit problems if it is ever unable to meet
frame rate. If the target frame rate is 60 fps and it runs at 30 fps for a
second then I think that next_frame_time will be half a second in the past which
means that a negative time will be passed to PostDelayedTask. That should cause
the task to run immediately, so there would be no immediate bad consequences.

If the code then started running at 1,000 fps (unlikely, but let's hypothesize)
then all of the skipped frames would be generated as quickly as possible -- the
code would try to 'catch up'.

I think that 'catch up' is not the desired behavior. Should this check be added?

if (next_frame_time_ < current_time)
    next_frame_time_ = current_time;

[magjed_chromium, 2015-06-05 12:26:55]

On 2015/06/04 20:16:45, brucedawson wrote:
> This looks great and should be much more stable.
> 
> However it looks like the code could hit problems if it is ever unable to meet
> frame rate. If the target frame rate is 60 fps and it runs at 30 fps for a
> second then I think that next_frame_time will be half a second in the past
which
> means that a negative time will be passed to PostDelayedTask. That should
cause
> the task to run immediately, so there would be no immediate bad consequences.
> 
> If the code then started running at 1,000 fps (unlikely, but let's
hypothesize)
> then all of the skipped frames would be generated as quickly as possible --
the
> code would try to 'catch up'.
> 
> I think that 'catch up' is not the desired behavior. Should this check be
added?
> 
> if (next_frame_time_ < current_time)
>     next_frame_time_ = current_time;

I agree, your change makes the behavior more similar to a normal webcam. I
uploaded a new CL here: https://codereview.chromium.org/1170573003/