media: Lazily reconfigure MediaCodecs in AVDA for JellyBean

content/common/gpu/media/android_video_decode_accelerator.cc

Index: content/common/gpu/media/android_video_decode_accelerator.cc
diff --git a/content/common/gpu/media/android_video_decode_accelerator.cc b/content/common/gpu/media/android_video_decode_accelerator.cc
index 5e719f94f3ab07f039087975375c4b5e37302e54..40c984ca2c3e5c3422c1c137eb862275522863e2 100644
--- a/content/common/gpu/media/android_video_decode_accelerator.cc
+++ b/content/common/gpu/media/android_video_decode_accelerator.cc
@@ -408,8 +408,18 @@ void AndroidVideoDecodeAccelerator::SetCdm(int cdm_id) {
 void AndroidVideoDecodeAccelerator::DoIOTask() {
   DCHECK(thread_checker_.CalledOnValidThread());
   TRACE_EVENT0("media", "AVDA::DoIOTask");
+
   if (state_ == ERROR) {
     return;
+  } else if (state_ == PENDING_CODEC_RECONFIGURE) {

[watk, 2016/03/24 01:02:58]

Should probably put this in Decode().

[liberato (no reviews please), 2016/03/29 14:54:40]

probably, yes, since the codec can't take any action until a decode comes in.

alternatively, if you keep it here, then it might be a good idea to see if
|pending_bitstream_buffers_| is not empty before allocating the codec.  we call
DoIOTask a lot, though maybe not after a Reset.  still, makes more sense to
check if there's input to the codec before we bother.  then i think it's the
same as in Decode

+    // Defer errors here in case we're about to be destroyed because the
+    // pipeline is suspending.
+    base::AutoReset<bool> auto_reset(&defer_errors_, true);
+    if (!ConfigureMediaCodec()) {
+      POST_ERROR(PLATFORM_FAILURE, "Failed to create MediaCodec.");
+      return;
+    }
+    state_ = NO_ERROR;
   }
 
   bool did_work = QueueInput();
@@ -421,6 +431,7 @@ void AndroidVideoDecodeAccelerator::DoIOTask() {
 
 bool AndroidVideoDecodeAccelerator::QueueInput() {
   DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK_NE(state_, PENDING_CODEC_RECONFIGURE);
   TRACE_EVENT0("media", "AVDA::QueueInput");
   base::AutoReset<bool> auto_reset(&defer_errors_, true);
   if (bitstreams_notified_in_advance_.size() > kMaxBitstreamsNotifiedInAdvance)
@@ -535,12 +546,6 @@ bool AndroidVideoDecodeAccelerator::QueueInput() {
   pending_bitstream_buffers_.pop();
   TRACE_COUNTER1("media", "AVDA::PendingBitstreamBufferCount",
                  pending_bitstream_buffers_.size());
-
-  if (status != media::MEDIA_CODEC_OK) {
-    POST_ERROR(PLATFORM_FAILURE, "Failed to QueueInputBuffer: " << status);
-    return false;
-  }
-
   // We should call NotifyEndOfBitstreamBuffer(), when no more decoded output
   // will be returned from the bitstream buffer. However, MediaCodec API is
   // not enough to guarantee it.
@@ -554,11 +559,17 @@ bool AndroidVideoDecodeAccelerator::QueueInput() {
                  weak_this_factory_.GetWeakPtr(), bitstream_buffer.id()));
   bitstreams_notified_in_advance_.push_back(bitstream_buffer.id());
 
+  if (status != media::MEDIA_CODEC_OK) {
+    POST_ERROR(PLATFORM_FAILURE, "Failed to QueueInputBuffer: " << status);
+    return false;

[watk, 2016/03/24 01:02:58]

This has to be moved because otherwise we might pop an input buffer, and never
NotifyEnded, which will get GVD out of sync.

+  }
+
   return true;
 }
 
 bool AndroidVideoDecodeAccelerator::DequeueOutput() {
   DCHECK(thread_checker_.CalledOnValidThread());
+  DCHECK_NE(state_, PENDING_CODEC_RECONFIGURE);
   TRACE_EVENT0("media", "AVDA::DequeueOutput");
   base::AutoReset<bool> auto_reset(&defer_errors_, true);
   if (picturebuffers_requested_ && output_picture_buffers_.empty())
@@ -937,11 +948,10 @@ void AndroidVideoDecodeAccelerator::ResetCodecState() {
              << " Deleting the MediaCodec and creating a new one.";
     g_avda_timer.Pointer()->StopTimer(this);
     media_codec_.reset();
+    strategy_->CodecChanged(media_codec_.get(), output_picture_buffers_);
     // Changing the codec will also notify the strategy to forget about any
     // output buffers it has currently.
-    state_ = NO_ERROR;
-    if (!ConfigureMediaCodec())
-      POST_ERROR(PLATFORM_FAILURE, "Failed to create MediaCodec.");
+    state_ = PENDING_CODEC_RECONFIGURE;

[liberato (no reviews please), 2016/03/29 14:54:40]

you might consider not deferring the reset in the case of a call to
ResetCodecState for a format change -- it's a slow process, and we know that
more decodes will happen.  might as well start sooner to hide the latency. 
right now, it doesn't make much difference, because it's all synchronous.  with
async init, it could actually proceed in parallel.

alternatively, please add a comment about it here, else i'll forget to change it
later.  :)

   }
 }