media/gpu/chromeos: Refactor ImageProcessor.

In general, image format conversion is time-consuming task.
ImageProcessor, the interface for image conversion, should offload the
conversion tasks to a different thread to avoid occupying the working
thread for long time. Originally, thread hopping is handled by each
implementation. It makes implementations complicated and error-prone.

In this CL, we refactor the structure of ImageProcessor. We add a new
interface "ImageProcessorBackend". Each implementation change to
inherit this interface. The interface only works on backend sequence.
ImageProcessor becomes a concrete class instead of a interface. It
has a ImageProcessorBackend instance, and is in charge of handling
sequence hopping between client sequence and backend sequence.
ImageProcessor also guarantees to call or destroy the client's
callback on the client sequence.

Bug: 1028889
Test: run vda_tests on kukui and hana
Test: build media/gpu/{chromeos,vaapi,v4l2}

Change-Id: I0f7774bc62adc7d4b4adf68ab33d95d9013656be
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1942600
Commit-Queue: Chih-Yu Huang <akahuang@chromium.org>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#725476}

media/gpu/chromeos/BUILD.gn

@@ -51,6 +51,8 @@ source_set("common") {
     "gpu_buffer_layout.h",
     "image_processor.cc",
     "image_processor.h",
+    "image_processor_backend.cc",
+    "image_processor_backend.h",
     "image_processor_factory.h",
     "image_processor_with_pool.cc",
     "image_processor_with_pool.h",

media/gpu/chromeos/image_processor.cc

@@ -4,11 +4,14 @@
 
 #include "media/gpu/chromeos/image_processor.h"
 
+#include <memory>
 #include <ostream>
 #include <sstream>
 
-#include "base/strings/stringprintf.h"
-#include "media/base/video_frame.h"
+#include "base/bind.h"
+#include "base/bind_helpers.h"
+#include "base/task/post_task.h"
+#include "base/task/task_traits.h"
 #include "media/base/video_types.h"
 #include "media/gpu/macros.h"
 
@@ -22,20 +25,6 @@ std::ostream& operator<<(std::ostream& ostream,
   return ostream;
 }
 
-template <class T>
-std::string VectorToString(const std::vector<T>& vec) {
-  std::ostringstream result;
-  std::string delim;
-  result << "[";
-  for (const T& v : vec) {
-    result << delim << v;
-    if (delim.size() == 0)
-      delim = ", ";
-  }
-  result << "]";
-  return result.str();
-}
-
 // Verify if the format of |frame| matches |config|.
 bool CheckVideoFrameFormat(const ImageProcessor::PortConfig& config,
                            const VideoFrame& frame) {
@@ -65,75 +54,192 @@ bool CheckVideoFrameFormat(const ImageProcessor::PortConfig& config,
 
 }  // namespace
 
-ImageProcessor::PortConfig::PortConfig(const PortConfig&) = default;
-
-ImageProcessor::PortConfig::PortConfig(
-    Fourcc fourcc,
-    const gfx::Size& size,
-    const std::vector<ColorPlaneLayout>& planes,
-    const gfx::Size& visible_size,
-    const std::vector<VideoFrame::StorageType>& preferred_storage_types)
-    : fourcc(fourcc),
-      size(size),
-      planes(planes),
-      visible_size(visible_size),
-      preferred_storage_types(preferred_storage_types) {}
-
-ImageProcessor::PortConfig::~PortConfig() = default;
-
-std::string ImageProcessor::PortConfig::ToString() const {
-  return base::StringPrintf(
-      "PortConfig(format:%s, size:%s, planes: %s, visible_size:%s, "
-      "storage_types:%s)",
-      fourcc.ToString().c_str(), size.ToString().c_str(),
-      VectorToString(planes).c_str(), visible_size.ToString().c_str(),
-      VectorToString(preferred_storage_types).c_str());
+ImageProcessor::ClientCallback::ClientCallback(FrameReadyCB ready_cb)
+    : ready_cb(std::move(ready_cb)) {}
+ImageProcessor::ClientCallback::ClientCallback(
+    LegacyFrameReadyCB legacy_ready_cb)
+    : legacy_ready_cb(std::move(legacy_ready_cb)) {}
+ImageProcessor::ClientCallback::ClientCallback(ClientCallback&&) = default;
+ImageProcessor::ClientCallback::~ClientCallback() = default;
+
+// static
+std::unique_ptr<ImageProcessor> ImageProcessor::Create(
+    CreateBackendCB create_backend_cb,
+    const PortConfig& input_config,
+    const PortConfig& output_config,
+    const std::vector<OutputMode>& preferred_output_modes,
+    ErrorCB error_cb,
+    scoped_refptr<base::SequencedTaskRunner> client_task_runner) {
+  scoped_refptr<base::SequencedTaskRunner> backend_task_runner =
+      base::CreateSequencedTaskRunner({base::ThreadPool()});
+  auto wrapped_error_cb = base::BindRepeating(
+      base::IgnoreResult(&base::SequencedTaskRunner::PostTask),
+      client_task_runner, FROM_HERE, std::move(error_cb));
+  std::unique_ptr<ImageProcessorBackend> backend =
+      create_backend_cb.Run(input_config, output_config, preferred_output_modes,
+                            std::move(wrapped_error_cb), backend_task_runner);
+  if (!backend)
+    return nullptr;
+
+  return base::WrapUnique(new ImageProcessor(std::move(backend),
+                                             std::move(client_task_runner),
+                                             std::move(backend_task_runner)));
 }
 
 ImageProcessor::ImageProcessor(
-    const ImageProcessor::PortConfig& input_config,
-    const ImageProcessor::PortConfig& output_config,
-    OutputMode output_mode,
-    scoped_refptr<base::SequencedTaskRunner> client_task_runner)
-    : input_config_(input_config),
-      output_config_(output_config),
-      output_mode_(output_mode),
-      client_task_runner_(std::move(client_task_runner)) {
+    std::unique_ptr<ImageProcessorBackend> backend,
+    scoped_refptr<base::SequencedTaskRunner> client_task_runner,
+    scoped_refptr<base::SequencedTaskRunner> backend_task_runner)
+    : backend_(std::move(backend)),
+      client_task_runner_(std::move(client_task_runner)),
+      backend_task_runner_(std::move(backend_task_runner)) {
+  DVLOGF(2);
   DETACH_FROM_SEQUENCE(client_sequence_checker_);
-}
 
-ImageProcessor::~ImageProcessor() = default;
+  weak_this_ = weak_this_factory_.GetWeakPtr();
+}
 
-#if defined(OS_POSIX) || defined(OS_FUCHSIA)
-bool ImageProcessor::Process(scoped_refptr<VideoFrame> frame,
-                             LegacyFrameReadyCB cb) {
+ImageProcessor::~ImageProcessor() {
+  DVLOGF(3);
   DCHECK_CALLED_ON_VALID_SEQUENCE(client_sequence_checker_);
-  DCHECK_EQ(output_mode(), OutputMode::ALLOCATE);
 
-  return ProcessInternal(std::move(frame), std::move(cb));
-}
+  weak_this_factory_.InvalidateWeakPtrs();
 
-bool ImageProcessor::ProcessInternal(scoped_refptr<VideoFrame> frame,
-                                     LegacyFrameReadyCB cb) {
-  NOTIMPLEMENTED();
-  return false;
+  // Delete |backend_| on |backend_task_runner_|.
+  backend_task_runner_->PostTask(
+      FROM_HERE,
+      base::BindOnce(
+          base::DoNothing::Once<std::unique_ptr<ImageProcessorBackend>>(),
+          std::move(backend_)));
 }
-#endif
 
 bool ImageProcessor::Process(scoped_refptr<VideoFrame> input_frame,
                              scoped_refptr<VideoFrame> output_frame,
                              FrameReadyCB cb) {
+  DVLOGF(4);
   DCHECK_CALLED_ON_VALID_SEQUENCE(client_sequence_checker_);
   DCHECK_EQ(output_mode(), OutputMode::IMPORT);
   DCHECK(input_frame);
   DCHECK(output_frame);
 
-  if (!CheckVideoFrameFormat(input_config_, *input_frame) ||
-      !CheckVideoFrameFormat(output_config_, *output_frame))
+  if (!CheckVideoFrameFormat(input_config(), *input_frame) ||
+      !CheckVideoFrameFormat(output_config(), *output_frame))
     return false;
 
-  return ProcessInternal(std::move(input_frame), std::move(output_frame),
-                         std::move(cb));
+  int cb_index = StoreCallback(std::move(cb));
+  auto ready_cb = base::BindOnce(&ImageProcessor::OnProcessDoneThunk,
+                                 client_task_runner_, weak_this_, cb_index);
+  backend_task_runner_->PostTask(
+      FROM_HERE,
+      base::BindOnce(&ImageProcessorBackend::Process,
+                     base::Unretained(backend_.get()), std::move(input_frame),
+                     std::move(output_frame), std::move(ready_cb)));
+  return true;
+}
+
+// static
+void ImageProcessor::OnProcessDoneThunk(
+    scoped_refptr<base::SequencedTaskRunner> task_runner,
+    base::Optional<base::WeakPtr<ImageProcessor>> weak_this,
+    int cb_index,
+    scoped_refptr<VideoFrame> frame) {
+  DVLOGF(4);
+  DCHECK(weak_this);
+
+  task_runner->PostTask(
+      FROM_HERE, base::BindOnce(&ImageProcessor::OnProcessDone, *weak_this,
+                                cb_index, std::move(frame)));
+}
+
+void ImageProcessor::OnProcessDone(int cb_index,
+                                   scoped_refptr<VideoFrame> frame) {
+  DVLOGF(4);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(client_sequence_checker_);
+
+  auto it = pending_cbs_.find(cb_index);
+  // Skip if the callback is dropped by Reset().
+  if (it == pending_cbs_.end())
+    return;
+
+  DCHECK(it->second.ready_cb);
+  FrameReadyCB cb = std::move(it->second.ready_cb);
+  pending_cbs_.erase(it);
+
+  std::move(cb).Run(std::move(frame));
+}
+
+bool ImageProcessor::Process(scoped_refptr<VideoFrame> frame,
+                             LegacyFrameReadyCB cb) {
+  DVLOGF(4);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(client_sequence_checker_);
+  DCHECK_EQ(output_mode(), OutputMode::ALLOCATE);
+
+  int cb_index = StoreCallback(std::move(cb));
+  auto ready_cb = base::BindOnce(&ImageProcessor::OnProcessLegacyDoneThunk,
+                                 client_task_runner_, weak_this_, cb_index);
+  backend_task_runner_->PostTask(
+      FROM_HERE, base::BindOnce(&ImageProcessorBackend::ProcessLegacy,
+                                base::Unretained(backend_.get()),
+                                std::move(frame), std::move(ready_cb)));
+  return true;
+}
+
+// static
+void ImageProcessor::OnProcessLegacyDoneThunk(
+    scoped_refptr<base::SequencedTaskRunner> task_runner,
+    base::Optional<base::WeakPtr<ImageProcessor>> weak_this,
+    int cb_index,
+    size_t buffer_id,
+    scoped_refptr<VideoFrame> frame) {
+  DVLOGF(4);
+  DCHECK(weak_this);
+
+  task_runner->PostTask(
+      FROM_HERE,
+      base::BindOnce(&ImageProcessor::OnProcessLegacyDone, *weak_this, cb_index,
+                     buffer_id, std::move(frame)));
+}
+
+void ImageProcessor::OnProcessLegacyDone(int cb_index,
+                                         size_t buffer_id,
+                                         scoped_refptr<VideoFrame> frame) {
+  DVLOGF(4);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(client_sequence_checker_);
+
+  auto it = pending_cbs_.find(cb_index);
+  // Skip if the callback is dropped by Reset().
+  if (it == pending_cbs_.end())
+    return;
+
+  DCHECK(it->second.legacy_ready_cb);
+  LegacyFrameReadyCB cb = std::move(it->second.legacy_ready_cb);
+  pending_cbs_.erase(it);
+
+  std::move(cb).Run(buffer_id, std::move(frame));
+}
+
+bool ImageProcessor::Reset() {
+  DVLOGF(3);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(client_sequence_checker_);
+
+  backend_task_runner_->PostTask(
+      FROM_HERE, base::BindOnce(&ImageProcessorBackend::Reset,
+                                base::Unretained(backend_.get())));
+
+  // After clearing all pending callbacks, we can guarantee no frame are
+  // returned after that.
+  pending_cbs_.clear();
+
+  return true;
+}
+
+int ImageProcessor::StoreCallback(ClientCallback cb) {
+  DVLOGF(4);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(client_sequence_checker_);
+
+  int cb_index = next_cb_index_++;
+  pending_cbs_.emplace(cb_index, std::move(cb));
+  return cb_index;
 }
 
 }  // namespace media

media/gpu/chromeos/image_processor_backend.cc

+// Copyright 2019 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.
+
+#include "media/gpu/chromeos/image_processor_backend.h"
+
+#include <memory>
+#include <ostream>
+#include <sstream>
+
+#include "base/bind.h"
+#include "base/strings/stringprintf.h"
+#include "base/task/post_task.h"
+#include "base/task/task_traits.h"
+#include "media/gpu/macros.h"
+
+namespace media {
+
+namespace {
+
+template <class T>
+std::string VectorToString(const std::vector<T>& vec) {
+  std::ostringstream result;
+  std::string delim;
+  result << "[";
+  for (const T& v : vec) {
+    result << delim << v;
+    if (delim.size() == 0)
+      delim = ", ";
+  }
+  result << "]";
+  return result.str();
+}
+
+}  // namespace
+
+ImageProcessorBackend::PortConfig::PortConfig(const PortConfig&) = default;
+
+ImageProcessorBackend::PortConfig::PortConfig(
+    Fourcc fourcc,
+    const gfx::Size& size,
+    const std::vector<ColorPlaneLayout>& planes,
+    const gfx::Size& visible_size,
+    const std::vector<VideoFrame::StorageType>& preferred_storage_types)
+    : fourcc(fourcc),
+      size(size),
+      planes(planes),
+      visible_size(visible_size),
+      preferred_storage_types(preferred_storage_types) {}
+
+ImageProcessorBackend::PortConfig::~PortConfig() = default;
+
+std::string ImageProcessorBackend::PortConfig::ToString() const {
+  return base::StringPrintf(
+      "PortConfig(format:%s, size:%s, planes: %s, visible_size:%s, "
+      "storage_types:%s)",
+      fourcc.ToString().c_str(), size.ToString().c_str(),
+      VectorToString(planes).c_str(), visible_size.ToString().c_str(),
+      VectorToString(preferred_storage_types).c_str());
+}
+
+ImageProcessorBackend::ImageProcessorBackend(
+    const PortConfig& input_config,
+    const PortConfig& output_config,
+    OutputMode output_mode,
+    ErrorCB error_cb,
+    scoped_refptr<base::SequencedTaskRunner> backend_task_runner)
+    : input_config_(input_config),
+      output_config_(output_config),
+      output_mode_(output_mode),
+      error_cb_(error_cb),
+      backend_task_runner_(std::move(backend_task_runner)) {
+  DETACH_FROM_SEQUENCE(backend_sequence_checker_);
+}
+
+ImageProcessorBackend::~ImageProcessorBackend() = default;
+
+void ImageProcessorBackend::Destroy() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(backend_sequence_checker_);
+
+  delete this;
+}
+
+void ImageProcessorBackend::ProcessLegacy(scoped_refptr<VideoFrame> frame,
+                                          LegacyFrameReadyCB cb) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(backend_sequence_checker_);
+
+  NOTIMPLEMENTED();
+}
+
+void ImageProcessorBackend::Reset() {
+  DVLOGF(3);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(backend_sequence_checker_);
+  // Do nothing as the default action.
+}
+
+}  // namespace media
+
+namespace std {
+
+void default_delete<media::ImageProcessorBackend>::operator()(
+    media::ImageProcessorBackend* ptr) const {
+  ptr->Destroy();
+}
+
+}  // namespace std

media/gpu/chromeos/image_processor_backend.h

+// Copyright 2019 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_CHROMEOS_IMAGE_PROCESSOR_BACKEND_H_
+#define MEDIA_GPU_CHROMEOS_IMAGE_PROCESSOR_BACKEND_H_
+
+#include <string>
+#include <vector>
+
+#include "base/callback_forward.h"
+#include "base/macros.h"
+#include "base/memory/scoped_refptr.h"
+#include "base/sequence_checker.h"
+#include "base/sequenced_task_runner.h"
+#include "media/base/color_plane_layout.h"
+#include "media/base/video_frame.h"
+#include "media/gpu/chromeos/fourcc.h"
+#include "media/gpu/media_gpu_export.h"
+#include "ui/gfx/geometry/size.h"
+
+namespace media {
+
+class MEDIA_GPU_EXPORT ImageProcessorBackend {
+ public:
+  // Callback for returning a processed image to the client.
+  using FrameReadyCB = base::OnceCallback<void(scoped_refptr<VideoFrame>)>;
+  // Callback for returning a processed image to the client.
+  // Used when calling the "legacy" Process() method with buffers that are
+  // managed by the processor. The first argument is the index of the returned
+  // buffer.
+  using LegacyFrameReadyCB =
+      base::OnceCallback<void(size_t, scoped_refptr<VideoFrame>)>;
+
+  // Callback for notifying client when error occurs.
+  using ErrorCB = base::RepeatingClosure;
+
+  // OutputMode is used as intermediate stage. The ultimate goal is to make
+  // ImageProcessor's clients all use IMPORT output mode.
+  // TODO(crbug.com/907767): Remove this once ImageProcessor always works as
+  // IMPORT mode for output.
+  enum class OutputMode { ALLOCATE, IMPORT };
+
+  // Encapsulates ImageProcessor input / output configurations.
+  struct MEDIA_GPU_EXPORT PortConfig {
+    PortConfig() = delete;
+    PortConfig(const PortConfig&);
+    PortConfig(
+        Fourcc fourcc,
+        const gfx::Size& size,
+        const std::vector<ColorPlaneLayout>& planes,
+        const gfx::Size& visible_size,
+        const std::vector<VideoFrame::StorageType>& preferred_storage_types);
+    ~PortConfig();
+
+    // Get the first |preferred_storage_types|.
+    // If |preferred_storage_types| is empty, return STORAGE_UNKNOWN.
+    VideoFrame::StorageType storage_type() const {
+      return preferred_storage_types.empty() ? VideoFrame::STORAGE_UNKNOWN
+                                             : preferred_storage_types.front();
+    }
+
+    // Output human readable string of PortConfig.
+    // Example:
+    // PortConfig(format::NV12, size:640x480, planes:[(640, 0, 307200),
+    // (640,0,153600)], visible_size:640x480, storage_types:[DMABUFS])
+    std::string ToString() const;
+
+    // Video frame format represented as fourcc type.
+    const Fourcc fourcc;
+
+    // Width and height of the video frame in pixels. This must include pixel
+    // data for the whole image; i.e. for YUV formats with subsampled chroma
+    // planes. If a visible portion of the image does not line up on a sample
+    // boundary, |size_| must be rounded up appropriately.
+    const gfx::Size size;
+
+    // Layout property (stride, offset, size of bytes) for each color plane.
+    const std::vector<ColorPlaneLayout> planes;
+    const gfx::Size visible_size;
+    // List of preferred storage types.
+    const std::vector<VideoFrame::StorageType> preferred_storage_types;
+  };
+
+  // Process |input_frame| and store in |output_frame|. Only used when output
+  // mode is IMPORT. After processing, call |cb| with |output_frame|.
+  virtual void Process(scoped_refptr<VideoFrame> input_frame,
+                       scoped_refptr<VideoFrame> output_frame,
+                       FrameReadyCB cb) = 0;
+
+  // Process |frame| and store in in a ImageProcessor-owned output buffer. Only
+  // used when output mode is ALLOCATE. After processing, call |cb| with the
+  // buffer.
+  // If ALLOCATE mode is not supported, the implementation is optional. In this
+  // case, this method should not be called and the default implementation will
+  // panic.
+  virtual void ProcessLegacy(scoped_refptr<VideoFrame> frame,
+                             LegacyFrameReadyCB cb);
+
+  // Drop all pending process requests. The default implementation is no-op.
+  virtual void Reset();
+
+  // Getter methods of data members, which could be called on any sequence.
+  const PortConfig& input_config() const { return input_config_; }
+  const PortConfig& output_config() const { return output_config_; }
+  OutputMode output_mode() const { return output_mode_; }
+
+ protected:
+  friend struct std::default_delete<ImageProcessorBackend>;
+
+  ImageProcessorBackend(
+      const PortConfig& input_config,
+      const PortConfig& output_config,
+      OutputMode output_mode,
+      ErrorCB error_cb,
+      scoped_refptr<base::SequencedTaskRunner> backend_task_runner);
+  virtual ~ImageProcessorBackend();
+  virtual void Destroy();
+
+  const PortConfig input_config_;
+  const PortConfig output_config_;
+
+  // TODO(crbug.com/907767): Remove |output_mode_| once ImageProcessor always
+  // works as IMPORT mode for output.
+  const OutputMode output_mode_;
+
+  // Call this callback when any error occurs.
+  const ErrorCB error_cb_;
+
+  // The main sequence and its checker. Except getter methods, all public
+  // methods and callbacks are called on this sequence.
+  scoped_refptr<base::SequencedTaskRunner> backend_task_runner_;
+  SEQUENCE_CHECKER(backend_sequence_checker_);
+};
+
+}  // namespace media
+
+namespace std {
+
+// Specialize std::default_delete to call Destroy().
+template <>
+struct MEDIA_GPU_EXPORT default_delete<media::ImageProcessorBackend> {
+  constexpr default_delete() = default;
+
+  template <
+      typename U,
+      typename = typename std::enable_if<
+          std::is_convertible<U*, media::ImageProcessorBackend*>::value>::type>
+  default_delete(const default_delete<U>& d) {}
+
+  void operator()(media::ImageProcessorBackend* ptr) const;
+};
+
+}  // namespace std
+
+#endif  // MEDIA_GPU_CHROMEOS_IMAGE_PROCESSOR_BACKEND_H_

media/gpu/chromeos/image_processor_factory.cc

@@ -6,6 +6,7 @@
 
 #include <stddef.h>
 
+#include "base/bind.h"
 #include "base/callback.h"
 #include "base/memory/scoped_refptr.h"
 #include "media/base/video_types.h"
@@ -97,26 +98,21 @@ std::unique_ptr<ImageProcessor> ImageProcessorFactory::Create(
     size_t num_buffers,
     scoped_refptr<base::SequencedTaskRunner> client_task_runner,
     ImageProcessor::ErrorCB error_cb) {
-  std::unique_ptr<ImageProcessor> image_processor;
+  std::vector<ImageProcessor::CreateBackendCB> create_funcs;
 #if BUILDFLAG(USE_VAAPI)
-  image_processor = VaapiImageProcessor::Create(input_config, output_config,
-                                                preferred_output_modes,
-                                                client_task_runner, error_cb);
-  if (image_processor)
-    return image_processor;
+  create_funcs.push_back(base::BindRepeating(&VaapiImageProcessor::Create));
 #endif  // BUILDFLAG(USE_VAAPI)
 #if BUILDFLAG(USE_V4L2_CODEC)
-  for (auto output_mode : preferred_output_modes) {
-    image_processor = V4L2ImageProcessor::Create(
-        client_task_runner, V4L2Device::Create(), input_config, output_config,
-        output_mode, num_buffers, error_cb);
-    if (image_processor)
-      return image_processor;
-  }
+  create_funcs.push_back(base::BindRepeating(
+      &V4L2ImageProcessor::Create, V4L2Device::Create(), num_buffers));
 #endif  // BUILDFLAG(USE_V4L2_CODEC)
-  for (auto output_mode : preferred_output_modes) {
-    image_processor = LibYUVImageProcessor::Create(
-        input_config, output_config, output_mode, client_task_runner, error_cb);
+  create_funcs.push_back(base::BindRepeating(&LibYUVImageProcessor::Create));
+
+  std::unique_ptr<ImageProcessor> image_processor;
+  for (auto& create_func : create_funcs) {
+    image_processor = ImageProcessor::Create(
+        std::move(create_func), input_config, output_config,
+        preferred_output_modes, error_cb, client_task_runner);
     if (image_processor)
       return image_processor;
   }

media/gpu/chromeos/libyuv_image_processor.cc

@@ -4,8 +4,6 @@
 
 #include "media/gpu/chromeos/libyuv_image_processor.h"
 
-#include "base/bind.h"
-#include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "media/gpu/chromeos/fourcc.h"
 #include "media/gpu/macros.h"
@@ -54,34 +52,14 @@ SupportResult IsFormatSupported(Fourcc input_fourcc, Fourcc output_fourcc) {
 
 }  // namespace
 
-LibYUVImageProcessor::LibYUVImageProcessor(
-    const ImageProcessor::PortConfig& input_config,
-    const ImageProcessor::PortConfig& output_config,
-    std::unique_ptr<VideoFrameMapper> video_frame_mapper,
-    scoped_refptr<base::SequencedTaskRunner> client_task_runner,
-    ErrorCB error_cb)
-    : ImageProcessor(input_config,
-                     output_config,
-                     OutputMode::IMPORT,
-                     std::move(client_task_runner)),
-      video_frame_mapper_(std::move(video_frame_mapper)),
-      error_cb_(error_cb),
-      process_thread_("LibYUVImageProcessorThread") {}
-
-LibYUVImageProcessor::~LibYUVImageProcessor() {
-  DCHECK_CALLED_ON_VALID_THREAD(client_thread_checker_);
-  Reset();
-
-  process_thread_.Stop();
-}
 
 // static
-std::unique_ptr<LibYUVImageProcessor> LibYUVImageProcessor::Create(
-    const ImageProcessor::PortConfig& input_config,
-    const ImageProcessor::PortConfig& output_config,
-    ImageProcessor::OutputMode output_mode,
-    scoped_refptr<base::SequencedTaskRunner> client_task_runner,
-    ErrorCB error_cb) {
+std::unique_ptr<ImageProcessorBackend> LibYUVImageProcessor::Create(
+    const PortConfig& input_config,
+    const PortConfig& output_config,
+    const std::vector<OutputMode>& preferred_output_modes,
+    ErrorCB error_cb,
+    scoped_refptr<base::SequencedTaskRunner> backend_task_runner) {
   VLOGF(2);
 
   std::unique_ptr<VideoFrameMapper> video_frame_mapper;
@@ -121,7 +99,7 @@ std::unique_ptr<LibYUVImageProcessor> LibYUVImageProcessor::Create(
     return nullptr;
   }
 
-  if (output_mode != OutputMode::IMPORT) {
+  if (!base::Contains(preferred_output_modes, OutputMode::IMPORT)) {
     VLOGF(2) << "Only support OutputMode::IMPORT";
     return nullptr;
   }
@@ -134,73 +112,66 @@ std::unique_ptr<LibYUVImageProcessor> LibYUVImageProcessor::Create(
     return nullptr;
   }
 
-  auto processor = base::WrapUnique(new LibYUVImageProcessor(
-      ImageProcessor::PortConfig(input_config.fourcc, input_config.size,
-                                 input_config.planes, input_config.visible_size,
-                                 {input_storage_type}),
-      ImageProcessor::PortConfig(
-          output_config.fourcc, output_config.size, output_config.planes,
-          output_config.visible_size, {output_storage_type}),
-      std::move(video_frame_mapper), std::move(client_task_runner),
-      std::move(error_cb)));
+  scoped_refptr<VideoFrame> intermediate_frame;
+
   if (res == SupportResult::SupportedWithPivot) {
-    processor->intermediate_frame_ =
+    intermediate_frame =
         VideoFrame::CreateFrame(PIXEL_FORMAT_I420, input_config.visible_size,
                                 gfx::Rect(input_config.visible_size),
                                 input_config.visible_size, base::TimeDelta());
-    if (!processor->intermediate_frame_) {
+    if (!intermediate_frame) {
       VLOGF(1) << "Failed to create intermediate frame";
       return nullptr;
     }
   }
 
-  if (!processor->process_thread_.Start()) {
-    VLOGF(1) << "Failed to start processing thread";
-    return nullptr;
-  }
-
+  auto processor =
+      base::WrapUnique<ImageProcessorBackend>(new LibYUVImageProcessor(
+          std::move(video_frame_mapper), std::move(intermediate_frame),
+          PortConfig(input_config.fourcc, input_config.size,
+                     input_config.planes, input_config.visible_size,
+                     {input_storage_type}),
+          PortConfig(output_config.fourcc, output_config.size,
+                     output_config.planes, output_config.visible_size,
+                     {output_storage_type}),
+          OutputMode::IMPORT, std::move(error_cb),
+          std::move(backend_task_runner)));
   VLOGF(2) << "LibYUVImageProcessor created for converting from "
            << input_config.ToString() << " to " << output_config.ToString();
   return processor;
 }
 
-bool LibYUVImageProcessor::ProcessInternal(
-    scoped_refptr<VideoFrame> input_frame,
-    scoped_refptr<VideoFrame> output_frame,
-    FrameReadyCB cb) {
-  DCHECK_CALLED_ON_VALID_THREAD(client_thread_checker_);
-  DVLOGF(4);
-  DCHECK_EQ(input_frame->layout().format(),
-            input_config_.fourcc.ToVideoPixelFormat());
-  DCHECK(input_frame->layout().coded_size() == input_config_.size);
-  DCHECK_EQ(output_frame->layout().format(),
-            output_config_.fourcc.ToVideoPixelFormat());
-  DCHECK(output_frame->layout().coded_size() == output_config_.size);
-  DCHECK(input_config_.storage_type() == input_frame->storage_type() ||
-         VideoFrame::IsStorageTypeMappable(input_frame->storage_type()));
-  DCHECK(VideoFrame::IsStorageTypeMappable(output_frame->storage_type()));
-
-  // Since process_thread_ is owned by this class. base::Unretained(this) and
-  // the raw pointer of that task runner are safe.
-  process_task_tracker_.PostTask(
-      process_thread_.task_runner().get(), FROM_HERE,
-      base::BindOnce(&LibYUVImageProcessor::ProcessTask, base::Unretained(this),
-                     std::move(input_frame), std::move(output_frame),
-                     std::move(cb)));
-  return true;
+LibYUVImageProcessor::LibYUVImageProcessor(
+    std::unique_ptr<VideoFrameMapper> video_frame_mapper,
+    scoped_refptr<VideoFrame> intermediate_frame,
+    const PortConfig& input_config,
+    const PortConfig& output_config,
+    OutputMode output_mode,
+    ErrorCB error_cb,
+    scoped_refptr<base::SequencedTaskRunner> backend_task_runner)
+    : ImageProcessorBackend(input_config,
+                            output_config,
+                            output_mode,
+                            std::move(error_cb),
+                            std::move(backend_task_runner)),
+      video_frame_mapper_(std::move(video_frame_mapper)),
+      intermediate_frame_(std::move(intermediate_frame)) {}
+
+LibYUVImageProcessor::~LibYUVImageProcessor() {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(backend_sequence_checker_);
 }
 
-void LibYUVImageProcessor::ProcessTask(scoped_refptr<VideoFrame> input_frame,
-                                       scoped_refptr<VideoFrame> output_frame,
-                                       FrameReadyCB cb) {
-  DCHECK(process_thread_.task_runner()->BelongsToCurrentThread());
+void LibYUVImageProcessor::Process(scoped_refptr<VideoFrame> input_frame,
+                                   scoped_refptr<VideoFrame> output_frame,
+                                   FrameReadyCB cb) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(backend_sequence_checker_);
   DVLOGF(4);
   if (input_frame->storage_type() == VideoFrame::STORAGE_DMABUFS) {
     DCHECK_NE(video_frame_mapper_.get(), nullptr);
     input_frame = video_frame_mapper_->Map(std::move(input_frame));
     if (!input_frame) {
       VLOGF(1) << "Failed to map input VideoFrame";
-      NotifyError();
+      error_cb_.Run();
       return;
     }
   }
@@ -208,30 +179,17 @@ void LibYUVImageProcessor::ProcessTask(scoped_refptr<VideoFrame> input_frame,
   int res = DoConversion(input_frame.get(), output_frame.get());
   if (res != 0) {
     VLOGF(1) << "libyuv::I420ToNV12 returns non-zero code: " << res;
-    NotifyError();
+    error_cb_.Run();
     return;
   }
   output_frame->set_timestamp(input_frame->timestamp());
-  client_task_runner_->PostTask(
-      FROM_HERE, base::BindOnce(std::move(cb), std::move(output_frame)));
-}
-
-bool LibYUVImageProcessor::Reset() {
-  DCHECK_CALLED_ON_VALID_THREAD(client_thread_checker_);
-
-  process_task_tracker_.TryCancelAll();
-  return true;
-}
-
-void LibYUVImageProcessor::NotifyError() {
-  VLOGF(1);
 
-  client_task_runner_->PostTask(FROM_HERE, error_cb_);
+  std::move(cb).Run(std::move(output_frame));
 }
 
 int LibYUVImageProcessor::DoConversion(const VideoFrame* const input,
                                        VideoFrame* const output) {
-  DCHECK(process_thread_.task_runner()->BelongsToCurrentThread());
+  DCHECK_CALLED_ON_VALID_SEQUENCE(backend_sequence_checker_);
 
 #define Y_U_V_DATA(fr)                                                \
   fr->data(VideoFrame::kYPlane), fr->stride(VideoFrame::kYPlane),     \

media/gpu/chromeos/libyuv_image_processor.h

@@ -5,24 +5,13 @@
 #ifndef MEDIA_GPU_CHROMEOS_LIBYUV_IMAGE_PROCESSOR_H_
 #define MEDIA_GPU_CHROMEOS_LIBYUV_IMAGE_PROCESSOR_H_
 
-#include <atomic>
 #include <memory>
 #include <vector>
 
-#include "base/callback_forward.h"
 #include "base/macros.h"
-#include "base/memory/scoped_refptr.h"
-#include "base/task/cancelable_task_tracker.h"
-#include "base/threading/thread.h"
-#include "base/threading/thread_checker.h"
-#include "build/build_config.h"
-#include "media/base/video_frame.h"
-#include "media/base/video_frame_layout.h"
-#include "media/base/video_types.h"
-#include "media/gpu/chromeos/image_processor.h"
+#include "media/gpu/chromeos/image_processor_backend.h"
 #include "media/gpu/media_gpu_export.h"
 #include "ui/gfx/geometry/rect.h"
-#include "ui/gfx/geometry/size.h"
 
 namespace media {
 
@@ -31,40 +20,35 @@ class VideoFrameMapper;
 // A software image processor which uses libyuv to perform format conversion.
 // It expects input VideoFrame is mapped into CPU space, and output VideoFrame
 // is allocated in user space.
-class MEDIA_GPU_EXPORT LibYUVImageProcessor : public ImageProcessor {
+class MEDIA_GPU_EXPORT LibYUVImageProcessor : public ImageProcessorBackend {
  public:
-  // ImageProcessor override
-  ~LibYUVImageProcessor() override;
-  bool Reset() override;
-
   // Factory method to create LibYUVImageProcessor to convert video format
   // specified in input_config and output_config. Provided |error_cb| will be
   // posted to the same thread Create() is called if an error occurs after
   // initialization.
   // Returns nullptr if it fails to create LibYUVImageProcessor.
-  static std::unique_ptr<LibYUVImageProcessor> Create(
-      const ImageProcessor::PortConfig& input_config,
-      const ImageProcessor::PortConfig& output_config,
-      ImageProcessor::OutputMode output_mode,
-      scoped_refptr<base::SequencedTaskRunner> client_task_runner,
-      ErrorCB error_cb);
+  static std::unique_ptr<ImageProcessorBackend> Create(
+      const PortConfig& input_config,
+      const PortConfig& output_config,
+      const std::vector<OutputMode>& preferred_output_modes,
+      ErrorCB error_cb,
+      scoped_refptr<base::SequencedTaskRunner> backend_task_runner);
+
+  // ImageProcessorBackend override
+  void Process(scoped_refptr<VideoFrame> input_frame,
+               scoped_refptr<VideoFrame> output_frame,
+               FrameReadyCB cb) override;
 
  private:
   LibYUVImageProcessor(
-      const ImageProcessor::PortConfig& input_config,
-      const ImageProcessor::PortConfig& output_config,
       std::unique_ptr<VideoFrameMapper> video_frame_mapper,
-      scoped_refptr<base::SequencedTaskRunner> client_task_runner,
-      ErrorCB error_cb);
-
-  // ImageProcessor override
-  bool ProcessInternal(scoped_refptr<VideoFrame> input_frame,
-                       scoped_refptr<VideoFrame> output_frame,
-                       FrameReadyCB cb) override;
-
-  void ProcessTask(scoped_refptr<VideoFrame> input_frame,
-                   scoped_refptr<VideoFrame> output_frame,
-                   FrameReadyCB cb);
+      scoped_refptr<VideoFrame> intermediate_frame,
+      const PortConfig& input_config,
+      const PortConfig& output_config,
+      OutputMode output_mode,
+      ErrorCB error_cb,
+      scoped_refptr<base::SequencedTaskRunner> backend_task_runner);
+  ~LibYUVImageProcessor() override;
 
   void NotifyError();
 
@@ -80,22 +64,6 @@ class MEDIA_GPU_EXPORT LibYUVImageProcessor : public ImageProcessor {
   // conversion method in libyuv, e.g., RGBA -> I420 (pivot) -> NV12.
   scoped_refptr<VideoFrame> intermediate_frame_;
 
-  // Error callback to the client.
-  ErrorCB error_cb_;
-
-  // Thread to process frame format conversion.
-  base::Thread process_thread_;
-
-  // CancelableTaskTracker for ProcessTask().
-  // Because ProcessTask is posted from |client_task_runner_|'s thread to
-  // another sequence, |process_thread_|, it is unsafe to cancel the posted task
-  // from |client_task_runner_|'s thread using CancelableCallback and WeakPtr
-  // binding. CancelableTaskTracker is designed to deal with this scenario.
-  base::CancelableTaskTracker process_task_tracker_;
-
-  // Checker for the thread that creates this LibYUVImageProcessor.
-  THREAD_CHECKER(client_thread_checker_);
-
   DISALLOW_COPY_AND_ASSIGN(LibYUVImageProcessor);
 };
 

media/gpu/v4l2/v4l2_image_processor.h

@@ -13,21 +13,15 @@
 
 #include <linux/videodev2.h>
 
-#include "base/callback_forward.h"
 #include "base/containers/queue.h"
-#include "base/files/scoped_file.h"
 #include "base/macros.h"
 #include "base/memory/scoped_refptr.h"
+#include "base/memory/weak_ptr.h"
 #include "base/optional.h"
 #include "base/sequence_checker.h"
 #include "base/sequenced_task_runner.h"
 #include "base/single_thread_task_runner.h"
-#include "base/task/cancelable_task_tracker.h"
-#include "base/threading/thread.h"
-#include "base/threading/thread_checker.h"
-#include "media/base/video_frame.h"
-#include "media/base/video_frame_layout.h"
-#include "media/gpu/chromeos/image_processor.h"
+#include "media/gpu/chromeos/image_processor_backend.h"
 #include "media/gpu/media_gpu_export.h"
 #include "media/gpu/v4l2/v4l2_device.h"
 #include "ui/gfx/geometry/size.h"
@@ -37,7 +31,7 @@ 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 : public ImageProcessor {
+class MEDIA_GPU_EXPORT V4L2ImageProcessor : public ImageProcessorBackend {
  public:
   // Factory method to create V4L2ImageProcessor to convert from
   // input_config to output_config. The number of input buffers and output
@@ -48,18 +42,22 @@ class MEDIA_GPU_EXPORT V4L2ImageProcessor : public ImageProcessor {
   // TODO(crbug.com/917798): remove |device| parameter once
   //     V4L2VideoDecodeAccelerator no longer creates and uses
   //     |image_processor_device_| before V4L2ImageProcessor is created.
-  static std::unique_ptr<V4L2ImageProcessor> Create(
-      scoped_refptr<base::SequencedTaskRunner> client_task_runner,
+  static std::unique_ptr<ImageProcessorBackend> Create(
       scoped_refptr<V4L2Device> device,
-      const ImageProcessor::PortConfig& input_config,
-      const ImageProcessor::PortConfig& output_config,
-      const ImageProcessor::OutputMode output_mode,
       size_t num_buffers,
-      ErrorCB error_cb);
+      const PortConfig& input_config,
+      const PortConfig& output_config,
+      const std::vector<OutputMode>& preferred_output_modes,
+      ErrorCB error_cb,
+      scoped_refptr<base::SequencedTaskRunner> backend_task_runner);
 
   // ImageProcessor implementation.
-  ~V4L2ImageProcessor() override;
-  bool Reset() override;
+  void Process(scoped_refptr<VideoFrame> input_frame,
+               scoped_refptr<VideoFrame> output_frame,
+               FrameReadyCB cb) override;
+  void ProcessLegacy(scoped_refptr<VideoFrame> frame,
+                     LegacyFrameReadyCB cb) override;
+  void Reset() override;
 
   // Returns true if image processing is supported on this platform.
   static bool IsSupported();
@@ -82,6 +80,9 @@ class MEDIA_GPU_EXPORT V4L2ImageProcessor : public ImageProcessor {
                               size_t* num_planes);
 
  private:
+  // Callback for initialization.
+  using InitCB = base::OnceCallback<void(bool)>;
+
   // Job record. Jobs are processed in a FIFO order. |input_frame| will be
   // processed and the result written into |output_frame|. Once processing is
   // complete, |ready_cb| or |legacy_ready_cb| will be called depending on which
@@ -96,18 +97,34 @@ class MEDIA_GPU_EXPORT V4L2ImageProcessor : public ImageProcessor {
     size_t output_buffer_id;
   };
 
+  static std::unique_ptr<ImageProcessorBackend> CreateWithOutputMode(
+      scoped_refptr<V4L2Device> device,
+      size_t num_buffers,
+      const PortConfig& input_config,
+      const PortConfig& output_config,
+      const OutputMode& preferred_output_modes,
+      ErrorCB error_cb,
+      scoped_refptr<base::SequencedTaskRunner> backend_task_runner);
+
   V4L2ImageProcessor(
-      scoped_refptr<base::SequencedTaskRunner> client_task_runner,
+      scoped_refptr<base::SequencedTaskRunner> backend_task_runner,
       scoped_refptr<V4L2Device> device,
-      const ImageProcessor::PortConfig& input_config,
-      const ImageProcessor::PortConfig& output_config,
+      const PortConfig& input_config,
+      const PortConfig& output_config,
       v4l2_memory input_memory_type,
       v4l2_memory output_memory_type,
       OutputMode output_mode,
       size_t num_buffers,
       ErrorCB error_cb);
+  ~V4L2ImageProcessor() override;
+  void Destroy() override;
+  // Stop all processing on |poll_task_runner_|.
+  void DestroyOnPollSequence();
+
+  // Initialize on |backend_task_runner_|. After finished, call |init_cb|
+  // with the result whether initialization is successful or not
+  void Initialize(InitCB init_cb);
 
-  bool Initialize();
   void EnqueueInput(const JobRecord* job_record);
   void EnqueueOutput(JobRecord* job_record);
   void Dequeue();
@@ -128,50 +145,18 @@ class MEDIA_GPU_EXPORT V4L2ImageProcessor : public ImageProcessor {
   void NotifyError();
 
   // ImageProcessor implementation.
-  bool ProcessInternal(scoped_refptr<VideoFrame> frame,
-                       LegacyFrameReadyCB cb) override;
-  bool ProcessInternal(scoped_refptr<VideoFrame> input_frame,
-                       scoped_refptr<VideoFrame> output_frame,
-                       FrameReadyCB cb) override;
-
-  void ProcessTask(std::unique_ptr<JobRecord> job_record);
   void ProcessJobsTask();
   void ServiceDeviceTask();
 
-  // Call |output_cb| on |client_task_runner_|.
-  void OutputFrameOnClientSequence(base::OnceClosure output_cb);
-
-  // Allocate/Destroy the input/output V4L2 buffers.
-  void AllocateBuffersTask(bool* result, base::WaitableEvent* done);
-
-  // Ran on device_poll_thread_ to wait for device events.
+  // Ran on |poll_task_runner_| to wait for device events.
   void DevicePollTask(bool poll_device);
 
-  // Stop all processing and clean up on |device_task_runner_|.
-  void DestroyOnDeviceSequence(base::WaitableEvent* event);
-  // Stop all processing on |poll_task_runner_|.
-  void DestroyOnPollSequence(base::WaitableEvent* event);
-
-  // Clean up pending job on |device_task_runner_|, and signal |event| after
-  // reset is finished.
-  void ResetTask(base::WaitableEvent* event);
-
   const v4l2_memory input_memory_type_;
   const v4l2_memory output_memory_type_;
 
   // V4L2 device in use.
   scoped_refptr<V4L2Device> device_;
 
-  // Sequence to communicate with the V4L2 device.
-  scoped_refptr<base::SingleThreadTaskRunner> device_task_runner_;
-  // Thread used to poll the V4L2 for events only.
-  scoped_refptr<base::SingleThreadTaskRunner> poll_task_runner_;
-
-  // It is unsafe to cancel the posted tasks from different sequence using
-  // CancelableCallback and WeakPtr binding. We use CancelableTaskTracker to
-  // safely cancel tasks on |device_task_runner_| from |client_task_runner_|.
-  base::CancelableTaskTracker process_task_tracker_;
-
   // All the below members are to be accessed from |device_task_runner_| only
   // (if it's running).
   base::queue<std::unique_ptr<JobRecord>> input_job_queue_;
@@ -183,22 +168,15 @@ class MEDIA_GPU_EXPORT V4L2ImageProcessor : public ImageProcessor {
   // The number of input or output buffers.
   const size_t num_buffers_;
 
-  // Error callback to the client.
-  ErrorCB error_cb_;
-
-  // Checker for the device thread owned by this V4L2ImageProcessor.
-  SEQUENCE_CHECKER(device_sequence_checker_);
-  // Checker for the device thread owned by this V4L2ImageProcessor.
+  // Sequence and its checker used to poll the V4L2 for events only.
+  scoped_refptr<base::SingleThreadTaskRunner> poll_task_runner_;
   SEQUENCE_CHECKER(poll_sequence_checker_);
 
-  // WeakPtr bound to |client_task_runner_|.
-  base::WeakPtr<V4L2ImageProcessor> client_weak_this_;
-  // WeakPtr bound to |device_task_runner_|.
-  base::WeakPtr<V4L2ImageProcessor> device_weak_this_;
+  // WeakPtr bound to |backend_task_runner_|.
+  base::WeakPtr<V4L2ImageProcessor> backend_weak_this_;
   // WeakPtr bound to |poll_task_runner_|.
   base::WeakPtr<V4L2ImageProcessor> poll_weak_this_;
-  base::WeakPtrFactory<V4L2ImageProcessor> client_weak_this_factory_{this};
-  base::WeakPtrFactory<V4L2ImageProcessor> device_weak_this_factory_{this};
+  base::WeakPtrFactory<V4L2ImageProcessor> backend_weak_this_factory_{this};
   base::WeakPtrFactory<V4L2ImageProcessor> poll_weak_this_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(V4L2ImageProcessor);

media/gpu/v4l2/v4l2_vda_helpers.cc

@@ -4,6 +4,7 @@
 
 #include "media/gpu/v4l2/v4l2_vda_helpers.h"
 
+#include "base/bind.h"
 #include "media/base/color_plane_layout.h"
 #include "media/gpu/chromeos/fourcc.h"
 #include "media/gpu/macros.h"
@@ -74,13 +75,15 @@ std::unique_ptr<ImageProcessor> CreateImageProcessor(
     ImageProcessor::ErrorCB error_cb) {
   // TODO(crbug.com/917798): Use ImageProcessorFactory::Create() once we remove
   //     |image_processor_device_| from V4L2VideoDecodeAccelerator.
-  auto image_processor = V4L2ImageProcessor::Create(
-      std::move(client_task_runner), image_processor_device,
+  auto image_processor = ImageProcessor::Create(
+      base::BindRepeating(&V4L2ImageProcessor::Create, image_processor_device,
+                          nb_buffers),
       ImageProcessor::PortConfig(vda_output_format, vda_output_coded_size, {},
                                  visible_size, {VideoFrame::STORAGE_DMABUFS}),
       ImageProcessor::PortConfig(ip_output_format, ip_output_coded_size, {},
                                  visible_size, {VideoFrame::STORAGE_DMABUFS}),
-      image_processor_output_mode, nb_buffers, std::move(error_cb));
+      {image_processor_output_mode}, std::move(error_cb),
+      std::move(client_task_runner));
   if (!image_processor)
     return nullptr;
 

media/gpu/vaapi/vaapi_image_processor.cc

@@ -13,11 +13,8 @@
 #include "base/memory/ptr_util.h"
 #include "base/memory/scoped_refptr.h"
 #include "base/metrics/histogram_functions.h"
-#include "base/sequence_checker.h"
-#include "base/sequenced_task_runner.h"
 #include "base/stl_util.h"
 #include "base/task/post_task.h"
-#include "media/base/video_frame.h"
 #include "media/gpu/chromeos/fourcc.h"
 #include "media/gpu/chromeos/platform_video_frame_utils.h"
 #include "media/gpu/macros.h"
@@ -35,11 +32,9 @@ enum class VaIPFailure {
   kMaxValue = kVaapiVppError,
 };
 
-void ReportToUMA(scoped_refptr<base::SequencedTaskRunner> task_runner,
-                 base::RepeatingClosure error_cb,
-                 VaIPFailure failure) {
+void ReportToUMA(base::RepeatingClosure error_cb, VaIPFailure failure) {
   base::UmaHistogramEnumeration("Media.VAIP.VppFailure", failure);
-  task_runner->PostTask(FROM_HERE, error_cb);
+  error_cb.Run();
 }
 
 bool IsSupported(uint32_t input_va_fourcc,
@@ -71,37 +66,15 @@ bool IsSupported(uint32_t input_va_fourcc,
   return true;
 }
 
-void ProcessFrame(scoped_refptr<VaapiWrapper> vaapi_wrapper,
-                  scoped_refptr<VideoFrame> input_frame,
-                  scoped_refptr<VideoFrame> output_frame,
-                  scoped_refptr<base::SequencedTaskRunner> client_task_runner,
-                  ImageProcessor::FrameReadyCB cb) {
-  DVLOGF(4);
-
-  auto src_va_surface =
-      vaapi_wrapper->CreateVASurfaceForVideoFrame(input_frame.get());
-  auto dst_va_surface =
-      vaapi_wrapper->CreateVASurfaceForVideoFrame(output_frame.get());
-  if (!src_va_surface || !dst_va_surface) {
-    // Failed to create VASurface for frames. |cb| isn't executed in the case.
-    return;
-  }
-  // VA-API performs pixel format conversion and scaling without any filters.
-  vaapi_wrapper->BlitSurface(std::move(src_va_surface),
-                             std::move(dst_va_surface));
-  client_task_runner->PostTask(
-      FROM_HERE, base::BindOnce(std::move(cb), std::move(output_frame)));
-}
-
 }  // namespace
 
 // static
-std::unique_ptr<VaapiImageProcessor> VaapiImageProcessor::Create(
-    const ImageProcessor::PortConfig& input_config,
-    const ImageProcessor::PortConfig& output_config,
-    const std::vector<ImageProcessor::OutputMode>& preferred_output_modes,
-    scoped_refptr<base::SequencedTaskRunner> client_task_runner,
-    const base::RepeatingClosure& error_cb) {
+std::unique_ptr<ImageProcessorBackend> VaapiImageProcessor::Create(
+    const PortConfig& input_config,
+    const PortConfig& output_config,
+    const std::vector<OutputMode>& preferred_output_modes,
+    ErrorCB error_cb,
+    scoped_refptr<base::SequencedTaskRunner> backend_task_runner) {
 // VaapiImageProcessor supports ChromeOS only.
 #if !defined(OS_CHROMEOS)
   return nullptr;
@@ -150,8 +123,7 @@ std::unique_ptr<VaapiImageProcessor> VaapiImageProcessor::Create(
 
   auto vaapi_wrapper = VaapiWrapper::Create(
       VaapiWrapper::kVideoProcess, VAProfileNone,
-      base::BindRepeating(&ReportToUMA, client_task_runner, error_cb,
-                          VaIPFailure::kVaapiVppError));
+      base::BindRepeating(&ReportToUMA, error_cb, VaIPFailure::kVaapiVppError));
   if (!vaapi_wrapper) {
     VLOGF(1) << "Failed to create VaapiWrapper";
     return nullptr;
@@ -171,58 +143,48 @@ std::unique_ptr<VaapiImageProcessor> VaapiImageProcessor::Create(
   // GetPlatformVideoFrameLayout() with a proper gfx::BufferUsage.
   // TODO(crbug.com/898423): Adjust layout once ImageProcessor provide the use
   // scenario.
-  return base::WrapUnique(new VaapiImageProcessor(
-      input_config, output_config, std::move(vaapi_wrapper),
-      std::move(client_task_runner)));
+  return base::WrapUnique<ImageProcessorBackend>(new VaapiImageProcessor(
+      std::move(vaapi_wrapper), input_config, output_config, OutputMode::IMPORT,
+      std::move(error_cb), std::move(backend_task_runner)));
 }
 
 VaapiImageProcessor::VaapiImageProcessor(
-    const ImageProcessor::PortConfig& input_config,
-    const ImageProcessor::PortConfig& output_config,
     scoped_refptr<VaapiWrapper> vaapi_wrapper,
-    scoped_refptr<base::SequencedTaskRunner> client_task_runner)
-    : ImageProcessor(input_config,
-                     output_config,
-                     OutputMode::IMPORT,
-                     std::move(client_task_runner)),
-      processor_task_runner_(base::CreateSequencedTaskRunner(
-          base::TaskTraits{base::ThreadPool()})),
-      vaapi_wrapper_(std::move(vaapi_wrapper)) {
-  DETACH_FROM_SEQUENCE(client_sequence_checker_);
-}
+    const PortConfig& input_config,
+    const PortConfig& output_config,
+    OutputMode output_mode,
+    ErrorCB error_cb,
+    scoped_refptr<base::SequencedTaskRunner> backend_task_runner)
+    : ImageProcessorBackend(input_config,
+                            output_config,
+                            output_mode,
+                            std::move(error_cb),
+                            std::move(backend_task_runner)),
+      vaapi_wrapper_(std::move(vaapi_wrapper)) {}
 
 VaapiImageProcessor::~VaapiImageProcessor() {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(client_sequence_checker_);
+  DCHECK_CALLED_ON_VALID_SEQUENCE(backend_sequence_checker_);
 }
 
-bool VaapiImageProcessor::ProcessInternal(
-    scoped_refptr<VideoFrame> input_frame,
-    scoped_refptr<VideoFrame> output_frame,
-    FrameReadyCB cb) {
+void VaapiImageProcessor::Process(scoped_refptr<VideoFrame> input_frame,
+                                  scoped_refptr<VideoFrame> output_frame,
+                                  FrameReadyCB cb) {
   DVLOGF(4);
-  DCHECK_CALLED_ON_VALID_SEQUENCE(client_sequence_checker_);
-
-  FrameReadyCB on_frame_ready =
-      base::BindOnce(&VaapiImageProcessor::OnOutputFrameReady,
-                     weak_factory_.GetWeakPtr(), std::move(cb));
-  processor_task_runner_->PostTask(
-      FROM_HERE,
-      base::BindOnce(&ProcessFrame, vaapi_wrapper_, std::move(input_frame),
-                     std::move(output_frame), client_task_runner_,
-                     std::move(on_frame_ready)));
-  return true;
-}
-
-void VaapiImageProcessor::OnOutputFrameReady(FrameReadyCB cb,
-                                             scoped_refptr<VideoFrame> frame) {
-  std::move(cb).Run(std::move(frame));
-}
+  DCHECK_CALLED_ON_VALID_SEQUENCE(backend_sequence_checker_);
 
-bool VaapiImageProcessor::Reset() {
-  VLOGF(2);
-  DCHECK_CALLED_ON_VALID_SEQUENCE(client_sequence_checker_);
+  auto src_va_surface =
+      vaapi_wrapper_->CreateVASurfaceForVideoFrame(input_frame.get());
+  auto dst_va_surface =
+      vaapi_wrapper_->CreateVASurfaceForVideoFrame(output_frame.get());
+  if (!src_va_surface || !dst_va_surface) {
+    // Failed to create VASurface for frames. |cb| isn't executed in the case.
+    return;
+  }
+  // VA-API performs pixel format conversion and scaling without any filters.
+  vaapi_wrapper_->BlitSurface(std::move(src_va_surface),
+                              std::move(dst_va_surface));
 
-  weak_factory_.InvalidateWeakPtrs();
-  return true;
+  std::move(cb).Run(std::move(output_frame));
 }
+
 }  // namespace media

media/gpu/vaapi/vaapi_image_processor.h

@@ -7,12 +7,9 @@
 
 #include <memory>
 
-#include "base/callback_forward.h"
 #include "base/macros.h"
-#include "base/memory/weak_ptr.h"
-#include "base/sequenced_task_runner.h"
-#include "build/build_config.h"
-#include "media/gpu/chromeos/image_processor.h"
+#include "media/gpu/chromeos/image_processor_backend.h"
+#include "media/gpu/media_gpu_export.h"
 
 namespace media {
 
@@ -20,46 +17,37 @@ class VaapiWrapper;
 
 // ImageProcessor that is hardware accelerated with VA-API. This ImageProcessor
 // supports DmaBuf only for both input and output.
-class VaapiImageProcessor : public ImageProcessor {
+class VaapiImageProcessor : public ImageProcessorBackend {
  public:
   // Factory method to create VaapiImageProcessor for a buffer conversion
   // specified by |input_config| and |output_config|. Provided |error_cb| will
   // be posted to the same thread that executes Create(), if an error occurs
   // after initialization.
   // Returns nullptr if it fails to create VaapiImageProcessor.
-  static std::unique_ptr<VaapiImageProcessor> Create(
-      const ImageProcessor::PortConfig& input_config,
-      const ImageProcessor::PortConfig& output_config,
-      const std::vector<ImageProcessor::OutputMode>& preferred_output_modes,
-      scoped_refptr<base::SequencedTaskRunner> client_task_runner,
-      const base::RepeatingClosure& error_cb);
+  static std::unique_ptr<ImageProcessorBackend> Create(
+      const PortConfig& input_config,
+      const PortConfig& output_config,
+      const std::vector<OutputMode>& preferred_output_modes,
+      ErrorCB error_cb,
+      scoped_refptr<base::SequencedTaskRunner> backend_task_runner);
 
   // ImageProcessor implementation.
-  ~VaapiImageProcessor() override;
-  bool Reset() override;
+  void Process(scoped_refptr<VideoFrame> input_frame,
+               scoped_refptr<VideoFrame> output_frame,
+               FrameReadyCB cb) override;
 
  private:
   VaapiImageProcessor(
-      const ImageProcessor::PortConfig& input_config,
-      const ImageProcessor::PortConfig& output_config,
       scoped_refptr<VaapiWrapper> vaapi_wrapper,
-      scoped_refptr<base::SequencedTaskRunner> client_task_runner);
-
-  // ImageProcessor implementation.
-  bool ProcessInternal(scoped_refptr<VideoFrame> input_frame,
-                       scoped_refptr<VideoFrame> output_frame,
-                       FrameReadyCB cb) override;
-
-  // Callback invoked on completion of VAAPI frame processing tasks.
-  void OnOutputFrameReady(FrameReadyCB cb, scoped_refptr<VideoFrame> frame);
-
-  // Sequence task runner in which the buffer conversion is performed.
-  const scoped_refptr<base::SequencedTaskRunner> processor_task_runner_;
+      const PortConfig& input_config,
+      const PortConfig& output_config,
+      OutputMode output_mode,
+      ErrorCB error_cb,
+      scoped_refptr<base::SequencedTaskRunner> backend_task_runner);
+  ~VaapiImageProcessor() override;
 
   const scoped_refptr<VaapiWrapper> vaapi_wrapper_;
 
-  base::WeakPtrFactory<VaapiImageProcessor> weak_factory_{this};
-
   DISALLOW_COPY_AND_ASSIGN(VaapiImageProcessor);
 };