Don't reset the codec state for a flush; this kills the frames.

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 365 matching lines @@
       get_gles2_decoder_cb_(get_gles2_decoder_cb),
       state_(NO_ERROR),
       picturebuffers_requested_(false),
       drain_type_(DRAIN_TYPE_NONE),
       media_drm_bridge_cdm_context_(nullptr),
       cdm_registration_id_(0),
       pending_input_buf_index_(-1),
       error_sequence_token_(0),
       defer_errors_(false),
       deferred_initialization_pending_(false),
+      codec_needs_reset_(false),
       weak_this_factory_(this) {}
 
 AndroidVideoDecodeAccelerator::~AndroidVideoDecodeAccelerator() {
   DCHECK(thread_checker_.CalledOnValidThread());
   g_avda_timer.Pointer()->StopTimer(this);
   g_avda_timer.Pointer()->StopThread(this);
 
 #if defined(ENABLE_MOJO_MEDIA_IN_GPU_PROCESS)
   if (!media_drm_bridge_cdm_context_)
     return;
@@ skipping 497 matching lines @@
 
   // Connect the PictureBuffer to the decoded frame, via whatever mechanism the
   // strategy likes.
   strategy_->UseCodecBufferForPictureBuffer(codec_buffer_index, i->second);
 }
 
 void AndroidVideoDecodeAccelerator::Decode(
     const media::BitstreamBuffer& bitstream_buffer) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
+  // If we previously deferred a codec restart, take care of it now. This can

[Tima Vaisburd, 2016/05/24 19:06:26]

Do you want to DCHECK that drain_type_ == DRAIN_TYPE_NONE ?

[DaleCurtis, 2016/05/24 23:53:14]

Done.

+  // happen on older devices where configuration changes require a codec reset.
+  if (codec_needs_reset_)
+    ResetCodecState(base::Closure());
+
   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());
 
   if (bitstream_buffer.id() < 0) {
     POST_ERROR(INVALID_ARGUMENT,
@@ skipping 259 matching lines @@
     // 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;
   }
 
   const bool did_codec_error_happen = state_ == ERROR;
   state_ = NO_ERROR;
 
+  // Don't reset the codec here if there's no error and we're only flushing;
+  // instead defer until the next decode call; this prevents us from unbacking
+  // frames that might be out for display at end of stream.
+  codec_needs_reset_ = false;
+  if (drain_type_ == DRAIN_FOR_FLUSH && !did_codec_error_happen) {

[watk, 2016/05/24 19:47:26]

Can we do this even if there's no drain going on? Then this would also fix the
unnecessary codec creation we do for Suspends for fullscreen 
https://bugs.chromium.org/p/chromium/issues/detail?id=599288

[DaleCurtis, 2016/05/24 23:53:14]

We probably can, but doing so exposed a few issues with how we're handling reset
right now. Namely that ERROR state is non-terminal and ResetCodecState can
escape out of it. Probably this should be fixed first, so I'm going to defer
this change to a followup CL.

+    codec_needs_reset_ = true;
+    if (!done_cb.is_null())
+      done_cb.Run();
+    return;
+  }
+
   // We might increment error_sequence_token here to cancel any delayed errors,
   // but right now it's unclear that it's safe to do so.  If we are in an error
   // state because of a codec error, then it would be okay.  Otherwise, it's
   // less obvious that we are exiting the error state.  Since deferred errors
   // are only intended for fullscreen transitions right now, we take the more
   // conservative approach and let the errors post.
   // TODO(liberato): revisit this once we sort out the error state a bit more.
 
   // 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
@@ skipping 402 matching lines @@
     if (media::MediaCodecUtil::IsSurfaceViewOutputSupported()) {
       capabilities.flags |= media::VideoDecodeAccelerator::Capabilities::
           SUPPORTS_EXTERNAL_OUTPUT_SURFACE;
     }
   }
 
   return capabilities;
 }
 
 }  // namespace media