ImageCapture: Implement takePhoto() for Windows

ImageCapture: Implement takePhoto() for Windows

This CL implements VideoCaptureDevice::TakePhoto() for
Windows DirectShow (which is essentially used for all
Windows re. video capture). For that, the V4L2CaptureDelegate::GetPhotoBlob() is factored out
into media/capture/video/blob_utils.{cc,h}, and renamed
to Blobify().

BUG=648490
TEST=Enabled VideoCaptureDeviceTest::TakePhoto in Win.
All other ImageCapture related tests still passing.

Committed: https://crrev.com/3ff7e8f966c0c56019cb51cdbb93316a772191b6
Cr-Commit-Position: refs/heads/master@{#419915}

[mcasas, 2016-09-20 02:05:33]

Patchset #1 (id:1) has been deleted

[mcasas, 2016-09-20 03:01:42]

Description was changed from

==========
ImageCapture: Implement takePhoto() for Windows

wip

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

to

==========
ImageCapture: Implement takePhoto() for Windows

This CL implements VideoCaptureDevice::TakePhoto() for
Windows DirectShow (which is essentially used for all 
Windows re. video capture). For that, the V4L2CaptureDelegate::GetPhotoBlob() is
factored out
into media/capture/video/blob_utils.{cc,h}, and renamed
to Blobify().

BUG=648490
TEST=Enabled VideoCaptureDeviceTest::TakePhoto in Win.
==========

[mcasas, 2016-09-20 03:02:05]

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

[mcasas, 2016-09-20 03:02:06]

xianglu@ PTAL

[xianglu, 2016-09-20 18:42:07]

On 2016/09/20 03:02:06, mcasas wrote:
> xianglu@ PTAL

[xianglu, 2016-09-20 18:43:04]

https://codereview.chromium.org/2354783002/diff/10009/media/capture/video/blo...
File media/capture/video/blob_utils.cc (right):

https://codereview.chromium.org/2354783002/diff/10009/media/capture/video/blo...
media/capture/video/blob_utils.cc:6: 
nit: redundant line

https://codereview.chromium.org/2354783002/diff/10009/media/capture/video/blo...
media/capture/video/blob_utils.cc:44: std::unique_ptr<uint8_t[]> tmp_argb(new
uint8_t[VideoFrame::AllocationSize(
Since it's used several times below, maybe you can factor out this too:
const gfx::Size frame_size = capture_format.frame_size;

https://codereview.chromium.org/2354783002/diff/10009/media/capture/video/blo...
File media/capture/video/blob_utils.h (right):

https://codereview.chromium.org/2354783002/diff/10009/media/capture/video/blo...
media/capture/video/blob_utils.h:19: VideoCaptureFormat capture_format);
Consider:
    const VideoCaptureFormat& capture_format
Prefer pass-by-reference to pass-by-value, especially when you are going to make
a copy of its members inside the function.

[mcasas, 2016-09-20 19:50:51]

Description was changed from

==========
ImageCapture: Implement takePhoto() for Windows

This CL implements VideoCaptureDevice::TakePhoto() for
Windows DirectShow (which is essentially used for all 
Windows re. video capture). For that, the V4L2CaptureDelegate::GetPhotoBlob() is
factored out
into media/capture/video/blob_utils.{cc,h}, and renamed
to Blobify().

BUG=648490
TEST=Enabled VideoCaptureDeviceTest::TakePhoto in Win.
==========

to

==========
ImageCapture: Implement takePhoto() for Windows

This CL implements VideoCaptureDevice::TakePhoto() for
Windows DirectShow (which is essentially used for all 
Windows re. video capture). For that, the V4L2CaptureDelegate::GetPhotoBlob() is
factored out
into media/capture/video/blob_utils.{cc,h}, and renamed
to Blobify().

BUG=648490
TEST=Enabled VideoCaptureDeviceTest::TakePhoto in Win.
All other ImageCapture related tests still passing.
==========

[mcasas, 2016-09-20 19:51:10]

ptal

https://codereview.chromium.org/2354783002/diff/10009/media/capture/video/blo...
File media/capture/video/blob_utils.cc (right):

https://codereview.chromium.org/2354783002/diff/10009/media/capture/video/blo...
media/capture/video/blob_utils.cc:6: 
On 2016/09/20 18:43:03, xianglu wrote:
> nit: redundant line

?
l.5 needs to be separated [1], and I should
include both l.7 and l.8: "If you rely on 
symbols from bar.h, don't count on the fact 
that you included foo.h which (currently) 
includes bar.h: include bar.h yourself" ([1]).

[1]
https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes

https://codereview.chromium.org/2354783002/diff/10009/media/capture/video/blo...
media/capture/video/blob_utils.cc:44: std::unique_ptr<uint8_t[]> tmp_argb(new
uint8_t[VideoFrame::AllocationSize(
On 2016/09/20 18:43:03, xianglu wrote:
> Since it's used several times below, maybe you can factor out this too:
> const gfx::Size frame_size = capture_format.frame_size;

Done.

https://codereview.chromium.org/2354783002/diff/10009/media/capture/video/blo...
File media/capture/video/blob_utils.h (right):

https://codereview.chromium.org/2354783002/diff/10009/media/capture/video/blo...
media/capture/video/blob_utils.h:19: VideoCaptureFormat capture_format);
On 2016/09/20 18:43:03, xianglu wrote:
> Consider:
>     const VideoCaptureFormat& capture_format
> Prefer pass-by-reference to pass-by-value, especially when you are going to
make
> a copy of its members inside the function.

Good one. Done.

[xianglu, 2016-09-20 20:42:58]

lgtm.

https://codereview.chromium.org/2354783002/diff/10009/media/capture/video/blo...
File media/capture/video/blob_utils.cc (right):

https://codereview.chromium.org/2354783002/diff/10009/media/capture/video/blo...
media/capture/video/blob_utils.cc:6: 
On 2016/09/20 19:51:10, mcasas wrote:
> On 2016/09/20 18:43:03, xianglu wrote:
> > nit: redundant line
> 
> ?
> l.5 needs to be separated [1], and I should
> include both l.7 and l.8: "If you rely on 
> symbols from bar.h, don't count on the fact 
> that you included foo.h which (currently) 
> includes bar.h: include bar.h yourself" ([1]).
> 
> [1]
> https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes

I thought l.6 was redundant.

[mcasas, 2016-09-20 22:05:44]

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

[commit-bot: I haz the power, 2016-09-20 22:06:40]

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-09-20 23:40:04]

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

[commit-bot: I haz the power, 2016-09-20 23:40:05]

Dry run: This issue passed the CQ dry run.

[mcasas, 2016-09-21 00:09:37]

The CQ bit was checked by mcasas@chromium.org

[commit-bot: I haz the power, 2016-09-21 00:10:08]

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-09-21 00:17:24]

Description was changed from

==========
ImageCapture: Implement takePhoto() for Windows

This CL implements VideoCaptureDevice::TakePhoto() for
Windows DirectShow (which is essentially used for all 
Windows re. video capture). For that, the V4L2CaptureDelegate::GetPhotoBlob() is
factored out
into media/capture/video/blob_utils.{cc,h}, and renamed
to Blobify().

BUG=648490
TEST=Enabled VideoCaptureDeviceTest::TakePhoto in Win.
All other ImageCapture related tests still passing.
==========

to

==========
ImageCapture: Implement takePhoto() for Windows

This CL implements VideoCaptureDevice::TakePhoto() for
Windows DirectShow (which is essentially used for all 
Windows re. video capture). For that, the V4L2CaptureDelegate::GetPhotoBlob() is
factored out
into media/capture/video/blob_utils.{cc,h}, and renamed
to Blobify().

BUG=648490
TEST=Enabled VideoCaptureDeviceTest::TakePhoto in Win.
All other ImageCapture related tests still passing.
==========

[commit-bot: I haz the power, 2016-09-21 00:17:25]

Committed patchset #2 (id:30001)

[commit-bot: I haz the power, 2016-09-21 00:20:31]

Description was changed from

==========
ImageCapture: Implement takePhoto() for Windows

This CL implements VideoCaptureDevice::TakePhoto() for
Windows DirectShow (which is essentially used for all 
Windows re. video capture). For that, the V4L2CaptureDelegate::GetPhotoBlob() is
factored out
into media/capture/video/blob_utils.{cc,h}, and renamed
to Blobify().

BUG=648490
TEST=Enabled VideoCaptureDeviceTest::TakePhoto in Win.
All other ImageCapture related tests still passing.
==========

to

==========
ImageCapture: Implement takePhoto() for Windows

This CL implements VideoCaptureDevice::TakePhoto() for
Windows DirectShow (which is essentially used for all
Windows re. video capture). For that, the V4L2CaptureDelegate::GetPhotoBlob() is
factored out
into media/capture/video/blob_utils.{cc,h}, and renamed
to Blobify().

BUG=648490
TEST=Enabled VideoCaptureDeviceTest::TakePhoto in Win.
All other ImageCapture related tests still passing.

Committed: https://crrev.com/3ff7e8f966c0c56019cb51cdbb93316a772191b6
Cr-Commit-Position: refs/heads/master@{#419915}
==========

[commit-bot: I haz the power, 2016-09-21 00:20:32]

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