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_);