Commit 3e957297 authored by Sammie Quon's avatar Sammie Quon Committed by Commit Bot

Revert "media/gpu/vaapi: Create VAImage with va surface size on UploadVideoFrameToSurface()"

This reverts commit 95ad9e5a.

Reason for revert: crbug.com/1057857

Original change's description:
> media/gpu/vaapi: Create VAImage with va surface size on UploadVideoFrameToSurface()
>
> This is a workaround of the failure at 1080p video encoding
> test on grunt.
>
> Originally vaCreateImage is created with a coded frame size.
> However, it causes SEGV_MAPERR on vaPutImage, which copies
> vaImage to vaSurface.
>
> As a result of experiments, the vaImage size needs to be the
> same as va surface size. Since this is not stated in VA-API
> document, so this might be a bug of AMD gallium driver.
>
> Besides of the workaround, this CL copies visible size area
> of VideoFrame to the VAImage.
>
> Bug: 1048908
> Bug: 1050377
> Bug: b:148744040
> Test: video.EncodeAcel.* on grunt, eve and soraka
> Test: camera.* on grunt, eve and soraka
> Change-Id: I1ee853964387846d2452e72cfbd4c939f0a18f62
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2041382
> Commit-Queue: Hirokazu Honda <hiroh@chromium.org>
> Reviewed-by: Andres Calderon Jaramillo <andrescj@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#744953}

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

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1048908, 1050377, b:148744040, 1057857
Change-Id: I21fa0d014fdf2bd18fa2629772fb4b822913c82d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2083867Reviewed-by: default avatarSammie Quon <sammiequon@chromium.org>
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#746186}
parent ddae613f
...@@ -326,7 +326,7 @@ void VaapiJpegEncodeAccelerator::Encoder::EncodeTask( ...@@ -326,7 +326,7 @@ void VaapiJpegEncodeAccelerator::Encoder::EncodeTask(
} }
if (!vaapi_wrapper_->UploadVideoFrameToSurface(*request->video_frame, if (!vaapi_wrapper_->UploadVideoFrameToSurface(*request->video_frame,
va_surface_id_, input_size_)) { va_surface_id_)) {
VLOGF(1) << "Failed to upload video frame to VA surface"; VLOGF(1) << "Failed to upload video frame to VA surface";
notify_error_cb_.Run(task_id, PLATFORM_FAILURE); notify_error_cb_.Run(task_id, PLATFORM_FAILURE);
return; return;
......
...@@ -394,10 +394,6 @@ void VaapiVideoEncodeAccelerator::InitializeTask(const Config& config) { ...@@ -394,10 +394,6 @@ void VaapiVideoEncodeAccelerator::InitializeTask(const Config& config) {
visible_rect_ = gfx::Rect(config.input_visible_size); visible_rect_ = gfx::Rect(config.input_visible_size);
expected_input_coded_size_ = VideoFrame::DetermineAlignedSize( expected_input_coded_size_ = VideoFrame::DetermineAlignedSize(
config.input_format, config.input_visible_size); config.input_format, config.input_visible_size);
DCHECK(
expected_input_coded_size_.width() <= encoder_->GetCodedSize().width() &&
expected_input_coded_size_.height() <= encoder_->GetCodedSize().height());
// The aligned surface size must be the same as a size of a native graphic // The aligned surface size must be the same as a size of a native graphic
// buffer. // buffer.
aligned_va_surface_size_ = aligned_va_surface_size_ =
...@@ -491,15 +487,12 @@ void VaapiVideoEncodeAccelerator::ExecuteEncode(VASurfaceID va_surface_id) { ...@@ -491,15 +487,12 @@ void VaapiVideoEncodeAccelerator::ExecuteEncode(VASurfaceID va_surface_id) {
NOTIFY_ERROR(kPlatformFailureError, "Failed to execute encode"); NOTIFY_ERROR(kPlatformFailureError, "Failed to execute encode");
} }
void VaapiVideoEncodeAccelerator::UploadFrame( void VaapiVideoEncodeAccelerator::UploadFrame(scoped_refptr<VideoFrame> frame,
scoped_refptr<VideoFrame> frame, VASurfaceID va_surface_id) {
VASurfaceID va_surface_id,
const gfx::Size& va_surface_size) {
DCHECK_CALLED_ON_VALID_SEQUENCE(encoder_sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(encoder_sequence_checker_);
DVLOGF(4) << "frame is uploading: " << va_surface_id; DVLOGF(4) << "frame is uploading: " << va_surface_id;
if (!vaapi_wrapper_->UploadVideoFrameToSurface(*frame, va_surface_id, if (!vaapi_wrapper_->UploadVideoFrameToSurface(*frame, va_surface_id))
va_surface_size))
NOTIFY_ERROR(kPlatformFailureError, "Failed to upload frame"); NOTIFY_ERROR(kPlatformFailureError, "Failed to upload frame");
} }
...@@ -758,9 +751,9 @@ std::unique_ptr<VaapiEncodeJob> VaapiVideoEncodeAccelerator::CreateEncodeJob( ...@@ -758,9 +751,9 @@ std::unique_ptr<VaapiEncodeJob> VaapiVideoEncodeAccelerator::CreateEncodeJob(
input_surface, std::move(reconstructed_surface), coded_buffer_id); input_surface, std::move(reconstructed_surface), coded_buffer_id);
if (!native_input_mode_) { if (!native_input_mode_) {
job->AddSetupCallback(base::BindOnce( job->AddSetupCallback(
&VaapiVideoEncodeAccelerator::UploadFrame, encoder_weak_this_, frame, base::BindOnce(&VaapiVideoEncodeAccelerator::UploadFrame,
input_surface->id(), input_surface->size())); encoder_weak_this_, frame, input_surface->id()));
} }
return job; return job;
......
...@@ -109,9 +109,7 @@ class MEDIA_GPU_EXPORT VaapiVideoEncodeAccelerator ...@@ -109,9 +109,7 @@ class MEDIA_GPU_EXPORT VaapiVideoEncodeAccelerator
void EncodePendingInputs(); void EncodePendingInputs();
// Uploads image data from |frame| to |va_surface_id|. // Uploads image data from |frame| to |va_surface_id|.
void UploadFrame(scoped_refptr<VideoFrame> frame, void UploadFrame(scoped_refptr<VideoFrame> frame, VASurfaceID va_surface_id);
VASurfaceID va_surface_id,
const gfx::Size& va_surface_size);
// Executes encode in hardware. This does not block and may return before // Executes encode in hardware. This does not block and may return before
// the job is finished. // the job is finished.
......
...@@ -19,7 +19,6 @@ ...@@ -19,7 +19,6 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/bind_helpers.h" #include "base/bind_helpers.h"
#include "base/bits.h"
#include "base/callback_helpers.h" #include "base/callback_helpers.h"
#include "base/environment.h" #include "base/environment.h"
#include "base/files/scoped_file.h" #include "base/files/scoped_file.h"
...@@ -143,70 +142,7 @@ namespace media { ...@@ -143,70 +142,7 @@ namespace media {
namespace { namespace {
bool GetNV12VisibleWidthBytes(int visible_width, // Maximum framerate of encoded profile. This value is an arbitary limit
uint32_t plane,
size_t* bytes) {
if (plane == 0) {
*bytes = base::checked_cast<size_t>(visible_width);
return true;
}
*bytes = base::checked_cast<size_t>(visible_width);
return visible_width % 2 == 0 ||
base::CheckAdd<int>(visible_width, 1).AssignIfValid(bytes);
}
// Fill 0 on VAImage's non visible area.
bool ClearNV12Padding(const VAImage& image,
const gfx::Size& visible_size,
uint8_t* data) {
DCHECK_EQ(2u, image.num_planes);
DCHECK_EQ(image.format.fourcc, VA_FOURCC_NV12);
size_t visible_width_bytes[2] = {};
if (!GetNV12VisibleWidthBytes(visible_size.width(), 0u,
&visible_width_bytes[0]) ||
!GetNV12VisibleWidthBytes(visible_size.width(), 1u,
&visible_width_bytes[1])) {
return false;
}
for (uint32_t plane = 0; plane < image.num_planes; plane++) {
size_t row_bytes = base::strict_cast<size_t>(image.pitches[plane]);
if (row_bytes == visible_width_bytes[plane])
continue;
CHECK_GT(row_bytes, visible_width_bytes[plane]);
int visible_height = visible_size.height();
if (plane == 1 && !(base::CheckAdd<int>(visible_size.height(), 1) / 2)
.AssignIfValid(&visible_height)) {
return false;
}
const size_t padding_bytes = row_bytes - visible_width_bytes[plane];
uint8_t* plane_data = data + image.offsets[plane];
for (int row = 0; row < visible_height; row++, plane_data += row_bytes)
memset(plane_data + visible_width_bytes[plane], 0, padding_bytes);
CHECK_GE(base::strict_cast<int>(image.height), visible_height);
size_t image_height = base::strict_cast<size_t>(image.height);
if (plane == 1 && !(base::CheckAdd<size_t>(image.height, 1) / 2)
.AssignIfValid(&image_height)) {
return false;
}
base::CheckedNumeric<size_t> remaining_area(image_height);
remaining_area -= base::checked_cast<size_t>(visible_height);
remaining_area *= row_bytes;
if (!remaining_area.IsValid())
return false;
memset(plane_data, 0, remaining_area.ValueOrDie());
}
return true;
}
// Maximum framerate of encoded profile. This value is an arbitrary limit
// and not taken from HW documentation. // and not taken from HW documentation.
constexpr int kMaxEncoderFramerate = 30; constexpr int kMaxEncoderFramerate = 30;
...@@ -1852,20 +1788,12 @@ std::unique_ptr<ScopedVAImage> VaapiWrapper::CreateVaImage( ...@@ -1852,20 +1788,12 @@ std::unique_ptr<ScopedVAImage> VaapiWrapper::CreateVaImage(
} }
bool VaapiWrapper::UploadVideoFrameToSurface(const VideoFrame& frame, bool VaapiWrapper::UploadVideoFrameToSurface(const VideoFrame& frame,
VASurfaceID va_surface_id, VASurfaceID va_surface_id) {
const gfx::Size& va_surface_size) {
TRACE_EVENT0("media,gpu", "VaapiWrapper::UploadVideoFrameToSurface"); TRACE_EVENT0("media,gpu", "VaapiWrapper::UploadVideoFrameToSurface");
base::AutoLock auto_lock(*va_lock_); base::AutoLock auto_lock(*va_lock_);
TRACE_EVENT0("media,gpu", "VaapiWrapper::UploadVideoFrameToSurfaceLocked"); TRACE_EVENT0("media,gpu", "VaapiWrapper::UploadVideoFrameToSurfaceLocked");
if (frame.visible_rect().origin() != gfx::Point(0, 0)) { const gfx::Size size = frame.coded_size();
LOG(ERROR) << "The origin of the frame's visible rectangle is not (0, 0), "
<< "frame.visible_rect().origin()="
<< frame.visible_rect().origin().ToString();
return false;
}
const gfx::Size visible_size = frame.visible_rect().size();
bool va_create_put_fallback = false; bool va_create_put_fallback = false;
VAImage image; VAImage image;
VAStatus va_res = vaDeriveImage(va_display_, va_surface_id, &image); VAStatus va_res = vaDeriveImage(va_display_, va_surface_id, &image);
...@@ -1877,8 +1805,8 @@ bool VaapiWrapper::UploadVideoFrameToSurface(const VideoFrame& frame, ...@@ -1877,8 +1805,8 @@ bool VaapiWrapper::UploadVideoFrameToSurface(const VideoFrame& frame,
.bits_per_pixel = 12}; .bits_per_pixel = 12};
VAImageFormat image_format = kImageFormatNV12; VAImageFormat image_format = kImageFormatNV12;
va_res = vaCreateImage(va_display_, &image_format, va_surface_size.width(), va_res = vaCreateImage(va_display_, &image_format, size.width(),
va_surface_size.height(), &image); size.height(), &image);
VA_SUCCESS_OR_RETURN(va_res, "vaCreateImage", false); VA_SUCCESS_OR_RETURN(va_res, "vaCreateImage", false);
} }
base::ScopedClosureRunner vaimage_deleter( base::ScopedClosureRunner vaimage_deleter(
...@@ -1889,13 +1817,7 @@ bool VaapiWrapper::UploadVideoFrameToSurface(const VideoFrame& frame, ...@@ -1889,13 +1817,7 @@ bool VaapiWrapper::UploadVideoFrameToSurface(const VideoFrame& frame,
return false; return false;
} }
if (image.width % 2 != 0 || image.height % 2 != 0) { if (gfx::Rect(image.width, image.height) < gfx::Rect(size)) {
LOG(ERROR) << "Buffer's width and height are not even, "
<< "width=" << image.width << ", height=" << image.height;
return false;
}
if (!gfx::Rect(image.width, image.height).Contains(gfx::Rect(visible_size))) {
LOG(ERROR) << "Buffer too small to fit the frame."; LOG(ERROR) << "Buffer too small to fit the frame.";
return false; return false;
} }
...@@ -1905,11 +1827,6 @@ bool VaapiWrapper::UploadVideoFrameToSurface(const VideoFrame& frame, ...@@ -1905,11 +1827,6 @@ bool VaapiWrapper::UploadVideoFrameToSurface(const VideoFrame& frame,
return false; return false;
uint8_t* image_ptr = static_cast<uint8_t*>(mapping.data()); uint8_t* image_ptr = static_cast<uint8_t*>(mapping.data());
if (!ClearNV12Padding(image, visible_size, image_ptr)) {
LOG(ERROR) << "Failed to clear non visible area of VAImage";
return false;
}
int ret = 0; int ret = 0;
{ {
base::AutoUnlock auto_unlock(*va_lock_); base::AutoUnlock auto_unlock(*va_lock_);
...@@ -1920,32 +1837,19 @@ bool VaapiWrapper::UploadVideoFrameToSurface(const VideoFrame& frame, ...@@ -1920,32 +1837,19 @@ bool VaapiWrapper::UploadVideoFrameToSurface(const VideoFrame& frame,
frame.data(VideoFrame::kUPlane), frame.stride(VideoFrame::kUPlane), frame.data(VideoFrame::kUPlane), frame.stride(VideoFrame::kUPlane),
frame.data(VideoFrame::kVPlane), frame.stride(VideoFrame::kVPlane), frame.data(VideoFrame::kVPlane), frame.stride(VideoFrame::kVPlane),
image_ptr + image.offsets[0], image.pitches[0], image_ptr + image.offsets[0], image.pitches[0],
image_ptr + image.offsets[1], image.pitches[1], image_ptr + image.offsets[1], image.pitches[1], image.width,
visible_size.width(), visible_size.height()); image.height);
break; break;
case PIXEL_FORMAT_NV12: { case PIXEL_FORMAT_NV12:
int uv_width = visible_size.width();
if (visible_size.width() % 2 != 0 &&
!base::CheckAdd<int>(visible_size.width(), 1)
.AssignIfValid(&uv_width)) {
return false;
}
int uv_height = 0;
if (!(base::CheckAdd<int>(visible_size.height(), 1) / 2)
.AssignIfValid(&uv_height)) {
return false;
}
libyuv::CopyPlane(frame.data(VideoFrame::kYPlane), libyuv::CopyPlane(frame.data(VideoFrame::kYPlane),
frame.stride(VideoFrame::kYPlane), frame.stride(VideoFrame::kYPlane),
image_ptr + image.offsets[0], image.pitches[0], image_ptr + image.offsets[0], image.pitches[0],
visible_size.width(), visible_size.height()); image.width, image.height);
libyuv::CopyPlane(frame.data(VideoFrame::kUVPlane), libyuv::CopyPlane(frame.data(VideoFrame::kUVPlane),
frame.stride(VideoFrame::kUVPlane), frame.stride(VideoFrame::kUVPlane),
image_ptr + image.offsets[1], image.pitches[1], image_ptr + image.offsets[1], image.pitches[1],
uv_width, uv_height); image.width, image.height / 2);
} break; break;
default: default:
LOG(ERROR) << "Unsupported pixel format: " << frame.format(); LOG(ERROR) << "Unsupported pixel format: " << frame.format();
return false; return false;
...@@ -1953,8 +1857,8 @@ bool VaapiWrapper::UploadVideoFrameToSurface(const VideoFrame& frame, ...@@ -1953,8 +1857,8 @@ bool VaapiWrapper::UploadVideoFrameToSurface(const VideoFrame& frame,
} }
if (va_create_put_fallback) { if (va_create_put_fallback) {
va_res = vaPutImage(va_display_, va_surface_id, image.image_id, 0, 0, va_res = vaPutImage(va_display_, va_surface_id, image.image_id, 0, 0,
visible_size.width(), visible_size.height(), 0, 0, size.width(), size.height(), 0, 0, size.width(),
visible_size.width(), visible_size.height()); size.height());
VA_SUCCESS_OR_RETURN(va_res, "vaPutImage", false); VA_SUCCESS_OR_RETURN(va_res, "vaPutImage", false);
} }
return ret == 0; return ret == 0;
......
...@@ -344,8 +344,7 @@ class MEDIA_GPU_EXPORT VaapiWrapper ...@@ -344,8 +344,7 @@ class MEDIA_GPU_EXPORT VaapiWrapper
// Upload contents of |frame| into |va_surface_id| for encode. // Upload contents of |frame| into |va_surface_id| for encode.
bool UploadVideoFrameToSurface(const VideoFrame& frame, bool UploadVideoFrameToSurface(const VideoFrame& frame,
VASurfaceID va_surface_id, VASurfaceID va_surface_id);
const gfx::Size& va_surface_size);
// Create a buffer of |size| bytes to be used as encode output. // Create a buffer of |size| bytes to be used as encode output.
bool CreateVABuffer(size_t size, VABufferID* buffer_id); bool CreateVABuffer(size_t size, VABufferID* buffer_id);
......
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