You can not select more than 25 topics Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
chromium/chromium-v4l2-fix.patch

138 lines
6.7 KiB

commit e775ac4770bb1e5dfbfe22c5b56752ed4a2317c5
Author: Hirokazu Honda <hiroh@chromium.org>
Date: Tue Jan 17 17:54:02 2023 +0000
Revert "media/gpu/v4l2VEA,IP: Workaround to USERPTR API against read-only buf"
This reverts commit 203fdd03a4317f2eac57161ee101515a82df704f.
Reason for revert: This workaround is no longer required because of
the kernel patch was reverted.
Original change's description:
> media/gpu/v4l2VEA,IP: Workaround to USERPTR API against read-only buf
>
> VideoFrame fed in VEA::Encode() is read-only since R107 if the
> VideoFrame has a shared memory. V4L2VideoEncodeAccelerator fails
> because VIDOC_QBUF with USERPTR buffer fails if the pointers
> references a read-only buffer.
> The USERPTR API issue is apparently to be fixed by a kernel side.
> In the meantime, this CL adds the workaround to the issue; the read
> only buffer is copied to a writable temporary buffer every Encode
> before VIDIOC_QBUF.
> Not that VideoFrame has a shared memory in the case of screen share
> because a camera stack produces GpuMemoryBuffer.
>
> Bug: b:243883312
> Test: video_encode_accelerator_tests on elm, kevin and trogdor
> Test: screen share in Google Meet on elm, kevin and trogdor
> Change-Id: I922d98eaf52f80d9cd6c3784a8544bffb1232856
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3891421
> Reviewed-by: Nathan Hebert <nhebert@chromium.org>
> Commit-Queue: Hirokazu Honda <hiroh@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1046659}
Bug: b:243883312, b:261660224
Change-Id: I6fe37276f2ebe8c8730cfd3dd766b04277b32a67
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4091205
Reviewed-by: Nathan Hebert <nhebert@chromium.org>
Commit-Queue: Hirokazu Honda <hiroh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1093388}
diff --git a/media/gpu/v4l2/v4l2_image_processor_backend.cc b/media/gpu/v4l2/v4l2_image_processor_backend.cc
index dc72a4ac060a9..f3049f4546e2e 100644
--- a/media/gpu/v4l2/v4l2_image_processor_backend.cc
+++ b/media/gpu/v4l2/v4l2_image_processor_backend.cc
@@ -925,35 +925,16 @@ bool V4L2ImageProcessorBackend::EnqueueInputRecord(
switch (input_memory_type_) {
case V4L2_MEMORY_USERPTR: {
- VideoFrame& frame = *job_record->input_frame;
const size_t num_planes = V4L2Device::GetNumPlanesOfV4L2PixFmt(
input_config_.fourcc.ToV4L2PixFmt());
std::vector<void*> user_ptrs(num_planes);
- if (frame.storage_type() == VideoFrame::STORAGE_SHMEM) {
- // TODO(b/243883312): This copies the video frame to a writable buffer
- // since the USERPTR API requires writable permission. Remove this
- // workaround once the unreasonable permission is fixed.
- const size_t buffer_size = frame.shm_region()->GetSize();
- std::vector<uint8_t> writable_buffer(buffer_size);
- std::memcpy(writable_buffer.data(), frame.data(0), buffer_size);
- for (size_t i = 0; i < num_planes; ++i) {
- const std::intptr_t plane_offset =
- reinterpret_cast<std::intptr_t>(frame.data(i)) -
- reinterpret_cast<std::intptr_t>(frame.data(0));
- user_ptrs[i] = writable_buffer.data() + plane_offset;
- }
- job_record->input_frame->AddDestructionObserver(base::BindOnce(
- [](std::vector<uint8_t>) {}, std::move(writable_buffer)));
- } else {
- for (size_t i = 0; i < num_planes; ++i)
- user_ptrs[i] = frame.writable_data(i);
- }
-
for (size_t i = 0; i < num_planes; ++i) {
int bytes_used =
- VideoFrame::PlaneSize(frame.format(), i, input_config_.size)
+ VideoFrame::PlaneSize(job_record->input_frame->format(), i,
+ input_config_.size)
.GetArea();
buffer.SetPlaneBytesUsed(i, bytes_used);
+ user_ptrs[i] = const_cast<uint8_t*>(job_record->input_frame->data(i));
}
if (!std::move(buffer).QueueUserPtr(user_ptrs)) {
VPLOGF(1) << "Failed to queue a DMABUF buffer to input queue";
diff --git a/media/gpu/v4l2/v4l2_video_encode_accelerator.cc b/media/gpu/v4l2/v4l2_video_encode_accelerator.cc
index 56389ea3eec08..d452ba48a69e2 100644
--- a/media/gpu/v4l2/v4l2_video_encode_accelerator.cc
+++ b/media/gpu/v4l2/v4l2_video_encode_accelerator.cc
@@ -783,7 +783,7 @@ void V4L2VideoEncodeAccelerator::EncodeTask(scoped_refptr<VideoFrame> frame,
const bool is_expected_storage_type =
native_input_mode_
? frame->storage_type() == VideoFrame::STORAGE_GPU_MEMORY_BUFFER
- : frame->storage_type() == VideoFrame::STORAGE_SHMEM;
+ : frame->IsMappable();
if (!is_expected_storage_type) {
VLOGF(1) << "Unexpected storage: "
<< VideoFrame::StorageTypeToString(frame->storage_type());
@@ -1392,27 +1392,16 @@ bool V4L2VideoEncodeAccelerator::EnqueueInputRecord(
NOTIFY_ERROR(kPlatformFailureError);
}
- // TODO(b/243883312): This copies the video frame to a writable buffer
- // since the USERPTR API requires writable permission. Remove this
- // workaround once the unreasonable permission is fixed.
- const size_t buffer_size = frame->shm_region()->GetSize();
- std::vector<uint8_t> writable_buffer(buffer_size);
- std::memcpy(writable_buffer.data(), frame->data(0), buffer_size);
+ // The frame data is readable only and the driver doesn't actually write
+ // the buffer. But USRPTR buffer needs void*. So const_cast<> is required.
std::vector<void*> user_ptrs(num_planes);
for (size_t i = 0; i < num_planes; ++i) {
- const std::intptr_t plane_offset =
- reinterpret_cast<std::intptr_t>(frame->data(i)) -
- reinterpret_cast<std::intptr_t>(frame->data(0));
- user_ptrs[i] = writable_buffer.data() + plane_offset;
+ user_ptrs[i] = const_cast<uint8_t*>(frame->data(i));
}
-
if (!std::move(input_buf).QueueUserPtr(std::move(user_ptrs))) {
- VPLOGF(1) << "Failed to queue a USRPTR buffer to input queue";
- NOTIFY_ERROR(kPlatformFailureError);
+ VPLOGF(1) << "Failed queue a USRPTR buffer to input queue";
return false;
}
- frame->AddDestructionObserver(
- base::DoNothingWithBoundArgs(std::move(writable_buffer)));
break;
}
case V4L2_MEMORY_DMABUF: {
@@ -1421,6 +1410,7 @@ bool V4L2VideoEncodeAccelerator::EnqueueInputRecord(
VPLOGF(1) << "Failed queue a DMABUF buffer to input queue";
return false;
}
+
// Keep |gmb_handle| alive as long as |frame| is alive so that fds passed
// to the driver are valid during encoding.
frame->AddDestructionObserver(base::BindOnce(