Move FilterType into MediaFilterCollection

media/base/media_filter_collection.h

Index: media/base/media_filter_collection.h
diff --git a/media/base/media_filter_collection.h b/media/base/media_filter_collection.h
index 31ea69c315dfcc100905dcb7c14894bb50029d62..bac43ab07d3d5062820e58d91feb253ea1458190 100644
--- a/media/base/media_filter_collection.h
+++ b/media/base/media_filter_collection.h
@@ -19,7 +19,12 @@ class MediaFilterCollection {
   MediaFilterCollection();
 
   // Adds a filter to the collection.
-  void AddFilter(MediaFilter* filter);
+  void AddFilter(DataSource* filter);

[scherkus (not reviewing), 2010/11/11 01:47:12]

FYI this will increase the size of this change but style guide isn't too hot on
function overloading and usually recommends it for equivalent types (i.e, char*
and std::string)

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Function_Overl...

I tend to agree and I'm considering we make it painfully obvious what's going on
and have AddDataSource, AddDemuxer, etc.. and same with SelectXXX()

I don't think we're relying on function overloading in pipeline_impl (i.e.,
there isn't any crazy template magic code) so this should be ok.

[acolwell GONE FROM CHROMIUM, 2010/11/11 19:24:07]

Done. I totally agree.

On 2010/11/11 01:47:12, scherkus wrote:

+  void AddFilter(Demuxer* filter);
+  void AddFilter(VideoDecoder* filter);
+  void AddFilter(AudioDecoder* filter);
+  void AddFilter(VideoRenderer* filter);
+  void AddFilter(AudioRenderer* filter);
 
   // Is the collection empty?
   bool IsEmpty() const;
@@ -30,16 +35,39 @@ class MediaFilterCollection {
   // Selects a filter of the specified type from the collection.
   // If the required filter cannot be found, NULL is returned.
   // If a filter is returned it is removed from the collection.
-  template <class Filter>
-  void SelectFilter(scoped_refptr<Filter>* filter_out) {
-    scoped_refptr<MediaFilter> filter;
-    SelectFilter(Filter::static_filter_type(), &filter);
-    *filter_out = reinterpret_cast<Filter*>(filter.get());
-  }
+  void SelectFilter(scoped_refptr<DataSource>* filter_out);
+  void SelectFilter(scoped_refptr<Demuxer>* filter_out);
+  void SelectFilter(scoped_refptr<VideoDecoder>* filter_out);
+  void SelectFilter(scoped_refptr<AudioDecoder>* filter_out);
+  void SelectFilter(scoped_refptr<VideoRenderer>* filter_out);
+  void SelectFilter(scoped_refptr<AudioRenderer>* filter_out);
 
  private:
+  // Identifies the type of filter implementation. Each filter has to be one of
+  // the following types. This is used to mark, identify, and support
+  // downcasting of different filter types stored in the filters_ list.
+  enum FilterType {

[scherkus (not reviewing), 2010/11/11 01:47:12]

I've always hated the Filter prefix and given this is private I say we lop off
the Filter and FILTER_ prefixes

[acolwell GONE FROM CHROMIUM, 2010/11/11 19:24:07]

Done.

+    FILTER_DATA_SOURCE,
+    FILTER_DEMUXER,
+    FILTER_AUDIO_DECODER,
+    FILTER_VIDEO_DECODER,
+    FILTER_AUDIO_RENDERER,
+    FILTER_VIDEO_RENDERER
+  };
+

[scherkus (not reviewing), 2010/11/11 01:47:12]

nit: remove extra blank line

[acolwell GONE FROM CHROMIUM, 2010/11/11 19:24:07]

Done.

+
   // List of filters managed by this collection.
-  std::list<scoped_refptr<MediaFilter> > filters_;
+  typedef std::list<Tuple2<FilterType,

[scherkus (not reviewing), 2010/11/11 01:47:12]

instead of mixing std and google Tuple2 maybe use std::pair/std::make_pair?

pretty indifferent about that one :)

[acolwell GONE FROM CHROMIUM, 2010/11/11 19:24:07]

Done.

+                           scoped_refptr<MediaFilter> > > FilterList;
+  FilterList filters_;
+
+  // Helper funtion that adds a filter to the filter list.

[scherkus (not reviewing), 2010/11/11 01:47:12]

funtion -> function

[acolwell GONE FROM CHROMIUM, 2010/11/11 19:24:07]

Done.

+  void AddFilter(FilterType filter_type, MediaFilter* filter);
+
+  // Helper function for SelectFilter() that manages the
+  // downcasting and mapping between FilterType & Filter class.

[scherkus (not reviewing), 2010/11/11 01:47:12]

nit: & -> and

from scanning I thought you meant a ref type so I imagine others might do the
same (pedantic nit, I know!)

[acolwell GONE FROM CHROMIUM, 2010/11/11 19:24:07]

Done.

+  template<FilterType filter_type, class Filter>
+  void SelectFilter(scoped_refptr<Filter>* filter_out);
 
   // Helper function that searches the filters list for a specific
   // filter type.