ImageCapture: Implement TakePhoto() for Linux/CrOs

ImageCapture: Implement TakePhoto() for Linux/CrOs

This CL implements ImageCapture's takePhoto() method by enqueueing a
callback every time the method is called. Check the queue when a new
frame is captured in DoCapture(), and execute callbacks with a blob
argument.

Add a new method GetPhotoBlob() to construct blob. If the source image
is originally in JPEG format, it's copied to blob directly. Otherwise
convert the source image to RGB format and encode it to PNG.

R=mcasas@chromium.org
BUG=646430
TEST=All tests in VideoCaptureDeviceTests passed.

Committed: https://crrev.com/5a18d02471b9d6bdfcc3f38ac449f084699cbe26
Cr-Commit-Position: refs/heads/master@{#419484}

[mcasas, 2016-09-16 03:50:37]

Some comments. 

Also, on the redaction of the CL:
- give better coordinates in the Title, e.g.:
 "ImageCapture: implement TakePhoto() in Linux/CrOs"
- give first an overview of what you are doing,
e.g. "This CL implements ImageCapture's takePhoto()
method for Linux/CrOs", and then continue with your
more detailed description, if any.
- CLs are usually redacted in present tense, IIRC.

https://codereview.chromium.org/2344123002/diff/1/media/capture/BUILD.gn
File media/capture/BUILD.gn (right):

https://codereview.chromium.org/2344123002/diff/1/media/capture/BUILD.gn#newc...
media/capture/BUILD.gn:82: "//third_party/libyuv",
Since this dependency is only necessary for
Linux/Cros, move it to somewhere between
l.95 and 136 and add an OS guard:

if (is_linux || is_chromeos) {
  deps += ["//third_party/libyuv"]
}

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/linux/v...
File media/capture/video/linux/v4l2_capture_delegate.cc (right):

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/linux/v...
media/capture/video/linux/v4l2_capture_delegate.cc:326:
take_photo_callbacks_.push(std::move(callback));
nit: add
DCHECK(v4l2_task_runner_->BelongsToCurrentThread());

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/linux/v...
media/capture/video/linux/v4l2_capture_delegate.cc:327: DVLOG(1) << "in
V4L2CaptureDelegate: callback moved";
nit: remove DVLOG()s when you submit for review,

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/linux/v...
media/capture/video/linux/v4l2_capture_delegate.cc:410: // Process all the
callbacks from TakePhoto()
nit: No need for this comment.

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/linux/v...
media/capture/video/linux/v4l2_capture_delegate.cc:416: if (mojom::BlobPtr blob
= GetPhotoBlob(buffer_tracker, buffer))
nit: Don't mix assignments and conditionals.

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/linux/v...
media/capture/video/linux/v4l2_capture_delegate.cc:445:
blob->data.resize(buffer.bytesused);
If we take this path, |dst_argb| would be unused and
we'll be bashing memory. Instead, try to allocate as
close as possible to the use, and not allocate what
you might not need (lazy developer). E.g.:

mojom::BlobPtr V4L2CaptureDelegate::GetPhotoBlob(
    const scoped_refptr<BufferTracker>& buffer_tracker,
    const v4l2_buffer& buffer) {

  uint32 src_format;
  switch (capture_format_.pixel_format) {
    case VideoPixelFormat::PIXEL_FORMAT_MJPEG:
      mojom::BlobPtr blob = mojom::Blob::New();
      blob->data.resize(buffer.bytesused);
      memcpy(blob->data.data(), buffer_tracker->start(), buffer.bytesused);
      blob->mime_type = "image/jpeg";
      return blob;
    case VideoPixelFormat::PIXEL_FORMAT_UYVY:
      src_format = libyuv::FOURCC_UYVY;
      break;
    case VideoPixelFormat::PIXEL_FORMAT_YUY2:
      src_format = libyuv::FOURCC_YUY2;
      break;
    default:
      DVLOG(3) << "FORMAT not supported.";
      return nullptr;
  }

  std::unique_ptr<uint8_t[]> dst_argb(new uint8_t[VideoFrame::AllocationSize(
      PIXEL_FORMAT_ARGB, capture_format_.frame_size)]);
  if (ConvertToARGB(...) {

  }

  mojom::BlobPtr blob = mojom::Blob::New();
  const bool result = gfx::PNGCodec::Encode(...)

}

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/linux/v...
media/capture/video/linux/v4l2_capture_delegate.cc:456: DVLOG(3) << "FORMAT not
supported.";
Instead of DVLOG(3), consider using

DLOG(ERROR) << VideoPixelFormatToString(capture_format_.pixel_format);

Which better conveys meaning. This also applies
to l.467, although I would remove that log.

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/linux/v...
File media/capture/video/linux/v4l2_capture_delegate.h (right):

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/linux/v...
media/capture/video/linux/v4l2_capture_delegate.h:19: #include
"ui/gfx/codec/png_codec.h"
The skia includes are unnecessary since
we don't use skia functionality.

Also, move the libyuv and png_coded.h
to the implementation file, since they are
not used in this header.

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/linux/v...
media/capture/video/linux/v4l2_capture_delegate.h:92:
std::queue<VideoCaptureDevice::TakePhotoCallback> take_photo_callbacks_;
The comment in l.88 would affect this member as
well, but |take_photo_callbacks_|, but it doesn't 
apply to it: move this variable to a group on its
own after, e.g. l.94,

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/linux/v...
File media/capture/video/linux/video_capture_device_linux.cc (right):

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/linux/v...
media/capture/video/linux/video_capture_device_linux.cc:82: capture_impl_ =
NULL;
nit: not your code, but can you s/NULL/nullptr/ plz?

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/linux/v...
media/capture/video/linux/video_capture_device_linux.cc:87:
DCHECK(v4l2_thread_.IsRunning());
This should be a 
if (!v4l2_thread_.IsRunning())
  return;

otherwise when DCHECK()s are disconnected
(i.e. in a Release build), we'd crash in l.90.

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/linux/v...
media/capture/video/linux/video_capture_device_linux.cc:88: DVLOG(1) << "in
VCDLinux: TakePhoto() is called";
nit: Remove this. In any case, it should be
before the DCHECK()s (so if they are hit, you'd
still see something), and using __func__:

DVLOG(1) << __func__;

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/video_c...
File media/capture/video/video_capture_device_unittest.cc (right):

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/video_c...
media/capture/video/video_capture_device_unittest.cc:74: using ::testing::AnyOf;
Not used, remove.

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/video_c...
media/capture/video/video_capture_device_unittest.cc:161: NOTREACHED() << "Photo
format should be jpeg or png";
NOTREACHED() would crash the test executable and
prevent other tests from running. Instead, use
FAIL() or ADD_FAILURE():
https://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuid...

[xianglu, 2016-09-16 16:48:44]

Description was changed from

==========
TakePhoto() Implemented for Linux

Enqueue a callback everytime the method is called. Check the queue when a new
frame is captured in DoCapture(), and execute callbacks with a blob argument.

The blob is constructed in GetPhotoBlob(). If the source image is originally in
JPEG format, it's copied to blob directly. Otherwise convert the source image to
RGB format and encode it to PNG.

New method added: V4L2CaptureDelegate::GetPhotoBlob


R=mcasas@chromium.org
BUG=646430
TEST=All tests in VideoCaptureDeviceTests passed.
==========

to

==========
This CL implements ImageCapture's takePhoto() method for Linux/CrOs

Enqueue a callback every time the method is called. Check the queue when a new
frame is captured in DoCapture(), and execute callbacks with a blob argument.

Add a new method GetPhotoBlob() to construct blob. If the source image is
originally in JPEG format, it's copied to blob directly. Otherwise convert the
source image to RGB format and encode it to PNG.


R=mcasas@chromium.org
BUG=646430
TEST=All tests in VideoCaptureDeviceTests passed.
==========

[xianglu, 2016-09-16 18:22:34]

ptal.

https://codereview.chromium.org/2344123002/diff/1/media/capture/BUILD.gn
File media/capture/BUILD.gn (right):

https://codereview.chromium.org/2344123002/diff/1/media/capture/BUILD.gn#newc...
media/capture/BUILD.gn:82: "//third_party/libyuv",
On 2016/09/16 03:50:36, mcasas wrote:
> Since this dependency is only necessary for
> Linux/Cros, move it to somewhere between
> l.95 and 136 and add an OS guard:
> 
> if (is_linux || is_chromeos) {
>   deps += ["//third_party/libyuv"]
> } 

Done.

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/linux/v...
File media/capture/video/linux/v4l2_capture_delegate.cc (right):

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/linux/v...
media/capture/video/linux/v4l2_capture_delegate.cc:326:
take_photo_callbacks_.push(std::move(callback));
On 2016/09/16 03:50:36, mcasas wrote:
> nit: add
> DCHECK(v4l2_task_runner_->BelongsToCurrentThread());

Done.

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/linux/v...
media/capture/video/linux/v4l2_capture_delegate.cc:327: DVLOG(1) << "in
V4L2CaptureDelegate: callback moved";
On 2016/09/16 03:50:36, mcasas wrote:
> nit: remove DVLOG()s when you submit for review,

Done.

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/linux/v...
media/capture/video/linux/v4l2_capture_delegate.cc:410: // Process all the
callbacks from TakePhoto()
On 2016/09/16 03:50:36, mcasas wrote:
> nit: No need for this comment.

Done.

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/linux/v...
media/capture/video/linux/v4l2_capture_delegate.cc:416: if (mojom::BlobPtr blob
= GetPhotoBlob(buffer_tracker, buffer))
On 2016/09/16 03:50:36, mcasas wrote:
> nit: Don't mix assignments and conditionals.

Done.

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/linux/v...
media/capture/video/linux/v4l2_capture_delegate.cc:445:
blob->data.resize(buffer.bytesused);
On 2016/09/16 03:50:36, mcasas wrote:
> If we take this path, |dst_argb| would be unused and
> we'll be bashing memory. Instead, try to allocate as
> close as possible to the use, and not allocate what
> you might not need (lazy developer). E.g.:
> 
> mojom::BlobPtr V4L2CaptureDelegate::GetPhotoBlob(
>     const scoped_refptr<BufferTracker>& buffer_tracker,
>     const v4l2_buffer& buffer) {
> 
>   uint32 src_format;
>   switch (capture_format_.pixel_format) {
>     case VideoPixelFormat::PIXEL_FORMAT_MJPEG:
>       mojom::BlobPtr blob = mojom::Blob::New();
>       blob->data.resize(buffer.bytesused);
>       memcpy(blob->data.data(), buffer_tracker->start(), buffer.bytesused);
>       blob->mime_type = "image/jpeg";
>       return blob;
>     case VideoPixelFormat::PIXEL_FORMAT_UYVY:
>       src_format = libyuv::FOURCC_UYVY;
>       break;
>     case VideoPixelFormat::PIXEL_FORMAT_YUY2:
>       src_format = libyuv::FOURCC_YUY2;
>       break;
>     default:
>       DVLOG(3) << "FORMAT not supported.";
>       return nullptr;
>   }
> 
>   std::unique_ptr<uint8_t[]> dst_argb(new uint8_t[VideoFrame::AllocationSize(
>       PIXEL_FORMAT_ARGB, capture_format_.frame_size)]);
>   if (ConvertToARGB(...) {
> 
>   }
> 
>   mojom::BlobPtr blob = mojom::Blob::New();
>   const bool result = gfx::PNGCodec::Encode(...)
> 
> }

Indeed. Should work this way.

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/linux/v...
media/capture/video/linux/v4l2_capture_delegate.cc:456: DVLOG(3) << "FORMAT not
supported.";
On 2016/09/16 03:50:36, mcasas wrote:
> Instead of DVLOG(3), consider using
> 
> DLOG(ERROR) << VideoPixelFormatToString(capture_format_.pixel_format);
> 
> Which better conveys meaning. This also applies
> to l.467, although I would remove that log.

Why would you remove ConvertFailure log?

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/linux/v...
File media/capture/video/linux/v4l2_capture_delegate.h (right):

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/linux/v...
media/capture/video/linux/v4l2_capture_delegate.h:19: #include
"ui/gfx/codec/png_codec.h"
On 2016/09/16 03:50:37, mcasas wrote:
> The skia includes are unnecessary since
> we don't use skia functionality.
> 
> Also, move the libyuv and png_coded.h
> to the implementation file, since they are
> not used in this header.

kN32_SkColorType is from SkImage.

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/linux/v...
media/capture/video/linux/v4l2_capture_delegate.h:92:
std::queue<VideoCaptureDevice::TakePhotoCallback> take_photo_callbacks_;
On 2016/09/16 03:50:36, mcasas wrote:
> The comment in l.88 would affect this member as
> well, but |take_photo_callbacks_|, but it doesn't 
> apply to it: move this variable to a group on its
> own after, e.g. l.94,

Done.

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/linux/v...
File media/capture/video/linux/video_capture_device_linux.cc (right):

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/linux/v...
media/capture/video/linux/video_capture_device_linux.cc:82: capture_impl_ =
NULL;
On 2016/09/16 03:50:37, mcasas wrote:
> nit: not your code, but can you s/NULL/nullptr/ plz?

Done.

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/linux/v...
media/capture/video/linux/video_capture_device_linux.cc:87:
DCHECK(v4l2_thread_.IsRunning());
On 2016/09/16 03:50:37, mcasas wrote:
> This should be a 
> if (!v4l2_thread_.IsRunning())
>   return;
> 
> otherwise when DCHECK()s are disconnected
> (i.e. in a Release build), we'd crash in l.90.

Done.

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/linux/v...
media/capture/video/linux/video_capture_device_linux.cc:88: DVLOG(1) << "in
VCDLinux: TakePhoto() is called";
On 2016/09/16 03:50:37, mcasas wrote:
> nit: Remove this. In any case, it should be
> before the DCHECK()s (so if they are hit, you'd
> still see something), and using __func__:
> 
> DVLOG(1) << __func__;

Done.

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/video_c...
File media/capture/video/video_capture_device_unittest.cc (right):

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/video_c...
media/capture/video/video_capture_device_unittest.cc:74: using ::testing::AnyOf;
On 2016/09/16 03:50:37, mcasas wrote:
> Not used, remove.

Done.

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/video_c...
media/capture/video/video_capture_device_unittest.cc:161: NOTREACHED() << "Photo
format should be jpeg or png";
On 2016/09/16 03:50:37, mcasas wrote:
> NOTREACHED() would crash the test executable and
> prevent other tests from running. Instead, use
> FAIL() or ADD_FAILURE():
>
https://github.com/google/googletest/blob/master/googletest/docs/AdvancedGuid...

Done.

[mcasas, 2016-09-16 18:59:15]

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/linux/v...
File media/capture/video/linux/v4l2_capture_delegate.cc (right):

https://codereview.chromium.org/2344123002/diff/1/media/capture/video/linux/v...
media/capture/video/linux/v4l2_capture_delegate.cc:456: DVLOG(3) << "FORMAT not
supported.";
On 2016/09/16 18:22:33, xianglu wrote:
> On 2016/09/16 03:50:36, mcasas wrote:
> > Instead of DVLOG(3), consider using
> > 
> > DLOG(ERROR) << VideoPixelFormatToString(capture_format_.pixel_format);
> > 
> > Which better conveys meaning. This also applies
> > to l.467, although I would remove that log.
> 
> Why would you remove ConvertFailure log?

All those error logs are only really useful for 
developers/development; IOW there's a 
realistic possibility that the other formats
would be hit, but what's this error going to 
tell you that you don't know already? Returning 
nullptr will make takePhoto() fail ultimately.

https://codereview.chromium.org/2344123002/diff/20001/media/capture/video/lin...
File media/capture/video/linux/v4l2_capture_delegate.cc (right):

https://codereview.chromium.org/2344123002/diff/20001/media/capture/video/lin...
media/capture/video/linux/v4l2_capture_delegate.cc:453: default:
I missed this one. It's a good practice to
not use default: but enumerate all cases
individually, so that, if someone adds a 
new VideoPixelFormat in the future, the 
compiler will fail (unhandled enum entry), 
forcing that developer to think about what 
to do in this routine. (I thought this was 
enforced via a presubmit script, but could 
be in another folder).

Since, in this case you only contemplate
three cases, namely YUY2, UYVY and MJPEG
(although you should also support the other
two in l.50 and l.53), and there's a large
number of other formats you really don't 
expect here, switch-case might be too 
verbose, so I'd say consider if/else instead.

[xianglu, 2016-09-16 20:27:58]

ptal.

[mcasas, 2016-09-16 21:17:07]

lgtm % comments below.

https://codereview.chromium.org/2344123002/diff/40001/media/capture/video/lin...
File media/capture/video/linux/v4l2_capture_delegate.cc (right):

https://codereview.chromium.org/2344123002/diff/40001/media/capture/video/lin...
media/capture/video/linux/v4l2_capture_delegate.cc:422: DLOG(ERROR) << "blob
pointer is null.";
Same reasoning as before, remove these two lines.
Note that in this case is worse than others, since a
Release build would remove DLOG(ERROR), but will
have to deal with an empty else branch (which might
be optimized off, but that's something else).

Or, if you really really want to see an error in this case,
consider instead:
DLOG_IF(ERROR, !blob) << "converting captured frame to Blob";
if (blob)
  cb.Run(std::move(blob));

Note the message has change to give information as to
the process that has failed, as opposed to now, where 
the message is essentially stating a fact that could be
deduced by the surrounding code; again, imagine a 
developer of the future trying to decypher some code
that they have never seen.

https://codereview.chromium.org/2344123002/diff/40001/media/capture/video/lin...
media/capture/video/linux/v4l2_capture_delegate.cc:437: const v4l2_buffer&
buffer) {
We're only using buffer.bytesused, so pass that uint32_t
instead (try to send as parameters the barest minimum).

[xianglu, 2016-09-16 21:26:58]

Description was changed from

==========
This CL implements ImageCapture's takePhoto() method for Linux/CrOs

Enqueue a callback every time the method is called. Check the queue when a new
frame is captured in DoCapture(), and execute callbacks with a blob argument.

Add a new method GetPhotoBlob() to construct blob. If the source image is
originally in JPEG format, it's copied to blob directly. Otherwise convert the
source image to RGB format and encode it to PNG.


R=mcasas@chromium.org
BUG=646430
TEST=All tests in VideoCaptureDeviceTests passed.
==========

to

==========
ImageCapture: Implement TakePhoto() for Linux/CrOs

This CL implements ImageCapture's takePhoto() method by enqueueing a 
callback every time the method is called. Then we check the queue when
a new frame is captured in DoCapture(), and execute callbacks with a blob 
argument.

Add a new method GetPhotoBlob() to construct blob. If the source image
is originally in JPEG format, it's copied to blob directly. Otherwise 
convert the source image to RGB format and encode it to PNG.


R=mcasas@chromium.org
BUG=646430
TEST=All tests in VideoCaptureDeviceTests passed.
==========

[xianglu, 2016-09-16 21:27:44]

Description was changed from

==========
ImageCapture: Implement TakePhoto() for Linux/CrOs

This CL implements ImageCapture's takePhoto() method by enqueueing a 
callback every time the method is called. Then we check the queue when
a new frame is captured in DoCapture(), and execute callbacks with a blob 
argument.

Add a new method GetPhotoBlob() to construct blob. If the source image
is originally in JPEG format, it's copied to blob directly. Otherwise 
convert the source image to RGB format and encode it to PNG.


R=mcasas@chromium.org
BUG=646430
TEST=All tests in VideoCaptureDeviceTests passed.
==========

to

==========
ImageCapture: Implement TakePhoto() for Linux/CrOs

This CL implements ImageCapture's takePhoto() method by enqueueing a 
callback every time the method is called. Check the queue when
a new frame is captured in DoCapture(), and execute callbacks with a blob 
argument.

Add a new method GetPhotoBlob() to construct blob. If the source image
is originally in JPEG format, it's copied to blob directly. Otherwise 
convert the source image to RGB format and encode it to PNG.


R=mcasas@chromium.org
BUG=646430
TEST=All tests in VideoCaptureDeviceTests passed.
==========

[xianglu, 2016-09-16 21:28:05]

Description was changed from

==========
ImageCapture: Implement TakePhoto() for Linux/CrOs

This CL implements ImageCapture's takePhoto() method by enqueueing a 
callback every time the method is called. Check the queue when
a new frame is captured in DoCapture(), and execute callbacks with a blob 
argument.

Add a new method GetPhotoBlob() to construct blob. If the source image
is originally in JPEG format, it's copied to blob directly. Otherwise 
convert the source image to RGB format and encode it to PNG.


R=mcasas@chromium.org
BUG=646430
TEST=All tests in VideoCaptureDeviceTests passed.
==========

to

==========
ImageCapture: Implement TakePhoto() for Linux/CrOs

This CL implements ImageCapture's takePhoto() method by enqueueing a 
callback every time the method is called. Check the queue when a new
frame is captured in DoCapture(), and execute callbacks with a blob 
argument.

Add a new method GetPhotoBlob() to construct blob. If the source image
is originally in JPEG format, it's copied to blob directly. Otherwise 
convert the source image to RGB format and encode it to PNG.


R=mcasas@chromium.org
BUG=646430
TEST=All tests in VideoCaptureDeviceTests passed.
==========

[xianglu, 2016-09-16 21:51:19]

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

[commit-bot: I haz the power, 2016-09-16 21:52:18]

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-16 21:56:03]

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

[commit-bot: I haz the power, 2016-09-16 21:56:04]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[xianglu, 2016-09-16 22:37:59]

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

[commit-bot: I haz the power, 2016-09-16 22:38:35]

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-16 23:10:30]

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

[commit-bot: I haz the power, 2016-09-16 23:10:31]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[xianglu, 2016-09-17 01:17:54]

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

[commit-bot: I haz the power, 2016-09-17 01:18:13]

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-17 03:23:51]

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

[commit-bot: I haz the power, 2016-09-17 03:23:52]

Dry run: This issue passed the CQ dry run.

[xianglu, 2016-09-19 06:10:56]

The CQ bit was checked by xianglu@chromium.org

[xianglu, 2016-09-19 06:10:57]

The patchset sent to the CQ was uploaded after l-g-t-m from mcasas@chromium.org
Link to the patchset: https://codereview.chromium.org/2344123002/#ps100001
(title: "v4l2_capture_delegate.cc l.152: Added back '&0xf0'")

[commit-bot: I haz the power, 2016-09-19 06:11:06]

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-19 07:16:29]

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

[commit-bot: I haz the power, 2016-09-19 07:16:30]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[xianglu, 2016-09-19 16:25:26]

The CQ bit was checked by xianglu@chromium.org

[commit-bot: I haz the power, 2016-09-19 16:25:39]

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-19 16:58:28]

Description was changed from

==========
ImageCapture: Implement TakePhoto() for Linux/CrOs

This CL implements ImageCapture's takePhoto() method by enqueueing a 
callback every time the method is called. Check the queue when a new
frame is captured in DoCapture(), and execute callbacks with a blob 
argument.

Add a new method GetPhotoBlob() to construct blob. If the source image
is originally in JPEG format, it's copied to blob directly. Otherwise 
convert the source image to RGB format and encode it to PNG.


R=mcasas@chromium.org
BUG=646430
TEST=All tests in VideoCaptureDeviceTests passed.
==========

to

==========
ImageCapture: Implement TakePhoto() for Linux/CrOs

This CL implements ImageCapture's takePhoto() method by enqueueing a 
callback every time the method is called. Check the queue when a new
frame is captured in DoCapture(), and execute callbacks with a blob 
argument.

Add a new method GetPhotoBlob() to construct blob. If the source image
is originally in JPEG format, it's copied to blob directly. Otherwise 
convert the source image to RGB format and encode it to PNG.


R=mcasas@chromium.org
BUG=646430
TEST=All tests in VideoCaptureDeviceTests passed.
==========

[commit-bot: I haz the power, 2016-09-19 16:58:29]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-09-19 17:00:01]

Description was changed from

==========
ImageCapture: Implement TakePhoto() for Linux/CrOs

This CL implements ImageCapture's takePhoto() method by enqueueing a 
callback every time the method is called. Check the queue when a new
frame is captured in DoCapture(), and execute callbacks with a blob 
argument.

Add a new method GetPhotoBlob() to construct blob. If the source image
is originally in JPEG format, it's copied to blob directly. Otherwise 
convert the source image to RGB format and encode it to PNG.


R=mcasas@chromium.org
BUG=646430
TEST=All tests in VideoCaptureDeviceTests passed.
==========

to

==========
ImageCapture: Implement TakePhoto() for Linux/CrOs

This CL implements ImageCapture's takePhoto() method by enqueueing a
callback every time the method is called. Check the queue when a new
frame is captured in DoCapture(), and execute callbacks with a blob
argument.

Add a new method GetPhotoBlob() to construct blob. If the source image
is originally in JPEG format, it's copied to blob directly. Otherwise
convert the source image to RGB format and encode it to PNG.

R=mcasas@chromium.org
BUG=646430
TEST=All tests in VideoCaptureDeviceTests passed.

Committed: https://crrev.com/5a18d02471b9d6bdfcc3f38ac449f084699cbe26
Cr-Commit-Position: refs/heads/master@{#419484}
==========

[commit-bot: I haz the power, 2016-09-19 17:00:03]

Patchset 6 (id:??) landed as
https://crrev.com/5a18d02471b9d6bdfcc3f38ac449f084699cbe26
Cr-Commit-Position: refs/heads/master@{#419484}