Add callback in VideoDecoder and AudioDecoder to return decoded frames.

media/filters/audio_renderer_impl_unittest.cc

Index: media/filters/audio_renderer_impl_unittest.cc
diff --git a/media/filters/audio_renderer_impl_unittest.cc b/media/filters/audio_renderer_impl_unittest.cc
index 92391f83859a5f21a936541c176a4544b5d07654..0976e7e29300a8bd2af099524ec3936472e8061b 100644
--- a/media/filters/audio_renderer_impl_unittest.cc
+++ b/media/filters/audio_renderer_impl_unittest.cc
@@ -29,6 +29,7 @@ using ::testing::_;
 using ::testing::AnyNumber;
 using ::testing::Invoke;
 using ::testing::Return;
+using ::testing::SaveArg;
 
 namespace media {
 
@@ -77,9 +78,10 @@ class AudioRendererImplTest : public ::testing::Test {
     EXPECT_CALL(*decoder_, Reset(_))
         .WillRepeatedly(Invoke(this, &AudioRendererImplTest::ResetDecoder));
 
-    // Mock out demuxer reads
+    // Mock out demuxer reads.
     EXPECT_CALL(demuxer_stream_, Read(_)).WillRepeatedly(
-        RunCallback<0>(DemuxerStream::kOk, DecoderBuffer::CreateEOSBuffer()));
+        RunCallback<0>(DemuxerStream::kOk,
+                       scoped_refptr<DecoderBuffer>(new DecoderBuffer(0))));

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

why making this change?

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

DecoderStream<StreamType>::OnBufferReady() stops reading after EOF packet has
been demuxed. This breaks Underflow_EndOfStream() which calls
WaitForPendingRead() after SatisfyPendingRead().

     EXPECT_CALL(demuxer_stream_, SupportsConfigChanges())
         .WillRepeatedly(Return(true));
     AudioParameters out_params(AudioParameters::AUDIO_PCM_LOW_LATENCY,
@@ -112,8 +114,9 @@ class AudioRendererImplTest : public ::testing::Test {
   }
 
   void ExpectUnsupportedAudioDecoder() {
-    EXPECT_CALL(*decoder_, Initialize(_, _))
-        .WillOnce(RunCallback<1>(DECODER_ERROR_NOT_SUPPORTED));
+    EXPECT_CALL(*decoder_, Initialize(_, _, _))
+        .WillOnce(DoAll(SaveArg<2>(&output_cb_),
+                        RunCallback<1>(DECODER_ERROR_NOT_SUPPORTED)));
   }
 
   MOCK_METHOD1(OnStatistics, void(const PipelineStatistics&));
@@ -141,8 +144,9 @@ class AudioRendererImplTest : public ::testing::Test {
   }
 
   void Initialize() {
-    EXPECT_CALL(*decoder_, Initialize(_, _))
-        .WillOnce(RunCallback<1>(PIPELINE_OK));
+    EXPECT_CALL(*decoder_, Initialize(_, _, _))
+        .WillOnce(DoAll(SaveArg<2>(&output_cb_),
+                        RunCallback<1>(PIPELINE_OK)));
     EXPECT_CALL(*decoder_, Stop());
     InitializeWithStatus(PIPELINE_OK);
 
@@ -162,8 +166,9 @@ class AudioRendererImplTest : public ::testing::Test {
   }
 
   void InitializeAndStop() {
-    EXPECT_CALL(*decoder_, Initialize(_, _))
-        .WillOnce(RunCallback<1>(PIPELINE_OK));
+    EXPECT_CALL(*decoder_, Initialize(_, _, _))
+        .WillOnce(DoAll(SaveArg<2>(&output_cb_),
+                        RunCallback<1>(PIPELINE_OK)));
     EXPECT_CALL(*decoder_, Stop());
 
     WaitableMessageLoopEvent event;
@@ -178,8 +183,9 @@ class AudioRendererImplTest : public ::testing::Test {
   }
 
   void InitializeAndStopDuringDecoderInit() {
-    EXPECT_CALL(*decoder_, Initialize(_, _))
-        .WillOnce(EnterPendingDecoderInitStateAction(this));
+    EXPECT_CALL(*decoder_, Initialize(_, _, _))
+        .WillOnce(DoAll(SaveArg<2>(&output_cb_),
+                        EnterPendingDecoderInitStateAction(this)));
     EXPECT_CALL(*decoder_, Stop());
 
     WaitableMessageLoopEvent event;
@@ -223,9 +229,6 @@ class AudioRendererImplTest : public ::testing::Test {
     WaitForPendingRead();
     DeliverRemainingAudio();
     event.RunAndWaitForStatus(PIPELINE_OK);
-
-    // We should have no reads.
-    EXPECT_TRUE(decode_cb_.is_null());
   }
 
   void StartRendering() {
@@ -284,6 +287,7 @@ class AudioRendererImplTest : public ::testing::Test {
     next_timestamp_->AddFrames(size);
 
     DeliverBuffer(AudioDecoder::kOk, buffer);
+    message_loop_.RunUntilIdle();
   }
 
   void AbortPendingRead() {
@@ -291,13 +295,17 @@ class AudioRendererImplTest : public ::testing::Test {
   }
 
   void DeliverEndOfStream() {
-    DeliverBuffer(AudioDecoder::kOk, AudioBuffer::CreateEOSBuffer());
+    while (!decode_cb_.is_null()) {
+      DeliverBuffer(AudioDecoder::kOk, AudioBuffer::CreateEOSBuffer());
+      message_loop_.RunUntilIdle();

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

Run the message loop in DeliverBuffer()?

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

Done.

+    }

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

why "while" loop? There could be only one pending |decode_cb_|, which will be
reset in DliverBuffer?

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

DecoderStream::OnDecodeDone() may get another buffer from the stream. and call
the decoder again. This loop fills up the queue in DecodeStream with EOF
buffers. I agree it's ugly.
I would prefer to rewrite tests in this file to use a fake demuxer, then it
would be possible to control what's being read from the demuxer, but I don't
think it's needs to be in this CL.

   }
 
   // Delivers frames until |renderer_|'s internal buffer is full and no longer
   // has pending reads.
   void DeliverRemainingAudio() {
     SatisfyPendingRead(frames_remaining_in_buffer());
+    message_loop_.RunUntilIdle();

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

SatisfyPendingRead() already runs the message loop at the end of the function.

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

Done.

   }
 
   // Attempts to consume |requested_frames| frames from |renderer_|'s internal
@@ -465,8 +473,10 @@ class AudioRendererImplTest : public ::testing::Test {
   }
 
   void ResetDecoder(const base::Closure& reset_cb) {
-    CHECK(decode_cb_.is_null())
-        << "Reset overlapping with reads is not permitted";
+    if (!decode_cb_.is_null()) {
+      reset_cb_ = reset_cb;

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

Add a comment that in this case the |reset_cb| is fired in DeliverBuffer() after
the decoder is flushed.

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

Done.

+      return;
+    }
 
     message_loop_.PostTask(FROM_HERE, reset_cb);
   }
@@ -474,7 +484,12 @@ class AudioRendererImplTest : public ::testing::Test {
   void DeliverBuffer(AudioDecoder::Status status,
                      const scoped_refptr<AudioBuffer>& buffer) {
     CHECK(!decode_cb_.is_null());
-    base::ResetAndReturn(&decode_cb_).Run(status, buffer);
+    if (buffer)
+      output_cb_.Run(buffer);
+    base::ResetAndReturn(&decode_cb_).Run(status);
+
+    if (!reset_cb_.is_null())
+      base::ResetAndReturn(&reset_cb_).Run();
   }
 
   MockDemuxerStream demuxer_stream_;
@@ -485,7 +500,9 @@ class AudioRendererImplTest : public ::testing::Test {
   TimeTicks time_;
 
   // Used for satisfying reads.
+  AudioDecoder::OutputCB output_cb_;
   AudioDecoder::DecodeCB decode_cb_;
+  base::Closure reset_cb_;
   scoped_ptr<AudioTimestampHelper> next_timestamp_;
 
   WaitableMessageLoopEvent ended_event_;