[Fuchsia] Fix EOS handling in FuchsiaVideoDecoder

FuchsiaVideoDecoder wasn't handling the end-of-stream correctly. The
DecodeCB for EOS buffer was called out of order when there are other
pending Decode requests. Also DCHECKs were failing.

Also updated unittests to detect this issue.

Bug: 997853
Change-Id: Iae04250521e929d8ad220ead08d701f274a15979
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1810397
Auto-Submit: Sergey Ulanov <sergeyu@chromium.org>
Commit-Queue: Yuchen Liu <yucliu@chromium.org>
Reviewed-by: Yuchen Liu <yucliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697672}

media/filters/fuchsia/fuchsia_video_decoder.cc

@@ -276,9 +276,6 @@ class FuchsiaVideoDecoder : public VideoDecoder,
   int num_used_output_buffers_ = 0;
   int max_used_output_buffers_ = 0;
 
-  // Non-null when flush is pending.
-  VideoDecoder::DecodeCB pending_flush_cb_;
-
   base::WeakPtr<FuchsiaVideoDecoder> weak_this_;
   base::WeakPtrFactory<FuchsiaVideoDecoder> weak_factory_;
 
@@ -591,13 +588,6 @@ void FuchsiaVideoDecoder::ProcessEndOfStream() {
   active_stream_ = true;
   codec_->QueueInputEndOfStream(stream_lifetime_ordinal_);
   codec_->FlushEndOfStreamAndCloseStream(stream_lifetime_ordinal_);
-
-  DCHECK(!decode_callbacks_.empty());
-  pending_flush_cb_ = std::move(decode_callbacks_.front());
-  decode_callbacks_.pop_front();
-
-  // Decode() is not supposed to be called after EOF.
-  DCHECK(decode_callbacks_.empty());
 }
 
 void FuchsiaVideoDecoder::OnFreeInputPacket(
@@ -865,7 +855,12 @@ void FuchsiaVideoDecoder::OnOutputEndOfStream(uint64_t stream_lifetime_ordinal,
   stream_lifetime_ordinal_ += 2;
   active_stream_ = false;
 
-  std::move(pending_flush_cb_).Run(DecodeStatus::OK);
+  // Decode() is not supposed to be called after EOF.
+  DCHECK_EQ(decode_callbacks_.size(), 1U);
+  auto flush_cb = std::move(decode_callbacks_.front());
+  decode_callbacks_.pop_front();
+
+  std::move(flush_cb).Run(DecodeStatus::OK);
 }
 
 void FuchsiaVideoDecoder::OnError() {

media/filters/fuchsia/fuchsia_video_decoder_unittest.cc

@@ -215,29 +215,39 @@ class FuchsiaVideoDecoderTest : public testing::Test {
     while (output_frames_.size() > frames_to_keep_) {
       output_frames_.pop_front();
     }
-    if (on_frame_)
-      on_frame_.Run();
+    if (run_loop_)
+      run_loop_->Quit();
   }
 
-  DecodeStatus DecodeBuffer(scoped_refptr<DecoderBuffer> buffer) {
-    base::RunLoop run_loop;
-    DecodeStatus status;
-    decoder_->Decode(buffer,
-                     base::BindRepeating(
-                         [](DecodeStatus* status, base::RunLoop* run_loop,
-                            DecodeStatus result) {
-                           *status = result;
-                           run_loop->Quit();
-                         },
-                         &status, &run_loop));
+  void DecodeBuffer(scoped_refptr<DecoderBuffer> buffer) {
+    decoder_->Decode(buffer, base::BindRepeating(
+                                 &FuchsiaVideoDecoderTest::OnFrameDecoded,
+                                 base::Unretained(this), num_input_buffers_));
+    num_input_buffers_ += 1;
+  }
 
-    run_loop.Run();
+  void ReadAndDecodeFrame(const std::string& name) {
+    DecodeBuffer(ReadTestDataFile(name));
+  }
 
-    return status;
+  void OnFrameDecoded(size_t frame_pos, DecodeStatus status) {
+    EXPECT_EQ(frame_pos, num_decoded_buffers_);
+    num_decoded_buffers_ += 1;
+    last_decode_status_ = status;
+    if (run_loop_)
+      run_loop_->Quit();
   }
 
-  DecodeStatus ReadAndDecodeFrame(const std::string& name) {
-    return DecodeBuffer(ReadTestDataFile(name));
+  // Waits until all pending decode requests are finished.
+  void WaitDecodeDone() {
+    size_t target_pos = num_input_buffers_;
+    while (num_decoded_buffers_ < target_pos) {
+      base::RunLoop run_loop;
+      run_loop_ = &run_loop;
+      run_loop.Run();
+      run_loop_ = nullptr;
+      ASSERT_EQ(last_decode_status_, DecodeStatus::OK);
+    }
   }
 
  protected:
@@ -249,13 +259,20 @@ class FuchsiaVideoDecoderTest : public testing::Test {
 
   std::unique_ptr<VideoDecoder> decoder_;
 
+  size_t num_input_buffers_ = 0;
+
+  // Number of frames for which DecodeCB has been called. That doesn't mean
+  // we've received corresponding output frames.
+  size_t num_decoded_buffers_ = 0;
+
   std::list<scoped_refptr<VideoFrame>> output_frames_;
-  int num_output_frames_ = 0;
+  size_t num_output_frames_ = 0;
+
+  DecodeStatus last_decode_status_ = DecodeStatus::OK;
+  base::RunLoop* run_loop_ = nullptr;
 
   // Number of frames that OnVideoFrame() should keep in |output_frames_|.
   size_t frames_to_keep_ = 2;
-
-  base::RepeatingClosure on_frame_;
 };
 
 TEST_F(FuchsiaVideoDecoderTest, CreateAndDestroy) {}
@@ -267,24 +284,26 @@ TEST_F(FuchsiaVideoDecoderTest, CreateInitDestroy) {
 TEST_F(FuchsiaVideoDecoderTest, DISABLED_VP9) {
   ASSERT_TRUE(Initialize(TestVideoConfig::Normal(kCodecVP9)));
 
-  ASSERT_TRUE(ReadAndDecodeFrame("vp9-I-frame-320x240") == DecodeStatus::OK);
-  ASSERT_TRUE(DecodeBuffer(DecoderBuffer::CreateEOSBuffer()) ==
-              DecodeStatus::OK);
+  ReadAndDecodeFrame("vp9-I-frame-320x240");
+  DecodeBuffer(DecoderBuffer::CreateEOSBuffer());
+  ASSERT_NO_FATAL_FAILURE(WaitDecodeDone());
 
-  EXPECT_EQ(num_output_frames_, 1);
+  EXPECT_EQ(num_output_frames_, 1U);
 }
 
 TEST_F(FuchsiaVideoDecoderTest, H264) {
   ASSERT_TRUE(Initialize(TestVideoConfig::NormalH264()));
 
-  ASSERT_TRUE(ReadAndDecodeFrame("h264-320x180-frame-0") == DecodeStatus::OK);
-  ASSERT_TRUE(ReadAndDecodeFrame("h264-320x180-frame-1") == DecodeStatus::OK);
-  ASSERT_TRUE(ReadAndDecodeFrame("h264-320x180-frame-2") == DecodeStatus::OK);
-  ASSERT_TRUE(ReadAndDecodeFrame("h264-320x180-frame-3") == DecodeStatus::OK);
-  ASSERT_TRUE(DecodeBuffer(DecoderBuffer::CreateEOSBuffer()) ==
-              DecodeStatus::OK);
+  ReadAndDecodeFrame("h264-320x180-frame-0");
+  ReadAndDecodeFrame("h264-320x180-frame-1");
+  ASSERT_NO_FATAL_FAILURE(WaitDecodeDone());
+
+  ReadAndDecodeFrame("h264-320x180-frame-2");
+  ReadAndDecodeFrame("h264-320x180-frame-3");
+  DecodeBuffer(DecoderBuffer::CreateEOSBuffer());
+  ASSERT_NO_FATAL_FAILURE(WaitDecodeDone());
 
-  EXPECT_EQ(num_output_frames_, 4);
+  EXPECT_EQ(num_output_frames_, 4U);
 }
 
 }  // namespace media