Add callback in VideoDecoder and AudioDecoder to return decoded frames.

media/filters/decrypting_audio_decoder.cc

Index: media/filters/decrypting_audio_decoder.cc
diff --git a/media/filters/decrypting_audio_decoder.cc b/media/filters/decrypting_audio_decoder.cc
index d00e3b99518762695ef0140a832a7aee75bf3f2e..54687c9640dc83bd63c22ef1819da41f30c5856a 100644
--- a/media/filters/decrypting_audio_decoder.cc
+++ b/media/filters/decrypting_audio_decoder.cc
@@ -45,7 +45,8 @@ DecryptingAudioDecoder::DecryptingAudioDecoder(
       weak_factory_(this) {}
 
 void DecryptingAudioDecoder::Initialize(const AudioDecoderConfig& config,
-                                        const PipelineStatusCB& status_cb) {
+                                        const PipelineStatusCB& status_cb,
+                                        const OutputCB& output_cb) {
   DVLOG(2) << "Initialize()";
   DCHECK(task_runner_->BelongsToCurrentThread());
   DCHECK(decode_cb_.is_null());
@@ -53,6 +54,7 @@ void DecryptingAudioDecoder::Initialize(const AudioDecoderConfig& config,
 
   weak_this_ = weak_factory_.GetWeakPtr();
   init_cb_ = BindToCurrentLoop(status_cb);
+  output_cb_ = BindToCurrentLoop(output_cb);

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

nit: BindToCurrentLoop() introduce an extra post. We are probably over-posting
the |output_cb| by doing this. I can see that on l.97, in Decode() call, we
should post the |output_cb|. But in EnqueueSamples (called in DeliverFrames(),
which is always posted), we don't really need to post the |output_cb|. Since I
assume most frames are delivered in EnqueueSamples(), it's worth dropping this
extra post.

I notice that in the original code, we always BindToCurrentLoop(decode_cb). So
this is not introduced by your CL. Feel free to keep it and I'll see whether
it's worth to improve it.

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

VideoDecoder contract says that |decode_cb| is not called before Decode()
returns, so |decode_cb| must be posted. Similarly DecoderStream contract
requires that Read() is completed asynchronously. It should be possible to allow
output_cb to be called from Decode(), but I think it's better to do it later in
a separate CL.

 
   if (!config.IsValidConfig()) {
     DLOG(ERROR) << "Invalid audio stream config.";
@@ -92,14 +94,8 @@ void DecryptingAudioDecoder::Decode(const scoped_refptr<DecoderBuffer>& buffer,
 
   // Return empty (end-of-stream) frames if decoding has finished.
   if (state_ == kDecodeFinished) {
-    base::ResetAndReturn(&decode_cb_).Run(kOk, AudioBuffer::CreateEOSBuffer());
-    return;
-  }
-
-  if (!queued_audio_frames_.empty()) {
-    DCHECK(!buffer);
-    base::ResetAndReturn(&decode_cb_).Run(kOk, queued_audio_frames_.front());
-    queued_audio_frames_.pop_front();
+    output_cb_.Run(AudioBuffer::CreateEOSBuffer());
+    base::ResetAndReturn(&decode_cb_).Run(kOk);
     return;
   }
 
@@ -115,14 +111,6 @@ void DecryptingAudioDecoder::Decode(const scoped_refptr<DecoderBuffer>& buffer,
   DecodePendingBuffer();
 }
 
-scoped_refptr<AudioBuffer> DecryptingAudioDecoder::GetDecodeOutput() {
-  if (queued_audio_frames_.empty())
-    return NULL;
-  scoped_refptr<AudioBuffer> out = queued_audio_frames_.front();
-  queued_audio_frames_.pop_front();
-  return out;
-}
-
 void DecryptingAudioDecoder::Reset(const base::Closure& closure) {
   DVLOG(2) << "Reset() - state: " << state_;
   DCHECK(task_runner_->BelongsToCurrentThread());
@@ -149,7 +137,7 @@ void DecryptingAudioDecoder::Reset(const base::Closure& closure) {
   if (state_ == kWaitingForKey) {
     DCHECK(!decode_cb_.is_null());
     pending_buffer_to_decode_ = NULL;
-    base::ResetAndReturn(&decode_cb_).Run(kAborted, NULL);
+    base::ResetAndReturn(&decode_cb_).Run(kAborted);
   }
 
   DCHECK(decode_cb_.is_null());
@@ -174,7 +162,7 @@ void DecryptingAudioDecoder::Stop() {
   if (!init_cb_.is_null())
     base::ResetAndReturn(&init_cb_).Run(DECODER_ERROR_NOT_SUPPORTED);
   if (!decode_cb_.is_null())
-    base::ResetAndReturn(&decode_cb_).Run(kAborted, NULL);
+    base::ResetAndReturn(&decode_cb_).Run(kAborted);
   if (!reset_cb_.is_null())
     base::ResetAndReturn(&reset_cb_).Run();
 
@@ -265,7 +253,6 @@ void DecryptingAudioDecoder::DeliverFrame(
   DCHECK_EQ(state_, kPendingDecode) << state_;
   DCHECK(!decode_cb_.is_null());
   DCHECK(pending_buffer_to_decode_.get());
-  DCHECK(queued_audio_frames_.empty());
 
   bool need_to_try_again_if_nokey_is_returned = key_added_while_decode_pending_;
   key_added_while_decode_pending_ = false;
@@ -275,7 +262,7 @@ void DecryptingAudioDecoder::DeliverFrame(
   pending_buffer_to_decode_ = NULL;
 
   if (!reset_cb_.is_null()) {
-    base::ResetAndReturn(&decode_cb_).Run(kAborted, NULL);
+    base::ResetAndReturn(&decode_cb_).Run(kAborted);
     DoReset();
     return;
   }
@@ -285,7 +272,7 @@ void DecryptingAudioDecoder::DeliverFrame(
   if (status == Decryptor::kError) {
     DVLOG(2) << "DeliverFrame() - kError";
     state_ = kDecodeFinished; // TODO add kError state
-    base::ResetAndReturn(&decode_cb_).Run(kDecodeError, NULL);
+    base::ResetAndReturn(&decode_cb_).Run(kDecodeError);
     return;
   }
 
@@ -309,13 +296,13 @@ void DecryptingAudioDecoder::DeliverFrame(
     DVLOG(2) << "DeliverFrame() - kNeedMoreData";
     if (scoped_pending_buffer_to_decode->end_of_stream()) {
       state_ = kDecodeFinished;
-      base::ResetAndReturn(&decode_cb_)
-          .Run(kOk, AudioBuffer::CreateEOSBuffer());
+      output_cb_.Run(AudioBuffer::CreateEOSBuffer());
+      base::ResetAndReturn(&decode_cb_).Run(kOk);
       return;
     }
 
     state_ = kIdle;
-    base::ResetAndReturn(&decode_cb_).Run(kNotEnoughData, NULL);
+    base::ResetAndReturn(&decode_cb_).Run(kOk);
     return;
   }
 
@@ -324,8 +311,7 @@ void DecryptingAudioDecoder::DeliverFrame(
   EnqueueFrames(frames);
 
   state_ = kIdle;
-  base::ResetAndReturn(&decode_cb_).Run(kOk, queued_audio_frames_.front());
-  queued_audio_frames_.pop_front();
+  base::ResetAndReturn(&decode_cb_).Run(kOk);
 }
 
 void DecryptingAudioDecoder::OnKeyAdded() {
@@ -352,12 +338,10 @@ void DecryptingAudioDecoder::DoReset() {
 
 void DecryptingAudioDecoder::EnqueueFrames(
     const Decryptor::AudioBuffers& frames) {
-  queued_audio_frames_ = frames;
-
-  for (Decryptor::AudioBuffers::iterator iter = queued_audio_frames_.begin();
-       iter != queued_audio_frames_.end();
+  for (Decryptor::AudioBuffers::const_iterator iter = frames.begin();
+       iter != frames.end();
        ++iter) {
-    scoped_refptr<AudioBuffer>& frame = *iter;
+    scoped_refptr<AudioBuffer> frame = *iter;
 
     DCHECK(!frame->end_of_stream()) << "EOS frame returned.";
     DCHECK_GT(frame->frame_count(), 0) << "Empty frame returned.";
@@ -372,6 +356,8 @@ void DecryptingAudioDecoder::EnqueueFrames(
 
     frame->set_timestamp(current_time);
     timestamp_helper_->AddFrames(frame->frame_count());
+
+    output_cb_.Run(frame);
   }
 }