media: Do a TimedWait() for video surface teardown in 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>
 
 #include "base/android/build_info.h"
 #include "base/auto_reset.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback_helpers.h"
 #include "base/command_line.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
+#include "base/memory/ref_counted.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram.h"
 #include "base/sys_info.h"
 #include "base/task_runner_util.h"
 #include "base/threading/thread.h"
 #include "base/threading/thread_checker.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/trace_event/trace_event.h"
 #include "gpu/command_buffer/service/gles2_cmd_decoder.h"
 #include "gpu/command_buffer/service/mailbox_manager.h"
@@ skipping 70 matching lines @@
 // pictures have been fed to saturate any internal buffering).  This is
 // speculative and it's unclear that this would be a win (nor that there's a
 // reasonably device-agnostic way to fill in the "believes" above).
 constexpr base::TimeDelta DecodePollDelay =
     base::TimeDelta::FromMilliseconds(10);
 
 constexpr base::TimeDelta NoWaitTimeOut = base::TimeDelta::FromMicroseconds(0);
 
 constexpr base::TimeDelta IdleTimerTimeOut = base::TimeDelta::FromSeconds(1);
 
+// Time between when we notice an error, and when we actually notify somebody.
+// This is to prevent codec errors caused by SurfaceView fullscreen transitions
+// from breaking the pipeline, if we're about to be reset anyway.
+constexpr base::TimeDelta ErrorPostingDelay = base::TimeDelta::FromSeconds(2);
+
+// On low end devices (< KitKat is always low-end due to buggy MediaCodec),
+// defer the surface creation until the codec is actually used if we know no
+// software fallback exists.
+bool ShouldDeferSurfaceCreation(int surface_id, VideoCodec codec) {
+  return surface_id == SurfaceManager::kNoSurfaceID && codec == kCodecH264 &&
+         AVDACodecAllocator::Get().IsAnyRegisteredAVDA() &&
+         (base::android::BuildInfo::GetInstance()->sdk_int() <= 18 ||
+          base::SysInfo::IsLowEndDevice());
+}
+
 }  // namespace
 
-static base::LazyInstance<AVDACodecAllocator>::Leaky g_avda_codec_allocator =
-    LAZY_INSTANCE_INITIALIZER;
-
 // AVDAManager manages shared resources for a number of AVDA instances.
 // Its responsibilities include:
 //  - Starting and stopping a shared "construction" thread for instantiating and
 //    releasing MediaCodecs.
 //  - Detecting when a task has hung on the construction thread so AVDAs can
 //    stop using it.
 //  - Running a RepeatingTimer so that AVDAs can get a regular callback to
 //    DoIOTask().
 //  - Tracking the allocation of surfaces to AVDAs and delivering callbacks when
 //    surfaces are released.
@@ skipping 27 matching lines @@
     if (timer_running_) {
       pending_erase_.insert(avda);
       return;
     }
 
     timer_avda_instances_.erase(avda);
     if (timer_avda_instances_.empty())
       io_timer_.Stop();
   }
 
-  // |avda| 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 |avda| 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 == SurfaceManager::kNoSurfaceID)
-      return true;
-
-    auto iter = surface_waiter_map_.find(surface_id);
-    if (iter == surface_waiter_map_.end()) {
-      // SurfaceView isn't allocated.  Succeed.
-      surface_waiter_map_[surface_id].owner = avda;
-      return true;
-    }
-
-    // SurfaceView is already allocated.
-    if (iter->second.waiter) {
-      // 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;
-
-    AndroidVideoDecodeAccelerator* waiter = iter->second.waiter;
-    if (!waiter) {
-      // No waiter -- remove the record and return explicitly since |iter| is
-      // no longer valid.
-      surface_waiter_map_.erase(iter);
-      return;
-    }
-
-    // Promote |waiter| to be the owner.
-    iter->second.owner = waiter;
-    iter->second.waiter = nullptr;
-    waiter->OnSurfaceAvailable(true);
-  }
-
-  // On low end devices (< KitKat is always low-end due to buggy MediaCodec),
-  // defer the surface creation until the codec is actually used if we know no
-  // software fallback exists.
-  bool ShouldDeferSurfaceCreation(int surface_id, VideoCodec codec) {
-    return surface_id == SurfaceManager::kNoSurfaceID && codec == kCodecH264 &&
-           g_avda_codec_allocator.Get().IsAnyRegisteredAVDA() &&
-           (base::android::BuildInfo::GetInstance()->sdk_int() <= 18 ||
-            base::SysInfo::IsLowEndDevice());
-  }
-
  private:
   friend struct base::DefaultLazyInstanceTraits<AVDAManager>;
 
   AVDAManager() {}
   ~AVDAManager() { NOTREACHED(); }
 
   void RunTimer() {
     {
       // Call out to all AVDA instances, some of which may attempt to remove
       // themselves from the list during this operation; those removals will be
       // deferred until after all iterations are complete.
       base::AutoReset<bool> scoper(&timer_running_, true);
       for (auto* avda : timer_avda_instances_)
         avda->DoIOTask(false);
     }
 
     // Take care of any deferred erasures.
     for (auto* avda : pending_erase_)
       StopTimer(avda);
     pending_erase_.clear();
 
     // TODO(dalecurtis): We may want to consider chunking this if task execution
     // takes too long for the combined timer.
   }
 
   // All AVDA instances that would like us to poll DoIOTask.
   std::set<AndroidVideoDecodeAccelerator*> timer_avda_instances_;
 
-  struct OwnerRecord {
-    AndroidVideoDecodeAccelerator* owner = nullptr;
-    AndroidVideoDecodeAccelerator* waiter = nullptr;
-  };
-  // [surface id] = OwnerRecord for that surface.
-  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;
   std::set<AndroidVideoDecodeAccelerator*> pending_erase_;
 
   // Repeating timer responsible for draining pending IO to the codecs.
   base::RepeatingTimer io_timer_;
 
   base::ThreadChecker thread_checker_;
 
   DISALLOW_COPY_AND_ASSIGN(AVDAManager);
 };
 
 static base::LazyInstance<AVDAManager>::Leaky g_avda_manager =
     LAZY_INSTANCE_INITIALIZER;
 
-AndroidVideoDecodeAccelerator::CodecConfig::CodecConfig() {}
-
-AndroidVideoDecodeAccelerator::CodecConfig::~CodecConfig() {}
-
 AndroidVideoDecodeAccelerator::BitstreamRecord::BitstreamRecord(
     const BitstreamBuffer& bitstream_buffer)
     : buffer(bitstream_buffer) {
   if (buffer.id() != -1)
     memory.reset(new SharedMemoryRegion(buffer, true));
 }
 
 AndroidVideoDecodeAccelerator::BitstreamRecord::BitstreamRecord(
     BitstreamRecord&& other)
     : buffer(std::move(other.buffer)), memory(std::move(other.memory)) {}
@@ skipping 14 matching lines @@
       cdm_registration_id_(0),
       pending_input_buf_index_(-1),
       deferred_initialization_pending_(false),
       codec_needs_reset_(false),
       defer_surface_creation_(false),
       weak_this_factory_(this) {}
 
 AndroidVideoDecodeAccelerator::~AndroidVideoDecodeAccelerator() {
   DCHECK(thread_checker_.CalledOnValidThread());
   g_avda_manager.Get().StopTimer(this);
-  g_avda_codec_allocator.Get().StopThread(this);
+  AVDACodecAllocator::Get().StopThread(this);
 
 #if defined(ENABLE_MOJO_MEDIA_IN_GPU_PROCESS)
   if (!media_drm_bridge_cdm_context_)
     return;
 
   DCHECK(cdm_registration_id_);
 
   // Cancel previously registered callback (if any).
   media_drm_bridge_cdm_context_->SetMediaCryptoReadyCB(
       MediaDrmBridgeCdmContext::MediaCryptoReadyCB());
@@ skipping 58 matching lines @@
     DLOG(ERROR) << "Failed to get gles2 decoder instance.";
     return false;
   }
 
   // SetSurface() can't be called before Initialize(), so we pick up our first
   // surface ID from the codec configuration.
   DCHECK(!pending_surface_id_);
 
   // If we're low on resources, we may decide to defer creation of the surface
   // until the codec is actually used.
-  if (g_avda_manager.Get().ShouldDeferSurfaceCreation(config_.surface_id,
-                                                      codec_config_->codec_)) {
+  if (ShouldDeferSurfaceCreation(config_.surface_id, codec_config_->codec_)) {
     DCHECK(!deferred_initialization_pending_);
-
     // We should never be here if a SurfaceView is required.
     DCHECK_EQ(config_.surface_id, SurfaceManager::kNoSurfaceID);
-    DCHECK(g_avda_manager.Get().AllocateSurface(config_.surface_id, this));

[liberato (no reviews please), 2016/11/17 07:28:19]

where did this go?

[watk, 2016/11/17 20:37:05]

Ah, I added a unit test that mostly obseletes this. (And dcheng@ didn't like
this DCHECK in Dale's last CL).

-
     defer_surface_creation_ = true;
     NotifyInitializationComplete(true);
     return true;
   }
 
   // We signaled that we support deferred initialization, so see if the client
   // does also.
   deferred_initialization_pending_ = config.is_deferred_initialization_allowed;
   if (config_.is_encrypted && !deferred_initialization_pending_) {
     DLOG(ERROR) << "Deferred initialization must be used for encrypted streams";
     return false;
   }
 
-  if (g_avda_manager.Get().AllocateSurface(config_.surface_id, this)) {
-    // We have successfully owned the surface, so finish initialization now.
+  if (AVDACodecAllocator::Get().AllocateSurface(this, config_.surface_id)) {
+    // We now own the surface, so finish initialization.
     return InitializePictureBufferManager();
   }
 
   // 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_);
   DCHECK(!defer_surface_creation_);
 
   if (!success || !InitializePictureBufferManager()) {
     NotifyInitializationComplete(false);
     deferred_initialization_pending_ = false;
   }
 }
 
 bool AndroidVideoDecodeAccelerator::InitializePictureBufferManager() {
   if (!make_context_current_cb_.Run()) {
     LOG(ERROR) << "Failed to make this decoder's GL context current.";
     return false;
   }
 
   codec_config_->surface_ =
       picture_buffer_manager_.Initialize(config_.surface_id);
   if (codec_config_->surface_.IsEmpty())
     return false;
 
-  on_destroying_surface_cb_ =
-      base::Bind(&AndroidVideoDecodeAccelerator::OnDestroyingSurface,
-                 weak_this_factory_.GetWeakPtr());
-  AVDASurfaceTracker::GetInstance()->RegisterOnDestroyingSurfaceCallback(
-      on_destroying_surface_cb_);
-
-  if (!g_avda_codec_allocator.Get().StartThread(this))
+  if (!AVDACodecAllocator::Get().StartThread(this))
     return false;
 
   // If we are encrypted, then we aren't able to create the codec yet.
   if (config_.is_encrypted) {
     InitializeCdm();
     return true;
   }
 
   if (deferred_initialization_pending_ || defer_surface_creation_) {
     defer_surface_creation_ = false;
@@ skipping 483 matching lines @@
 
   if (state_ == SURFACE_DESTROYED || defer_surface_creation_)
     NotifyFlushDone();
   else
     StartCodecDrain(DRAIN_FOR_FLUSH);
 }
 
 void AndroidVideoDecodeAccelerator::ConfigureMediaCodecAsynchronously() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  // It's probably okay just to return here, since the codec will be configured
-  // asynchronously.  It's unclear that any state for the new request could
-  // be different, unless somebody modifies |codec_config_| while we're already
-  // waiting for a codec.  One shouldn't do that for thread safety.
   DCHECK_NE(state_, WAITING_FOR_CODEC);
-
   state_ = WAITING_FOR_CODEC;
 
-  // Tell the picture buffer manager that we're changing codecs.  The codec
-  // itself could be used normally, since we don't replace it until we're back
-  // on the main thread.  However, if we're using an output surface, then the
-  // incoming codec might access that surface while the main thread is drawing.
-  // Telling the manager to forget the codec avoids this.
   if (media_codec_) {
-    ReleaseMediaCodec();
+    AVDACodecAllocator::Get().ReleaseMediaCodec(
+        std::move(media_codec_), codec_config_->task_type_, config_.surface_id);
     picture_buffer_manager_.CodecChanged(nullptr);
   }
 
-  codec_config_->task_type_ =
-      g_avda_codec_allocator.Get().TaskTypeForAllocation();
-  if (codec_config_->task_type_ == AVDACodecAllocator::TaskType::FAILED_CODEC) {
+  codec_config_->task_type_ = AVDACodecAllocator::Get().TaskTypeForAllocation();
+  if (codec_config_->task_type_ == TaskType::FAILED_CODEC) {
     // If there is no free thread, then just fail.
     OnCodecConfigured(nullptr);
     return;
   }
 
   // If autodetection is disallowed, fall back to Chrome's software decoders
   // instead of using the software decoders provided by MediaCodec.
-  if (codec_config_->task_type_ == AVDACodecAllocator::TaskType::SW_CODEC &&
+  if (codec_config_->task_type_ == TaskType::SW_CODEC &&
       IsMediaCodecSoftwareDecodingForbidden()) {
     OnCodecConfigured(nullptr);
     return;
   }
 
-  base::PostTaskAndReplyWithResult(
-      g_avda_codec_allocator.Get()
-          .TaskRunnerFor(codec_config_->task_type_)
-          .get(),
-      FROM_HERE,
-      base::Bind(&AndroidVideoDecodeAccelerator::ConfigureMediaCodecOnAnyThread,
-                 codec_config_),
-      base::Bind(&AndroidVideoDecodeAccelerator::OnCodecConfigured,
-                 weak_this_factory_.GetWeakPtr()));
+  AVDACodecAllocator::Get().CreateMediaCodecAsync(
+      weak_this_factory_.GetWeakPtr(), codec_config_);
 }
 
 bool AndroidVideoDecodeAccelerator::ConfigureMediaCodecSynchronously() {
   state_ = WAITING_FOR_CODEC;
 
-  ReleaseMediaCodec();
-
-  codec_config_->task_type_ =
-      g_avda_codec_allocator.Get().TaskTypeForAllocation();
-  if (codec_config_->task_type_ == AVDACodecAllocator::TaskType::FAILED_CODEC) {
+  codec_config_->task_type_ = AVDACodecAllocator::Get().TaskTypeForAllocation();
+  if (codec_config_->task_type_ == TaskType::FAILED_CODEC) {
     OnCodecConfigured(nullptr);
     return false;
   }
 
   std::unique_ptr<VideoCodecBridge> media_codec =
-      ConfigureMediaCodecOnAnyThread(codec_config_);
+      AVDACodecAllocator::Get().CreateMediaCodecSync(codec_config_);
   OnCodecConfigured(std::move(media_codec));
   return !!media_codec_;
 }
 
-std::unique_ptr<VideoCodecBridge>
-AndroidVideoDecodeAccelerator::ConfigureMediaCodecOnAnyThread(
-    scoped_refptr<CodecConfig> codec_config) {
-  TRACE_EVENT0("media", "AVDA::ConfigureMediaCodec");
-
-  jobject media_crypto = codec_config->media_crypto_
-                             ? codec_config->media_crypto_->obj()
-                             : nullptr;
-
-  // |needs_protected_surface_| implies encrypted stream.
-  DCHECK(!codec_config->needs_protected_surface_ || media_crypto);
-
-  const bool require_software_codec =
-      codec_config->task_type_ == AVDACodecAllocator::TaskType::SW_CODEC;
-
-  std::unique_ptr<VideoCodecBridge> codec(VideoCodecBridge::CreateDecoder(
-      codec_config->codec_, codec_config->needs_protected_surface_,
-      codec_config->initial_expected_coded_size_,
-      codec_config->surface_.j_surface().obj(), media_crypto,
-      codec_config->csd0_, codec_config->csd1_, true, require_software_codec));
-
-  return codec;
-}
-
 void AndroidVideoDecodeAccelerator::OnCodecConfigured(
     std::unique_ptr<VideoCodecBridge> media_codec) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(state_ == WAITING_FOR_CODEC || state_ == SURFACE_DESTROYED);
 
   // If we are supposed to notify that initialization is complete, then do so
   // now.  Otherwise, this is a reconfiguration.
   if (deferred_initialization_pending_) {
     // Losing the output surface is not considered an error state, so notify
     // success. The client will destroy this soon.
@@ skipping 169 matching lines @@
     StartCodecDrain(DRAIN_FOR_RESET);
   } else {
     ResetCodecState();
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE, base::Bind(&AndroidVideoDecodeAccelerator::NotifyResetDone,
                               weak_this_factory_.GetWeakPtr()));
   }
 }
 
 void AndroidVideoDecodeAccelerator::SetSurface(int32_t surface_id) {
+  DVLOG(1) << __func__;
+  DCHECK(thread_checker_.CalledOnValidThread());
+
   if (surface_id == config_.surface_id) {
     pending_surface_id_.reset();
     return;
   }
 
   // Surface changes never take effect immediately, they will be handled during
   // DequeOutput() once we get to a good switch point or immediately during an
-  // OnDestroyingSurface() call.
+  // OnSurfaceDestroyed() call.
   pending_surface_id_ = surface_id;
 }
 
 void AndroidVideoDecodeAccelerator::Destroy() {
   DVLOG(1) << __FUNCTION__;
   DCHECK(thread_checker_.CalledOnValidThread());
 
   picture_buffer_manager_.Destroy(output_picture_buffers_);
 
   client_ = nullptr;
 
   // Some VP8 files require a complete MediaCodec drain before we can call
   // MediaCodec.flush() or MediaCodec.release(). http://crbug.com/598963. In
   // that case, postpone ActualDestroy() until after the drain.
   if (media_codec_ && codec_config_->codec_ == kCodecVP8) {
     // Clear |pending_bitstream_records_|.
     while (!pending_bitstream_records_.empty())
       pending_bitstream_records_.pop();
 
     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_);
-  }
-
   // 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_manager.Get().DeallocateSurface(config_.surface_id, this);
+  AVDACodecAllocator::Get().DeallocateSurface(this, config_.surface_id);
 
   // 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();
+  g_avda_manager.Get().StopTimer(this);

[liberato (no reviews please), 2016/11/17 07:28:19]

hrm, that seems like an improvement.

   if (media_codec_) {
-    g_avda_manager.Get().StopTimer(this);
-    ReleaseMediaCodec();
+    AVDACodecAllocator::Get().ReleaseMediaCodec(
+        std::move(media_codec_), codec_config_->task_type_, config_.surface_id);
   }
 
   delete this;
 }
 
 bool AndroidVideoDecodeAccelerator::TryToSetupDecodeOnSeparateThread(
     const base::WeakPtr<Client>& decode_client,
     const scoped_refptr<base::SingleThreadTaskRunner>& decode_task_runner) {
   return false;
 }
 
 const gfx::Size& AndroidVideoDecodeAccelerator::GetSize() const {
   return size_;
 }
 
 base::WeakPtr<gpu::gles2::GLES2Decoder>
 AndroidVideoDecodeAccelerator::GetGlDecoder() const {
   return get_gles2_decoder_cb_.Run();
 }
 
-void AndroidVideoDecodeAccelerator::OnDestroyingSurface(int surface_id) {
+void AndroidVideoDecodeAccelerator::OnSurfaceDestroyed() {
+  DVLOG(1) << __func__;
+  TRACE_EVENT0("media", "AVDA::OnSurfaceDestroyed");
   DCHECK(thread_checker_.CalledOnValidThread());
-  TRACE_EVENT0("media", "AVDA::OnDestroyingSurface");
-  DVLOG(1) << __FUNCTION__ << " surface_id: " << surface_id;
-
-  if (surface_id != config_.surface_id)
-    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.
     DCHECK_NE(config_.surface_id, SurfaceManager::kNoSurfaceID);
 
     // The leaving fullscreen notification may come in before this point.
     if (pending_surface_id_)
       DCHECK_EQ(pending_surface_id_.value(), SurfaceManager::kNoSurfaceID);
 
     pending_surface_id_ = SurfaceManager::kNoSurfaceID;
     UpdateSurface();
     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_) {
-    ReleaseMediaCodec();
+    AVDACodecAllocator::Get().ReleaseMediaCodec(
+        std::move(media_codec_), codec_config_->task_type_, config_.surface_id);
     picture_buffer_manager_.CodecChanged(media_codec_.get());
   }
+
   // If we're draining, signal completion now because the drain can no longer
   // proceed.
   if (drain_type_ != DRAIN_TYPE_NONE)
     OnDrainCompleted();
 }
 
 void AndroidVideoDecodeAccelerator::InitializeCdm() {
   DVLOG(2) << __FUNCTION__ << ": " << config_.cdm_id;
 
 #if !defined(ENABLE_MOJO_MEDIA_IN_GPU_PROCESS)
@@ skipping 110 matching lines @@
   } else {
     most_recent_work_ = now;
   }
 
   if (should_be_running)
     g_avda_manager.Get().StartTimer(this);
   else
     g_avda_manager.Get().StopTimer(this);
 }
 
-void AndroidVideoDecodeAccelerator::ReleaseMediaCodec() {
-  if (!media_codec_)
-    return;
-
-  // Post it to the same thread as was used to allocate it, and it'll get freed
-  // if that thread isn't hung waiting on mediaserver.  We can't release it
-  // on this thread since it might hang up.
-  g_avda_codec_allocator.Get()
-      .TaskRunnerFor(codec_config_->task_type_)
-      ->DeleteSoon(FROM_HERE, media_codec_.release());
-}
-
 // static
 VideoDecodeAccelerator::Capabilities
 AndroidVideoDecodeAccelerator::GetCapabilities(
     const gpu::GpuPreferences& gpu_preferences) {
   Capabilities capabilities;
   SupportedProfiles& profiles = capabilities.supported_profiles;
 
   if (MediaCodecUtil::IsVp8DecoderAvailable()) {
     SupportedProfile profile;
     profile.profile = VP8PROFILE_ANY;
@@ skipping 90 matching lines @@
   // Prevent MediaCodec from using its internal software decoders when we have
   // more secure and up to date versions in the renderer process.
   return !config_.is_encrypted && (codec_config_->codec_ == kCodecVP8 ||
                                    codec_config_->codec_ == kCodecVP9);
 }
 
 bool AndroidVideoDecodeAccelerator::UpdateSurface() {
   DCHECK(pending_surface_id_);
   DCHECK_NE(config_.surface_id, pending_surface_id_.value());
 
+  // Deallocate our current surface.
+  AVDACodecAllocator::Get().DeallocateSurface(this, config_.surface_id);

[liberato (no reviews please), 2016/11/17 07:28:19]

one of these has to be "no surface", i think.  works okay, but might be worth a
comment.

good catch, btw.

[watk, 2016/11/17 20:37:05]

sg will do

+  config_.surface_id = pending_surface_id_.value();
+  pending_surface_id_.reset();
+
+  // TODO(watk): Fix this so we can wait for the new surface to be allocated.
+  if (!AVDACodecAllocator::Get().AllocateSurface(this, config_.surface_id)) {
+    NOTIFY_ERROR(PLATFORM_FAILURE, "Failed to allocate the new surface");
+    return false;
+  }
+
   // Ensure the current context is active when switching surfaces; we may need
   // to create a new texture.
   if (!make_context_current_cb_.Run()) {
     NOTIFY_ERROR(PLATFORM_FAILURE,
                  "Failed to make this decoder's GL context current when "
                  "switching surfaces.");
     return false;
   }
 
-  config_.surface_id = pending_surface_id_.value();
   codec_config_->surface_ =
-      picture_buffer_manager_.Initialize(pending_surface_id_.value());
+      picture_buffer_manager_.Initialize(config_.surface_id);
   if (codec_config_->surface_.IsEmpty()) {
     NOTIFY_ERROR(PLATFORM_FAILURE, "Failed to switch surfaces.");
     return false;
   }
 
   if (media_codec_ &&
       !media_codec_->SetSurface(codec_config_->surface_.j_surface().obj())) {
     NOTIFY_ERROR(PLATFORM_FAILURE, "MediaCodec failed to switch surfaces.");
     return false;
   }
 
-  pending_surface_id_.reset();
   return true;
 }
 
 }  // namespace media