Add frame refresh to VideoCaptureDevice, and buffer pool resurrection.

Add frame refresh to VideoCaptureDevice, and buffer pool resurrection.

This is the first in a series of changes to solve the problems related
to VideoCaptureDevices that can pause (i.e., basically, screen capture
devices).  This introduces new optional-to-implement methods to the
media::VideoCaptureDevice interface, and the supporting functionality in
content::VideoCaptureBufferPool.

This change should not result in any end-user-visible differences, as it
is just laying the groundwork for future changes.  For more details on
the overall plan, please see the crbug.

BUG=486274
Committed: https://crrev.com/e4a406a55b1fb94080258f5801338ff63b8dcbed
Cr-Commit-Position: refs/heads/master@{#383426}

[miu, 2016-03-23 01:10:51]

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

[miu, 2016-03-23 01:10:52]

mcasas: PTAL.  Recommend starting with video_capture_device.h, and the rest
should all make sense.  Also, the diff algorithm made things look really wonky
in video_capture_buffer_pool.cc, so if your eyes cross, feel free to just review
from line 302 to the end.

https://codereview.chromium.org/1826643003/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/media/video_capture_buffer_pool.cc (left):

https://codereview.chromium.org/1826643003/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/media/video_capture_buffer_pool.cc:20: class
SimpleBufferHandle final : public VideoCaptureBufferPool::BufferHandle {
Note: I moved this and GpuMemoryBufferBufferHandle as child classes of their
corresponding Tracker classes, as my minor changes (to dimensions tracking)
didn't otherwise fit nicely with the code structure that existed here before.

[miu, 2016-03-24 22:01:33]

Description was changed from

==========
Add frame refresh to VideoCaptureDevice, and buffer pool resurrection.

This is the first in a series of changes to solve the problems related
to VideoCaptureDevices that can pause (i.e., basically, screen capture
devices).  This introduces new optional-to-implement methods to the
media::VideoCaptureDevice interface, and the supporting functionality in
content::VideoCaptureBufferPool.

This change should not result in any end-user-visible differences, as it
is just laying the groundwork for future changes.  For more details on
the overall plan, please see the crbug.

BUG=486274
==========

to

==========
Add frame refresh to VideoCaptureDevice, and buffer pool resurrection.

This is the first in a series of changes to solve the problems related
to VideoCaptureDevices that can pause (i.e., basically, screen capture
devices).  This introduces new optional-to-implement methods to the
media::VideoCaptureDevice interface, and the supporting functionality in
content::VideoCaptureBufferPool.

This change should not result in any end-user-visible differences, as it
is just laying the groundwork for future changes.  For more details on
the overall plan, please see the crbug.

BUG=486274
==========

[miu, 2016-03-24 22:01:33]

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

[miu, 2016-03-24 22:01:46]

+emircan

[emircan, 2016-03-25 22:44:25]

lgtm % nits.

https://codereview.chromium.org/1826643003/diff/20001/content/browser/rendere...
File content/browser/renderer_host/media/video_capture_buffer_pool.cc (right):

https://codereview.chromium.org/1826643003/diff/20001/content/browser/rendere...
content/browser/renderer_host/media/video_capture_buffer_pool.cc:435: // size,
then either use the "last resort" tracker (if any) or reallocate by
"then try using |tracker_of_last_resort| or reallocate"

https://codereview.chromium.org/1826643003/diff/20001/media/capture/video/vid...
File media/capture/video/video_capture_device.cc (right):

https://codereview.chromium.org/1826643003/diff/20001/media/capture/video/vid...
media/capture/video/video_capture_device.cc:150: DVLOG(2) << "Ignoring request
for refresh frame (no implementation).";
|NOTIMPLEMENTED();| ?

https://codereview.chromium.org/1826643003/diff/20001/media/capture/video/vid...
File media/capture/video/video_capture_device.h (right):

https://codereview.chromium.org/1826643003/diff/20001/media/capture/video/vid...
media/capture/video/video_capture_device.h:249: // When this operation fails,
null will be returned.
s/null/nullptr/

https://codereview.chromium.org/1826643003/diff/20001/media/capture/video/vid...
media/capture/video/video_capture_device.h:292: virtual void
RequestRefreshFrame();
If default would be no-op, can we make it defined here directly?
virtual void RequestRefreshFrame() {}

[miu, 2016-03-25 23:13:48]

Thanks.  Addressed all the comments:

https://codereview.chromium.org/1826643003/diff/20001/content/browser/rendere...
File content/browser/renderer_host/media/video_capture_buffer_pool.cc (right):

https://codereview.chromium.org/1826643003/diff/20001/content/browser/rendere...
content/browser/renderer_host/media/video_capture_buffer_pool.cc:435: // size,
then either use the "last resort" tracker (if any) or reallocate by
On 2016/03/25 22:44:24, emircan wrote:
> "then try using |tracker_of_last_resort| or reallocate"

Done.

https://codereview.chromium.org/1826643003/diff/20001/media/capture/video/vid...
File media/capture/video/video_capture_device.cc (right):

https://codereview.chromium.org/1826643003/diff/20001/media/capture/video/vid...
media/capture/video/video_capture_device.cc:150: DVLOG(2) << "Ignoring request
for refresh frame (no implementation).";
On 2016/03/25 22:44:24, emircan wrote:
> |NOTIMPLEMENTED();| ?

This wouldn't be appropriate for normal webcams.  They all do not need to do
anything since they provide constant frame-rate video.

No longer applicable, since I made the no-op default explicit in the header file
(per your other comment).

https://codereview.chromium.org/1826643003/diff/20001/media/capture/video/vid...
File media/capture/video/video_capture_device.h (right):

https://codereview.chromium.org/1826643003/diff/20001/media/capture/video/vid...
media/capture/video/video_capture_device.h:249: // When this operation fails,
null will be returned.
On 2016/03/25 22:44:24, emircan wrote:
> s/null/nullptr/

Done.

https://codereview.chromium.org/1826643003/diff/20001/media/capture/video/vid...
media/capture/video/video_capture_device.h:292: virtual void
RequestRefreshFrame();
On 2016/03/25 22:44:24, emircan wrote:
> If default would be no-op, can we make it defined here directly?
> virtual void RequestRefreshFrame() {}

Done.

[mcasas, 2016-03-25 23:23:54]

RS LGTM

[miu, 2016-03-25 23:38:36]

The CQ bit was checked by miu@chromium.org

[miu, 2016-03-25 23:38:36]

The patchset sent to the CQ was uploaded after l-g-t-m from
emircan@chromium.org, mcasas@chromium.org
Link to the patchset: https://codereview.chromium.org/1826643003/#ps60001
(title: "REBASE before commit")

[commit-bot: I haz the power, 2016-03-25 23:38:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1826643003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1826643003/60001

[commit-bot: I haz the power, 2016-03-26 00:57:29]

Description was changed from

==========
Add frame refresh to VideoCaptureDevice, and buffer pool resurrection.

This is the first in a series of changes to solve the problems related
to VideoCaptureDevices that can pause (i.e., basically, screen capture
devices).  This introduces new optional-to-implement methods to the
media::VideoCaptureDevice interface, and the supporting functionality in
content::VideoCaptureBufferPool.

This change should not result in any end-user-visible differences, as it
is just laying the groundwork for future changes.  For more details on
the overall plan, please see the crbug.

BUG=486274
==========

to

==========
Add frame refresh to VideoCaptureDevice, and buffer pool resurrection.

This is the first in a series of changes to solve the problems related
to VideoCaptureDevices that can pause (i.e., basically, screen capture
devices).  This introduces new optional-to-implement methods to the
media::VideoCaptureDevice interface, and the supporting functionality in
content::VideoCaptureBufferPool.

This change should not result in any end-user-visible differences, as it
is just laying the groundwork for future changes.  For more details on
the overall plan, please see the crbug.

BUG=486274
==========

[commit-bot: I haz the power, 2016-03-26 00:57:29]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-03-26 00:58:48]

Description was changed from

==========
Add frame refresh to VideoCaptureDevice, and buffer pool resurrection.

This is the first in a series of changes to solve the problems related
to VideoCaptureDevices that can pause (i.e., basically, screen capture
devices).  This introduces new optional-to-implement methods to the
media::VideoCaptureDevice interface, and the supporting functionality in
content::VideoCaptureBufferPool.

This change should not result in any end-user-visible differences, as it
is just laying the groundwork for future changes.  For more details on
the overall plan, please see the crbug.

BUG=486274
==========

to

==========
Add frame refresh to VideoCaptureDevice, and buffer pool resurrection.

This is the first in a series of changes to solve the problems related
to VideoCaptureDevices that can pause (i.e., basically, screen capture
devices).  This introduces new optional-to-implement methods to the
media::VideoCaptureDevice interface, and the supporting functionality in
content::VideoCaptureBufferPool.

This change should not result in any end-user-visible differences, as it
is just laying the groundwork for future changes.  For more details on
the overall plan, please see the crbug.

BUG=486274

Committed: https://crrev.com/e4a406a55b1fb94080258f5801338ff63b8dcbed
Cr-Commit-Position: refs/heads/master@{#383426}
==========

[commit-bot: I haz the power, 2016-03-26 00:58:49]

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