Simplify VideoDecodeEngine interface by making everything synchronous.

media/base/filters.h

 // Copyright (c) 2011 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 @@
   // Notify the DataSource of the bitrate of the media.
   // Values of |bitrate| <= 0 are invalid and should be ignored.
   virtual void SetBitrate(int bitrate) = 0;
 };
 
 class MEDIA_EXPORT VideoDecoder : public Filter {
  public:
   // 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.
   //
   // Clients should NOT rely on these values to remain constant. Instead, use
   // the width/height from decoded video frames themselves.
   //
   // TODO(scherkus): why not rely on prerolling and decoding a single frame to
   // get dimensions?
   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_;
 };
 
 
 class MEDIA_EXPORT AudioDecoder : public Filter {
  public:
   // 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;
 
   // Renderer provides an output buffer for Decoder to write to. These buffers
   // will be recycled to renderer via the permanent callback.
   //
   // We could also pass empty pointer here to let decoder provide buffers pool.
   virtual void ProduceAudioSamples(scoped_refptr<Buffer> buffer) = 0;
 
   // Installs a permanent callback for passing decoded audio output.
@@ skipping 54 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_