[{"id":23324,"web_url":"https://patchwork.libcamera.org/comment/23324/","msgid":"<20220605123156.j6veszk3zqsdmsfz@uno.localdomain>","date":"2022-06-05T12:31:56","subject":"Re: [libcamera-devel] [PATCH v4 14/16] py: examples: Add\n\tsimple-continuous-capture.py","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Tomi,\n\nOn Mon, May 30, 2022 at 05:27:20PM +0300, Tomi Valkeinen wrote:\n> Add a slightly more complex, and I think a more realistic, example,\n> where the script reacts to events and re-queues the buffers.\n>\n\nMy first question is if such similar examples are useful or they will\nbecome a maintainership burden.\n\nFrom my side, the more examples the better, but I think the question\nif it's woth it stays..\n\n> Signed-off-by: Tomi Valkeinen \n> ---\n> src/py/examples/simple-continuous-capture.py | 189 +++++++++++++++++++\n> 1 file changed, 189 insertions(+)\n> create mode 100755 src/py/examples/simple-continuous-capture.py\n>\n> diff --git a/src/py/examples/simple-continuous-capture.py b/src/py/examples/simple-continuous-capture.py\n> new file mode 100755\n> index 00000000..d0f8a7e9\n> --- /dev/null\n> +++ b/src/py/examples/simple-continuous-capture.py\n> @@ -0,0 +1,189 @@\n> +#!/usr/bin/env python3\n> +\n> +# SPDX-License-Identifier: BSD-3-Clause\n> +# Copyright (C) 2022, Tomi Valkeinen \n> +\n> +# A simple capture example extending the simple-capture.py example:\n> +# - Capture frames using events from multiple cameras\n> +# - Listening events from stdin to exit the application\n> +# - Memory mapping the frames and calculating CRC\n> +\n> +import binascii\n> +import libcamera as libcam\n> +import libcamera.utils\n> +import selectors\n> +import sys\n> +\n> +\n> +# A container class for our state per camera\n> +class CameraCaptureContext:\n> + idx: int\n> + cam: libcam.Camera\n> + reqs: list[libcam.Request]\n> + mfbs: dict[libcam.FrameBuffer, libcamera.utils.MappedFrameBuffer]\n> +\n> + def __init__(self, cam, idx):\n> + # Acquire the camera for our use\n> +\n> + ret = cam.acquire()\n> + assert ret == 0\n> +\n> + # Configure the camera\n> +\n> + cam_config = cam.generate_configuration([libcam.StreamRole.Viewfinder])\n> +\n> + stream_config = cam_config.at(0)\n> +\n> + ret = cam.configure(cam_config)\n> + assert ret == 0\n> +\n> + stream = stream_config.stream\n> +\n> + # Allocate the buffers for capture\n> +\n> + allocator = libcam.FrameBufferAllocator(cam)\n> + ret = allocator.allocate(stream)\n> + assert ret > 0\n> +\n> + num_bufs = len(allocator.buffers(stream))\n> +\n> + print(f'cam{idx} ({cam.id}): capturing {num_bufs} buffers with {stream_config}')\n> +\n> + # Create the requests and assign a buffer for each request\n> +\n> + reqs = []\n> + for i in range(num_bufs):\n> + # Use the buffer index as the \"cookie\"\n> + req = cam.create_request(idx)\n> +\n> + buffer = allocator.buffers(stream)[i]\n> + ret = req.add_buffer(stream, buffer)\n> + assert ret == 0\n> +\n> + reqs.append(req)\n> +\n> + self.idx = idx\n> + self.cam = cam\n> + self.reqs = reqs\n> + self.mfbs = dict([(fb, libcamera.utils.MappedFrameBuffer(fb).mmap()) for fb in allocator.buffers(stream)])\n\nNot that relevant, but you could populate self.reqs directly and\nappend a {buffer, libcamera.utils.MappedFrameBuffer(fb).mmap()} pair\nto the self.mfbs dictionary in the request creation loop ?\n\n> +\n> + def uninit_camera(self):\n> + # Stop the camera\n> +\n> + ret = self.cam.stop()\n> + assert ret == 0\n> +\n> + # Release the camera\n> +\n> + ret = self.cam.release()\n> + assert ret == 0\n> +\n> +\n> +# A container class for our state\n> +class CaptureContext:\n> + cm: libcam.CameraManager\n> + camera_contexts: list[CameraCaptureContext] = []\n> +\n> + def handle_camera_event(self):\n> + # cm.get_ready_requests() will not block here, as we know there is an event\n> + # to read.\n> +\n> + reqs = self.cm.get_ready_requests()\n> +\n> + # Process the captured frames\n> +\n> + for req in reqs:\n> + self.handle_request(req)\n> +\n> + return True\n> +\n> + def handle_request(self, req: libcam.Request):\n> + cam_ctx = self.camera_contexts[req.cookie]\n> +\n> + buffers = req.buffers\n> +\n> + assert len(buffers) == 1\n> +\n> + # A ready Request could contain multiple buffers if multiple streams\n> + # were being used. Here we know we only have a single stream,\n> + # and we use next(iter()) to get the first and only buffer.\n> +\n> + stream, fb = next(iter(buffers.items()))\n> +\n> + # Use the MappedFrameBuffer to access the pixel data with CPU. We calculate\n> + # the crc for each plane.\n> +\n> + mfb = cam_ctx.mfbs[fb]\n> + crcs = [binascii.crc32(p) for p in mfb.planes]\n> +\n> + meta = fb.metadata\n> +\n> + print('cam{:<6} seq {:<6} bytes {:10} CRCs {}'\n> + .format(cam_ctx.idx,\n> + meta.sequence,\n> + '/'.join([str(p.bytes_used) for p in meta.planes]),\n> + crcs))\n> +\n> + # We want to re-queue the buffer we just handled. Instead of creating\n> + # a new Request, we re-use the old one. We need to call req.reuse()\n> + # to re-initialize the Request before queuing.\n> +\n> + req.reuse()\n> + cam_ctx.cam.queue_request(req)\n> +\n> + def capture(self):\n> + # Queue the requests to the camera\n> +\n> + for cam_ctx in self.camera_contexts:\n> + for req in cam_ctx.reqs:\n> + ret = cam_ctx.cam.queue_request(req)\n> + assert ret == 0\n> +\n> + # Use Selector to wait for events from the camera and from the keyboard\n> +\n> + sel = selectors.DefaultSelector()\n> + sel.register(sys.stdin, selectors.EVENT_READ, handle_key_event)\n> + sel.register(self.cm.event_fd, selectors.EVENT_READ, lambda: self.handle_camera_event())\n> +\n> + running = True\n> +\n> + while running:\n> + events = sel.select()\n> + for key, mask in events:\n> + # If the handler return False, we should exit\n> + if not key.data():\n> + running = False\n> +\n> +\n> +def handle_key_event():\n> + sys.stdin.readline()\n> + print('Exiting...')\n> + return False\n\nNit: why is this a global function handle_camera_event() a class\nmember ?\n\nTo be honest I would get rid of the CaptureContext class and open-code\nCaptureContext::capture() in your main function, with all the handlers\nbeing global functions. Not a big deal though\n\n> +\n> +\n> +def main():\n> + cm = libcam.CameraManager.singleton()\n> +\n> + ctx = CaptureContext()\n> + ctx.cm = cm\n> +\n> + for idx, cam in enumerate(cm.cameras):\n> + cam_ctx = CameraCaptureContext(cam, idx)\n\nCan't you start the camera here ?\n\n> + ctx.camera_contexts.append(cam_ctx)\n> +\n> + # Start the cameras\n> +\n> + for cam_ctx in ctx.camera_contexts:\n> + ret = cam_ctx.cam.start()\n> + assert ret == 0\n> +\n> + ctx.capture()\n> +\n> + for cam_ctx in ctx.camera_contexts:\n> + cam_ctx.uninit_camera()\n> +\n> + return 0\n> +\n> +\n> +if __name__ == '__main__':\n> + sys.exit(main())\n\nNits apart\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> --\n> 2.34.1\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id CC75FBD161\n\tfor ;\n\tSun, 5 Jun 2022 12:32:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 43FE165634;\n\tSun, 5 Jun 2022 14:32:01 +0200 (CEST)","from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net\n\t[217.70.183.199])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C727960104\n\tfor ;\n\tSun, 5 Jun 2022 14:31:59 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 9D883FF802;\n\tSun, 5 Jun 2022 12:31:57 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1654432321;\n\tbh=DcMTZWaRhQD2JAAF9TPbGZGvIV9XXC64wXZrbTN5QwY=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=mivf/W/M28SPFvZe5JoCC/ULkqOjP+8M93G8NZjisQtgzWub8VKyGajcFE59/cV56\n\tzDsbeqY14d7MH1L8rkQh3bB+ps5c4+HaC8uDVXMMVXEf/0fphKquY245WruX/lrv5V\n\tlAZUpc4ZBEog/Q5M6Ol4l3YrbgZBDI82aQwRFudgt3LnwDBKbEidPshNACfPqiXAUo\n\tPx46G73SDMJWmozt8oyPtlhlln1NbBNqQ9Zpn2aQXHz5CcvyS2nRT5hYwACgL2dCe+\n\tyzJuSL3Qez1aSkkuTwcLxFU9RKS4maErFT0c72KXWfPx8s0akbqTfRYjEV3WaBA54z\n\tw0VYSI0AypY5g==","Date":"Sun, 5 Jun 2022 14:31:56 +0200","To":"Tomi Valkeinen ","Message-ID":"<20220605123156.j6veszk3zqsdmsfz@uno.localdomain>","References":"<20220530142722.57618-1-tomi.valkeinen@ideasonboard.com>\n\t<20220530142722.57618-15-tomi.valkeinen@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220530142722.57618-15-tomi.valkeinen@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 14/16] py: examples: Add\n\tsimple-continuous-capture.py","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23332,"web_url":"https://patchwork.libcamera.org/comment/23332/","msgid":"<0a1f10d0-3769-51ab-9d6a-89d1908f9a0d@ideasonboard.com>","date":"2022-06-06T08:56:11","subject":"Re: [libcamera-devel] [PATCH v4 14/16] py: examples: Add\n\tsimple-continuous-capture.py","submitter":{"id":109,"url":"https://patchwork.libcamera.org/api/people/109/","name":"Tomi Valkeinen","email":"tomi.valkeinen@ideasonboard.com"},"content":"On 05/06/2022 15:31, Jacopo Mondi wrote:\n> Hi Tomi,\n> \n> On Mon, May 30, 2022 at 05:27:20PM +0300, Tomi Valkeinen wrote:\n>> Add a slightly more complex, and I think a more realistic, example,\n>> where the script reacts to events and re-queues the buffers.\n>>\n> \n> My first question is if such similar examples are useful or they will\n> become a maintainership burden.\n> \n> From my side, the more examples the better, but I think the question\n> if it's woth it stays..\n\nWe can always change and drop these later.\n\nBut my reasoning was summarized in the intro letter. I think this and \nsimple-capture.py are quite different. Maybe this could be renamed. \nsimple-capture-app.py?\n\n>> Signed-off-by: Tomi Valkeinen \n>> ---\n>> src/py/examples/simple-continuous-capture.py | 189 +++++++++++++++++++\n>> 1 file changed, 189 insertions(+)\n>> create mode 100755 src/py/examples/simple-continuous-capture.py\n>>\n>> diff --git a/src/py/examples/simple-continuous-capture.py b/src/py/examples/simple-continuous-capture.py\n>> new file mode 100755\n>> index 00000000..d0f8a7e9\n>> --- /dev/null\n>> +++ b/src/py/examples/simple-continuous-capture.py\n>> @@ -0,0 +1,189 @@\n>> +#!/usr/bin/env python3\n>> +\n>> +# SPDX-License-Identifier: BSD-3-Clause\n>> +# Copyright (C) 2022, Tomi Valkeinen \n>> +\n>> +# A simple capture example extending the simple-capture.py example:\n>> +# - Capture frames using events from multiple cameras\n>> +# - Listening events from stdin to exit the application\n>> +# - Memory mapping the frames and calculating CRC\n>> +\n>> +import binascii\n>> +import libcamera as libcam\n>> +import libcamera.utils\n>> +import selectors\n>> +import sys\n>> +\n>> +\n>> +# A container class for our state per camera\n>> +class CameraCaptureContext:\n>> + idx: int\n>> + cam: libcam.Camera\n>> + reqs: list[libcam.Request]\n>> + mfbs: dict[libcam.FrameBuffer, libcamera.utils.MappedFrameBuffer]\n>> +\n>> + def __init__(self, cam, idx):\n>> + # Acquire the camera for our use\n>> +\n>> + ret = cam.acquire()\n>> + assert ret == 0\n>> +\n>> + # Configure the camera\n>> +\n>> + cam_config = cam.generate_configuration([libcam.StreamRole.Viewfinder])\n>> +\n>> + stream_config = cam_config.at(0)\n>> +\n>> + ret = cam.configure(cam_config)\n>> + assert ret == 0\n>> +\n>> + stream = stream_config.stream\n>> +\n>> + # Allocate the buffers for capture\n>> +\n>> + allocator = libcam.FrameBufferAllocator(cam)\n>> + ret = allocator.allocate(stream)\n>> + assert ret > 0\n>> +\n>> + num_bufs = len(allocator.buffers(stream))\n>> +\n>> + print(f'cam{idx} ({cam.id}): capturing {num_bufs} buffers with {stream_config}')\n>> +\n>> + # Create the requests and assign a buffer for each request\n>> +\n>> + reqs = []\n>> + for i in range(num_bufs):\n>> + # Use the buffer index as the \"cookie\"\n>> + req = cam.create_request(idx)\n>> +\n>> + buffer = allocator.buffers(stream)[i]\n>> + ret = req.add_buffer(stream, buffer)\n>> + assert ret == 0\n>> +\n>> + reqs.append(req)\n>> +\n>> + self.idx = idx\n>> + self.cam = cam\n>> + self.reqs = reqs\n>> + self.mfbs = dict([(fb, libcamera.utils.MappedFrameBuffer(fb).mmap()) for fb in allocator.buffers(stream)])\n> \n> Not that relevant, but you could populate self.reqs directly and\n> append a {buffer, libcamera.utils.MappedFrameBuffer(fb).mmap()} pair\n> to the self.mfbs dictionary in the request creation loop ?\n\nYes, you're right. It makes the code easier to read.\n\n>> +\n>> + def uninit_camera(self):\n>> + # Stop the camera\n>> +\n>> + ret = self.cam.stop()\n>> + assert ret == 0\n>> +\n>> + # Release the camera\n>> +\n>> + ret = self.cam.release()\n>> + assert ret == 0\n>> +\n>> +\n>> +# A container class for our state\n>> +class CaptureContext:\n>> + cm: libcam.CameraManager\n>> + camera_contexts: list[CameraCaptureContext] = []\n>> +\n>> + def handle_camera_event(self):\n>> + # cm.get_ready_requests() will not block here, as we know there is an event\n>> + # to read.\n>> +\n>> + reqs = self.cm.get_ready_requests()\n>> +\n>> + # Process the captured frames\n>> +\n>> + for req in reqs:\n>> + self.handle_request(req)\n>> +\n>> + return True\n>> +\n>> + def handle_request(self, req: libcam.Request):\n>> + cam_ctx = self.camera_contexts[req.cookie]\n>> +\n>> + buffers = req.buffers\n>> +\n>> + assert len(buffers) == 1\n>> +\n>> + # A ready Request could contain multiple buffers if multiple streams\n>> + # were being used. Here we know we only have a single stream,\n>> + # and we use next(iter()) to get the first and only buffer.\n>> +\n>> + stream, fb = next(iter(buffers.items()))\n>> +\n>> + # Use the MappedFrameBuffer to access the pixel data with CPU. We calculate\n>> + # the crc for each plane.\n>> +\n>> + mfb = cam_ctx.mfbs[fb]\n>> + crcs = [binascii.crc32(p) for p in mfb.planes]\n>> +\n>> + meta = fb.metadata\n>> +\n>> + print('cam{:<6} seq {:<6} bytes {:10} CRCs {}'\n>> + .format(cam_ctx.idx,\n>> + meta.sequence,\n>> + '/'.join([str(p.bytes_used) for p in meta.planes]),\n>> + crcs))\n>> +\n>> + # We want to re-queue the buffer we just handled. Instead of creating\n>> + # a new Request, we re-use the old one. We need to call req.reuse()\n>> + # to re-initialize the Request before queuing.\n>> +\n>> + req.reuse()\n>> + cam_ctx.cam.queue_request(req)\n>> +\n>> + def capture(self):\n>> + # Queue the requests to the camera\n>> +\n>> + for cam_ctx in self.camera_contexts:\n>> + for req in cam_ctx.reqs:\n>> + ret = cam_ctx.cam.queue_request(req)\n>> + assert ret == 0\n>> +\n>> + # Use Selector to wait for events from the camera and from the keyboard\n>> +\n>> + sel = selectors.DefaultSelector()\n>> + sel.register(sys.stdin, selectors.EVENT_READ, handle_key_event)\n>> + sel.register(self.cm.event_fd, selectors.EVENT_READ, lambda: self.handle_camera_event())\n>> +\n>> + running = True\n>> +\n>> + while running:\n>> + events = sel.select()\n>> + for key, mask in events:\n>> + # If the handler return False, we should exit\n>> + if not key.data():\n>> + running = False\n>> +\n>> +\n>> +def handle_key_event():\n>> + sys.stdin.readline()\n>> + print('Exiting...')\n>> + return False\n> \n> Nit: why is this a global function handle_camera_event() a class\n> member ?\n\nHmm, yes, I think I can move this inside CaptureContext too.\n\n> To be honest I would get rid of the CaptureContext class and open-code\n> CaptureContext::capture() in your main function, with all the handlers\n> being global functions. Not a big deal though\n\nI wanted to structure this example to look a bit more like a bigger \napplication, even if it's not really needed in this example.\n\n>> +\n>> +\n>> +def main():\n>> + cm = libcam.CameraManager.singleton()\n>> +\n>> + ctx = CaptureContext()\n>> + ctx.cm = cm\n>> +\n>> + for idx, cam in enumerate(cm.cameras):\n>> + cam_ctx = CameraCaptureContext(cam, idx)\n> \n> Can't you start the camera here ?\n\nI could. My thinking was that you'd first collect the cameras and \nconfigure them, and if everything is still good, then you go and start \nthem. Is there a reason you think it's better to start here?\n\n\n>> + ctx.camera_contexts.append(cam_ctx)\n>> +\n>> + # Start the cameras\n>> +\n>> + for cam_ctx in ctx.camera_contexts:\n>> + ret = cam_ctx.cam.start()\n>> + assert ret == 0\n>> +\n>> + ctx.capture()\n>> +\n>> + for cam_ctx in ctx.camera_contexts:\n>> + cam_ctx.uninit_camera()\n>> +\n>> + return 0\n>> +\n>> +\n>> +if __name__ == '__main__':\n>> + sys.exit(main())\n> \n> Nits apart\n> Reviewed-by: Jacopo Mondi \n\nThanks!\n\n Tomi","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 80879BD161\n\tfor ;\n\tMon, 6 Jun 2022 08:56:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CF7E965635;\n\tMon, 6 Jun 2022 10:56:16 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6E8BF633A4\n\tfor ;\n\tMon, 6 Jun 2022 10:56:15 +0200 (CEST)","from [192.168.1.111] (91-156-85-209.elisa-laajakaista.fi\n\t[91.156.85.209])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9DA4530A;\n\tMon, 6 Jun 2022 10:56:14 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1654505776;\n\tbh=/O4y1D8xz58Hjzz1YQUntfzWs1SDcurXar+fdhn96T0=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=UhilNpJFkkG+lqFckS55OaOC6EfvvM2TCRs0Yvba9KMZaZTrtqEUq4ivNJbG+reHI\n\t74ftmGE5+PU3PxdtyDOXY6IcdPwFRCMgo5beidNG+WsLTVtTq/Vo1zDMLchPuv23jG\n\toc8e0C/Iqi+/XQq312Yux3mK5qPc0HWnWzIWnnuSf9giX4IEk0GLKBxMpc5cRR2yHm\n\t2u9NWOvwy8uETQNrCzqW9dxS99/O3bXZ5iAe0D0DEJqCnYtO+dgkHm+Ugb2Q/jxkXu\n\t1FqKPvpnxsf+P6T8e7UdzHrkUkMg29cdanzSqx2PtTTCkQYk98kuzvnEGMbmRBdg3O\n\ty/Aq1EwCLNIwg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1654505774;\n\tbh=/O4y1D8xz58Hjzz1YQUntfzWs1SDcurXar+fdhn96T0=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=rkosBdyiKD33/dhek6pF7QkOMgxi82EIJcMu/+bO6+oQXE/ssrDFqvGL/livwVovI\n\tmMmSYZSfWc/NChKEdVdm1SHMYj/ji3oH+28oHXFUWvSGcExgcMiOl+hYcWQAtyYSfS\n\ttqwj/lr+eEtLObIlwoW3N0JqnNx/5uK8C/KV7Mf4="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"rkosBdyi\"; dkim-atps=neutral","Message-ID":"<0a1f10d0-3769-51ab-9d6a-89d1908f9a0d@ideasonboard.com>","Date":"Mon, 6 Jun 2022 11:56:11 +0300","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.9.1","Content-Language":"en-US","To":"Jacopo Mondi ","References":"<20220530142722.57618-1-tomi.valkeinen@ideasonboard.com>\n\t<20220530142722.57618-15-tomi.valkeinen@ideasonboard.com>\n\t<20220605123156.j6veszk3zqsdmsfz@uno.localdomain>","In-Reply-To":"<20220605123156.j6veszk3zqsdmsfz@uno.localdomain>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v4 14/16] py: examples: Add\n\tsimple-continuous-capture.py","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Tomi Valkeinen via libcamera-devel\n\t","Reply-To":"Tomi Valkeinen ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23337,"web_url":"https://patchwork.libcamera.org/comment/23337/","msgid":"<20220606091901.y7djqd3tuw4nku4a@uno.localdomain>","date":"2022-06-06T09:19:01","subject":"Re: [libcamera-devel] [PATCH v4 14/16] py: examples: Add\n\tsimple-continuous-capture.py","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Tomi\n\nOn Mon, Jun 06, 2022 at 11:56:11AM +0300, Tomi Valkeinen wrote:\n> On 05/06/2022 15:31, Jacopo Mondi wrote:\n> > Hi Tomi,\n> >\n> > On Mon, May 30, 2022 at 05:27:20PM +0300, Tomi Valkeinen wrote:\n> > > Add a slightly more complex, and I think a more realistic, example,\n> > > where the script reacts to events and re-queues the buffers.\n> > >\n> >\n> > My first question is if such similar examples are useful or they will\n> > become a maintainership burden.\n> >\n> > From my side, the more examples the better, but I think the question\n> > if it's woth it stays..\n>\n> We can always change and drop these later.\n>\n> But my reasoning was summarized in the intro letter. I think this and\n> simple-capture.py are quite different. Maybe this could be renamed.\n> simple-capture-app.py?\n>\n> > > Signed-off-by: Tomi Valkeinen \n> > > ---\n> > > src/py/examples/simple-continuous-capture.py | 189 +++++++++++++++++++\n> > > 1 file changed, 189 insertions(+)\n> > > create mode 100755 src/py/examples/simple-continuous-capture.py\n> > >\n> > > diff --git a/src/py/examples/simple-continuous-capture.py b/src/py/examples/simple-continuous-capture.py\n> > > new file mode 100755\n> > > index 00000000..d0f8a7e9\n> > > --- /dev/null\n> > > +++ b/src/py/examples/simple-continuous-capture.py\n> > > @@ -0,0 +1,189 @@\n> > > +#!/usr/bin/env python3\n> > > +\n> > > +# SPDX-License-Identifier: BSD-3-Clause\n> > > +# Copyright (C) 2022, Tomi Valkeinen \n> > > +\n> > > +# A simple capture example extending the simple-capture.py example:\n> > > +# - Capture frames using events from multiple cameras\n> > > +# - Listening events from stdin to exit the application\n> > > +# - Memory mapping the frames and calculating CRC\n> > > +\n> > > +import binascii\n> > > +import libcamera as libcam\n> > > +import libcamera.utils\n> > > +import selectors\n> > > +import sys\n> > > +\n> > > +\n> > > +# A container class for our state per camera\n> > > +class CameraCaptureContext:\n> > > + idx: int\n> > > + cam: libcam.Camera\n> > > + reqs: list[libcam.Request]\n> > > + mfbs: dict[libcam.FrameBuffer, libcamera.utils.MappedFrameBuffer]\n> > > +\n> > > + def __init__(self, cam, idx):\n> > > + # Acquire the camera for our use\n> > > +\n> > > + ret = cam.acquire()\n> > > + assert ret == 0\n> > > +\n> > > + # Configure the camera\n> > > +\n> > > + cam_config = cam.generate_configuration([libcam.StreamRole.Viewfinder])\n> > > +\n> > > + stream_config = cam_config.at(0)\n> > > +\n> > > + ret = cam.configure(cam_config)\n> > > + assert ret == 0\n> > > +\n> > > + stream = stream_config.stream\n> > > +\n> > > + # Allocate the buffers for capture\n> > > +\n> > > + allocator = libcam.FrameBufferAllocator(cam)\n> > > + ret = allocator.allocate(stream)\n> > > + assert ret > 0\n> > > +\n> > > + num_bufs = len(allocator.buffers(stream))\n> > > +\n> > > + print(f'cam{idx} ({cam.id}): capturing {num_bufs} buffers with {stream_config}')\n> > > +\n> > > + # Create the requests and assign a buffer for each request\n> > > +\n> > > + reqs = []\n> > > + for i in range(num_bufs):\n> > > + # Use the buffer index as the \"cookie\"\n> > > + req = cam.create_request(idx)\n> > > +\n> > > + buffer = allocator.buffers(stream)[i]\n> > > + ret = req.add_buffer(stream, buffer)\n> > > + assert ret == 0\n> > > +\n> > > + reqs.append(req)\n> > > +\n> > > + self.idx = idx\n> > > + self.cam = cam\n> > > + self.reqs = reqs\n> > > + self.mfbs = dict([(fb, libcamera.utils.MappedFrameBuffer(fb).mmap()) for fb in allocator.buffers(stream)])\n> >\n> > Not that relevant, but you could populate self.reqs directly and\n> > append a {buffer, libcamera.utils.MappedFrameBuffer(fb).mmap()} pair\n> > to the self.mfbs dictionary in the request creation loop ?\n>\n> Yes, you're right. It makes the code easier to read.\n>\n> > > +\n> > > + def uninit_camera(self):\n> > > + # Stop the camera\n> > > +\n> > > + ret = self.cam.stop()\n> > > + assert ret == 0\n> > > +\n> > > + # Release the camera\n> > > +\n> > > + ret = self.cam.release()\n> > > + assert ret == 0\n> > > +\n> > > +\n> > > +# A container class for our state\n> > > +class CaptureContext:\n> > > + cm: libcam.CameraManager\n> > > + camera_contexts: list[CameraCaptureContext] = []\n> > > +\n> > > + def handle_camera_event(self):\n> > > + # cm.get_ready_requests() will not block here, as we know there is an event\n> > > + # to read.\n> > > +\n> > > + reqs = self.cm.get_ready_requests()\n> > > +\n> > > + # Process the captured frames\n> > > +\n> > > + for req in reqs:\n> > > + self.handle_request(req)\n> > > +\n> > > + return True\n> > > +\n> > > + def handle_request(self, req: libcam.Request):\n> > > + cam_ctx = self.camera_contexts[req.cookie]\n> > > +\n> > > + buffers = req.buffers\n> > > +\n> > > + assert len(buffers) == 1\n> > > +\n> > > + # A ready Request could contain multiple buffers if multiple streams\n> > > + # were being used. Here we know we only have a single stream,\n> > > + # and we use next(iter()) to get the first and only buffer.\n> > > +\n> > > + stream, fb = next(iter(buffers.items()))\n> > > +\n> > > + # Use the MappedFrameBuffer to access the pixel data with CPU. We calculate\n> > > + # the crc for each plane.\n> > > +\n> > > + mfb = cam_ctx.mfbs[fb]\n> > > + crcs = [binascii.crc32(p) for p in mfb.planes]\n> > > +\n> > > + meta = fb.metadata\n> > > +\n> > > + print('cam{:<6} seq {:<6} bytes {:10} CRCs {}'\n> > > + .format(cam_ctx.idx,\n> > > + meta.sequence,\n> > > + '/'.join([str(p.bytes_used) for p in meta.planes]),\n> > > + crcs))\n> > > +\n> > > + # We want to re-queue the buffer we just handled. Instead of creating\n> > > + # a new Request, we re-use the old one. We need to call req.reuse()\n> > > + # to re-initialize the Request before queuing.\n> > > +\n> > > + req.reuse()\n> > > + cam_ctx.cam.queue_request(req)\n> > > +\n> > > + def capture(self):\n> > > + # Queue the requests to the camera\n> > > +\n> > > + for cam_ctx in self.camera_contexts:\n> > > + for req in cam_ctx.reqs:\n> > > + ret = cam_ctx.cam.queue_request(req)\n> > > + assert ret == 0\n> > > +\n> > > + # Use Selector to wait for events from the camera and from the keyboard\n> > > +\n> > > + sel = selectors.DefaultSelector()\n> > > + sel.register(sys.stdin, selectors.EVENT_READ, handle_key_event)\n> > > + sel.register(self.cm.event_fd, selectors.EVENT_READ, lambda: self.handle_camera_event())\n> > > +\n> > > + running = True\n> > > +\n> > > + while running:\n> > > + events = sel.select()\n> > > + for key, mask in events:\n> > > + # If the handler return False, we should exit\n> > > + if not key.data():\n> > > + running = False\n> > > +\n> > > +\n> > > +def handle_key_event():\n> > > + sys.stdin.readline()\n> > > + print('Exiting...')\n> > > + return False\n> >\n> > Nit: why is this a global function handle_camera_event() a class\n> > member ?\n>\n> Hmm, yes, I think I can move this inside CaptureContext too.\n>\n> > To be honest I would get rid of the CaptureContext class and open-code\n> > CaptureContext::capture() in your main function, with all the handlers\n> > being global functions. Not a big deal though\n>\n> I wanted to structure this example to look a bit more like a bigger\n> application, even if it's not really needed in this example.\n>\n> > > +\n> > > +\n> > > +def main():\n> > > + cm = libcam.CameraManager.singleton()\n> > > +\n> > > + ctx = CaptureContext()\n> > > + ctx.cm = cm\n> > > +\n> > > + for idx, cam in enumerate(cm.cameras):\n> > > + cam_ctx = CameraCaptureContext(cam, idx)\n> >\n> > Can't you start the camera here ?\n>\n> I could. My thinking was that you'd first collect the cameras and configure\n> them, and if everything is still good, then you go and start them. Is there\n> a reason you think it's better to start here?\n\nNot particularly, but seeing an additional loop had me wonder if there\nwas a reason to do so or not, as the CameraCaptureContext() is created\nunconditionally. Even more, the configuration correctness is actually\nasserted, which I assume will raise an exception that will terminate\nthe program and will not simply skip the faulty camera..\n\nThanks\n j\n>\n>\n> > > + ctx.camera_contexts.append(cam_ctx)\n> > > +\n> > > + # Start the cameras\n> > > +\n> > > + for cam_ctx in ctx.camera_contexts:\n> > > + ret = cam_ctx.cam.start()\n> > > + assert ret == 0\n> > > +\n> > > + ctx.capture()\n> > > +\n> > > + for cam_ctx in ctx.camera_contexts:\n> > > + cam_ctx.uninit_camera()\n> > > +\n> > > + return 0\n> > > +\n> > > +\n> > > +if __name__ == '__main__':\n> > > + sys.exit(main())\n> >\n> > Nits apart\n> > Reviewed-by: Jacopo Mondi \n>\n> Thanks!\n>\n> Tomi","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 2AFEDBD161\n\tfor ;\n\tMon, 6 Jun 2022 09:19:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6652865637;\n\tMon, 6 Jun 2022 11:19:04 +0200 (CEST)","from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C309E633A4\n\tfor ;\n\tMon, 6 Jun 2022 11:19:03 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id B63464000E;\n\tMon, 6 Jun 2022 09:19:02 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1654507144;\n\tbh=Ygs90mmiu4eHRyvKzSEgtEiRvwf4QzWb8ul4kUAABvM=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=cD2sHAFaffh6QK+S3Zyp+TqB3We8oM0KcFHFD1l9DIEKOL7J7JWvh/mob24mIIAzx\n\tD1igY/Yml5ze3tDJxcui/EvJvtAm4SfZWBSNxHq7rWWP7gh3ieS82vbGye8n17oCg6\n\tA/h0n68PFKrf+YdwQhGKxOURrkekSMfjXZym04WjhaWczCtMSredxeYHKtAuDDHlcU\n\tJ1nc5PeK88vTKPvVq91ns1PCsvYAxhkBH5Va/icF83WHhd3pY/3a147ZJNPwBGlyC8\n\tUnOCmMFlqUlswtN9z381XQMMUauk7LocbUHfSoqDznLQcepDXcqd1QVZgVoG1DS/+r\n\t2ZrhBgw/XuqRw==","Date":"Mon, 6 Jun 2022 11:19:01 +0200","To":"Tomi Valkeinen ","Message-ID":"<20220606091901.y7djqd3tuw4nku4a@uno.localdomain>","References":"<20220530142722.57618-1-tomi.valkeinen@ideasonboard.com>\n\t<20220530142722.57618-15-tomi.valkeinen@ideasonboard.com>\n\t<20220605123156.j6veszk3zqsdmsfz@uno.localdomain>\n\t<0a1f10d0-3769-51ab-9d6a-89d1908f9a0d@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<0a1f10d0-3769-51ab-9d6a-89d1908f9a0d@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 14/16] py: examples: Add\n\tsimple-continuous-capture.py","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]