[libcamera-devel] [PATCH v7 13/13] py: implement MappedFrameBuffer
Tomi Valkeinen
tomi.valkeinen at ideasonboard.com
Fri May 6 18:45:28 CEST 2022
On 06/05/2022 18:22, Laurent Pinchart wrote:
> Hi Tomi,
>
> On Fri, May 06, 2022 at 05:44:23PM +0300, Tomi Valkeinen wrote:
>> On 05/05/2022 21:24, Laurent Pinchart wrote:
>>> 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.
>
> It means that it won't be possible to write
>
> mfb = fb.mmap()
> for plane in mfb.planes:
> ...
>
> but only
>
> with fb.mmap() as mfb:
> for plane in mfb.planes:
> ...
>
> compared to open() which allows both
>
> with open('file.name', 'rb') as f:
> f.write(data)
>
> and
>
> f = open('file.name', 'rb')
> f.write(data)
>
> It may not be a problem, but I wonder if there's some recommendation for
> this. It can be handled later anyway.
Right. That's why I wrote the first version as I did. I'll try to find
time to read more on the contextmanager recommendations at some point.
Tomi
More information about the libcamera-devel
mailing list