Handle Alpha channel in Canvas capture

media/base/video_frame.cc

Index: media/base/video_frame.cc
diff --git a/media/base/video_frame.cc b/media/base/video_frame.cc
index 4dedd82d10b20633d091bd24aebd31b203c32719..138e61d92c9fb054d2306fbd2ed756d516e59d35 100644
--- a/media/base/video_frame.cc
+++ b/media/base/video_frame.cc
@@ -200,6 +200,25 @@ static int BytesPerElement(VideoPixelFormat format, size_t plane) {
   return 0;
 }
 
+// Checks if |source_format| can be wrapped into a |target_format| frame.
+static bool AreValidPixelFormatsForWrap(VideoPixelFormat source_format,
+                                        VideoPixelFormat target_format) {
+  if (source_format == target_format)
+    return true;
+
+  switch (source_format) {
+    case PIXEL_FORMAT_YV12A:
+      switch (target_format) {
+        case PIXEL_FORMAT_I420:
+          return true;
+        default:
+          return false;
+      }
+    default:
+      return false;
+  }

[mcasas, 2016/02/29 23:48:01]

Hmm actually I'd say either they are the same
format, or are immediately compatible, so the
comparison should be sth reflecting that
either |source_format| or |target_format| is
PIXEL_FORMAT_{YV12,I420,YV12A}, --> OK 

otherwise we might skip previously supported 
cases?

[emircan, 2016/03/01 20:07:55]

l.206 covers the previously supported cases. 

For the new cases, I wanted to only expose PIXEL_FORMAT_YV12A->PIXEL_FORMAT_I420
because only that would be used for now. Additional cases may be added and
tested as use cases come. WDYT?

[mcasas, 2016/03/02 22:01:10]

SG。I'd say just add a comment saying that
nothing prevents trivially adding other 
triplanar-to-triplanar conversions, and change 
this code to

  return source_format == PIXEL_FORMAT_YV12A &&
         target_format == PIXEL_FORMAT_I420;

[emircan, 2016/03/03 22:05:58]

Done.

+}
+
 // static
 bool VideoFrame::IsValidConfig(VideoPixelFormat format,
                                StorageType storage_type,
@@ -495,33 +514,40 @@ scoped_refptr<VideoFrame> VideoFrame::WrapCVPixelBuffer(
 
 // static
 scoped_refptr<VideoFrame> VideoFrame::WrapVideoFrame(
-      const scoped_refptr<VideoFrame>& frame,
-      const gfx::Rect& visible_rect,
-      const gfx::Size& natural_size) {
+    const scoped_refptr<VideoFrame>& frame,
+    VideoPixelFormat format,
+    const gfx::Rect& visible_rect,
+    const gfx::Size& natural_size) {
   // Frames with textures need mailbox info propagated, and there's no support
   // for that here yet, see http://crbug/362521.
   CHECK(!frame->HasTextures());
-
   DCHECK(frame->visible_rect().Contains(visible_rect));
 
-  if (!IsValidConfig(frame->format(), frame->storage_type(),
-                     frame->coded_size(), visible_rect, natural_size)) {
+  if (!AreValidPixelFormatsForWrap(frame->format(), format)) {
+    LOG(DFATAL) << __FUNCTION__ << " Invalid formats."

[mcasas, 2016/02/29 23:48:01]

s/formats./format conversion: /

[emircan, 2016/03/01 20:07:55]

Done.

+                << VideoPixelFormatToString(frame->format()) << " to "
+                << VideoPixelFormatToString(format);
+    return nullptr;
+  }
+
+  if (!IsValidConfig(format, frame->storage_type(), frame->coded_size(),
+                     visible_rect, natural_size)) {
     LOG(DFATAL) << __FUNCTION__ << " Invalid config."
-                << ConfigToString(frame->format(), frame->storage_type(),
+                << ConfigToString(format, frame->storage_type(),
                                   frame->coded_size(), visible_rect,
                                   natural_size);
     return nullptr;
   }
 
-  scoped_refptr<VideoFrame> wrapping_frame(new VideoFrame(
-      frame->format(), frame->storage_type(), frame->coded_size(), visible_rect,
-      natural_size, frame->timestamp()));
+  scoped_refptr<VideoFrame> wrapping_frame(
+      new VideoFrame(format, frame->storage_type(), frame->coded_size(),
+                     visible_rect, natural_size, frame->timestamp()));
   if (frame->metadata()->IsTrue(VideoFrameMetadata::END_OF_STREAM)) {
     wrapping_frame->metadata()->SetBoolean(VideoFrameMetadata::END_OF_STREAM,
-                                          true);
+                                           true);
   }

[mcasas, 2016/02/29 23:48:01]

nit: Maybe we should copy all VideoFrameMetadata::Key ?

I'm ok with crbug.com too.

[emircan, 2016/03/01 20:07:55]

Done.

 
-  for (size_t i = 0; i < NumPlanes(frame->format()); ++i) {
+  for (size_t i = 0; i < NumPlanes(format); ++i) {
     wrapping_frame->strides_[i] = frame->stride(i);
     wrapping_frame->data_[i] = frame->data(i);
   }