Revert 144496 - Fix several shutdown/error-path bugs in VAVDA and avoid unnecessary MakeCurrent() calls:

- GpuVideoDecodeAcceleratorHost is already AddRoute'd to
  GpuChannelHost on creation, so
  CommandBufferProxyImpl::OnChannelError doesn't have to call
  GVDAH::OnChannelError as well (was causing double-notifications
  of errors to GVDAH).

- GpuVideoDecodeAcceleratorHost::Send() would DCHECK that no
  error had previously been reported on the channel, but that's
  incorrect because its client will likely still have async
  posted tasks from before the error notifications.  Now instead
  of DCHECK'ing simply early-return after an error was seen.

- gpu_video_decode_accelerator.cc:MakeDecoderContextCurrent()
  used to unconditionally bind the GpuCommandBufferStub* into its
  closure, but nothing guarantees the lifetime of the stub; in
  fact context loss will cause the stub to be deleted so this
  could crash as well.  Now take a WeakPtr to the stub and check
  its validity before every use (and return false if invalid to
  allow clients to avoid assuming context currency).

- GpuVideoDecodeAccelerator is now a
  GpuCommandBufferStub::DestructionObserver to avoid attempting
  further work once the stub is about to be deleted.

- Removed a bunch of unnecessary MakeCurrent() calls; VAAPI_*
  calls don't need it, and are sometimes made on the decoder (not
  main) thread, from which it is incorrect to call GL-related
  methods such as MakeCurrent() anyway.

BUG=134020
TEST=manual playback of multiple HW-decoded .mp4's doesn't crash anything when different tabs are closed.


Review URL: https://chromiumcodereview.appspot.com/10668030

TBR=fischman@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10689011

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@144503 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 28 insertions, 11 deletions

diff --git a/content/common/gpu/media/vaapi_h264_decoder.cc b/content/common/gpu/media/vaapi_h264_decoder.cc
index b35a5e3..78859f8 100644
--- a/content/common/gpu/media/vaapi_h264_decoder.cc
+++ b/content/common/gpu/media/vaapi_h264_decoder.cc
@@ -395,28 +395,24 @@ void VaapiH264Decoder::Reset() {
 }
 
 void VaapiH264Decoder::Destroy() {
+  VAStatus va_res;
+
   if (state_ == kUninitialized)
     return;
 
-  VAStatus va_res;
-  bool destroy_surfaces = false;
   switch (state_) {
     case kDecoding:
     case kAfterReset:
     case kError:
-      destroy_surfaces = true;
+      DestroyVASurfaces();
       // fallthrough
     case kInitialized:
       if (!make_context_current_.Run())
         break;
-      if (destroy_surfaces)
-        DestroyVASurfaces();
       va_res = VAAPI_DestroyConfig(va_display_, va_config_id_);
-      // TODO(fischman,posciak): call vaTerminate when we figure out why it
-      // crashes.
-      // VA_LOG_ON_ERROR(va_res, "vaDestroyConfig failed");
-      // va_res = VAAPI_Terminate(va_display_);
-      // VA_LOG_ON_ERROR(va_res, "vaTerminate failed");
+      VA_LOG_ON_ERROR(va_res, "vaDestroyConfig failed");
+      va_res = VAAPI_Terminate(va_display_);
+      VA_LOG_ON_ERROR(va_res, "vaTerminate failed");
       // fallthrough
     case kUninitialized:
       break;
@@ -581,6 +577,8 @@ bool VaapiH264Decoder::CreateVASurfaces() {
   DCHECK_NE(pic_width_, -1);
   DCHECK_NE(pic_height_, -1);
   DCHECK_EQ(state_, kInitialized);
+  if (!make_context_current_.Run())
+    return false;
 
   // Allocate VASurfaces in driver.
   VAStatus va_res = VAAPI_CreateSurfaces(va_display_, pic_width_,
@@ -590,6 +588,7 @@ bool VaapiH264Decoder::CreateVASurfaces() {
   VA_SUCCESS_OR_RETURN(va_res, "vaCreateSurfaces failed", false);
 
   DCHECK(decode_surfaces_.empty());
+
   // And create a context associated with them.
   va_res = VAAPI_CreateContext(va_display_, va_config_id_,
                                pic_width_, pic_height_, VA_PROGRESSIVE,
@@ -604,6 +603,9 @@ void VaapiH264Decoder::DestroyVASurfaces() {
   DCHECK(state_ == kDecoding || state_ == kError || state_ == kAfterReset);
   decode_surfaces_.clear();
 
+  if (!make_context_current_.Run())
+    return;
+
   VAStatus va_res = VAAPI_DestroyContext(va_display_, va_context_id_);
   VA_LOG_ON_ERROR(va_res, "vaDestroyContext failed");
 
@@ -716,6 +718,9 @@ VaapiH264Decoder::DecodeSurface* VaapiH264Decoder::UnassignSurfaceFromPoC(
 
 // Fill a VAPictureParameterBufferH264 to be later sent to the HW decoder.
 bool VaapiH264Decoder::SendPPS() {
+  if (!make_context_current_.Run())
+    return false;
+
   const H264PPS* pps = parser_.GetPPS(curr_pps_id_);
   DCHECK(pps);
 
@@ -813,6 +818,9 @@ bool VaapiH264Decoder::SendPPS() {
 
 // Fill a VAIQMatrixBufferH264 to be later sent to the HW decoder.
 bool VaapiH264Decoder::SendIQMatrix() {
+  if (!make_context_current_.Run())
+    return false;
+
   const H264PPS* pps = parser_.GetPPS(curr_pps_id_);
   DCHECK(pps);
 
@@ -859,6 +867,9 @@ bool VaapiH264Decoder::SendIQMatrix() {
 }
 
 bool VaapiH264Decoder::SendVASliceParam(H264SliceHeader* slice_hdr) {
+  if (!make_context_current_.Run())
+    return false;
+
   const H264PPS* pps = parser_.GetPPS(slice_hdr->pic_parameter_set_id);
   DCHECK(pps);
 
@@ -960,7 +971,11 @@ bool VaapiH264Decoder::SendVASliceParam(H264SliceHeader* slice_hdr) {
   return true;
 }
 
-bool VaapiH264Decoder::SendSliceData(const uint8* ptr, size_t size) {
+bool VaapiH264Decoder::SendSliceData(const uint8* ptr, size_t size)
+{
+    if (!make_context_current_.Run())
+      return false;
+
     // Can't help it, blame libva...
     void* non_const_ptr = const_cast<uint8*>(ptr);
 
@@ -993,6 +1008,8 @@ bool VaapiH264Decoder::QueueSlice(H264SliceHeader* slice_hdr) {
 bool VaapiH264Decoder::DecodePicture() {
   DCHECK(!frame_ready_at_hw_);
   DCHECK(curr_pic_.get());
+  if (!make_context_current_.Run())
+    return false;
 
   static const size_t kMaxVABuffers = 32;
   DCHECK_LE(pending_va_bufs_.size(), kMaxVABuffers);