media: Handle output SurfaceView destruction in AVDA

content/common/gpu/media/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 "content/common/gpu/media/android_video_decode_accelerator.h"
 
 #include <stddef.h>
 
 #include <memory>
 
@@ skipping 301 matching lines @@
       is_encrypted_(false),
       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),
+      surface_id_(media::VideoDecodeAccelerator::Config::kNoSurfaceID),
       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 77 matching lines @@
   } else {
     DVLOG(1) << __FUNCTION__ << ", using copy back strategy.";
     strategy_.reset(new AndroidCopyingBackingStrategy(this));
   }
 
   if (!make_context_current_cb_.Run()) {
     LOG(ERROR) << "Failed to make this decoder's GL context current.";
     return false;
   }
 
-  codec_config_->surface_ = strategy_->Initialize(config.surface_id);
+  surface_id_ = config.surface_id;
+  codec_config_->surface_ = strategy_->Initialize(surface_id_);
   if (codec_config_->surface_.IsEmpty()) {
     LOG(ERROR) << "Failed to initialize the backing strategy. The returned "
                   "Java surface is empty.";
     return false;
   }
 
+  on_destroying_surface_cb_ =

[liberato (no reviews please), 2016/04/27 16:50:04]

all of the callback code might be better in the deferred strategy.  as in, it
can register for the destruction callback, and just notify avda to change state.

this might fit better when the strategy chooses when / if to get a surface.

STATE_DESTROYED can stay in avda, but it can expose a "SurfaceWasDestroyed" or
something.  that call would end up moving into MediaCodecLoop, i think.

[watk, 2016/04/27 18:54:04]

I'm not sure if I agree. I think this is simpler in terms of less plumbing, and
conceptually in my mind. The reason we really care about surface destruction is
because we need to release the MediaCodec, and that's AVDAs job.

[liberato (no reviews please), 2016/04/27 21:00:37]

what brought it up is that AVDA didn't even record the surface_id before.  it
was all contained in the strategy.  now, avda peeks at the surface, and assumes
what the strategy is going to do with it.  granted, right now, the strategy has
no choice since WMPI also assumes it, but that's another issue.  :)

to be clear, i'm not suggesting moving anything beside the callback registration
/ unregistration itself.  it's not a big deal at this point, but will probably
happen anyway when the strategy starts worrying about when to allocate the
surface.

[watk, 2016/04/27 21:14:07]

Gotcha, well since you're not too worried I'll defer that change until it's more
clear to me how it will look.

+      base::Bind(&AndroidVideoDecodeAccelerator::OnDestroyingSurface,
+                 weak_this_factory_.GetWeakPtr());
+  AVDASurfaceTracker::GetInstance()->RegisterOnDestroyingSurfaceCallback(
+      on_destroying_surface_cb_);
+
   // TODO(watk,liberato): move this into the strategy.
   scoped_refptr<gfx::SurfaceTexture> surface_texture =
       strategy_->GetSurfaceTexture();
   if (surface_texture) {
     on_frame_available_handler_ =
         new OnFrameAvailableHandler(this, surface_texture);
   }
 
   // Start the thread for async configuration, even if we don't need it now.
   // ResetCodecState might rebuild the codec later, for example.
@@ skipping 61 matching lines @@
 
   NOTIMPLEMENTED();
   NotifyInitializationComplete(false);
 
 #endif  // !defined(ENABLE_MOJO_MEDIA_IN_GPU_PROCESS)
 }
 
 void AndroidVideoDecodeAccelerator::DoIOTask(bool start_timer) {
   DCHECK(thread_checker_.CalledOnValidThread());
   TRACE_EVENT0("media", "AVDA::DoIOTask");
-  if (state_ == ERROR || state_ == WAITING_FOR_CODEC)
+  if (state_ == ERROR || state_ == WAITING_FOR_CODEC ||
+      state_ == SURFACE_DESTROYED) {
     return;
+  }
 
   strategy_->MaybeRenderEarly();
   bool did_work = QueueInput();
   while (DequeueOutput())
     did_work = true;
 
   ManageTimer(did_work || start_timer);
 }
 
 bool AndroidVideoDecodeAccelerator::QueueInput() {
@@ skipping 480 matching lines @@
   return std::unique_ptr<media::VideoCodecBridge>(
       media::VideoCodecBridge::CreateDecoder(
           codec_config->codec_, codec_config->needs_protected_surface_,
           codec_config->initial_expected_coded_size_,
           codec_config->surface_.j_surface().obj(), media_crypto, true));
 }
 
 void AndroidVideoDecodeAccelerator::OnCodecConfigured(
     std::unique_ptr<media::VideoCodecBridge> media_codec) {
   DCHECK(thread_checker_.CalledOnValidThread());
-  DCHECK_EQ(state_, WAITING_FOR_CODEC);
-
-  media_codec_ = std::move(media_codec);
+  DCHECK(state_ ==  WAITING_FOR_CODEC || state_ == SURFACE_DESTROYED);
 
   // Record one instance of the codec being initialized.
   RecordFormatChangedMetric(FormatChangedValue::CodecInitialized);
 
-  strategy_->CodecChanged(media_codec_.get());
-
   // If we are supposed to notify that initialization is complete, then do so
   // now.  Otherwise, this is a reconfiguration.
   if (deferred_initialization_pending_) {
-    NotifyInitializationComplete(!!media_codec_);
+    // Losing the output surface is not considered an error state, so notify
+    // success. The client will destroy this soon.
+    NotifyInitializationComplete(state_ == SURFACE_DESTROYED ? true
+                                                             : !!media_codec);
     deferred_initialization_pending_ = false;
   }
 
+  if (state_ == SURFACE_DESTROYED)

[liberato (no reviews please), 2016/04/27 16:50:04]

please add a comment that this will release the media codec and why we want
that.

[watk, 2016/04/27 18:54:04]

Done.

+    return;
+
+  media_codec_ = std::move(media_codec);
+  strategy_->CodecChanged(media_codec_.get());
   if (!media_codec_) {
     POST_ERROR(PLATFORM_FAILURE, "Failed to create MediaCodec.");
     return;
   }
 
   state_ = NO_ERROR;
 
   ManageTimer(true);
 }
 
@@ skipping 55 matching lines @@
   }
   drain_type_ = DRAIN_TYPE_NONE;
 }
 
 void AndroidVideoDecodeAccelerator::ResetCodecState(
     const base::Closure& done_cb) {
   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 (state_ == WAITING_FOR_CODEC) {
+  // If the surface is destroyed there's nothing to do.
+  if (state_ == WAITING_FOR_CODEC || state_ == SURFACE_DESTROYED) {
     if (!done_cb.is_null())
       done_cb.Run();
     return;
   }
 
   bitstream_buffers_in_decoder_.clear();
 
   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.
@@ skipping 106 matching lines @@
     StartCodecDrain(DRAIN_FOR_DESTROY);
   } else {
     ActualDestroy();
   }
 }
 
 void AndroidVideoDecodeAccelerator::ActualDestroy() {
   DVLOG(1) << __FUNCTION__;
   DCHECK(thread_checker_.CalledOnValidThread());
 
+  if (!on_destroying_surface_cb_.is_null()) {
+    AVDASurfaceTracker::GetInstance()->UnregisterOnDestroyingSurfaceCallback(
+        on_destroying_surface_cb_);
+  }
+
   // 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();
   if (media_codec_) {
     g_avda_timer.Pointer()->StopTimer(this);
     media_codec_.reset();
   }
   delete this;
 }
@@ skipping 33 matching lines @@
 
   DCHECK_LE(1u, picture_buffer.internal_texture_ids().size());
   gpu::gles2::TextureRef* texture_ref =
       texture_manager->GetTexture(picture_buffer.internal_texture_ids()[0]);
   RETURN_ON_FAILURE(this, texture_manager, "Null texture_ref", ILLEGAL_STATE,
                     nullptr);
 
   return texture_ref;
 }
 
+void AndroidVideoDecodeAccelerator::OnDestroyingSurface(int surface_id) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  TRACE_EVENT0("media", "AVDA::OnDestroyingSurface");
+  DVLOG(1) << __FUNCTION__ << " surface_id: " << surface_id;
+
+  if (surface_id != surface_id_)
+    return;
+
+  // If we're currently asynchronously configuring a codec, it will be destroyed
+  // when configuration completes and it notices that |state_| has changed to
+  // SURFACE_DESTROYED.
+  state_ = SURFACE_DESTROYED;
+  if (media_codec_) {
+    media_codec_.reset();
+    strategy_->CodecChanged(media_codec_.get());

[liberato (no reviews please), 2016/04/27 16:50:04]

should this also clear |drain_type_|, for completeness?

i realize that it doesn't really matter, but it seems like a good idea.

[watk, 2016/04/27 18:54:04]

Good call, I think this was broken if a drain was ongoing (especially
DRAIN_FOR_DESTROY). Added logic to complete the drain. I also made flush return
early if the surface is lost.

[liberato (no reviews please), 2016/04/27 21:00:37]

actually, i didn't think about it that much.  good point on OnDrainCompleted.

+  }
+}
+
 void AndroidVideoDecodeAccelerator::OnFrameAvailable() {
   // Remember: this may be on any thread.
   DCHECK(strategy_);
   strategy_->OnFrameAvailable();
 }
 
 void AndroidVideoDecodeAccelerator::PostError(
     const ::tracked_objects::Location& from_here,
     media::VideoDecodeAccelerator::Error error) {
   base::MessageLoop::current()->PostDelayedTask(
@@ skipping 168 matching lines @@
     if (media::MediaCodecUtil::IsSurfaceViewOutputSupported()) {
       capabilities.flags |= media::VideoDecodeAccelerator::Capabilities::
           SUPPORTS_EXTERNAL_OUTPUT_SURFACE;
     }
   }
 
   return capabilities;
 }
 
 }  // namespace content