Simplify VideoDecodeEngine interface by making everything synchronous.

media/base/filters.h

Index: media/base/filters.h
diff --git a/media/base/filters.h b/media/base/filters.h
index 1624a60b0ebfd4fab9c88909c3e0c0089a61d9b8..bbb283115a2004ca00db03dfc733d183c383508f 100644
--- a/media/base/filters.h
+++ b/media/base/filters.h
@@ -164,22 +164,23 @@ class MEDIA_EXPORT VideoDecoder : public Filter {
   // Initialize a VideoDecoder with the given DemuxerStream, executing the
   // callback upon completion.
   // stats_callback is used to update global pipeline statistics.
+  //
+  // TODO(scherkus): switch to PipelineStatus callback.
   virtual void Initialize(DemuxerStream* stream, const base::Closure& callback,
                           const StatisticsCallback& stats_callback) = 0;
 
-  // Renderer provides an output buffer for Decoder to write to. These buffers
-  // will be recycled to renderer via the permanent callback.
+  // Request a frame to be decoded and returned via the provided callback.
+  // Only one read may be in flight at any given time.
   //
-  // We could also pass empty pointer here to let decoder provide buffers pool.
-  virtual void ProduceVideoFrame(scoped_refptr<VideoFrame> frame) = 0;
-
-  // Installs a permanent callback for passing decoded video output.
+  // This method must return immediately without executing the callback.

[acolwell GONE FROM CHROMIUM, 2011/11/03 20:00:43]

Consider adding or replacing with "Implementations guarantee that the callback
will not be called from within this method."

We should also be explicit about whether it is ok for Read() to be called from
within the callback.

[scherkus (not reviewing), 2011/11/03 20:34:38]

Done.

   //
-  // A NULL frame represents a decoding error.
-  typedef base::Callback<void(scoped_refptr<VideoFrame>)> ConsumeVideoFrameCB;
-  void set_consume_video_frame_callback(const ConsumeVideoFrameCB& callback) {
-    consume_video_frame_callback_ = callback;
-  }
+  // ^^^^^^^^^^^ it would be better to require callees to post directly to

[scherkus (not reviewing), 2011/11/03 04:55:59]

FYI

[acolwell GONE FROM CHROMIUM, 2011/11/03 20:00:43]

I don't think this commentary is useful especially since it refers to
implementation details of the FFmpegVideoDecoder & VideoRendererBase
implementations.

[scherkus (not reviewing), 2011/11/03 20:34:38]

Oh I wasn't planning on checking this comment in!  It was for you reviewers to
look at and comment on (I guess it worked!)

+  // the correct thread and have objects that only expect to live on one
+  // thread, but I can dream, can't I?
+  //
+  // Frames will be non-NULL yet may be end of stream frames.
+  typedef base::Callback<void(scoped_refptr<VideoFrame>)> ReadCB;
+  virtual void Read(const ReadCB& callback) = 0;
 
   // Returns the natural width and height of decoded video in pixels.
   //
@@ -191,19 +192,8 @@ class MEDIA_EXPORT VideoDecoder : public Filter {
   virtual gfx::Size natural_size() = 0;
 
  protected:
-  // Executes the permanent callback to pass off decoded video.
-  //
-  // TODO(scherkus): name this ConsumeVideoFrame() once we fix the TODO in
-  // VideoDecodeEngine::EventHandler to remove ConsumeVideoFrame() from there.
-  void VideoFrameReady(scoped_refptr<VideoFrame> frame) {
-    consume_video_frame_callback_.Run(frame);
-  }
-
   VideoDecoder();
   virtual ~VideoDecoder();
-
- private:
-  ConsumeVideoFrameCB consume_video_frame_callback_;
 };
 
 
@@ -212,6 +202,8 @@ class MEDIA_EXPORT AudioDecoder : public Filter {
   // Initialize a AudioDecoder with the given DemuxerStream, executing the
   // callback upon completion.
   // stats_callback is used to update global pipeline statistics.
+  //
+  // TODO(scherkus): switch to PipelineStatus callback.
   virtual void Initialize(DemuxerStream* stream, const base::Closure& callback,
                           const StatisticsCallback& stats_callback) = 0;