Refactor code so PipelineImpl doesn't rely on FilterType anymore.

- Replaced SupportsSetMessageLoop() & GetThreadName() with new methods in MediaFilter.
- Reworked pipeline initialization logic so it keeps track of type references instead of downcasting based on filter_type().

Patch by acolwell@chromium.org:
http://codereview.chromium.org/4397003/show

BUG=61823
TEST=media_unittests

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

4 files changed, 115 insertions, 121 deletions

diff --git a/media/base/filters.cc b/media/base/filters.cc
index fb8f66a..0773b89 100644
--- a/media/base/filters.cc
+++ b/media/base/filters.cc
@@ -25,6 +25,14 @@ FilterHost* MediaFilter::host() {
   return host_;
 }
 
+bool MediaFilter::requires_message_loop() const {
+  return false;
+}
+
+const char* MediaFilter::message_loop_name() const {
+  return "FilterThread";
+}
+
 void MediaFilter::set_message_loop(MessageLoop* message_loop) {
   DCHECK(message_loop);
   DCHECK(!message_loop_);
@@ -93,6 +101,14 @@ FilterType Demuxer::filter_type() const {
   return static_filter_type();
 }
 
+bool Demuxer::requires_message_loop() const {
+  return true;
+}
+
+const char* Demuxer::message_loop_name() const {
+  return "DemuxerThread";
+}
+
 FilterType AudioDecoder::filter_type() const {
   return static_filter_type();
 }
@@ -101,6 +117,14 @@ const char* AudioDecoder::major_mime_type() const {
   return mime_type::kMajorTypeAudio;
 }
 
+bool AudioDecoder::requires_message_loop() const {
+  return true;
+}
+
+const char* AudioDecoder::message_loop_name() const {
+  return "AudioDecoderThread";
+}
+
 FilterType AudioRenderer::filter_type() const {
   return static_filter_type();
 }
@@ -117,6 +141,14 @@ const char* VideoDecoder::major_mime_type() const {
   return mime_type::kMajorTypeVideo;
 }
 
+bool VideoDecoder::requires_message_loop() const {
+  return true;
+}
+
+const char* VideoDecoder::message_loop_name() const {
+  return "VideoDecoderThread";
+}
+
 FilterType VideoRenderer::filter_type() const {
   return static_filter_type();
 }
diff --git a/media/base/filters.h b/media/base/filters.h
index f386111..d12e3e8 100644
--- a/media/base/filters.h
+++ b/media/base/filters.h
@@ -78,6 +78,12 @@ class MediaFilter : public base::RefCountedThreadSafe<MediaFilter> {
 
   virtual FilterHost* host();
 
+  // Indicates whether this filter requires a message loop to operate.
+  virtual bool requires_message_loop() const;
+
+  // The name to associate with this filter's message loop.
+  virtual const char* message_loop_name() const;
+
   // Sets the private member |message_loop_|, which is used by filters for
   // processing asynchronous tasks and maintaining synchronized access to
   // internal data members.  The message loop should be running and exceed the
@@ -171,6 +177,9 @@ class Demuxer : public MediaFilter {
   static FilterType static_filter_type() { return FILTER_DEMUXER; }
   virtual FilterType filter_type() const;
 
+  virtual bool requires_message_loop() const;
+  virtual const char* message_loop_name() const;
+
   // Initialize a Demuxer with the given DataSource, executing the callback upon
   // completion.
   virtual void Initialize(DataSource* data_source,
@@ -228,6 +237,9 @@ class VideoDecoder : public MediaFilter {
   virtual FilterType filter_type() const;
 
   virtual const char* major_mime_type() const;
+  virtual bool requires_message_loop() const;
+  virtual const char* message_loop_name() const;
+
 
   // Initialize a VideoDecoder with the given DemuxerStream, executing the
   // callback upon completion.
@@ -273,6 +285,8 @@ class AudioDecoder : public MediaFilter {
   virtual FilterType filter_type() const;
 
   virtual const char* major_mime_type() const;
+  virtual bool requires_message_loop() const;
+  virtual const char* message_loop_name() const;
 
   // Initialize a AudioDecoder with the given DemuxerStream, executing the
   // callback upon completion.
diff --git a/media/base/pipeline_impl.cc b/media/base/pipeline_impl.cc
index 2493f3b..516b230 100644
--- a/media/base/pipeline_impl.cc
+++ b/media/base/pipeline_impl.cc
@@ -14,49 +14,6 @@
 
 namespace media {
 
-namespace {
-
-// Small helper function to help us transition over to injected message loops.
-//
-// TODO(scherkus): have every filter support injected message loops.
-bool SupportsSetMessageLoop(FilterType type) {
-  switch (type) {
-    case FILTER_DEMUXER:
-    case FILTER_AUDIO_DECODER:
-    case FILTER_VIDEO_DECODER:
-      return true;
-
-    case FILTER_DATA_SOURCE:
-    case FILTER_AUDIO_RENDERER:
-    case FILTER_VIDEO_RENDERER:
-      return false;
-
-    // Skipping default case so compiler will warn on a missed enumeration.
-  }
-
-  NOTREACHED() << "Unexpected filter type " << type;
-  return false;
-}
-
-// Small helper function to help us name filter threads for debugging.
-//
-// TODO(scherkus): figure out a cleaner way to derive the filter thread name.
-const char* GetThreadName(FilterType type) {
-  DCHECK(SupportsSetMessageLoop(type));
-  switch (type) {
-    case FILTER_DEMUXER:
-      return "DemuxerThread";
-    case FILTER_AUDIO_DECODER:
-      return "AudioDecoderThread";
-    case FILTER_VIDEO_DECODER:
-      return "VideoDecoderThread";
-    default:
-      return "FilterThread";
-  }
-}
-
-}  // namespace
-
 PipelineImpl::PipelineImpl(MessageLoop* message_loop)
     : message_loop_(message_loop),
       clock_(&base::Time::Now),
@@ -614,6 +571,7 @@ void PipelineImpl::InitializeTask() {
   // Just created, create data source.
   if (state_ == kCreated) {
     state_ = kInitDataSource;
+    pipeline_init_state_.reset(new PipelineInitState());
     InitializeDataSource();
     return;
   }
@@ -621,7 +579,7 @@ void PipelineImpl::InitializeTask() {
   // Data source created, create demuxer.
   if (state_ == kInitDataSource) {
     state_ = kInitDemuxer;
-    InitializeDemuxer();
+    InitializeDemuxer(pipeline_init_state_->data_source_);
     return;
   }
 
@@ -629,7 +587,7 @@ void PipelineImpl::InitializeTask() {
   if (state_ == kInitDemuxer) {
     state_ = kInitAudioDecoder;
     // If this method returns false, then there's no audio stream.
-    if (InitializeAudioDecoder())
+    if (InitializeAudioDecoder(pipeline_init_state_->demuxer_))
       return;
   }
 
@@ -637,7 +595,7 @@ void PipelineImpl::InitializeTask() {
   if (state_ == kInitAudioDecoder) {
     state_ = kInitAudioRenderer;
     // Returns false if there's no audio stream.
-    if (InitializeAudioRenderer()) {
+    if (InitializeAudioRenderer(pipeline_init_state_->audio_decoder_)) {
       InsertRenderedMimeType(mime_type::kMajorTypeAudio);
       return;
     }
@@ -647,14 +605,14 @@ void PipelineImpl::InitializeTask() {
   if (state_ == kInitAudioRenderer) {
     // Then perform the stage of initialization, i.e. initialize video decoder.
     state_ = kInitVideoDecoder;
-    if (InitializeVideoDecoder())
+    if (InitializeVideoDecoder(pipeline_init_state_->demuxer_))
       return;
   }
 
   // Assuming video decoder was created, create video renderer.
   if (state_ == kInitVideoDecoder) {
     state_ = kInitVideoRenderer;
-    if (InitializeVideoRenderer()) {
+    if (InitializeVideoRenderer(pipeline_init_state_->video_decoder_)) {
       InsertRenderedMimeType(mime_type::kMajorTypeVideo);
       return;
     }
@@ -669,6 +627,9 @@ void PipelineImpl::InitializeTask() {
     // Clear the collection of filters.
     filter_collection_->Clear();
 
+    // Clear init state since we're done initializing.
+    pipeline_init_state_.reset();
+
     // Initialization was successful, we are now considered paused, so it's safe
     // to set the initial playback rate and volume.
     PlaybackRateChangedTask(GetPlaybackRate());
@@ -752,10 +713,8 @@ void PipelineImpl::PlaybackRateChangedTask(float playback_rate) {
 void PipelineImpl::VolumeChangedTask(float volume) {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
 
-  scoped_refptr<AudioRenderer> audio_renderer;
-  GetInitializedFilter(&audio_renderer);
-  if (audio_renderer) {
-    audio_renderer->SetVolume(volume);
+  if (audio_renderer_) {
+    audio_renderer_->SetVolume(volume);
   }
 }
 
@@ -808,16 +767,11 @@ void PipelineImpl::NotifyEndedTask() {
     return;
   }
 
-  // Grab the renderers, if they exist.
-  scoped_refptr<AudioRenderer> audio_renderer;
-  scoped_refptr<VideoRenderer> video_renderer;
-  GetInitializedFilter(&audio_renderer);
-  GetInitializedFilter(&video_renderer);
-  DCHECK(audio_renderer || video_renderer);
+  DCHECK(audio_renderer_ || video_renderer_);
 
   // Make sure every extant renderer has ended.
-  if ((audio_renderer && !audio_renderer->HasEnded() && !audio_disabled_) ||
-      (video_renderer && !video_renderer->HasEnded())) {
+  if ((audio_renderer_ && !audio_renderer_->HasEnded() && !audio_disabled_) ||
+      (video_renderer_ && !video_renderer_->HasEnded())) {
     return;
   }
 
@@ -946,11 +900,14 @@ void PipelineImpl::FinishDestroyingFiltersTask() {
     (*iter)->Stop();
   }
 
+  // Clear renderer references.
+  audio_renderer_ = NULL;
+  video_renderer_ = NULL;
+
   // Reset the pipeline, which will decrement a reference to this object.
   // We will get destroyed as soon as the remaining tasks finish executing.
   // To be safe, we'll set our pipeline reference to NULL.
   filters_.clear();
-  filter_types_.clear();
   STLDeleteElements(&filter_threads_);
 
   stop_pending_ = false;
@@ -980,9 +937,9 @@ void PipelineImpl::PrepareFilter(scoped_refptr<MediaFilter> filter) {
   DCHECK(IsPipelineOk());
 
   // Create a dedicated thread for this filter if applicable.
-  if (SupportsSetMessageLoop(filter->filter_type())) {
+  if (filter->requires_message_loop()) {
     scoped_ptr<base::Thread> thread(
-        new base::Thread(GetThreadName(filter->filter_type())));
+        new base::Thread(filter->message_loop_name()));
     if (!thread.get() || !thread->Start()) {
       NOTREACHED() << "Could not start filter thread";
       SetError(PIPELINE_ERROR_INITIALIZATION_FAILED);
@@ -995,11 +952,8 @@ void PipelineImpl::PrepareFilter(scoped_refptr<MediaFilter> filter) {
 
   // Register ourselves as the filter's host.
   DCHECK(IsPipelineOk());
-  DCHECK(filter_types_.find(filter->filter_type()) == filter_types_.end())
-      << "Filter type " << filter->filter_type() << " already exists";
   filter->set_host(this);
   filters_.push_back(make_scoped_refptr(filter.get()));
-  filter_types_[filter->filter_type()] = filter.get();
 }
 
 void PipelineImpl::InitializeDataSource() {
@@ -1019,18 +973,18 @@ void PipelineImpl::InitializeDataSource() {
   }
 
   PrepareFilter(data_source);
+  pipeline_init_state_->data_source_ = data_source;
   data_source->Initialize(
       url_, NewCallback(this, &PipelineImpl::OnFilterInitialize));
 }
 
-void PipelineImpl::InitializeDemuxer() {
+void PipelineImpl::InitializeDemuxer(
+    const scoped_refptr<DataSource>& data_source) {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
   DCHECK(IsPipelineOk());
 
-  scoped_refptr<DataSource> data_source;
   scoped_refptr<Demuxer> demuxer;
 
-  GetInitializedFilter(&data_source);
   CHECK(data_source);
 
   filter_collection_->SelectFilter(&demuxer);
@@ -1040,16 +994,18 @@ void PipelineImpl::InitializeDemuxer() {
   }
 
   PrepareFilter(demuxer);
+  pipeline_init_state_->demuxer_ = demuxer;
   demuxer->Initialize(data_source,
                       NewCallback(this, &PipelineImpl::OnFilterInitialize));
 }
 
-bool PipelineImpl::InitializeAudioDecoder() {
+bool PipelineImpl::InitializeAudioDecoder(
+    const scoped_refptr<Demuxer>& demuxer) {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
   DCHECK(IsPipelineOk());
 
   scoped_refptr<DemuxerStream> stream =
-      FindDemuxerStream(mime_type::kMajorTypeAudio);
+      FindDemuxerStream(demuxer, mime_type::kMajorTypeAudio);
 
   if (stream) {
     scoped_refptr<AudioDecoder> audio_decoder;
@@ -1057,6 +1013,7 @@ bool PipelineImpl::InitializeAudioDecoder() {
 
     if (audio_decoder) {
       PrepareFilter(audio_decoder);
+      pipeline_init_state_->audio_decoder_ = audio_decoder;
       audio_decoder->Initialize(
           stream,
           NewCallback(this, &PipelineImpl::OnFilterInitialize));
@@ -1069,12 +1026,13 @@ bool PipelineImpl::InitializeAudioDecoder() {
   return false;
 }
 
-bool PipelineImpl::InitializeVideoDecoder() {
+bool PipelineImpl::InitializeVideoDecoder(
+    const scoped_refptr<Demuxer>& demuxer) {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
   DCHECK(IsPipelineOk());
 
   scoped_refptr<DemuxerStream> stream =
-      FindDemuxerStream(mime_type::kMajorTypeVideo);
+      FindDemuxerStream(demuxer, mime_type::kMajorTypeVideo);
 
   if (stream) {
     scoped_refptr<VideoDecoder> video_decoder;
@@ -1082,6 +1040,7 @@ bool PipelineImpl::InitializeVideoDecoder() {
 
     if (video_decoder) {
       PrepareFilter(video_decoder);
+      pipeline_init_state_->video_decoder_ = video_decoder;
       video_decoder->Initialize(
           stream,
           NewCallback(this, &PipelineImpl::OnFilterInitialize));
@@ -1093,20 +1052,17 @@ bool PipelineImpl::InitializeVideoDecoder() {
   return false;
 }
 
-bool PipelineImpl::InitializeAudioRenderer() {
+bool PipelineImpl::InitializeAudioRenderer(
+    const scoped_refptr<AudioDecoder>& decoder) {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
   DCHECK(IsPipelineOk());
 
-  scoped_refptr<AudioDecoder> decoder;
-  GetInitializedFilter(&decoder);
-
   if (decoder) {
-    scoped_refptr<AudioRenderer> audio_renderer;
-    filter_collection_->SelectFilter(&audio_renderer);
+    filter_collection_->SelectFilter(&audio_renderer_);
 
-    if (audio_renderer) {
-      PrepareFilter(audio_renderer);
-      audio_renderer->Initialize(
+    if (audio_renderer_) {
+      PrepareFilter(audio_renderer_);
+      audio_renderer_->Initialize(
           decoder, NewCallback(this, &PipelineImpl::OnFilterInitialize));
       return true;
     } else {
@@ -1116,20 +1072,17 @@ bool PipelineImpl::InitializeAudioRenderer() {
   return false;
 }
 
-bool PipelineImpl::InitializeVideoRenderer() {
+bool PipelineImpl::InitializeVideoRenderer(
+    const scoped_refptr<VideoDecoder>& decoder) {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
   DCHECK(IsPipelineOk());
 
-  scoped_refptr<VideoDecoder> decoder;
-  GetInitializedFilter(&decoder);
-
   if (decoder) {
-    scoped_refptr<VideoRenderer> video_renderer;
-    filter_collection_->SelectFilter(&video_renderer);
+    filter_collection_->SelectFilter(&video_renderer_);
 
-    if (video_renderer) {
-      PrepareFilter(video_renderer);
-      video_renderer->Initialize(
+    if (video_renderer_) {
+      PrepareFilter(video_renderer_);
+      video_renderer_->Initialize(
           decoder, NewCallback(this, &PipelineImpl::OnFilterInitialize));
       return true;
     } else {
@@ -1140,9 +1093,8 @@ bool PipelineImpl::InitializeVideoRenderer() {
 }
 
 scoped_refptr<DemuxerStream> PipelineImpl::FindDemuxerStream(
+    const scoped_refptr<Demuxer>& demuxer,
     std::string major_mime_type) {
-  scoped_refptr<Demuxer> demuxer;
-  GetInitializedFilter(&demuxer);
   DCHECK(demuxer);
 
   const int num_outputs = demuxer->GetNumberOfStreams();
@@ -1157,20 +1109,6 @@ scoped_refptr<DemuxerStream> PipelineImpl::FindDemuxerStream(
   return NULL;
 }
 
-template <class Filter>
-void PipelineImpl::GetInitializedFilter(
-    scoped_refptr<Filter>* filter_out) const {
-  DCHECK_EQ(MessageLoop::current(), message_loop_);
-
-  FilterTypeMap::const_iterator ft =
-      filter_types_.find(Filter::static_filter_type());
-  if (ft == filter_types_.end()) {
-    *filter_out = NULL;
-  } else {
-    *filter_out = reinterpret_cast<Filter*>(ft->second.get());
-  }
-}
-
 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 c03f8c5..6dff897 100644
--- a/media/base/pipeline_impl.h
+++ b/media/base/pipeline_impl.h
@@ -249,29 +249,25 @@ class PipelineImpl : public Pipeline, public FilterHost {
   // MediaFilter object from MediaFilterCollection and initialize it
   // asynchronously.
   void InitializeDataSource();
-  void InitializeDemuxer();
+  void InitializeDemuxer(const scoped_refptr<DataSource>& data_source);
 
   // Returns true if the asynchronous action of creating decoder has started.
   // Returns false if this method did nothing because the corresponding
   // audio/video stream does not exist.
-  bool InitializeAudioDecoder();
-  bool InitializeVideoDecoder();
+  bool InitializeAudioDecoder(const scoped_refptr<Demuxer>& demuxer);
+  bool InitializeVideoDecoder(const scoped_refptr<Demuxer>& demuxer);
 
   // Initializes a renderer and connects it with decoder. Returns true if the
   // asynchronous action of creating renderer has started. Returns
   // false if this method did nothing because the corresponding audio/video
   // stream does not exist.
-  bool InitializeAudioRenderer();
-  bool InitializeVideoRenderer();
+  bool InitializeAudioRenderer(const scoped_refptr<AudioDecoder>& decoder);
+  bool InitializeVideoRenderer(const scoped_refptr<VideoDecoder>& decoder);
 
   // Helper to find the demuxer of |major_mime_type| from Demuxer.
-  scoped_refptr<DemuxerStream> FindDemuxerStream(std::string major_mime_type);
-
-  // Examine the list of initialized filters to find one that matches the
-  // specified filter type. If one exists, the |filter_out| will contain
-  // the filter, |*filter_out| will be NULL.
-  template <class Filter>
-  void GetInitializedFilter(scoped_refptr<Filter>* filter_out) const;
+  scoped_refptr<DemuxerStream> FindDemuxerStream(
+      const scoped_refptr<Demuxer>& demuxer,
+      std::string major_mime_type);
 
   // Kicks off destroying filters. Called by StopTask() and ErrorChangedTask().
   // When we start to tear down the pipeline, we will consider two cases:
@@ -407,13 +403,27 @@ class PipelineImpl : public Pipeline, public FilterHost {
   typedef std::vector<scoped_refptr<MediaFilter> > FilterVector;
   FilterVector filters_;
 
-  typedef std::map<FilterType, scoped_refptr<MediaFilter> > FilterTypeMap;
-  FilterTypeMap filter_types_;
+  // Renderer references used for setting the volume and determining
+  // when playback has finished.
+  scoped_refptr<AudioRenderer> audio_renderer_;
+  scoped_refptr<VideoRenderer> video_renderer_;
 
   // Vector of threads owned by the pipeline and being used by filters.
   typedef std::vector<base::Thread*> FilterThreadVector;
   FilterThreadVector filter_threads_;
 
+  // Helper class that stores filter references during pipeline
+  // initialization.
+  class PipelineInitState {
+   public:
+    scoped_refptr<DataSource> data_source_;
+    scoped_refptr<Demuxer> demuxer_;
+    scoped_refptr<AudioDecoder> audio_decoder_;
+    scoped_refptr<VideoDecoder> video_decoder_;
+  };
+
+  scoped_ptr<PipelineInitState> pipeline_init_state_;
+
   FRIEND_TEST_ALL_PREFIXES(PipelineImplTest, GetBufferedTime);
 
   DISALLOW_COPY_AND_ASSIGN(PipelineImpl);