Allow multiple concurrent Decode() requests in VideoDecoder interface.

media/filters/video_frame_stream_unittest.cc

Index: media/filters/video_frame_stream_unittest.cc
diff --git a/media/filters/video_frame_stream_unittest.cc b/media/filters/video_frame_stream_unittest.cc
index 5225e98a543e9cefc301b42727ac3ea6167e2c13..dc5ba58d8696a06302bfee90f2294df447dbf4a0 100644
--- a/media/filters/video_frame_stream_unittest.cc
+++ b/media/filters/video_frame_stream_unittest.cc
@@ -26,17 +26,34 @@ static const int kDecodingDelay = 7;
 
 namespace media {
 
+struct VideoFrameStreamTestParams {
+  VideoFrameStreamTestParams(bool is_encrypted,
+                             bool enable_get_decode_output,
+                             int decoding_delay,
+                             int parallel_decoding)
+      : is_encrypted(is_encrypted),
+        enable_get_decode_output(enable_get_decode_output),
+        decoding_delay(decoding_delay),
+        parallel_decoding(parallel_decoding) {}
+
+  bool is_encrypted;
+  bool enable_get_decode_output;
+  int decoding_delay;
+  int parallel_decoding;
+};
+
 class VideoFrameStreamTest
     : public testing::Test,
-      public testing::WithParamInterface<std::tr1::tuple<bool, bool> > {
+      public testing::WithParamInterface<VideoFrameStreamTestParams> {
  public:
   VideoFrameStreamTest()
       : demuxer_stream_(new FakeDemuxerStream(kNumConfigs,
                                               kNumBuffersInOneConfig,
-                                              IsEncrypted())),
+                                              GetParam().is_encrypted)),
         decryptor_(new NiceMock<MockDecryptor>()),
-        decoder_(
-            new FakeVideoDecoder(kDecodingDelay, IsGetDecodeOutputEnabled())),
+        decoder_(new FakeVideoDecoder(GetParam().decoding_delay,
+                                      GetParam().enable_get_decode_output,
+                                      GetParam().parallel_decoding)),
         is_initialized_(false),
         num_decoded_frames_(0),
         pending_initialize_(false),
@@ -73,14 +90,6 @@ class VideoFrameStreamTest
     EXPECT_FALSE(is_initialized_);
   }
 
-  bool IsEncrypted() {
-    return std::tr1::get<0>(GetParam());
-  }
-
-  bool IsGetDecodeOutputEnabled() {
-    return std::tr1::get<1>(GetParam());
-  }
-
   MOCK_METHOD1(SetDecryptorReadyCallback, void(const media::DecryptorReadyCB&));
 
   void OnStatistics(const PipelineStatistics& statistics) {
@@ -120,8 +129,8 @@ class VideoFrameStreamTest
     }
 
     DCHECK_EQ(stream_type, Decryptor::kVideo);
-    scoped_refptr<DecoderBuffer> decrypted = DecoderBuffer::CopyFrom(
-        encrypted->data(), encrypted->data_size());
+    scoped_refptr<DecoderBuffer> decrypted =
+        DecoderBuffer::CopyFrom(encrypted->data(), encrypted->data_size());
     decrypted->set_timestamp(encrypted->timestamp());
     decrypted->set_duration(encrypted->duration());
     decrypt_cb.Run(Decryptor::kSuccess, decrypted);
@@ -131,16 +140,19 @@ class VideoFrameStreamTest
   void FrameReady(VideoFrameStream::Status status,
                   const scoped_refptr<VideoFrame>& frame) {
     DCHECK(pending_read_);
-    // TODO(xhwang): Add test cases where the fake decoder returns error or
-    // the fake demuxer aborts demuxer read.
-    ASSERT_TRUE(status == VideoFrameStream::OK ||
-                status == VideoFrameStream::ABORTED) << status;
     frame_read_ = frame;
+    last_read_status_ = status;
     if (frame.get() && !frame->end_of_stream())
       num_decoded_frames_++;
     pending_read_ = false;
   }
 
+  void FrameReadyHoldDemuxer(VideoFrameStream::Status status,
+                             const scoped_refptr<VideoFrame>& frame) {
+    FrameReady(status, frame);
+
+  }
+
   void OnReset() {
     DCHECK(!pending_read_);
     DCHECK(pending_reset_);
@@ -224,7 +236,7 @@ class VideoFrameStreamTest
         break;
 
       case DECODER_DECODE:
-        decoder_->HoldNextDecode();
+        decoder_->HoldDecode();
         ReadUntilPending();
         break;
 
@@ -322,6 +334,7 @@ class VideoFrameStreamTest
   bool pending_stop_;
   int total_bytes_decoded_;
   scoped_refptr<VideoFrame> frame_read_;
+  VideoFrameStream::Status last_read_status_;
 
   // Decryptor has no key to decrypt a frame.
   bool has_no_key_;
@@ -330,14 +343,33 @@ class VideoFrameStreamTest
   DISALLOW_COPY_AND_ASSIGN(VideoFrameStreamTest);
 };
 
-INSTANTIATE_TEST_CASE_P(Clear, VideoFrameStreamTest,
-  testing::Combine(testing::Values(false), testing::Values(false)));
-INSTANTIATE_TEST_CASE_P(Clear_GetDecodeOutput, VideoFrameStreamTest,
-  testing::Combine(testing::Values(false), testing::Values(true)));
-INSTANTIATE_TEST_CASE_P(Encrypted, VideoFrameStreamTest,
-  testing::Combine(testing::Values(true), testing::Values(false)));
-INSTANTIATE_TEST_CASE_P(Encrypted_GetDecodeOutput, VideoFrameStreamTest,
-  testing::Combine(testing::Values(true), testing::Values(true)));
+INSTANTIATE_TEST_CASE_P(
+    Clear,
+    VideoFrameStreamTest,
+    ::testing::Values(
+        VideoFrameStreamTestParams(false, false, kDecodingDelay, 1)));
+INSTANTIATE_TEST_CASE_P(
+    Clear_GetDecodeOutput,
+    VideoFrameStreamTest,
+    ::testing::Values(
+        VideoFrameStreamTestParams(false, true, kDecodingDelay, 1)));
+INSTANTIATE_TEST_CASE_P(
+    Encrypted,
+    VideoFrameStreamTest,
+    ::testing::Values(
+        VideoFrameStreamTestParams(true, false, kDecodingDelay, 1)));
+INSTANTIATE_TEST_CASE_P(
+    Encrypted_GetDecodeOutput,
+    VideoFrameStreamTest,
+    ::testing::Values(
+        VideoFrameStreamTestParams(true, true, kDecodingDelay, 1)));
+
+INSTANTIATE_TEST_CASE_P(
+    Clear_Parallel,
+    VideoFrameStreamTest,
+    ::testing::Values(
+        VideoFrameStreamTestParams(true, false, 0, 3)));

[xhwang, 2014/04/25 00:36:03]

Add a comment that we don't support parallel decode with decoding delays.

[Sergey Ulanov, 2014/04/26 00:59:29]

There is nothing that disallows a delay in parallel decoders. Added a test with
kDecodingDelay and parallel_decoding = 3

+
 
 TEST_P(VideoFrameStreamTest, Initialization) {
   Initialize();
@@ -366,6 +398,23 @@ TEST_P(VideoFrameStreamTest, Read_AfterReset) {
   Read();
 }
 
+TEST_P(VideoFrameStreamTest, Read_BlockedDemuxer) {
+  Initialize();
+  Reset();

[xhwang, 2014/04/25 00:36:03]

Is this Reset() necessary?

[Sergey Ulanov, 2014/04/26 00:59:29]

Done.

+  demuxer_stream_->HoldNextRead();
+  ReadOneFrame();
+  EXPECT_TRUE(pending_read_);
+
+  while (pending_read_) {
+    demuxer_stream_->SatisfyAndHoldRead();
+    message_loop_.RunUntilIdle();
+  }
+  ReadUntilPending();

[xhwang, 2014/04/25 00:36:03]

It'd be nice to verify that all decoded frames are read even though the demuxer
stream is blocked.

[Sergey Ulanov, 2014/04/26 00:59:29]

Done.

+  demuxer_stream_->SatisfyRead();
+  message_loop_.RunUntilIdle();
+  EXPECT_FALSE(pending_read_);
+}
+
 // No Reset() before initialization is successfully completed.
 
 TEST_P(VideoFrameStreamTest, Reset_AfterInitialization) {
@@ -449,7 +498,7 @@ TEST_P(VideoFrameStreamTest, Stop_BeforeInitialization) {
 }
 
 TEST_P(VideoFrameStreamTest, Stop_DuringSetDecryptor) {
-  if (!IsEncrypted()) {
+  if (!GetParam().is_encrypted) {
     DVLOG(1) << "SetDecryptor test only runs when the stream is encrytped.";
     return;
   }
@@ -555,4 +604,38 @@ TEST_P(VideoFrameStreamTest, Stop_AfterRead_AfterReset) {
   Stop();
 }
 
+TEST_P(VideoFrameStreamTest, DecoderErrorWhenReading) {
+  Initialize();
+  EnterPendingState(DECODER_DECODE);
+  decoder_->SimulateError();
+  message_loop_.RunUntilIdle();
+  ASSERT_FALSE(pending_read_);
+  ASSERT_EQ(VideoFrameStream::DECODE_ERROR, last_read_status_);
+}
+
+TEST_P(VideoFrameStreamTest, DecoderErrorWhenNotReading) {
+  Initialize();
+
+  decoder_->HoldDecode();
+  ReadOneFrame();
+  EXPECT_TRUE(pending_read_);
+
+  // Satisfy decode requests until we get the first frame out.
+  while (pending_read_) {
+    decoder_->SatisfySingleDecode();
+    message_loop_.RunUntilIdle();
+  }
+
+  // Trigger an error in the decoding.
+  decoder_->SimulateError();
+
+  // The error must surface from Read() as DECODE_ERROR.
+  while (last_read_status_ == VideoFrameStream::OK) {
+    ReadOneFrame();
+    message_loop_.RunUntilIdle();
+    EXPECT_FALSE(pending_read_);
+  }
+  EXPECT_EQ(VideoFrameStream::DECODE_ERROR, last_read_status_);
+}
+
 }  // namespace media