16 bpp video stream capture, render and createImageBitmap(video) using (CPU) shared memory buffers

media/capture/video/video_capture_device_client.cc

Index: media/capture/video/video_capture_device_client.cc
diff --git a/media/capture/video/video_capture_device_client.cc b/media/capture/video/video_capture_device_client.cc
index d9874a78814cbb6d8c36d5f2a82822436908e9f4..4f7ab430f9ab1dab588ba3927ed8fcd246710fca 100644
--- a/media/capture/video/video_capture_device_client.cc
+++ b/media/capture/video/video_capture_device_client.cc
@@ -27,6 +27,15 @@ using media::VideoCaptureFormat;
 using media::VideoFrame;
 using media::VideoFrameMetadata;
 
+namespace {
+
+bool isFormatSupported(media::VideoPixelFormat pixel_format) {

[mcasas, 2016/10/21 00:10:51]

Capitalize this: IsFormatSupported().

[aleksandar.stojiljkovic, 2016/10/21 22:11:11]

Done.

+  // Currently, only I420 and Y16 pixel formats are supported.

[mcasas, 2016/10/21 00:10:50]

Remove superfluous comment (the return says the same),
unless there are plans for more, case in which a TODO 
should be added.

[aleksandar.stojiljkovic, 2016/10/21 22:11:11]

Done.

+  return (pixel_format == media::PIXEL_FORMAT_I420 ||
+          pixel_format == media::PIXEL_FORMAT_Y16);
+}
+}

[mcasas, 2016/10/21 00:10:51]

Add empty line.

[aleksandar.stojiljkovic, 2016/10/21 22:11:11]

Done.

+
 namespace media {
 
 // Class combining a Client::Buffer interface implementation and a pool buffer
@@ -106,12 +115,22 @@ void VideoCaptureDeviceClient::OnIncomingCapturedData(
   if (!frame_format.IsValid())
     return;
 
+  // The input |length| can be greater than the required buffer size because of
+  // paddings and/or alignments, but it cannot be smaller.
+  DCHECK_GE(static_cast<size_t>(length), frame_format.ImageAllocationSize());
+
+  const bool useFullSize = frame_format.pixel_format == media::PIXEL_FORMAT_Y16;

[mcasas, 2016/10/21 00:10:50]

Don't use camel case.

[aleksandar.stojiljkovic, 2016/10/21 22:11:10]

Done.

+
   // |chopped_{width,height} and |new_unrotated_{width,height}| are the lowest
   // bit decomposition of {width, height}, grabbing the odd and even parts.
   const int chopped_width = frame_format.frame_size.width() & 1;
   const int chopped_height = frame_format.frame_size.height() & 1;
-  const int new_unrotated_width = frame_format.frame_size.width() & ~1;
-  const int new_unrotated_height = frame_format.frame_size.height() & ~1;
+  const int new_unrotated_width = (useFullSize)
+                                      ? frame_format.frame_size.width()
+                                      : (frame_format.frame_size.width() & ~1);
+  const int new_unrotated_height =
+      (useFullSize) ? frame_format.frame_size.height()
+                    : (frame_format.frame_size.height() & ~1);

[mcasas, 2016/10/21 00:10:50]

Parentheses are not needed around |useFullSize| (the name
of which should change anyway), here and in l.128.

[aleksandar.stojiljkovic, 2016/10/21 22:11:11]

Done.

 
   int destination_width = new_unrotated_width;
   int destination_height = new_unrotated_height;
@@ -129,19 +148,36 @@ void VideoCaptureDeviceClient::OnIncomingCapturedData(
     rotation_mode = libyuv::kRotate270;
 
   const gfx::Size dimensions(destination_width, destination_height);
-  uint8_t *y_plane_data, *u_plane_data, *v_plane_data;
+  uint8_t* y_plane_data = 0;
+  uint8_t* u_plane_data = 0;
+  uint8_t* v_plane_data = 0;

[mcasas, 2016/10/21 00:10:50]

To be precise, these three should be = nullptr;

[aleksandar.stojiljkovic, 2016/10/21 22:11:11]

Done.

+
   std::unique_ptr<Buffer> buffer(
-      ReserveI420OutputBuffer(dimensions, media::PIXEL_STORAGE_CPU,
-                              &y_plane_data, &u_plane_data, &v_plane_data));
+      (frame_format.pixel_format == media::PIXEL_FORMAT_Y16)
+          ? ReserveOutputBuffer(dimensions, frame_format.pixel_format,
+                                media::PIXEL_STORAGE_CPU)
+          : ReserveI420OutputBuffer(dimensions, media::PIXEL_STORAGE_CPU,
+                                    &y_plane_data, &u_plane_data,
+                                    &v_plane_data));
 #if DCHECK_IS_ON()
   dropped_frame_counter_ = buffer.get() ? 0 : dropped_frame_counter_ + 1;
   if (dropped_frame_counter_ >= kMaxDroppedFrames)
     OnError(FROM_HERE, "Too many frames dropped");
 #endif
-  // Failed to reserve I420 output buffer, so drop the frame.
+  // Failed to reserve output buffer, so drop the frame.
   if (!buffer.get())
     return;
 
+  if (frame_format.pixel_format == media::PIXEL_FORMAT_Y16) {
+    memcpy(buffer->data(), data, length);
+    const VideoCaptureFormat output_format =
+        VideoCaptureFormat(dimensions, frame_format.frame_rate,
+                           frame_format.pixel_format, media::PIXEL_STORAGE_CPU);
+    OnIncomingCapturedBuffer(std::move(buffer), output_format, reference_time,
+                             timestamp);
+    return;
+  }

[mcasas, 2016/10/21 00:10:50]

Hmm unlike in the content/ code, there's specific
treatment of the Y16 formats here in VCDClient.
Is there no way we can homogeneize them?  I would
rather have a specific VCDClient method 
OnIncomingSuperCaptureData() where you treat it
specifically, WDYT?

[aleksandar.stojiljkovic, 2016/10/21 22:11:10]

Done. Thanks, looks much better now.
Now, some formats are converted to I420 and we can skip this for Y16, and in
future for Y8 (uncompressed 8bpp grayscale format of some older depth and IR
cameras) and RGB based formats (don't know what is the priority on those
though).
Except this, no special handling for Y16 - moved the logic bellow to the same
path as other formats, under MPEG handling.

+
   const int yplane_stride = dimensions.width();
   const int uv_plane_stride = yplane_stride / 2;
   int crop_x = 0;
@@ -257,8 +293,7 @@ VideoCaptureDeviceClient::ReserveOutputBuffer(
     media::VideoPixelStorage pixel_storage) {
   DCHECK_GT(frame_size.width(), 0);
   DCHECK_GT(frame_size.height(), 0);
-  // Currently, only I420 pixel format is supported.
-  DCHECK_EQ(media::PIXEL_FORMAT_I420, pixel_format);
+  DCHECK(isFormatSupported(pixel_format));
 
   // TODO(mcasas): For PIXEL_STORAGE_GPUMEMORYBUFFER, find a way to indicate if
   // it's a ShMem GMB or a DmaBuf GMB.
@@ -278,8 +313,7 @@ void VideoCaptureDeviceClient::OnIncomingCapturedBuffer(
     const VideoCaptureFormat& frame_format,
     base::TimeTicks reference_time,
     base::TimeDelta timestamp) {
-  // Currently, only I420 pixel format is supported.
-  DCHECK_EQ(media::PIXEL_FORMAT_I420, frame_format.pixel_format);
+  DCHECK(isFormatSupported(frame_format.pixel_format));
   DCHECK_EQ(media::PIXEL_STORAGE_CPU, frame_format.pixel_storage);
 
   scoped_refptr<VideoFrame> frame;
@@ -288,10 +322,10 @@ void VideoCaptureDeviceClient::OnIncomingCapturedBuffer(
     frame->set_timestamp(timestamp);
   } else {
     frame = VideoFrame::WrapExternalSharedMemory(
-        media::PIXEL_FORMAT_I420, frame_format.frame_size,
+        frame_format.pixel_format, frame_format.frame_size,
         gfx::Rect(frame_format.frame_size), frame_format.frame_size,
         reinterpret_cast<uint8_t*>(buffer->data()),
-        VideoFrame::AllocationSize(media::PIXEL_FORMAT_I420,
+        VideoFrame::AllocationSize(frame_format.pixel_format,
                                    frame_format.frame_size),
         base::SharedMemory::NULLHandle(), 0u, timestamp);
   }