vaapi: fold wrapper's CreateUnownedSurface() into caller

This CL folds the method VaapiWrapper::CreateUnownedSurface() into its
only caller. This nicely cuts the amount of lines, reduces the scope
of the |va_lock_|, and improves error mgmt by only having one place
for VA_SUCCESS_OR_RETURN (because the VASurface ctor cannot fail).

Also vavda's CloseGpuMemoryBufferHandle is moved to the unnamed
namespace.

Bug: 822346
Cq-Include-Trybots: luci.chromium.try:linux_optional_gpu_tests_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: I7aa690e3d3976540f2edf07600511ca65f76b6f0
Reviewed-on: https://chromium-review.googlesource.com/964591
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544290}

media/gpu/vaapi/va_surface.h

@@ -87,6 +87,7 @@ class VASurface : public base::RefCountedThreadSafe<VASurface> {
  public:
   // Provided by user, will be called when all references to the surface
   // are released.
+  // TODO(mcasas): make this a OnceCallback, https://crbug.com/822346.
   using ReleaseCB = base::Callback<void(VASurfaceID)>;
 
   VASurface(VASurfaceID va_surface_id,

media/gpu/vaapi/vaapi_video_decode_accelerator.cc

@@ -55,13 +55,23 @@ unsigned int GetVaFormatForVideoCodecProfile(VideoCodecProfile profile) {
   return VA_RT_FORMAT_YUV420;
 }
 
-}  // namespace
-
-static void ReportToUMA(VAVDADecoderFailure failure) {
+void ReportToUMA(VAVDADecoderFailure failure) {
   UMA_HISTOGRAM_ENUMERATION("Media.VAVDA.DecoderFailure", failure,
                             VAVDA_DECODER_FAILURES_MAX + 1);
 }
 
+#if defined(USE_OZONE)
+void CloseGpuMemoryBufferHandle(const gfx::GpuMemoryBufferHandle& handle) {
+  for (const auto& fd : handle.native_pixmap_handle.fds) {
+    // Close the fd by wrapping it in a ScopedFD and letting
+    // it fall out of scope.
+    base::ScopedFD scoped_fd(fd.fd);
+  }
+}
+#endif
+
+}  // namespace
+
 #define RETURN_AND_NOTIFY_ON_FAILURE(result, log, error_code, ret) \
   do {                                                             \
     if (!(result)) {                                               \
@@ -610,15 +620,6 @@ void VaapiVideoDecodeAccelerator::AssignPictureBuffers(
 }
 
 #if defined(USE_OZONE)
-static void CloseGpuMemoryBufferHandle(
-    const gfx::GpuMemoryBufferHandle& handle) {
-  for (const auto& fd : handle.native_pixmap_handle.fds) {
-    // Close the fd by wrapping it in a ScopedFD and letting
-    // it fall out of scope.
-    base::ScopedFD scoped_fd(fd.fd);
-  }
-}
-
 void VaapiVideoDecodeAccelerator::ImportBufferForPicture(
     int32_t picture_buffer_id,
     VideoPixelFormat pixel_format,

media/gpu/vaapi/vaapi_wrapper.cc

@@ -824,14 +824,23 @@ scoped_refptr<VASurface> VaapiWrapper::CreateVASurfaceForPixmap(
   va_attribs[1].value.type = VAGenericValueTypePointer;
   va_attribs[1].value.value.p = &va_attrib_extbuf;
 
-  scoped_refptr<VASurface> va_surface = CreateUnownedSurface(
-      BufferFormatToVARTFormat(pixmap->GetBufferFormat()), size, va_attribs);
-  if (!va_surface) {
-    LOG(ERROR) << "Failed to create VASurface for an Ozone NativePixmap";
-    return nullptr;
+  const unsigned int va_format =
+      BufferFormatToVARTFormat(pixmap->GetBufferFormat());
+
+  VASurfaceID va_surface_id = VA_INVALID_ID;
+  {
+    base::AutoLock auto_lock(*va_lock_);
+    VAStatus va_res =
+        vaCreateSurfaces(va_display_, va_format, size.width(), size.height(),
+                         &va_surface_id, 1, &va_attribs[0], va_attribs.size());
+    VA_SUCCESS_OR_RETURN(va_res, "Failed to create unowned VASurface", nullptr);
   }
 
-  return va_surface;
+  // It's safe to use Unretained() here, because the caller takes care of the
+  // destruction order. All the surfaces will be destroyed before VaapiWrapper.
+  return new VASurface(
+      va_surface_id, size, va_format,
+      base::Bind(&VaapiWrapper::DestroySurface, base::Unretained(this)));
 }
 
 bool VaapiWrapper::SubmitBuffer(VABufferType va_buffer_type,
@@ -1280,33 +1289,7 @@ void VaapiWrapper::DestroySurfaces_Locked() {
   va_surface_format_ = 0;
 }
 
-scoped_refptr<VASurface> VaapiWrapper::CreateUnownedSurface(
-    unsigned int va_format,
-    const gfx::Size& size,
-    const std::vector<VASurfaceAttrib>& va_attribs) {
-  base::AutoLock auto_lock(*va_lock_);
-
-  std::vector<VASurfaceAttrib> attribs(va_attribs);
-  VASurfaceID va_surface_id;
-  VAStatus va_res =
-      vaCreateSurfaces(va_display_, va_format, size.width(), size.height(),
-                       &va_surface_id, 1, &attribs[0], attribs.size());
-
-  scoped_refptr<VASurface> va_surface;
-  VA_SUCCESS_OR_RETURN(va_res, "Failed to create unowned VASurface",
-                       va_surface);
-
-  // This is safe to use Unretained() here, because the VDA takes care
-  // of the destruction order. All the surfaces will be destroyed
-  // before VaapiWrapper.
-  va_surface = new VASurface(
-      va_surface_id, size, va_format,
-      base::Bind(&VaapiWrapper::DestroyUnownedSurface, base::Unretained(this)));
-
-  return va_surface;
-}
-
-void VaapiWrapper::DestroyUnownedSurface(VASurfaceID va_surface_id) {
+void VaapiWrapper::DestroySurface(VASurfaceID va_surface_id) {
   base::AutoLock auto_lock(*va_lock_);
 
   VAStatus va_res = vaDestroySurfaces(va_display_, &va_surface_id, 1);

media/gpu/vaapi/vaapi_wrapper.h

@@ -218,16 +218,8 @@ class MEDIA_GPU_EXPORT VaapiWrapper
   // Free all memory allocated in CreateSurfaces.
   void DestroySurfaces_Locked();
 
-  // Create a VASurface of |va_format|, |size| and using |va_attribs|
-  // attributes. The ownership of the surface is transferred to the
-  // caller. It differs from surfaces created using CreateSurfaces(),
-  // where VaapiWrapper is the owner of the surfaces.
-  scoped_refptr<VASurface> CreateUnownedSurface(
-      unsigned int va_format,
-      const gfx::Size& size,
-      const std::vector<VASurfaceAttrib>& va_attribs);
-  // Destroys a |va_surface| created using CreateUnownedSurface.
-  void DestroyUnownedSurface(VASurfaceID va_surface_id);
+  // Destroys a |va_surface_id|.
+  void DestroySurface(VASurfaceID va_surface_id);
 
   // Initialize the video post processing context with the |size| of
   // the input pictures to be processed.