Don't try to decode more than one EOS at a time.

BUG=405642

Review URL: https://codereview.chromium.org/490033002

Cr-Commit-Position: refs/heads/master@{#292268}

5 files changed, 58 insertions, 5 deletions

diff --git a/media/base/video_decoder.h b/media/base/video_decoder.h
index edca238..89c62db 100644
--- a/media/base/video_decoder.h
+++ b/media/base/video_decoder.h
@@ -69,7 +69,7 @@ class MEDIA_EXPORT VideoDecoder {
   //
   // Implementations guarantee that the callback will not be called from within
   // this method and that |decode_cb| will not be blocked on the following
-  // Decode() calls (i.e. |decode_cb| will be called even Decode() is never
+  // Decode() calls (i.e. |decode_cb| will be called even if Decode() is never
   // called again).
   //
   // After decoding is finished the decoder calls |output_cb| specified in
@@ -78,7 +78,8 @@ class MEDIA_EXPORT VideoDecoder {
   //
   // If |buffer| is an EOS buffer then the decoder must be flushed, i.e.
   // |output_cb| must be called for each frame pending in the queue and
-  // |decode_cb| must be called after that.
+  // |decode_cb| must be called after that. Callers will not call Decode()
+  // again until after the flush completes.
   virtual void Decode(const scoped_refptr<DecoderBuffer>& buffer,
                       const DecodeCB& decode_cb) = 0;
 
diff --git a/media/filters/decoder_stream.cc b/media/filters/decoder_stream.cc
index bd5cdb7..b3d58ae 100644
--- a/media/filters/decoder_stream.cc
+++ b/media/filters/decoder_stream.cc
@@ -52,6 +52,7 @@ DecoderStream<StreamType>::DecoderStream(
                                           decoders.Pass(),
                                           set_decryptor_ready_cb)),
       active_splice_(false),
+      decoding_eos_(false),
       pending_decode_requests_(0),
       weak_factory_(this) {}
 
@@ -207,7 +208,7 @@ bool DecoderStream<StreamType>::CanDecodeMore() const {
   // empty.
   int num_decodes =
       static_cast<int>(ready_outputs_.size()) + pending_decode_requests_;
-  return num_decodes < GetMaxDecodeRequests();
+  return !decoding_eos_ && num_decodes < GetMaxDecodeRequests();
 }
 
 template <DemuxerStream::Type StreamType>
@@ -260,6 +261,10 @@ void DecoderStream<StreamType>::Decode(
   int buffer_size = buffer->end_of_stream() ? 0 : buffer->data_size();
 
   TRACE_EVENT_ASYNC_BEGIN0("media", GetTraceString<StreamType>(), this);
+
+  if (buffer->end_of_stream())
+    decoding_eos_ = true;
+
   ++pending_decode_requests_;
   decoder_->Decode(buffer,
                    base::Bind(&DecoderStream<StreamType>::OnDecodeDone,
@@ -287,6 +292,9 @@ void DecoderStream<StreamType>::OnDecodeDone(int buffer_size,
 
   TRACE_EVENT_ASYNC_END0("media", GetTraceString<StreamType>(), this);
 
+  if (end_of_stream)
+    decoding_eos_ = false;
+
   if (state_ == STATE_ERROR) {
     DCHECK(read_cb_.is_null());
     return;
@@ -449,7 +457,7 @@ void DecoderStream<StreamType>::OnBufferReady(
   Decode(buffer);
 
   // Read more data if the decoder supports multiple parallel decoding requests.
-  if (CanDecodeMore() && !buffer->end_of_stream())
+  if (CanDecodeMore())
     ReadFromDemuxerStream();
 }
 
diff --git a/media/filters/decoder_stream.h b/media/filters/decoder_stream.h
index d6ee126..5afd0b8 100644
--- a/media/filters/decoder_stream.h
+++ b/media/filters/decoder_stream.h
@@ -183,6 +183,11 @@ class MEDIA_EXPORT DecoderStream {
   // splice_timestamp() of kNoTimestamp() is encountered.
   bool active_splice_;
 
+  // An end-of-stream buffer has been sent for decoding, no more buffers should
+  // be sent for decoding until it completes.
+  // TODO(sandersd): Turn this into a State. http://crbug.com/408316
+  bool decoding_eos_;
+
   // Decoded buffers that haven't been read yet. Used when the decoder supports
   // parallel decoding.
   std::list<scoped_refptr<Output> > ready_outputs_;
diff --git a/media/filters/fake_video_decoder.cc b/media/filters/fake_video_decoder.cc
index 05dc410..cf26ed5 100644
--- a/media/filters/fake_video_decoder.cc
+++ b/media/filters/fake_video_decoder.cc
@@ -74,6 +74,7 @@ void FakeVideoDecoder::Decode(const scoped_refptr<DecoderBuffer>& buffer,
             decoding_delay_ + held_decode_callbacks_.size());
   DCHECK_LT(static_cast<int>(held_decode_callbacks_.size()),
             max_parallel_decoding_requests_);
+  DCHECK_NE(state_, STATE_END_OF_STREAM);
 
   int buffer_size = buffer->end_of_stream() ? 0 : buffer->data_size();
   DecodeCB wrapped_decode_cb = base::Bind(&FakeVideoDecoder::OnFrameDecoded,
@@ -223,6 +224,7 @@ void FakeVideoDecoder::RunDecodeCallback(const DecodeCB& decode_cb) {
         output_cb_.Run(decoded_frames_.front());
         decoded_frames_.pop_front();
       }
+      state_ = STATE_NORMAL;
     } else if (!decoded_frames_.empty()) {
       output_cb_.Run(decoded_frames_.front());
       decoded_frames_.pop_front();
diff --git a/media/filters/video_frame_stream_unittest.cc b/media/filters/video_frame_stream_unittest.cc
index 96848e4..e36172c 100644
--- a/media/filters/video_frame_stream_unittest.cc
+++ b/media/filters/video_frame_stream_unittest.cc
@@ -372,6 +372,7 @@ INSTANTIATE_TEST_CASE_P(
         VideoFrameStreamTestParams(false, 0, 1),
         VideoFrameStreamTestParams(false, 3, 1),
         VideoFrameStreamTestParams(false, 7, 1)));
+
 INSTANTIATE_TEST_CASE_P(
     Encrypted,
     VideoFrameStreamTest,
@@ -488,8 +489,44 @@ TEST_P(VideoFrameStreamTest, Read_BlockedDemuxerAndDecoder) {
   EXPECT_FALSE(pending_read_);
 }
 
-// No Reset() before initialization is successfully completed.
+TEST_P(VideoFrameStreamTest, Read_DuringEndOfStreamDecode) {
+  // Test applies only when the decoder allows multiple parallel requests, and
+  // they are not satisfied in a single batch.
+  if (GetParam().parallel_decoding == 1 || GetParam().decoding_delay != 0)
+    return;
+
+  Initialize();
+  decoder_->HoldDecode();
+
+  // Read all of the frames up to end of stream. Since parallel decoding is
+  // enabled, the end of stream buffer will be sent to the decoder immediately,
+  // but we don't satisfy it yet.
+  for (int configuration = 0; configuration < kNumConfigs; configuration++) {
+    for (int frame = 0; frame < kNumBuffersInOneConfig; frame++) {
+      ReadOneFrame();
+      while (pending_read_) {
+        decoder_->SatisfySingleDecode();
+        message_loop_.RunUntilIdle();
+      }
+    }
+  }
+
+  // Read() again. The callback must be delayed until the decode completes.
+  ReadOneFrame();
+  ASSERT_TRUE(pending_read_);
+
+  // Satisfy decoding of the end of stream buffer. The read should complete.
+  decoder_->SatisfySingleDecode();
+  message_loop_.RunUntilIdle();
+  ASSERT_FALSE(pending_read_);
+  EXPECT_EQ(last_read_status_, VideoFrameStream::OK);
 
+  // The read output should indicate end of stream.
+  ASSERT_TRUE(frame_read_.get());
+  EXPECT_TRUE(frame_read_->end_of_stream());
+}
+
+// No Reset() before initialization is successfully completed.
 TEST_P(VideoFrameStreamTest, Reset_AfterInitialization) {
   Initialize();
   Reset();