RELAND: media/gpu/vaapi: use new |va_buffer_for_vpp_| for Vpp/BlitSurface()

Original CL was reverted because it wrongly removed the line
  va_buffers_.erase(buffer_id);

Reland changes in
  crrev.com/c/2363833/1..2/media/gpu/vaapi/vaapi_wrapper.cc


Original CL description ------------------------------------------------

media/gpu/vaapi: use new |va_buffer_for_vpp_| for Vpp/BlitSurface()

ToT VaapiWrapper uses |va_buffers_| for internal Vpp uses and also for
external VEA/JEA use. This CL starts refactoring these VABufferIDs
lifetime, by splitting the Vpp VABufferID out of |va_buffers_| and
into a ScopedID |va_buffer_for_vpp_|.

Bug: b:162962069
Change-Id: Id377ebb44b6add14eb9227d1450c68a24eaee42b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2352522
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#799410}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2363833
Cr-Commit-Position: refs/heads/master@{#799891}

media/gpu/vaapi/vaapi_wrapper.cc

@@ -46,7 +46,6 @@
 // Auto-generated for dlopen libva libraries
 #include "media/gpu/vaapi/va_stubs.h"
 
-#include "media/gpu/vaapi/vaapi_utils.h"
 #include "third_party/libyuv/include/libyuv.h"
 #include "ui/gfx/buffer_format_util.h"
 #include "ui/gfx/buffer_types.h"
@@ -2114,18 +2113,20 @@ void VaapiWrapper::DestroyVABuffer(VABufferID buffer_id) {
   base::AutoLock auto_lock(*va_lock_);
   VAStatus va_res = vaDestroyBuffer(va_display_, buffer_id);
   VA_LOG_ON_ERROR(va_res, VaapiFunctions::kVADestroyBuffer);
-  const auto was_found = va_buffers_.erase(buffer_id);
-  DCHECK(was_found);
+  // If this |buffer_id| comes from CreateVABuffer() it'll be in |va_buffers_|,
+  // but otherwise (BlitSurface() - |va_buffer_for_vpp_|), it won't. This is
+  // needed temporarily while moving the VABufferID lifetime management to the
+  // clients, see crbug.com/1016219 and b/162962069.
+  va_buffers_.erase(buffer_id);
 }
 
 void VaapiWrapper::DestroyVABuffers() {
   base::AutoLock auto_lock(*va_lock_);
 
-  for (auto it = va_buffers_.begin(); it != va_buffers_.end(); ++it) {
-    VAStatus va_res = vaDestroyBuffer(va_display_, *it);
+  for (const VABufferID va_buffer_id : va_buffers_) {
+    VAStatus va_res = vaDestroyBuffer(va_display_, va_buffer_id);
     VA_LOG_ON_ERROR(va_res, VaapiFunctions::kVADestroyBuffer);
   }
-
   va_buffers_.clear();
 }
 
@@ -2152,22 +2153,23 @@ bool VaapiWrapper::BlitSurface(const VASurface& va_surface_src,
                                VideoRotation rotation) {
   base::AutoLock auto_lock(*va_lock_);
 
-  if (va_buffers_.empty()) {
+  // Create a buffer for VPP if it has not been created.
+  if (!va_buffer_for_vpp_) {
     DCHECK_NE(VA_INVALID_ID, va_context_id_);
-    // Create a buffer for VPP if it has not been created.
-    VABufferID buffer_id;
+    VABufferID buffer_id = VA_INVALID_ID;
     const VAStatus va_res = vaCreateBuffer(
         va_display_, va_context_id_, VAProcPipelineParameterBufferType,
         sizeof(VAProcPipelineParameterBuffer), 1, nullptr, &buffer_id);
     VA_SUCCESS_OR_RETURN(va_res, VaapiFunctions::kVACreateBuffer, false);
     DCHECK_NE(buffer_id, VA_INVALID_ID);
-    va_buffers_.emplace(buffer_id);
+
+    va_buffer_for_vpp_ = std::make_unique<ScopedID<VABufferID>>(
+        buffer_id, base::BindOnce(&VaapiWrapper::DestroyVABuffer, this));
   }
 
-  DCHECK_EQ(va_buffers_.size(), 1u);
-  VABufferID buffer_id = *va_buffers_.begin();
   {
-    ScopedVABufferMapping mapping(va_lock_, va_display_, buffer_id);
+    ScopedVABufferMapping mapping(va_lock_, va_display_,
+                                  va_buffer_for_vpp_->id());
     if (!mapping.IsValid())
       return false;
     auto* pipeline_param =
@@ -2221,8 +2223,9 @@ bool VaapiWrapper::BlitSurface(const VASurface& va_surface_src,
       vaBeginPicture(va_display_, va_context_id_, va_surface_dest.id()),
       VaapiFunctions::kVABeginPicture, false);
 
+  VABufferID va_buffer_id = va_buffer_for_vpp_->id();
   VA_SUCCESS_OR_RETURN(
-      vaRenderPicture(va_display_, va_context_id_, &buffer_id, 1),
+      vaRenderPicture(va_display_, va_context_id_, &va_buffer_id, 1),
       VaapiFunctions::kVARenderPicture_Vpp, false);
 
   VA_SUCCESS_OR_RETURN(vaEndPicture(va_display_, va_context_id_),

media/gpu/vaapi/vaapi_wrapper.h

@@ -28,6 +28,7 @@
 #include "base/thread_annotations.h"
 #include "media/gpu/media_gpu_export.h"
 #include "media/gpu/vaapi/va_surface.h"
+#include "media/gpu/vaapi/vaapi_utils.h"
 #include "media/video/video_decode_accelerator.h"
 #include "media/video/video_encode_accelerator.h"
 #include "ui/gfx/geometry/size.h"
@@ -46,8 +47,6 @@ class Rect;
 namespace media {
 constexpr unsigned int kInvalidVaRtFormat = 0u;
 
-class ScopedVAImage;
-class ScopedVASurface;
 class VideoFrame;
 
 // Enum, function and callback type to allow VaapiWrapper to log errors in VA
@@ -467,9 +466,13 @@ class MEDIA_GPU_EXPORT VaapiWrapper
   // Data queued up for HW codec, to be committed on next execution.
   std::vector<VABufferID> pending_va_buffers_;
 
-  // Buffers for kEncode or kVideoProcess.
+  // VABufferIDs for kEncode*.
   std::set<VABufferID> va_buffers_;
 
+  // VABufferID to be used for kVideoProcess. Allocated the first time around,
+  // and reused afterwards.
+  std::unique_ptr<ScopedID<VABufferID>> va_buffer_for_vpp_;
+
   // Called to report codec errors to UMA. Errors to clients are reported via
   // return values from public methods.
   ReportErrorToUMACB report_error_to_uma_cb_;