media/gpu/vaapiVEA: Manage VASurfaceID by ScopedID

VaapiVEA manages VA Surfaces with the ids. They need to be destroyed
by vaDestroySurface(). The ids are pooled for recycling. Some
ids has not returned to the pool, so that they are never
destroyed. This CL fixes the leakage by managing VASurfaceID
by ScopedID.

Bug: b:143957628
Test: video.EncodeAccel.*
Test: Open & Record & Close CCA multiple times
Change-Id: I86743fe80973fc3e850d543e38dcfe2764b2a710
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2060388
Commit-Queue: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Andres Calderon Jaramillo <andrescj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#745351}

media/gpu/vaapi/vaapi_video_encode_accelerator.cc

@@ -39,6 +39,7 @@
 #include "media/gpu/macros.h"
 #include "media/gpu/vaapi/h264_encoder.h"
 #include "media/gpu/vaapi/vaapi_common.h"
+#include "media/gpu/vaapi/vaapi_utils.h"
 #include "media/gpu/vaapi/vp8_encoder.h"
 #include "media/gpu/vaapi/vp9_encoder.h"
 #include "media/gpu/vp8_reference_frame_vector.h"
@@ -390,11 +391,6 @@ void VaapiVideoEncodeAccelerator::InitializeTask(const Config& config) {
 
   output_buffer_byte_size_ = encoder_->GetBitstreamBufferSize();
 
-  va_surface_release_cb_ = BindToCurrentLoop(base::BindRepeating(
-      &VaapiVideoEncodeAccelerator::RecycleVASurfaceID, encoder_weak_this_));
-  vpp_va_surface_release_cb_ = BindToCurrentLoop(base::BindRepeating(
-      &VaapiVideoEncodeAccelerator::RecycleVPPVASurfaceID, encoder_weak_this_));
-
   visible_rect_ = gfx::Rect(config.input_visible_size);
   expected_input_coded_size_ = VideoFrame::DetermineAlignedSize(
       config.input_format, config.input_visible_size);
@@ -429,14 +425,20 @@ void VaapiVideoEncodeAccelerator::InitializeTask(const Config& config) {
 
   // The surface size for the reconstructed surface (and input surface in non
   // native input mode) is the coded size.
+  std::vector<VASurfaceID> surface_ids;
   if (!vaapi_wrapper_->CreateContextAndSurfaces(
           kVaSurfaceFormat, encoder_->GetCodedSize(),
           VaapiWrapper::SurfaceUsageHint::kVideoEncoder,
           (num_frames_in_flight_ + 1) * va_surfaces_per_video_frame_,
-          &available_va_surface_ids_)) {
+          &surface_ids)) {
     NOTIFY_ERROR(kPlatformFailureError, "Failed creating VASurfaces");
     return;
   }
+  for (const VASurfaceID surface_id : surface_ids) {
+    available_va_surfaces_.emplace_back(std::make_unique<ScopedVASurfaceID>(
+        surface_id,
+        base::BindOnce(&VaapiWrapper::DestroySurface, vaapi_wrapper_)));
+  }
 
   child_task_runner_->PostTask(
       FROM_HERE,
@@ -461,20 +463,24 @@ void VaapiVideoEncodeAccelerator::InitializeTask(const Config& config) {
 }
 
 void VaapiVideoEncodeAccelerator::RecycleVASurfaceID(
-    VASurfaceID va_surface_id) {
-  DVLOGF(4) << "va_surface_id: " << va_surface_id;
+    std::unique_ptr<ScopedVASurfaceID> scoped_va_surface_id,
+    VASurfaceID /* va_surface_id */) {
+  DCHECK(scoped_va_surface_id);
+  DVLOGF(4) << "va_surface_id: " << scoped_va_surface_id->id();
   DCHECK_CALLED_ON_VALID_SEQUENCE(encoder_sequence_checker_);
 
-  available_va_surface_ids_.push_back(va_surface_id);
+  available_va_surfaces_.push_back(std::move(scoped_va_surface_id));
   EncodePendingInputs();
 }
 
 void VaapiVideoEncodeAccelerator::RecycleVPPVASurfaceID(
-    VASurfaceID va_surface_id) {
-  DVLOGF(4) << "va_surface_id: " << va_surface_id;
+    std::unique_ptr<ScopedVASurfaceID> scoped_va_surface_id,
+    VASurfaceID /* va_surface_id */) {
+  DCHECK(scoped_va_surface_id);
+  DVLOGF(4) << "va_surface_id: " << scoped_va_surface_id->id();
   DCHECK_CALLED_ON_VALID_SEQUENCE(encoder_sequence_checker_);
 
-  available_vpp_va_surface_ids_.push_back(va_surface_id);
+  available_vpp_va_surfaces_.push_back(std::move(scoped_va_surface_id));
   EncodePendingInputs();
 }
 
@@ -615,8 +621,8 @@ std::unique_ptr<VaapiEncodeJob> VaapiVideoEncodeAccelerator::CreateEncodeJob(
     return nullptr;
   }
 
-  if (available_va_surface_ids_.size() < va_surfaces_per_video_frame_ ||
-      (vpp_vaapi_wrapper_ && available_vpp_va_surface_ids_.empty())) {
+  if (available_va_surfaces_.size() < va_surfaces_per_video_frame_ ||
+      (vpp_vaapi_wrapper_ && available_vpp_va_surfaces_.empty())) {
     DVLOGF(4) << "Not enough surfaces available";
     return nullptr;
   }
@@ -663,10 +669,14 @@ std::unique_ptr<VaapiEncodeJob> VaapiVideoEncodeAccelerator::CreateEncodeJob(
                        << ", but got: " << frame->visible_rect().ToString());
       return nullptr;
     }
-    input_surface = new VASurface(available_va_surface_ids_.back(),
-                                  encoder_->GetCodedSize(), kVaSurfaceFormat,
-                                  base::BindOnce(va_surface_release_cb_));
-    available_va_surface_ids_.pop_back();
+
+    const VASurfaceID input_surface_id = available_va_surfaces_.back()->id();
+    input_surface = new VASurface(
+        input_surface_id, encoder_->GetCodedSize(), kVaSurfaceFormat,
+        base::BindOnce(&VaapiVideoEncodeAccelerator::RecycleVASurfaceID,
+                       encoder_weak_this_,
+                       std::move(available_va_surfaces_.back())));
+    available_va_surfaces_.pop_back();
   }
 
   if (visible_rect_ != frame->visible_rect()) {
@@ -684,20 +694,32 @@ std::unique_ptr<VaapiEncodeJob> VaapiVideoEncodeAccelerator::CreateEncodeJob(
       }
 
       // Allocate the same number of surfaces as reconstructed surfaces.
+      std::vector<VASurfaceID> surface_ids;
       if (!vpp_vaapi_wrapper_->CreateContextAndSurfaces(
               kVaSurfaceFormat, aligned_va_surface_size_,
               VaapiWrapper::SurfaceUsageHint::kVideoProcessWrite,
-              num_frames_in_flight_ + 1, &available_vpp_va_surface_ids_)) {
+              num_frames_in_flight_ + 1, &surface_ids)) {
         NOTIFY_ERROR(kPlatformFailureError,
                      "Failed creating VASurfaces for scaling");
         vpp_vaapi_wrapper_ = nullptr;
         return nullptr;
-      };
+      }
+      for (const VASurfaceID surface_id : surface_ids) {
+        available_vpp_va_surfaces_.push_back(
+            std::make_unique<ScopedVASurfaceID>(
+                surface_id, base::BindOnce(&VaapiWrapper::DestroySurface,
+                                           vpp_vaapi_wrapper_)));
+      }
     }
+
+    const VASurfaceID blit_surface_id = available_vpp_va_surfaces_.back()->id();
     scoped_refptr<VASurface> blit_surface = new VASurface(
-        available_vpp_va_surface_ids_.back(), aligned_va_surface_size_,
-        kVaSurfaceFormat, base::BindOnce(vpp_va_surface_release_cb_));
-    available_vpp_va_surface_ids_.pop_back();
+        blit_surface_id, aligned_va_surface_size_, kVaSurfaceFormat,
+        base::BindOnce(&VaapiVideoEncodeAccelerator::RecycleVPPVASurfaceID,
+                       encoder_weak_this_,
+                       std::move(available_vpp_va_surfaces_.back())));
+    available_vpp_va_surfaces_.pop_back();
+
     // Crop/Scale the visible area of |frame->visible_rect()| ->
     // |visible_rect_|.
     if (!vpp_vaapi_wrapper_->BlitSurface(*input_surface, *blit_surface,
@@ -720,10 +742,14 @@ std::unique_ptr<VaapiEncodeJob> VaapiVideoEncodeAccelerator::CreateEncodeJob(
   // Here, the surface size contained in |input_surface| is
   // |aligned_va_surface_size_| regardless of scaling in zero-copy mode, and
   // encoder_->GetCodedSize().
-  scoped_refptr<VASurface> reconstructed_surface =
-      new VASurface(available_va_surface_ids_.back(), encoder_->GetCodedSize(),
-                    kVaSurfaceFormat, base::BindOnce(va_surface_release_cb_));
-  available_va_surface_ids_.pop_back();
+  const VASurfaceID reconstructed_surface_id =
+      available_va_surfaces_.back()->id();
+  scoped_refptr<VASurface> reconstructed_surface = new VASurface(
+      reconstructed_surface_id, encoder_->GetCodedSize(), kVaSurfaceFormat,
+      base::BindOnce(&VaapiVideoEncodeAccelerator::RecycleVASurfaceID,
+                     encoder_weak_this_,
+                     std::move(available_va_surfaces_.back())));
+  available_va_surfaces_.pop_back();
 
   auto job = std::make_unique<VaapiEncodeJob>(
       frame, force_keyframe,
@@ -899,33 +925,34 @@ void VaapiVideoEncodeAccelerator::DestroyTask() {
   VLOGF(2);
   DCHECK_CALLED_ON_VALID_SEQUENCE(encoder_sequence_checker_);
 
-  encoder_weak_this_factory_.InvalidateWeakPtrs();
-
   if (flush_callback_) {
     child_task_runner_->PostTask(
         FROM_HERE, base::BindOnce(std::move(flush_callback_), false));
   }
 
   // Clean up members that are to be accessed on the encoder thread only.
-  if (vaapi_wrapper_)
-    vaapi_wrapper_->DestroyContextAndSurfaces(available_va_surface_ids_);
-  if (vpp_vaapi_wrapper_) {
-    vpp_vaapi_wrapper_->DestroyContextAndSurfaces(
-        available_vpp_va_surface_ids_);
-  }
+  available_bitstream_buffers_ = {};
+  input_queue_ = {};
 
-  available_va_buffer_ids_.clear();
+  // Release input and reconstructed surfaces held by
+  // |submitted_encode_jobs_|.
+  submitted_encode_jobs_ = {};
 
-  while (!available_bitstream_buffers_.empty())
-    available_bitstream_buffers_.pop();
+  // Release reconstructed surfaces held by |encoder_| as reference pictures.
+  encoder_ = nullptr;
 
-  while (!input_queue_.empty())
-    input_queue_.pop();
+  // Invalidate |encoder_weak_this_| so that the above clean up can let surfaces
+  // be back to |available_va_surfaces_ids_| and
+  // |available_vpp_va_surfaces_ids_|.
+  encoder_weak_this_factory_.InvalidateWeakPtrs();
 
-  while (!submitted_encode_jobs_.empty())
-    submitted_encode_jobs_.pop();
+  if (vaapi_wrapper_)
+    vaapi_wrapper_->DestroyContext();
+  if (vpp_vaapi_wrapper_)
+    vpp_vaapi_wrapper_->DestroyContext();
 
-  encoder_ = nullptr;
+  available_va_surfaces_.clear();
+  available_vpp_va_surfaces_.clear();
 
   delete this;
 }

media/gpu/vaapi/vaapi_video_encode_accelerator.h

@@ -25,6 +25,8 @@
 namespace media {
 
 class VaapiEncodeJob;
+template <typename T>
+class ScopedID;
 
 // A VideoEncodeAccelerator implementation that uses VA-API
 // (https://01.org/vaapi) for HW-accelerated video encode.
@@ -53,6 +55,9 @@ class MEDIA_GPU_EXPORT VaapiVideoEncodeAccelerator
   class VP8Accelerator;
   class VP9Accelerator;
 
+  // A self-cleaning VASurfaceID.
+  using ScopedVASurfaceID = ScopedID<VASurfaceID>;
+
   // Encoder state.
   enum State {
     kUninitialized,
@@ -113,12 +118,16 @@ class MEDIA_GPU_EXPORT VaapiVideoEncodeAccelerator
   void ExecuteEncode(VASurfaceID va_surface_id);
 
   // Callback that returns a no longer used VASurfaceID to
-  // |available_va_surface_ids_| for reuse.
-  void RecycleVASurfaceID(VASurfaceID va_surface_id);
+  // |available_va_surfaces_| for reuse.
+  void RecycleVASurfaceID(
+      std::unique_ptr<ScopedVASurfaceID> scoped_va_surface_id,
+      VASurfaceID va_surface_id);
 
   // Callback that returns a no longer used VASurfaceID to
-  // |available_vpp_va_surface_ids_| for reuse.
-  void RecycleVPPVASurfaceID(VASurfaceID va_surface_id);
+  // |available_vpp_va_surfaces_| for reuse.
+  void RecycleVPPVASurfaceID(
+      std::unique_ptr<ScopedVASurfaceID> scoped_va_surface_id,
+      VASurfaceID va_surface_id);
 
   // Returns a bitstream buffer to the client if both a previously executed job
   // awaits to be completed and we have bitstream buffers available to download
@@ -195,9 +204,9 @@ class MEDIA_GPU_EXPORT VaapiVideoEncodeAccelerator
   std::unique_ptr<AcceleratedVideoEncoder> encoder_;
 
   // VA surfaces available for encoding.
-  std::vector<VASurfaceID> available_va_surface_ids_;
+  std::vector<std::unique_ptr<ScopedVASurfaceID>> available_va_surfaces_;
   // VA surfaces available for scaling.
-  std::vector<VASurfaceID> available_vpp_va_surface_ids_;
+  std::vector<std::unique_ptr<ScopedVASurfaceID>> available_vpp_va_surfaces_;
 
   // VASurfaceIDs internal format.
   static constexpr unsigned int kVaSurfaceFormat = VA_RT_FORMAT_YUV420;
@@ -205,10 +214,6 @@ class MEDIA_GPU_EXPORT VaapiVideoEncodeAccelerator
   // VA buffers for coded frames.
   std::vector<VABufferID> available_va_buffer_ids_;
 
-  // Callback via which finished VA surfaces are returned to us.
-  base::RepeatingCallback<void(VASurfaceID)> va_surface_release_cb_;
-  base::RepeatingCallback<void(VASurfaceID)> vpp_va_surface_release_cb_;
-
   // Queue of input frames to be encoded.
   base::queue<std::unique_ptr<InputFrameRef>> input_queue_;