Revert "ozone: Remove mesa i965 same-fd workaround restriction"
This reverts commit 46eb9e2770083a3789d4bde29645f905a5a9c417.
Reason for revert: causing tast failures on amd64-generic VM bot
See https://bugs.chromium.org/p/chromium/issues/detail?id=923061#c2
Original change's description:
> ozone: Remove mesa i965 same-fd workaround restriction
>
> NOTE: This is based on crrev.com/c/549576.
>
> After Mesa same-fd for eglCreateImageKHR restriction was relaxed in
> crrev.com/c/378198/, it is now possible to remove the workaround in chromium.
>
> The changes are as follows:
> 1. ClientNativePixmapDmaBuf can map multiple plane using multiple FDs.
> 2. GLImageNativePixmap can create EGLImage using multiple FDs.
> 3. GbmBuffer/GbmPixmap manages multiple FDs.
>
> BUG=642410, 911370
> TEST=VDA unittest and VEA unittest
> TEST=Playback with Chrome ARC++ and ARC++ DRM L1 on kevin, hana, eve
> TEST=Playback with Chrome on peach_pit and nyan_big
>
> Change-Id: I23848b8bcd0e240307633a5201b4b8e58b374282
> Reviewed-on: https://chromium-review.googlesource.com/c/1362777
> Commit-Queue: Hirokazu Honda <hiroh@chromium.org>
> Reviewed-by: Robert Kroeger <rjkroege@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
> Reviewed-by: Pawel Osciak <posciak@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#623686}
TBR=rjkroege@chromium.org,posciak@chromium.org,dcheng@chromium.org,dcastagna@chromium.org,hiroh@chromium.org
Change-Id: Ie9620b2022ec37ca5e55613d4fbe63247d357862
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 642410, 911370, 923061
Reviewed-on: https://chromium-review.googlesource.com/c/1418124
Reviewed-by: Ben Pastene <bpastene@chromium.org>
Commit-Queue: Ben Pastene <bpastene@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623761}diff --git a/components/arc/video_accelerator/gpu_arc_video_decode_accelerator.cc b/components/arc/video_accelerator/gpu_arc_video_decode_accelerator.cc
index 2af5d03..29193169 100644
--- a/components/arc/video_accelerator/gpu_arc_video_decode_accelerator.cc
+++ b/components/arc/video_accelerator/gpu_arc_video_decode_accelerator.cc
@@ -537,27 +537,8 @@
mojom::VideoDecodeAccelerator::Result::INVALID_ARGUMENT);
return;
}
-
- const size_t num_planes = media::VideoFrame::NumPlanes(pixel_format);
-
- // TODO(crbug.com/911370): Remove this workaround once Android passes one fd
- // per plane.
- std::array<base::ScopedFD, media::VideoFrame::kMaxPlanes> scoped_fds;
- scoped_fds[0].reset(handle_fd.release());
- for (size_t i = 1; i < num_planes; ++i) {
- scoped_fds[i].reset(HANDLE_EINTR(dup(scoped_fds[0].get())));
- if (!scoped_fds[i].is_valid()) {
- VLOGF(1) << "Failed to duplicate fd.";
- client_->NotifyError(
- mojom::VideoDecodeAccelerator::Result::PLATFORM_FAILURE);
- return;
- }
- }
- for (size_t i = 0; i < num_planes; ++i) {
- gmb_handle.native_pixmap_handle.fds.push_back(
- base::FileDescriptor(scoped_fds[i].release(), true));
- }
-
+ gmb_handle.native_pixmap_handle.fds.push_back(
+ base::FileDescriptor(handle_fd.release(), true));
for (const auto& plane : planes) {
gmb_handle.native_pixmap_handle.planes.emplace_back(plane.stride,
plane.offset, 0);
diff --git a/gpu/ipc/common/gpu_memory_buffer_impl_native_pixmap.cc b/gpu/ipc/common/gpu_memory_buffer_impl_native_pixmap.cc
index c8bac21..38c2569 100644
--- a/gpu/ipc/common/gpu_memory_buffer_impl_native_pixmap.cc
+++ b/gpu/ipc/common/gpu_memory_buffer_impl_native_pixmap.cc
@@ -35,11 +35,11 @@
const DestructionCallback& callback,
std::unique_ptr<gfx::ClientNativePixmap> pixmap,
const std::vector<gfx::NativePixmapPlane>& planes,
- std::vector<base::ScopedFD> fds)
+ base::ScopedFD fd)
: GpuMemoryBufferImpl(id, size, format, callback),
pixmap_(std::move(pixmap)),
planes_(planes),
- fds_(std::move(fds)) {}
+ fd_(std::move(fd)) {}
GpuMemoryBufferImplNativePixmap::~GpuMemoryBufferImplNativePixmap() = default;
@@ -52,33 +52,42 @@
gfx::BufferFormat format,
gfx::BufferUsage usage,
const DestructionCallback& callback) {
- std::vector<base::ScopedFD> fds;
- std::vector<base::ScopedFD> dup_fds;
- for (auto& fd : handle.native_pixmap_handle.fds) {
- DCHECK(fd.auto_close);
- // Take ownership of FD
- fds.emplace_back(fd.fd);
+ // GpuMemoryBufferImpl needs the FD to implement GetHandle() but
+ // gfx::ClientNativePixmapFactory::ImportFromHandle is expected to take
+ // ownership of the FD passed in the handle so we have to dup it here in
+ // order to pass a valid FD to the GpuMemoryBufferImpl ctor.
+ base::ScopedFD scoped_native_pixmap_handle_fd;
+ base::ScopedFD scoped_fd;
+ if (!handle.native_pixmap_handle.fds.empty()) {
+ // Take ownership of FD at index 0.
+ scoped_native_pixmap_handle_fd.reset(handle.native_pixmap_handle.fds[0].fd);
+
+ // Close all remaining FDs.
+ for (size_t i = 1; i < handle.native_pixmap_handle.fds.size(); ++i)
+ base::ScopedFD scoped_fd(handle.native_pixmap_handle.fds[i].fd);
+
// Duplicate FD for GpuMemoryBufferImplNativePixmap ctor.
- dup_fds.emplace_back(HANDLE_EINTR(dup(fd.fd)));
- if (!dup_fds.back().is_valid()) {
+ scoped_fd.reset(HANDLE_EINTR(dup(scoped_native_pixmap_handle_fd.get())));
+ if (!scoped_fd.is_valid()) {
PLOG(ERROR) << "dup";
return nullptr;
}
}
gfx::NativePixmapHandle native_pixmap_handle;
- for (auto& fd : dup_fds) {
- native_pixmap_handle.fds.emplace_back(fd.release(), true /* auto_close */);
+ if (scoped_native_pixmap_handle_fd.is_valid()) {
+ native_pixmap_handle.fds.emplace_back(
+ scoped_native_pixmap_handle_fd.release(), true /* auto_close */);
}
native_pixmap_handle.planes = handle.native_pixmap_handle.planes;
std::unique_ptr<gfx::ClientNativePixmap> native_pixmap =
- client_native_pixmap_factory->ImportFromHandle(
- std::move(native_pixmap_handle), size, usage);
+ client_native_pixmap_factory->ImportFromHandle(native_pixmap_handle, size,
+ usage);
DCHECK(native_pixmap);
return base::WrapUnique(new GpuMemoryBufferImplNativePixmap(
handle.id, size, format, callback, std::move(native_pixmap),
- handle.native_pixmap_handle.planes, std::move(fds)));
+ handle.native_pixmap_handle.planes, std::move(scoped_fd)));
}
// static
@@ -135,9 +144,8 @@
handle.type = gfx::NATIVE_PIXMAP;
handle.id = id_;
gfx::NativePixmapHandle native_pixmap_handle;
- for (const auto& fd : fds_) {
- native_pixmap_handle.fds.emplace_back(fd.get(), false /* auto_close */);
- }
+ if (fd_.is_valid())
+ native_pixmap_handle.fds.emplace_back(fd_.get(), false /* auto_close */);
native_pixmap_handle.planes = planes_;
handle.native_pixmap_handle = gfx::CloneHandleForIPC(native_pixmap_handle);
return handle;
diff --git a/gpu/ipc/common/gpu_memory_buffer_impl_native_pixmap.h b/gpu/ipc/common/gpu_memory_buffer_impl_native_pixmap.h
index 7117b56..ede8d50 100644
--- a/gpu/ipc/common/gpu_memory_buffer_impl_native_pixmap.h
+++ b/gpu/ipc/common/gpu_memory_buffer_impl_native_pixmap.h
@@ -56,11 +56,11 @@
const DestructionCallback& callback,
std::unique_ptr<gfx::ClientNativePixmap> native_pixmap,
const std::vector<gfx::NativePixmapPlane>& planes,
- std::vector<base::ScopedFD> fds);
+ base::ScopedFD fd);
- const std::unique_ptr<gfx::ClientNativePixmap> pixmap_;
+ std::unique_ptr<gfx::ClientNativePixmap> pixmap_;
std::vector<gfx::NativePixmapPlane> planes_;
- std::vector<base::ScopedFD> fds_;
+ base::ScopedFD fd_;
DISALLOW_COPY_AND_ASSIGN(GpuMemoryBufferImplNativePixmap);
};
diff --git a/media/gpu/v4l2/generic_v4l2_device.cc b/media/gpu/v4l2/generic_v4l2_device.cc
index 6246b375..d59dc6c 100644
--- a/media/gpu/v4l2/generic_v4l2_device.cc
+++ b/media/gpu/v4l2/generic_v4l2_device.cc
@@ -299,14 +299,8 @@
gfx::NativePixmapHandle native_pixmap_handle;
std::vector<base::ScopedFD> duped_fds;
- // The number of file descriptors can be less than the number of planes when
- // v4l2 pix fmt, |fourcc|, is a single plane format. Duplicating the last
- // file descriptor should be safely used for the later planes, because they
- // are on the last buffer.
- for (size_t i = 0; i < num_planes; ++i) {
- int fd =
- i < dmabuf_fds.size() ? dmabuf_fds[i].get() : dmabuf_fds.back().get();
- duped_fds.emplace_back(HANDLE_EINTR(dup(fd)));
+ for (const auto& fd : dmabuf_fds) {
+ duped_fds.emplace_back(HANDLE_EINTR(dup(fd.get())));
if (!duped_fds.back().is_valid()) {
VPLOGF(1) << "Failed duplicating a dmabuf fd";
return nullptr;
diff --git a/media/gpu/v4l2/v4l2_slice_video_decode_accelerator.cc b/media/gpu/v4l2/v4l2_slice_video_decode_accelerator.cc
index 586a6e2..a2450e0 100644
--- a/media/gpu/v4l2/v4l2_slice_video_decode_accelerator.cc
+++ b/media/gpu/v4l2/v4l2_slice_video_decode_accelerator.cc
@@ -1484,29 +1484,11 @@
DVLOGF(3) << "picture_buffer_id=" << picture_buffer_id;
DCHECK(child_task_runner_->BelongsToCurrentThread());
- std::vector<base::ScopedFD> dmabuf_fds;
+ std::vector<base::ScopedFD> passed_dmabuf_fds;
#if defined(USE_OZONE)
- DCHECK_EQ(gpu_memory_buffer_handle.native_pixmap_handle.fds.size(),
- gpu_memory_buffer_handle.native_pixmap_handle.planes.size());
- // If the driver does not accept as many fds as we received from the client,
- // we have to check if the additional fds are actually duplicated fds pointing
- // to previous planes; if so, we can close the duplicates and keep only the
- // original fd(s).
- // Assume that an fd is a duplicate of a previous plane's fd if offset != 0.
- // Otherwise, if offset == 0, return error as it may be pointing to a new
- // plane.
- for (auto& fd : gpu_memory_buffer_handle.native_pixmap_handle.fds) {
- dmabuf_fds.emplace_back(fd.fd);
- }
- for (size_t i = dmabuf_fds.size() - 1; i >= output_planes_count_; i--) {
- if (gpu_memory_buffer_handle.native_pixmap_handle.planes[i].offset == 0) {
- VLOGF(1) << "The dmabuf fd points to a new buffer, ";
- NOTIFY_ERROR(INVALID_ARGUMENT);
- return;
- }
- // Drop safely, because this fd is duplicate dmabuf fd pointing to previous
- // buffer and the appropriate address can be accessed by associated offset.
- dmabuf_fds.pop_back();
+ for (const auto& fd : gpu_memory_buffer_handle.native_pixmap_handle.fds) {
+ DCHECK_NE(fd.fd, -1);
+ passed_dmabuf_fds.push_back(base::ScopedFD(fd.fd));
}
#endif
@@ -1528,7 +1510,8 @@
FROM_HERE,
base::BindOnce(
&V4L2SliceVideoDecodeAccelerator::ImportBufferForPictureTask,
- base::Unretained(this), picture_buffer_id, std::move(dmabuf_fds)));
+ base::Unretained(this), picture_buffer_id,
+ std::move(passed_dmabuf_fds)));
}
void V4L2SliceVideoDecodeAccelerator::ImportBufferForPictureTask(
diff --git a/media/gpu/v4l2/v4l2_video_decode_accelerator.cc b/media/gpu/v4l2/v4l2_video_decode_accelerator.cc
index 69a91530..13bd1af 100644
--- a/media/gpu/v4l2/v4l2_video_decode_accelerator.cc
+++ b/media/gpu/v4l2/v4l2_video_decode_accelerator.cc
@@ -544,40 +544,6 @@
DVLOGF(3) << "picture_buffer_id=" << picture_buffer_id;
DCHECK(child_task_runner_->BelongsToCurrentThread());
- std::vector<base::ScopedFD> dmabuf_fds;
- int32_t stride = 0;
-#if defined(USE_OZONE)
- DCHECK_EQ(gpu_memory_buffer_handle.native_pixmap_handle.fds.size(),
- gpu_memory_buffer_handle.native_pixmap_handle.planes.size());
-
- // If the driver does not accept as many fds as we received from the client,
- // we have to check if the additional fds are actually duplicated fds pointing
- // to previous planes; if so, we can close the duplicates and keep only the
- // original fd(s).
- // Assume that an fd is a duplicate of a previous plane's fd if offset != 0.
- // Otherwise, if offset == 0, return error as it may be pointing to a new
- // plane.
- for (auto& fd : gpu_memory_buffer_handle.native_pixmap_handle.fds) {
- dmabuf_fds.emplace_back(fd.fd);
- }
- for (size_t i = dmabuf_fds.size() - 1; i >= egl_image_planes_count_; i--) {
- if (gpu_memory_buffer_handle.native_pixmap_handle.planes[i].offset == 0) {
- VLOGF(1) << "The dmabuf fd points to a new buffer, ";
- NOTIFY_ERROR(INVALID_ARGUMENT);
- return;
- }
- // Drop safely, because this fd is duplicate dmabuf fd pointing to previous
- // buffer and the appropriate address can be accessed by associated offset.
- dmabuf_fds.pop_back();
- }
-
- stride = gpu_memory_buffer_handle.native_pixmap_handle.planes[0].stride;
- for (const auto& plane :
- gpu_memory_buffer_handle.native_pixmap_handle.planes) {
- DVLOGF(3) << ": offset=" << plane.offset << ", stride=" << plane.stride;
- }
-#endif
-
if (output_mode_ != Config::OutputMode::IMPORT) {
VLOGF(1) << "Cannot import in non-import mode";
NOTIFY_ERROR(INVALID_ARGUMENT);
@@ -594,6 +560,20 @@
return;
}
+ std::vector<base::ScopedFD> dmabuf_fds;
+ int32_t stride = 0;
+#if defined(USE_OZONE)
+ for (const auto& fd : gpu_memory_buffer_handle.native_pixmap_handle.fds) {
+ DCHECK_NE(fd.fd, -1);
+ dmabuf_fds.push_back(base::ScopedFD(fd.fd));
+ }
+ stride = gpu_memory_buffer_handle.native_pixmap_handle.planes[0].stride;
+ for (const auto& plane :
+ gpu_memory_buffer_handle.native_pixmap_handle.planes) {
+ DVLOGF(3) << ": offset=" << plane.offset << ", stride=" << plane.stride;
+ }
+#endif
+
decoder_thread_.task_runner()->PostTask(
FROM_HERE,
base::Bind(&V4L2VideoDecodeAccelerator::ImportBufferForPictureTask,
diff --git a/ui/gfx/linux/client_native_pixmap_dmabuf.cc b/ui/gfx/linux/client_native_pixmap_dmabuf.cc
index 57ebec3..76f6b0b 100644
--- a/ui/gfx/linux/client_native_pixmap_dmabuf.cc
+++ b/ui/gfx/linux/client_native_pixmap_dmabuf.cc
@@ -12,10 +12,8 @@
#include <xf86drm.h>
#include "base/memory/ptr_util.h"
-#include "base/numerics/safe_conversions.h"
#include "base/posix/eintr_wrapper.h"
#include "base/process/memory.h"
-#include "base/process/process_metrics.h"
#include "base/strings/stringprintf.h"
#include "base/trace_event/trace_event.h"
#include "build/build_config.h"
@@ -61,24 +59,6 @@
} // namespace
-ClientNativePixmapDmaBuf::PlaneInfo::PlaneInfo() {}
-
-ClientNativePixmapDmaBuf::PlaneInfo::PlaneInfo(PlaneInfo&& info)
- : fd(std::move(info.fd)),
- data(info.data),
- offset(info.offset),
- size(info.size) {
- // Set nullptr to info.data in order not to call munmap in |info| dtor.
- info.data = nullptr;
-}
-
-ClientNativePixmapDmaBuf::PlaneInfo::~PlaneInfo() {
- if (data) {
- int ret = munmap(data, size);
- DCHECK(!ret);
- }
-}
-
// static
bool ClientNativePixmapDmaBuf::IsConfigurationSupported(
gfx::BufferFormat format,
@@ -157,74 +137,59 @@
ClientNativePixmapDmaBuf::ImportFromDmabuf(
const gfx::NativePixmapHandle& handle,
const gfx::Size& size) {
- std::array<PlaneInfo, kMaxPlanes> plane_info;
- for (size_t i = 0; i < handle.fds.size(); ++i) {
- const auto& fd = handle.fds[i];
- DCHECK(fd.auto_close);
- plane_info[i].fd.reset(fd.fd);
- DCHECK(plane_info[i].fd.is_valid());
- }
-
- DCHECK_EQ(handle.planes.size(), handle.fds.size());
-
- const size_t page_size = base::GetPageSize();
- for (size_t i = 0; i < handle.fds.size(); ++i) {
- // mmap() fails if the offset argument is not page-aligned.
- // Since handle.planes[i].offset is possibly not page-aligned, we
- // have to map with an additional offset to be aligned to the page.
- const size_t extra_offset = handle.planes[i].offset % page_size;
- size_t map_size =
- base::checked_cast<size_t>(handle.planes[i].size + extra_offset);
- plane_info[i].offset = extra_offset;
- plane_info[i].size = map_size;
-
- void* data =
- mmap(nullptr, map_size, (PROT_READ | PROT_WRITE), MAP_SHARED,
- plane_info[i].fd.get(), handle.planes[i].offset - extra_offset);
- if (data == MAP_FAILED) {
- logging::SystemErrorCode mmap_error = logging::GetLastSystemErrorCode();
- if (mmap_error == ENOMEM)
- base::TerminateBecauseOutOfMemory(map_size);
- LOG(ERROR) << "Failed to mmap dmabuf: "
- << logging::SystemErrorCodeToString(mmap_error);
- return nullptr;
- }
- plane_info[i].data = data;
- }
-
- return base::WrapUnique(
- new ClientNativePixmapDmaBuf(handle, size, std::move(plane_info)));
+ return base::WrapUnique(new ClientNativePixmapDmaBuf(handle, size));
}
ClientNativePixmapDmaBuf::ClientNativePixmapDmaBuf(
const gfx::NativePixmapHandle& handle,
- const gfx::Size& size,
- std::array<PlaneInfo, kMaxPlanes> plane_info)
- : pixmap_handle_(handle), size_(size), plane_info_(std::move(plane_info)) {
+ const gfx::Size& size)
+ : pixmap_handle_(handle), size_(size), data_{0} {
TRACE_EVENT0("drm", "ClientNativePixmapDmaBuf");
+ // TODO(dcastagna): support multiple fds.
+ DCHECK_EQ(1u, handle.fds.size());
+ DCHECK_GE(handle.fds.front().fd, 0);
+ dmabuf_fd_.reset(handle.fds.front().fd);
+
+ DCHECK_GE(handle.planes.back().size, 0u);
+ size_t map_size = handle.planes.back().offset + handle.planes.back().size;
+ data_ = mmap(nullptr, map_size, (PROT_READ | PROT_WRITE), MAP_SHARED,
+ dmabuf_fd_.get(), 0);
+ if (data_ == MAP_FAILED) {
+ logging::SystemErrorCode mmap_error = logging::GetLastSystemErrorCode();
+ if (mmap_error == ENOMEM)
+ base::TerminateBecauseOutOfMemory(map_size);
+
+ CHECK(false) << "Failed to mmap dmabuf: "
+ << logging::SystemErrorCodeToString(mmap_error);
+ }
}
ClientNativePixmapDmaBuf::~ClientNativePixmapDmaBuf() {
TRACE_EVENT0("drm", "~ClientNativePixmapDmaBuf");
+ size_t map_size =
+ pixmap_handle_.planes.back().offset + pixmap_handle_.planes.back().size;
+ int ret = munmap(data_, map_size);
+ DCHECK(!ret);
}
bool ClientNativePixmapDmaBuf::Map() {
TRACE_EVENT0("drm", "DmaBuf:Map");
- for (size_t i = 0; i < pixmap_handle_.planes.size(); ++i)
- PrimeSyncStart(plane_info_[i].fd.get());
- return true;
+ if (data_ != nullptr) {
+ PrimeSyncStart(dmabuf_fd_.get());
+ return true;
+ }
+ return false;
}
void ClientNativePixmapDmaBuf::Unmap() {
TRACE_EVENT0("drm", "DmaBuf:Unmap");
- for (size_t i = 0; i < pixmap_handle_.planes.size(); ++i)
- PrimeSyncEnd(plane_info_[i].fd.get());
+ PrimeSyncEnd(dmabuf_fd_.get());
}
void* ClientNativePixmapDmaBuf::GetMemoryAddress(size_t plane) const {
DCHECK_LT(plane, pixmap_handle_.planes.size());
- return static_cast<uint8_t*>(plane_info_[plane].data) +
- plane_info_[plane].offset;
+ uint8_t* address = reinterpret_cast<uint8_t*>(data_);
+ return address + pixmap_handle_.planes[plane].offset;
}
int ClientNativePixmapDmaBuf::GetStride(size_t plane) const {
diff --git a/ui/gfx/linux/client_native_pixmap_dmabuf.h b/ui/gfx/linux/client_native_pixmap_dmabuf.h
index ddd9006..f5da2b3 100644
--- a/ui/gfx/linux/client_native_pixmap_dmabuf.h
+++ b/ui/gfx/linux/client_native_pixmap_dmabuf.h
@@ -7,7 +7,6 @@
#include <stdint.h>
-#include <array>
#include <memory>
#include "base/files/scoped_file.h"
@@ -39,25 +38,13 @@
int GetStride(size_t plane) const override;
private:
- static constexpr size_t kMaxPlanes = 4;
-
- struct PlaneInfo {
- PlaneInfo();
- PlaneInfo(PlaneInfo&& plane_info);
- ~PlaneInfo();
-
- base::ScopedFD fd;
- void* data = nullptr;
- size_t offset = 0;
- size_t size = 0;
- };
ClientNativePixmapDmaBuf(const gfx::NativePixmapHandle& handle,
- const gfx::Size& size,
- std::array<PlaneInfo, kMaxPlanes> plane_info);
+ const gfx::Size& size);
const gfx::NativePixmapHandle pixmap_handle_;
const gfx::Size size_;
- const std::array<PlaneInfo, kMaxPlanes> plane_info_;
+ base::ScopedFD dmabuf_fd_;
+ void* data_;
DISALLOW_COPY_AND_ASSIGN(ClientNativePixmapDmaBuf);
};
diff --git a/ui/gfx/native_pixmap.h b/ui/gfx/native_pixmap.h
index a4cbe0b..1e6fda1 100644
--- a/ui/gfx/native_pixmap.h
+++ b/ui/gfx/native_pixmap.h
@@ -26,8 +26,6 @@
NativePixmap() {}
virtual bool AreDmaBufFdsValid() const = 0;
- // TODO(crbug.com/911370): Remove this because the number of fds will always
- // be equal to the number of planes.
virtual size_t GetDmaBufFdCount() const = 0;
virtual int GetDmaBufFd(size_t plane) const = 0;
virtual int GetDmaBufPitch(size_t plane) const = 0;
diff --git a/ui/gl/gl_image_native_pixmap.cc b/ui/gl/gl_image_native_pixmap.cc
index 0bae74d..a29c9c67 100644
--- a/ui/gl/gl_image_native_pixmap.cc
+++ b/ui/gl/gl_image_native_pixmap.cc
@@ -181,7 +181,8 @@
size_t pixmap_plane = attrs_plane;
- attrs.push_back(pixmap->GetDmaBufFd(pixmap_plane));
+ attrs.push_back(pixmap->GetDmaBufFd(
+ pixmap_plane < pixmap->GetDmaBufFdCount() ? pixmap_plane : 0));
attrs.push_back(EGL_DMA_BUF_PLANE0_OFFSET_EXT + attrs_plane * 3);
attrs.push_back(pixmap->GetDmaBufOffset(pixmap_plane));
attrs.push_back(EGL_DMA_BUF_PLANE0_PITCH_EXT + attrs_plane * 3);
diff --git a/ui/gl/test/gl_image_test_template.h b/ui/gl/test/gl_image_test_template.h
index edc0f1d..a9fa046 100644
--- a/ui/gl/test/gl_image_test_template.h
+++ b/ui/gl/test/gl_image_test_template.h
@@ -80,8 +80,8 @@
return;
// NOTE: On some drm devices (mediatek) the mininum width/height to add an fb
- // for a bo must be 64, and YVU_420 in i915 requires at least 128 length.
- const gfx::Size small_image_size(128, 128);
+ // for a bo must be 64.
+ const gfx::Size small_image_size(64, 64);
const gfx::Size large_image_size(512, 512);
const uint8_t* image_color = this->delegate_.GetImageColor();
diff --git a/ui/ozone/common/linux/gbm_buffer.h b/ui/ozone/common/linux/gbm_buffer.h
index e72b531..c6e0dac 100644
--- a/ui/ozone/common/linux/gbm_buffer.h
+++ b/ui/ozone/common/linux/gbm_buffer.h
@@ -23,8 +23,6 @@
virtual uint32_t GetFormat() const = 0;
virtual uint64_t GetFormatModifier() const = 0;
virtual uint32_t GetFlags() const = 0;
- // TODO(crbug.com/911370): Remove this because the number of fds will always
- // be equal to the number of planes.
virtual size_t GetFdCount() const = 0;
// TODO(reveman): This should not be needed once crbug.com/597932 is
// fixed, as the size would be queried directly from the underlying bo.
diff --git a/ui/ozone/common/linux/gbm_wrapper.cc b/ui/ozone/common/linux/gbm_wrapper.cc
index 0e0a9b56..15efd7b 100644
--- a/ui/ozone/common/linux/gbm_wrapper.cc
+++ b/ui/ozone/common/linux/gbm_wrapper.cc
@@ -38,9 +38,7 @@
flags_(flags),
fds_(std::move(fds)),
size_(size),
- planes_(std::move(planes)) {
- DCHECK_EQ(fds_.size(), planes_.size());
- }
+ planes_(std::move(planes)) {}
~Buffer() override {
DCHECK(!mmap_data_);
@@ -95,13 +93,16 @@
// TODO(dcastagna): Use gbm_bo_get_num_planes once all the formats we use
// are supported by gbm.
for (size_t i = 0; i < gfx::NumberOfPlanesForBufferFormat(format); ++i) {
- base::ScopedFD scoped_fd(HANDLE_EINTR(dup(GetPlaneFd(i))));
- if (!scoped_fd.is_valid()) {
- PLOG(ERROR) << "dup";
- return gfx::NativePixmapHandle();
+ // Some formats (e.g: YVU_420) might have less than one fd per plane.
+ if (i < fds_.size()) {
+ base::ScopedFD scoped_fd(HANDLE_EINTR(dup(GetPlaneFd(i))));
+ if (!scoped_fd.is_valid()) {
+ PLOG(ERROR) << "dup";
+ return gfx::NativePixmapHandle();
+ }
+ handle.fds.emplace_back(
+ base::FileDescriptor(scoped_fd.release(), true /* auto_close */));
}
- handle.fds.emplace_back(
- base::FileDescriptor(scoped_fd.release(), true /* auto_close */));
handle.planes.emplace_back(GetPlaneStride(i), GetPlaneOffset(i),
GetPlaneSize(i), GetFormatModifier());
}
@@ -160,12 +161,16 @@
// kept open for the lifetime of the buffer.
base::ScopedFD fd(gbm_bo_get_plane_fd(bo, i));
- if (!fd.is_valid()) {
- PLOG(ERROR) << "Failed to export buffer to dma_buf";
- gbm_bo_destroy(bo);
- return nullptr;
+ // TODO(dcastagna): support multiple fds.
+ // crbug.com/642410
+ if (!i) {
+ if (!fd.is_valid()) {
+ PLOG(ERROR) << "Failed to export buffer to dma_buf";
+ gbm_bo_destroy(bo);
+ return nullptr;
+ }
+ fds.emplace_back(std::move(fd));
}
- fds.emplace_back(std::move(fd));
planes.emplace_back(gbm_bo_get_plane_stride(bo, i),
gbm_bo_get_plane_offset(bo, i),
diff --git a/ui/ozone/platform/drm/gpu/gbm_surface_factory.cc b/ui/ozone/platform/drm/gpu/gbm_surface_factory.cc
index c9c5c56..6d08b23 100644
--- a/ui/ozone/platform/drm/gpu/gbm_surface_factory.cc
+++ b/ui/ozone/platform/drm/gpu/gbm_surface_factory.cc
@@ -244,15 +244,15 @@
gfx::BufferFormat format,
const gfx::NativePixmapHandle& handle) {
size_t num_planes = gfx::NumberOfPlanesForBufferFormat(format);
- DCHECK_GE(num_planes, handle.fds.size());
- if (handle.planes.size() != num_planes) {
+ if (handle.planes.size() != num_planes ||
+ (handle.fds.size() != 1 && handle.fds.size() != num_planes)) {
return nullptr;
}
-
std::vector<base::ScopedFD> scoped_fds;
for (auto& fd : handle.fds) {
scoped_fds.emplace_back(fd.fd);
}
+
std::vector<gfx::NativePixmapPlane> planes;
for (const auto& plane : handle.planes) {
planes.push_back(plane);
