Add ContentVideoViewOverlay to AVDA.

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 17 matching lines @@
 #include "gpu/ipc/service/gpu_channel.h"
 #include "media/base/android/media_codec_bridge_impl.h"
 #include "media/base/android/media_codec_util.h"
 #include "media/base/bind_to_current_loop.h"
 #include "media/base/bitstream_buffer.h"
 #include "media/base/limits.h"
 #include "media/base/media.h"
 #include "media/base/timestamp_constants.h"
 #include "media/base/video_decoder_config.h"
 #include "media/gpu/avda_picture_buffer_manager.h"
+#include "media/gpu/content_video_view_overlay.h"
 #include "media/gpu/shared_memory_region.h"
 #include "media/video/picture.h"
 #include "ui/gl/android/scoped_java_surface.h"
 #include "ui/gl/android/surface_texture.h"
 #include "ui/gl/gl_bindings.h"
 
 #if defined(ENABLE_MOJO_MEDIA_IN_GPU_PROCESS)
 #include "media/mojo/services/mojo_cdm_service.h"
 #endif
 
@@ skipping 309 matching lines @@
   }
 
   // Fail / complete / defer initialization.
   return state_ != ERROR;
 }
 
 void AndroidVideoDecodeAccelerator::StartSurfaceCreation() {
   // We might be called during Initialize, during deferred initialization, or
   // afterwards (::Decode, for deferred surface init, UpdateSurface).
 
+  // Whoever calls us should release any overlay we had.
+  DCHECK(!codec_config_->overlay);
+  // Note that we don't enforce that for any SurfaceTexture or its Surface,

[liberato (no reviews please), 2017/02/17 18:34:38]

watk: do you agree with this reasoning?  i believe it was our old behavior --
PictureBufferManager::Initialize did it before.  we now do it at the same place
in InitializePictureBufferManager.

[watk, 2017/02/17 22:40:36]

I don't know to be honest. I'm pretty confused :(

[liberato (no reviews please), 2017/03/07 21:30:25]

i think that this is now okay, since surface bundles handle it.

+  // since there might be a codec that's using them.  They'll get cleared
+  // later, in InitializePictureBufferManager.
+
   // If surface creation is deferred, then do nothing except signal that init
   // is complete, if needed.  We might still fail to get a surface or codec,
   // which would normally be an init error.  Since we're deferring init until a
   // decode to save resources, though, we're signaling success now.  If we're
   // wrong, then decoding might fail when we might have been able to use a
   // fallback renderer in WMPI if we failed init.
   if (defer_surface_creation_) {
     if (deferred_initialization_pending_)
       NotifyInitializationSucceeded();
 
     return;
   }
 
-  if (!codec_allocator_->AllocateSurface(this, config_.surface_id)) {
+  if (config_.surface_id != SurfaceManager::kNoSurfaceID) {
+    // Create the overlay.  Note that it will never call us back immedaitely.

[watk, 2017/02/17 22:40:36]

immediately

[liberato (no reviews please), 2017/03/07 21:30:25]

Done.

+    // It will post when the surface is available.
+    AndroidOverlay::Config overlay_config;
+    // We use weak ptrs here since |overlay| can outlive us, if we send it for
+    // async codec config.
+    overlay_config.ready_cb =
+        base::Bind(&AndroidVideoDecodeAccelerator::OnSurfaceAvailable,
+                   weak_this_factory_.GetWeakPtr());
+    overlay_config.destroyed_cb =
+        base::Bind(&AndroidVideoDecodeAccelerator::OnSurfaceDestroyed,
+                   weak_this_factory_.GetWeakPtr());
+    codec_config_->overlay = base::MakeUnique<ContentVideoViewOverlay>(
+        codec_allocator_, config_.surface_id, overlay_config);
     // We have to wait for some other AVDA instance to free up the surface.
     // OnSurfaceAvailable will be called when it's available.
     // Note that if we aren't deferring init, then we'll signal success, and
     // if we fail later then it will fail decoding instead.  However, since
     // nobody that provides a SurfaceView requires sync init, it doesn't matter.
     state_ = WAITING_FOR_SURFACE;
     return;
   }
 
-  // We now own the surface, so finish initialization.
+  // We're creating a SurfaceTexture.
   InitializePictureBufferManager();
 }
 
-void AndroidVideoDecodeAccelerator::OnSurfaceAvailable(bool success) {
+void AndroidVideoDecodeAccelerator::OnSurfaceAvailable() {
   DCHECK(!defer_surface_creation_);
   DCHECK_EQ(state_, WAITING_FOR_SURFACE);
 
-  if (!success) {
-    NOTIFY_ERROR(PLATFORM_FAILURE, "Surface is not available");
-    return;
-  }
+  // TODO(liberato): We could short-circuit the AndroidOverlay callback to call

[watk, 2017/02/17 22:40:36]

Remove TODO? I don't think we need a comment here but up to you.

[liberato (no reviews please), 2017/03/07 21:30:25]

Done.

+  // InitializePictureBufferManager, but it's somewhat clearer this way.  We
+  // also get to keep the WAITING_FOR_SURFACE DCHECK, whcih isn't always true
+  // in InitializePictureBufferManager.
 
   InitializePictureBufferManager();
 }
 
 void AndroidVideoDecodeAccelerator::InitializePictureBufferManager() {
   DCHECK(!defer_surface_creation_);
 
   if (!make_context_current_cb_.Run()) {
     NOTIFY_ERROR(PLATFORM_FAILURE,
                  "Failed to make this decoder's GL context current");
     return;
   }
 
-  codec_config_->surface =
-      picture_buffer_manager_.Initialize(config_.surface_id);
-  codec_config_->surface_texture = picture_buffer_manager_.surface_texture();
-  if (codec_config_->surface.IsEmpty()) {
-    NOTIFY_ERROR(PLATFORM_FAILURE, "Codec surface is empty");
-    return;
+  // TODO(liberato): it doesn't make sense anymore for the PictureBufferManager
+  // to create the surface texture.
+  if (codec_config_->overlay) {
+    picture_buffer_manager_.InitializeForOverlay();
+    // Note that we might want to do this after SetSurface, or on return, or

[liberato (no reviews please), 2017/02/17 18:34:38]

hrm, this also exists in the current code.  i'm pretty sure that it's okay,
since we're not releasing any buffers.

[watk, 2017/02/17 22:40:36]

Yeah +1 to already exists.

[liberato (no reviews please), 2017/03/07 21:30:25]

surface bundle should fix this.

+    // something like that.  There might still be a MediaCodec that's using
+    // them now.  Perhaps put them into local vars?
+    codec_config_->surface_texture_surface = gl::ScopedJavaSurface();
+    codec_config_->surface_texture = nullptr;

[watk, 2017/02/17 22:40:36]

This seems broken? Before this change. I think it was introduced by the
StartSurfaceCreation change. 

This releases the ST before SetSurface succeeded. So the codec has a dead
surface?

[liberato (no reviews please), 2017/03/07 21:30:25]

it did this before, too.  it used to be that AVDA::UpdateSurface would call
PBM::Initialize, then call MCB::SetSurface.

ah, but wait -- in both cases, the outgoing |shared_state_| still has a ref to
it.  that's why it's okay -- we won't drop the ST until all the picture buffers
are re-issued and the last ref to |shared_state_| is dropped.  wow, that's
hacky.  surface owners are so much better at this -- MCVD really, really needs
to use them.

however, |surface_texture_surface| is now broken.  it will auto-release when we
drop it here.

i think that this bug was introduced by me in the Unify CL.  AVDA:UpdateSurface
used to use |new_surface| to hold the new surface, and only std::move() it to
|surface| on success.  we should similarly preserve |surface_texture_surface|
here.  i won't preserve surface_texture, since we don't want it to outlive the
outgoing |shared_state_|.  |shared_state_| will delete the GL objects during
destruction.

note that for overlay surfaces, it doesn't matter, since the surface won't
auto-release anyway.  hacky, but if we're switching to a SurfaceTexture and
SetSurface fails, we've already dropped the overlay.  it might be
OnSurfaceDetroyed anyway (we don't know here).  in any case SetSurfaceFailure =>
NOTIFY_ERROR => UpdateSurface notices ERROR and ReleaseCodec.

you know, a SurfaceTexture wrapper that holds the GL objects (rather than
letting shared_state do it) might make this easier.  then we can post a ref to
it with MC Release, and probably skip all the special ST processing.  i'll think
more about it.  it's quite similar to the SurfaceTextureProducer interface, but
with a lot less work.  could probably be a subclass, in fact.

== weeks later ==

surface bundle + SurfaceTextureGLOwner deal with this case.

+  } else {
+    codec_config_->surface_texture_surface =
+        picture_buffer_manager_.InitializeForSurfaceTexture();
+    codec_config_->surface_texture = picture_buffer_manager_.surface_texture();
   }
 
-  // If we have a media codec, then setSurface.  If that doesn't work, then we
+  // If we have a media codec, then SetSurface.  If that doesn't work, then we
   // do not try to allocate a new codec; we might not be at a keyframe, etc.
   // If we get here with a codec, then we must setSurface.
   if (media_codec_) {
     // TODO(liberato): fail on api check?
-    if (!media_codec_->SetSurface(codec_config_->surface.j_surface().obj())) {
+    if (!media_codec_->SetSurface(codec_config_->j_surface().obj())) {
       NOTIFY_ERROR(PLATFORM_FAILURE, "MediaCodec failed to switch surfaces.");
+    } else {
+      state_ = NO_ERROR;
     }
     return;
   }
 
   // If the client doesn't support deferred initialization (WebRTC), then we
   // should complete it now and return a meaningful result.  Note that it would
   // be nice if we didn't have to worry about starting codec configuration at
   // all (::Initialize or the wrapper can do it), but then they have to remember
   // not to start codec config if we have to wait for the cdm.  It's somewhat
   // clearer for us to handle both cases.
@@ skipping 503 matching lines @@
 
   DCHECK_NE(state_, WAITING_FOR_CODEC);
   state_ = WAITING_FOR_CODEC;
 
   ReleaseCodec();
 
   base::Optional<TaskType> task_type =
       codec_allocator_->TaskTypeForAllocation();
   if (!task_type) {
     // If there is no free thread, then just fail.
-    OnCodecConfigured(nullptr);
+    OnCodecConfigured(nullptr, nullptr);
     return;
   }
 
   // If autodetection is disallowed, fall back to Chrome's software decoders
   // instead of using the software decoders provided by MediaCodec.
   if (task_type == TaskType::SW_CODEC &&
       IsMediaCodecSoftwareDecodingForbidden()) {
-    OnCodecConfigured(nullptr);
+    OnCodecConfigured(nullptr, nullptr);
     return;
   }
 
   codec_config_->task_type = task_type.value();
   codec_allocator_->CreateMediaCodecAsync(weak_this_factory_.GetWeakPtr(),
                                           codec_config_);
 }
 
 void AndroidVideoDecodeAccelerator::ConfigureMediaCodecSynchronously() {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!media_codec_);
   DCHECK_NE(state_, WAITING_FOR_CODEC);
   state_ = WAITING_FOR_CODEC;
 
   base::Optional<TaskType> task_type =
       codec_allocator_->TaskTypeForAllocation();
   if (!task_type) {
     // If there is no free thread, then just fail.
-    OnCodecConfigured(nullptr);
+    OnCodecConfigured(nullptr, nullptr);
     return;
   }
 
   codec_config_->task_type = task_type.value();
   std::unique_ptr<VideoCodecBridge> media_codec =
       codec_allocator_->CreateMediaCodecSync(codec_config_);
   // Note that |media_codec| might be null, which will NotifyError.
-  OnCodecConfigured(std::move(media_codec));
+  OnCodecConfigured(codec_config_, std::move(media_codec));
 }
 
 void AndroidVideoDecodeAccelerator::OnCodecConfigured(
+    scoped_refptr<CodecConfig> codec_config,
     std::unique_ptr<VideoCodecBridge> media_codec) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(state_ == WAITING_FOR_CODEC || state_ == SURFACE_DESTROYED);
+  // We don't use |codec_config|.  It's present only in case the callback
+  // fails, so that |overlay| will be dropped on the right thread if it's the
+  // last reference.
 
   // If we are supposed to notify that initialization is complete, then do so
   // before returning.  Otherwise, this is a reconfiguration.
 
   // If |state_| changed to SURFACE_DESTROYED while we were configuring a codec,
   // then the codec is already invalid so we return early and drop it.
   // TODO(liberato): We're going to drop the codec when |media_codec| goes out
   // of scope, on this thread.  We really should post it to the proper thread
   // to avoid potentially hanging.
   if (state_ == SURFACE_DESTROYED) {
@@ skipping 124 matching lines @@
   // Flush the codec if possible, or create a new one if not.
   if (!MediaCodecUtil::CodecNeedsFlushWorkaround(media_codec_.get())) {
     DVLOG(3) << __func__ << " Flushing MediaCodec.";
     media_codec_->Flush();
     // Since we just flushed all the output buffers, make sure that nothing is
     // using them.
     picture_buffer_manager_.CodecChanged(media_codec_.get());
   } else {
     DVLOG(3) << __func__ << " Deleting the MediaCodec and creating a new one.";
     GetManager()->StopTimer(this);
+    // Note that this will release the codec, then allocate a new one.  It will
+    // not wait for the old one to finish up with the surface, which is bad.
+    // It works (usually) because it ends up allocating the codec on the same
+    // thread as is used to release the old one, so it's serialized anyway.
     ConfigureMediaCodecAsynchronously();
   }
 }
 
 void AndroidVideoDecodeAccelerator::Reset() {
   DVLOG(1) << __func__;
   DCHECK(thread_checker_.CalledOnValidThread());
   TRACE_EVENT0("media", "AVDA::Reset");
 
   if (defer_surface_creation_) {
@@ skipping 56 matching lines @@
   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();
   GetManager()->StopTimer(this);
   ReleaseCodec();
 
-  // We no longer care about |surface_id|, in case we did before.  It's okay
-  // if we have no surface and/or weren't the owner or a waiter.
-  codec_allocator_->DeallocateSurface(this, config_.surface_id);
+  // If we own |surface_id|, then we'll delete it when |codec_config_| goes
+  // out of scope.  That might be after an in-progress async config completes.
 
   // Hop the SurfaceTexture release call through the task runner used last time
   // we released a codec. This ensures that we release the surface texture after
   // the codec it's attached to (if any) is released. It's not sufficient to use
   // |codec_config_->task_type| because that might have changed since we
   // released the codec this surface was attached to.
   if (codec_config_->surface_texture) {
     codec_allocator_->TaskRunnerFor(last_release_task_type_)
         ->PostTaskAndReply(
             FROM_HERE, base::Bind(&base::DoNothing),
@@ skipping 17 matching lines @@
 base::WeakPtr<gpu::gles2::GLES2Decoder>
 AndroidVideoDecodeAccelerator::GetGlDecoder() const {
   return get_gles2_decoder_cb_.Run();
 }
 
 void AndroidVideoDecodeAccelerator::OnSurfaceDestroyed() {
   DVLOG(1) << __func__;
   TRACE_EVENT0("media", "AVDA::OnSurfaceDestroyed");
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  // We cannot get here if we're before surface allocation, since we transition
-  // to WAITING_FOR_CODEC (or NO_ERROR, if sync) when we get the surface without
-  // posting.  If we do ever lose the surface before starting codec allocation,
-  // then we could just update the config to use a SurfaceTexture and return
-  // without changing state.
-  DCHECK_NE(state_, WAITING_FOR_SURFACE);
+  // If we're notified that the overlay is destroyed while we're waiting for a
+  // surface from it, then we could just switch back to SurfaceTexture.  For
+  // now, we just fail.
+  if (state_ == WAITING_FOR_SURFACE) {
+    NOTIFY_ERROR(PLATFORM_FAILURE, "Surface is not available");
+    return;
+  }
 
   // If the API is available avoid having to restart the decoder in order to
   // leave fullscreen. If we don't clear the surface immediately during this
   // callback, the MediaCodec will throw an error as the surface is destroyed.
   if (base::android::BuildInfo::GetInstance()->sdk_int() >= 23) {
     // Since we can't wait for a transition, we must invalidate all outstanding
     // picture buffers to avoid putting the GL system in a broken state.
     picture_buffer_manager_.ReleaseCodecBuffers(output_picture_buffers_);
 
     // Switch away from the surface being destroyed to a surface texture.
@@ skipping 280 matching lines @@
          pending_surface_id_.value() == SurfaceManager::kNoSurfaceID);
 
   const int previous_surface_id = config_.surface_id;
   const int new_surface_id = pending_surface_id_.value();
   pending_surface_id_.reset();
 
   // Start surface creation.  Note that if we're called via surfaceDestroyed,
   // then this must complete synchronously or it will DCHECK.  Otherwise, we
   // might still be using the destroyed surface.  We don't enforce this, but
   // it's worth remembering that there are cases where it's required.
+
+  // Regardless of whether we succeed, we will no longer own the previous
+  // surface, if any, when we return.  |previous_overlay| will be dropped then.
+  std::unique_ptr<AndroidOverlay> previous_overlay =
+      std::move(codec_config_->overlay);
+
   config_.surface_id = new_surface_id;
   StartSurfaceCreation();
   if (state_ == ERROR) {
     // This might be called from OnSurfaceDestroyed(), so we have to release the
     // MediaCodec if we failed to switch the surface.  We reset the surface ID
     // to the previous one, since failures never result in the codec using the
     // new surface.  This is only guaranteed because of how OnCodecConfigured
     // works.  If it could fail after getting a codec, then this assumption
     // wouldn't be necessarily true anymore.
     // Also note that we might not have switched surfaces yet, which is also bad
     // for OnSurfaceDestroyed, because of WAITING_FOR_SURFACE.  Shouldn't
     // happen with SurfaceTexture, and OnSurfaceDestroyed checks for it.
     config_.surface_id = previous_surface_id;
     ReleaseCodec();
-    codec_allocator_->DeallocateSurface(this, new_surface_id);
+    // For CVV, this is the right thing to do, since dropping |overlay| doesn't
+    // really destroy the surface.  However, for DS, we'll want to post it in
+    // ReleaseCodec in this case since the codec is still using it.
+    codec_config_->overlay = nullptr;
   }
 
-  // Regardless of whether we succeeded, we no longer own the previous surface.
-  codec_allocator_->DeallocateSurface(this, previous_surface_id);
-
   return state_ != ERROR;
 }
 
 void AndroidVideoDecodeAccelerator::ReleaseCodec() {
   if (!media_codec_)
     return;
 
   picture_buffer_manager_.CodecChanged(nullptr);
   codec_allocator_->ReleaseMediaCodec(
       std::move(media_codec_), codec_config_->task_type, config_.surface_id);
   last_release_task_type_ = codec_config_->task_type;
 }
 
 }  // namespace media