Fix support for yuv_422 pixel format.

Added pix_fmt field to the VideoDecoderConfig class. The pixel format is passed to the
codec_context_ and used to correctly initialize VideoFrames.

Patch by shadi@chromium.org:
http://codereview.chromium.org/8052002/

BUG=95642

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@103961 0039d316-1c4b-4281-b951-d872f2087c98

8 files changed, 81 insertions, 27 deletions

diff --git a/media/base/video_decoder_config.cc b/media/base/video_decoder_config.cc
index ef5e0cb..81583c1 100644
--- a/media/base/video_decoder_config.cc
+++ b/media/base/video_decoder_config.cc
@@ -10,19 +10,21 @@ namespace media {
 
 VideoDecoderConfig::VideoDecoderConfig()
     : codec_(kUnknownVideoCodec),
+      format_(VideoFrame::INVALID),
       frame_rate_numerator_(0),
       frame_rate_denominator_(0),
       extra_data_size_(0) {
 }
 
 VideoDecoderConfig::VideoDecoderConfig(VideoCodec codec,
+                                       VideoFrame::Format format,
                                        const gfx::Size& coded_size,
                                        const gfx::Rect& visible_rect,
                                        int frame_rate_numerator,
                                        int frame_rate_denominator,
                                        const uint8* extra_data,
                                        size_t extra_data_size) {
-  Initialize(codec, coded_size, visible_rect,
+  Initialize(codec, format, coded_size, visible_rect,
              frame_rate_numerator, frame_rate_denominator,
              extra_data, extra_data_size);
 }
@@ -30,6 +32,7 @@ VideoDecoderConfig::VideoDecoderConfig(VideoCodec codec,
 VideoDecoderConfig::~VideoDecoderConfig() {}
 
 void VideoDecoderConfig::Initialize(VideoCodec codec,
+                                    VideoFrame::Format format,
                                     const gfx::Size& coded_size,
                                     const gfx::Rect& visible_rect,
                                     int frame_rate_numerator,
@@ -39,6 +42,7 @@ void VideoDecoderConfig::Initialize(VideoCodec codec,
   CHECK((extra_data_size != 0) == (extra_data != NULL));
 
   codec_ = codec;
+  format_ = format;
   coded_size_ = coded_size;
   visible_rect_ = visible_rect;
   frame_rate_numerator_ = frame_rate_numerator;
@@ -55,6 +59,7 @@ void VideoDecoderConfig::Initialize(VideoCodec codec,
 
 bool VideoDecoderConfig::IsValidConfig() const {
   return codec_ != kUnknownVideoCodec &&
+      format_ != VideoFrame::INVALID &&
       frame_rate_numerator_ > 0 &&
       frame_rate_denominator_ > 0;
 }
@@ -63,6 +68,10 @@ VideoCodec VideoDecoderConfig::codec() const {
   return codec_;
 }
 
+VideoFrame::Format VideoDecoderConfig::format() const {
+  return format_;
+}
+
 gfx::Size VideoDecoderConfig::coded_size() const {
   return coded_size_;
 }
diff --git a/media/base/video_decoder_config.h b/media/base/video_decoder_config.h
index 305bad6..44182df4 100644
--- a/media/base/video_decoder_config.h
+++ b/media/base/video_decoder_config.h
@@ -8,6 +8,7 @@
 #include "base/basictypes.h"
 #include "base/memory/scoped_ptr.h"
 #include "media/base/media_export.h"
+#include "media/base/video_frame.h"
 #include "ui/gfx/rect.h"
 #include "ui/gfx/size.h"
 
@@ -36,7 +37,9 @@ class MEDIA_EXPORT VideoDecoderConfig {
 
   // Constructs an initialized object. It is acceptable to pass in NULL for
   // |extra_data|, otherwise the memory is copied.
-  VideoDecoderConfig(VideoCodec codec, const gfx::Size& coded_size,
+  VideoDecoderConfig(VideoCodec codec,
+                     VideoFrame::Format format,
+                     const gfx::Size& coded_size,
                      const gfx::Rect& visible_rect,
                      int frame_rate_numerator, int frame_rate_denominator,
                      const uint8* extra_data, size_t extra_data_size);
@@ -44,7 +47,9 @@ class MEDIA_EXPORT VideoDecoderConfig {
   ~VideoDecoderConfig();
 
   // Resets the internal state of this object.
-  void Initialize(VideoCodec codec, const gfx::Size& coded_size,
+  void Initialize(VideoCodec codec,
+                  VideoFrame::Format format,
+                  const gfx::Size& coded_size,
                   const gfx::Rect& visible_rect,
                   int frame_rate_numerator, int frame_rate_denominator,
                   const uint8* extra_data, size_t extra_data_size);
@@ -55,6 +60,9 @@ class MEDIA_EXPORT VideoDecoderConfig {
 
   VideoCodec codec() const;
 
+  // Video format used to determine YUV buffer sizes.
+  VideoFrame::Format format() const;
+
   // Width and height of video frame immediately post-decode. Not all pixels
   // in this region are valid.
   gfx::Size coded_size() const;
@@ -75,6 +83,8 @@ class MEDIA_EXPORT VideoDecoderConfig {
  private:
   VideoCodec codec_;
 
+  VideoFrame::Format format_;
+
   gfx::Size coded_size_;
   gfx::Rect visible_rect_;
 
diff --git a/media/base/video_frame.cc b/media/base/video_frame.cc
index c40c58f..340ba19 100644
--- a/media/base/video_frame.cc
+++ b/media/base/video_frame.cc
@@ -75,9 +75,12 @@ scoped_refptr<VideoFrame> VideoFrame::CreateBlackFrame(int width, int height) {
   // Fill the U and V planes.
   uint8* u_plane = frame->data(VideoFrame::kUPlane);
   uint8* v_plane = frame->data(VideoFrame::kVPlane);
-  for (size_t i = 0; i < (frame->height_ / 2); ++i) {
-    memset(u_plane, kBlackUV, frame->width_ / 2);
-    memset(v_plane, kBlackUV, frame->width_ / 2);
+  int uv_rows = frame->rows(VideoFrame::kUPlane);
+  int u_row_bytes = frame->row_bytes(VideoFrame::kUPlane);
+  int v_row_bytes = frame->row_bytes(VideoFrame::kVPlane);
+  for (size_t i = 0; i < (size_t)uv_rows; ++i) {
+    memset(u_plane, kBlackUV, u_row_bytes);
+    memset(v_plane, kBlackUV, v_row_bytes);
     u_plane += frame->stride(VideoFrame::kUPlane);
     v_plane += frame->stride(VideoFrame::kVPlane);
   }
@@ -115,21 +118,20 @@ void VideoFrame::AllocateYUV() {
   // to avoid any potential of faulting by code that attempts to access the Y
   // values of the final row, but assumes that the last row of U & V applies to
   // a full two rows of Y.
-  size_t alloc_height = RoundUp(height_, 2);
-  size_t y_bytes_per_row = RoundUp(width_, 4);
-  size_t uv_stride = RoundUp(y_bytes_per_row / 2, 4);
-  size_t y_bytes = alloc_height * y_bytes_per_row;
-  size_t uv_bytes = alloc_height * uv_stride;
-  if (format_ == VideoFrame::YV12) {
-    uv_bytes /= 2;
-  }
+  size_t y_height = RoundUp(rows(VideoFrame::kYPlane), 2);
+  size_t y_stride = RoundUp(row_bytes(VideoFrame::kYPlane), 4);
+  size_t uv_stride = RoundUp(row_bytes(VideoFrame::kUPlane), 4);
+  size_t uv_height = RoundUp(rows(VideoFrame::kUPlane), 2);
+  size_t y_bytes = y_height * y_stride;
+  size_t uv_bytes = uv_height * uv_stride;
+
   uint8* data = new uint8[y_bytes + (uv_bytes * 2) + kFramePadBytes];
   planes_ = VideoFrame::kNumYUVPlanes;
   COMPILE_ASSERT(0 == VideoFrame::kYPlane, y_plane_data_must_be_index_0);
   data_[VideoFrame::kYPlane] = data;
   data_[VideoFrame::kUPlane] = data + y_bytes;
   data_[VideoFrame::kVPlane] = data + y_bytes + uv_bytes;
-  strides_[VideoFrame::kYPlane] = y_bytes_per_row;
+  strides_[VideoFrame::kYPlane] = y_stride;
   strides_[VideoFrame::kUPlane] = uv_stride;
   strides_[VideoFrame::kVPlane] = uv_stride;
 }
diff --git a/media/ffmpeg/ffmpeg_common.cc b/media/ffmpeg/ffmpeg_common.cc
index f1835ce..7807d3f 100644
--- a/media/ffmpeg/ffmpeg_common.cc
+++ b/media/ffmpeg/ffmpeg_common.cc
@@ -215,6 +215,30 @@ ChannelLayout ChannelLayoutToChromeChannelLayout(int64_t layout,
   }
 }
 
+VideoFrame::Format PixelFormatToVideoFormat(PixelFormat pixel_format) {
+  switch (pixel_format) {
+    case PIX_FMT_YUV422P:
+      return VideoFrame::YV16;
+    case PIX_FMT_YUV420P:
+      return VideoFrame::YV12;
+    default:
+      NOTREACHED() << "Unsupported PixelFormat: " << pixel_format;
+  }
+  return VideoFrame::INVALID;
+}
+
+PixelFormat VideoFormatToPixelFormat(VideoFrame::Format video_format) {
+  switch (video_format) {
+    case VideoFrame::YV16:
+      return PIX_FMT_YUV422P;
+    case VideoFrame::YV12:
+      return PIX_FMT_YUV420P;
+    default:
+      NOTREACHED() << "Unsupported VideoFrame Format: " << video_format;
+  }
+  return PIX_FMT_NONE;
+}
+
 base::TimeDelta GetFrameDuration(AVStream* stream) {
   AVRational time_base = { stream->r_frame_rate.den, stream->r_frame_rate.num };
   return ConvertFromTimeBase(time_base, 1);
diff --git a/media/ffmpeg/ffmpeg_common.h b/media/ffmpeg/ffmpeg_common.h
index cc15e58..8808f45 100644
--- a/media/ffmpeg/ffmpeg_common.h
+++ b/media/ffmpeg/ffmpeg_common.h
@@ -83,6 +83,12 @@ CodecID VideoCodecToCodecID(VideoCodec video_codec);
 ChannelLayout ChannelLayoutToChromeChannelLayout(int64_t layout,
                                                  int channels);
 
+// Converts FFmpeg's pixel formats to its corresponding supported video format.
+VideoFrame::Format PixelFormatToVideoFormat(PixelFormat pixel_format);
+
+// Converts video formats to its corresponding FFmpeg's pixel formats.
+PixelFormat VideoFormatToPixelFormat(VideoFrame::Format video_format);
+
 // Calculates duration of one frame in the |stream| based on its frame rate.
 base::TimeDelta GetFrameDuration(AVStream* stream);
 
diff --git a/media/filters/ffmpeg_video_decoder.cc b/media/filters/ffmpeg_video_decoder.cc
index 677fae5..d41b796 100644
--- a/media/filters/ffmpeg_video_decoder.cc
+++ b/media/filters/ffmpeg_video_decoder.cc
@@ -82,6 +82,7 @@ void FFmpegVideoDecoder::Initialize(DemuxerStream* demuxer_stream,
   }
 
   VideoDecoderConfig config(CodecIDToVideoCodec(av_stream->codec->codec_id),
+                            PixelFormatToVideoFormat(av_stream->codec->pix_fmt),
                             coded_size, visible_rect,
                             av_stream->r_frame_rate.num,
                             av_stream->r_frame_rate.den,
diff --git a/media/video/ffmpeg_video_decode_engine.cc b/media/video/ffmpeg_video_decode_engine.cc
index 8d547b9..46c02d9 100644
--- a/media/video/ffmpeg_video_decode_engine.cc
+++ b/media/video/ffmpeg_video_decode_engine.cc
@@ -55,9 +55,7 @@ void FFmpegVideoDecodeEngine::Initialize(
 
   // Initialize AVCodecContext structure.
   codec_context_ = avcodec_alloc_context();
-
-  // TODO(scherkus): should video format get passed in via VideoDecoderConfig?
-  codec_context_->pix_fmt = PIX_FMT_YUV420P;
+  codec_context_->pix_fmt = VideoFormatToPixelFormat(config.format());
   codec_context_->codec_type = AVMEDIA_TYPE_VIDEO;
   codec_context_->codec_id = VideoCodecToCodecID(config.codec());
   codec_context_->coded_width = config.coded_size().width();
@@ -110,11 +108,11 @@ void FFmpegVideoDecodeEngine::Initialize(
   // Create output buffer pool when direct rendering is not used.
   for (size_t i = 0; i < Limits::kMaxVideoFrames; ++i) {
     scoped_refptr<VideoFrame> video_frame =
-        VideoFrame::CreateFrame(VideoFrame::YV12,
-                                config.visible_rect().width(),
-                                config.visible_rect().height(),
-                                kNoTimestamp,
-                                kNoTimestamp);
+      VideoFrame::CreateFrame(PixelFormatToVideoFormat(codec_context_->pix_fmt),
+                              config.visible_rect().width(),
+                              config.visible_rect().height(),
+                              kNoTimestamp,
+                              kNoTimestamp);
     frame_queue_available_.push_back(video_frame);
   }
 
@@ -244,7 +242,8 @@ void FFmpegVideoDecodeEngine::DecodeFrame(scoped_refptr<Buffer> buffer) {
   // output, meaning the data is only valid until the next
   // avcodec_decode_video() call.
   int y_rows = codec_context_->height;
-  int uv_rows = codec_context_->height / 2;
+  int uv_rows = video_frame->rows(VideoFrame::kUPlane);
+
   CopyYPlane(av_frame_->data[0], av_frame_->linesize[0], y_rows, video_frame);
   CopyUPlane(av_frame_->data[1], av_frame_->linesize[1], uv_rows, video_frame);
   CopyVPlane(av_frame_->data[2], av_frame_->linesize[2], uv_rows, video_frame);
diff --git a/media/video/ffmpeg_video_decode_engine_unittest.cc b/media/video/ffmpeg_video_decode_engine_unittest.cc
index 581fb3d..b4ce626 100644
--- a/media/video/ffmpeg_video_decode_engine_unittest.cc
+++ b/media/video/ffmpeg_video_decode_engine_unittest.cc
@@ -22,6 +22,7 @@ using ::testing::StrictMock;
 
 namespace media {
 
+static const VideoFrame::Format kVideoFormat = VideoFrame::YV12;
 static const gfx::Size kCodedSize(320, 240);
 static const gfx::Rect kVisibleRect(320, 240);
 static const gfx::Size kNaturalSize(522, 288);
@@ -36,7 +37,7 @@ class FFmpegVideoDecodeEngineTest
       public VideoDecodeEngine::EventHandler {
  public:
   FFmpegVideoDecodeEngineTest()
-      : config_(kCodecVP8, kCodedSize, kVisibleRect,
+      : config_(kCodecVP8, kVideoFormat, kCodedSize, kVisibleRect,
                 kFrameRate.num, kFrameRate.den, NULL, 0) {
     CHECK(FFmpegGlue::GetInstance());
 
@@ -142,7 +143,8 @@ TEST_F(FFmpegVideoDecodeEngineTest, Initialize_Normal) {
 }
 
 TEST_F(FFmpegVideoDecodeEngineTest, Initialize_FindDecoderFails) {
-  VideoDecoderConfig config(kUnknownVideoCodec, kCodedSize, kVisibleRect,
+  VideoDecoderConfig config(kUnknownVideoCodec, kVideoFormat,
+                            kCodedSize, kVisibleRect,
                             kFrameRate.num, kFrameRate.den,
                             NULL, 0);
 
@@ -153,7 +155,8 @@ TEST_F(FFmpegVideoDecodeEngineTest, Initialize_FindDecoderFails) {
 
 TEST_F(FFmpegVideoDecodeEngineTest, Initialize_OpenDecoderFails) {
   // Specify Theora w/o extra data so that avcodec_open() fails.
-  VideoDecoderConfig config(kCodecTheora, kCodedSize, kVisibleRect,
+  VideoDecoderConfig config(kCodecTheora, kVideoFormat,
+                            kCodedSize, kVisibleRect,
                             kFrameRate.num, kFrameRate.den,
                             NULL, 0);
   EXPECT_CALL(*this, OnInitializeComplete(false));