Remove DemuxerStream::GetAVStream() once and for all.

We now use AudioDecoderConfig and VideoDecoderConfig to pass decoder initialization information.


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

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

8 files changed, 71 insertions, 75 deletions

diff --git a/media/filters/chunk_demuxer.cc b/media/filters/chunk_demuxer.cc
index b75a71a..383b9e3 100644
--- a/media/filters/chunk_demuxer.cc
+++ b/media/filters/chunk_demuxer.cc
@@ -29,8 +29,8 @@ namespace media {
 // we are making the INFO & TRACKS data look like a small WebM
 // file so we can use FFmpeg to initialize the AVFormatContext.
 //
-// TODO(acolwell): Remove this once GetAVStream() has been removed from
-// the DemuxerStream interface.
+// TODO(acolwell): Remove this when we construct AudioDecoderConfig and
+// VideoDecoderConfig without requiring an AVStream object.
 static const uint8 kWebMHeader[] = {
   0x1A, 0x45, 0xDF, 0xA3, 0x9F,  // EBML (size = 0x1f)
   0x42, 0x86, 0x81, 0x01,  // EBMLVersion = 1
@@ -79,13 +79,14 @@ class ChunkDemuxerStream : public DemuxerStream {
   virtual void Read(const ReadCallback& read_callback);
   virtual Type type();
   virtual void EnableBitstreamConverter();
-  virtual AVStream* GetAVStream();
   virtual const AudioDecoderConfig& audio_decoder_config();
+  virtual const VideoDecoderConfig& video_decoder_config();
 
  private:
   Type type_;
   AVStream* av_stream_;
   AudioDecoderConfig audio_config_;
+  VideoDecoderConfig video_config_;
 
   mutable base::Lock lock_;
   ReadCBQueue read_cbs_;
@@ -109,6 +110,8 @@ ChunkDemuxerStream::ChunkDemuxerStream(Type type, AVStream* stream)
       last_buffer_timestamp_(kNoTimestamp) {
   if (type_ == AUDIO) {
     AVCodecContextToAudioDecoderConfig(stream->codec, &audio_config_);
+  } else if (type_ == VIDEO) {
+    AVStreamToVideoDecoderConfig(stream, &video_config_);
   }
 }
 
@@ -271,13 +274,16 @@ DemuxerStream::Type ChunkDemuxerStream::type() { return type_; }
 
 void ChunkDemuxerStream::EnableBitstreamConverter() {}
 
-AVStream* ChunkDemuxerStream::GetAVStream() { return av_stream_; }
-
 const AudioDecoderConfig& ChunkDemuxerStream::audio_decoder_config() {
   CHECK_EQ(type_, AUDIO);
   return audio_config_;
 }
 
+const VideoDecoderConfig& ChunkDemuxerStream::video_decoder_config() {
+  CHECK_EQ(type_, VIDEO);
+  return video_config_;
+}
+
 ChunkDemuxer::ChunkDemuxer(ChunkDemuxerClient* client)
     : state_(WAITING_FOR_INIT),
       client_(client),
diff --git a/media/filters/dummy_demuxer.cc b/media/filters/dummy_demuxer.cc
index a9afbfc..24307b9 100644
--- a/media/filters/dummy_demuxer.cc
+++ b/media/filters/dummy_demuxer.cc
@@ -23,6 +23,11 @@ const AudioDecoderConfig& DummyDemuxerStream::audio_decoder_config() {
   return audio_config_;
 }
 
+const VideoDecoderConfig& DummyDemuxerStream::video_decoder_config() {
+  CHECK_EQ(type_, VIDEO);
+  return video_config_;
+}
+
 void DummyDemuxerStream::Read(const ReadCallback& read_callback) {}
 
 void DummyDemuxerStream::EnableBitstreamConverter() {}
diff --git a/media/filters/dummy_demuxer.h b/media/filters/dummy_demuxer.h
index 1b74d01..b029aa5 100644
--- a/media/filters/dummy_demuxer.h
+++ b/media/filters/dummy_demuxer.h
@@ -13,6 +13,7 @@
 
 #include "media/base/audio_decoder_config.h"
 #include "media/base/demuxer.h"
+#include "media/base/video_decoder_config.h"
 
 namespace media {
 
@@ -24,6 +25,7 @@ class DummyDemuxerStream : public DemuxerStream {
   virtual void Read(const ReadCallback& read_callback) OVERRIDE;
   virtual Type type() OVERRIDE;
   virtual const AudioDecoderConfig& audio_decoder_config() OVERRIDE;
+  virtual const VideoDecoderConfig& video_decoder_config() OVERRIDE;
   virtual void EnableBitstreamConverter() OVERRIDE;
 
  private:
@@ -31,6 +33,7 @@ class DummyDemuxerStream : public DemuxerStream {
 
   Type type_;
   AudioDecoderConfig audio_config_;
+  VideoDecoderConfig video_config_;
 
   DISALLOW_COPY_AND_ASSIGN(DummyDemuxerStream);
 };
diff --git a/media/filters/ffmpeg_demuxer.cc b/media/filters/ffmpeg_demuxer.cc
index 45e5298..0c63fde 100644
--- a/media/filters/ffmpeg_demuxer.cc
+++ b/media/filters/ffmpeg_demuxer.cc
@@ -73,6 +73,7 @@ FFmpegDemuxerStream::FFmpegDemuxerStream(FFmpegDemuxer* demuxer,
       break;
     case AVMEDIA_TYPE_VIDEO:
       type_ = VIDEO;
+      AVStreamToVideoDecoderConfig(stream, &video_config_);
       break;
     default:
       NOTREACHED();
@@ -254,15 +255,16 @@ void FFmpegDemuxerStream::EnableBitstreamConverter() {
   }
 }
 
-AVStream* FFmpegDemuxerStream::GetAVStream() {
-  return stream_;
-}
-
 const AudioDecoderConfig& FFmpegDemuxerStream::audio_decoder_config() {
   CHECK_EQ(type_, AUDIO);
   return audio_config_;
 }
 
+const VideoDecoderConfig& FFmpegDemuxerStream::video_decoder_config() {
+  CHECK_EQ(type_, VIDEO);
+  return video_config_;
+}
+
 // static
 base::TimeDelta FFmpegDemuxerStream::ConvertStreamTimestamp(
     const AVRational& time_base, int64 timestamp) {
@@ -684,8 +686,7 @@ void FFmpegDemuxer::DisableAudioStreamTask() {
     // If the codec type is audio, remove the reference. DemuxTask() will
     // look for such reference, and this will result in deleting the
     // audio packets after they are demuxed.
-    if (packet_streams_[i]->GetAVStream()->codec->codec_type ==
-        AVMEDIA_TYPE_AUDIO) {
+    if (packet_streams_[i]->type() == DemuxerStream::AUDIO) {
       packet_streams_[i] = NULL;
     }
   }
diff --git a/media/filters/ffmpeg_demuxer.h b/media/filters/ffmpeg_demuxer.h
index 1c1dfc2..8bf7705 100644
--- a/media/filters/ffmpeg_demuxer.h
+++ b/media/filters/ffmpeg_demuxer.h
@@ -32,6 +32,7 @@
 #include "media/base/buffers.h"
 #include "media/base/demuxer.h"
 #include "media/base/pipeline.h"
+#include "media/base/video_decoder_config.h"
 #include "media/filters/ffmpeg_glue.h"
 
 // FFmpeg forward declarations.
@@ -81,8 +82,8 @@ class FFmpegDemuxerStream : public DemuxerStream {
   // |lock_| is held throughout the life of the callback.
   virtual void Read(const ReadCallback& read_callback) OVERRIDE;
   virtual void EnableBitstreamConverter() OVERRIDE;
-  virtual AVStream* GetAVStream() OVERRIDE;
   virtual const AudioDecoderConfig& audio_decoder_config() OVERRIDE;
+  virtual const VideoDecoderConfig& video_decoder_config() OVERRIDE;
 
  private:
   virtual ~FFmpegDemuxerStream();
@@ -102,6 +103,7 @@ class FFmpegDemuxerStream : public DemuxerStream {
   FFmpegDemuxer* demuxer_;
   AVStream* stream_;
   AudioDecoderConfig audio_config_;
+  VideoDecoderConfig video_config_;
   Type type_;
   base::TimeDelta duration_;
   bool discontinuous_;
diff --git a/media/filters/ffmpeg_demuxer_unittest.cc b/media/filters/ffmpeg_demuxer_unittest.cc
index 64c6095..4af3060 100644
--- a/media/filters/ffmpeg_demuxer_unittest.cc
+++ b/media/filters/ffmpeg_demuxer_unittest.cc
@@ -182,22 +182,35 @@ TEST_F(FFmpegDemuxerTest, Initialize_Successful) {
       demuxer_->GetStream(DemuxerStream::VIDEO);
   ASSERT_TRUE(stream);
   EXPECT_EQ(DemuxerStream::VIDEO, stream->type());
-  ASSERT_TRUE(stream->GetAVStream());
+
+  const VideoDecoderConfig& video_config = stream->video_decoder_config();
+  EXPECT_EQ(kCodecVP8, video_config.codec());
+  EXPECT_EQ(VideoFrame::YV12, video_config.format());
+  EXPECT_EQ(320, video_config.coded_size().width());
+  EXPECT_EQ(240, video_config.coded_size().height());
+  EXPECT_EQ(0, video_config.visible_rect().x());
+  EXPECT_EQ(0, video_config.visible_rect().y());
+  EXPECT_EQ(320, video_config.visible_rect().width());
+  EXPECT_EQ(240, video_config.visible_rect().height());
+  EXPECT_EQ(30000, video_config.frame_rate_numerator());
+  EXPECT_EQ(1001, video_config.frame_rate_denominator());
+  EXPECT_EQ(1, video_config.aspect_ratio_numerator());
+  EXPECT_EQ(1, video_config.aspect_ratio_denominator());
+  EXPECT_FALSE(video_config.extra_data());
+  EXPECT_EQ(0u, video_config.extra_data_size());
 
   // Audio stream should be present.
   stream = demuxer_->GetStream(DemuxerStream::AUDIO);
   ASSERT_TRUE(stream);
   EXPECT_EQ(DemuxerStream::AUDIO, stream->type());
-  ASSERT_TRUE(stream->GetAVStream());
-
-  // FFmpegDemuxer's audio streams support AudioDecoderConfig structs.
-  const AudioDecoderConfig& config = stream->audio_decoder_config();
-  EXPECT_EQ(kCodecVorbis, config.codec());
-  EXPECT_EQ(16, config.bits_per_channel());
-  EXPECT_EQ(CHANNEL_LAYOUT_STEREO, config.channel_layout());
-  EXPECT_EQ(44100, config.samples_per_second());
-  EXPECT_TRUE(config.extra_data());
-  EXPECT_GT(config.extra_data_size(), 0u);
+
+  const AudioDecoderConfig& audio_config = stream->audio_decoder_config();
+  EXPECT_EQ(kCodecVorbis, audio_config.codec());
+  EXPECT_EQ(16, audio_config.bits_per_channel());
+  EXPECT_EQ(CHANNEL_LAYOUT_STEREO, audio_config.channel_layout());
+  EXPECT_EQ(44100, audio_config.samples_per_second());
+  EXPECT_TRUE(audio_config.extra_data());
+  EXPECT_GT(audio_config.extra_data_size(), 0u);
 }
 
 TEST_F(FFmpegDemuxerTest, Read_Audio) {
diff --git a/media/filters/ffmpeg_video_decoder.cc b/media/filters/ffmpeg_video_decoder.cc
index d540227..998eea9 100644
--- a/media/filters/ffmpeg_video_decoder.cc
+++ b/media/filters/ffmpeg_video_decoder.cc
@@ -58,22 +58,11 @@ void FFmpegVideoDecoder::Initialize(DemuxerStream* demuxer_stream,
   initialize_callback_ = callback;
   statistics_callback_ = stats_callback;
 
-  AVStream* av_stream = demuxer_stream->GetAVStream();
-  if (!av_stream) {
-    OnInitializeComplete(false);
-    return;
-  }
+  const VideoDecoderConfig& config = demuxer_stream->video_decoder_config();
 
-  pts_stream_.Initialize(GetFrameDuration(av_stream));
+  pts_stream_.Initialize(GetFrameDuration(config));
 
-  gfx::Size coded_size(
-      av_stream->codec->coded_width, av_stream->codec->coded_height);
-  // TODO(vrk): This assumes decoded frame data starts at (0, 0), which is true
-  // for now, but may not always be true forever. Fix this in the future.
-  gfx::Rect visible_rect(
-      av_stream->codec->width, av_stream->codec->height);
-
-  natural_size_ = GetNaturalSize(av_stream);
+  natural_size_ = config.natural_size();
   if (natural_size_.width() > Limits::kMaxDimension ||
       natural_size_.height() > Limits::kMaxDimension ||
       natural_size_.GetArea() > Limits::kMaxCanvas) {
@@ -81,14 +70,6 @@ void FFmpegVideoDecoder::Initialize(DemuxerStream* demuxer_stream,
     return;
   }
 
-  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,
-                            av_stream->codec->extradata,
-                            av_stream->codec->extradata_size);
-
   state_ = kInitializing;
   decode_engine_->Initialize(message_loop_, this, NULL, config);
 }
diff --git a/media/filters/ffmpeg_video_decoder_unittest.cc b/media/filters/ffmpeg_video_decoder_unittest.cc
index 8b6a8e6..a2ba238 100644
--- a/media/filters/ffmpeg_video_decoder_unittest.cc
+++ b/media/filters/ffmpeg_video_decoder_unittest.cc
@@ -26,6 +26,7 @@ using ::testing::DoAll;
 using ::testing::Message;
 using ::testing::Return;
 using ::testing::ReturnNull;
+using ::testing::ReturnRef;
 using ::testing::SetArgumentPointee;
 using ::testing::StrictMock;
 using ::testing::WithArg;
@@ -33,9 +34,12 @@ using ::testing::Invoke;
 
 namespace media {
 
+static const VideoFrame::Format kVideoFormat = VideoFrame::YV12;
 static const gfx::Size kCodedSize(1280, 720);
 static const gfx::Rect kVisibleRect(1280, 720);
 static const gfx::Size kNaturalSize(1280, 720);
+static const AVRational kFrameRate = { 100, 1 };
+static const AVRational kAspectRatio = { 1, 1 };
 
 // Holds timestamp and duration data needed for properly enqueuing a frame.
 struct TimeTuple {
@@ -134,26 +138,22 @@ class FFmpegVideoDecoderTest : public testing::Test {
     demuxer_ = new StrictMock<MockDemuxerStream>();
 
     // Initialize FFmpeg fixtures.
-    memset(&stream_, 0, sizeof(stream_));
-    memset(&codec_context_, 0, sizeof(codec_context_));
-    memset(&codec_, 0, sizeof(codec_));
     memset(&yuv_frame_, 0, sizeof(yuv_frame_));
     base::TimeDelta zero;
     video_frame_ = VideoFrame::CreateFrame(VideoFrame::YV12,
                                            kVisibleRect.width(),
                                            kVisibleRect.height(),
                                            zero, zero);
-    stream_.codec = &codec_context_;
-    codec_context_.width = kVisibleRect.width();
-    codec_context_.height = kVisibleRect.height();
-    codec_context_.codec_id = CODEC_ID_H264;
-    stream_.r_frame_rate.num = 1;
-    stream_.r_frame_rate.den = 1;
     buffer_ = new DataBuffer(1);
     end_of_stream_buffer_ = new DataBuffer(0);
 
     EXPECT_CALL(stats_callback_object_, OnStatistics(_))
         .Times(AnyNumber());
+
+    config_.Initialize(kCodecVP8, kVideoFormat, kCodedSize, kVisibleRect,
+                       kFrameRate.num, kFrameRate.den,
+                       kAspectRatio.num, kAspectRatio.den,
+                       NULL, 0);
   }
 
   virtual ~FFmpegVideoDecoderTest() {
@@ -170,9 +170,8 @@ class FFmpegVideoDecoderTest : public testing::Test {
   }
 
   void InitializeDecoderSuccessfully() {
-    // Test successful initialization.
-    EXPECT_CALL(*demuxer_, GetAVStream())
-        .WillOnce(Return(&stream_));
+    EXPECT_CALL(*demuxer_, video_decoder_config())
+        .WillOnce(ReturnRef(config_));
 
     EXPECT_CALL(*engine_, Initialize(_, _, _, _))
         .WillOnce(EngineInitialize(engine_, true));
@@ -199,32 +198,18 @@ class FFmpegVideoDecoderTest : public testing::Test {
   MessageLoop message_loop_;
 
   // FFmpeg fixtures.
-  AVStream stream_;
-  AVCodecContext codec_context_;
-  AVCodec codec_;
   AVFrame yuv_frame_;
   scoped_refptr<VideoFrame> video_frame_;
 
+  VideoDecoderConfig config_;
+
  private:
   DISALLOW_COPY_AND_ASSIGN(FFmpegVideoDecoderTest);
 };
 
-TEST_F(FFmpegVideoDecoderTest, Initialize_GetAVStreamFails) {
-  // Test GetAVStream returning NULL.
-  EXPECT_CALL(*demuxer_, GetAVStream())
-      .WillOnce(ReturnNull());
-  EXPECT_CALL(host_, SetError(PIPELINE_ERROR_DECODE));
-
-  decoder_->Initialize(demuxer_,
-                       NewExpectedClosure(), NewStatisticsCallback());
-
-  message_loop_.RunAllPending();
-}
-
 TEST_F(FFmpegVideoDecoderTest, Initialize_EngineFails) {
-  // Test successful initialization.
-  EXPECT_CALL(*demuxer_, GetAVStream())
-      .WillOnce(Return(&stream_));
+  EXPECT_CALL(*demuxer_, video_decoder_config())
+      .WillOnce(ReturnRef(config_));
 
   EXPECT_CALL(*engine_, Initialize(_, _, _, _))
       .WillOnce(EngineInitialize(engine_, false));