media: AVDA now doesn't queue EOS for a flush if the codec is empty

media/gpu/android_video_decode_accelerator.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/gpu/android_video_decode_accelerator.h"
 
 #include <stddef.h>
 
 #include <memory>
 
@@ skipping 213 matching lines @@
 
 AndroidVideoDecodeAccelerator::AndroidVideoDecodeAccelerator(
     const MakeGLContextCurrentCallback& make_context_current_cb,
     const GetGLES2DecoderCallback& get_gles2_decoder_cb)
     : client_(NULL),
       make_context_current_cb_(make_context_current_cb),
       get_gles2_decoder_cb_(get_gles2_decoder_cb),
       state_(NO_ERROR),
       picturebuffers_requested_(false),
       picture_buffer_manager_(this),
-      drain_type_(DRAIN_TYPE_NONE),
       media_drm_bridge_cdm_context_(nullptr),
       cdm_registration_id_(0),
       pending_input_buf_index_(-1),
       deferred_initialization_pending_(false),
       codec_needs_reset_(false),
       defer_surface_creation_(false),
       weak_this_factory_(this) {}
 
 AndroidVideoDecodeAccelerator::~AndroidVideoDecodeAccelerator() {
   DCHECK(thread_checker_.CalledOnValidThread());
@@ skipping 341 matching lines @@
     MediaCodecStatus status = media_codec_->DequeueOutputBuffer(
         NoWaitTimeOut, &buf_index, &offset, &size, &presentation_timestamp,
         &eos, NULL);
     TRACE_EVENT_END2("media", "AVDA::DequeueOutput", "status", status,
                      "presentation_timestamp (ms)",
                      presentation_timestamp.InMilliseconds());
 
     switch (status) {
       case MEDIA_CODEC_ERROR:
         // Do not post an error if we are draining for reset and destroy.
-        // Instead, run the drain completion task.
+        // Instead, signal completion of the drain.
         if (IsDrainingForResetOrDestroy()) {
-          DVLOG(1) << __func__ << ": error while codec draining";
+          DVLOG(1) << __func__ << ": error while draining";
           state_ = ERROR;
           OnDrainCompleted();
         } else {
           NOTIFY_ERROR(PLATFORM_FAILURE, "DequeueOutputBuffer failed.");
         }
         return false;
 
       case MEDIA_CODEC_DEQUEUE_OUTPUT_AGAIN_LATER:
         return false;
 
@@ skipping 155 matching lines @@
 
   if (defer_surface_creation_ && !InitializePictureBufferManager()) {
     NOTIFY_ERROR(PLATFORM_FAILURE,
                  "Failed deferred surface and MediaCodec initialization.");
     return;
   }
 
   // If we previously deferred a codec restart, take care of it now. This can
   // happen on older devices where configuration changes require a codec reset.
   if (codec_needs_reset_) {
-    DCHECK_EQ(drain_type_, DRAIN_TYPE_NONE);
+    DCHECK(!drain_type_);
     ResetCodecState();
   }
 
   if (bitstream_buffer.id() >= 0 && bitstream_buffer.size() > 0) {
     DecodeBuffer(bitstream_buffer);
     return;
   }
 
   if (base::SharedMemory::IsHandleValid(bitstream_buffer.handle()))
     base::SharedMemory::CloseHandle(bitstream_buffer.handle());
@@ skipping 72 matching lines @@
     return;
   }
 
   picture_buffer_manager_.ReuseOnePictureBuffer(it->second);
   DoIOTask(true);
 }
 
 void AndroidVideoDecodeAccelerator::Flush() {
   DVLOG(1) << __func__;
   DCHECK(thread_checker_.CalledOnValidThread());
-
-  if (state_ == SURFACE_DESTROYED || defer_surface_creation_)

[watk, 2016/12/07 23:30:32]

These both imply media_codec == nullptr so I used that directly below.

-    NotifyFlushDone();
-  else
-    StartCodecDrain(DRAIN_FOR_FLUSH);
+  StartCodecDrain(DRAIN_FOR_FLUSH);
 }
 
 void AndroidVideoDecodeAccelerator::ConfigureMediaCodecAsynchronously() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   DCHECK_NE(state_, WAITING_FOR_CODEC);
   state_ = WAITING_FOR_CODEC;
 
   if (media_codec_) {
     AVDACodecAllocator::Instance()->ReleaseMediaCodec(
@@ skipping 73 matching lines @@
 
   state_ = NO_ERROR;
 
   ManageTimer(true);
 }
 
 void AndroidVideoDecodeAccelerator::StartCodecDrain(DrainType drain_type) {
   DVLOG(2) << __func__ << " drain_type:" << drain_type;
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  // We assume that DRAIN_FOR_FLUSH and DRAIN_FOR_RESET cannot come while
-  // another drain request is present, but DRAIN_FOR_DESTROY can.
-  DCHECK_NE(drain_type, DRAIN_TYPE_NONE);
-  DCHECK(drain_type_ == DRAIN_TYPE_NONE || drain_type == DRAIN_FOR_DESTROY)
-      << "Unexpected StartCodecDrain() with drain type " << drain_type
-      << " while already draining with drain type " << drain_type_;
-
-  const bool enqueue_eos = drain_type_ == DRAIN_TYPE_NONE;
+  auto previous_drain_type = drain_type_;
   drain_type_ = drain_type;
 
-  if (enqueue_eos)
+  // Only DRAIN_FOR_DESTROY is allowed while a drain is already in progress.
+  DCHECK(!previous_drain_type || drain_type == DRAIN_FOR_DESTROY)
+      << "StartCodecDrain(" << drain_type
+      << ") while already draining with type " << previous_drain_type.value();
+
+  // Skip the drain if possible. We still need to drain VP8 for destroy and
+  // reset because MediaCodec might hang in release() or flush() if we don't.

[DaleCurtis, 2016/12/08 00:11:13]

Comment is a bit misleading, code skips drain if there's nothing in it even for
vp8.

+  // http://crbug.com/598963
+  if (!media_codec_ || (pending_bitstream_records_.empty() &&

[DaleCurtis, 2016/12/08 00:11:14]

Maybe clarify that in the case of Reset() pending bitstream records is already
empty by this point?

+                        bitstream_buffers_in_decoder_.empty()) ||
+      (drain_type != DRAIN_FOR_FLUSH && codec_config_->codec != kCodecVP8)) {
+    OnDrainCompleted();
+    return;
+  }
+
+  // Queue EOS if one is not already queued.
+  if (!previous_drain_type)
     DecodeBuffer(BitstreamBuffer(-1, base::SharedMemoryHandle(), 0));
 }
 
 bool AndroidVideoDecodeAccelerator::IsDrainingForResetOrDestroy() const {
   return drain_type_ == DRAIN_FOR_RESET || drain_type_ == DRAIN_FOR_DESTROY;
 }
 
 void AndroidVideoDecodeAccelerator::OnDrainCompleted() {
   DVLOG(2) << __func__;
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  // If we were waiting for an EOS, clear the state and reset the MediaCodec
-  // as normal.
-  //
-  // Some Android platforms seem to send an EOS buffer even when we're not
-  // expecting it. In this case, destroy and reset the codec but don't notify
-  // flush done since it violates the state machine. http://crbug.com/585959.
+  // Sometimes MediaCodec returns an EOS buffer even if we didn't queue one.
+  // Consider it an error. http://crbug.com/585959.
+  if (!drain_type_) {
+    state_ = ERROR;
+    ResetCodecState();
+    return;
+  }
 
-  switch (drain_type_) {
-    case DRAIN_TYPE_NONE:
-      // Unexpected EOS.
-      state_ = ERROR;
-      ResetCodecState();
-      break;
+  ResetCodecState();
+  switch (*drain_type_) {
     case DRAIN_FOR_FLUSH:
-      ResetCodecState();
       base::ThreadTaskRunnerHandle::Get()->PostTask(
           FROM_HERE, base::Bind(&AndroidVideoDecodeAccelerator::NotifyFlushDone,
                                 weak_this_factory_.GetWeakPtr()));
       break;
     case DRAIN_FOR_RESET:
-      ResetCodecState();
       base::ThreadTaskRunnerHandle::Get()->PostTask(
           FROM_HERE, base::Bind(&AndroidVideoDecodeAccelerator::NotifyResetDone,
                                 weak_this_factory_.GetWeakPtr()));
       break;
     case DRAIN_FOR_DESTROY:
-      ResetCodecState();
       base::ThreadTaskRunnerHandle::Get()->PostTask(
           FROM_HERE, base::Bind(&AndroidVideoDecodeAccelerator::ActualDestroy,
                                 weak_this_factory_.GetWeakPtr()));
       break;
   }
-  drain_type_ = DRAIN_TYPE_NONE;
+  drain_type_.reset();
 }
 
 void AndroidVideoDecodeAccelerator::ResetCodecState() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   // If there is already a reset in flight, then that counts.  This can really
   // only happen if somebody calls Reset.
   // If the surface is destroyed there's nothing to do.
   if (state_ == WAITING_FOR_CODEC || state_ == SURFACE_DESTROYED)
     return;
@@ skipping 60 matching lines @@
       base::ThreadTaskRunnerHandle::Get()->PostTask(
           FROM_HERE,
           base::Bind(&AndroidVideoDecodeAccelerator::NotifyEndOfBitstreamBuffer,
                      weak_this_factory_.GetWeakPtr(), bitstream_buffer_id));
     }
   }
   TRACE_COUNTER1("media", "AVDA::PendingBitstreamBufferCount", 0);
   bitstreams_notified_in_advance_.clear();
 
   picture_buffer_manager_.ReleaseCodecBuffers(output_picture_buffers_);
-
-  // Some VP8 files require complete MediaCodec drain before we can call
-  // MediaCodec.flush() or MediaCodec.reset(). http://crbug.com/598963.
-  if (media_codec_ && codec_config_->codec == kCodecVP8 &&
-      !bitstream_buffers_in_decoder_.empty()) {
-    // Postpone ResetCodecState() after the drain.
-    StartCodecDrain(DRAIN_FOR_RESET);
-  } else {
-    ResetCodecState();
-    base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::Bind(&AndroidVideoDecodeAccelerator::NotifyResetDone,
-                              weak_this_factory_.GetWeakPtr()));
-  }
+  StartCodecDrain(DRAIN_FOR_RESET);
 }
 
 void AndroidVideoDecodeAccelerator::SetSurface(int32_t surface_id) {
   DVLOG(1) << __func__;
   DCHECK(thread_checker_.CalledOnValidThread());
 
   if (surface_id == config_.surface_id) {
     pending_surface_id_.reset();
     return;
   }
 
   // Surface changes never take effect immediately, they will be handled during
   // DequeOutput() once we get to a good switch point or immediately during an
   // OnSurfaceDestroyed() call.
   pending_surface_id_ = surface_id;
 }
 
 void AndroidVideoDecodeAccelerator::Destroy() {
   DVLOG(1) << __func__;
   DCHECK(thread_checker_.CalledOnValidThread());
 
   picture_buffer_manager_.Destroy(output_picture_buffers_);
 
   client_ = nullptr;
 
-  // Some VP8 files require a complete MediaCodec drain before we can call
-  // MediaCodec.flush() or MediaCodec.release(). http://crbug.com/598963. In
-  // that case, postpone ActualDestroy() until after the drain.
-  if (media_codec_ && codec_config_->codec == kCodecVP8) {
-    // Clear |pending_bitstream_records_|.
-    while (!pending_bitstream_records_.empty())
-      pending_bitstream_records_.pop();
-
-    StartCodecDrain(DRAIN_FOR_DESTROY);
-  } else {
-    ActualDestroy();
-  }
+  // We don't want to queue more inputs while draining.
+  while (!pending_bitstream_records_.empty())

[DaleCurtis, 2016/12/08 00:29:35]

Because I was curious and have been benchmarking too many things lately, on my
z620:

std::queue<int>().swap(queue); ~50 microseconds for 100000 objects versus ~100
microseconds for the while() pop version. Just as an FYI in case it ever matters
(it certainly doesn't here).

+    pending_bitstream_records_.pop();
+  StartCodecDrain(DRAIN_FOR_DESTROY);
 }
 
 void AndroidVideoDecodeAccelerator::ActualDestroy() {
   DVLOG(1) << __func__;
   DCHECK(thread_checker_.CalledOnValidThread());
 
   // Note that async codec construction might still be in progress.  In that
   // case, the codec will be deleted when it completes once we invalidate all
   // our weak refs.
   weak_this_factory_.InvalidateWeakPtrs();
@@ skipping 55 matching lines @@
   // SURFACE_DESTROYED.
   state_ = SURFACE_DESTROYED;
   if (media_codec_) {
     AVDACodecAllocator::Instance()->ReleaseMediaCodec(
         std::move(media_codec_), codec_config_->task_type, config_.surface_id);
     picture_buffer_manager_.CodecChanged(nullptr);
   }
 
   // If we're draining, signal completion now because the drain can no longer
   // proceed.
-  if (drain_type_ != DRAIN_TYPE_NONE)
+  if (drain_type_)
     OnDrainCompleted();
 }
 
 void AndroidVideoDecodeAccelerator::InitializeCdm() {
   DVLOG(2) << __func__ << ": " << config_.cdm_id;
 
 #if !defined(ENABLE_MOJO_MEDIA_IN_GPU_PROCESS)
   NOTIMPLEMENTED();
   NotifyInitializationComplete(false);
 #else
@@ skipping 274 matching lines @@
     AVDACodecAllocator::Instance()->DeallocateSurface(this, new_surface_id);
   }
 
   // Regardless of whether we succeeded, we no longer own the previous surface.
   AVDACodecAllocator::Instance()->DeallocateSurface(this, previous_surface_id);
 
   return success;
 }
 
 }  // namespace media