[libcamera-devel] [PATCH v7 13/13] py: implement MappedFrameBuffer
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu May 5 20:24:33 CEST 2022
Hi Tomi,
Thank you for the patch.
On Thu, May 05, 2022 at 01:41:04PM +0300, Tomi Valkeinen wrote:
> Instead of just exposing plain mmap via fb.mmap(planenum), implement a
> MappedFrameBuffer class, similar to C++'s MappedFrameBuffer.
> MappedFrameBuffer mmaps the underlying filedescriptors and provides
> Python memoryviews for each plane.
>
> As an example, to save a Framebuffer to a file:
>
> with fb.mmap() as mfb:
> with open(filename, "wb") as f:
> for p in mfb.planes:
> f.write(p)
>
> The objects in mfb.planes are memoryviews that cover only the plane in
> question.
>
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
> ---
> src/py/cam/cam.py | 11 +++---
> src/py/cam/cam_qt.py | 6 +--
> src/py/libcamera/__init__.py | 72 +++++++++++++++++++++++++++++++++++-
> src/py/libcamera/pymain.cpp | 7 ++++
> 4 files changed, 86 insertions(+), 10 deletions(-)
>
> diff --git a/src/py/cam/cam.py b/src/py/cam/cam.py
> index 4efa6459..c0ebb186 100755
> --- a/src/py/cam/cam.py
> +++ b/src/py/cam/cam.py
> @@ -329,9 +329,9 @@ def request_handler(state, ctx, req):
>
> crcs = []
> if ctx["opt-crc"]:
> - with fb.mmap(0) as b:
> - crc = binascii.crc32(b)
> - crcs.append(crc)
> + with fb.mmap() as mfb:
> + plane_crcs = [binascii.crc32(p) for p in mfb.planes]
> + crcs.append(plane_crcs)
>
> meta = fb.metadata
>
> @@ -347,10 +347,11 @@ def request_handler(state, ctx, req):
> print(f"\t{ctrl} = {val}")
>
> if ctx["opt-save-frames"]:
> - with fb.mmap(0) as b:
> + with fb.mmap() as mfb:
> filename = "frame-{}-{}-{}.data".format(ctx["id"], stream_name, ctx["reqs-completed"])
> with open(filename, "wb") as f:
> - f.write(b)
> + for p in mfb.planes:
> + f.write(p)
>
> state["renderer"].request_handler(ctx, req)
>
> diff --git a/src/py/cam/cam_qt.py b/src/py/cam/cam_qt.py
> index 30fb7a1d..d394987b 100644
> --- a/src/py/cam/cam_qt.py
> +++ b/src/py/cam/cam_qt.py
> @@ -324,17 +324,17 @@ class MainWindow(QtWidgets.QWidget):
> controlsLayout.addStretch()
>
> def buf_to_qpixmap(self, stream, fb):
> - with fb.mmap(0) as b:
> + with fb.mmap() as mfb:
> cfg = stream.configuration
> w, h = cfg.size
> pitch = cfg.stride
>
> if cfg.pixelFormat == "MJPEG":
> - img = Image.open(BytesIO(b))
> + img = Image.open(BytesIO(mfb.planes[0]))
> qim = ImageQt(img).copy()
> pix = QtGui.QPixmap.fromImage(qim)
> else:
> - data = np.array(b, dtype=np.uint8)
> + data = np.array(mfb.planes[0], dtype=np.uint8)
> rgb = to_rgb(cfg.pixelFormat, cfg.size, data)
>
> if rgb is None:
> diff --git a/src/py/libcamera/__init__.py b/src/py/libcamera/__init__.py
> index cd7512a2..caf06af7 100644
> --- a/src/py/libcamera/__init__.py
> +++ b/src/py/libcamera/__init__.py
> @@ -1,12 +1,80 @@
> # SPDX-License-Identifier: LGPL-2.1-or-later
> # Copyright (C) 2021, Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
>
> +from os import lseek, SEEK_END
> from ._libcamera import *
> import mmap
>
>
> -def __FrameBuffer__mmap(self, plane):
> - return mmap.mmap(self.fd(plane), self.length(plane), mmap.MAP_SHARED, mmap.PROT_READ)
> +def __FrameBuffer__mmap(self):
> + return MappedFrameBuffer(self)
>
>
> FrameBuffer.mmap = __FrameBuffer__mmap
I'd move this after the MappedFrameBuffer class definition.
> +
> +
> +class MappedFrameBuffer:
> + def __init__(self, fb):
> + self.fb = fb
> +
> + # Collect information about the buffers
> +
> + bufinfos = {}
> +
> + for i in range(fb.num_planes):
> + fd = fb.fd(i)
> +
> + if fd not in bufinfos:
> + buflen = lseek(fd, 0, SEEK_END)
> + bufinfos[fd] = {"maplen": 0, "buflen": buflen}
Single quotes here too please.
> + else:
> + buflen = bufinfos[fd]["buflen"]
> +
> + if fb.offset(i) > buflen or fb.offset(i) + fb.length(i) > buflen:
> + raise RuntimeError(f"plane is out of buffer: buffer length={buflen}, "
> + f"plane offset={fb.offset(i)}, plane length={fb.length(i)}")
> +
> + bufinfos[fd]["maplen"] = max(bufinfos[fd]["maplen"], fb.offset(i) + fb.length(i))
> +
> + # mmap the buffers
> +
> + maps = []
> +
> + for fd, info in bufinfos.items():
> + map = mmap.mmap(fd, info["maplen"], mmap.MAP_SHARED, mmap.PROT_READ | mmap.PROT_WRITE)
> + info["map"] = map
> + maps.append(map)
> +
> + self.maps = tuple(maps)
> +
> + # Create memoryviews for the planes
> +
> + planes = []
> +
> + for i in range(fb.num_planes):
> + fd = fb.fd(i)
> + info = bufinfos[fd]
> +
> + mv = memoryview(info["map"])
> +
> + start = fb.offset(i)
> + end = fb.offset(i) + fb.length(i)
> +
> + mv = mv[start:end]
> +
> + planes.append(mv)
> +
> + self.planes = tuple(planes)
> +
> + def __enter__(self):
> + return self
> +
> + def __exit__(self, exc_type, exc_value, exc_traceback):
> + for p in self.planes:
> + p.release()
> +
> + for mm in self.maps:
> + mm.close()
This looks a bit unbalanced, with the mapping created in __init__.
Should the mapping code be moved to __enter__ ? Or is this meant to be
this way to avoid forcing the use of the "with" construct ? If so, are
__enter__ and __exit__ needed, given that the object will be destroyed
once execution exits from the "with" scope ?
> +
> + def planes(self):
> + return self.planes
Is this function actually used, or does the Python code access
self.planes as assigned in __init__ ? You may want to rename the member
to __planes, and turn this function into a getter for a read-only
property. I would also turn self.maps into self.__maps if not needed by
the users of this class.
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> diff --git a/src/py/libcamera/pymain.cpp b/src/py/libcamera/pymain.cpp
> index b9b52f6b..73d29479 100644
> --- a/src/py/libcamera/pymain.cpp
> +++ b/src/py/libcamera/pymain.cpp
> @@ -439,6 +439,9 @@ PYBIND11_MODULE(_libcamera, m)
> return new FrameBuffer(v, cookie);
> }))
> .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;
> @@ -447,6 +450,10 @@ PYBIND11_MODULE(_libcamera, m)
> 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("cookie", &FrameBuffer::cookie, &FrameBuffer::setCookie);
>
> pyStream
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list