Merged FilterHostImpl into PipelineInternal as it was no longer needed and added complexity.

PipelineInternal now directly implements FilterHost and uses an std::map to safely cast from MediaFilter to specialized filter subclasses.

PipelineImpl also provides the same set of functions as FilterHost to allow PiplineInternal to acquire the lock and safely update data.

BUG=17107
TEST=media_unittests should still pass

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

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

5 files changed, 110 insertions, 238 deletions

diff --git a/media/base/filter_host_impl.cc b/media/base/filter_host_impl.cc
deleted file mode 100644
index 93e3ca7..0000000
--- a/media/base/filter_host_impl.cc
+++ /dev/null
@@ -1,49 +0,0 @@
-// Copyright (c) 2008-2009 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.
-
-#include "media/base/filter_host_impl.h"
-
-namespace media {
-
-void FilterHostImpl::SetError(PipelineError error) {
-  pipeline_internal_->SetError(error);
-}
-
-base::TimeDelta FilterHostImpl::GetTime() const {
-  return pipeline()->GetCurrentTime();
-}
-
-void FilterHostImpl::SetTime(base::TimeDelta time) {
-  pipeline_internal_->SetTime(time);
-}
-
-void FilterHostImpl::SetDuration(base::TimeDelta duration) {
-  pipeline()->SetDuration(duration);
-}
-
-void FilterHostImpl::SetBufferedTime(base::TimeDelta buffered_time) {
-  pipeline()->SetBufferedTime(buffered_time);
-}
-
-void FilterHostImpl::SetTotalBytes(int64 total_bytes) {
-  pipeline()->SetTotalBytes(total_bytes);
-}
-
-void FilterHostImpl::SetBufferedBytes(int64 buffered_bytes) {
-  pipeline()->SetBufferedBytes(buffered_bytes);
-}
-
-void FilterHostImpl::SetVideoSize(size_t width, size_t height) {
-  pipeline()->SetVideoSize(width, height);
-}
-
-void FilterHostImpl::Stop() {
-  if (!stopped_) {
-    filter_->Stop();
-    AutoLock auto_lock(time_update_lock_);
-    stopped_ = true;
-  }
-}
-
-}  // namespace media
diff --git a/media/base/filter_host_impl.h b/media/base/filter_host_impl.h
deleted file mode 100644
index a619654..0000000
--- a/media/base/filter_host_impl.h
+++ /dev/null
@@ -1,84 +0,0 @@
-// Copyright (c) 2008-2009 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.
-
-// Implementation of FilterHost.
-
-#ifndef MEDIA_BASE_FILTER_HOST_IMPL_H_
-#define MEDIA_BASE_FILTER_HOST_IMPL_H_
-
-#include "base/task.h"
-#include "media/base/filter_host.h"
-#include "media/base/pipeline_impl.h"
-
-namespace media {
-
-class FilterHostImpl : public FilterHost {
- public:
-  // FilterHost interface.
-  virtual void SetError(PipelineError error);
-  virtual base::TimeDelta GetTime() const;
-  virtual void SetTime(base::TimeDelta time);
-  virtual void SetDuration(base::TimeDelta duration);
-  virtual void SetBufferedTime(base::TimeDelta buffered_time);
-  virtual void SetTotalBytes(int64 total_bytes);
-  virtual void SetBufferedBytes(int64 buffered_bytes);
-  virtual void SetVideoSize(size_t width, size_t height);
-
-  // These methods are public, but are intended for use by the
-  // PipelineInternal class only.
-
-  // Creates a FilterHostImpl object and populates the |filter_type_| member
-  // by calling the Filter class's static filter_type() method.  This ensures
-  // that the GetFilter method can safely cast the filter interface from the
-  // MediaFilter base class interface to the specific Filter interface.
-  template <class Filter>
-  FilterHostImpl(PipelineInternal* pipeline_internal, Filter* filter)
-      : pipeline_internal_(pipeline_internal),
-        filter_type_(Filter::filter_type()),
-        filter_(filter),
-        stopped_(false) {
-  }
-  ~FilterHostImpl() {}
-
-  // If this FilterHost contains a filter of the specified Filter class, then
-  // this method returns a pointer to the interface, otherwise it returns NULL
-  // in |*filter_out|.
-  template <class Filter>
-  void GetFilter(scoped_refptr<Filter>* filter_out) const {
-    *filter_out = (Filter::filter_type() == filter_type_) ?
-                      reinterpret_cast<Filter*>(media_filter()) : NULL;
-  }
-
-  // Stops the filter.
-  void Stop();
-
-  // Used by the PipelineInternal to call Seek() and SetRate() methods on
-  // filters.
-  MediaFilter* media_filter() const { return filter_; }
-
- private:
-  // Useful method for getting the pipeline.
-  PipelineImpl* pipeline() const { return pipeline_internal_->pipeline(); }
-
-  // PipelineInternal that owns this FilterHostImpl.
-  PipelineInternal* const pipeline_internal_;
-
-  // The FilterType of the filter this host contains.
-  FilterType const filter_type_;
-
-  // A pointer to the filter's MediaFilter base interface.
-  scoped_refptr<MediaFilter> filter_;
-
-  // Critical section used for scheduled time update callbacks.
-  Lock time_update_lock_;
-
-  // Used to avoid calling Filter's Stop() method multiple times.
-  bool stopped_;
-
-  DISALLOW_COPY_AND_ASSIGN(FilterHostImpl);
-};
-
-}  // namespace media
-
-#endif  // MEDIA_BASE_FILTER_HOST_IMPL_H_
diff --git a/media/base/pipeline_impl.cc b/media/base/pipeline_impl.cc
index 1da8366..9c2fb10 100644
--- a/media/base/pipeline_impl.cc
+++ b/media/base/pipeline_impl.cc
@@ -8,7 +8,6 @@
 #include "base/compiler_specific.h"
 #include "base/condition_variable.h"
 #include "base/stl_util-inl.h"
-#include "media/base/filter_host_impl.h"
 #include "media/base/media_format.h"
 #include "media/base/pipeline_impl.h"
 
@@ -228,6 +227,20 @@ bool PipelineImpl::IsPipelineOk() const {
   return pipeline_internal_ && PIPELINE_OK == error_;
 }
 
+void PipelineImpl::SetError(PipelineError error) {
+  AutoLock auto_lock(lock_);
+  error_ = error;
+}
+
+base::TimeDelta PipelineImpl::GetTime() const {
+  return GetCurrentTime();
+}
+
+void PipelineImpl::SetTime(base::TimeDelta time) {
+  AutoLock auto_lock(lock_);
+  time_ = time;
+}
+
 void PipelineImpl::SetDuration(base::TimeDelta duration) {
   AutoLock auto_lock(lock_);
   duration_ = duration;
@@ -254,24 +267,6 @@ void PipelineImpl::SetVideoSize(size_t width, size_t height) {
   video_height_ = height;
 }
 
-void PipelineImpl::SetTime(base::TimeDelta time) {
-  AutoLock auto_lock(lock_);
-  time_ = time;
-}
-
-bool PipelineImpl::InternalSetError(PipelineError error) {
-  // Don't want callers to set an error of "OK".  STOPPING is a special value
-  // that should only be used internally by the StopTask() method.
-  DCHECK(PIPELINE_OK != error && PIPELINE_STOPPING != error);
-  AutoLock auto_lock(lock_);
-  bool changed_error = false;
-  if (PIPELINE_OK == error_) {
-    error_ = error;
-    changed_error = true;
-  }
-  return changed_error;
-}
-
 void PipelineImpl::InsertRenderedMimeType(const std::string& major_mime_type) {
   AutoLock auto_lock(lock_);
   rendered_mime_types_.insert(major_mime_type);
@@ -328,17 +323,45 @@ void PipelineInternal::VolumeChanged(float volume) {
       NewRunnableMethod(this, &PipelineInternal::VolumeChangedTask, volume));
 }
 
-// Called from any thread.  Updates the pipeline time.
+// Called from any thread.
+void PipelineInternal::SetError(PipelineError error) {
+  message_loop_->PostTask(FROM_HERE,
+      NewRunnableMethod(this, &PipelineInternal::ErrorTask, error));
+}
+
+// Called from any thread.
+base::TimeDelta PipelineInternal::GetTime() const {
+  return pipeline_->GetCurrentTime();
+}
+
+// Called from any thread.
 void PipelineInternal::SetTime(base::TimeDelta time) {
-  // TODO(scherkus): why not post a task?
   pipeline_->SetTime(time);
 }
 
-// Called from any thread.  Sets the pipeline |error_| member and destroys all
-// filters.
-void PipelineInternal::SetError(PipelineError error) {
-  message_loop_->PostTask(FROM_HERE,
-      NewRunnableMethod(this, &PipelineInternal::ErrorTask, error));
+// Called from any thread.
+void PipelineInternal::SetDuration(base::TimeDelta duration) {
+  pipeline_->SetDuration(duration);
+}
+
+// Called from any thread.
+void PipelineInternal::SetBufferedTime(base::TimeDelta buffered_time) {
+  pipeline_->SetBufferedTime(buffered_time);
+}
+
+// Called from any thread.
+void PipelineInternal::SetTotalBytes(int64 total_bytes) {
+  pipeline_->SetTotalBytes(total_bytes);
+}
+
+// Called from any thread.
+void PipelineInternal::SetBufferedBytes(int64 buffered_bytes) {
+  pipeline_->SetBufferedBytes(buffered_bytes);
+}
+
+// Called from any thread.
+void PipelineInternal::SetVideoSize(size_t width, size_t height) {
+  pipeline_->SetVideoSize(width, height);
 }
 
 // Called from any thread.
@@ -506,7 +529,7 @@ void PipelineInternal::ErrorTask(PipelineError error) {
   }
 
   // Update our error code first in case we execute the start callback.
-  pipeline_->error_ = error;
+  pipeline_->SetError(error);
 
   // Notify the client that starting did not complete, if necessary.
   if (IsPipelineInitializing() && start_callback_.get()) {
@@ -524,11 +547,10 @@ void PipelineInternal::ErrorTask(PipelineError error) {
 
 void PipelineInternal::PlaybackRateChangedTask(float playback_rate) {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
-
-  for (FilterHostVector::iterator iter = filter_hosts_.begin();
-       iter != filter_hosts_.end();
+  for (FilterVector::iterator iter = filters_.begin();
+       iter != filters_.end();
        ++iter) {
-    (*iter)->media_filter()->SetPlaybackRate(playback_rate);
+    (*iter)->SetPlaybackRate(playback_rate);
   }
 }
 
@@ -552,11 +574,10 @@ void PipelineInternal::SeekTask(base::TimeDelta time,
     return;
   }
 
-  for (FilterHostVector::iterator iter = filter_hosts_.begin();
-       iter != filter_hosts_.end();
+  for (FilterVector::iterator iter = filters_.begin();
+       iter != filters_.end();
        ++iter) {
-    (*iter)->media_filter()->Seek(time,
-        NewCallback(this, &PipelineInternal::OnFilterSeek));
+    (*iter)->Seek(time, NewCallback(this, &PipelineInternal::OnFilterSeek));
   }
 
   // TODO(hclam): we should set the time when the above seek operations were all
@@ -600,11 +621,13 @@ void PipelineInternal::CreateFilter(FilterFactory* filter_factory,
     filter_threads_.push_back(thread.release());
   }
 
-  // Create the filter's host.
+  // Register ourselves as the filter's host.
   DCHECK(IsPipelineOk());
-  scoped_ptr<FilterHostImpl> host(new FilterHostImpl(this, filter.get()));
-  filter->set_host(host.get());
-  filter_hosts_.push_back(host.release());
+  DCHECK(filter_types_.find(Filter::filter_type()) == filter_types_.end())
+      << "Filter type " << Filter::filter_type() << " already exists";
+  filter->set_host(this);
+  filters_.push_back(filter.get());
+  filter_types_[Filter::filter_type()] = filter.get();
 
   // Now initialize the filter.
   filter->Initialize(source,
@@ -675,18 +698,18 @@ template <class Filter>
 void PipelineInternal::GetFilter(scoped_refptr<Filter>* filter_out) const {
   DCHECK_EQ(MessageLoop::current(), message_loop_);
 
-  *filter_out = NULL;
-  for (FilterHostVector::const_iterator iter = filter_hosts_.begin();
-       iter != filter_hosts_.end() && NULL == *filter_out;
-       iter++) {
-    (*iter)->GetFilter(filter_out);
+  FilterTypeMap::const_iterator ft = filter_types_.find(Filter::filter_type());
+  if (ft == filter_types_.end()) {
+    *filter_out = NULL;
+  } else {
+    *filter_out = reinterpret_cast<Filter*>(ft->second.get());
   }
 }
 
 void PipelineInternal::DestroyFilters() {
   // Stop every filter.
-  for (FilterHostVector::iterator iter = filter_hosts_.begin();
-       iter != filter_hosts_.end();
+  for (FilterVector::iterator iter = filters_.begin();
+       iter != filters_.end();
        ++iter) {
     (*iter)->Stop();
   }
@@ -728,7 +751,8 @@ void PipelineInternal::DestroyFilters() {
   // 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.
-  STLDeleteElements(&filter_hosts_);
+  filters_.clear();
+  filter_types_.clear();
   STLDeleteElements(&filter_threads_);
 }
 
diff --git a/media/base/pipeline_impl.h b/media/base/pipeline_impl.h
index 3efcbe4..b72e368 100644
--- a/media/base/pipeline_impl.h
+++ b/media/base/pipeline_impl.h
@@ -15,11 +15,11 @@
 #include "base/ref_counted.h"
 #include "base/thread.h"
 #include "base/time.h"
+#include "media/base/filter_host.h"
 #include "media/base/pipeline.h"
 
 namespace media {
 
-class FilterHostImpl;
 class PipelineInternal;
 
 // Class which implements the Media::Pipeline contract.  The majority of the
@@ -53,7 +53,6 @@ class PipelineImpl : public Pipeline {
   virtual PipelineError GetError() const;
 
  private:
-  friend class FilterHostImpl;
   friend class PipelineInternal;
   virtual ~PipelineImpl();
 
@@ -66,18 +65,19 @@ class PipelineImpl : public Pipeline {
   // must not be an error.
   bool IsPipelineOk() const;
 
-  // Methods called by FilterHostImpl to update pipeline state.
+  // Methods called by |pipeline_internal_| to update global pipeline data.
+  //
+  // Although this is the exact same as the FilterHost interface, we need to
+  // let |pipeline_internal_| receive the call first so it can post tasks as
+  // necessary.
+  void SetError(PipelineError error);
+  base::TimeDelta GetTime() const;
+  void SetTime(base::TimeDelta time);
   void SetDuration(base::TimeDelta duration);
   void SetBufferedTime(base::TimeDelta buffered_time);
   void SetTotalBytes(int64 total_bytes);
   void SetBufferedBytes(int64 buffered_bytes);
   void SetVideoSize(size_t width, size_t height);
-  void SetTime(base::TimeDelta time);
-
-  // Sets the error to the new error code only if the current error state is
-  // PIPELINE_OK. Returns true if error set, otherwise leaves current error
-  // alone, and returns false.
-  bool InternalSetError(PipelineError error);
 
   // Method called by the |pipeline_internal_| to insert a mime type into
   // the |rendered_mime_types_| set.
@@ -95,28 +95,22 @@ class PipelineImpl : public Pipeline {
   // initialized, this member will be set to true by the pipeline thread.
   bool initialized_;
 
-  // Duration of the media in microseconds.  Set by a FilterHostImpl object on
-  // behalf of a filter.
+  // Duration of the media in microseconds.  Set by filters.
   base::TimeDelta duration_;
 
-  // Amount of available buffered data in microseconds.  Set by a
-  // FilterHostImpl object on behalf of a filter.
+  // Amount of available buffered data in microseconds.  Set by filters.
   base::TimeDelta buffered_time_;
 
-  // Amount of available buffered data.  Set by a FilterHostImpl object
-  // on behalf of a filter.
+  // Amount of available buffered data.  Set by filters.
   int64 buffered_bytes_;
 
-  // Total size of the media.  Set by a FilterHostImpl object on behalf
-  // of a filter.
+  // Total size of the media.  Set by filters.
   int64 total_bytes_;
 
   // Lock used to serialize access for getter/setter methods.
   mutable Lock lock_;
 
-  // Video width and height.  Set by a FilterHostImpl object on behalf
-  // of a filter.  The video_size_access_lock_ is used to make sure access
-  // to the pair of width and height are modified or read in thread safe way.
+  // Video width and height.  Set by filters.
   size_t video_width_;
   size_t video_height_;
 
@@ -130,8 +124,7 @@ class PipelineImpl : public Pipeline {
   // the filters.
   float playback_rate_;
 
-  // Current playback time.  Set by a FilterHostImpl object on behalf of the
-  // audio renderer filter.
+  // Current playback time.  Set by filters.
   base::TimeDelta time_;
 
   // Status of the pipeline.  Initialized to PIPELINE_OK which indicates that
@@ -168,12 +161,13 @@ class PipelineImpl : public Pipeline {
 // transition to the "Error" state from any state. If Stop() is called during
 // initialization, this object will transition to "Stopped" state.
 
-class PipelineInternal : public base::RefCountedThreadSafe<PipelineInternal> {
+class PipelineInternal : public FilterHost,
+    public base::RefCountedThreadSafe<PipelineInternal> {
  public:
   // Methods called by PipelineImpl object on the client's thread.  These
   // methods post a task to call a corresponding xxxTask() method on the
   // message loop.  For example, Seek posts a task to call SeekTask.
-  explicit PipelineInternal(PipelineImpl* pipeline, MessageLoop* message_loop);
+  PipelineInternal(PipelineImpl* pipeline, MessageLoop* message_loop);
 
   // After Start() is called, a task of StartTask() is posted on the pipeline
   // thread to perform initialization. See StartTask() to learn more about
@@ -188,25 +182,6 @@ class PipelineInternal : public base::RefCountedThreadSafe<PipelineInternal> {
   void PlaybackRateChanged(float playback_rate);
   void VolumeChanged(float volume);
 
-  // Methods called by a FilterHostImpl object.  These methods may be called
-  // on any thread, either the pipeline's thread or any other.
-
-  // Sets the pipeline time and schedules a task to call back to any filters
-  // that have registered a time update callback.
-  void SetTime(base::TimeDelta time);
-
-  // Called by a FilterHostImpl on behalf of a filter calling
-  // FilterHost::SetError().  If the pipeline is running a nested message loop,
-  // it will be exited.
-  void SetError(PipelineError error);
-
-  // Simple accessor used by the FilterHostImpl class to get access to the
-  // pipeline object.
-  //
-  // TODO(scherkus): I think FilterHostImpl should not be talking to
-  // PipelineImpl but rather PipelineInternal.
-  PipelineImpl* pipeline() const { return pipeline_; }
-
   // Returns true if the pipeline has fully initialized.
   bool IsInitialized() { return state_ == kStarted; }
 
@@ -215,6 +190,16 @@ class PipelineInternal : public base::RefCountedThreadSafe<PipelineInternal> {
   friend class base::RefCountedThreadSafe<PipelineInternal>;
   virtual ~PipelineInternal();
 
+  // FilterHost implementation.
+  virtual void SetError(PipelineError error);
+  virtual base::TimeDelta GetTime() const;
+  virtual void SetTime(base::TimeDelta time);
+  virtual void SetDuration(base::TimeDelta duration);
+  virtual void SetBufferedTime(base::TimeDelta buffered_time);
+  virtual void SetTotalBytes(int64 total_bytes);
+  virtual void SetBufferedBytes(int64 buffered_bytes);
+  virtual void SetVideoSize(size_t width, size_t height);
+
   enum State {
     kCreated,
     kInitDataSource,
@@ -290,18 +275,11 @@ class PipelineInternal : public base::RefCountedThreadSafe<PipelineInternal> {
   // or it could be a string in the case of a DataSource.
   //
   // The CreateFilter() method actually does much more than simply creating the
-  // filter.  It creates the FilterHostImpl object, creates the filter using
-  // the filter factory, calls the MediaFilter::SetHost() method on the filter,
-  // and then calls the filter's type-specific Initialize(source) method to
-  // initialize the filter.  If the required filter cannot be created,
+  // filter.  It also creates the filter's thread and injects a FilterHost and
+  // MessageLoop.  Finally, it calls the filter's type-specific Initialize()
+  // method to initialize the filter.  If the required filter cannot be created,
   // PIPELINE_ERROR_REQUIRED_FILTER_MISSING is raised, initialization is halted
   // and this object will remain in the "Error" state.
-  //
-  // Callers can optionally use the returned Filter for further processing,
-  // but since the call already placed the filter in the list of filter hosts,
-  // callers can ignore the return value.  In any case, if this function can
-  // not create and initializes the specified Filter, then this method will
-  // return with |pipeline_->error_| != PIPELINE_OK.
   template <class Filter, class Source>
   void CreateFilter(FilterFactory* filter_factory,
                     Source source,
@@ -368,10 +346,15 @@ class PipelineInternal : public base::RefCountedThreadSafe<PipelineInternal> {
   scoped_ptr<PipelineCallback> seek_callback_;
   scoped_ptr<PipelineCallback> stop_callback_;
 
-  // Vector of FilterHostImpl objects that contain the filters for the pipeline.
-  typedef std::vector<FilterHostImpl*> FilterHostVector;
-  FilterHostVector filter_hosts_;
+  // Vector of our filters and map maintaining the relationship between the
+  // FilterType and the filter itself.
+  typedef std::vector<scoped_refptr<MediaFilter> > FilterVector;
+  FilterVector filters_;
+
+  typedef std::map<FilterType, scoped_refptr<MediaFilter> > FilterTypeMap;
+  FilterTypeMap filter_types_;
 
+  // Vector of threads owned by the pipeline and being used by filters.
   typedef std::vector<base::Thread*> FilterThreadVector;
   FilterThreadVector filter_threads_;
 
diff --git a/media/media.gyp b/media/media.gyp
index 853d3db..2c8e963 100644
--- a/media/media.gyp
+++ b/media/media.gyp
@@ -55,8 +55,6 @@
         'base/data_buffer.h',
         'base/factory.h',
         'base/filter_host.h',
-        'base/filter_host_impl.cc',
-        'base/filter_host_impl.h',
         'base/filters.h',
         'base/media_posix.cc',
         'base/media_win.cc',