Add logic to determine which picture types need processor wrappers.

Bug: 1136730

R=andrescj

Change-Id: I60f2704313adfae0f76ce0d1c55e30051a48267b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2506770
Commit-Queue: Ted Meyer <tmathmeyer@chromium.org>
Reviewed-by: Andres Calderon Jaramillo <andrescj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825284}

media/gpu/vaapi/vaapi_picture_factory.cc

@@ -22,6 +22,25 @@
 
 namespace media {
 
+namespace {
+
+template <typename PictureType>
+std::unique_ptr<VaapiPicture> CreateVaapiPictureNativeImpl(
+    scoped_refptr<VaapiWrapper> vaapi_wrapper,
+    const MakeGLContextCurrentCallback& make_context_current_cb,
+    const BindGLImageCallback& bind_image_cb,
+    const PictureBuffer& picture_buffer,
+    const gfx::Size& visible_size,
+    uint32_t client_texture_id,
+    uint32_t service_texture_id) {
+  return std::make_unique<PictureType>(
+      std::move(vaapi_wrapper), make_context_current_cb, bind_image_cb,
+      picture_buffer.id(), picture_buffer.size(), visible_size,
+      service_texture_id, client_texture_id, picture_buffer.texture_target());
+}
+
+}  // namespace
+
 VaapiPictureFactory::VaapiPictureFactory() {
   vaapi_impl_pairs_.insert(
       std::make_pair(gl::kGLImplementationEGLGLES2,
@@ -36,6 +55,8 @@ VaapiPictureFactory::VaapiPictureFactory() {
                        VaapiPictureFactory::kVaapiImplementationX11));
   }
 #endif
+
+  DeterminePictureCreationAndDownloadingMechanism();
 }
 
 VaapiPictureFactory::~VaapiPictureFactory() = default;
@@ -61,13 +82,6 @@ std::unique_ptr<VaapiPicture> VaapiPictureFactory::Create(
           : 0;
 
   // Select DRM(egl) / TFP(glx) at runtime with --use-gl=egl / --use-gl=desktop
-
-#if defined(USE_OZONE)
-  if (features::IsUsingOzonePlatform())
-    return CreateVaapiPictureNativeForOzone(
-        vaapi_wrapper, make_context_current_cb, bind_image_cb, picture_buffer,
-        visible_size, client_texture_id, service_texture_id);
-#endif
   return CreateVaapiPictureNative(vaapi_wrapper, make_context_current_cb,
                                   bind_image_cb, picture_buffer, visible_size,
                                   client_texture_id, service_texture_id);
@@ -96,37 +110,62 @@ gfx::BufferFormat VaapiPictureFactory::GetBufferFormat() {
   return gfx::BufferFormat::RGBX_8888;
 }
 
-#if defined(USE_OZONE)
-std::unique_ptr<VaapiPicture>
-VaapiPictureFactory::CreateVaapiPictureNativeForOzone(
-    scoped_refptr<VaapiWrapper> vaapi_wrapper,
-    const MakeGLContextCurrentCallback& make_context_current_cb,
-    const BindGLImageCallback& bind_image_cb,
-    const PictureBuffer& picture_buffer,
-    const gfx::Size& visible_size,
-    uint32_t client_texture_id,
-    uint32_t service_texture_id) {
-  DCHECK(features::IsUsingOzonePlatform());
+void VaapiPictureFactory::DeterminePictureCreationAndDownloadingMechanism() {
   switch (GetVaapiImplementation(gl::GetGLImplementation())) {
+#if defined(USE_OZONE)
     // We can be called without GL initialized, which is valid if we use Ozone.
     case kVaapiImplementationNone:
-      FALLTHROUGH;
+      if (features::IsUsingOzonePlatform()) {
+        create_picture_cb_ = base::BindRepeating(
+            &CreateVaapiPictureNativeImpl<VaapiPictureNativePixmapOzone>);
+        needs_vpp_for_downloading_ = true;
+      }
+
+      // This is reached by unit tests which don't require create_picture_cb_
+      // to be initialized or called.
+      break;
+#endif  // defined(USE_OZONE)
+#if defined(USE_X11)
+    case kVaapiImplementationX11:
+      DCHECK(!features::IsUsingOzonePlatform());
+      create_picture_cb_ =
+          base::BindRepeating(&CreateVaapiPictureNativeImpl<VaapiTFPPicture>);
+      // Neither VaapiTFPPicture or VaapiPictureNativePixmapAngle needs the VPP.
+      needs_vpp_for_downloading_ = false;
+      break;
+    case kVaapiImplementationAngle:
+      DCHECK(!features::IsUsingOzonePlatform());
+      create_picture_cb_ = base::BindRepeating(
+          &CreateVaapiPictureNativeImpl<VaapiPictureNativePixmapAngle>);
+      // Neither VaapiTFPPicture or VaapiPictureNativePixmapAngle needs the VPP.
+      needs_vpp_for_downloading_ = false;
+      break;
+#endif  // defined(USE_X11)
     case kVaapiImplementationDrm:
-      return std::make_unique<VaapiPictureNativePixmapOzone>(
-          std::move(vaapi_wrapper), make_context_current_cb, bind_image_cb,
-          picture_buffer.id(), picture_buffer.size(), visible_size,
-          service_texture_id, client_texture_id,
-          picture_buffer.texture_target());
+#if defined(USE_OZONE)
+      if (features::IsUsingOzonePlatform()) {
+        create_picture_cb_ = base::BindRepeating(
+            &CreateVaapiPictureNativeImpl<VaapiPictureNativePixmapOzone>);
+        needs_vpp_for_downloading_ = true;
         break;
-
+      }
+#endif  // defined(USE_OZONE)
+#if defined(USE_EGL)
+      create_picture_cb_ = base::BindRepeating(
+          &CreateVaapiPictureNativeImpl<VaapiPictureNativePixmapEgl>);
+      needs_vpp_for_downloading_ = true;
+      break;
+#endif  // defined(USE_EGL)
+      // ozone or egl must be used to use the DRM implementation.
+      NOTREACHED();
     default:
       NOTREACHED();
-      return nullptr;
   }
+}
 
-  return nullptr;
+bool VaapiPictureFactory::NeedsProcessingPipelineForDownloading() const {
+  return needs_vpp_for_downloading_;
 }
-#endif  // USE_OZONE
 
 std::unique_ptr<VaapiPicture> VaapiPictureFactory::CreateVaapiPictureNative(
     scoped_refptr<VaapiWrapper> vaapi_wrapper,
@@ -136,40 +175,10 @@ std::unique_ptr<VaapiPicture> VaapiPictureFactory::CreateVaapiPictureNative(
     const gfx::Size& visible_size,
     uint32_t client_texture_id,
     uint32_t service_texture_id) {
-  switch (GetVaapiImplementation(gl::GetGLImplementation())) {
-#if defined(USE_EGL)
-    case kVaapiImplementationDrm:
-      return std::make_unique<VaapiPictureNativePixmapEgl>(
+  CHECK(create_picture_cb_);
+  return create_picture_cb_.Run(
       std::move(vaapi_wrapper), make_context_current_cb, bind_image_cb,
-          picture_buffer.id(), picture_buffer.size(), visible_size,
-          service_texture_id, client_texture_id,
-          picture_buffer.texture_target());
-#endif  // USE_EGL
-
-#if defined(USE_X11)
-    case kVaapiImplementationX11:
-      DCHECK(!features::IsUsingOzonePlatform());
-      return std::make_unique<VaapiTFPPicture>(
-          std::move(vaapi_wrapper), make_context_current_cb, bind_image_cb,
-          picture_buffer.id(), picture_buffer.size(), visible_size,
-          service_texture_id, client_texture_id,
-          picture_buffer.texture_target());
-      break;
-    case kVaapiImplementationAngle:
-      return std::make_unique<VaapiPictureNativePixmapAngle>(
-          std::move(vaapi_wrapper), make_context_current_cb, bind_image_cb,
-          picture_buffer.id(), picture_buffer.size(), visible_size,
-          service_texture_id, client_texture_id,
-          picture_buffer.texture_target());
-      break;
-#endif  // USE_X11
-
-    default:
-      NOTREACHED();
-      return nullptr;
-  }
-
-  return nullptr;
+      picture_buffer, visible_size, client_texture_id, service_texture_id);
 }
 
 }  // namespace media

media/gpu/vaapi/vaapi_picture_factory.h

@@ -18,6 +18,15 @@ namespace media {
 class PictureBuffer;
 class VaapiWrapper;
 
+using CreatePictureCB = base::RepeatingCallback<std::unique_ptr<VaapiPicture>(
+    scoped_refptr<VaapiWrapper>,
+    const MakeGLContextCurrentCallback&,
+    const BindGLImageCallback&,
+    const PictureBuffer&,
+    const gfx::Size&,
+    uint32_t,
+    uint32_t)>;
+
 // Factory of platform dependent VaapiPictures.
 class MEDIA_GPU_EXPORT VaapiPictureFactory {
  public:
@@ -44,6 +53,10 @@ class MEDIA_GPU_EXPORT VaapiPictureFactory {
   // implementation.
   VaapiImplementation GetVaapiImplementation(gl::GLImplementation gl_impl);
 
+  // Determines whether the DownloadFromSurface() method of the VaapiPictures
+  // created by this factory requires a processing pipeline VaapiWrapper.
+  bool NeedsProcessingPipelineForDownloading() const;
+
   // Gets the texture target used to bind EGLImages (either GL_TEXTURE_2D on X11
   // or GL_TEXTURE_EXTERNAL_OES on DRM).
   uint32_t GetGLTextureTarget();
@@ -52,17 +65,6 @@ class MEDIA_GPU_EXPORT VaapiPictureFactory {
   // the format decoded frames in VASurfaces are converted into.
   gfx::BufferFormat GetBufferFormat();
 
-#if defined(USE_OZONE)
-  std::unique_ptr<VaapiPicture> CreateVaapiPictureNativeForOzone(
-      scoped_refptr<VaapiWrapper> vaapi_wrapper,
-      const MakeGLContextCurrentCallback& make_context_current_cb,
-      const BindGLImageCallback& bind_image_cb,
-      const PictureBuffer& picture_buffer,
-      const gfx::Size& visible_size,
-      uint32_t client_texture_id,
-      uint32_t service_texture_id);
-#endif
-
   std::unique_ptr<VaapiPicture> CreateVaapiPictureNative(
       scoped_refptr<VaapiWrapper> vaapi_wrapper,
       const MakeGLContextCurrentCallback& make_context_current_cb,
@@ -76,6 +78,11 @@ class MEDIA_GPU_EXPORT VaapiPictureFactory {
       vaapi_impl_pairs_;
 
  private:
+  void DeterminePictureCreationAndDownloadingMechanism();
+
+  CreatePictureCB create_picture_cb_;
+  bool needs_vpp_for_downloading_ = false;
+
   DISALLOW_COPY_AND_ASSIGN(VaapiPictureFactory);
 };
 

media/gpu/vaapi/vaapi_video_decode_accelerator.cc

@@ -699,14 +699,13 @@ void VaapiVideoDecodeAccelerator::AssignPictureBuffers(
   std::vector<VASurfaceID> va_surface_ids;
   scoped_refptr<VaapiWrapper> vaapi_wrapper_for_picture = vaapi_wrapper_;
 
-  // The X11/ANGLE implementation can use |vaapi_wrapper_| to copy from an
-  // internal libva buffer into an X Pixmap without having to use a processing
-  // wrapper.
-#if !defined(USE_X11)
-  // If we aren't in BufferAllocationMode::kNone, we have to allocate a
-  // |vpp_vaapi_wrapper_| for VaapiPicture to DownloadFromSurface() the VA's
-  // internal decoded frame.
-  if (buffer_allocation_mode_ != BufferAllocationMode::kNone) {
+  const bool requires_vpp =
+      vaapi_picture_factory_->NeedsProcessingPipelineForDownloading();
+  // If we aren't in BufferAllocationMode::kNone mode and the VaapiPicture
+  // implementation we get from |vaapi_picture_factory_| requires the video
+  // processing pipeline for downloading the decoded frame from the internal
+  // surface, we need to create a |vpp_vaapi_wrapper_|.
+  if (requires_vpp && buffer_allocation_mode_ != BufferAllocationMode::kNone) {
     if (!vpp_vaapi_wrapper_) {
       vpp_vaapi_wrapper_ = VaapiWrapper::Create(
           VaapiWrapper::kVideoProcess, VAProfileNone,
@@ -724,8 +723,6 @@ void VaapiVideoDecodeAccelerator::AssignPictureBuffers(
     vaapi_wrapper_for_picture = vpp_vaapi_wrapper_;
   }
 
-#endif  // !defined(USE_X11)
-
   for (size_t i = 0; i < buffers.size(); ++i) {
     // TODO(b/139460315): Create with buffers[i] once the AMD driver issue is
     // resolved.