Don't assume correct image format in CopyVpxImageToVideoFrame()

media/filters/vpx_video_decoder.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/filters/vpx_video_decoder.h"
 
 #include <algorithm>
 #include <string>
 #include <vector>
 
@@ skipping 445 matching lines @@
   if (!vpx_image) {
     *video_frame = nullptr;
     return true;
   }
 
   if (vpx_image->user_priv != user_priv) {
     DLOG(ERROR) << "Invalid output timestamp.";
     return false;
   }
 
-  CopyVpxImageToVideoFrame(vpx_image, video_frame);
+  if (!CopyVpxImageToVideoFrame(vpx_image, video_frame))
+    return false;
+
   (*video_frame)->set_timestamp(base::TimeDelta::FromMicroseconds(timestamp));
 
   // Default to the color space from the config, but if the bistream specifies
   // one, prefer that instead.
   ColorSpace color_space = config_.color_space();
   if (vpx_image->cs == VPX_CS_BT_709)
     color_space = COLOR_SPACE_HD_REC709;
   else if (vpx_image->cs == VPX_CS_BT_601)
     color_space = COLOR_SPACE_SD_REC601;
   (*video_frame)
@@ skipping 59 matching lines @@
 
   libyuv::CopyPlane(vpx_image_alpha->planes[VPX_PLANE_Y],
                     vpx_image_alpha->stride[VPX_PLANE_Y],
                     (*video_frame)->visible_data(VideoFrame::kAPlane),
                     (*video_frame)->stride(VideoFrame::kAPlane),
                     (*video_frame)->visible_rect().width(),
                     (*video_frame)->visible_rect().height());
   return true;
 }
 
-void VpxVideoDecoder::CopyVpxImageToVideoFrame(
+bool VpxVideoDecoder::CopyVpxImageToVideoFrame(
     const struct vpx_image* vpx_image,
     scoped_refptr<VideoFrame>* video_frame) {
   DCHECK(vpx_image);
-  DCHECK(vpx_image->fmt == VPX_IMG_FMT_I420 ||
-         vpx_image->fmt == VPX_IMG_FMT_I444);
 
-  VideoPixelFormat codec_format = PIXEL_FORMAT_YV12;
-  if (vpx_image->fmt == VPX_IMG_FMT_I444)
-    codec_format = PIXEL_FORMAT_YV24;
-  else if (vpx_codec_alpha_)
-    codec_format = PIXEL_FORMAT_YV12A;
+  VideoPixelFormat codec_format;
+  switch (vpx_image->fmt) {
+    case VPX_IMG_FMT_I420:
+      codec_format = vpx_codec_alpha_ ? PIXEL_FORMAT_YV12A : PIXEL_FORMAT_YV12;

[mcasas, 2015/12/15 17:04:54]

VpxVideoDecoder should not be used for YV12 
if there is FFmpeg enabled  [1, 2], so I wonder 
if we should avoid this format or add here another
#if !defined(DISABLE_FFMPEG_VIDEO_DECODERS)
or DCHECK() it or sth on those lines?

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/media/filters/vpx_...
[2]
https://code.google.com/p/chromium/codesearch#chromium/src/media/filters/vpx_...

[vignesh, 2015/12/15 17:10:23]

The comment on [1] may be perceived as ambiguous. What it means is that
VpxDecoder accepts YV12A VP8 and *all* VP9 content (including YV12). The YV12
format here is intended for VP9.

So nothing needs to be done here as far as i understand.

+      break;
+
+    case VPX_IMG_FMT_I444:
+      codec_format = PIXEL_FORMAT_YV24;
+      break;
+
+    default:
+      DLOG(ERROR) << "Unsupported pixel format: " << vpx_image->fmt;
+      return false;
+  }
 
   // The mixed |w|/|d_h| in |coded_size| is intentional. Setting the correct
   // coded width is necessary to allow coalesced memory access, which may avoid
   // frame copies. Setting the correct coded height however does not have any
   // benefit, and only risk copying too much data.
   const gfx::Size coded_size(vpx_image->w, vpx_image->d_h);
   const gfx::Size visible_size(vpx_image->d_w, vpx_image->d_h);
 
   if (memory_pool_.get()) {
     DCHECK_EQ(kCodecVP9, config_.codec());
@@ skipping 10 matching lines @@
         kNoTimestamp());
     video_frame->get()->AddDestructionObserver(
         memory_pool_->CreateFrameCallback(vpx_image->fb_priv));
 
     UMA_HISTOGRAM_COUNTS("Media.Vpx.VideoDecoderBuffersInUseByDecoder",
                          memory_pool_->NumberOfFrameBuffersInUseByDecoder());
     UMA_HISTOGRAM_COUNTS(
         "Media.Vpx.VideoDecoderBuffersInUseByDecoderAndVideoFrame",
         memory_pool_->NumberOfFrameBuffersInUseByDecoderAndVideoFrame());
 
-    return;
+    return true;
   }
 
   DCHECK(codec_format == PIXEL_FORMAT_YV12 ||
          codec_format == PIXEL_FORMAT_YV12A);
 
   *video_frame = frame_pool_.CreateFrame(
       codec_format, visible_size, gfx::Rect(visible_size),
       config_.natural_size(), kNoTimestamp());
 
   libyuv::I420Copy(
       vpx_image->planes[VPX_PLANE_Y], vpx_image->stride[VPX_PLANE_Y],
       vpx_image->planes[VPX_PLANE_U], vpx_image->stride[VPX_PLANE_U],
       vpx_image->planes[VPX_PLANE_V], vpx_image->stride[VPX_PLANE_V],
       (*video_frame)->visible_data(VideoFrame::kYPlane),
       (*video_frame)->stride(VideoFrame::kYPlane),
       (*video_frame)->visible_data(VideoFrame::kUPlane),
       (*video_frame)->stride(VideoFrame::kUPlane),
       (*video_frame)->visible_data(VideoFrame::kVPlane),
       (*video_frame)->stride(VideoFrame::kVPlane), coded_size.width(),
       coded_size.height());
+
+  return true;
 }
 
 }  // namespace media