[Ozone] Extends the lifetime of VaapiWrapper

content/common/gpu/media/vaapi_video_decode_accelerator.cc

Index: content/common/gpu/media/vaapi_video_decode_accelerator.cc
diff --git a/content/common/gpu/media/vaapi_video_decode_accelerator.cc b/content/common/gpu/media/vaapi_video_decode_accelerator.cc
index 704f7f2257382e8536d9cf894342e03bce46b3c0..3bf69ad7712bc9efe2842c9d4d8cb86f56bc82a9 100644
--- a/content/common/gpu/media/vaapi_video_decode_accelerator.cc
+++ b/content/common/gpu/media/vaapi_video_decode_accelerator.cc
@@ -729,8 +729,8 @@ void VaapiVideoDecodeAccelerator::AssignPictureBuffers(
              << " to texture id: " << buffers[i].texture_id()
              << " VASurfaceID: " << va_surface_ids[i];
 
-    linked_ptr<VaapiPicture> picture(VaapiPicture::CreatePicture(
-        vaapi_wrapper_.get(), make_context_current_, buffers[i].id(),
+    scoped_refptr<VaapiPicture> picture(VaapiPicture::CreatePicture(
+        vaapi_wrapper_, make_context_current_, buffers[i].id(),
         buffers[i].texture_id(), requested_pic_size_));
 
     scoped_refptr<gl::GLImage> image = picture->GetImageToBind();
@@ -930,6 +930,8 @@ void VaapiVideoDecodeAccelerator::Cleanup() {
 
   client_ptr_factory_.reset();
   weak_this_factory_.InvalidateWeakPtrs();
+  pictures_.clear();

[kalyank, 2015/11/14 10:42:06]

I have played around with this a bit and issue is that DrmPicture holds on to
the refs of GLImage(As you mentioned) and pixmap_. We should release them when
VaapiVideoDecodeAccelerator decides to clear the pictures. We could have
ClearResources call in VaapiPicture which would be called by
VaapiDecodeAccelerator before it clears the pictures cache. In SurfaceOzone, we
would still have a ref to any in use GLImage(which has a ref to NativePixmap).
So, the image and native pixmap will not be destroyed till SurfaceOzone is using
them.

In VaapiDrmPicture::ClearResources, we should do what ever we are doing
currently in the destructor along with releasing references to gl_image_ and
pixmap_. However, bit confusing thing is that we need to hold onto
processed_pixmap_ here as VaapiDecodeAccelerator doesn't really manage it but
GbmPixmap and GLImage doesn't have a ref to it. 

[Pawel Osciak, 2015/11/16 04:24:35]

If there are any references to VaapiDrmPicture being held by ScalingCallback, we
shouldn't be releasing them at this point. But we should be ok already,
~VaapiDrmPicture will free them when all references to it are dropped.
I'm confused why we'd need this. If I understand this CL correctly, this extends
the lifetime of VaapiDrmPicture for it to stay alive while it's waiting for the
ScalingCallback to execute. After that, the last reference to VaapiDrmPicture
will be dropped and GLImage and Pixmap should be released in the destructor of
VaapiDrmPicture.

VaapiVideoDecodeAccelerator will not be holding any references to
VaapiDrmPicture since we released them here and only VaapiDrmPicture holds
references to GLImage and Pixmap, so VaapiDrmPicture should be responsible for
releasing them (and it does so in ~VaapiDrmPicture).

[william.xie1, 2015/11/16 08:07:26]

Done.

[william.xie1, 2015/11/16 08:07:26]

Done.

[kalyank, 2015/11/18 00:21:43]

We want to ensure VaapiDrmPicture is alive while any scaling callbacks are in 
use. For that, we changed callback to hold a ref of VaapiDrmPicture. Now:
1) VaapiVideoDecodeAccelerator and Callback has a reference to VaapiDrmPicture.
2)VaapiDrmPicture holds a reference to the GLImage and Native pixmap it creates.
  GLSurfaceOzone holds a reference to the same GLImage (In case its queued to be
  displayed). GLImage internally holds a reference to the Native Pixmap its 
  initialized with.
3)GLSurfaceOzone will not hold any reference to GLImage, once it's done using 
  it. However, GLImage is not destroyed as VaapiDrmPicture still holds a 
  reference to it.
4)As GLImage is not destroyed, we don't release the NativePixmap it's 
  initialized with. This results in keeping the Callback alive. 

Thus, GLImage, Native Pixmap and VaapiDrmPicture will never be destroyed.

Releasing the resources in use by VaapiDrmPicture here, ensures that we clear
all the resources once SurfaceOzone is done using the GLImage.

I am not sure how easy is to separate the scaling pixmap logic out of DrmPicture
and see if we can ensure only VaapiWrapper is alive (As it manages the VPP
context).

+  vaapi_wrapper_ = nullptr;
 
   // Signal all potential waiters on the decoder_thread_, let them early-exit,
   // as we've just moved to the kDestroying state, and wait for all tasks
@@ -948,6 +950,7 @@ void VaapiVideoDecodeAccelerator::Destroy() {
   DCHECK_EQ(message_loop_, base::MessageLoop::current());
   Cleanup();
   delete this;
+  LOG(WARNING) << "!!!!!!!!!!!!!!!qxie2:" << __FUNCTION__;
 }
 
 bool VaapiVideoDecodeAccelerator::CanDecodeOnIOThread() {