Fix aspect ratio and clarify video frame dimensions

BUG=18941,94861
TEST=video-aspect-ratio.html

Review URL: http://codereview.chromium.org/7864009

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

3 files changed, 27 insertions, 31 deletions

diff --git a/media/video/ffmpeg_video_decode_engine.cc b/media/video/ffmpeg_video_decode_engine.cc
index ced1b60..65edc2f 100644
--- a/media/video/ffmpeg_video_decode_engine.cc
+++ b/media/video/ffmpeg_video_decode_engine.cc
@@ -61,8 +61,8 @@ void FFmpegVideoDecodeEngine::Initialize(
   codec_context_->pix_fmt = PIX_FMT_YUV420P;
   codec_context_->codec_type = AVMEDIA_TYPE_VIDEO;
   codec_context_->codec_id = VideoCodecToCodecID(config.codec());
-  codec_context_->coded_width = config.width();
-  codec_context_->coded_height = config.height();
+  codec_context_->coded_width = config.coded_size().width();
+  codec_context_->coded_height = config.coded_size().height();
 
   frame_rate_numerator_ = config.frame_rate_numerator();
   frame_rate_denominator_ = config.frame_rate_denominator();
@@ -104,8 +104,7 @@ void FFmpegVideoDecodeEngine::Initialize(
   av_frame_.reset(avcodec_alloc_frame());
   VideoCodecInfo info;
   info.success = false;
-  info.surface_width = config.surface_width();
-  info.surface_height = config.surface_height();
+  info.natural_size = config.natural_size();
 
   // If we do not have enough buffers, we will report error too.
   frame_queue_available_.clear();
@@ -114,8 +113,8 @@ void FFmpegVideoDecodeEngine::Initialize(
   for (size_t i = 0; i < Limits::kMaxVideoFrames; ++i) {
     scoped_refptr<VideoFrame> video_frame =
         VideoFrame::CreateFrame(VideoFrame::YV12,
-                                config.surface_width(),
-                                config.surface_height(),
+                                config.visible_rect().width(),
+                                config.visible_rect().height(),
                                 kNoTimestamp,
                                 kNoTimestamp);
     frame_queue_available_.push_back(video_frame);
diff --git a/media/video/ffmpeg_video_decode_engine_unittest.cc b/media/video/ffmpeg_video_decode_engine_unittest.cc
index 0368e63..e8238ff 100644
--- a/media/video/ffmpeg_video_decode_engine_unittest.cc
+++ b/media/video/ffmpeg_video_decode_engine_unittest.cc
@@ -23,10 +23,9 @@ using ::testing::StrictMock;
 
 namespace media {
 
-static const size_t kWidth = 320;
-static const size_t kHeight = 240;
-static const size_t kSurfaceWidth = 522;
-static const size_t kSurfaceHeight = 288;
+static const gfx::Size kCodedSize(320, 240);
+static const gfx::Rect kVisibleRect(320, 240);
+static const gfx::Size kNaturalSize(522, 288);
 static const AVRational kFrameRate = { 100, 1 };
 
 ACTION_P2(DemuxComplete, engine, buffer) {
@@ -38,12 +37,12 @@ class FFmpegVideoDecodeEngineTest
       public VideoDecodeEngine::EventHandler {
  public:
   FFmpegVideoDecodeEngineTest()
-      : config_(kCodecVP8, kWidth, kHeight, kSurfaceWidth, kSurfaceHeight,
+      : config_(kCodecVP8, kCodedSize, kVisibleRect, kNaturalSize,
                 kFrameRate.num, kFrameRate.den, NULL, 0) {
     CHECK(FFmpegGlue::GetInstance());
 
     // Setup FFmpeg structures.
-    frame_buffer_.reset(new uint8[kWidth * kHeight]);
+    frame_buffer_.reset(new uint8[kCodedSize.GetArea()]);
 
     test_engine_.reset(new FFmpegVideoDecodeEngine());
 
@@ -105,10 +104,13 @@ class FFmpegVideoDecodeEngineTest
     CallProduceVideoFrame();
     CallProduceVideoFrame();
 
-    EXPECT_EQ(kSurfaceWidth, video_frame_a->width());
-    EXPECT_EQ(kSurfaceHeight, video_frame_a->height());
-    EXPECT_EQ(kSurfaceWidth, video_frame_b->width());
-    EXPECT_EQ(kSurfaceHeight, video_frame_b->height());
+    size_t expected_width = static_cast<size_t>(kVisibleRect.width());
+    size_t expected_height = static_cast<size_t>(kVisibleRect.height());
+
+    EXPECT_EQ(expected_width, video_frame_a->width());
+    EXPECT_EQ(expected_height, video_frame_a->height());
+    EXPECT_EQ(expected_width, video_frame_b->width());
+    EXPECT_EQ(expected_height, video_frame_b->height());
   }
 
   // VideoDecodeEngine::EventHandler implementation.
@@ -125,11 +127,9 @@ class FFmpegVideoDecodeEngineTest
   MOCK_METHOD0(OnError, void());
 
   void CallProduceVideoFrame() {
-    test_engine_->ProduceVideoFrame(VideoFrame::CreateFrame(VideoFrame::YV12,
-                                                            kWidth,
-                                                            kHeight,
-                                                            kNoTimestamp,
-                                                            kNoTimestamp));
+    test_engine_->ProduceVideoFrame(VideoFrame::CreateFrame(
+        VideoFrame::YV12, kVisibleRect.width(), kVisibleRect.height(),
+        kNoTimestamp, kNoTimestamp));
   }
 
  protected:
@@ -149,9 +149,8 @@ TEST_F(FFmpegVideoDecodeEngineTest, Initialize_Normal) {
 }
 
 TEST_F(FFmpegVideoDecodeEngineTest, Initialize_FindDecoderFails) {
-  VideoDecoderConfig config(kUnknown, kWidth, kHeight, kSurfaceWidth,
-                            kSurfaceHeight, kFrameRate.num, kFrameRate.den,
-                            NULL, 0);
+  VideoDecoderConfig config(kUnknown, kCodedSize, kVisibleRect, kNaturalSize,
+                            kFrameRate.num, kFrameRate.den, NULL, 0);
   // Test avcodec_find_decoder() returning NULL.
   VideoCodecInfo info;
   EXPECT_CALL(*this, OnInitializeComplete(_))
@@ -162,8 +161,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, kWidth, kHeight, kSurfaceWidth,
-                            kSurfaceHeight, kFrameRate.num, kFrameRate.den,
+  VideoDecoderConfig config(kCodecTheora, kCodedSize, kVisibleRect,
+                            kNaturalSize, kFrameRate.num, kFrameRate.den,
                             NULL, 0);
   VideoCodecInfo info;
   EXPECT_CALL(*this, OnInitializeComplete(_))
diff --git a/media/video/video_decode_engine.h b/media/video/video_decode_engine.h
index 6171232..4ec7cf5 100644
--- a/media/video/video_decode_engine.h
+++ b/media/video/video_decode_engine.h
@@ -10,6 +10,7 @@
 #include "media/base/media_export.h"
 #include "media/base/video_decoder_config.h"
 #include "media/base/video_frame.h"
+#include "ui/gfx/size.h"
 
 class MessageLoop;
 
@@ -24,11 +25,8 @@ struct VideoCodecInfo {
   // Other parameter is only meaningful when this is true.
   bool success;
 
-  // Can be different with container's value.
-  uint32 surface_width;
-
-  // Can be different with container's value.
-  uint32 surface_height;
+  // Natural dimensions of video. May be different from coded and visible sizes.
+  gfx::Size natural_size;
 };
 
 class MEDIA_EXPORT VideoDecodeEngine {