Add callback in VideoDecoder and AudioDecoder to return decoded frames.

media/base/audio_decoder.h

Index: media/base/audio_decoder.h
diff --git a/media/base/audio_decoder.h b/media/base/audio_decoder.h
index 5c5e2941175dc8911cda4ae89db80c5aa7b982e0..39d4096e29cfd3a906eec02685d0b653a790cf81 100644
--- a/media/base/audio_decoder.h
+++ b/media/base/audio_decoder.h
@@ -26,11 +26,16 @@ class MEDIA_EXPORT AudioDecoder {
   enum Status {
     kOk,  // We're all good.
     kAborted,  // We aborted as a result of Stop() or Reset().
-    kNotEnoughData,  // Not enough data to produce a video frame.
     kDecodeError,  // A decoding error occurred.
     kDecryptError  // Decrypting error happened.
   };
 
+  // Callback to return decoded buffers.
+  typedef base::Callback<void(const scoped_refptr<AudioBuffer>&)> OutputCB;
+
+  // Callback for Decode();
+  typedef base::Callback<void(Status)> DecodeCB;

[xhwang, 2014/05/29 22:15:14]

What does this exactly mean? We need a comment here to make the API contract
clear. For example (if I understand it correctly): Called when the input has
been processed and the VideoDecoder can accept more inputs for decode.

[Sergey Ulanov, 2014/06/03 00:08:11]

Done.

+
   AudioDecoder();
   virtual ~AudioDecoder();
 
@@ -38,7 +43,8 @@ class MEDIA_EXPORT AudioDecoder {
   // callback upon completion.
   // statistics_cb is used to update global pipeline statistics.

[xhwang, 2014/05/29 22:15:14]

Add a comment on |output_cb|.

[Sergey Ulanov, 2014/06/03 00:08:11]

Done.

   virtual void Initialize(const AudioDecoderConfig& config,
-                          const PipelineStatusCB& status_cb) = 0;
+                          const PipelineStatusCB& status_cb,
+                          const OutputCB& output_cb) = 0;
 
   // Requests samples to be decoded and returned via the provided callback.

[xhwang, 2014/05/29 22:15:14]

This needs to be updated.

[Sergey Ulanov, 2014/06/03 00:08:11]

Done.

   // Only one decode may be in flight at any given time.
@@ -49,16 +55,9 @@ class MEDIA_EXPORT AudioDecoder {
   // Non-NULL sample buffer pointers will contain decoded audio data or may
   // indicate the end of the stream. A NULL buffer pointer indicates an aborted
   // Decode().
-  typedef base::Callback<void(Status, const scoped_refptr<AudioBuffer>&)>
-      DecodeCB;
   virtual void Decode(const scoped_refptr<DecoderBuffer>& buffer,
                       const DecodeCB& decode_cb) = 0;
 
-  // Some AudioDecoders will queue up multiple AudioBuffers from a single
-  // DecoderBuffer, if we have any such queued buffers this will return the next
-  // one. Otherwise we return a NULL AudioBuffer.
-  virtual scoped_refptr<AudioBuffer> GetDecodeOutput();
-
   // Resets decoder state, dropping any queued encoded data.
   virtual void Reset(const base::Closure& closure) = 0;

[xhwang, 2014/05/29 22:15:14]

In this case, shall all decoded frames be simply dropped, or fired via OutputCB?

[Sergey Ulanov, 2014/06/03 00:08:11]

I think the comment is not entirely true. The buffers may not be actually
dropped. Audio decoders post a task for all callbacks, and these tasks are not
canceled by Reset(). Updated it to reflect this issue.

[xhwang, 2014/06/05 21:53:48]

To clarify, do you mean when Reset() is called, there may already be DecodeCBs
posted to the message loop but not executed yet? In that case, the AudioDecoders
can always post the reset callback. Callbacks should be executed in the order
they are posted so my previous comment will be valid. Am I missing anything?

[Sergey Ulanov, 2014/06/06 22:49:39]

Yes, that's right.
My point was that pending Decode requests may still finish with kOk, so from
callers perspective it doesn't look like all data is dropped as the comment was
saying. Reworded the comment as follows:
  // Resets decoder state. All pending Decode() requests will be finished or
  // aborted before |closure| is called.