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

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/1Qw7rw1AJy0QHXjha36jZNiEuxsxWslJ_X-zpOhijvI8/edit?usp=sharing

Committed: https://crrev.com/fc2e2a6f6597cf6582673572bbf22268e445dac0
Cr-Commit-Position: refs/heads/master@{#441391}

[chfremer, 2016-12-22 01:05:22]

Description was changed from

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

to

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

This is exactly the same as reviewed previously.
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:26]

Description was changed from

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

This is exactly the same as reviewed previously.
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

==========
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:09:13]

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

[chfremer, 2016-12-22 01:09:13]

mcasas@: Please RS
miu@: Please RS

[chfremer, 2016-12-22 01:09:20]

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

[commit-bot: I haz the power, 2016-12-22 01:09:49]

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

[miu, 2016-12-22 01:30:31]

lgtm

RS lgtm

[miu, 2016-12-22 01:30:31]

lgtm

lgtm

RS lgtm

[miu, 2016-12-22 01:30:58]

On 2016/12/22 01:30:31, miu wrote:
> lgtm
> 
> lgtm
> 
> RS lgtm

Woah! Go rapid-click mode! Sorry for the spam. ;)

[mcasas, 2016-12-22 01:31:49]

On 2016/12/22 01:09:13, chfremer wrote:
> mcasas@: Please RS
> miu@: Please RS

still lgtm since the code is ==

[commit-bot: I haz the power, 2016-12-22 02:45:16]

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

[commit-bot: I haz the power, 2016-12-22 02:45:17]

Dry run: This issue passed the CQ dry run.

[chfremer, 2017-01-03 23:16:09]

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

[commit-bot: I haz the power, 2017-01-03 23:16:28]

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, 2017-01-04 02:43:50]

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

[commit-bot: I haz the power, 2017-01-04 02:43:51]

Dry run: This issue passed the CQ dry run.

[chfremer1, 2017-01-04 16:33:34]

The CQ bit was checked by chfremer@google.com

[chfremer1, 2017-01-04 16:33:35]

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

[commit-bot: I haz the power, 2017-01-04 16:33:57]

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, 2017-01-04 16:38:21]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1483547614954340,
"parent_rev": "80816623aec17a8ff64995cc3a0580fa78cf3f28", "commit_rev":
"4f0834b23eabc76d275c4393f791b451b1ee5ab3"}

[commit-bot: I haz the power, 2017-01-04 16:38:54]

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

==========
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...

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

[commit-bot: I haz the power, 2017-01-04 16:38:55]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2017-01-04 16:42:18]

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...

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

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...

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

[commit-bot: I haz the power, 2017-01-04 16:42:22]

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

[chfremer, 2017-01-07 01:24:22]

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

The reason for reverting is: This caused a crash during video capture, see 
https://bugs.chromium.org/p/chromium/issues/detail?id=678766.