Fix MediaCodec's ERROR_NO_KEY processing in AVDA

When MediaCodec.queueSecureInputBuffer() returns with NO_KEY
error, the input buffer is still owned by the client (i.e.
not by MediaCodec) and is filled with proper input data.

The correct behavior is to keep trying to queue it using
the same input buffer index and not dequeue new input buffers,
otherwise the input buffers pool depletes and might end,
in which case the MediaCodec would be blocked.

In addition, this CL assumes that OnKeyAdded() will be called
after we get the NO_KEY error and retries the queuing attempt
only after the OnKeyAdded() call.

BUG=577416

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

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

3 files changed, 75 insertions, 30 deletions

diff --git a/content/common/gpu/media/android_video_decode_accelerator.cc b/content/common/gpu/media/android_video_decode_accelerator.cc
index 832d8dc..dfe0fc5 100644
--- a/content/common/gpu/media/android_video_decode_accelerator.cc
+++ b/content/common/gpu/media/android_video_decode_accelerator.cc
@@ -155,6 +155,7 @@ AndroidVideoDecodeAccelerator::AndroidVideoDecodeAccelerator(
       picturebuffers_requested_(false),
       gl_decoder_(decoder),
       cdm_registration_id_(0),
+      pending_input_buf_index_(-1),
       weak_this_factory_(this) {
   if (UseDeferredRenderingStrategy())
     strategy_.reset(new AndroidDeferredRenderingBackingStrategy());
@@ -303,18 +304,32 @@ bool AndroidVideoDecodeAccelerator::QueueInput() {
     return false;
   if (pending_bitstream_buffers_.empty())
     return false;
+  if (state_ == WAITING_FOR_KEY)
+    return false;
 
-  int input_buf_index = 0;
-  media::MediaCodecStatus status =
-      media_codec_->DequeueInputBuffer(NoWaitTimeOut(), &input_buf_index);
+  int input_buf_index = pending_input_buf_index_;
 
-  if (status == media::MEDIA_CODEC_DEQUEUE_INPUT_AGAIN_LATER)
-    return false;
-  if (status == media::MEDIA_CODEC_ERROR) {
-    POST_ERROR(PLATFORM_FAILURE, "Failed to DequeueInputBuffer");
-    return false;
+  // Do not dequeue a new input buffer if we failed with MEDIA_CODEC_NO_KEY.
+  // That status does not return this buffer back to the pool of
+  // available input buffers. We have to reuse it in QueueSecureInputBuffer().
+  if (input_buf_index == -1) {
+    media::MediaCodecStatus status =
+        media_codec_->DequeueInputBuffer(NoWaitTimeOut(), &input_buf_index);
+    switch (status) {
+      case media::MEDIA_CODEC_DEQUEUE_INPUT_AGAIN_LATER:
+        return false;
+      case media::MEDIA_CODEC_ERROR:
+        POST_ERROR(PLATFORM_FAILURE, "Failed to DequeueInputBuffer");
+        return false;
+      case media::MEDIA_CODEC_OK:
+        break;
+      default:
+        NOTREACHED() << "Unknown DequeueInputBuffer status " << status;
+        return false;
+    }
   }
-  DCHECK_EQ(status, media::MEDIA_CODEC_OK);
+
+  DCHECK_NE(input_buf_index, -1);
 
   base::Time queued_time = pending_bitstream_buffers_.front().second;
   UMA_HISTOGRAM_TIMES("Media.AVDA.InputQueueTime",
@@ -331,11 +346,19 @@ bool AndroidVideoDecodeAccelerator::QueueInput() {
     return true;
   }
 
-  scoped_ptr<base::SharedMemory> shm(
-      new base::SharedMemory(bitstream_buffer.handle(), true));
-  if (!shm->Map(bitstream_buffer.size())) {
-    POST_ERROR(UNREADABLE_INPUT, "Failed to SharedMemory::Map()");
-    return false;
+  scoped_ptr<base::SharedMemory> shm;
+
+  if (pending_input_buf_index_ != -1) {
+    // The buffer is already dequeued from MediaCodec, filled with data and
+    // bitstream_buffer.handle() is closed.
+    shm.reset(new base::SharedMemory());
+  } else {
+    shm.reset(new base::SharedMemory(bitstream_buffer.handle(), true));
+
+    if (!shm->Map(bitstream_buffer.size())) {
+      POST_ERROR(UNREADABLE_INPUT, "Failed to SharedMemory::Map()");
+      return false;
+    }
   }
 
   const base::TimeDelta presentation_timestamp =
@@ -351,12 +374,15 @@ bool AndroidVideoDecodeAccelerator::QueueInput() {
   // result in them finding the right timestamp.
   bitstream_buffers_in_decoder_[presentation_timestamp] = bitstream_buffer.id();
 
+  // Notice that |memory| will be null if we repeatedly enqueue the same buffer,
+  // this happens after MEDIA_CODEC_NO_KEY.
   const uint8_t* memory = static_cast<const uint8_t*>(shm->memory());
   const std::string& key_id = bitstream_buffer.key_id();
   const std::string& iv = bitstream_buffer.iv();
   const std::vector<media::SubsampleEntry>& subsamples =
       bitstream_buffer.subsamples();
 
+  media::MediaCodecStatus status;
   if (key_id.empty() || iv.empty()) {
     status = media_codec_->QueueInputBuffer(input_buf_index, memory,
                                             bitstream_buffer.size(),
@@ -372,15 +398,15 @@ bool AndroidVideoDecodeAccelerator::QueueInput() {
            << " status:" << status;
 
   if (status == media::MEDIA_CODEC_NO_KEY) {
-    // Keep trying to enqueue the front pending buffer.
-    //
-    // TODO(timav): Figure out whether stopping the pipeline in response to
-    // this error and restarting it in OnKeyAdded() has significant benefits
-    // (e.g. saving power).
+    // Keep trying to enqueue the same input buffer.
+    // The buffer is owned by us (not the MediaCodec) and is filled with data.
     DVLOG(1) << "QueueSecureInputBuffer failed: NO_KEY";
-    return true;
+    pending_input_buf_index_ = input_buf_index;
+    state_ = WAITING_FOR_KEY;
+    return false;
   }
 
+  pending_input_buf_index_ = -1;
   pending_bitstream_buffers_.pop();
   TRACE_COUNTER1("media", "AVDA::PendingBitstreamBufferCount",
                  pending_bitstream_buffers_.size());
@@ -432,10 +458,6 @@ bool AndroidVideoDecodeAccelerator::DequeueOutput() {
                      "presentation_timestamp (ms)",
                      presentation_timestamp.InMilliseconds());
 
-    DVLOG(3) << "AVDA::DequeueOutput: pts:" << presentation_timestamp
-             << " buf_index:" << buf_index << " offset:" << offset
-             << " size:" << size << " eos:" << eos;
-
     switch (status) {
       case media::MEDIA_CODEC_ERROR:
         POST_ERROR(PLATFORM_FAILURE, "DequeueOutputBuffer failed.");
@@ -469,6 +491,9 @@ bool AndroidVideoDecodeAccelerator::DequeueOutput() {
 
       case media::MEDIA_CODEC_OK:
         DCHECK_GE(buf_index, 0);
+        DVLOG(3) << "AVDA::DequeueOutput: pts:" << presentation_timestamp
+                 << " buf_index:" << buf_index << " offset:" << offset
+                 << " size:" << size << " eos:" << eos;
         break;
 
       default:
@@ -688,6 +713,18 @@ void AndroidVideoDecodeAccelerator::ResetCodecState() {
   // all the output buffers, so we must be sure that the strategy no longer
   // refers to them.
 
+  if (pending_input_buf_index_ != -1) {
+    // The data for that index exists in the input buffer, but corresponding
+    // shm block been deleted. Check that it is safe to flush the coec, i.e.
+    // |pending_bitstream_buffers_| is empty.
+    // TODO(timav): keep shm block for that buffer and remove this restriction.
+    DCHECK(pending_bitstream_buffers_.empty());
+    pending_input_buf_index_ = -1;
+  }
+
+  if (state_ == WAITING_FOR_KEY)
+    state_ = NO_ERROR;
+
   // When codec is not in error state we can quickly reset (internally calls
   // flush()) for JB-MR2 and beyond. Prior to JB-MR2, flush() had several bugs
   // (b/8125974, b/8347958) so we must stop() and reconfigure MediaCodec. The
@@ -835,9 +872,11 @@ void AndroidVideoDecodeAccelerator::OnMediaCryptoReady(
 
 void AndroidVideoDecodeAccelerator::OnKeyAdded() {
   DVLOG(1) << __FUNCTION__;
-  // TODO(timav): Figure out whether stopping the pipeline in response to
-  // NO_KEY error and restarting it here has significant benefits (e.g. saving
-  // power). Right now do nothing here.
+
+  if (state_ == WAITING_FOR_KEY)
+    state_ = NO_ERROR;
+
+  DoIOTask();
 }
 
 void AndroidVideoDecodeAccelerator::NotifyCdmAttached(bool success) {
diff --git a/content/common/gpu/media/android_video_decode_accelerator.h b/content/common/gpu/media/android_video_decode_accelerator.h
index 1dd6816..3f20037 100644
--- a/content/common/gpu/media/android_video_decode_accelerator.h
+++ b/content/common/gpu/media/android_video_decode_accelerator.h
@@ -125,9 +125,11 @@ class CONTENT_EXPORT AndroidVideoDecodeAccelerator
   void OnFrameAvailable();
 
  private:
+  // TODO(timav): evaluate the need for more states in the AVDA state machine.
   enum State {
     NO_ERROR,
     ERROR,
+    WAITING_FOR_KEY,
   };
 
   static const base::TimeDelta kDecodePollDelay;
@@ -294,6 +296,10 @@ class CONTENT_EXPORT AndroidVideoDecodeAccelerator
   // an encrypted stream.
   media::MediaDrmBridge::JavaObjectPtr media_crypto_;
 
+  // Index of the dequeued and filled buffer that we keep trying to enqueue.
+  // Such buffer appears in MEDIA_CODEC_NO_KEY processing.
+  int pending_input_buf_index_;
+
   // WeakPtrFactory for posting tasks back to |this|.
   base::WeakPtrFactory<AndroidVideoDecodeAccelerator> weak_this_factory_;
 
diff --git a/media/base/android/java/src/org/chromium/media/MediaCodecBridge.java b/media/base/android/java/src/org/chromium/media/MediaCodecBridge.java
index a6fe0a8..e62927b 100644
--- a/media/base/android/java/src/org/chromium/media/MediaCodecBridge.java
+++ b/media/base/android/java/src/org/chromium/media/MediaCodecBridge.java
@@ -351,12 +351,12 @@ class MediaCodecBridge {
                     keyId, iv, MediaCodec.CRYPTO_MODE_AES_CTR);
             mMediaCodec.queueSecureInputBuffer(index, offset, cryptoInfo, presentationTimeUs, 0);
         } catch (MediaCodec.CryptoException e) {
-            Log.e(TAG, "Failed to queue secure input buffer", e);
             if (e.getErrorCode() == MediaCodec.CryptoException.ERROR_NO_KEY) {
-                Log.e(TAG, "MediaCodec.CryptoException.ERROR_NO_KEY");
+                Log.d(TAG, "Failed to queue secure input buffer: CryptoException.ERROR_NO_KEY");
                 return MEDIA_CODEC_NO_KEY;
             }
-            Log.e(TAG, "MediaCodec.CryptoException with error code " + e.getErrorCode());
+            Log.e(TAG, "Failed to queue secure input buffer, CryptoException with error code "
+                            + e.getErrorCode());
             return MEDIA_CODEC_ERROR;
         } catch (IllegalStateException e) {
             Log.e(TAG, "Failed to queue secure input buffer, IllegalStateException " + e);