Ensure the codec loop is pumped after EOS while waiting for outputs.

With the async API, MediaCodec will stop sending callbacks once an
EOS buffer has been enqueued. Our MediaCodec based decoders give
up their timer for the async API, so without these notifications
we can end up in a hung state.

This modifies CodecWrapper::output_buffer_release_cb to include the
draining||drained state and then has MediaCodecVideoDecoder make an
informed decision to elide the PumpCodec() if possible.

BUG=868670
TEST=https://shaka-player-demo.appspot.com/demo/#asset=https://demo.unified-streaming.com/video/tears-of-steel/tears-of-steel.ism/.mpd;lang=en-US;build=compiled
will hang for multiple seconds without patch between adaptations.

Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: I442b3ecdeaa5e87c28002f3cf45a905d31fd6d2e
Reviewed-on: https://chromium-review.googlesource.com/1155640Reviewed-by: Frank Liberato <liberato@chromium.org>
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#579218}

media/gpu/android/codec_wrapper.cc

@@ -24,7 +24,7 @@ namespace media {
 class CodecWrapperImpl : public base::RefCountedThreadSafe<CodecWrapperImpl> {
  public:
   CodecWrapperImpl(CodecSurfacePair codec_surface_pair,
-                   base::Closure output_buffer_release_cb);
+                   CodecWrapper::OutputReleasedCB output_buffer_release_cb);
 
   using DequeueStatus = CodecWrapper::DequeueStatus;
   using QueueStatus = CodecWrapper::QueueStatus;
@@ -89,7 +89,7 @@ class CodecWrapperImpl : public base::RefCountedThreadSafe<CodecWrapperImpl> {
 
   // A callback that's called whenever an output buffer is released back to the
   // codec.
-  base::Closure output_buffer_release_cb_;
+  CodecWrapper::OutputReleasedCB output_buffer_release_cb_;
 
   // Do we owe the client an EOS in DequeueOutput, due to an eos that we elided
   // while we're already flushed?
@@ -111,8 +111,9 @@ bool CodecOutputBuffer::ReleaseToSurface() {
   return codec_->ReleaseCodecOutputBuffer(id_, true);
 }
 
-CodecWrapperImpl::CodecWrapperImpl(CodecSurfacePair codec_surface_pair,
-                                   base::Closure output_buffer_release_cb)
+CodecWrapperImpl::CodecWrapperImpl(
+    CodecSurfacePair codec_surface_pair,
+    CodecWrapper::OutputReleasedCB output_buffer_release_cb)
     : state_(State::kFlushed),
       codec_(std::move(codec_surface_pair.first)),
       surface_bundle_(std::move(codec_surface_pair.second)),
@@ -386,13 +387,15 @@ bool CodecWrapperImpl::ReleaseCodecOutputBuffer(int64_t id, bool render) {
   int index = buffer_it->second;
   codec_->ReleaseOutputBuffer(index, render);
   buffer_ids_.erase(buffer_ids_.begin(), buffer_it + 1);
-  if (output_buffer_release_cb_)
-    output_buffer_release_cb_.Run();
+  if (output_buffer_release_cb_) {
+    output_buffer_release_cb_.Run(state_ == State::kDrained ||
+                                  state_ == State::kDraining);
+  }
   return true;
 }
 
 CodecWrapper::CodecWrapper(CodecSurfacePair codec_surface_pair,
-                           base::Closure output_buffer_release_cb)
+                           OutputReleasedCB output_buffer_release_cb)
     : impl_(new CodecWrapperImpl(std::move(codec_surface_pair),
                                  std::move(output_buffer_release_cb))) {}
 

media/gpu/android/codec_wrapper.h

@@ -68,8 +68,12 @@ class MEDIA_GPU_EXPORT CodecWrapper {
   // released back to the codec (whether it's rendered or not). This is a signal
   // that the codec might be ready to accept more input. It may be run on any
   // thread.
+  //
+  // OutputReleasedCB will be called with a bool indicating if CodecWrapper is
+  // currently draining or in the drained state.
+  using OutputReleasedCB = base::RepeatingCallback<void(bool)>;
   CodecWrapper(CodecSurfacePair codec_surface_pair,
-               base::Closure output_buffer_release_cb);
+               OutputReleasedCB output_buffer_release_cb);
   ~CodecWrapper();
 
   // Takes the backing codec and surface, implicitly discarding all outstanding

media/gpu/android/codec_wrapper_unittest.cc

@@ -63,7 +63,8 @@ class CodecWrapperTest : public testing::Test {
   NiceMock<MockMediaCodecBridge>* codec_;
   std::unique_ptr<CodecWrapper> wrapper_;
   scoped_refptr<AVDASurfaceBundle> surface_bundle_;
-  NiceMock<base::MockCallback<base::Closure>> output_buffer_release_cb_;
+  NiceMock<base::MockCallback<CodecWrapper::OutputReleasedCB>>
+      output_buffer_release_cb_;
   scoped_refptr<DecoderBuffer> fake_decoder_buffer_;
 };
 
@@ -211,13 +212,22 @@ TEST_F(CodecWrapperTest, CodecOutputBuffersHaveTheCorrectSize) {
 
 TEST_F(CodecWrapperTest, OutputBufferReleaseCbIsCalledWhenRendering) {
   auto codec_buffer = DequeueCodecOutputBuffer();
-  EXPECT_CALL(output_buffer_release_cb_, Run()).Times(1);
+  EXPECT_CALL(output_buffer_release_cb_, Run(false)).Times(1);
   codec_buffer->ReleaseToSurface();
 }
 
 TEST_F(CodecWrapperTest, OutputBufferReleaseCbIsCalledWhenDestructing) {
   auto codec_buffer = DequeueCodecOutputBuffer();
-  EXPECT_CALL(output_buffer_release_cb_, Run()).Times(1);
+  EXPECT_CALL(output_buffer_release_cb_, Run(false)).Times(1);
+}
+
+TEST_F(CodecWrapperTest, OutputBufferReflectsDrainingOrDrainedStatus) {
+  wrapper_->QueueInputBuffer(*fake_decoder_buffer_, EncryptionScheme());
+  auto eos = DecoderBuffer::CreateEOSBuffer();
+  wrapper_->QueueInputBuffer(*eos, EncryptionScheme());
+  ASSERT_TRUE(wrapper_->IsDraining());
+  auto codec_buffer = DequeueCodecOutputBuffer();
+  EXPECT_CALL(output_buffer_release_cb_, Run(true)).Times(1);
 }
 
 TEST_F(CodecWrapperTest, CodecStartsInFlushedState) {

media/gpu/android/media_codec_video_decoder.cc

@@ -89,6 +89,16 @@ bool ConfigSupported(const VideoDecoderConfig& config,
   }
 }
 
+void OutputBufferReleased(bool using_async_api,
+                          base::RepeatingClosure pump_cb,
+                          bool is_drained_or_draining) {
+  // The asynchronous API doesn't need pumping upon calls to ReleaseOutputBuffer
+  // unless we're draining or drained.
+  if (using_async_api && !is_drained_or_draining)
+    return;
+  pump_cb.Run();
+}
+
 }  // namespace
 
 // static
@@ -455,12 +465,10 @@ void MediaCodecVideoDecoder::OnCodecConfigured(
 
   codec_ = std::make_unique<CodecWrapper>(
       CodecSurfacePair(std::move(codec), std::move(surface_bundle)),
-      // The async API will always get an OnBuffersAvailable callback once an
-      // output buffer is available, so we don't need to manually pump here.
-      using_async_api_ ? base::RepeatingClosure()
-                       : BindToCurrentLoop(base::BindRepeating(
-                             &MediaCodecVideoDecoder::StartTimerOrPumpCodec,
-                             weak_factory_.GetWeakPtr())));
+      base::BindRepeating(&OutputBufferReleased, using_async_api_,
+                          BindToCurrentLoop(base::BindRepeating(
+                              &MediaCodecVideoDecoder::StartTimerOrPumpCodec,
+                              weak_factory_.GetWeakPtr()))));
 
   // If the target surface changed while codec creation was in progress,
   // transition to it immediately.