Simplify VideoDecodeEngine interface by making everything synchronous.

media/video/video_decode_engine.h

Index: media/video/video_decode_engine.h
diff --git a/media/video/video_decode_engine.h b/media/video/video_decode_engine.h
index 2d52b2450dfde768837b1f80a84e70487a980eb0..9abf4e23f1ca987440135728c582563ba21211d6 100644
--- a/media/video/video_decode_engine.h
+++ b/media/video/video_decode_engine.h
@@ -5,101 +5,38 @@
 #ifndef MEDIA_VIDEO_VIDEO_DECODE_ENGINE_H_
 #define MEDIA_VIDEO_VIDEO_DECODE_ENGINE_H_
 
-#include "base/callback.h"
-#include "base/memory/scoped_ptr.h"
+#include "base/memory/ref_counted.h"
 #include "media/base/media_export.h"
-#include "media/base/video_decoder_config.h"
-#include "media/base/video_frame.h"
 
 namespace media {
 
 class Buffer;
-struct PipelineStatistics;
+class VideoDecoderConfig;
+class VideoFrame;
 
 class MEDIA_EXPORT VideoDecodeEngine {
  public:
-  struct MEDIA_EXPORT EventHandler {

[Ami GONE FROM CHROMIUM, 2011/11/01 22:17:40]

<SheddingHappyTears/>

-   public:
-    virtual ~EventHandler() {}
-    virtual void OnInitializeComplete(bool success) = 0;
-    virtual void OnUninitializeComplete() = 0;
-    virtual void OnFlushComplete() = 0;
-    virtual void OnSeekComplete() = 0;
-    virtual void OnError() = 0;
-
-    // TODO(hclam): The following two methods shouldn't belong to this class
-    // because they are not video decode events but used to send decoded
-    // video frames and request video packets.
-    //
-    // Signal the user of VideoDecodeEngine to provide a video sample.
-    //
-    // In the normal running state, this method is called by the video decode
-    // engine to request video samples used for decoding.
-    //
-    // In the case when the video decode engine is flushing, this method is
-    // called to return video samples acquired by the video decode engine.
-    //
-    // |buffer| can be NULL in which case this method call is purely for
-    // requesting new video samples. If |buffer| is non-NULL, the buffer is
-    // returned to the owner at the same time as a request for video sample
-    // is made.
-    virtual void ProduceVideoSample(scoped_refptr<Buffer> buffer) = 0;
-
-    // Signal the user of VideoDecodeEngine that a video frame is ready to
-    // be consumed or a video frame is returned to the owner.
-    //
-    // In the normal running state, this method is called to signal that
-    // |frame| contains a decoded video frame and is ready to be used.
-    //
-    // In the case of flushing and video frame is provided externally, this
-    // method is called to return the video frame object to the owner.
-    // The content of the video frame may be invalid.
-    virtual void ConsumeVideoFrame(scoped_refptr<VideoFrame> frame,
-                                   const PipelineStatistics& statistics) = 0;
-  };
-
   virtual ~VideoDecodeEngine() {}
 
-  // Initialize the engine with specified configuration.
-  //
-  // Engine should call EventHandler::OnInitializeDone() whether the
-  // initialization operation finished successfully or not.
-  virtual void Initialize(EventHandler* event_handler,
-                          const VideoDecoderConfig& config) = 0;
+  // Initialize the engine with specified configuration, returning true if
+  // successful.
+  virtual bool Initialize(const VideoDecoderConfig& config) = 0;
 
-  // Uninitialize the engine. Engine should destroy all resources and call
-  // EventHandler::OnUninitializeComplete().
+  // Uninitialize the engine, freeing all resources. Calls to Flush() or
+  // Decode() will have no effect afterwards.
   virtual void Uninitialize() = 0;
 
-  // Flush the engine. Engine should return all the buffers to owner ( which
-  // could be itself. ) then call EventHandler::OnFlushDone().
-  virtual void Flush() = 0;
-
-  // This method is used as a signal for the decode engine to preroll and
-  // issue read requests after Flush() is made.
-  virtual void Seek() = 0;
-
-  // Provide a video sample to be used by the video decode engine.
+  // Decode the compressed video data and store the result (if any) into

[Ami GONE FROM CHROMIUM, 2011/11/01 22:17:40]

s/compressed/encoded/
(here and below)

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

Done.

+  // |video_frame|. Note that a frame may not always be produced if the
+  // decode engine has insufficient compressed data. In such circumstances,
+  // additional calls to Decode() may be required.

[Ami GONE FROM CHROMIUM, 2011/11/01 22:17:40]

what if a buffer contains multiple frames?
(smells like maybe the client may need to detect this by "draining" Decode()
with empty buffers sometimes?  haven't looked at the rest of the CL yet).

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

Not sure how most of libavcodec handles that case. If the API contract is to
only deliver full frames at a time then that shouldn't happen. If the decoders
are resilient then I'd assume a loop around Decode() w/ empty buffers would be
required. AFAIK it's the former but haven't tested the latter.

   //
-  // This method is called in response to ProvideVideoSample() called to the
-  // user.
-  virtual void ConsumeVideoSample(scoped_refptr<Buffer> buffer) = 0;
+  // Returns true if operation was successful, false if an error occurred.

[Ami GONE FROM CHROMIUM, 2011/11/01 22:17:40]

More explicitly: false if decode error happens, true if no error happens?
(is EOS==false?  is no frame produced but no decode error == false?)

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

Done.

+  virtual bool Decode(scoped_refptr<Buffer> buffer,

[Ami GONE FROM CHROMIUM, 2011/11/01 22:17:40]

Should this be a const&?

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

Done.

+                      scoped_refptr<VideoFrame>* video_frame) = 0;
 
-  // Signal the video decode engine to produce a video frame or return the
-  // video frame object to the video decode engine.
-  //
-  // In the normal running state, this method is called by the user of the
-  // video decode engine to request a decoded video frame. If |frame| is
-  // NULL the video decode engine should allocate a video frame object.
-  // Otherwise video decode engine should try to use the video frame object
-  // provided as output.
-  //
-  // In flushing state and video frames are allocated internally this method
-  // is called by the user to return the video frame object.
-  //
-  // In response to this method call, ConsumeVideoFrame() is called with a
-  // video frame object containing decoded video content.
-  virtual void ProduceVideoFrame(scoped_refptr<VideoFrame> frame) = 0;
+  // Flush the internal state of the engine.

[Ami GONE FROM CHROMIUM, 2011/11/01 22:17:40]

Explicitly: drop pending encoded data that hasn't been returned as video_frames
yet on the floor?

(in particular this is different to VDA's Flush() which decodes all pending data
and delivers frames; what you have is closer to VDA's Reset())

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

Done.

+  virtual void Flush() = 0;
 };
 
 }  // namespace media