Commit 7881e219 authored by Miguel Casas's avatar Miguel Casas Committed by Commit Bot

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

This reverts commit 9881e6cd.

Reason for revert: Needs a follow up crrev.com/c/2362068, but
the code is still faulty because it'd hit a DCHECK. Let's revert
this one and land a correct CL instead.

Original change's 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: Ic54d0d26f5d17ea49fe896cf7ca5e1c931d07e0f
> 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-Commit-Position: refs/heads/master@{#799410}

TBR=mcasas@chromium.org,hiroh@chromium.org

Change-Id: I2b13e096fb7fce00c4dbbaed64bd4d231de34d15
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: b:162962069
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2363798Reviewed-by: default avatarMiguel Casas <mcasas@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799455}
parent eb0c5166
...@@ -46,6 +46,7 @@ ...@@ -46,6 +46,7 @@
// Auto-generated for dlopen libva libraries // Auto-generated for dlopen libva libraries
#include "media/gpu/vaapi/va_stubs.h" #include "media/gpu/vaapi/va_stubs.h"
#include "media/gpu/vaapi/vaapi_utils.h"
#include "third_party/libyuv/include/libyuv.h" #include "third_party/libyuv/include/libyuv.h"
#include "ui/gfx/buffer_format_util.h" #include "ui/gfx/buffer_format_util.h"
#include "ui/gfx/buffer_types.h" #include "ui/gfx/buffer_types.h"
...@@ -2114,15 +2115,18 @@ void VaapiWrapper::DestroyVABuffer(VABufferID buffer_id) { ...@@ -2114,15 +2115,18 @@ void VaapiWrapper::DestroyVABuffer(VABufferID buffer_id) {
base::AutoLock auto_lock(*va_lock_); base::AutoLock auto_lock(*va_lock_);
VAStatus va_res = vaDestroyBuffer(va_display_, buffer_id); VAStatus va_res = vaDestroyBuffer(va_display_, buffer_id);
VA_LOG_ON_ERROR(va_res, VaapiFunctions::kVADestroyBuffer); VA_LOG_ON_ERROR(va_res, VaapiFunctions::kVADestroyBuffer);
const auto was_found = va_buffers_.erase(buffer_id);
DCHECK(was_found);
} }
void VaapiWrapper::DestroyVABuffers() { void VaapiWrapper::DestroyVABuffers() {
base::AutoLock auto_lock(*va_lock_); base::AutoLock auto_lock(*va_lock_);
for (const VABufferID va_buffer_id : va_buffers_) { for (auto it = va_buffers_.begin(); it != va_buffers_.end(); ++it) {
VAStatus va_res = vaDestroyBuffer(va_display_, va_buffer_id); VAStatus va_res = vaDestroyBuffer(va_display_, *it);
VA_LOG_ON_ERROR(va_res, VaapiFunctions::kVADestroyBuffer); VA_LOG_ON_ERROR(va_res, VaapiFunctions::kVADestroyBuffer);
} }
va_buffers_.clear(); va_buffers_.clear();
} }
...@@ -2149,23 +2153,22 @@ bool VaapiWrapper::BlitSurface(const VASurface& va_surface_src, ...@@ -2149,23 +2153,22 @@ bool VaapiWrapper::BlitSurface(const VASurface& va_surface_src,
VideoRotation rotation) { VideoRotation rotation) {
base::AutoLock auto_lock(*va_lock_); base::AutoLock auto_lock(*va_lock_);
// Create a buffer for VPP if it has not been created. if (va_buffers_.empty()) {
if (!va_buffer_for_vpp_) {
DCHECK_NE(VA_INVALID_ID, va_context_id_); DCHECK_NE(VA_INVALID_ID, va_context_id_);
VABufferID buffer_id = VA_INVALID_ID; // Create a buffer for VPP if it has not been created.
VABufferID buffer_id;
const VAStatus va_res = vaCreateBuffer( const VAStatus va_res = vaCreateBuffer(
va_display_, va_context_id_, VAProcPipelineParameterBufferType, va_display_, va_context_id_, VAProcPipelineParameterBufferType,
sizeof(VAProcPipelineParameterBuffer), 1, nullptr, &buffer_id); sizeof(VAProcPipelineParameterBuffer), 1, nullptr, &buffer_id);
VA_SUCCESS_OR_RETURN(va_res, VaapiFunctions::kVACreateBuffer, false); VA_SUCCESS_OR_RETURN(va_res, VaapiFunctions::kVACreateBuffer, false);
DCHECK_NE(buffer_id, VA_INVALID_ID); 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_, ScopedVABufferMapping mapping(va_lock_, va_display_, buffer_id);
va_buffer_for_vpp_->id());
if (!mapping.IsValid()) if (!mapping.IsValid())
return false; return false;
auto* pipeline_param = auto* pipeline_param =
...@@ -2219,9 +2222,8 @@ bool VaapiWrapper::BlitSurface(const VASurface& va_surface_src, ...@@ -2219,9 +2222,8 @@ bool VaapiWrapper::BlitSurface(const VASurface& va_surface_src,
vaBeginPicture(va_display_, va_context_id_, va_surface_dest.id()), vaBeginPicture(va_display_, va_context_id_, va_surface_dest.id()),
VaapiFunctions::kVABeginPicture, false); VaapiFunctions::kVABeginPicture, false);
VABufferID va_buffer_id = va_buffer_for_vpp_->id();
VA_SUCCESS_OR_RETURN( VA_SUCCESS_OR_RETURN(
vaRenderPicture(va_display_, va_context_id_, &va_buffer_id, 1), vaRenderPicture(va_display_, va_context_id_, &buffer_id, 1),
VaapiFunctions::kVARenderPicture_Vpp, false); VaapiFunctions::kVARenderPicture_Vpp, false);
VA_SUCCESS_OR_RETURN(vaEndPicture(va_display_, va_context_id_), VA_SUCCESS_OR_RETURN(vaEndPicture(va_display_, va_context_id_),
......
...@@ -28,7 +28,6 @@ ...@@ -28,7 +28,6 @@
#include "base/thread_annotations.h" #include "base/thread_annotations.h"
#include "media/gpu/media_gpu_export.h" #include "media/gpu/media_gpu_export.h"
#include "media/gpu/vaapi/va_surface.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_decode_accelerator.h"
#include "media/video/video_encode_accelerator.h" #include "media/video/video_encode_accelerator.h"
#include "ui/gfx/geometry/size.h" #include "ui/gfx/geometry/size.h"
...@@ -47,6 +46,8 @@ class Rect; ...@@ -47,6 +46,8 @@ class Rect;
namespace media { namespace media {
constexpr unsigned int kInvalidVaRtFormat = 0u; constexpr unsigned int kInvalidVaRtFormat = 0u;
class ScopedVAImage;
class ScopedVASurface;
class VideoFrame; class VideoFrame;
// Enum, function and callback type to allow VaapiWrapper to log errors in VA // Enum, function and callback type to allow VaapiWrapper to log errors in VA
...@@ -466,13 +467,9 @@ class MEDIA_GPU_EXPORT VaapiWrapper ...@@ -466,13 +467,9 @@ class MEDIA_GPU_EXPORT VaapiWrapper
// Data queued up for HW codec, to be committed on next execution. // Data queued up for HW codec, to be committed on next execution.
std::vector<VABufferID> pending_va_buffers_; std::vector<VABufferID> pending_va_buffers_;
// VABufferIDs for kEncode*. // Buffers for kEncode or kVideoProcess.
std::set<VABufferID> va_buffers_; 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 // Called to report codec errors to UMA. Errors to clients are reported via
// return values from public methods. // return values from public methods.
ReportErrorToUMACB report_error_to_uma_cb_; ReportErrorToUMACB report_error_to_uma_cb_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment