[Mojo Video Capture] Replace method OnIncomingCapturedVideoFrame with OnIncomingCapturedBufferExt

[Mojo Video Capture] Replace method OnIncomingCapturedVideoFrame() with OnIncomingCapturedBufferExt()

In interface media:VideoCaptureDevice::Client, replaced the method
OnIncomingCapturedVideoFrame() with OnIncomingCapturedBufferExt(). Compared to
method OnIncomingCaptureBuffer(), the VideoFrame parameter in
OnIncomingCapturedVideoFrame() was only used for passing along a bit of extra
information, namely a custom |visible_rect| as well as some
|additional_metadata|. The new method makes the intent clear and eliminates the
need for passing a media::VideoFrame that is partly redundant with the also
passed media::VideoCaptureDevice::Client::Buffer.

This CL breaks the video_capture_unittests, because current implementation of
the video_capture service wants to use VideoCaptureDeviceClient with buffers
that are backed by MojoSharedMemoryBufferTracker instances instead of the "regular" shared memory buffers. With this CL, VideoCaptureDeviceClient is
dropping support for this. We are working towards making the video_capture
service work with the regular buffers. To keep CLs small, we defer changes
to the service to after refactorings of VideoCaptureController and
VideoCaptureManager are complete and deactive the video_capture_unittests
temporarily. This is acceptable for the time being, since they are not run on
the bots yet. The CL that re-enables them is currently labeled CL1.9.22 in the
design doc.

This CL is part of the Mojo Video Capture work. For the bigger picture,
see [1] CL1.9.9

BUG=584797
TEST=
  content_unittests --gtest_filter="*Video*",
  video_capture_unittests,
  Apprtc loopback on Debug,
  Desktop Capture Example extension on Release

[1] https://docs.google.com/a/chromium.org/document/d/1Qw7rw1AJy0QHXjha36jZNiEuxsxWslJ_X-zpOhijvI8/edit?usp=sharing

[chfremer, 2016-12-13 23:29:58]

Description was changed from

==========
presubmit


Added erroneously removed expectation in test


Merge branch '1.9.8' into 1.9.9


Merge branch '1.9.8' into 1.9.9


Merge branch '1.9.8' into 1.9.9


fix merge errors


Merge branch '1.9.8' into 1.9.9


Merge branch '1.9.8' into 1.9.9


Merge branch '1.9.8' into 1.9.9


Disable video capture service tests


Fix tests


Merge branch '1.9.8' into 1.9.9


Merge branch '1.9.8' into 1.9.9


Merge branch '1.9.8' into 1.9.9


Merge branch '1.9.8' into 1.9.9


Merge branch '1.9.8' into 1.9.9


Merge branch '1.9.8' into 1.9.9


Merge branch '1.9.8' into 1.9.9


Merge branch '1.9.8' into 1.9.9


Merge branch '1.9.8' into 1.9.9


Apply patch to 1.9.9

BUG=
==========

to

==========
[Mojo Video Capture] Replace method OnIncomingCapturedVideoFrame() with
OnIncomingCapturedBufferExt()

In interface media:VideoCaptureDevice::Client, replaced the method 
OnIncomingCapturedVideoFrame() with OnIncomingCapturedBufferExt(). Compared to 
method OnIncomingCaptureBuffer(), the VideoFrame parameter in 
OnIncomingCapturedVideoFrame() was only used for passing along a bit of extra 
information, namely a custom |visible_rect| as well as some 
|additional_metadata|. The new method makes the intent clear and eliminates the 
need for passing a media::VideoFrame that is partly redundant with the also 
passed media::VideoCaptureDevice::Client::Buffer.

This CL breaks the video_capture_unittests. This is acceptable for the time 
being, since they are not run on the bots yet. Since there are a few more CLs 
needed to finish required refactorings in VideoCaptureController and 
VideoCaptureManager, we disable them for now and will fix and re-enable them 
when after having finished the refactorings.

This CL is part of the Mojo Video Capture work. For the bigger picture, 
see [1] CL1.9.9

BUG=584797
TEST=
  content_unittests --gtest_filter="*Video*", 
  video_capture_unittests, 
  Apprtc loopback on Debug, 
  Desktop Capture Example extension on Release

[1]
https://docs.google.com/a/chromium.org/document/d/1Qw7rw1AJy0QHXjha36jZNiEuxs...
==========

[chfremer, 2016-12-13 23:30:21]

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

[commit-bot: I haz the power, 2016-12-13 23:30:48]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-13 23:47:36]

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

[commit-bot: I haz the power, 2016-12-13 23:47:37]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[chfremer, 2016-12-14 00:22:34]

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

[commit-bot: I haz the power, 2016-12-14 00:23:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-14 00:57:17]

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

[commit-bot: I haz the power, 2016-12-14 00:57:18]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[chfremer, 2016-12-14 17:52:35]

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

[commit-bot: I haz the power, 2016-12-14 17:53:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[chfremer, 2016-12-14 18:05:16]

Patchset #2 (id:20001) has been deleted

[chfremer, 2016-12-14 18:05:27]

Patchset #1 (id:1) has been deleted

[chfremer, 2016-12-14 18:07:40]

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

[chfremer, 2016-12-14 18:07:43]

miu@: PTAL
mcasas@: PTAL

[miu, 2016-12-14 20:01:44]

lgtm, but I'm curious why the tests fail? I assume you feel this doesn't break
any functionality, but I suggest explaining what's up with the tests in the
change description.

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

https://codereview.chromium.org/2566983007/diff/40001/media/capture/video/vid...
media/capture/video/video_capture_device.h:150: // Notifies
VideoCaptureDevice::Client that a video frame is ready in the
nit: "Notify" doesn't sound quite right. How about: "Provides VCD::Client with a
populated Buffer containing the content of the next video frame."

https://codereview.chromium.org/2566983007/diff/40001/media/capture/video/vid...
media/capture/video/video_capture_device.h:165: gfx::Rect visible_rect,
nit: Since this version of the method only adds args, should we maintain same
arg ordering (i.e., visible_rect goes after timestamp)?

[chfremer, 2016-12-14 20:38:00]

Description was changed from

==========
[Mojo Video Capture] Replace method OnIncomingCapturedVideoFrame() with
OnIncomingCapturedBufferExt()

In interface media:VideoCaptureDevice::Client, replaced the method 
OnIncomingCapturedVideoFrame() with OnIncomingCapturedBufferExt(). Compared to 
method OnIncomingCaptureBuffer(), the VideoFrame parameter in 
OnIncomingCapturedVideoFrame() was only used for passing along a bit of extra 
information, namely a custom |visible_rect| as well as some 
|additional_metadata|. The new method makes the intent clear and eliminates the 
need for passing a media::VideoFrame that is partly redundant with the also 
passed media::VideoCaptureDevice::Client::Buffer.

This CL breaks the video_capture_unittests. This is acceptable for the time 
being, since they are not run on the bots yet. Since there are a few more CLs 
needed to finish required refactorings in VideoCaptureController and 
VideoCaptureManager, we disable them for now and will fix and re-enable them 
when after having finished the refactorings.

This CL is part of the Mojo Video Capture work. For the bigger picture, 
see [1] CL1.9.9

BUG=584797
TEST=
  content_unittests --gtest_filter="*Video*", 
  video_capture_unittests, 
  Apprtc loopback on Debug, 
  Desktop Capture Example extension on Release

[1]
https://docs.google.com/a/chromium.org/document/d/1Qw7rw1AJy0QHXjha36jZNiEuxs...
==========

to

==========
[Mojo Video Capture] Replace method OnIncomingCapturedVideoFrame() with
OnIncomingCapturedBufferExt()

In interface media:VideoCaptureDevice::Client, replaced the method 
OnIncomingCapturedVideoFrame() with OnIncomingCapturedBufferExt(). Compared to 
method OnIncomingCaptureBuffer(), the VideoFrame parameter in 
OnIncomingCapturedVideoFrame() was only used for passing along a bit of extra 
information, namely a custom |visible_rect| as well as some 
|additional_metadata|. The new method makes the intent clear and eliminates the 
need for passing a media::VideoFrame that is partly redundant with the also 
passed media::VideoCaptureDevice::Client::Buffer.

This CL breaks the video_capture_unittests, because current implementation of 
the video_capture service wants to use VideoCaptureDeviceClient with buffers 
that are backed by MojoSharedMemoryBufferTracker instances instead of the
"regular" shared memory buffers. With this CL, VideoCaptureDeviceClient is 
dropping support for this. We are working towards making the video_capture 
service work with the regular buffers. To keep CLs small, we defer changes 
to the service to after refactorings of VideoCaptureController and 
VideoCaptureManager are complete and deactive the video_capture_unittests 
temporarily. This is acceptable for the time being, since they are not run on 
the bots yet. The CL that re-enables them is currently labeled CL1.9.22 in the
design doc.

This CL is part of the Mojo Video Capture work. For the bigger picture, 
see [1] CL1.9.9

BUG=584797
TEST=
  content_unittests --gtest_filter="*Video*", 
  video_capture_unittests, 
  Apprtc loopback on Debug, 
  Desktop Capture Example extension on Release

[1]
https://docs.google.com/a/chromium.org/document/d/1Qw7rw1AJy0QHXjha36jZNiEuxs...
==========

[chfremer, 2016-12-14 20:41:55]

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

[chfremer, 2016-12-14 20:42:28]

Addressed miu's comments.

Note: See updated description for explanation about disabled tests.

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

https://codereview.chromium.org/2566983007/diff/40001/media/capture/video/vid...
media/capture/video/video_capture_device.h:150: // Notifies
VideoCaptureDevice::Client that a video frame is ready in the
On 2016/12/14 20:01:44, miu wrote:
> nit: "Notify" doesn't sound quite right. How about: "Provides VCD::Client with
a
> populated Buffer containing the content of the next video frame."

Liking it. Done.

https://codereview.chromium.org/2566983007/diff/40001/media/capture/video/vid...
media/capture/video/video_capture_device.h:165: gfx::Rect visible_rect,
On 2016/12/14 20:01:44, miu wrote:
> nit: Since this version of the method only adds args, should we maintain same
> arg ordering (i.e., visible_rect goes after timestamp)?

Done.

[commit-bot: I haz the power, 2016-12-14 20:42:35]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[mcasas, 2016-12-14 21:04:27]

lgtm % comments

https://codereview.chromium.org/2566983007/diff/40001/media/capture/content/t...
File media/capture/content/thread_safe_capture_oracle.cc (right):

https://codereview.chromium.org/2566983007/diff/40001/media/capture/content/t...
media/capture/content/thread_safe_capture_oracle.cc:214: if
(oracle_.CompleteCapture(frame_number, success, &reference_time)) {
nit: This should use early return:

 if !(oracle_.CompleteCapture(frame_number, success, &reference_time))
  return;

https://codereview.chromium.org/2566983007/diff/40001/media/capture/content/t...
media/capture/content/thread_safe_capture_oracle.cc:234: frame->format(),
media::PIXEL_STORAGE_CPU);
Are we sure that media::PIXEL_STORAGE_CPU is always
the case? IIRC, the capture machinery could produce
textures, but probably that path is gone now. Perhaps
we could add 
  DCHECK(frame->IsMappable());
to document this?

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

https://codereview.chromium.org/2566983007/diff/40001/media/capture/video/vid...
media/capture/video/video_capture_device.h:168: const VideoFrameMetadata&
additional_metadata) = 0;
If it's not too much hassle, could we reorder these 
parameters so that the extended ones go after the 
OnIncomingCapturedBuffer() ones?

https://codereview.chromium.org/2566983007/diff/40001/media/capture/video/vid...
File media/capture/video/video_capture_device_client.cc (left):

https://codereview.chromium.org/2566983007/diff/40001/media/capture/video/vid...
media/capture/video/video_capture_device_client.cc:302: if
(buffer->IsBackedByVideoFrame()) {
I think we can remove IsBackedByVideoFrame() from
the local AutoReleaseBuffer and from the 
VideoCaptureDevice::Client::Buffer.

[chfremer, 2016-12-14 21:42:07]

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

[chfremer, 2016-12-14 21:42:29]

Addressed mcasas@ comments.

Thanks for the review!

https://codereview.chromium.org/2566983007/diff/40001/media/capture/content/t...
File media/capture/content/thread_safe_capture_oracle.cc (right):

https://codereview.chromium.org/2566983007/diff/40001/media/capture/content/t...
media/capture/content/thread_safe_capture_oracle.cc:214: if
(oracle_.CompleteCapture(frame_number, success, &reference_time)) {
On 2016/12/14 21:04:27, mcasas wrote:
> nit: This should use early return:
> 
>  if !(oracle_.CompleteCapture(frame_number, success, &reference_time))
>   return;

Done.

https://codereview.chromium.org/2566983007/diff/40001/media/capture/content/t...
media/capture/content/thread_safe_capture_oracle.cc:234: frame->format(),
media::PIXEL_STORAGE_CPU);
On 2016/12/14 21:04:27, mcasas wrote:
> Are we sure that media::PIXEL_STORAGE_CPU is always
> the case? IIRC, the capture machinery could produce
> textures, but probably that path is gone now. Perhaps
> we could add 
>   DCHECK(frame->IsMappable());
> to document this?

Good point. Right now we are sure, because it is the only possible value for
this enum [1]. 

Added the DCHECK.

[1]
https://cs.chromium.org/chromium/src/media/capture/video_capture_types.h?q=PI...

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

https://codereview.chromium.org/2566983007/diff/40001/media/capture/video/vid...
media/capture/video/video_capture_device.h:168: const VideoFrameMetadata&
additional_metadata) = 0;
On 2016/12/14 21:04:27, mcasas wrote:
> If it's not too much hassle, could we reorder these 
> parameters so that the extended ones go after the 
> OnIncomingCapturedBuffer() ones?

Done.

https://codereview.chromium.org/2566983007/diff/40001/media/capture/video/vid...
File media/capture/video/video_capture_device_client.cc (left):

https://codereview.chromium.org/2566983007/diff/40001/media/capture/video/vid...
media/capture/video/video_capture_device_client.cc:302: if
(buffer->IsBackedByVideoFrame()) {
On 2016/12/14 21:04:27, mcasas wrote:
> I think we can remove IsBackedByVideoFrame() from
> the local AutoReleaseBuffer and from the 
> VideoCaptureDevice::Client::Buffer.

Good observation. Actually, since the next CL1.9.10 will simplify
VideoCaptureDevice::Client::Buffer to a struct and eliminate the need for
AutoReleaseBuffer, I decided to bundle all changes to
VideoCaptureDevice::Client::Buffer to that upcoming CL.

[commit-bot: I haz the power, 2016-12-14 21:43:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-14 23:10:54]

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

[commit-bot: I haz the power, 2016-12-14 23:10:55]

Dry run: This issue passed the CQ dry run.

[chfremer, 2016-12-14 23:15:09]

The CQ bit was checked by chfremer@chromium.org

[chfremer, 2016-12-14 23:15:09]

The patchset sent to the CQ was uploaded after l-g-t-m from miu@chromium.org,
mcasas@chromium.org
Link to the patchset: https://codereview.chromium.org/2566983007/#ps80001
(title: "mcasas comments")

[commit-bot: I haz the power, 2016-12-14 23:16:10]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-14 23:35:02]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1481757309385980,
"parent_rev": "8ecb5425c0d35506d505068b33d61f8d42979e92", "commit_rev":
"972b78f53b998cb43bd99039c039a315c5542932"}

[commit-bot: I haz the power, 2016-12-14 23:35:42]

Description was changed from

==========
[Mojo Video Capture] Replace method OnIncomingCapturedVideoFrame() with
OnIncomingCapturedBufferExt()

In interface media:VideoCaptureDevice::Client, replaced the method 
OnIncomingCapturedVideoFrame() with OnIncomingCapturedBufferExt(). Compared to 
method OnIncomingCaptureBuffer(), the VideoFrame parameter in 
OnIncomingCapturedVideoFrame() was only used for passing along a bit of extra 
information, namely a custom |visible_rect| as well as some 
|additional_metadata|. The new method makes the intent clear and eliminates the 
need for passing a media::VideoFrame that is partly redundant with the also 
passed media::VideoCaptureDevice::Client::Buffer.

This CL breaks the video_capture_unittests, because current implementation of 
the video_capture service wants to use VideoCaptureDeviceClient with buffers 
that are backed by MojoSharedMemoryBufferTracker instances instead of the
"regular" shared memory buffers. With this CL, VideoCaptureDeviceClient is 
dropping support for this. We are working towards making the video_capture 
service work with the regular buffers. To keep CLs small, we defer changes 
to the service to after refactorings of VideoCaptureController and 
VideoCaptureManager are complete and deactive the video_capture_unittests 
temporarily. This is acceptable for the time being, since they are not run on 
the bots yet. The CL that re-enables them is currently labeled CL1.9.22 in the
design doc.

This CL is part of the Mojo Video Capture work. For the bigger picture, 
see [1] CL1.9.9

BUG=584797
TEST=
  content_unittests --gtest_filter="*Video*", 
  video_capture_unittests, 
  Apprtc loopback on Debug, 
  Desktop Capture Example extension on Release

[1]
https://docs.google.com/a/chromium.org/document/d/1Qw7rw1AJy0QHXjha36jZNiEuxs...
==========

to

==========
[Mojo Video Capture] Replace method OnIncomingCapturedVideoFrame() with
OnIncomingCapturedBufferExt()

In interface media:VideoCaptureDevice::Client, replaced the method 
OnIncomingCapturedVideoFrame() with OnIncomingCapturedBufferExt(). Compared to 
method OnIncomingCaptureBuffer(), the VideoFrame parameter in 
OnIncomingCapturedVideoFrame() was only used for passing along a bit of extra 
information, namely a custom |visible_rect| as well as some 
|additional_metadata|. The new method makes the intent clear and eliminates the 
need for passing a media::VideoFrame that is partly redundant with the also 
passed media::VideoCaptureDevice::Client::Buffer.

This CL breaks the video_capture_unittests, because current implementation of 
the video_capture service wants to use VideoCaptureDeviceClient with buffers 
that are backed by MojoSharedMemoryBufferTracker instances instead of the
"regular" shared memory buffers. With this CL, VideoCaptureDeviceClient is 
dropping support for this. We are working towards making the video_capture 
service work with the regular buffers. To keep CLs small, we defer changes 
to the service to after refactorings of VideoCaptureController and 
VideoCaptureManager are complete and deactive the video_capture_unittests 
temporarily. This is acceptable for the time being, since they are not run on 
the bots yet. The CL that re-enables them is currently labeled CL1.9.22 in the
design doc.

This CL is part of the Mojo Video Capture work. For the bigger picture, 
see [1] CL1.9.9

BUG=584797
TEST=
  content_unittests --gtest_filter="*Video*", 
  video_capture_unittests, 
  Apprtc loopback on Debug, 
  Desktop Capture Example extension on Release

[1]
https://docs.google.com/a/chromium.org/document/d/1Qw7rw1AJy0QHXjha36jZNiEuxs...

Review-Url: https://codereview.chromium.org/2566983007
==========

[commit-bot: I haz the power, 2016-12-14 23:35:44]

Committed patchset #3 (id:80001)

[commit-bot: I haz the power, 2016-12-14 23:39:27]

Description was changed from

==========
[Mojo Video Capture] Replace method OnIncomingCapturedVideoFrame() with
OnIncomingCapturedBufferExt()

In interface media:VideoCaptureDevice::Client, replaced the method 
OnIncomingCapturedVideoFrame() with OnIncomingCapturedBufferExt(). Compared to 
method OnIncomingCaptureBuffer(), the VideoFrame parameter in 
OnIncomingCapturedVideoFrame() was only used for passing along a bit of extra 
information, namely a custom |visible_rect| as well as some 
|additional_metadata|. The new method makes the intent clear and eliminates the 
need for passing a media::VideoFrame that is partly redundant with the also 
passed media::VideoCaptureDevice::Client::Buffer.

This CL breaks the video_capture_unittests, because current implementation of 
the video_capture service wants to use VideoCaptureDeviceClient with buffers 
that are backed by MojoSharedMemoryBufferTracker instances instead of the
"regular" shared memory buffers. With this CL, VideoCaptureDeviceClient is 
dropping support for this. We are working towards making the video_capture 
service work with the regular buffers. To keep CLs small, we defer changes 
to the service to after refactorings of VideoCaptureController and 
VideoCaptureManager are complete and deactive the video_capture_unittests 
temporarily. This is acceptable for the time being, since they are not run on 
the bots yet. The CL that re-enables them is currently labeled CL1.9.22 in the
design doc.

This CL is part of the Mojo Video Capture work. For the bigger picture, 
see [1] CL1.9.9

BUG=584797
TEST=
  content_unittests --gtest_filter="*Video*", 
  video_capture_unittests, 
  Apprtc loopback on Debug, 
  Desktop Capture Example extension on Release

[1]
https://docs.google.com/a/chromium.org/document/d/1Qw7rw1AJy0QHXjha36jZNiEuxs...

Review-Url: https://codereview.chromium.org/2566983007
==========

to

==========
[Mojo Video Capture] Replace method OnIncomingCapturedVideoFrame() with
OnIncomingCapturedBufferExt()

In interface media:VideoCaptureDevice::Client, replaced the method
OnIncomingCapturedVideoFrame() with OnIncomingCapturedBufferExt(). Compared to
method OnIncomingCaptureBuffer(), the VideoFrame parameter in
OnIncomingCapturedVideoFrame() was only used for passing along a bit of extra
information, namely a custom |visible_rect| as well as some
|additional_metadata|. The new method makes the intent clear and eliminates the
need for passing a media::VideoFrame that is partly redundant with the also
passed media::VideoCaptureDevice::Client::Buffer.

This CL breaks the video_capture_unittests, because current implementation of
the video_capture service wants to use VideoCaptureDeviceClient with buffers
that are backed by MojoSharedMemoryBufferTracker instances instead of the
"regular" shared memory buffers. With this CL, VideoCaptureDeviceClient is
dropping support for this. We are working towards making the video_capture
service work with the regular buffers. To keep CLs small, we defer changes
to the service to after refactorings of VideoCaptureController and
VideoCaptureManager are complete and deactive the video_capture_unittests
temporarily. This is acceptable for the time being, since they are not run on
the bots yet. The CL that re-enables them is currently labeled CL1.9.22 in the
design doc.

This CL is part of the Mojo Video Capture work. For the bigger picture,
see [1] CL1.9.9

BUG=584797
TEST=
  content_unittests --gtest_filter="*Video*",
  video_capture_unittests,
  Apprtc loopback on Debug,
  Desktop Capture Example extension on Release

[1]
https://docs.google.com/a/chromium.org/document/d/1Qw7rw1AJy0QHXjha36jZNiEuxs...

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

[commit-bot: I haz the power, 2016-12-14 23:39:28]

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

[chfremer, 2016-12-16 21:59:23]

A revert of this CL (patchset #3 id:80001) has been created in
https://codereview.chromium.org/2582883002/ by chfremer@chromium.org.

The reason for reverting is: Broke a test in webrtc.peerconnection, see
crbug.com/675020.

[chfremer, 2016-12-22 01:07:40]

Description was changed from

==========
[Mojo Video Capture] Replace method OnIncomingCapturedVideoFrame() with
OnIncomingCapturedBufferExt()

In interface media:VideoCaptureDevice::Client, replaced the method
OnIncomingCapturedVideoFrame() with OnIncomingCapturedBufferExt(). Compared to
method OnIncomingCaptureBuffer(), the VideoFrame parameter in
OnIncomingCapturedVideoFrame() was only used for passing along a bit of extra
information, namely a custom |visible_rect| as well as some
|additional_metadata|. The new method makes the intent clear and eliminates the
need for passing a media::VideoFrame that is partly redundant with the also
passed media::VideoCaptureDevice::Client::Buffer.

This CL breaks the video_capture_unittests, because current implementation of
the video_capture service wants to use VideoCaptureDeviceClient with buffers
that are backed by MojoSharedMemoryBufferTracker instances instead of the
"regular" shared memory buffers. With this CL, VideoCaptureDeviceClient is
dropping support for this. We are working towards making the video_capture
service work with the regular buffers. To keep CLs small, we defer changes
to the service to after refactorings of VideoCaptureController and
VideoCaptureManager are complete and deactive the video_capture_unittests
temporarily. This is acceptable for the time being, since they are not run on
the bots yet. The CL that re-enables them is currently labeled CL1.9.22 in the
design doc.

This CL is part of the Mojo Video Capture work. For the bigger picture,
see [1] CL1.9.9

BUG=584797
TEST=
  content_unittests --gtest_filter="*Video*",
  video_capture_unittests,
  Apprtc loopback on Debug,
  Desktop Capture Example extension on Release

[1]
https://docs.google.com/a/chromium.org/document/d/1Qw7rw1AJy0QHXjha36jZNiEuxs...

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

to

==========
Reland [Mojo Video Capture] Replace method OnIncomingCapturedVideoFrame() with
OnIncomingCapturedBufferExt()

This is exactly the same as reviewed previously 
https://codereview.chromium.org/2592303002/

The reason for the revert was a crash in a performance test on Android, which 
pointed to a bad file handle while Mojo-unwrapping a shared buffer. This was 
most likely caused by the same bug 675528 that was just fixed for the previous 
CL1.9.8. I ran the test on a local Android device an confirmed that it is no 
longer crashing.


Description of original CL:

In interface media:VideoCaptureDevice::Client, replaced the method
OnIncomingCapturedVideoFrame() with OnIncomingCapturedBufferExt(). Compared to
method OnIncomingCaptureBuffer(), the VideoFrame parameter in
OnIncomingCapturedVideoFrame() was only used for passing along a bit of extra
information, namely a custom |visible_rect| as well as some
|additional_metadata|. The new method makes the intent clear and eliminates the
need for passing a media::VideoFrame that is partly redundant with the also
passed media::VideoCaptureDevice::Client::Buffer.

This CL breaks the video_capture_unittests, because current implementation of
the video_capture service wants to use VideoCaptureDeviceClient with buffers
that are backed by MojoSharedMemoryBufferTracker instances instead of the
"regular" shared memory buffers. With this CL, VideoCaptureDeviceClient is
dropping support for this. We are working towards making the video_capture
service work with the regular buffers. To keep CLs small, we defer changes
to the service to after refactorings of VideoCaptureController and
VideoCaptureManager are complete and deactive the video_capture_unittests
temporarily. This is acceptable for the time being, since they are not run on
the bots yet. The CL that re-enables them is currently labeled CL1.9.22 in the
design doc.

This CL is part of the Mojo Video Capture work. For the bigger picture,
see [1] CL1.9.9

BUG=584797
TEST=
  content_unittests --gtest_filter="*Video*",
  video_capture_unittests,
  Apprtc loopback on Debug,
  Desktop Capture Example extension on Release

[1]
https://docs.google.com/a/chromium.org/document/d/1Qw7rw1AJy0QHXjha36jZNiEuxs...
==========

[chfremer, 2016-12-22 01:08:11]

Description was changed from

==========
Reland [Mojo Video Capture] Replace method OnIncomingCapturedVideoFrame() with
OnIncomingCapturedBufferExt()

This is exactly the same as reviewed previously 
https://codereview.chromium.org/2592303002/

The reason for the revert was a crash in a performance test on Android, which 
pointed to a bad file handle while Mojo-unwrapping a shared buffer. This was 
most likely caused by the same bug 675528 that was just fixed for the previous 
CL1.9.8. I ran the test on a local Android device an confirmed that it is no 
longer crashing.


Description of original CL:

In interface media:VideoCaptureDevice::Client, replaced the method
OnIncomingCapturedVideoFrame() with OnIncomingCapturedBufferExt(). Compared to
method OnIncomingCaptureBuffer(), the VideoFrame parameter in
OnIncomingCapturedVideoFrame() was only used for passing along a bit of extra
information, namely a custom |visible_rect| as well as some
|additional_metadata|. The new method makes the intent clear and eliminates the
need for passing a media::VideoFrame that is partly redundant with the also
passed media::VideoCaptureDevice::Client::Buffer.

This CL breaks the video_capture_unittests, because current implementation of
the video_capture service wants to use VideoCaptureDeviceClient with buffers
that are backed by MojoSharedMemoryBufferTracker instances instead of the
"regular" shared memory buffers. With this CL, VideoCaptureDeviceClient is
dropping support for this. We are working towards making the video_capture
service work with the regular buffers. To keep CLs small, we defer changes
to the service to after refactorings of VideoCaptureController and
VideoCaptureManager are complete and deactive the video_capture_unittests
temporarily. This is acceptable for the time being, since they are not run on
the bots yet. The CL that re-enables them is currently labeled CL1.9.22 in the
design doc.

This CL is part of the Mojo Video Capture work. For the bigger picture,
see [1] CL1.9.9

BUG=584797
TEST=
  content_unittests --gtest_filter="*Video*",
  video_capture_unittests,
  Apprtc loopback on Debug,
  Desktop Capture Example extension on Release

[1]
https://docs.google.com/a/chromium.org/document/d/1Qw7rw1AJy0QHXjha36jZNiEuxs...
==========

to

==========
[Mojo Video Capture] Replace method OnIncomingCapturedVideoFrame() with
OnIncomingCapturedBufferExt()

In interface media:VideoCaptureDevice::Client, replaced the method
OnIncomingCapturedVideoFrame() with OnIncomingCapturedBufferExt(). Compared to
method OnIncomingCaptureBuffer(), the VideoFrame parameter in
OnIncomingCapturedVideoFrame() was only used for passing along a bit of extra
information, namely a custom |visible_rect| as well as some
|additional_metadata|. The new method makes the intent clear and eliminates the
need for passing a media::VideoFrame that is partly redundant with the also
passed media::VideoCaptureDevice::Client::Buffer.

This CL breaks the video_capture_unittests, because current implementation of
the video_capture service wants to use VideoCaptureDeviceClient with buffers
that are backed by MojoSharedMemoryBufferTracker instances instead of the
"regular" shared memory buffers. With this CL, VideoCaptureDeviceClient is
dropping support for this. We are working towards making the video_capture
service work with the regular buffers. To keep CLs small, we defer changes
to the service to after refactorings of VideoCaptureController and
VideoCaptureManager are complete and deactive the video_capture_unittests
temporarily. This is acceptable for the time being, since they are not run on
the bots yet. The CL that re-enables them is currently labeled CL1.9.22 in the
design doc.

This CL is part of the Mojo Video Capture work. For the bigger picture,
see [1] CL1.9.9

BUG=584797
TEST=
  content_unittests --gtest_filter="*Video*",
  video_capture_unittests,
  Apprtc loopback on Debug,
  Desktop Capture Example extension on Release

[1]
https://docs.google.com/a/chromium.org/document/d/1Qw7rw1AJy0QHXjha36jZNiEuxs...
==========