media/gpu: extract image processor interface

Extract the useful interface from the V4L2ImageProcessor into a generic
interface that can be used for other kinds of processors and codecs.
Update the V4L2 encoder and decoder to use this more generic interface.

Also convert one instance of Callback into OnceCallback in the image
processor.

BUG=850375
BUG=b:73752373
TEST=Checked both VDA and VEA unittests on Hana.

Change-Id: Ia40a3989717f3cb03711b13a2ec574e018d2c614
Reviewed-on: https://chromium-review.googlesource.com/1084364Reviewed-by: Kuang-che Wu <kcwu@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#569165}

media/gpu/BUILD.gn

@@ -105,6 +105,7 @@ component("gpu") {
     "gpu_video_decode_accelerator_factory.h",
     "gpu_video_encode_accelerator_factory.cc",
     "gpu_video_encode_accelerator_factory.h",
+    "image_processor.h",
   ]
 
   public_deps = [

media/gpu/image_processor.h

+// Copyright 2018 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.
+
+#ifndef MEDIA_GPU_IMAGE_PROCESSOR_H_
+#define MEDIA_GPU_IMAGE_PROCESSOR_H_
+
+#include <vector>
+
+#include "base/callback.h"
+#include "base/files/scoped_file.h"
+#include "media/base/video_frame.h"
+#include "media/base/video_types.h"
+#include "ui/gfx/geometry/size.h"
+
+namespace media {
+
+// An image processor is used to convert from one image format to another (e.g.
+// I420 to NV12) while optionally scaling. It is useful in situations where
+// a given video hardware (e.g. decoder or encoder) accepts or produces data
+// in a format different from what the rest of the pipeline expects.
+//
+// This class exposes the interface that an image processor should implement.
+class ImageProcessor {
+ public:
+  // Initializes the processor to convert from |input_format| to |output_format|
+  // and/or scale from |input_visible_size| to |output_visible_size|.
+  // Request the input buffers to be of at least |input_allocated_size| and the
+  // output buffers to be of at least |output_allocated_size|. The number of
+  // input buffers and output buffers will be |num_buffers|. Provided |error_cb|
+  // will be posted to the child thread if an error occurs after initialization.
+  // Return true if the requested configuration is supported.
+  virtual bool Initialize(VideoPixelFormat input_format,
+                          VideoPixelFormat output_format,
+                          gfx::Size input_visible_size,
+                          gfx::Size input_allocated_size,
+                          gfx::Size output_visible_size,
+                          gfx::Size output_allocated_size,
+                          int num_buffers,
+                          const base::Closure& error_cb) = 0;
+
+  // Returns input allocated size required by the processor to be fed with.
+  virtual gfx::Size input_allocated_size() const = 0;
+
+  // Returns output allocated size required by the processor.
+  virtual gfx::Size output_allocated_size() const = 0;
+
+  // Callback to be used to return the index of a processed image to the
+  // client. After the client is done with the frame, call Process with the
+  // index to return the output buffer to the image processor.
+  using FrameReadyCB = base::OnceCallback<void(scoped_refptr<VideoFrame>)>;
+
+  // Called by client to process |frame|. The resulting processed frame will be
+  // stored in |output_buffer_index| output buffer and notified via |cb|. The
+  // processor will drop all its references to |frame| after it finishes
+  // accessing it. If the input buffers are DMA-backed, the caller
+  // should pass non-empty |output_dmabuf_fds| and the processed frame will be
+  // stored in those buffers. If the number of |output_dmabuf_fds| is not
+  // expected, this function will return false.
+  virtual bool Process(const scoped_refptr<VideoFrame>& frame,
+                       int output_buffer_index,
+                       std::vector<base::ScopedFD> output_dmabuf_fds,
+                       FrameReadyCB cb) = 0;
+
+  // Reset all processing frames. After this method returns, no more callbacks
+  // will be invoked. ImageProcessor is ready to process more frames.
+  virtual bool Reset() = 0;
+
+  virtual ~ImageProcessor() = default;
+};
+
+}  // namespace media
+
+#endif  // MEDIA_GPU_IMAGE_PROCESSOR_H_

media/gpu/v4l2/v4l2_image_processor.cc

@@ -63,11 +63,13 @@ V4L2ImageProcessor::JobRecord::JobRecord() : output_buffer_index(-1) {}
 
 V4L2ImageProcessor::JobRecord::~JobRecord() {}
 
-V4L2ImageProcessor::V4L2ImageProcessor(const scoped_refptr<V4L2Device>& device)
+V4L2ImageProcessor::V4L2ImageProcessor(const scoped_refptr<V4L2Device>& device,
+                                       v4l2_memory input_memory_type,
+                                       v4l2_memory output_memory_type)
     : input_format_(PIXEL_FORMAT_UNKNOWN),
       output_format_(PIXEL_FORMAT_UNKNOWN),
-      input_memory_type_(V4L2_MEMORY_USERPTR),
-      output_memory_type_(V4L2_MEMORY_MMAP),
+      input_memory_type_(input_memory_type),
+      output_memory_type_(output_memory_type),
       input_format_fourcc_(0),
       output_format_fourcc_(0),
       input_planes_count_(0),
@@ -82,11 +84,18 @@ V4L2ImageProcessor::V4L2ImageProcessor(const scoped_refptr<V4L2Device>& device)
       output_buffer_queued_count_(0),
       num_buffers_(0),
       weak_this_factory_(this) {
+  DCHECK(input_memory_type == V4L2_MEMORY_USERPTR ||
+         input_memory_type == V4L2_MEMORY_DMABUF);
+  DCHECK(output_memory_type == V4L2_MEMORY_MMAP ||
+         output_memory_type == V4L2_MEMORY_DMABUF);
   weak_this_ = weak_this_factory_.GetWeakPtr();
 }
 
 V4L2ImageProcessor::~V4L2ImageProcessor() {
   DCHECK(child_task_runner_->BelongsToCurrentThread());
+
+  Destroy();
+
   DCHECK(!device_thread_.IsRunning());
   DCHECK(!device_poll_thread_.IsRunning());
 
@@ -110,8 +119,6 @@ void V4L2ImageProcessor::NotifyErrorOnChildThread(
 
 bool V4L2ImageProcessor::Initialize(VideoPixelFormat input_format,
                                     VideoPixelFormat output_format,
-                                    v4l2_memory input_memory_type,
-                                    v4l2_memory output_memory_type,
                                     gfx::Size input_visible_size,
                                     gfx::Size input_allocated_size,
                                     gfx::Size output_visible_size,
@@ -121,10 +128,6 @@ bool V4L2ImageProcessor::Initialize(VideoPixelFormat input_format,
   VLOGF(2);
   DCHECK(!error_cb.is_null());
   DCHECK_GT(num_buffers, 0);
-  DCHECK(input_memory_type == V4L2_MEMORY_USERPTR ||
-         input_memory_type == V4L2_MEMORY_DMABUF);
-  DCHECK(output_memory_type == V4L2_MEMORY_MMAP ||
-         output_memory_type == V4L2_MEMORY_DMABUF);
   error_cb_ = error_cb;
 
   input_format_ = input_format;
@@ -139,8 +142,6 @@ bool V4L2ImageProcessor::Initialize(VideoPixelFormat input_format,
     return false;
   }
 
-  input_memory_type_ = input_memory_type;
-  output_memory_type_ = output_memory_type;
   input_visible_size_ = input_visible_size;
   input_allocated_size_ = input_allocated_size;
   output_visible_size_ = output_visible_size;
@@ -291,8 +292,8 @@ bool V4L2ImageProcessor::Process(const scoped_refptr<VideoFrame>& frame,
     return false;
 
   device_thread_.task_runner()->PostTask(
-      FROM_HERE, base::Bind(&V4L2ImageProcessor::ProcessTask,
-                            base::Unretained(this), base::Passed(&job_record)));
+      FROM_HERE, base::BindOnce(&V4L2ImageProcessor::ProcessTask,
+                                base::Unretained(this), std::move(job_record)));
   return true;
 }
 
@@ -345,8 +346,6 @@ void V4L2ImageProcessor::Destroy() {
     // Otherwise DestroyTask() is not needed.
     DCHECK(!device_poll_thread_.IsRunning());
   }
-
-  delete this;
 }
 
 bool V4L2ImageProcessor::CreateInputBuffers() {

media/gpu/v4l2/v4l2_image_processor.h

@@ -17,6 +17,7 @@
 #include "base/memory/weak_ptr.h"
 #include "base/threading/thread.h"
 #include "media/base/video_frame.h"
+#include "media/gpu/image_processor.h"
 #include "media/gpu/media_gpu_export.h"
 #include "media/gpu/v4l2/v4l2_device.h"
 
@@ -25,9 +26,11 @@ namespace media {
 // Handles image processing accelerators that expose a V4L2 memory-to-memory
 // interface. The threading model of this class is the same as for other V4L2
 // hardware accelerators (see V4L2VideoDecodeAccelerator) for more details.
-class MEDIA_GPU_EXPORT V4L2ImageProcessor {
+class MEDIA_GPU_EXPORT V4L2ImageProcessor : public ImageProcessor {
  public:
-  explicit V4L2ImageProcessor(const scoped_refptr<V4L2Device>& device);
+  explicit V4L2ImageProcessor(const scoped_refptr<V4L2Device>& device,
+                              v4l2_memory input_memory_type,
+                              v4l2_memory output_memory_type);
   virtual ~V4L2ImageProcessor();
 
   // Initializes the processor to convert from |input_format| to |output_format|
@@ -39,14 +42,12 @@ class MEDIA_GPU_EXPORT V4L2ImageProcessor {
   // configuration is supported.
   bool Initialize(VideoPixelFormat input_format,
                   VideoPixelFormat output_format,
-                  v4l2_memory input_memory_type,
-                  v4l2_memory output_memory_type,
                   gfx::Size input_visible_size,
                   gfx::Size input_allocated_size,
                   gfx::Size output_visible_size,
                   gfx::Size output_allocated_size,
                   int num_buffers,
-                  const base::Closure& error_cb);
+                  const base::Closure& error_cb) override;
 
   // Returns true if image processing is supported on this platform.
   static bool IsSupported();
@@ -66,14 +67,12 @@ class MEDIA_GPU_EXPORT V4L2ImageProcessor {
                               gfx::Size* size,
                               size_t* num_planes);
 
-  // Returns input allocated size required by the processor to be fed with.
-  gfx::Size input_allocated_size() const { return input_allocated_size_; }
-
-  // Returns output allocated size required by the processor.
-  gfx::Size output_allocated_size() const { return output_allocated_size_; }
-
-  // Callback to be used to return the processed image to the client.
-  typedef base::OnceCallback<void(scoped_refptr<VideoFrame>)> FrameReadyCB;
+  gfx::Size input_allocated_size() const override {
+    return input_allocated_size_;
+  }
+  gfx::Size output_allocated_size() const override {
+    return output_allocated_size_;
+  }
 
   // Called by client to process |frame|. The resulting processed frame will be
   // stored in |output_buffer_index| output buffer and notified via |cb|. The
@@ -85,16 +84,11 @@ class MEDIA_GPU_EXPORT V4L2ImageProcessor {
   bool Process(const scoped_refptr<VideoFrame>& frame,
                int output_buffer_index,
                std::vector<base::ScopedFD> output_dmabuf_fds,
-               FrameReadyCB cb);
+               FrameReadyCB cb) override;
 
   // Reset all processing frames. After this method returns, no more callbacks
   // will be invoked. V4L2ImageProcessor is ready to process more frames.
-  bool Reset();
-
-  // Stop all processing and clean up. After this method returns no more
-  // callbacks will be invoked.  Deletes |this| unconditionally, so make sure
-  // to drop all pointers to it!
-  void Destroy();
+  bool Reset() override;
 
  private:
   // Record for input buffers.
@@ -159,6 +153,10 @@ class MEDIA_GPU_EXPORT V4L2ImageProcessor {
   // A processed frame is ready.
   void FrameReady(FrameReadyCB cb, scoped_refptr<VideoFrame> frame);
 
+  // Stop all processing and clean up. After this method returns no more
+  // callbacks will be invoked.
+  void Destroy();
+
   // Size and format-related members remain constant after initialization.
   // The visible/allocated sizes of the input frame.
   gfx::Size input_visible_size_;

media/gpu/v4l2/v4l2_video_decode_accelerator.cc

@@ -25,6 +25,7 @@
 #include "media/base/media_switches.h"
 #include "media/base/scopedfd_helper.h"
 #include "media/base/unaligned_shared_memory.h"
+#include "media/gpu/v4l2/v4l2_image_processor.h"
 #include "media/video/h264_parser.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gl/gl_context.h"
@@ -1854,8 +1855,7 @@ void V4L2VideoDecodeAccelerator::DestroyTask() {
     decoder_input_queue_.pop();
   decoder_flushing_ = false;
 
-  if (image_processor_)
-    image_processor_.release()->Destroy();
+  image_processor_ = nullptr;
 
   // Set our state to kError.  Just in case.
   decoder_state_ = kError;
@@ -1976,8 +1976,7 @@ void V4L2VideoDecodeAccelerator::StartResolutionChange() {
     return;
   }
 
-  if (image_processor_)
-    image_processor_.release()->Destroy();
+  image_processor_ = nullptr;
 
   if (!DestroyOutputBuffers()) {
     VLOGF(1) << "Failed destroying output buffers.";
@@ -2394,17 +2393,18 @@ bool V4L2VideoDecodeAccelerator::ResetImageProcessor() {
 bool V4L2VideoDecodeAccelerator::CreateImageProcessor() {
   VLOGF(2);
   DCHECK(!image_processor_);
-  image_processor_.reset(new V4L2ImageProcessor(image_processor_device_));
   v4l2_memory output_memory_type =
       (output_mode_ == Config::OutputMode::ALLOCATE ? V4L2_MEMORY_MMAP
                                                     : V4L2_MEMORY_DMABUF);
+  image_processor_.reset(new V4L2ImageProcessor(
+      image_processor_device_, V4L2_MEMORY_DMABUF, output_memory_type));
   // Unretained is safe because |this| owns image processor and there will be
   // no callbacks after processor destroys.
   if (!image_processor_->Initialize(
           V4L2Device::V4L2PixFmtToVideoPixelFormat(output_format_fourcc_),
           V4L2Device::V4L2PixFmtToVideoPixelFormat(egl_image_format_fourcc_),
-          V4L2_MEMORY_DMABUF, output_memory_type, visible_size_, coded_size_,
-          visible_size_, egl_image_size_, output_buffer_map_.size(),
+          visible_size_, coded_size_, visible_size_, egl_image_size_,
+          output_buffer_map_.size(),
           base::Bind(&V4L2VideoDecodeAccelerator::ImageProcessorError,
                      base::Unretained(this)))) {
     VLOGF(1) << "Initialize image processor failed";

media/gpu/v4l2/v4l2_video_decode_accelerator.h

@@ -25,9 +25,9 @@
 #include "media/base/limits.h"
 #include "media/base/video_decoder_config.h"
 #include "media/gpu/gpu_video_decode_accelerator_helpers.h"
+#include "media/gpu/image_processor.h"
 #include "media/gpu/media_gpu_export.h"
 #include "media/gpu/v4l2/v4l2_device.h"
-#include "media/gpu/v4l2/v4l2_image_processor.h"
 #include "media/video/picture.h"
 #include "media/video/video_decode_accelerator.h"
 #include "ui/gfx/geometry/size.h"
@@ -566,7 +566,7 @@ class MEDIA_GPU_EXPORT V4L2VideoDecodeAccelerator
   // Image processor device, if one is in use.
   scoped_refptr<V4L2Device> image_processor_device_;
   // Image processor. Accessed on |decoder_thread_|.
-  std::unique_ptr<V4L2ImageProcessor> image_processor_;
+  std::unique_ptr<ImageProcessor> image_processor_;
 
   // The V4L2Device EGLImage is created from.
   scoped_refptr<V4L2Device> egl_image_device_;

media/gpu/v4l2/v4l2_video_encode_accelerator.cc

@@ -25,6 +25,7 @@
 #include "media/base/bitstream_buffer.h"
 #include "media/base/scopedfd_helper.h"
 #include "media/base/unaligned_shared_memory.h"
+#include "media/gpu/v4l2/v4l2_image_processor.h"
 #include "media/video/h264_parser.h"
 
 #define VLOGF(level) VLOG(level) << __func__ << "(): "
@@ -204,16 +205,16 @@ bool V4L2VideoEncodeAccelerator::Initialize(VideoPixelFormat input_format,
     }
 
     scoped_refptr<V4L2Device> device = V4L2Device::Create();
-    image_processor_.reset(new V4L2ImageProcessor(device));
+    image_processor_.reset(
+        new V4L2ImageProcessor(device, V4L2_MEMORY_USERPTR, V4L2_MEMORY_MMAP));
 
     // Convert from input_format to device_input_format_, keeping the size
     // at visible_size_ and requiring the output buffers to be of at least
     // input_allocated_size_. Unretained is safe because |this| owns image
     // processor and there will be no callbacks after processor destroys.
     if (!image_processor_->Initialize(
-            input_format, device_input_format_, V4L2_MEMORY_USERPTR,
-            V4L2_MEMORY_MMAP, visible_size_, visible_size_, visible_size_,
-            input_allocated_size_, kImageProcBufferCount,
+            input_format, device_input_format_, visible_size_, visible_size_,
+            visible_size_, input_allocated_size_, kImageProcBufferCount,
             base::Bind(&V4L2VideoEncodeAccelerator::ImageProcessorError,
                        base::Unretained(this)))) {
       VLOGF(1) << "Failed initializing image processor";
@@ -343,8 +344,7 @@ void V4L2VideoEncodeAccelerator::Destroy() {
   client_ptr_factory_.reset();
   weak_this_ptr_factory_.InvalidateWeakPtrs();
 
-  if (image_processor_.get())
-    image_processor_.release()->Destroy();
+  image_processor_ = nullptr;
 
   // If the encoder thread is running, destroy using posted task.
   if (encoder_thread_.IsRunning()) {

media/gpu/v4l2/v4l2_video_encode_accelerator.h

@@ -18,9 +18,9 @@
 #include "base/memory/weak_ptr.h"
 #include "base/threading/thread.h"
 #include "base/time/time.h"
+#include "media/gpu/image_processor.h"
 #include "media/gpu/media_gpu_export.h"
 #include "media/gpu/v4l2/v4l2_device.h"
-#include "media/gpu/v4l2/v4l2_image_processor.h"
 #include "media/video/video_encode_accelerator.h"
 #include "ui/gfx/geometry/size.h"
 
@@ -36,7 +36,7 @@ namespace media {
 // device exposed by the codec hardware driver. The threading model of this
 // class is the same as in the V4L2VideoDecodeAccelerator (from which class this
 // was designed).
-// This class may try to instantiate and use a V4L2ImageProcessor for input
+// This class may try to instantiate and use a ImageProcessor for input
 // format conversion, if the input format requested via Initialize() is not
 // accepted by the hardware codec.
 class MEDIA_GPU_EXPORT V4L2VideoEncodeAccelerator
@@ -192,7 +192,7 @@ class MEDIA_GPU_EXPORT V4L2VideoEncodeAccelerator
 
   // Try to set up the device to the input format we were Initialized() with,
   // or if the device doesn't support it, use one it can support, so that we
-  // can later instantiate a V4L2ImageProcessor to convert to it.
+  // can later instantiate an ImageProcessor to convert to it.
   bool NegotiateInputFormat(VideoPixelFormat input_format);
 
   // Set up the device to the output format requested in Initialize().
@@ -296,7 +296,7 @@ class MEDIA_GPU_EXPORT V4L2VideoEncodeAccelerator
   FlushCallback flush_callback_;
 
   // Image processor, if one is in use.
-  std::unique_ptr<V4L2ImageProcessor> image_processor_;
+  std::unique_ptr<ImageProcessor> image_processor_;
   // Indexes of free image processor output buffers. Only accessed on child
   // thread.
   std::vector<int> free_image_processor_output_buffers_;