media/gpu/vaapiVEA: Remove redundant VPP VaapiWrappers

VaapiVideoEncodeAccelerator creates a VaapiWrapper for each
spatial layer. However, VA context for VPP is not associated
with a specific resolution and any BlitSurface process can be
executed with one VaapiWrapper. This CL changes VaapiVEA to
create one VaapiWrapper for VPP and removes unnecessary ones.

Bug: 1186051
Test: video_encode_accelerator_tests --codec=vp9
Test: video_encode_accelerator_unittest
Change-Id: Ia525d93078ce26c0893aa2257132b62d2cf72044
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3011307
Commit-Queue: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Zhaoliang Ma <zhaoliang.ma@intel.com>
Cr-Commit-Position: refs/heads/master@{#900885}

media/gpu/vaapi/vaapi_video_encode_accelerator.cc

@@ -559,35 +559,35 @@ VaapiVideoEncodeAccelerator::BlitSurfaceWithCreateVppIfNeeded(
     size_t num_va_surfaces) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(encoder_sequence_checker_);
 
-  if (available_vpp_va_surface_ids_.find(encode_size) ==
+  if (!vpp_vaapi_wrapper_) {
-      available_vpp_va_surface_ids_.end()) {
+    vpp_vaapi_wrapper_ = VaapiWrapper::Create(
-    // |vpp_vaapi_wrapper_| is filled in advance in unit test for multi spatial
+        VaapiWrapper::kVideoProcess, VAProfileNone,
-    // layer encoding.
+        EncryptionScheme::kUnencrypted,
-    if (!IsConfiguredForTesting()) {
+        base::BindRepeating(
-      DCHECK(!base::Contains(vpp_vaapi_wrapper_, encode_size));
+            &ReportVaapiErrorToUMA,
-      auto vpp_va_wrapper = VaapiWrapper::Create(
+            "Media.VaapiVideoEncodeAccelerator.Vpp.VAAPIError"));
-          VaapiWrapper::kVideoProcess, VAProfileNone,
+    if (!vpp_vaapi_wrapper_) {
-          EncryptionScheme::kUnencrypted,
+      NOTIFY_ERROR(kPlatformFailureError,
-          base::BindRepeating(
+                   "Failed to initialize VppVaapiWrapper");
-              &ReportVaapiErrorToUMA,
+      return nullptr;
-              "Media.VaapiVideoEncodeAccelerator.Vpp.VAAPIError"));
-      if (!vpp_va_wrapper) {
-        NOTIFY_ERROR(kPlatformFailureError,
-                     "Failed to initialize VppVaapiWrapper");
-        return nullptr;
-      }
-
-      vpp_vaapi_wrapper_[encode_size] = std::move(vpp_va_wrapper);
     }
 
-    if (!vpp_vaapi_wrapper_[encode_size]->CreateContextAndSurfaces(
+    // VA context for VPP is not associated with a specific resolution.
+    if (!vpp_vaapi_wrapper_->CreateContext(gfx::Size())) {
+      NOTIFY_ERROR(kPlatformFailureError, "Failed creating Context for VPP");
+      return nullptr;
+    }
+  }
+
+  if (!base::Contains(available_vpp_va_surface_ids_, encode_size)) {
+    if (!vpp_vaapi_wrapper_->CreateSurfaces(
             kVaSurfaceFormat, encode_size,
             {VaapiWrapper::SurfaceUsageHint::kVideoProcessWrite,
              VaapiWrapper::SurfaceUsageHint::kVideoEncoder},
             num_va_surfaces, &available_vpp_va_surface_ids_[encode_size])) {
       NOTIFY_ERROR(kPlatformFailureError, "Failed creating VASurfaces");
       return nullptr;
-    };
+    }
 
     vpp_va_surface_release_cb_[encode_size] =
         BindToCurrentLoop(base::BindRepeating(
@@ -595,15 +595,16 @@ VaapiVideoEncodeAccelerator::BlitSurfaceWithCreateVppIfNeeded(
             encoder_weak_this_, &available_vpp_va_surface_ids_[encode_size]));
   }
 
+  DCHECK(!available_vpp_va_surface_ids_[encode_size].empty());
   // Scaling the input surface to dest size for K-SVC or simulcast.
   scoped_refptr<VASurface> blit_surface =
       new VASurface(available_vpp_va_surface_ids_[encode_size].back(),
                     encode_size, kVaSurfaceFormat,
                     base::BindOnce(vpp_va_surface_release_cb_[encode_size]));
   available_vpp_va_surface_ids_[encode_size].pop_back();
-  if (!vpp_vaapi_wrapper_[encode_size]->BlitSurface(
+  if (!vpp_vaapi_wrapper_->BlitSurface(input_surface, *blit_surface,
-          input_surface, *blit_surface, input_visible_rect,
+                                       input_visible_rect,
-          gfx::Rect(encode_size))) {
+                                       gfx::Rect(encode_size))) {
     NOTIFY_ERROR(kPlatformFailureError,
                  "Failed BlitSurface on frame size: "
                      << input_surface.size().ToString()
@@ -611,6 +612,7 @@ VaapiVideoEncodeAccelerator::BlitSurfaceWithCreateVppIfNeeded(
                      << ") -> encode size: " << encode_size.ToString());
     return nullptr;
   }
+
   return blit_surface;
 }
 
@@ -945,14 +947,16 @@ void VaapiVideoEncodeAccelerator::DestroyTask() {
   // Clean up members that are to be accessed on the encoder thread only.
   if (vaapi_wrapper_)
     vaapi_wrapper_->DestroyContextAndSurfaces(available_va_surface_ids_);
-  for (auto& vpp : vpp_vaapi_wrapper_) {
+  available_va_surface_ids_.clear();
-    DCHECK(base::Contains(available_vpp_va_surface_ids_, vpp.first));
-    vpp.second->DestroyContextAndSurfaces(
-        available_vpp_va_surface_ids_[vpp.first]);
-  }
-
   available_va_buffer_ids_.clear();
 
+  if (vpp_vaapi_wrapper_) {
+    vpp_vaapi_wrapper_->DestroyContext();
+    for (const auto& surfaces : available_vpp_va_surface_ids_)
+      vpp_vaapi_wrapper_->DestroySurfaces(surfaces.second);
+    available_vpp_va_surface_ids_.clear();
+  }
+
   while (!available_bitstream_buffers_.empty())
     available_bitstream_buffers_.pop();
 

media/gpu/vaapi/vaapi_video_encode_accelerator.h

@@ -203,9 +203,11 @@ class MEDIA_GPU_EXPORT VaapiVideoEncodeAccelerator
   // Should only be used on |encoder_task_runner_|.
   std::unique_ptr<VaapiVideoEncoderDelegate> encoder_;
 
+  // TODO(crbug.com/1186051): Store ScopedVASurface, not VASurfaceID.
   // VA surfaces available for encoding.
   std::vector<VASurfaceID> available_va_surface_ids_;
   // VA surfaces available for scaling.
+  // TODO(crbug.com/1186051): Use base::small_map.
   std::map<gfx::Size, std::vector<VASurfaceID>, SizeComparator>
       available_vpp_va_surface_ids_;
 
@@ -217,6 +219,7 @@ class MEDIA_GPU_EXPORT VaapiVideoEncodeAccelerator
 
   // Callback via which finished VA surfaces are returned to us.
   base::RepeatingCallback<void(VASurfaceID)> va_surface_release_cb_;
+  // TODO(crbug.com/1186051): Use base::small_map.
   std::map<gfx::Size, base::RepeatingCallback<void(VASurfaceID)>,
       SizeComparator> vpp_va_surface_release_cb_;
 
@@ -246,8 +249,7 @@ class MEDIA_GPU_EXPORT VaapiVideoEncodeAccelerator
 
   // VaapiWrapper for VPP (Video Pre Processing). This is used for scale down
   // for the picture send to vaapi encoder.
-  std::map<gfx::Size, scoped_refptr<VaapiWrapper>, SizeComparator>
+  scoped_refptr<VaapiWrapper> vpp_vaapi_wrapper_;
-      vpp_vaapi_wrapper_;
 
   // The completion callback of the Flush() function.
   FlushCallback flush_callback_;

media/gpu/vaapi/vaapi_video_encode_accelerator_unittest.cc

@@ -173,6 +173,12 @@ class MockVaapiWrapper : public VaapiWrapper {
                bool(const VideoFrame&, VASurfaceID, const gfx::Size&));
   MOCK_METHOD1(ExecuteAndDestroyPendingBuffers, bool(VASurfaceID));
   MOCK_METHOD0(DestroyContext, void());
+  MOCK_METHOD5(CreateSurfaces,
+               bool(unsigned int,
+                    const gfx::Size&,
+                    const std::vector<SurfaceUsageHint>&,
+                    size_t,
+                    std::vector<VASurfaceID>*));
   MOCK_METHOD1(DestroySurfaces, void(std::vector<VASurfaceID> va_surface_ids));
 
  private:
@@ -281,19 +287,12 @@ class VaapiVideoEncodeAcceleratorTest
     // Scaling is needed only for non highest spatial layer, so here the vpp
     // number is |num_spatial_layers - 1|.
     vpp_svc_va_surfaces_.resize(num_spatial_layers - 1);
-    vpp_svc_mock_vaapi_wrappers_.resize(num_spatial_layers - 1);
+    vpp_svc_mock_vaapi_wrapper_ =
+        base::MakeRefCounted<MockVaapiWrapper>(VaapiWrapper::kVideoProcess);
     auto* vaapi_encoder =
         reinterpret_cast<VaapiVideoEncodeAccelerator*>(encoder_.get());
-    for (size_t i = 0; i < num_spatial_layers - 1; ++i) {
+    vaapi_encoder->vpp_vaapi_wrapper_ = vpp_svc_mock_vaapi_wrapper_;
-      vpp_svc_mock_vaapi_wrappers_[i] =
+
-          base::MakeRefCounted<MockVaapiWrapper>(VaapiWrapper::kVideoProcess);
-      const int denom =
-          kSpatialLayersResolutionDenom[num_spatial_layers - 1][i];
-      gfx::Size layer_size = gfx::Size(kDefaultEncodeSize.width() / denom,
-                                       kDefaultEncodeSize.height() / denom);
-      vaapi_encoder->vpp_vaapi_wrapper_[layer_size] =
-          vpp_svc_mock_vaapi_wrappers_[i];
-    }
     EXPECT_CALL(*mock_encoder_,
                 Initialize(_, MatchesVaapiVideoEncoderDelegateConfig(
                                   kMaxNumOfRefFrames, kBitrateControl)))
@@ -472,8 +471,8 @@ class VaapiVideoEncodeAcceleratorTest
       if (i < num_spatial_layers - 1) {
         if (vpp_svc_va_surfaces_[i].empty()) {
           EXPECT_CALL(
-              *vpp_svc_mock_vaapi_wrappers_[i],
+              *vpp_svc_mock_vaapi_wrapper_,
-              CreateContextAndSurfaces(
+              CreateSurfaces(
                   VA_RT_FORMAT_YUV420, layer_size,
                   std::vector<VaapiWrapper::SurfaceUsageHint>{
                       VaapiWrapper::SurfaceUsageHint::kVideoProcessWrite,
@@ -492,7 +491,7 @@ class VaapiVideoEncodeAcceleratorTest
 
         absl::optional<gfx::Rect> default_rect = gfx::Rect(kDefaultEncodeSize);
         absl::optional<gfx::Rect> layer_rect = gfx::Rect(layer_size);
-        EXPECT_CALL(*vpp_svc_mock_vaapi_wrappers_[i],
+        EXPECT_CALL(*vpp_svc_mock_vaapi_wrapper_,
                     BlitSurface(_, _, default_rect, layer_rect,
                                 VideoRotation::VIDEO_ROTATION_0))
             .WillOnce(Return(true));
@@ -574,7 +573,7 @@ class VaapiVideoEncodeAcceleratorTest
   // calls Destroy() so that destruction threading is respected.
   std::unique_ptr<VideoEncodeAccelerator> encoder_;
   scoped_refptr<MockVaapiWrapper> mock_vaapi_wrapper_;
-  std::vector<scoped_refptr<MockVaapiWrapper>> vpp_svc_mock_vaapi_wrappers_;
+  scoped_refptr<MockVaapiWrapper> vpp_svc_mock_vaapi_wrapper_;
   MockVP9VaapiVideoEncoderDelegate* mock_encoder_ = nullptr;
 };
 

media/gpu/vaapi/vaapi_wrapper.h

@@ -476,6 +476,17 @@ class MEDIA_GPU_EXPORT VaapiWrapper
   // Initialize static data before sandbox is enabled.
   static void PreSandboxInitialization();
 
+  // TODO(crbug.com/1186051): Back to private in favor of using
+  // CreateScopedVASurface().
+  // Tries to allocate |num_surfaces| VASurfaceIDs of |size| and |va_format|.
+  // Fills |va_surfaces| and returns true if successful, or returns false.
+  virtual bool CreateSurfaces(unsigned int va_format,
+                              const gfx::Size& size,
+                              const std::vector<SurfaceUsageHint>& usage_hints,
+                              size_t num_surfaces,
+                              std::vector<VASurfaceID>* va_surfaces)
+      WARN_UNUSED_RESULT;
+
   // vaDestroySurfaces() a vector or a single VASurfaceID.
   virtual void DestroySurfaces(std::vector<VASurfaceID> va_surfaces);
   virtual void DestroySurface(VASurfaceID va_surface_id);
@@ -499,14 +510,6 @@ class MEDIA_GPU_EXPORT VaapiWrapper
   bool VaInitialize(const ReportErrorToUMACB& report_error_to_uma_cb)
       WARN_UNUSED_RESULT;
 
-  // Tries to allocate |num_surfaces| VASurfaceIDs of |size| and |va_format|.
-  // Fills |va_surfaces| and returns true if successful, or returns false.
-  bool CreateSurfaces(unsigned int va_format,
-                      const gfx::Size& size,
-                      const std::vector<SurfaceUsageHint>& usage_hints,
-                      size_t num_surfaces,
-                      std::vector<VASurfaceID>* va_surfaces) WARN_UNUSED_RESULT;
-
   // Carries out the vaBeginPicture()-vaRenderPicture()-vaEndPicture() on target
   // |va_surface_id|. Returns false if any of these calls fails.
   bool Execute_Locked(VASurfaceID va_surface_id,