VAAPI-JDA: Extract VideoFrame-independent code into VaapiJpegDecoder.

This CL takes VideoFrame-independent code from
VaapiJpegDecodeAccelerator's DecodeTask() and OutputPicture() methods
and puts it in VaapiJpegDecoder::DoDecode(). The idea is for
VaapiJpegDecoder to be responsible for dealing with the VaapiWrapper, so
now part of the initialization of the VaapiJpegDecodeAccelerator is
delegated to VaapiJpegDecoder.

Now, VaapiJpegDecoder::DoDecode() will take encoded data. It will do the
parsing, VAAPI buffer submission, decode execution, and VAImage
extraction. It will then return the decoded output as a ScopedVAImage.
The intent is to have a VaapiJpegDecoder member in two classes:
VaapiJpegDecodeAccelerator (existing) and the eventual implementation of
gpu::ImageDecodeAcceleratorWorker. In the first case, we use the
ScopedVAImage to copy the decoded data into a VideoFrame. In the second
case, we use it to copy the decoded data into a vector.

The unit tests are modified accordingly. Additionally,
::testing::FLAGS_gtest_death_test_style = "threadsafe" is added to two
of the tests due to safety issues when using EXPECT_DCHECK_DEATH when
there are multiple threads.

The reason for introducing the grayscale image is that before, the
DecodeFail test case was using a normal image and poking at the results
of the JPEG parsing so that it failed. Now, the decoder interface
doesn't take a JpegParseResult, so we can't do the same poking. Instead,
we give it a real unsupported image.

Bug: 924310
Test: jpeg_decode_accelerator_unittest passes on nocturne and grunt.
Change-Id: I52d9760d233e63b2831408f60309803984784903
Reviewed-on: https://chromium-review.googlesource.com/c/1427457
Commit-Queue: Andres Calderon Jaramillo <andrescj@chromium.org>
Auto-Submit: Andres Calderon Jaramillo <andrescj@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#634298}

media/gpu/vaapi/BUILD.gn

@@ -142,7 +142,6 @@ source_set("jpeg_decoder_unit_test") {
     ":vaapi",
     "//base",
     "//base/test:test_support",
-    "//media",
     "//media:test_support",
     "//testing/gtest",
     "//ui/gfx/geometry",

media/gpu/vaapi/vaapi_jpeg_decode_accelerator.h

@@ -14,13 +14,8 @@
 #include "base/memory/weak_ptr.h"
 #include "base/threading/thread.h"
 #include "media/gpu/media_gpu_export.h"
+#include "media/gpu/vaapi/vaapi_jpeg_decoder.h"
 #include "media/video/jpeg_decode_accelerator.h"
-#include "ui/gfx/geometry/size.h"
-
-// This data type is defined in va/va.h using typedef, reproduced here.
-// TODO(andrescj): remove this once VaapiJpegDecoder is responsible for
-// obtaining the VAImage.
-using VASurfaceID = unsigned int;
 
 namespace base {
 class SingleThreadTaskRunner;
@@ -29,8 +24,8 @@ class SingleThreadTaskRunner;
 namespace media {
 
 class BitstreamBuffer;
+class ScopedVAImage;
 class UnalignedSharedMemory;
-class VaapiWrapper;
 class VideoFrame;
 
 // Class to provide JPEG decode acceleration for Intel systems with hardware
@@ -69,13 +64,11 @@ class MEDIA_GPU_EXPORT VaapiJpegDecodeAccelerator
                   std::unique_ptr<UnalignedSharedMemory> shm,
                   scoped_refptr<VideoFrame> video_frame);
 
-  // Puts contents of |va_surface| into given |video_frame|, releases the
-  // surface and passes the |input_buffer_id| of the resulting picture to
-  // client for output.
-  bool OutputPicture(VASurfaceID va_surface_id,
-                     uint32_t va_surface_format,
-                     int32_t input_buffer_id,
-                     const scoped_refptr<VideoFrame>& video_frame);
+  // Puts contents of |image| into given |video_frame| and passes the
+  // |input_buffer_id| of the resulting picture to client for output.
+  bool OutputPictureOnTaskRunner(std::unique_ptr<ScopedVAImage> image,
+                                 int32_t input_buffer_id,
+                                 const scoped_refptr<VideoFrame>& video_frame);
 
   // ChildThread's task runner.
   const scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
@@ -86,23 +79,16 @@ class MEDIA_GPU_EXPORT VaapiJpegDecodeAccelerator
   // The client of this class.
   Client* client_;
 
-  scoped_refptr<VaapiWrapper> vaapi_wrapper_;
+  VaapiJpegDecoder decoder_;
 
-  // Comes after vaapi_wrapper_ to ensure its destructor is executed before
-  // |vaapi_wrapper_| is destroyed.
+  // Comes after |decoder_| to ensure its destructor is executed before
+  // |decoder_| is destroyed.
   base::Thread decoder_thread_;
   // Use this to post tasks to |decoder_thread_| instead of
   // |decoder_thread_.task_runner()| because the latter will be NULL once
   // |decoder_thread_.Stop()| returns.
   scoped_refptr<base::SingleThreadTaskRunner> decoder_task_runner_;
 
-  // The current VA surface for decoding.
-  VASurfaceID va_surface_id_;
-  // The coded size associated with |va_surface_id_|.
-  gfx::Size coded_size_;
-  // The VA RT format associated with |va_surface_id_|.
-  unsigned int va_rt_format_;
-
   // WeakPtr factory for use in posting tasks from |decoder_task_runner_| back
   // to |task_runner_|.  Since |decoder_thread_| is a fully owned member of
   // this class, tasks posted to it may use base::Unretained(this), and tasks

media/gpu/vaapi/vaapi_jpeg_decoder.cc

@@ -8,6 +8,7 @@
 
 #include <iostream>
 #include <type_traits>
+#include <vector>
 
 #include <va/va.h>
 
@@ -15,6 +16,7 @@
 #include "base/stl_util.h"
 #include "media/filters/jpeg_parser.h"
 #include "media/gpu/macros.h"
+#include "media/gpu/vaapi/vaapi_utils.h"
 #include "media/gpu/vaapi/vaapi_wrapper.h"
 
 namespace media {
@@ -33,6 +35,8 @@ constexpr VAImageFormat kImageFormatYUYV = {
     .bits_per_pixel = 16,
 };
 
+constexpr unsigned int kInvalidVaRtFormat = 0u;
+
 static void FillPictureParameters(
     const JpegFrameHeader& frame_header,
     VAPictureParameterBufferJPEGBaseline* pic_param) {
@@ -193,10 +197,9 @@ static bool IsVaapiSupportedJpeg(const JpegParseResult& jpeg) {
   return true;
 }
 
-}  // namespace
-
-bool VaSurfaceFormatToImageFormat(uint32_t va_rt_format,
-                                  VAImageFormat* va_image_format) {
+// Convert the specified surface format to the associated output image format.
+static bool VaSurfaceFormatToImageFormat(unsigned int va_rt_format,
+                                         VAImageFormat* va_image_format) {
   switch (va_rt_format) {
     case VA_RT_FORMAT_YUV420:
       *va_image_format = kImageFormatI420;
@@ -209,7 +212,8 @@ bool VaSurfaceFormatToImageFormat(uint32_t va_rt_format,
   }
 }
 
-unsigned int VaSurfaceFormatForJpeg(const JpegFrameHeader& frame_header) {
+static unsigned int VaSurfaceFormatForJpeg(
+    const JpegFrameHeader& frame_header) {
   // The range of sampling factor is [1, 4]. Pack them into integer to make the
   // matching code simpler. For example, 0x211 means the sampling factor are 2,
   // 1, 1 for 3 components.
@@ -244,53 +248,157 @@ unsigned int VaSurfaceFormatForJpeg(const JpegFrameHeader& frame_header) {
            << frame_header.num_components << ", h=" << std::hex << h
            << ", v=" << v;
 
-  return 0;
+  return kInvalidVaRtFormat;
 }
 
-bool VaapiJpegDecoder::DoDecode(VaapiWrapper* vaapi_wrapper,
-                                const JpegParseResult& parse_result,
-                                VASurfaceID va_surface) {
-  DCHECK_NE(va_surface, VA_INVALID_SURFACE);
-  if (!IsVaapiSupportedJpeg(parse_result))
+}  // namespace
+
+VaapiJpegDecoder::VaapiJpegDecoder()
+    : va_surface_id_(VA_INVALID_SURFACE), va_rt_format_(kInvalidVaRtFormat) {}
+
+VaapiJpegDecoder::~VaapiJpegDecoder() = default;
+
+bool VaapiJpegDecoder::Initialize(const base::RepeatingClosure& error_uma_cb) {
+  vaapi_wrapper_ = VaapiWrapper::Create(VaapiWrapper::kDecode,
+                                        VAProfileJPEGBaseline, error_uma_cb);
+  if (!vaapi_wrapper_) {
+    VLOGF(1) << "Failed initializing VAAPI";
     return false;
+  }
+  return true;
+}
+
+std::unique_ptr<ScopedVAImage> VaapiJpegDecoder::DoDecode(
+    base::span<const uint8_t> encoded_image,
+    VaapiJpegDecodeStatus* status) {
+  if (!vaapi_wrapper_) {
+    VLOGF(1) << "VaapiJpegDecoder has not been initialized";
+    *status = VaapiJpegDecodeStatus::kInvalidState;
+    return nullptr;
+  }
+
+  // Parse the JPEG encoded data.
+  JpegParseResult parse_result;
+  if (!ParseJpegPicture(encoded_image.data(), encoded_image.size(),
+                        &parse_result)) {
+    VLOGF(1) << "ParseJpegPicture failed";
+    *status = VaapiJpegDecodeStatus::kParseJpegFailed;
+    return nullptr;
+  }
+
+  // Figure out the right format for the VaSurface.
+  const unsigned int picture_va_rt_format =
+      VaSurfaceFormatForJpeg(parse_result.frame_header);
+  if (picture_va_rt_format == kInvalidVaRtFormat) {
+    VLOGF(1) << "Unsupported subsampling";
+    *status = VaapiJpegDecodeStatus::kUnsupportedSubsampling;
+    return nullptr;
+  }
+
+  // Make sure this JPEG can be decoded.
+  if (!IsVaapiSupportedJpeg(parse_result)) {
+    VLOGF(1) << "The supplied JPEG is unsupported";
+    *status = VaapiJpegDecodeStatus::kUnsupportedJpeg;
+    return nullptr;
+  }
+
+  // Prepare the VaSurface for decoding.
+  const gfx::Size new_coded_size(
+      base::strict_cast<int>(parse_result.frame_header.coded_width),
+      base::strict_cast<int>(parse_result.frame_header.coded_height));
+  if (new_coded_size != coded_size_ || va_surface_id_ == VA_INVALID_SURFACE ||
+      picture_va_rt_format != va_rt_format_) {
+    vaapi_wrapper_->DestroyContextAndSurfaces();
+    va_surface_id_ = VA_INVALID_SURFACE;
+    va_rt_format_ = picture_va_rt_format;
+
+    std::vector<VASurfaceID> va_surfaces;
+    if (!vaapi_wrapper_->CreateContextAndSurfaces(va_rt_format_, new_coded_size,
+                                                  1, &va_surfaces)) {
+      VLOGF(1) << "Could not create the context or the surface";
+      *status = VaapiJpegDecodeStatus::kSurfaceCreationFailed;
+      return nullptr;
+    }
+    va_surface_id_ = va_surfaces[0];
+    coded_size_ = new_coded_size;
+  }
 
   // Set picture parameters.
   VAPictureParameterBufferJPEGBaseline pic_param{};
   FillPictureParameters(parse_result.frame_header, &pic_param);
-  if (!vaapi_wrapper->SubmitBuffer(VAPictureParameterBufferType, &pic_param)) {
-    return false;
+  if (!vaapi_wrapper_->SubmitBuffer(VAPictureParameterBufferType, &pic_param)) {
+    VLOGF(1) << "Could not submit VAPictureParameterBufferType";
+    *status = VaapiJpegDecodeStatus::kSubmitPicParamsFailed;
+    return nullptr;
   }
 
   // Set quantization table.
   VAIQMatrixBufferJPEGBaseline iq_matrix{};
   FillIQMatrix(parse_result.q_table, &iq_matrix);
-  if (!vaapi_wrapper->SubmitBuffer(VAIQMatrixBufferType, &iq_matrix)) {
-    return false;
+  if (!vaapi_wrapper_->SubmitBuffer(VAIQMatrixBufferType, &iq_matrix)) {
+    VLOGF(1) << "Could not submit VAIQMatrixBufferType";
+    *status = VaapiJpegDecodeStatus::kSubmitIQMatrixFailed;
+    return nullptr;
   }
 
   // Set huffman table.
   VAHuffmanTableBufferJPEGBaseline huffman_table{};
   FillHuffmanTable(parse_result.dc_table, parse_result.ac_table,
                    &huffman_table);
-  if (!vaapi_wrapper->SubmitBuffer(VAHuffmanTableBufferType, &huffman_table)) {
-    return false;
+  if (!vaapi_wrapper_->SubmitBuffer(VAHuffmanTableBufferType, &huffman_table)) {
+    VLOGF(1) << "Could not submit VAHuffmanTableBufferType";
+    *status = VaapiJpegDecodeStatus::kSubmitHuffmanFailed;
+    return nullptr;
   }
 
   // Set slice parameters.
   VASliceParameterBufferJPEGBaseline slice_param{};
   FillSliceParameters(parse_result, &slice_param);
-  if (!vaapi_wrapper->SubmitBuffer(VASliceParameterBufferType, &slice_param)) {
-    return false;
+  if (!vaapi_wrapper_->SubmitBuffer(VASliceParameterBufferType, &slice_param)) {
+    VLOGF(1) << "Could not submit VASliceParameterBufferType";
+    *status = VaapiJpegDecodeStatus::kSubmitSliceParamsFailed;
+    return nullptr;
   }
 
   // Set scan data.
-  if (!vaapi_wrapper->SubmitBuffer(VASliceDataBufferType,
-                                   parse_result.data_size,
-                                   const_cast<char*>(parse_result.data))) {
-    return false;
+  if (!vaapi_wrapper_->SubmitBuffer(VASliceDataBufferType,
+                                    parse_result.data_size,
+                                    const_cast<char*>(parse_result.data))) {
+    VLOGF(1) << "Could not submit VASliceDataBufferType";
+    *status = VaapiJpegDecodeStatus::kSubmitSliceDataFailed;
+    return nullptr;
+  }
+
+  // Execute the decode.
+  if (!vaapi_wrapper_->ExecuteAndDestroyPendingBuffers(va_surface_id_)) {
+    VLOGF(1) << "Executing the decode failed";
+    *status = VaapiJpegDecodeStatus::kExecuteDecodeFailed;
+    return nullptr;
+  }
+
+  // Specify which image format we will request from the VAAPI. As the expected
+  // output format is I420, we will first try this format. If converting to I420
+  // is not supported by the decoder, we will request the image in its original
+  // chroma sampling format.
+  VAImageFormat va_image_format = kImageFormatI420;
+  if (!VaapiWrapper::IsImageFormatSupported(va_image_format) &&
+      !VaSurfaceFormatToImageFormat(va_rt_format_, &va_image_format)) {
+    VLOGF(1) << "Unsupported surface format";
+    *status = VaapiJpegDecodeStatus::kUnsupportedSurfaceFormat;
+    return nullptr;
+  }
+
+  auto scoped_image = vaapi_wrapper_->CreateVaImage(
+      va_surface_id_, &va_image_format, coded_size_);
+  if (!scoped_image) {
+    VLOGF(1) << "Cannot get VAImage";
+    *status = VaapiJpegDecodeStatus::kCannotGetImage;
+    return nullptr;
   }
 
-  return vaapi_wrapper->ExecuteAndDestroyPendingBuffers(va_surface);
+  DCHECK_EQ(va_image_format.fourcc, scoped_image->image()->format.fourcc);
+  *status = VaapiJpegDecodeStatus::kSuccess;
+  return scoped_image;
 }
 
 }  // namespace media

media/gpu/vaapi/vaapi_jpeg_decoder.h

@@ -7,39 +7,73 @@
 
 #include <stdint.h>
 
-// These data types are defined in va/va.h using typedef, reproduced here.
-// TODO(andrescj): revisit this once VaSurfaceFormatToImageFormat() and
-// VaSurfaceFormatForJpeg() are moved to the anonymous namespace in the .cc
-// file.
-using VAImageFormat = struct _VAImageFormat;
-using VASurfaceID = unsigned int;
+#include <memory>
+
+#include "base/callback_forward.h"
+#include "base/containers/span.h"
+#include "base/gtest_prod_util.h"
+#include "base/macros.h"
+#include "base/memory/scoped_refptr.h"
+#include "ui/gfx/geometry/size.h"
+
+// This data type is defined in va/va.h using typedef, reproduced here.
+typedef unsigned int VASurfaceID;
 
 namespace media {
 
-struct JpegFrameHeader;
-struct JpegParseResult;
+class ScopedVAImage;
 class VaapiWrapper;
 
-// Convert the specified surface format to the associated output image format.
-bool VaSurfaceFormatToImageFormat(uint32_t va_rt_format,
-                                  VAImageFormat* va_image_format);
-
-unsigned int VaSurfaceFormatForJpeg(const JpegFrameHeader& frame_header);
+enum class VaapiJpegDecodeStatus {
+  kSuccess,
+  kParseJpegFailed,
+  kUnsupportedJpeg,
+  kUnsupportedSubsampling,
+  kSurfaceCreationFailed,
+  kSubmitPicParamsFailed,
+  kSubmitIQMatrixFailed,
+  kSubmitHuffmanFailed,
+  kSubmitSliceParamsFailed,
+  kSubmitSliceDataFailed,
+  kExecuteDecodeFailed,
+  kUnsupportedSurfaceFormat,
+  kCannotGetImage,
+  kInvalidState,
+};
 
-class VaapiJpegDecoder {
+class VaapiJpegDecoder final {
  public:
-  VaapiJpegDecoder() = default;
-  virtual ~VaapiJpegDecoder() = default;
-
-  // Decodes a JPEG picture. It will fill VA-API parameters and call
-  // corresponding VA-API methods according to the JPEG |parse_result|. Decoded
-  // data will be outputted to the given |va_surface|. Returns false on failure.
-  // |vaapi_wrapper| should be initialized in kDecode mode with
-  // VAProfileJPEGBaseline profile. |va_surface| should be created with size at
-  // least as large as the picture size.
-  static bool DoDecode(VaapiWrapper* vaapi_wrapper,
-                       const JpegParseResult& parse_result,
-                       VASurfaceID va_surface);
+  VaapiJpegDecoder();
+  ~VaapiJpegDecoder();
+
+  // Initializes |vaapi_wrapper_| in kDecode mode with VAProfileJPEGBaseline
+  // profile and |error_uma_cb| for error reporting.
+  bool Initialize(const base::RepeatingClosure& error_uma_cb);
+
+  // Decodes a JPEG picture. It will fill VA-API parameters and call the
+  // corresponding VA-API methods according to the JPEG in |encoded_image|.
+  // Decoded data will be returned as a ScopedVAImage. Returns nullptr on
+  // failure and sets *|status| to the reason for failure.
+  std::unique_ptr<ScopedVAImage> DoDecode(
+      base::span<const uint8_t> encoded_image,
+      VaapiJpegDecodeStatus* status);
+
+ private:
+  // TODO(andrescj): move vaapi_utils tests out of vaapi_jpeg_decoder_unittest
+  // and remove this friend declaration.
+  friend class VaapiJpegDecoderTest;
+  FRIEND_TEST_ALL_PREFIXES(VaapiJpegDecoderTest, ScopedVAImage);
+
+  scoped_refptr<VaapiWrapper> vaapi_wrapper_;
+
+  // The current VA surface for decoding.
+  VASurfaceID va_surface_id_;
+  // The coded size associated with |va_surface_id_|.
+  gfx::Size coded_size_;
+  // The VA RT format associated with |va_surface_id_|.
+  unsigned int va_rt_format_;
+
+  DISALLOW_COPY_AND_ASSIGN(VaapiJpegDecoder);
 };
 
 }  // namespace media

media/test/data/README.md

@@ -818,6 +818,12 @@ ffmpeg -i red.webm -i a500hz.webm -map 0 -map 1 red-a500hz.webm
 Single MJEPG encoded frame of 1280x720, captured on Chromebook Pixel. This image
 does not have Huffman table.
 
+#### pixel-1280x720-grayscale.jpg
+A version of pixel-1280x720.jpg converted to grayscale using:
+```
+jpegtran -grayscale pixel-1280x720.jpg > pixel-1280x720-grayscale.jpg
+```
+
 #### peach_pi-1280x720.jpg
 Single MJPEG encoded frame of 1280x720, captured on Samsung Chromebook 2(13").
 This image has Huffman table.