[libcamera-devel] [PATCH v3 23/30] py: Implement FrameBufferPlane
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon May 30 11:21:13 CEST 2022
Hi Tomi,
Thank you for the patch.
On Fri, May 27, 2022 at 05:44:40PM +0300, Tomi Valkeinen wrote:
> Implement FrameBufferPlane class and adjust the methods and uses
> accordingly.
>
> Note that we don't expose the fd as a SharedFD, but as an int.
This may cause use-after-free (or rather use-after-close) issues and/or
fd leaks, but I think that's OK for now. We can improve it later.
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
> ---
> src/py/cam/cam_kms.py | 6 +--
> src/py/cam/cam_qtgl.py | 2 +-
> src/py/libcamera/py_main.cpp | 45 +++++++++++----------
> src/py/libcamera/utils/MappedFrameBuffer.py | 18 ++++-----
> 4 files changed, 36 insertions(+), 35 deletions(-)
>
> diff --git a/src/py/cam/cam_kms.py b/src/py/cam/cam_kms.py
> index 49b00211..3d1e8c27 100644
> --- a/src/py/cam/cam_kms.py
> +++ b/src/py/cam/cam_kms.py
> @@ -131,10 +131,10 @@ class KMSRenderer:
> fds = []
> strides = []
> offsets = []
> - for i in range(fb.num_planes):
> - fds.append(fb.fd(i))
> + for p in fb.planes:
> + fds.append(p.fd)
> strides.append(cfg.stride)
> - offsets.append(fb.offset(i))
> + offsets.append(p.offset)
>
> drmfb = pykms.DmabufFramebuffer(self.card, w, h, fmt,
> fds, strides, offsets)
> diff --git a/src/py/cam/cam_qtgl.py b/src/py/cam/cam_qtgl.py
> index 4b43f51d..6cfbd347 100644
> --- a/src/py/cam/cam_qtgl.py
> +++ b/src/py/cam/cam_qtgl.py
> @@ -269,7 +269,7 @@ class MainWindow(QtWidgets.QWidget):
> EGL_WIDTH, w,
> EGL_HEIGHT, h,
> EGL_LINUX_DRM_FOURCC_EXT, fmt,
> - EGL_DMA_BUF_PLANE0_FD_EXT, fb.fd(0),
> + EGL_DMA_BUF_PLANE0_FD_EXT, fb.planes[0].fd,
> EGL_DMA_BUF_PLANE0_OFFSET_EXT, 0,
> EGL_DMA_BUF_PLANE0_PITCH_EXT, cfg.stride,
> EGL_NONE,
> diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp
> index 52f70811..fcf009f0 100644
> --- a/src/py/libcamera/py_main.cpp
> +++ b/src/py/libcamera/py_main.cpp
> @@ -155,6 +155,7 @@ PYBIND11_MODULE(_libcamera, m)
> auto pyStreamFormats = py::class_<StreamFormats>(m, "StreamFormats");
> auto pyFrameBufferAllocator = py::class_<FrameBufferAllocator>(m, "FrameBufferAllocator");
> auto pyFrameBuffer = py::class_<FrameBuffer>(m, "FrameBuffer");
> + auto pyFrameBufferPlane = py::class_<FrameBuffer::Plane>(pyFrameBuffer, "Plane");
> auto pyStream = py::class_<Stream>(m, "Stream");
> auto pyControlId = py::class_<ControlId>(m, "ControlId");
> auto pyControlInfo = py::class_<ControlInfo>(m, "ControlInfo");
> @@ -408,31 +409,31 @@ PYBIND11_MODULE(_libcamera, m)
> });
>
> pyFrameBuffer
> - /* \todo implement FrameBuffer::Plane properly */
> - .def(py::init([](std::vector<std::tuple<int, unsigned int>> planes, unsigned int cookie) {
> - std::vector<FrameBuffer::Plane> v;
> - for (const auto &t : planes)
> - v.push_back({ SharedFD(std::get<0>(t)), FrameBuffer::Plane::kInvalidOffset, std::get<1>(t) });
> - return new FrameBuffer(v, cookie);
> - }))
> + .def(py::init<std::vector<FrameBuffer::Plane>, unsigned int>(),
> + py::arg("planes"), py::arg("cookie") = 0)
> .def_property_readonly("metadata", &FrameBuffer::metadata, py::return_value_policy::reference_internal)
> - .def_property_readonly("num_planes", [](const FrameBuffer &self) {
> - return self.planes().size();
> - })
> - .def("length", [](FrameBuffer &self, uint32_t idx) {
> - const FrameBuffer::Plane &plane = self.planes()[idx];
> - return plane.length;
> - })
> - .def("fd", [](FrameBuffer &self, uint32_t idx) {
> - const FrameBuffer::Plane &plane = self.planes()[idx];
> - return plane.fd.get();
> - })
> - .def("offset", [](FrameBuffer &self, uint32_t idx) {
> - const FrameBuffer::Plane &plane = self.planes()[idx];
> - return plane.offset;
> - })
> + .def_property_readonly("planes", &FrameBuffer::planes)
> .def_property("cookie", &FrameBuffer::cookie, &FrameBuffer::setCookie);
>
> + pyFrameBufferPlane
> + .def(py::init())
> + .def(py::init([](int fd, unsigned int offset, unsigned int length) {
> + auto p = FrameBuffer::Plane();
> + p.fd = SharedFD(fd);
> + p.offset = offset;
> + p.length = length;
> + return p;
> + }), py::arg("fd"), py::arg("offset"), py::arg("length"))
> + .def_property("fd",
> + [](const FrameBuffer::Plane &self) {
> + return self.fd.get();
> + },
> + [](FrameBuffer::Plane &self, int fd) {
> + self.fd = SharedFD(fd);
> + })
> + .def_readwrite("offset", &FrameBuffer::Plane::offset)
> + .def_readwrite("length", &FrameBuffer::Plane::length);
> +
> pyStream
> .def_property_readonly("configuration", &Stream::configuration);
>
> diff --git a/src/py/libcamera/utils/MappedFrameBuffer.py b/src/py/libcamera/utils/MappedFrameBuffer.py
> index fc2726b6..69315e76 100644
> --- a/src/py/libcamera/utils/MappedFrameBuffer.py
> +++ b/src/py/libcamera/utils/MappedFrameBuffer.py
> @@ -21,8 +21,8 @@ class MappedFrameBuffer:
>
> bufinfos = {}
>
> - for i in range(fb.num_planes):
> - fd = fb.fd(i)
> + for p in fb.planes:
Could we call the variable plane instead of p ? Same below.
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> + fd = p.fd
>
> if fd not in bufinfos:
> buflen = os.lseek(fd, 0, os.SEEK_END)
> @@ -30,11 +30,11 @@ class MappedFrameBuffer:
> else:
> buflen = bufinfos[fd]['buflen']
>
> - if fb.offset(i) > buflen or fb.offset(i) + fb.length(i) > buflen:
> + if p.offset > buflen or p.offset + p.length > buflen:
> raise RuntimeError(f'plane is out of buffer: buffer length={buflen}, ' +
> - f'plane offset={fb.offset(i)}, plane length={fb.length(i)}')
> + f'plane offset={p.offset}, plane length={p.length}')
>
> - bufinfos[fd]['maplen'] = max(bufinfos[fd]['maplen'], fb.offset(i) + fb.length(i))
> + bufinfos[fd]['maplen'] = max(bufinfos[fd]['maplen'], p.offset + p.length)
>
> # mmap the buffers
>
> @@ -51,14 +51,14 @@ class MappedFrameBuffer:
>
> planes = []
>
> - for i in range(fb.num_planes):
> - fd = fb.fd(i)
> + for p in fb.planes:
> + fd = p.fd
> info = bufinfos[fd]
>
> mv = memoryview(info['map'])
>
> - start = fb.offset(i)
> - end = fb.offset(i) + fb.length(i)
> + start = p.offset
> + end = p.offset + p.length
>
> mv = mv[start:end]
>
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list