[libcamera-devel] [PATCH v7 13/13] py: implement MappedFrameBuffer
Tomi Valkeinen
tomi.valkeinen at ideasonboard.com
Fri May 6 16:44:23 CEST 2022
On 05/05/2022 21:24, Laurent Pinchart wrote:
> 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 ?
I'm not sure if contextmanager classes are supposed to support multiple
enter & exit cycles. But I'll move the init code to enter. As you said,
it doesn't look balanced.
>> +
>> + 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.
Right, I was a bit hasty there. I've hidden the fields, and expose this
as a property.
Tomi
More information about the libcamera-devel
mailing list