Remove width_ and height_ from various VideoDecoder implementations.

Updated unit tests as well.

BUG=none
TEST=media_unittests

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

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

6 files changed, 62 insertions, 73 deletions

diff --git a/media/base/limits.h b/media/base/limits.h
index cef29ec..a2dbc43 100644
--- a/media/base/limits.h
+++ b/media/base/limits.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -13,8 +13,8 @@ namespace media {
 
 struct Limits {
   // For video.
-  static const size_t kMaxDimension = (1 << 15) - 1;  // 32767
-  static const size_t kMaxCanvas = (1 << (14 * 2));  // 16384 x 16384
+  static const int kMaxDimension = (1 << 15) - 1;  // 32767
+  static const int kMaxCanvas = (1 << (14 * 2));  // 16384 x 16384
 
   // Total number of video frames which are populating in the pipeline.
   static const size_t kMaxVideoFrames = 4;
diff --git a/media/filters/ffmpeg_video_decoder.cc b/media/filters/ffmpeg_video_decoder.cc
index d4dca2f..378f410 100644
--- a/media/filters/ffmpeg_video_decoder.cc
+++ b/media/filters/ffmpeg_video_decoder.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -24,8 +24,6 @@ namespace media {
 FFmpegVideoDecoder::FFmpegVideoDecoder(MessageLoop* message_loop,
                                        VideoDecodeContext* decode_context)
     : message_loop_(message_loop),
-      width_(0),
-      height_(0),
       time_base_(new AVRational()),
       state_(kUnInitialized),
       decode_engine_(new FFmpegVideoDecodeEngine()),
@@ -61,7 +59,7 @@ void FFmpegVideoDecoder::Initialize(DemuxerStream* demuxer_stream,
   AVStreamProvider* av_stream_provider;
   if (!demuxer_stream->QueryInterface(&av_stream_provider)) {
     VideoCodecInfo info = {0};
-    FFmpegVideoDecoder::OnInitializeComplete(info);
+    OnInitializeComplete(info);
     return;
   }
   AVStream* av_stream = av_stream_provider->GetAVStream();
@@ -69,15 +67,13 @@ void FFmpegVideoDecoder::Initialize(DemuxerStream* demuxer_stream,
   time_base_->den = av_stream->r_frame_rate.num;
   time_base_->num = av_stream->r_frame_rate.den;
 
-  // TODO(ajwong): We don't need these extra variables if |media_format_| has
-  // them.  Remove.
-  width_ = av_stream->codec->width;
-  height_ = av_stream->codec->height;
-  if (width_ > Limits::kMaxDimension ||
-      height_ > Limits::kMaxDimension ||
-      (width_ * height_) > Limits::kMaxCanvas) {
+  int width = av_stream->codec->width;
+  int height = av_stream->codec->height;
+  if (width > Limits::kMaxDimension ||
+      height > Limits::kMaxDimension ||
+      (width * height) > Limits::kMaxCanvas) {
     VideoCodecInfo info = {0};
-    FFmpegVideoDecoder::OnInitializeComplete(info);
+    OnInitializeComplete(info);
     return;
   }
 
@@ -99,23 +95,27 @@ void FFmpegVideoDecoder::Initialize(DemuxerStream* demuxer_stream,
       NOTREACHED();
   }
   config.opaque_context = av_stream;
-  config.width = width_;
-  config.height = height_;
+  config.width = width;
+  config.height = height;
   state_ = kInitializing;
   decode_engine_->Initialize(message_loop_, this, NULL, config);
 }
 
 void FFmpegVideoDecoder::OnInitializeComplete(const VideoCodecInfo& info) {
+  // TODO(scherkus): Dedup this from OmxVideoDecoder::OnInitializeComplete.
   DCHECK_EQ(MessageLoop::current(), message_loop_);
   DCHECK(initialize_callback_.get());
 
-  info_ = info;  // Save a copy.
+  info_ = info;
+  AutoCallbackRunner done_runner(initialize_callback_.release());
 
   if (info.success) {
     media_format_.SetAsString(MediaFormat::kMimeType,
                               mime_type::kUncompressedVideo);
-    media_format_.SetAsInteger(MediaFormat::kWidth, width_);
-    media_format_.SetAsInteger(MediaFormat::kHeight, height_);
+    media_format_.SetAsInteger(MediaFormat::kWidth,
+                               info.stream_info.surface_width);
+    media_format_.SetAsInteger(MediaFormat::kHeight,
+                               info.stream_info.surface_height);
     media_format_.SetAsInteger(
         MediaFormat::kSurfaceType,
         static_cast<int>(info.stream_info.surface_type));
@@ -126,9 +126,6 @@ void FFmpegVideoDecoder::OnInitializeComplete(const VideoCodecInfo& info) {
   } else {
     host()->SetError(PIPELINE_ERROR_DECODE);
   }
-
-  initialize_callback_->Run();
-  initialize_callback_.reset();
 }
 
 void FFmpegVideoDecoder::Stop(FilterCallback* callback) {
diff --git a/media/filters/ffmpeg_video_decoder.h b/media/filters/ffmpeg_video_decoder.h
index 4e6e132..cd73631 100644
--- a/media/filters/ffmpeg_video_decoder.h
+++ b/media/filters/ffmpeg_video_decoder.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -25,6 +25,12 @@ class VideoDecodeEngine;
 class FFmpegVideoDecoder : public VideoDecoder,
                            public VideoDecodeEngine::EventHandler {
  public:
+  // Holds timestamp and duration data needed for properly enqueuing a frame.
+  struct TimeTuple {
+    base::TimeDelta timestamp;
+    base::TimeDelta duration;
+  };
+
   FFmpegVideoDecoder(MessageLoop* message_loop,
                      VideoDecodeContext* decode_context);
   virtual ~FFmpegVideoDecoder();
@@ -66,13 +72,6 @@ class FFmpegVideoDecoder : public VideoDecoder,
                            DoDecode_TestStateTransition);
   FRIEND_TEST_ALL_PREFIXES(FFmpegVideoDecoderTest, DoSeek);
 
-  // The TimeTuple struct is used to hold the needed timestamp data needed for
-  // enqueuing a video frame.
-  struct TimeTuple {
-    base::TimeDelta timestamp;
-    base::TimeDelta duration;
-  };
-
   enum DecoderState {
     kUnInitialized,
     kInitializing,
@@ -113,8 +112,6 @@ class FFmpegVideoDecoder : public VideoDecoder,
   virtual void SetVideoDecodeEngineForTest(VideoDecodeEngine* engine);
 
   MessageLoop* message_loop_;
-  size_t width_;
-  size_t height_;
   MediaFormat media_format_;
 
   PtsHeap pts_heap_;  // Heap of presentation timestamps.
diff --git a/media/filters/ffmpeg_video_decoder_unittest.cc b/media/filters/ffmpeg_video_decoder_unittest.cc
index 3b91d12..0ddcdf9 100644
--- a/media/filters/ffmpeg_video_decoder_unittest.cc
+++ b/media/filters/ffmpeg_video_decoder_unittest.cc
@@ -34,6 +34,19 @@ using ::testing::Invoke;
 
 namespace media {
 
+static const int kWidth = 1280;
+static const int kHeight = 720;
+static const FFmpegVideoDecoder::TimeTuple kTestPts1 =
+    { base::TimeDelta::FromMicroseconds(123),
+      base::TimeDelta::FromMicroseconds(50) };
+static const FFmpegVideoDecoder::TimeTuple kTestPts2 =
+    { base::TimeDelta::FromMicroseconds(456),
+      base::TimeDelta::FromMicroseconds(60) };
+static const FFmpegVideoDecoder::TimeTuple kTestPts3 =
+    { base::TimeDelta::FromMicroseconds(789),
+      base::TimeDelta::FromMicroseconds(60) };
+static const PipelineStatistics kStatistics;
+
 class MockFFmpegDemuxerStream : public MockDemuxerStream,
                                 public AVStreamProvider {
  public:
@@ -93,6 +106,10 @@ class DecoderPrivateMock : public FFmpegVideoDecoder {
 ACTION_P2(EngineInitialize, engine, success) {
   engine->event_handler_ = arg1;
   engine->info_.success = success;
+  engine->info_.stream_info.surface_type = VideoFrame::TYPE_SYSTEM_MEMORY;
+  engine->info_.stream_info.surface_format = VideoFrame::YV12;
+  engine->info_.stream_info.surface_width = kWidth;
+  engine->info_.stream_info.surface_height = kHeight;
   engine->event_handler_->OnInitializeComplete(engine->info_);
 }
 
@@ -115,13 +132,6 @@ ACTION_P(EngineSeek, engine) {
 // FFmpeg, pipeline and filter host mocks.
 class FFmpegVideoDecoderTest : public testing::Test {
  protected:
-  static const int kWidth;
-  static const int kHeight;
-  static const FFmpegVideoDecoder::TimeTuple kTestPts1;
-  static const FFmpegVideoDecoder::TimeTuple kTestPts2;
-  static const FFmpegVideoDecoder::TimeTuple kTestPts3;
-  static const PipelineStatistics kStatistics;
-
   FFmpegVideoDecoderTest() {
     MediaFormat media_format;
     media_format.SetAsString(MediaFormat::kMimeType, mime_type::kFFmpegVideo);
@@ -219,20 +229,6 @@ class FFmpegVideoDecoderTest : public testing::Test {
   DISALLOW_COPY_AND_ASSIGN(FFmpegVideoDecoderTest);
 };
 
-const int FFmpegVideoDecoderTest::kWidth = 1280;
-const int FFmpegVideoDecoderTest::kHeight = 720;
-const FFmpegVideoDecoder::TimeTuple FFmpegVideoDecoderTest::kTestPts1 =
-    { base::TimeDelta::FromMicroseconds(123),
-      base::TimeDelta::FromMicroseconds(50) };
-const FFmpegVideoDecoder::TimeTuple FFmpegVideoDecoderTest::kTestPts2 =
-    { base::TimeDelta::FromMicroseconds(456),
-      base::TimeDelta::FromMicroseconds(60) };
-const FFmpegVideoDecoder::TimeTuple FFmpegVideoDecoderTest::kTestPts3 =
-    { base::TimeDelta::FromMicroseconds(789),
-      base::TimeDelta::FromMicroseconds(60) };
-
-const PipelineStatistics FFmpegVideoDecoderTest::kStatistics;
-
 TEST_F(FFmpegVideoDecoderTest, Initialize_QueryInterfaceFails) {
   // Test QueryInterface returning NULL.
   EXPECT_CALL(*demuxer_, QueryInterface(AVStreamProvider::interface_id()))
diff --git a/media/filters/omx_video_decoder.cc b/media/filters/omx_video_decoder.cc
index bc8617b..73816f9 100644
--- a/media/filters/omx_video_decoder.cc
+++ b/media/filters/omx_video_decoder.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -20,8 +20,7 @@ OmxVideoDecoder::OmxVideoDecoder(
     VideoDecodeContext* context)
     : message_loop_(message_loop),
       decode_engine_(new OmxVideoDecodeEngine()),
-      decode_context_(context),
-      width_(0), height_(0) {
+      decode_context_(context) {
   DCHECK(decode_engine_.get());
   memset(&info_, 0, sizeof(info_));
 }
@@ -58,19 +57,18 @@ void OmxVideoDecoder::Initialize(DemuxerStream* demuxer_stream,
   AVStreamProvider* av_stream_provider;
   if (!demuxer_stream->QueryInterface(&av_stream_provider)) {
     VideoCodecInfo info = {0};
-    OmxVideoDecoder::OnInitializeComplete(info);
+    OnInitializeComplete(info);
     return;
   }
   AVStream* av_stream = av_stream_provider->GetAVStream();
 
-  // TODO(jiesun): shouldn't we check this in demuxer?
-  width_ = av_stream->codec->width;
-  height_ = av_stream->codec->height;
-  if (width_ > Limits::kMaxDimension ||
-      height_ > Limits::kMaxDimension ||
-      (width_ * height_) > Limits::kMaxCanvas) {
+  int width = av_stream->codec->width;
+  int height = av_stream->codec->height;
+  if (width > Limits::kMaxDimension ||
+      height > Limits::kMaxDimension ||
+      (width * height) > Limits::kMaxCanvas) {
     VideoCodecInfo info = {0};
-    OmxVideoDecoder::OnInitializeComplete(info);
+    OnInitializeComplete(info);
     return;
   }
 
@@ -90,23 +88,26 @@ void OmxVideoDecoder::Initialize(DemuxerStream* demuxer_stream,
       NOTREACHED();
   }
   config.opaque_context = NULL;
-  config.width = width_;
-  config.height = height_;
+  config.width = width;
+  config.height = height;
   decode_engine_->Initialize(message_loop_, this, NULL, config);
 }
 
 void OmxVideoDecoder::OnInitializeComplete(const VideoCodecInfo& info) {
+  // TODO(scherkus): Dedup this from FFmpegVideoDecoder::OnInitializeComplete.
   DCHECK_EQ(MessageLoop::current(), message_loop_);
   DCHECK(initialize_callback_.get());
 
-  info_ = info;  // Save a copy.
+  info_ = info;
   AutoCallbackRunner done_runner(initialize_callback_.release());
 
   if (info.success) {
     media_format_.SetAsString(MediaFormat::kMimeType,
                               mime_type::kUncompressedVideo);
-    media_format_.SetAsInteger(MediaFormat::kWidth, width_);
-    media_format_.SetAsInteger(MediaFormat::kHeight, height_);
+    media_format_.SetAsInteger(MediaFormat::kWidth,
+                               info.stream_info.surface_width);
+    media_format_.SetAsInteger(MediaFormat::kHeight,
+                               info.stream_info.surface_height);
     media_format_.SetAsInteger(
         MediaFormat::kSurfaceType,
         static_cast<int>(info.stream_info.surface_type));
diff --git a/media/filters/omx_video_decoder.h b/media/filters/omx_video_decoder.h
index 7c6a118..5a2837c 100644
--- a/media/filters/omx_video_decoder.h
+++ b/media/filters/omx_video_decoder.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -61,8 +61,6 @@ class OmxVideoDecoder : public VideoDecoder,
   scoped_ptr<VideoDecodeEngine> decode_engine_;
   scoped_ptr<VideoDecodeContext> decode_context_;
   MediaFormat media_format_;
-  size_t width_;
-  size_t height_;
 
   scoped_ptr<FilterCallback> initialize_callback_;
   scoped_ptr<FilterCallback> uninitialize_callback_;