| Message ID | 20220527144447.94891-24-tomi.valkeinen@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
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@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@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] >
On 30/05/2022 12:21, Laurent Pinchart wrote: > 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. Yep. It felt odd to me to implement such a system level helper as SharedFD in Python. I'm not sure how this should be exposed. >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@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. Sure. Tomi
Hi Tomi, On Mon, May 30, 2022 at 12:38:58PM +0300, Tomi Valkeinen wrote: > On 30/05/2022 12:21, Laurent Pinchart wrote: > > 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. > > Yep. It felt odd to me to implement such a system level helper as > SharedFD in Python. I'm not sure how this should be exposed. Python doesn't seem to have a low-level file descriptor object. We could wrap the fd in a file using os.fdopen(), but that takes over the fd and thus would require a dup(), which isn't nice. We could also create bindings for SharedFD. Or something entirely different that would escape me right now. > >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@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.
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: + 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]
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. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@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(-)