Simplify the Demuxer interface to get streams by type.

Before this change Demuxer's interface looked as if it could return multiple
streams for a given type, but the implementations all assume/need at most a
single stream per type.  This change should make the (soon to be written)
AdaptiveDemuxer{,Stream} less confusing.

BUG=none
TEST=trybots

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

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

8 files changed, 62 insertions, 77 deletions

diff --git a/media/base/filters.h b/media/base/filters.h
index 654da5a..d6b962d 100644
--- a/media/base/filters.h
+++ b/media/base/filters.h
@@ -129,23 +129,13 @@ class DataSource : public Filter {
   virtual bool IsStreaming() = 0;
 };
 
-
-class Demuxer : public Filter {
- public:
-  // Returns the number of streams available
-  virtual size_t GetNumberOfStreams() = 0;
-
-  // Returns the stream for the given index, NULL otherwise
-  virtual scoped_refptr<DemuxerStream> GetStream(int stream_id) = 0;
-};
-
-
 class DemuxerStream : public base::RefCountedThreadSafe<DemuxerStream> {
  public:
   enum Type {
     UNKNOWN,
     AUDIO,
     VIDEO,
+    NUM_TYPES,  // Always keep this entry as the last one!
   };
 
   // Schedules a read.  When the |read_callback| is called, the downstream
@@ -186,6 +176,12 @@ class DemuxerStream : public base::RefCountedThreadSafe<DemuxerStream> {
   virtual ~DemuxerStream();
 };
 
+class Demuxer : public Filter {
+ public:
+  // Returns the given stream type, or NULL if that type is not present.
+  virtual scoped_refptr<DemuxerStream> GetStream(DemuxerStream::Type type) = 0;
+};
+
 
 class VideoDecoder : public Filter {
  public:
diff --git a/media/base/mock_filters.h b/media/base/mock_filters.h
index 8004190..c108c57 100644
--- a/media/base/mock_filters.h
+++ b/media/base/mock_filters.h
@@ -110,8 +110,7 @@ class MockDemuxer : public Demuxer {
   // Demuxer implementation.
   MOCK_METHOD2(Initialize, void(DataSource* data_source,
                                 FilterCallback* callback));
-  MOCK_METHOD0(GetNumberOfStreams, size_t());
-  MOCK_METHOD1(GetStream, scoped_refptr<DemuxerStream>(int stream_id));
+  MOCK_METHOD1(GetStream, scoped_refptr<DemuxerStream>(DemuxerStream::Type));
 
   // Sets the TotalBytes, BufferedBytes, & Duration values to be sent to host()
   // when set_host() is called.
diff --git a/media/base/pipeline_impl.cc b/media/base/pipeline_impl.cc
index 7c71e66..2b66661 100644
--- a/media/base/pipeline_impl.cc
+++ b/media/base/pipeline_impl.cc
@@ -1058,7 +1058,7 @@ bool PipelineImpl::InitializeAudioDecoder(
   DCHECK(IsPipelineOk());
 
   scoped_refptr<DemuxerStream> stream =
-      FindDemuxerStream(demuxer, DemuxerStream::AUDIO);
+      demuxer->GetStream(DemuxerStream::AUDIO);
 
   if (!stream)
     return false;
@@ -1088,7 +1088,7 @@ bool PipelineImpl::InitializeVideoDecoder(
   DCHECK(IsPipelineOk());
 
   scoped_refptr<DemuxerStream> stream =
-      FindDemuxerStream(demuxer, DemuxerStream::VIDEO);
+      demuxer->GetStream(DemuxerStream::VIDEO);
 
   if (!stream)
     return false;
@@ -1158,21 +1158,6 @@ bool PipelineImpl::InitializeVideoRenderer(
   return true;
 }
 
-scoped_refptr<DemuxerStream> PipelineImpl::FindDemuxerStream(
-    const scoped_refptr<Demuxer>& demuxer,
-    DemuxerStream::Type type) {
-  DCHECK(demuxer);
-
-  const int num_outputs = demuxer->GetNumberOfStreams();
-  for (int i = 0; i < num_outputs; ++i) {
-    std::string value;
-    if (demuxer->GetStream(i)->type() == type) {
-      return demuxer->GetStream(i);
-    }
-  }
-  return NULL;
-}
-
 void PipelineImpl::TearDownPipeline() {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
   DCHECK_NE(kStopped, state_);
diff --git a/media/base/pipeline_impl.h b/media/base/pipeline_impl.h
index e50322b..32de091 100644
--- a/media/base/pipeline_impl.h
+++ b/media/base/pipeline_impl.h
@@ -289,12 +289,6 @@ class PipelineImpl : public Pipeline, public FilterHost {
   bool InitializeAudioRenderer(const scoped_refptr<AudioDecoder>& decoder);
   bool InitializeVideoRenderer(const scoped_refptr<VideoDecoder>& decoder);
 
-  // Returns a reference to the DemuxerStream of the requested type if it
-  // exists, NULL otherwise.
-  scoped_refptr<DemuxerStream> FindDemuxerStream(
-      const scoped_refptr<Demuxer>& demuxer,
-      DemuxerStream::Type type);
-
   // Kicks off destroying filters. Called by StopTask() and ErrorChangedTask().
   // When we start to tear down the pipeline, we will consider two cases:
   // 1. when pipeline has not been initialized, we will transit to stopping
diff --git a/media/base/pipeline_impl_unittest.cc b/media/base/pipeline_impl_unittest.cc
index c5593f3..396b69c 100644
--- a/media/base/pipeline_impl_unittest.cc
+++ b/media/base/pipeline_impl_unittest.cc
@@ -67,6 +67,12 @@ class PipelineImplTest : public ::testing::Test {
                     &CallbackHelper::OnError),
         static_cast<PipelineStatusCallback*>(NULL));
     mocks_.reset(new MockFilterCollection());
+
+    // InitializeDemuxer adds overriding expectations for expected non-NULL
+    // streams.
+    DemuxerStream* null_pointer = NULL;
+    EXPECT_CALL(*mocks_->demuxer(), GetStream(_))
+        .WillRepeatedly(Return(null_pointer));
   }
 
   virtual ~PipelineImplTest() {
@@ -90,8 +96,6 @@ class PipelineImplTest : public ::testing::Test {
                          const base::TimeDelta& duration) {
     mocks_->demuxer()->SetTotalAndBufferedBytesAndDuration(
         kTotalBytes, kBufferedBytes, duration);
-    EXPECT_CALL(*mocks_->demuxer(), GetNumberOfStreams())
-        .WillRepeatedly(Return(streams->size()));
     EXPECT_CALL(*mocks_->demuxer(), SetPlaybackRate(0.0f));
     EXPECT_CALL(*mocks_->demuxer(), Seek(base::TimeDelta(), NotNull()))
         .WillOnce(Invoke(&RunFilterCallback));
@@ -101,7 +105,7 @@ class PipelineImplTest : public ::testing::Test {
     // Configure the demuxer to return the streams.
     for (size_t i = 0; i < streams->size(); ++i) {
       scoped_refptr<DemuxerStream> stream((*streams)[i]);
-      EXPECT_CALL(*mocks_->demuxer(), GetStream(i))
+      EXPECT_CALL(*mocks_->demuxer(), GetStream(stream->type()))
           .WillRepeatedly(Return(stream));
     }
   }
@@ -353,8 +357,6 @@ TEST_F(PipelineImplTest, URLNotFound) {
 TEST_F(PipelineImplTest, NoStreams) {
   // Manually set these expectations because SetPlaybackRate() is not called if
   // we cannot fully initialize the pipeline.
-  EXPECT_CALL(*mocks_->demuxer(), GetNumberOfStreams())
-      .WillRepeatedly(Return(0));
   EXPECT_CALL(*mocks_->demuxer(), Stop(NotNull()))
       .WillOnce(Invoke(&RunStopFilterCallback));
   // TODO(acolwell,fischman): see TODO in URLNotFound above.
diff --git a/media/filters/ffmpeg_demuxer.cc b/media/filters/ffmpeg_demuxer.cc
index 400ccf1..d0d4c34 100644
--- a/media/filters/ffmpeg_demuxer.cc
+++ b/media/filters/ffmpeg_demuxer.cc
@@ -337,14 +337,11 @@ void FFmpegDemuxer::Initialize(DataSource* data_source,
                         callback));
 }
 
-size_t FFmpegDemuxer::GetNumberOfStreams() {
-  return streams_.size();
-}
-
-scoped_refptr<DemuxerStream> FFmpegDemuxer::GetStream(int stream) {
-  DCHECK_GE(stream, 0);
-  DCHECK_LT(stream, static_cast<int>(streams_.size()));
-  return streams_[stream].get();
+scoped_refptr<DemuxerStream> FFmpegDemuxer::GetStream(
+    DemuxerStream::Type type) {
+  DCHECK_GE(type, 0);
+  DCHECK_LT(type, DemuxerStream::NUM_TYPES);
+  return streams_[type];
 }
 
 int FFmpegDemuxer::Read(int size, uint8* data) {
@@ -456,8 +453,10 @@ void FFmpegDemuxer::InitializeTask(DataSource* data_source,
   }
 
   // Create demuxer streams for all supported streams.
+  streams_.resize(DemuxerStream::NUM_TYPES);
   base::TimeDelta max_duration;
   const bool kDemuxerIsWebm = !strcmp("webm", format_context_->iformat->name);
+  bool no_supported_streams = true;
   for (size_t i = 0; i < format_context_->nb_streams; ++i) {
     AVCodecContext* codec_context = format_context_->streams[i]->codec;
     CodecType codec_type = codec_context->codec_type;
@@ -470,18 +469,19 @@ void FFmpegDemuxer::InitializeTask(DataSource* data_source,
         continue;
       }
 
-      FFmpegDemuxerStream* demuxer_stream
-          = new FFmpegDemuxerStream(this, stream);
-
-      DCHECK(demuxer_stream);
-      streams_.push_back(make_scoped_refptr(demuxer_stream));
-      packet_streams_.push_back(make_scoped_refptr(demuxer_stream));
-      max_duration = std::max(max_duration, demuxer_stream->duration());
+      scoped_refptr<FFmpegDemuxerStream> demuxer_stream(
+          new FFmpegDemuxerStream(this, stream));
+      if (!streams_[demuxer_stream->type()]) {
+        no_supported_streams = false;
+        streams_[demuxer_stream->type()] = demuxer_stream;
+        max_duration = std::max(max_duration, demuxer_stream->duration());
+      }
+      packet_streams_.push_back(demuxer_stream);
     } else {
       packet_streams_.push_back(NULL);
     }
   }
-  if (streams_.empty()) {
+  if (no_supported_streams) {
     callback->Run(DEMUXER_ERROR_NO_SUPPORTED_STREAMS);
     return;
   }
@@ -513,7 +513,8 @@ void FFmpegDemuxer::SeekTask(base::TimeDelta time, FilterCallback* callback) {
   // Tell streams to flush buffers due to seeking.
   StreamVector::iterator iter;
   for (iter = streams_.begin(); iter != streams_.end(); ++iter) {
-    (*iter)->FlushBuffers();
+    if (*iter)
+      (*iter)->FlushBuffers();
   }
 
   // Always seek to a timestamp less than or equal to the desired timestamp.
@@ -589,7 +590,8 @@ void FFmpegDemuxer::StopTask(FilterCallback* callback) {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
   StreamVector::iterator iter;
   for (iter = streams_.begin(); iter != streams_.end(); ++iter) {
-    (*iter)->Stop();
+    if (*iter)
+      (*iter)->Stop();
   }
   if (data_source_) {
     data_source_->Stop(callback);
@@ -621,7 +623,7 @@ bool FFmpegDemuxer::StreamsHavePendingReads() {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
   StreamVector::iterator iter;
   for (iter = streams_.begin(); iter != streams_.end(); ++iter) {
-    if ((*iter)->HasPendingReads()) {
+    if (*iter && (*iter)->HasPendingReads()) {
       return true;
     }
   }
@@ -632,6 +634,8 @@ void FFmpegDemuxer::StreamHasEnded() {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
   StreamVector::iterator iter;
   for (iter = streams_.begin(); iter != streams_.end(); ++iter) {
+    if (!*iter)
+      continue;
     AVPacket* packet = new AVPacket();
     memset(packet, 0, sizeof(*packet));
     (*iter)->EnqueuePacket(packet);
diff --git a/media/filters/ffmpeg_demuxer.h b/media/filters/ffmpeg_demuxer.h
index 3816a01..120b3b8 100644
--- a/media/filters/ffmpeg_demuxer.h
+++ b/media/filters/ffmpeg_demuxer.h
@@ -138,8 +138,7 @@ class FFmpegDemuxer : public Demuxer,
   virtual void set_host(FilterHost* filter_host);
 
   // Demuxer implementation.
-  virtual size_t GetNumberOfStreams();
-  virtual scoped_refptr<DemuxerStream> GetStream(int stream_id);
+  virtual scoped_refptr<DemuxerStream> GetStream(DemuxerStream::Type type);
 
   // FFmpegProtocol implementation.
   virtual int Read(int size, uint8* data);
@@ -201,8 +200,8 @@ class FFmpegDemuxer : public Demuxer,
   AVFormatContext* format_context_;
 
   // Two vector of streams:
-  //   - |streams_| is indexed for the Demuxer interface GetStream(), which only
-  //     contains supported streams and no NULL entries.
+  //   - |streams_| is indexed by type for the Demuxer interface GetStream(),
+  //     and contains NULLs for types which aren't present.
   //   - |packet_streams_| is indexed to mirror AVFormatContext when dealing
   //     with AVPackets returned from av_read_frame() and contain NULL entries
   //     representing unsupported streams where we throw away the data.
diff --git a/media/filters/ffmpeg_demuxer_unittest.cc b/media/filters/ffmpeg_demuxer_unittest.cc
index 96be01d..fdf2893 100644
--- a/media/filters/ffmpeg_demuxer_unittest.cc
+++ b/media/filters/ffmpeg_demuxer_unittest.cc
@@ -234,11 +234,9 @@ TEST_F(FFmpegDemuxerTest, Initialize_Successful) {
     InitializeDemuxer();
   }
 
-  // Verify that our demuxer streams were created from our AVStream structures.
-  EXPECT_EQ(DS_STREAM_MAX, static_cast<int>(demuxer_->GetNumberOfStreams()));
-
   // First stream should be video and support the FFmpegDemuxerStream interface.
-  scoped_refptr<DemuxerStream> stream = demuxer_->GetStream(DS_STREAM_VIDEO);
+  scoped_refptr<DemuxerStream> stream =
+      demuxer_->GetStream(DemuxerStream::VIDEO);
   AVStreamProvider* av_stream_provider = NULL;
   ASSERT_TRUE(stream);
   EXPECT_EQ(DemuxerStream::VIDEO, stream->type());
@@ -247,7 +245,7 @@ TEST_F(FFmpegDemuxerTest, Initialize_Successful) {
   EXPECT_EQ(&streams_[AV_STREAM_VIDEO], av_stream_provider->GetAVStream());
 
   // Other stream should be audio and support the FFmpegDemuxerStream interface.
-  stream = demuxer_->GetStream(DS_STREAM_AUDIO);
+  stream = demuxer_->GetStream(DemuxerStream::AUDIO);
   av_stream_provider = NULL;
   ASSERT_TRUE(stream);
   EXPECT_EQ(DemuxerStream::AUDIO, stream->type());
@@ -277,7 +275,8 @@ TEST_F(FFmpegDemuxerTest, Read_DiscardUninteresting) {
       .WillOnce(Return(AVERROR_IO));
 
   // Attempt a read from the audio stream and run the message loop until done.
-  scoped_refptr<DemuxerStream> audio = demuxer_->GetStream(DS_STREAM_AUDIO);
+  scoped_refptr<DemuxerStream> audio =
+      demuxer_->GetStream(DemuxerStream::AUDIO);
   scoped_refptr<DemuxerStreamReader> reader(new DemuxerStreamReader());
   reader->Read(audio);
   message_loop_.RunAllPending();
@@ -307,7 +306,8 @@ TEST_F(FFmpegDemuxerTest, Read_Audio) {
       .WillOnce(Return(0));
 
   // Attempt a read from the audio stream and run the message loop until done.
-  scoped_refptr<DemuxerStream> audio = demuxer_->GetStream(DS_STREAM_AUDIO);
+  scoped_refptr<DemuxerStream> audio =
+      demuxer_->GetStream(DemuxerStream::AUDIO);
   scoped_refptr<DemuxerStreamReader> reader(new DemuxerStreamReader());
   reader->Read(audio);
   message_loop_.RunAllPending();
@@ -338,7 +338,8 @@ TEST_F(FFmpegDemuxerTest, Read_Video) {
       .WillOnce(Return(0));
 
   // Attempt a read from the video stream and run the message loop until done.
-  scoped_refptr<DemuxerStream> video = demuxer_->GetStream(DS_STREAM_VIDEO);
+  scoped_refptr<DemuxerStream> video =
+      demuxer_->GetStream(DemuxerStream::VIDEO);
   scoped_refptr<DemuxerStreamReader> reader(new DemuxerStreamReader());
   reader->Read(video);
   message_loop_.RunAllPending();
@@ -366,7 +367,8 @@ TEST_F(FFmpegDemuxerTest, Read_EndOfStream) {
       .WillOnce(Return(AVERROR_IO));
 
   // We should now expect an end of stream buffer.
-  scoped_refptr<DemuxerStream> audio = demuxer_->GetStream(DS_STREAM_AUDIO);
+  scoped_refptr<DemuxerStream> audio =
+      demuxer_->GetStream(DemuxerStream::AUDIO);
   scoped_refptr<DemuxerStreamReader> reader(new DemuxerStreamReader());
   reader->Read(audio);
   message_loop_.RunAllPending();
@@ -389,8 +391,10 @@ TEST_F(FFmpegDemuxerTest, Seek) {
   }
 
   // Get our streams.
-  scoped_refptr<DemuxerStream> video = demuxer_->GetStream(DS_STREAM_VIDEO);
-  scoped_refptr<DemuxerStream> audio = demuxer_->GetStream(DS_STREAM_AUDIO);
+  scoped_refptr<DemuxerStream> video =
+      demuxer_->GetStream(DemuxerStream::VIDEO);
+  scoped_refptr<DemuxerStream> audio =
+      demuxer_->GetStream(DemuxerStream::AUDIO);
   ASSERT_TRUE(video);
   ASSERT_TRUE(audio);
 
@@ -552,7 +556,8 @@ TEST_F(FFmpegDemuxerTest, Stop) {
       new StrictMock<MockReadCallback>());
 
   // Get our stream.
-  scoped_refptr<DemuxerStream> audio = demuxer_->GetStream(DS_STREAM_AUDIO);
+  scoped_refptr<DemuxerStream> audio =
+      demuxer_->GetStream(DemuxerStream::AUDIO);
   ASSERT_TRUE(audio);
 
   // Stop the demuxer, overriding the default expectation to assert that
@@ -607,7 +612,8 @@ TEST_F(FFmpegDemuxerTest, DisableAudioStream) {
       .WillOnce(Return(AVERROR_IO));
 
   // Get our streams.
-  scoped_refptr<DemuxerStream> video = demuxer_->GetStream(DS_STREAM_VIDEO);
+  scoped_refptr<DemuxerStream> video =
+      demuxer_->GetStream(DemuxerStream::VIDEO);
   ASSERT_TRUE(video);
 
   // Attempt a read from the video stream and run the message loop until done.