Add AudioBufferStream.

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 5aefb84aa67e486971f5836cbdccb3e338abdb08..12ce075aeb26b885c3ef2a47ca19ccc680d7ba51 100644
--- a/media/base/audio_decoder.h
+++ b/media/base/audio_decoder.h
@@ -7,9 +7,11 @@
 
 #include "base/callback.h"
 #include "base/memory/ref_counted.h"
+#include "media/base/audio_decoder_config.h"
 #include "media/base/channel_layout.h"
-#include "media/base/pipeline_status.h"
+#include "media/base/decoder_buffer.h"
 #include "media/base/media_export.h"
+#include "media/base/pipeline_status.h"
 
 namespace media {
 
@@ -18,11 +20,13 @@ class DemuxerStream;
 
 class MEDIA_EXPORT AudioDecoder {
  public:
-  // Status codes for read operations.
+  // Status codes for decode operations.
   enum Status {

[DaleCurtis, 2014/02/21 21:48:48]

Since both AudioDecoder and VideoDecoder now have the same errors, should we put
these in a decoder_status.h file or something?

[rileya (GONE FROM CHROMIUM), 2014/02/21 23:04:02]

Sounds good, that'd also remove a DecoderStreamTraits member.

[rileya (GONE FROM CHROMIUM), 2014/02/24 21:04:11]

I'll save this for a future CL, as this one is quite large already, and this
change would add a fair bit of extra noise.

-    kOk,
-    kAborted,
-    kDecodeError,
+    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.
   };
 
   AudioDecoder();
@@ -31,9 +35,8 @@ class MEDIA_EXPORT AudioDecoder {
   // Initializes an AudioDecoder with the given DemuxerStream, executing the
   // callback upon completion.
   // statistics_cb is used to update global pipeline statistics.
-  virtual void Initialize(DemuxerStream* stream,
-                          const PipelineStatusCB& status_cb,
-                          const StatisticsCB& statistics_cb) = 0;
+  virtual void Initialize(const AudioDecoderConfig& config,
+                          const PipelineStatusCB& status_cb) = 0;
 
   // Requests samples to be decoded and returned via the provided callback.
   // Only one read may be in flight at any given time.
@@ -46,8 +49,9 @@ class MEDIA_EXPORT AudioDecoder {
   // Read(). This can happen if the DemuxerStream gets flushed and doesn't have
   // any more data to return.
   typedef base::Callback<void(Status, const scoped_refptr<AudioBuffer>&)>
-      ReadCB;
-  virtual void Read(const ReadCB& read_cb) = 0;
+      DecodeCB;
+  virtual void Decode(const scoped_refptr<DecoderBuffer>& buffer,
+                      const DecodeCB& decode_cb) = 0;
 
   // Resets decoder state, dropping any queued encoded data.
   virtual void Reset(const base::Closure& closure) = 0;
@@ -59,11 +63,22 @@ class MEDIA_EXPORT AudioDecoder {
   // complete before deleting the decoder.
   virtual void Stop(const base::Closure& closure) = 0;
 
+  // Returns true if the decoder currently has the ability to decode and return
+  // an AudioBuffer. Most implementations can allocate a new AudioBuffer and
+  // hence this will always return true. Override and return false for decoders
+  // that use a fixed set of AudioBuffers for decoding.
+  virtual bool CanReadWithoutStalling() const;

[DaleCurtis, 2014/02/21 21:48:48]

Seems this and HasQueuedData are pretty similar, but I'm not super familiar with
why we have this.  Looking through the code it seems to relate to available
allocations for GPU video decoding.

Is it possible to reuse this to do the same thing for audio?  I suspect the
answer is no, but it seems worth considering.

[rileya (GONE FROM CHROMIUM), 2014/02/21 23:04:02]

I'm unsure, I'm not really familiar with the gpu decoder...

[xhwang, 2014/02/21 23:57:56]

Yeah, this is a hacky and confusing function. For GVD, it wraps decoded frames
into picture buffers. A GVD only allocate M pictures buffers, i.e. there can be
at most M decoded frames outstanding. Now if the media pipeline wants to preroll
N frames, with N > M, the (M+1)'th decode will never finish, because the GVD
cannot grab the (M+1)'th picture buffer. Then the preroll never finishes,
because the VideoRendererImpl cannot preroll N frames.

This function basically let us know the the decoder cannot do any decoding
anymore, so that we don't get into this mud in the first place.

[rileya (GONE FROM CHROMIUM), 2014/02/24 21:04:11]

Alright, I removed this from AudioDecoder in favor of a specialization in
DecoderStream. It sounds like ideally it should be removed from VideoDecoder, so
no sense polluting AudioDecoder with it.

+
   // Returns various information about the decoded audio format.

[DaleCurtis, 2014/02/21 21:48:48]

Can these be removed in this CL or are you saving them for the next one?

[rileya (GONE FROM CHROMIUM), 2014/02/21 23:04:02]

I'm saving that for the next one, it didn't seem completely trivial to remove
them and I figured this CL was large enough as-is.

   virtual int bits_per_channel() = 0;
   virtual ChannelLayout channel_layout() = 0;
   virtual int samples_per_second() = 0;
 
+  // Some AudioDecoders will get multiple AudioBuffers out of a single
+  // DecoderBuffer, if this returns true, then the client should call Decode()
+  // with null DecoderBuffers until this returns false.
+  virtual bool HasQueuedData() const;

[xhwang, 2014/02/21 23:57:56]

Instead of
if (HasQueuedData())
  Decode(NULL);

Can we just have
scoped_refptr<AudioBuffer> GetAudioBuffer();
which return NULL if no data queued internally.

[rileya (GONE FROM CHROMIUM), 2014/02/24 21:04:11]

I did this, although we also need to pass back an AudioDecoder::Status (see line
61), this is not necessary for the DecryptingAudioDecoder since we only enqueue
if we're kOk, but this is not true of FFmpegAudioDecoder.

+
  private:
   DISALLOW_COPY_AND_ASSIGN(AudioDecoder);
 };