media/gpu: remove VaapiWrapper::va_surface_format_

This CL is a follow-up to crrev.com/c/1648812, where the lifetime mgmt
of the Vaapi-allocate VASurfaceIDs was moved to the clients. This CL
removes the VaapiWrapper::va_surface_format_, and just keeps the
value on the clients, which anyway have to compute it (and in the case
of the encoder, it's hardcoded).

Test: All unittests and CrOs video playback correct.
Bug: 971891
Change-Id: I81800cec5e62cc2c26fe244e86fb8196a4e072dc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1647118
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Andres Calderon Jaramillo <andrescj@chromium.org>
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#667996}
diff --git a/media/gpu/vaapi/vaapi_video_decode_accelerator.cc b/media/gpu/vaapi/vaapi_video_decode_accelerator.cc
index 5a4fdbf..1149d3c8 100644
--- a/media/gpu/vaapi/vaapi_video_decode_accelerator.cc
+++ b/media/gpu/vaapi/vaapi_video_decode_accelerator.cc
@@ -145,6 +145,7 @@
       vaapi_picture_factory_(new VaapiPictureFactory()),
       buffer_allocation_mode_(BufferAllocationMode::kNormal),
       surfaces_available_(&lock_),
+      va_surface_format_(VA_INVALID_ID),
       task_runner_(base::ThreadTaskRunnerHandle::Get()),
       decoder_thread_("VaapiDecoderThread"),
       finish_flush_pending_(false),
@@ -631,7 +632,7 @@
       << ", requested " << requested_num_pics_ << ")", INVALID_ARGUMENT, );
   DCHECK(requested_pic_size_ == buffers[0].size());
 
-  const unsigned int va_format = GetVaFormatForVideoCodecProfile(profile_);
+  va_surface_format_ = GetVaFormatForVideoCodecProfile(profile_);
   std::vector<VASurfaceID> va_surface_ids;
 
   // If we aren't in BufferAllocationMode::kNone, we have to allocate a
@@ -684,7 +685,7 @@
   if (buffer_allocation_mode_ == BufferAllocationMode::kNone) {
     DCHECK(!va_surface_ids.empty());
     RETURN_AND_NOTIFY_ON_FAILURE(
-        vaapi_wrapper_->CreateContext(va_format, requested_pic_size_),
+        vaapi_wrapper_->CreateContext(va_surface_format_, requested_pic_size_),
         "Failed creating VA Context", PLATFORM_FAILURE, );
     DCHECK_EQ(va_surface_ids.size(), buffers.size());
   } else {
@@ -695,7 +696,7 @@
     CHECK_NE(requested_num_surfaces, 0u);
     va_surface_ids.clear();
     RETURN_AND_NOTIFY_ON_FAILURE(vaapi_wrapper_->CreateContextAndSurfaces(
-                                     va_format, requested_pic_size_,
+                                     va_surface_format_, requested_pic_size_,
                                      requested_num_surfaces, &va_surface_ids),
                                  "Failed creating VA Surfaces",
                                  PLATFORM_FAILURE, );
@@ -1011,6 +1012,7 @@
   if (available_va_surfaces_.empty())
     return nullptr;
 
+  DCHECK_NE(VA_INVALID_ID, va_surface_format_);
   DCHECK(!awaiting_va_surfaces_recycle_);
   if (buffer_allocation_mode_ != BufferAllocationMode::kNone) {
     const VASurfaceID id = available_va_surfaces_.front();
@@ -1023,8 +1025,7 @@
                           available_va_surfaces_.size(),
                       "available", available_va_surfaces_.size());
 
-    return new VASurface(id, requested_pic_size_,
-                         vaapi_wrapper_->va_surface_format(),
+    return new VASurface(id, requested_pic_size_, va_surface_format_,
                          base::BindOnce(va_surface_release_cb_));
   }
 
@@ -1039,7 +1040,7 @@
         // to return a new VASurface.
         base::Erase(available_va_surfaces_, va_surface_id);
         return new VASurface(va_surface_id, requested_pic_size_,
-                             vaapi_wrapper_->va_surface_format(),
+                             va_surface_format_,
                              base::BindOnce(va_surface_release_cb_));
       }
     }
@@ -1132,12 +1133,11 @@
 
   constexpr float kNumBytesPerPixelYUV420 = 12.0 / 8;
   constexpr float kNumBytesPerPixelYUV420_10bpp = 2 * kNumBytesPerPixelYUV420;
-  unsigned int va_surface_format = GetVaFormatForVideoCodecProfile(profile_);
-  DCHECK(va_surface_format == VA_RT_FORMAT_YUV420 ||
-         va_surface_format == VA_RT_FORMAT_YUV420_10BPP);
+  DCHECK(va_surface_format_ == VA_RT_FORMAT_YUV420 ||
+         va_surface_format_ == VA_RT_FORMAT_YUV420_10BPP);
   const float va_surface_bytes_per_pixel =
-      va_surface_format == VA_RT_FORMAT_YUV420 ? kNumBytesPerPixelYUV420
-                                               : kNumBytesPerPixelYUV420_10bpp;
+      va_surface_format_ == VA_RT_FORMAT_YUV420 ? kNumBytesPerPixelYUV420
+                                                : kNumBytesPerPixelYUV420_10bpp;
   // Report |requested_num_surfaces| and the associated memory size. The
   // calculated size is an estimation since we don't know the internal VA
   // strides, texture compression, headers, etc, but is a good lower boundary.
diff --git a/media/gpu/vaapi/vaapi_video_decode_accelerator.h b/media/gpu/vaapi/vaapi_video_decode_accelerator.h
index 2d7fb0b0..3ba6e1ef 100644
--- a/media/gpu/vaapi/vaapi_video_decode_accelerator.h
+++ b/media/gpu/vaapi/vaapi_video_decode_accelerator.h
@@ -269,6 +269,8 @@
   std::list<VASurfaceID> available_va_surfaces_ GUARDED_BY(lock_);
   // Signalled when output surfaces are queued into |available_va_surfaces_|.
   base::ConditionVariable surfaces_available_;
+  // VASurfaceIDs format, filled in when created.
+  unsigned int va_surface_format_;
 
   // Pending output requests from the decoder. When it indicates that we should
   // output a surface and we have an available Picture (i.e. texture) ready
diff --git a/media/gpu/vaapi/vaapi_video_encode_accelerator.cc b/media/gpu/vaapi/vaapi_video_encode_accelerator.cc
index 48a703d6..0fca64e 100644
--- a/media/gpu/vaapi/vaapi_video_encode_accelerator.cc
+++ b/media/gpu/vaapi/vaapi_video_encode_accelerator.cc
@@ -384,7 +384,7 @@
       (native_input_mode_ ? 0 : kNumSurfacesPerInputVideoFrame);
 
   if (!vaapi_wrapper_->CreateContextAndSurfaces(
-          VA_RT_FORMAT_YUV420, coded_size_,
+          kVaSurfaceFormat, coded_size_,
           (num_frames_in_flight + 1) * va_surfaces_per_video_frame_,
           &available_va_surface_ids_)) {
     NOTIFY_ERROR(kPlatformFailureError, "Failed creating VASurfaces");
@@ -581,14 +581,13 @@
   }
 
   scoped_refptr<VASurface> input_surface = new VASurface(
-      va_input_surface_id, coded_size_, vaapi_wrapper_->va_surface_format(),
+      va_input_surface_id, coded_size_, kVaSurfaceFormat,
       native_input_mode_ ? base::DoNothing()
                          : base::BindOnce(va_surface_release_cb_));
 
   scoped_refptr<VASurface> reconstructed_surface =
       new VASurface(available_va_surface_ids_.back(), coded_size_,
-                    vaapi_wrapper_->va_surface_format(),
-                    base::BindOnce(va_surface_release_cb_));
+                    kVaSurfaceFormat, base::BindOnce(va_surface_release_cb_));
   available_va_surface_ids_.pop_back();
 
   auto job = base::MakeRefCounted<VaapiEncodeJob>(
diff --git a/media/gpu/vaapi/vaapi_video_encode_accelerator.h b/media/gpu/vaapi/vaapi_video_encode_accelerator.h
index 9aef0e0..8ff9ba8 100644
--- a/media/gpu/vaapi/vaapi_video_encode_accelerator.h
+++ b/media/gpu/vaapi/vaapi_video_encode_accelerator.h
@@ -181,6 +181,9 @@
   // VA surfaces available for reuse.
   std::vector<VASurfaceID> available_va_surface_ids_;
 
+  // VASurfaceIDs internal format.
+  static constexpr unsigned int kVaSurfaceFormat = VA_RT_FORMAT_YUV420;
+
   // VA buffers for coded frames.
   std::vector<VABufferID> available_va_buffer_ids_;
 
diff --git a/media/gpu/vaapi/vaapi_wrapper.cc b/media/gpu/vaapi/vaapi_wrapper.cc
index e1d29a9..e9cac02 100644
--- a/media/gpu/vaapi/vaapi_wrapper.cc
+++ b/media/gpu/vaapi/vaapi_wrapper.cc
@@ -1216,8 +1216,9 @@
   DVLOG(2) << "Creating " << num_surfaces << " surfaces";
   DCHECK(va_surfaces->empty());
 
-  if (va_surface_format_ != 0u) {
-    LOG(ERROR) << "Surfaces should be destroyed before creating new surfaces";
+  if (va_context_id_ != VA_INVALID_ID) {
+    LOG(ERROR)
+        << "The current context should be destroyed before creating a new one";
     return false;
   }
 
@@ -1250,10 +1251,7 @@
       vaCreateContext(va_display_, va_config_id_, size.width(), size.height(),
                       VA_PROGRESSIVE, empty_va_surfaces_ids_pointer,
                       empty_va_surfaces_ids_size, &va_context_id_);
-
   VA_LOG_ON_ERROR(va_res, "vaCreateContext failed");
-  if (va_res == VA_STATUS_SUCCESS)
-    va_surface_format_ = va_format;
   return va_res == VA_STATUS_SUCCESS;
 }
 
@@ -1711,7 +1709,6 @@
 
 VaapiWrapper::VaapiWrapper()
     : va_lock_(VADisplayState::Get()->va_lock()),
-      va_surface_format_(0),
       va_display_(NULL),
       va_config_id_(VA_INVALID_ID),
       va_context_id_(VA_INVALID_ID) {}
@@ -1808,7 +1805,6 @@
   }
 
   va_context_id_ = VA_INVALID_ID;
-  va_surface_format_ = 0;
 }
 
 bool VaapiWrapper::CreateSurfaces(unsigned int va_format,
diff --git a/media/gpu/vaapi/vaapi_wrapper.h b/media/gpu/vaapi/vaapi_wrapper.h
index 0f27d76..0c03759c 100644
--- a/media/gpu/vaapi/vaapi_wrapper.h
+++ b/media/gpu/vaapi/vaapi_wrapper.h
@@ -158,8 +158,7 @@
   // The client is responsible for releasing it via DestroyContext() or
   // DestroyContextAndSurfaces(), or it will be released on dtor.
   virtual bool CreateContext(unsigned int va_format, const gfx::Size& size);
-  // Destroys the context identified by |va_context_id_| and clears the local
-  // associated |va_surface_format_|.
+  // Destroys the context identified by |va_context_id_|.
   void DestroyContext();
 
   // Creates a self-releasing VASurface from |pixmap|. The ownership of the
@@ -266,9 +265,6 @@
   // Initialize static data before sandbox is enabled.
   static void PreSandboxInitialization();
 
-  // Get the created surfaces format. TODO(crbug.com/971891): remove.
-  unsigned int va_surface_format() const { return va_surface_format_; }
-
  protected:
   VaapiWrapper();
   virtual ~VaapiWrapper();
@@ -309,9 +305,6 @@
   // the lifetime of VaapiWrapper.
   base::Lock* va_lock_;
 
-  // VA format of allocated surfaces. TODO(crbug.com/971891): remove.
-  unsigned int va_surface_format_;
-
   // VA handles.
   // All valid after successful Initialize() and until Deinitialize().
   VADisplay va_display_ GUARDED_BY(va_lock_);