Use media::Status in a few places here to get improved debug info

out of the vaapi accelerator. Also add some todos for adding media log.

Bug: 1103510

Change-Id: I23d790356ee29e40a152a398b6bbd3a760c9369d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2288472
Commit-Queue: Ted Meyer <tmathmeyer@chromium.org>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797553}

media/base/status_codes.h

@@ -101,6 +101,20 @@ enum class StatusCode : StatusCodeType {
   kEncoderInitializationError = 0x00000607,
   kEncoderFailedFlush = 0x00000608,
 
+  // VaapiVideoDecoder: 0x07
+  kVaapiBadContext = 0x00000701,
+  kVaapiNoBuffer = 0x00000702,
+  kVaapiNoBufferHandle = 0x00000703,
+  kVaapiNoPixmap = 0x00000704,
+  kVaapiNoImage = 0x00000705,
+  kVaapiNoSurface = 0x00000706,
+  kVaapiFailedToInitializeImage = 0x00000707,
+  kVaapiFailedToBindTexture = 0x00000708,
+  kVaapiFailedToBindImage = 0x00000709,
+  kVaapiUnsupportedFormat = 0x0000070A,
+  kVaapiFailedToExportImage = 0x0000070B,
+  kVaapiBadImageSize = 0x0000070C,
+
   // Special codes
   kGenericErrorPleaseRemove = 0x79999999,
   kCodeOnlyForTesting = std::numeric_limits<StatusCodeType>::max(),

media/gpu/vaapi/vaapi_picture.h

@@ -41,7 +41,7 @@ class MEDIA_GPU_EXPORT VaapiPicture {
 
   // Allocates a buffer of |format| to use as backing storage for this picture.
   // Return true on success.
-  virtual bool Allocate(gfx::BufferFormat format) = 0;
+  virtual Status Allocate(gfx::BufferFormat format) = 0;
 
   int32_t picture_buffer_id() const { return picture_buffer_id_; }
 

media/gpu/vaapi/vaapi_picture_native_pixmap_angle.cc

@@ -51,10 +51,10 @@ VaapiPictureNativePixmapAngle::~VaapiPictureNativePixmapAngle() {
   }
 }
 
-bool VaapiPictureNativePixmapAngle::Allocate(gfx::BufferFormat format) {
+Status VaapiPictureNativePixmapAngle::Allocate(gfx::BufferFormat format) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   NOTIMPLEMENTED();
-  return false;
+  return StatusCode::kGenericErrorPleaseRemove;
 }
 
 bool VaapiPictureNativePixmapAngle::ImportGpuMemoryBufferHandle(

media/gpu/vaapi/vaapi_picture_native_pixmap_angle.h

@@ -35,7 +35,7 @@ class VaapiPictureNativePixmapAngle : public VaapiPictureNativePixmap {
   ~VaapiPictureNativePixmapAngle() override;
 
   // VaapiPicture implementation.
-  bool Allocate(gfx::BufferFormat format) override;
+  Status Allocate(gfx::BufferFormat format) override;
   bool ImportGpuMemoryBufferHandle(
       gfx::BufferFormat format,
       gfx::GpuMemoryBufferHandle gpu_memory_buffer_handle) override;

media/gpu/vaapi/vaapi_picture_native_pixmap_egl.cc

@@ -48,7 +48,7 @@ VaapiPictureNativePixmapEgl::~VaapiPictureNativePixmapEgl() {
   }
 }
 
-bool VaapiPictureNativePixmapEgl::Initialize(
+Status VaapiPictureNativePixmapEgl::Initialize(
     scoped_refptr<gfx::NativePixmap> pixmap) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(pixmap);
@@ -58,24 +58,24 @@ bool VaapiPictureNativePixmapEgl::Initialize(
   va_surface_ = vaapi_wrapper_->CreateVASurfaceForPixmap(std::move(pixmap));
   if (!va_surface_) {
     LOG(ERROR) << "Failed creating VASurface for NativePixmap";
-    return false;
+    return StatusCode::kVaapiNoSurface;
   }
 
   if (bind_image_cb_ &&
       !bind_image_cb_.Run(client_texture_id_, texture_target_, gl_image_,
                           true /* can_bind_to_sampler */)) {
     LOG(ERROR) << "Failed to bind client_texture_id";
-    return false;
+    return StatusCode::kVaapiFailedToBindImage;
   }
-  return true;
+  return OkStatus();
 }
 
-bool VaapiPictureNativePixmapEgl::Allocate(gfx::BufferFormat format) {
+Status VaapiPictureNativePixmapEgl::Allocate(gfx::BufferFormat format) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   // Export the gl texture as dmabuf.
   if (make_context_current_cb_ && !make_context_current_cb_.Run())
-    return false;
+    return StatusCode::kVaapiBadContext;
 
   auto image =
       base::MakeRefCounted<gl::GLImageNativePixmap>(visible_size_, format);
@@ -83,14 +83,14 @@ bool VaapiPictureNativePixmapEgl::Allocate(gfx::BufferFormat format) {
   if (!image->InitializeFromTexture(texture_id_)) {
     DLOG(ERROR) << "Failed to initialize eglimage from texture id: "
                 << texture_id_;
-    return false;
+    return StatusCode::kVaapiFailedToInitializeImage;
   }
 
   // Export the EGLImage as dmabuf.
   gfx::NativePixmapHandle native_pixmap_handle = image->ExportHandle();
   if (!native_pixmap_handle.planes.size()) {
     DLOG(ERROR) << "Failed to export EGLImage as dmabuf fds";
-    return false;
+    return StatusCode::kVaapiFailedToExportImage;
   }
 
   if (size_.width() > static_cast<int>(native_pixmap_handle.planes[0].stride) ||
@@ -98,7 +98,7 @@ bool VaapiPictureNativePixmapEgl::Allocate(gfx::BufferFormat format) {
     DLOG(ERROR) << "EGLImage (stride=" << native_pixmap_handle.planes[0].stride
                 << ", size=" << native_pixmap_handle.planes[0].size
                 << "is smaller than size_=" << size_.ToString();
-    return false;
+    return StatusCode::kVaapiBadImageSize;
   }
 
   // Convert NativePixmapHandle to NativePixmapDmaBuf.
@@ -107,12 +107,12 @@ bool VaapiPictureNativePixmapEgl::Allocate(gfx::BufferFormat format) {
                                   std::move(native_pixmap_handle)));
   if (!native_pixmap_dmabuf->AreDmaBufFdsValid()) {
     DLOG(ERROR) << "Invalid dmabuf fds";
-    return false;
+    return StatusCode::kVaapiNoBufferHandle;
   }
 
   if (!image->BindTexImage(texture_target_)) {
     DLOG(ERROR) << "Failed to bind texture to GLImage";
-    return false;
+    return StatusCode::kVaapiFailedToBindImage;
   }
 
   // The |va_surface_| created from |native_pixmap_dmabuf| shares the ownership

media/gpu/vaapi/vaapi_picture_native_pixmap_egl.h

@@ -40,13 +40,13 @@ class VaapiPictureNativePixmapEgl : public VaapiPictureNativePixmap {
   ~VaapiPictureNativePixmapEgl() override;
 
   // VaapiPicture implementation.
-  bool Allocate(gfx::BufferFormat format) override;
+  Status Allocate(gfx::BufferFormat format) override;
   bool ImportGpuMemoryBufferHandle(
       gfx::BufferFormat format,
       gfx::GpuMemoryBufferHandle gpu_memory_buffer_handle) override;
 
  private:
-  bool Initialize(scoped_refptr<gfx::NativePixmap> pixmap);
+  Status Initialize(scoped_refptr<gfx::NativePixmap> pixmap);
 
   DISALLOW_COPY_AND_ASSIGN(VaapiPictureNativePixmapEgl);
 };

media/gpu/vaapi/vaapi_picture_native_pixmap_ozone.cc

@@ -49,7 +49,7 @@ VaapiPictureNativePixmapOzone::~VaapiPictureNativePixmapOzone() {
   }
 }
 
-bool VaapiPictureNativePixmapOzone::Initialize(
+Status VaapiPictureNativePixmapOzone::Initialize(
     scoped_refptr<gfx::NativePixmap> pixmap) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(pixmap);
@@ -58,16 +58,16 @@ bool VaapiPictureNativePixmapOzone::Initialize(
   va_surface_ = vaapi_wrapper_->CreateVASurfaceForPixmap(pixmap);
   if (!va_surface_) {
     LOG(ERROR) << "Failed creating VASurface for NativePixmap";
-    return false;
+    return StatusCode::kVaapiNoSurface;
   }
 
   // ARC++ has no texture ids.
   if (texture_id_ == 0 && client_texture_id_ == 0)
-    return true;
+    return OkStatus();
 
   // Import dmabuf fds into the output gl texture through EGLImage.
   if (make_context_current_cb_ && !make_context_current_cb_.Run())
-    return false;
+    return StatusCode::kVaapiBadContext;
 
   gl::ScopedTextureBinder texture_binder(texture_target_, texture_id_);
 
@@ -77,26 +77,26 @@ bool VaapiPictureNativePixmapOzone::Initialize(
       base::MakeRefCounted<gl::GLImageNativePixmap>(visible_size_, format);
   if (!image->Initialize(std::move(pixmap))) {
     LOG(ERROR) << "Failed to create GLImage";
-    return false;
+    return StatusCode::kVaapiFailedToInitializeImage;
   }
 
   gl_image_ = image;
   if (!gl_image_->BindTexImage(texture_target_)) {
     LOG(ERROR) << "Failed to bind texture to GLImage";
-    return false;
+    return StatusCode::kVaapiFailedToBindTexture;
   }
 
   if (bind_image_cb_ &&
       !bind_image_cb_.Run(client_texture_id_, texture_target_, gl_image_,
                           true /* can_bind_to_sampler */)) {
     LOG(ERROR) << "Failed to bind client_texture_id";
-    return false;
+    return StatusCode::kVaapiFailedToBindImage;
   }
 
-  return true;
+  return OkStatus();
 }
 
-bool VaapiPictureNativePixmapOzone::Allocate(gfx::BufferFormat format) {
+Status VaapiPictureNativePixmapOzone::Allocate(gfx::BufferFormat format) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   ui::OzonePlatform* platform = ui::OzonePlatform::GetInstance();
@@ -105,8 +105,7 @@ bool VaapiPictureNativePixmapOzone::Allocate(gfx::BufferFormat format) {
       gfx::kNullAcceleratedWidget, VK_NULL_HANDLE, size_, format,
       gfx::BufferUsage::SCANOUT_VDA_WRITE, /*framebuffer_size=*/visible_size_);
   if (!pixmap) {
-    LOG(ERROR) << "Failed allocating a pixmap";
-    return false;
+    return StatusCode::kVaapiNoPixmap;
   }
 
   return Initialize(std::move(pixmap));
@@ -138,7 +137,7 @@ bool VaapiPictureNativePixmapOzone::ImportGpuMemoryBufferHandle(
     return false;
   }
 
-  return Initialize(std::move(pixmap));
+  return Initialize(std::move(pixmap)).is_ok();
 }
 
 }  // namespace media

media/gpu/vaapi/vaapi_picture_native_pixmap_ozone.h

@@ -39,13 +39,13 @@ class VaapiPictureNativePixmapOzone : public VaapiPictureNativePixmap {
   ~VaapiPictureNativePixmapOzone() override;
 
   // VaapiPicture implementation.
-  bool Allocate(gfx::BufferFormat format) override;
+  Status Allocate(gfx::BufferFormat format) override;
   bool ImportGpuMemoryBufferHandle(
       gfx::BufferFormat format,
       gfx::GpuMemoryBufferHandle gpu_memory_buffer_handle) override;
 
  private:
-  bool Initialize(scoped_refptr<gfx::NativePixmap> pixmap);
+  Status Initialize(scoped_refptr<gfx::NativePixmap> pixmap);
 
   DISALLOW_COPY_AND_ASSIGN(VaapiPictureNativePixmapOzone);
 };

media/gpu/vaapi/vaapi_picture_tfp.cc

@@ -52,36 +52,36 @@ VaapiTFPPicture::~VaapiTFPPicture() {
     XFreePixmap(x_display_, x_pixmap_);
 }
 
-bool VaapiTFPPicture::Initialize() {
+Status VaapiTFPPicture::Initialize() {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   DCHECK(x_pixmap_);
 
   if (make_context_current_cb_ && !make_context_current_cb_.Run())
-    return false;
+    return StatusCode::kVaapiBadContext;
 
   glx_image_ = new gl::GLImageGLX(size_, gfx::BufferFormat::BGRX_8888);
   if (!glx_image_->Initialize(x_pixmap_)) {
     // x_pixmap_ will be freed in the destructor.
     DLOG(ERROR) << "Failed creating a GLX Pixmap for TFP";
-    return false;
+    return StatusCode::kVaapiNoPixmap;
   }
 
   gl::ScopedTextureBinder texture_binder(texture_target_, texture_id_);
   if (!glx_image_->BindTexImage(texture_target_)) {
     DLOG(ERROR) << "Failed to bind texture to glx image";
-    return false;
+    return StatusCode::kVaapiFailedToBindTexture;
   }
 
-  return true;
+  return OkStatus();
 }
 
-bool VaapiTFPPicture::Allocate(gfx::BufferFormat format) {
+Status VaapiTFPPicture::Allocate(gfx::BufferFormat format) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   if (format != gfx::BufferFormat::BGRX_8888 &&
       format != gfx::BufferFormat::BGRA_8888 &&
       format != gfx::BufferFormat::RGBX_8888) {
     DLOG(ERROR) << "Unsupported format";
-    return false;
+    return StatusCode::kVaapiUnsupportedFormat;
   }
 
   XWindowAttributes win_attr;
@@ -93,7 +93,7 @@ bool VaapiTFPPicture::Allocate(gfx::BufferFormat format) {
                             size_.width(), size_.height(), win_attr.depth);
   if (!x_pixmap_) {
     DLOG(ERROR) << "Failed creating an X Pixmap for TFP";
-    return false;
+    return StatusCode::kVaapiNoPixmap;
   }
 
   return Initialize();

media/gpu/vaapi/vaapi_picture_tfp.h

@@ -38,14 +38,14 @@ class VaapiTFPPicture : public VaapiPicture {
   ~VaapiTFPPicture() override;
 
   // VaapiPicture implementation.
-  bool Allocate(gfx::BufferFormat format) override;
+  Status Allocate(gfx::BufferFormat format) override;
   bool ImportGpuMemoryBufferHandle(
       gfx::BufferFormat format,
       gfx::GpuMemoryBufferHandle gpu_memory_buffer_handle) override;
   bool DownloadFromSurface(scoped_refptr<VASurface> va_surface) override;
 
  private:
-  bool Initialize();
+  Status Initialize();
 
   Display* x_display_;
 

media/gpu/vaapi/vaapi_video_decode_accelerator.cc

@@ -13,6 +13,7 @@
 #include "base/bind_helpers.h"
 #include "base/cpu.h"
 #include "base/files/scoped_file.h"
+#include "base/json/json_writer.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/metrics/histogram_macros.h"
@@ -27,6 +28,7 @@
 #include "gpu/ipc/service/gpu_channel.h"
 #include "media/base/bind_to_current_loop.h"
 #include "media/base/format_utils.h"
+#include "media/base/media_log.h"
 #include "media/base/unaligned_shared_memory.h"
 #include "media/base/video_util.h"
 #include "media/gpu/accelerated_video_decoder.h"
@@ -90,6 +92,14 @@ bool IsGeminiLakeOrLater() {
     }                                                              \
   } while (0)
 
+#define RETURN_AND_NOTIFY_ON_STATUS(status, ret) \
+  do {                                           \
+    if (!status.is_ok()) {                       \
+      NotifyStatus(status);                      \
+      return ret;                                \
+    }                                            \
+  } while (0)
+
 class VaapiVideoDecodeAccelerator::InputBuffer {
  public:
   InputBuffer() : buffer_(nullptr) {}
@@ -118,6 +128,17 @@ class VaapiVideoDecodeAccelerator::InputBuffer {
   DISALLOW_COPY_AND_ASSIGN(InputBuffer);
 };
 
+void VaapiVideoDecodeAccelerator::NotifyStatus(Status status) {
+  DCHECK(!status.is_ok());
+  // Send a platform notification error
+  NotifyError(PLATFORM_FAILURE);
+
+  // TODO(crbug.com/1103510) there is no MediaLog here, we should change that.
+  std::string output_str;
+  base::JSONWriter::Write(MediaSerialize(status), &output_str);
+  DLOG(ERROR) << output_str;
+}
+
 void VaapiVideoDecodeAccelerator::NotifyError(Error error) {
   if (!task_runner_->BelongsToCurrentThread()) {
     DCHECK(decoder_thread_task_runner_->BelongsToCurrentThread());
@@ -716,9 +737,8 @@ void VaapiVideoDecodeAccelerator::AssignPictureBuffers(
                                  PLATFORM_FAILURE, );
 
     if (output_mode_ == Config::OutputMode::ALLOCATE) {
-      RETURN_AND_NOTIFY_ON_FAILURE(
-          picture->Allocate(vaapi_picture_factory_->GetBufferFormat()),
-          "Failed to allocate memory for a VaapiPicture", PLATFORM_FAILURE, );
+      RETURN_AND_NOTIFY_ON_STATUS(
+          picture->Allocate(vaapi_picture_factory_->GetBufferFormat()), );
       available_picture_buffers_.push_back(buffers[i].id());
       VASurfaceID va_surface_id = picture->va_surface_id();
       if (va_surface_id != VA_INVALID_ID)

media/gpu/vaapi/vaapi_video_decode_accelerator.h

@@ -111,6 +111,7 @@ class MEDIA_GPU_EXPORT VaapiVideoDecodeAccelerator
 
   // Notify the client that an error has occurred and decoding cannot continue.
   void NotifyError(Error error);
+  void NotifyStatus(Status status);
 
   // Queue a input buffer for decode.
   void QueueInputBuffer(scoped_refptr<DecoderBuffer> buffer,

media/gpu/vaapi/vaapi_video_decode_accelerator_unittest.cc

@@ -105,7 +105,7 @@ class MockVaapiPicture : public VaapiPicture {
   ~MockVaapiPicture() override = default;
 
   // VaapiPicture implementation.
-  bool Allocate(gfx::BufferFormat format) override { return true; }
+  Status Allocate(gfx::BufferFormat format) override { return OkStatus(); }
   bool ImportGpuMemoryBufferHandle(
       gfx::BufferFormat format,
       gfx::GpuMemoryBufferHandle gpu_memory_buffer_handle) override {

media/gpu/vaapi/vaapi_video_decoder.cc

@@ -158,8 +158,9 @@ void VaapiVideoDecoder::Initialize(const VideoDecoderConfig& config,
 
   profile_ = profile;
   color_space_ = config.color_space_info();
-  if (!CreateAcceleratedVideoDecoder()) {
-    std::move(init_cb).Run(StatusCode::kVaapiFailedAcceleratorCreation);
+  auto accel_status = CreateAcceleratedVideoDecoder();
+  if (!accel_status.is_ok()) {
+    std::move(init_cb).Run(std::move(accel_status));
     return;
   }
 
@@ -537,7 +538,7 @@ void VaapiVideoDecoder::Reset(base::OnceClosure reset_cb) {
   if (state_ == State::kChangingResolution) {
     // If we reset during resolution change, re-create AVD. Then the new AVD
     // will trigger resolution change again after reset.
-    if (!CreateAcceleratedVideoDecoder()) {
+    if (!CreateAcceleratedVideoDecoder().is_ok()) {
       SetState(State::kError);
       std::move(reset_cb).Run();
       return;
@@ -558,7 +559,7 @@ void VaapiVideoDecoder::Reset(base::OnceClosure reset_cb) {
                                 std::move(reset_cb)));
 }
 
-bool VaapiVideoDecoder::CreateAcceleratedVideoDecoder() {
+Status VaapiVideoDecoder::CreateAcceleratedVideoDecoder() {
   DVLOGF(2);
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
@@ -583,10 +584,10 @@ bool VaapiVideoDecoder::CreateAcceleratedVideoDecoder() {
     decoder_.reset(
         new VP9Decoder(std::move(accelerator), profile_, color_space_));
   } else {
-    VLOGF(1) << "Unsupported profile " << GetProfileName(profile_);
-    return false;
+    return Status(StatusCode::kDecoderUnsupportedProfile)
+        .WithData("profile", profile_);
   }
-  return true;
+  return OkStatus();
 }
 
 void VaapiVideoDecoder::ResetDone(base::OnceClosure reset_cb) {

media/gpu/vaapi/vaapi_video_decoder.h

@@ -22,6 +22,7 @@
 #include "base/optional.h"
 #include "base/sequence_checker.h"
 #include "base/time/time.h"
+#include "media/base/status.h"
 #include "media/base/video_codecs.h"
 #include "media/base/video_frame_layout.h"
 #include "media/gpu/chromeos/video_decoder_pipeline.h"
@@ -120,7 +121,7 @@ class VaapiVideoDecoder : public DecoderInterface,
   void ResetDone(base::OnceClosure reset_cb);
 
   // Create codec-specific AcceleratedVideoDecoder and reset related variables.
-  bool CreateAcceleratedVideoDecoder();
+  Status CreateAcceleratedVideoDecoder();
 
   // Change the current |state_| to the specified |state|.
   void SetState(State state);