Move VideoDecoder out of media pipeline.

media/base/filters.h

 // Copyright (c) 2012 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.
 
 // Filters are connected in a strongly typed manner, with downstream filters
 // always reading data from upstream filters.  Upstream filters have no clue
 // who is actually reading from them, and return the results via callbacks.
 //
 //                         DemuxerStream(Video) <- VideoDecoder <- VideoRenderer
 // DataSource <- Demuxer <
@@ skipping 146 matching lines @@
   // PipelineImpl::Stop() goes through a Pause/Flush/Stop dance to all its
   // filters, waiting for each state transition to complete before starting the
   // next, but WebMediaPlayerImpl::Destroy() holds the renderer loop hostage for
   // the duration.  Default implementation does nothing; derived decoders may
   // override as needed.  http://crbug.com/110228 tracks removing this.
   virtual void PrepareForShutdownHack();
 
  protected:
   VideoDecoder();
   virtual ~VideoDecoder();
 };

[Ami GONE FROM CHROMIUM, 2012/03/14 20:05:01]

Can you confirm that Pause/Seek/Play are never called on VideoDecoder by adding
private virtual overrides for them here (to hide the base implementation) and
then not implementing them (to trigger a linker failure if anyone tries to call
them)?

[xhwang, 2012/03/14 22:28:41]

I like this! Done.

 
 
 class MEDIA_EXPORT VideoRenderer : public Filter {
  public:
   // Used to update the pipeline's clock time. The parameter is the time that
   // the clock should not exceed.
   typedef base::Callback<void(base::TimeDelta)> VideoTimeCB;
 
   // Initialize a VideoRenderer with the given VideoDecoder, executing the
   // callback upon completion.
-  virtual void Initialize(VideoDecoder* decoder,
-                          const PipelineStatusCB& callback,
+  virtual void Initialize(const scoped_refptr<VideoDecoder>& decoder,
+                          const PipelineStatusCB& pipeline_status_cb,

[Ami GONE FROM CHROMIUM, 2012/03/14 20:05:01]

s/pipeline_// ?

[xhwang, 2012/03/14 22:28:41]

I chose this verbose name because status_cb and stats_cb (once used in various
places) were too similar. Now since we use statistics_cb for the latter, this
doesn't matter any more. Anything bad except it's too long? :)

[Ami GONE FROM CHROMIUM, 2012/03/14 22:42:50]

My only objection is the length.

[xhwang, 2012/03/14 22:59:42]

Okay, will change this in the next CL too.

                           const StatisticsCB& statistics_cb,
-                          const VideoTimeCB& time_cb) = 0;
+                          const VideoTimeCB& video_time_cb) = 0;

[Ami GONE FROM CHROMIUM, 2012/03/14 20:05:01]

IMO "Video" belongs in neither the param name nor the type name (look at the
typedef; there's nothing video-specific about it).

[xhwang, 2012/03/14 22:28:41]

Will do in another CL.

 
   // Returns true if this filter has received and processed an end-of-stream
   // buffer.
   virtual bool HasEnded() = 0;
 };
 
 
 class MEDIA_EXPORT AudioRenderer : public Filter {
  public:
   // Used to update the pipeline's clock time. The first parameter is the
@@ skipping 21 matching lines @@
 
   // Resumes playback after underflow occurs.
   // |buffer_more_audio| is set to true if you want to increase the size of the
   // decoded audio buffer.
   virtual void ResumeAfterUnderflow(bool buffer_more_audio) = 0;
 };
 
 }  // namespace media
 
 #endif  // MEDIA_BASE_FILTERS_H_