Add AudioBufferStream.

media/filters/decoder_stream.cc

Index: media/filters/decoder_stream.cc
diff --git a/media/filters/decoder_stream.cc b/media/filters/decoder_stream.cc
index ebda8c00594cc948e3fb528e85b4c1ee2f05d1ff..40ce25453bb3a8f59c3850e6136d27b50217fdd5 100644
--- a/media/filters/decoder_stream.cc
+++ b/media/filters/decoder_stream.cc
@@ -10,6 +10,7 @@
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/single_thread_task_runner.h"
+#include "media/base/audio_decoder.h"
 #include "media/base/bind_to_current_loop.h"
 #include "media/base/decoder_buffer.h"
 #include "media/base/demuxer_stream.h"
@@ -28,6 +29,11 @@ const char* GetTraceString<DemuxerStream::VIDEO>() {
   return "DecoderStream<VIDEO>::Decode";
 }
 
+template <>
+const char* GetTraceString<DemuxerStream::AUDIO>() {
+  return "DecoderStream<AUDIO>::Decode";
+}
+
 template <DemuxerStream::Type StreamType>
 DecoderStream<StreamType>::DecoderStream(
     const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
@@ -65,7 +71,6 @@ void DecoderStream<StreamType>::Initialize(DemuxerStream* stream,
   // TODO(xhwang): DecoderSelector only needs a config to select a decoder.
   decoder_selector_->SelectDecoder(
       stream,
-      StatisticsCB(),
       base::Bind(&DecoderStream<StreamType>::OnDecoderSelected,
                  weak_factory_.GetWeakPtr()));
 }
@@ -319,6 +324,7 @@ void DecoderStream<StreamType>::OnDecodeOutputReady(
     return;
   }
 
+  DCHECK(output);
   SatisfyRead(OK, output);
 }
 
@@ -335,6 +341,25 @@ void DecoderStream<StreamType>::ReadFromDemuxerStream() {
                            weak_factory_.GetWeakPtr()));
 }
 
+template <>
+void DecoderStream<DemuxerStream::AUDIO>::ReadFromDemuxerStream() {
+  DVLOG(2) << __FUNCTION__;
+  DCHECK_EQ(state_, STATE_NORMAL) << state_;
+  DCHECK(!read_cb_.is_null());
+  DCHECK(reset_cb_.is_null());
+  DCHECK(stop_cb_.is_null());
+  DCHECK(decoder_);
+
+  if (decoder_->HasQueuedData()) {
+    Decode(NULL);

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

Won't this happen naturally as the decoder is drained at EOS / seek?  I'm not
sure I understand why this needs to be special cased (since it wasn't before
that I can see).  Can you elaborate?

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

I just realized how unintuitive this looks now, haha. This logic shouldn't be in
here, it was just easy to put it here, basically if the Decoder has queued data
still, we can return that instead of actually doing a Demuxer read. Passing NULL
(since we're just getting something from the queue, we don't have a
DecoderBuffer) makes it look like an EOS though, which didn't occur to me until
now.

This is the problem we discussed at some point of one DecoderBuffer potentially
decoding into multiple AudioBuffers (for the ffmpeg and decrypting decoders
anyway). For one call to AudioBufferStream::Read, the caller expects a single
AudioBuffer. So unlike VideoFrames we may not need to initiate a Demuxer read
for every Read() call.

Previously the Decoder managed a queue of AudioBuffers and only when it ran out
did it initiate a Demuxer read. Now that the Decoder no longer controls the
DemuxerStream we have this awkward situation where the queue of additional
AudioBuffers lives in the Decoder still, and the DemuxerStream is controlled a
level above in the ABS.

So here I added HasQueuedData() and made the ABS keep calling Decode() with NULL
DecoderBuffers after the first call with the DecoderBuffer until HasQueuedData()
is false. This seems clunky.

If we're willing to change the interface of Decode() vs VideoDecoder, then maybe
we could return a vector of AudioBuffers and then move the queue of AudioBuffers
up into the ABS. This would however clutter DecoderStream with more
specialization for audio only (we could make VideoDecoder match I suppose, but
none of the current decoders require this behavior).

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

Some random ideas:

1, See comment above, if you have
scoped_refptr<AudioBuffer> GetAudioBuffer();
function, you can avoid this Decode(NULL) call.

2, Shouldn't this be checked/called in DS::Read() instead of in
DS::ReadFromDemuxerStream()?

3, Currently we don't async audio decoders, so (1) + (2) sgtm.

4, This can actually also be helpful for a problem in the video pipeline:
- GVD accumulates multiple decoded frames internally.
- Demuxer runs out of data (in the MSE case)
- VideoRendererImpl calls Read(). Since we only call into the decoder after the
demxuer read is returned (which never happens), the decoded frames in GVD is
stuck there.
We need to think more about this for video. But that's our of the scope of this
CL.

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

1) Sounds good, done.

2) Yup, this was originally placed here as a hack to avoid a huge specialization
of Read(), but I added a helper instead.

4) It sounds like maybe moving the queue of decoded output (plus status codes)
into the DecoderStream would help get rid of the VRD problem as well as this
one. I added a TODO to the header and I'll look into it for another CL.

[rileya (GONE FROM CHROMIUM), 2014/02/24 21:07:50]

s/VRD/GVD/

Not sure how I messed that up haha... must've been thinking of VRI...

+  } else {
+    state_ = STATE_PENDING_DEMUXER_READ;
+    stream_->Read(
+        base::Bind(&DecoderStream<DemuxerStream::AUDIO>::OnBufferReady,
+                   weak_factory_.GetWeakPtr()));
+  }
+}
+
 template <DemuxerStream::Type StreamType>
 void DecoderStream<StreamType>::OnBufferReady(
     DemuxerStream::Status status,
@@ -387,12 +412,10 @@ void DecoderStream<StreamType>::ReinitializeDecoder() {
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK_EQ(state_, STATE_FLUSHING_DECODER) << state_;
 
-  // TODO(rileya): Specialize this for audio, or, better yet, change
-  // DemuxerStream config getters to be templated.
-  DCHECK(stream_->video_decoder_config().IsValidConfig());
+  DCHECK(StreamTraits::GetDecoderConfig(*stream_).IsValidConfig());
   state_ = STATE_REINITIALIZING_DECODER;
   decoder_->Initialize(
-      stream_->video_decoder_config(),
+      StreamTraits::GetDecoderConfig(*stream_),
       base::Bind(&DecoderStream<StreamType>::OnDecoderReinitialized,
                  weak_factory_.GetWeakPtr()));
 }
@@ -492,4 +515,5 @@ void DecoderStream<StreamType>::OnDecoderStopped() {
 }
 
 template class DecoderStream<DemuxerStream::VIDEO>;
+template class DecoderStream<DemuxerStream::AUDIO>;
 }  // namespace media