Fix broken SurfaceView usage on < M devices.

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 293 matching lines @@
       state_(NO_ERROR),
       picturebuffers_requested_(false),
       picture_buffer_manager_(this),
       drain_type_(DRAIN_TYPE_NONE),
       media_drm_bridge_cdm_context_(nullptr),
       cdm_registration_id_(0),
       pending_input_buf_index_(-1),
       deferred_initialization_pending_(false),
       codec_needs_reset_(false),
       defer_surface_creation_(false),
-      surface_id_(SurfaceManager::kNoSurfaceID),
       weak_this_factory_(this) {}
 
 AndroidVideoDecodeAccelerator::~AndroidVideoDecodeAccelerator() {
   DCHECK(thread_checker_.CalledOnValidThread());
   g_avda_manager.Get().StopTimer(this);
   g_avda_codec_allocator.Get().StopThread(this);
 
 #if defined(ENABLE_MOJO_MEDIA_IN_GPU_PROCESS)
   if (!media_drm_bridge_cdm_context_)
     return;
@@ skipping 58 matching lines @@
              << " is not hardware accelerated";
     return false;
   }
 
   auto gles_decoder = get_gles2_decoder_cb_.Run();
   if (!gles_decoder) {
     DLOG(ERROR) << "Failed to get gles2 decoder instance.";
     return false;
   }
 
-  // If SetSurface() was called before initialize, pick up the surface.
-  if (pending_surface_id_) {
-    surface_id_ = pending_surface_id_.value();
-    pending_surface_id_.reset();
-  }
+  // SetSurface() can't be called before Initialize(), so we pick up our first
+  // surface ID from the codec configuration.
+  DCHECK(!pending_surface_id_);

[dcheng, 2016/11/16 00:58:45]

This isn't a new issue, but it makes me concerned that we're doing all these
DCHECKs on values that ultimately are controlled by the renderer, which is less
privileged by the GPU process. Will failing any of these DCHECKs horribly break
things in the gpu?

[DaleCurtis, 2016/11/16 01:04:29]

pending_surface_id_ is a base::Optional() so this is just checking that we have
no value for it. Since VDA construction and Initialize() are called in one fell
swoop, this is a textbook "impossible" DCHECK :) None of these DCHECKS should
affect anything in release mode even if they're failed.

If the surface is unknown by the surface manager we'll fail to create a surface
and fail the VDA creation / setSurface call via notifyError().

 
   // 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(surface_id_,
+  if (g_avda_manager.Get().ShouldDeferSurfaceCreation(config_.surface_id,
                                                       codec_config_->codec_)) {
     DCHECK(!deferred_initialization_pending_);
 
     // We should never be here if a SurfaceView is required.
-    DCHECK_EQ(surface_id_, SurfaceManager::kNoSurfaceID);
-    DCHECK(g_avda_manager.Get().AllocateSurface(surface_id_, this));
+    DCHECK_EQ(config_.surface_id, SurfaceManager::kNoSurfaceID);
+    DCHECK(g_avda_manager.Get().AllocateSurface(config_.surface_id, this));

[dcheng, 2016/11/16 00:58:45]

Why do we call this in a DCHECK()? Isn't it a no-op anyway?

[DaleCurtis, 2016/11/16 01:04:29]

It is with the current code, it's prevention against future code modifications
to AllocateSurface where it might try to manage allocations for SurfaceTextures.

[dcheng, 2016/11/16 01:06:33]

It seems weird to call something called AllocateX in a dcheck though: at most, I
would have expected this to be querying some state bits?

[DaleCurtis, 2016/11/16 01:16:15]

Will leave for now and let watk@ remove with upcoming changes. This is mostly
going to be scrapped in the near future.

 
     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(surface_id_, this)) {
+  if (g_avda_manager.Get().AllocateSurface(config_.surface_id, this)) {
     // We have successfully owned the surface, so finish initialization now.
     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(surface_id_);
+  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))
@@ skipping 769 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) {
-  if (surface_id == surface_id_) {
+  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.
   pending_surface_id_ = surface_id;
 }
 
@@ skipping 23 matching lines @@
   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(surface_id_, this);
+  g_avda_manager.Get().DeallocateSurface(config_.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.
   weak_this_factory_.InvalidateWeakPtrs();
   if (media_codec_) {
     g_avda_manager.Get().StopTimer(this);
     ReleaseMediaCodec();
   }
 
@@ skipping 13 matching lines @@
 base::WeakPtr<gpu::gles2::GLES2Decoder>
 AndroidVideoDecodeAccelerator::GetGlDecoder() const {
   return get_gles2_decoder_cb_.Run();
 }
 
 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_)
+  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(surface_id_, SurfaceManager::kNoSurfaceID);
+    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;
   }
 
@@ skipping 248 matching lines @@
 bool AndroidVideoDecodeAccelerator::IsMediaCodecSoftwareDecodingForbidden()
     const {
   // 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(surface_id_, pending_surface_id_.value());
+  DCHECK_NE(config_.surface_id, pending_surface_id_.value());
 
   // 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;
   }
 
-  surface_id_ = pending_surface_id_.value();
+  config_.surface_id = pending_surface_id_.value();
   codec_config_->surface_ =
       picture_buffer_manager_.Initialize(pending_surface_id_.value());
   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