Serialize usage of SurfaceView between AVDA instances.

media/gpu/android_video_decode_accelerator.cc

Index: media/gpu/android_video_decode_accelerator.cc
diff --git a/media/gpu/android_video_decode_accelerator.cc b/media/gpu/android_video_decode_accelerator.cc
index 526146156f8b64b473e3b66fa7f4237cf75d648a..284344a8609af60f414829d33bed2b89ce4a9508 100644
--- a/media/gpu/android_video_decode_accelerator.cc
+++ b/media/gpu/android_video_decode_accelerator.cc
@@ -249,6 +249,66 @@ class AVDATimerManager {
     return construction_thread_.task_runner();
   }
 
+  // |owner| would like to use |surface_id|.  If it is not busy, then mark it
+  // as busy and return true.  If it is busy, then replace any existing waiter,
+  // make |owner| the current waiter, and return false.  Any existing waiter
+  // is assumed to be on the way out, so we fail its allocation request.
+  bool AllocateSurface(int surface_id, AndroidVideoDecodeAccelerator* avda) {
+    // Nobody has to wait for no surface.
+    if (surface_id == AndroidVideoDecodeAccelerator::Config::kNoSurfaceID)
+      return true;
+
+    SurfaceWaiterMap::iterator iter = surface_waiter_map_.find(surface_id);

[DaleCurtis, 2016/05/18 19:58:48]

Can use auto here if you like, ditto below.

[liberato (no reviews please), 2016/05/18 21:13:13]

Done.

+

[DaleCurtis, 2016/05/18 19:58:48]

Delete extra line.

[liberato (no reviews please), 2016/05/18 21:13:13]

Done.

+    if (iter == surface_waiter_map_.end()) {
+      // SurfaceView isn't allocated.  Succeed.
+      surface_waiter_map_[surface_id].owner = avda;

[DaleCurtis, 2016/05/18 19:58:48]

Change this to = { avda, nullptr } to avoid two lookups and two assigns.

[liberato (no reviews please), 2016/05/18 21:13:13]

i removed the second assignment, since |waiter| is initialized to nullptr
anyway.

+      surface_waiter_map_[surface_id].waiter = nullptr;
+      return true;
+    }
+
+    // SurfaceView is already allocated.
+
+    if (iter->second.waiter != nullptr) {

[DaleCurtis, 2016/05/18 19:58:48]

if (iter->second.waiter)

[liberato (no reviews please), 2016/05/18 21:13:13]

Done.

+      // Some other AVDA is waiting.  |avda| will replace it, so notify it
+      // that it will fail.
+      iter->second.waiter->OnSurfaceAvailable(false);
+      iter->second.waiter = nullptr;
+    }
+
+    // |avda| is now waiting.
+    iter->second.waiter = avda;
+    return false;
+  }
+
+  // Clear any waiting request for |surface_id| by |avda|.  It is okay if
+  // |waiter| is not waiting and/or isn't the owner of |surface_id|.
+  void DeallocateSurface(int surface_id, AndroidVideoDecodeAccelerator* avda) {
+    SurfaceWaiterMap::iterator iter = surface_waiter_map_.find(surface_id);
+    if (iter == surface_waiter_map_.end())
+      return;
+
+    // If |avda| was waiting, then remove it without OnSurfaceAvailable.
+    if (iter->second.waiter == avda)
+      iter->second.waiter = nullptr;
+
+    // If |avda| is the owner, then let the waiter have it.
+    if (iter->second.owner != avda)
+      return;
+
+    if (AndroidVideoDecodeAccelerator* waiter = iter->second.waiter) {

[DaleCurtis, 2016/05/18 19:58:48]

Invert this so that it's if (!iter->second.waiter) { ...; return; }.

[liberato (no reviews please), 2016/05/18 21:13:13]

Done.

+      // Promote |waiter| to be the owner.
+      iter->second.owner = waiter;
+      iter->second.waiter = nullptr;
+      waiter->OnSurfaceAvailable(true);
+    } else {
+      // No waiter -- remove the record and return explicitly since |iter| is
+      // no longer valid.
+      surface_waiter_map_.erase(iter);
+      return;
+    }
+  }
+
  private:
   friend struct base::DefaultLazyInstanceTraits<AVDATimerManager>;
 
@@ -280,6 +340,14 @@ class AVDATimerManager {
   // All AVDA instances that might like to use the construction thread.
   std::set<AndroidVideoDecodeAccelerator*> thread_avda_instances_;
 
+  // [surface id] = accelerator that owns the surface, or nullptr.
+  struct OwnerRecord {
+    AndroidVideoDecodeAccelerator* owner = nullptr;
+    AndroidVideoDecodeAccelerator* waiter = nullptr;
+  };
+  using SurfaceWaiterMap = std::map<int, OwnerRecord>;
+  SurfaceWaiterMap surface_waiter_map_;
+
   // Since we can't delete while iterating when using a set, defer erasure until
   // after iteration complete.
   bool timer_running_ = false;
@@ -318,6 +386,7 @@ AndroidVideoDecodeAccelerator::AndroidVideoDecodeAccelerator(
       error_sequence_token_(0),
       defer_errors_(false),
       deferred_initialization_pending_(false),
+      cdm_id_(0),
       surface_id_(media::VideoDecodeAccelerator::Config::kNoSurfaceID),
       weak_this_factory_(this) {}
 
@@ -359,6 +428,7 @@ bool AndroidVideoDecodeAccelerator::Initialize(const Config& config,
   codec_config_->initial_expected_coded_size_ =
       config.initial_expected_coded_size;
   is_encrypted_ = config.is_encrypted;
+  cdm_id_ = config.cdm_id;

[DaleCurtis, 2016/05/18 19:58:48]

Seems like we should just keep a copy of config instead of all these single
members.

[liberato (no reviews please), 2016/05/18 21:13:13]

Done.

 
   // We signalled that we support deferred initialization, so see if the client
   // does also.
@@ -415,6 +485,33 @@ bool AndroidVideoDecodeAccelerator::Initialize(const Config& config,
   }
 
   surface_id_ = config.surface_id;
+  if (g_avda_timer.Pointer()->AllocateSurface(surface_id_, this)) {
+    // We have succesfully owned the surface, so finish initialization now.
+    return InitializeStrategy();
+  }
+
+  // We have to wait for some other AVDA instance to free up the surface.
+  // OnSurfaceAvailable will be called when it's available.
+  return true;
+}
+
+void AndroidVideoDecodeAccelerator::OnSurfaceAvailable(bool success) {
+  DCHECK(deferred_initialization_pending_);
+
+  if (!success) {
+    // Surface isn't available and/or we're being destroyed.
+    NotifyInitializationComplete(false);
+    deferred_initialization_pending_ = false;
+    return;
+  }
+
+  if (!InitializeStrategy()) {
+    NotifyInitializationComplete(false);
+    deferred_initialization_pending_ = false;
+  }
+}
+
+bool AndroidVideoDecodeAccelerator::InitializeStrategy() {
   codec_config_->surface_ = strategy_->Initialize(surface_id_);
   if (codec_config_->surface_.IsEmpty()) {
     LOG(ERROR) << "Failed to initialize the backing strategy. The returned "
@@ -445,7 +542,7 @@ bool AndroidVideoDecodeAccelerator::Initialize(const Config& config,
 
   // If we are encrypted, then we aren't able to create the codec yet.
   if (is_encrypted_) {
-    InitializeCdm(config.cdm_id);
+    InitializeCdm();
     return true;
   }
 
@@ -455,7 +552,11 @@ bool AndroidVideoDecodeAccelerator::Initialize(const Config& config,
   }
 
   // If the client doesn't support deferred initialization (WebRTC), then we
-  // should complete it now and return a meaningful result.
+  // 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.
   return ConfigureMediaCodecSynchronously();
 }
 
@@ -1204,6 +1305,10 @@ void AndroidVideoDecodeAccelerator::ActualDestroy() {
         on_destroying_surface_cb_);
   }
 
+  // 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.
+  g_avda_timer.Pointer()->DeallocateSurface(surface_id_, this);
+
   // 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.
@@ -1296,15 +1401,16 @@ void AndroidVideoDecodeAccelerator::PostError(
   state_ = ERROR;
 }
 
-void AndroidVideoDecodeAccelerator::InitializeCdm(int cdm_id) {
-  DVLOG(2) << __FUNCTION__ << ": " << cdm_id;
+void AndroidVideoDecodeAccelerator::InitializeCdm() {
+  DVLOG(2) << __FUNCTION__ << ": " << cdm_id_;
 
 #if !defined(ENABLE_MOJO_MEDIA_IN_GPU_PROCESS)
   NOTIMPLEMENTED();
   NotifyInitializationComplete(false);
 #else
   // Store the CDM to hold a reference to it.
-  cdm_for_reference_holding_only_ = media::MojoCdmService::LegacyGetCdm(cdm_id);
+  cdm_for_reference_holding_only_ =
+      media::MojoCdmService::LegacyGetCdm(cdm_id_);
   DCHECK(cdm_for_reference_holding_only_);
 
   // On Android platform the CdmContext must be a MediaDrmBridgeCdmContext.