[{"id":27238,"web_url":"https://patchwork.libcamera.org/comment/27238/","msgid":"<20230605051509.GC22604@pendragon.ideasonboard.com>","date":"2023-06-05T05:15:09","subject":"Re: [libcamera-devel] [PATCH v5 06/13] py: simple-capture.py: Use\n\tnew events support","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Tomi,\n\nThank you for the patch.\n\nOn Sat, Jun 03, 2023 at 10:56:08AM +0300, Tomi Valkeinen wrote:\n> Update simple-capture.py to the new event model.\n> \n> Signed-off-by: Tomi Valkeinen \n> Reviewed-by: Jacopo Mondi \n> Reviewed-by: Laurent Pinchart \n> ---\n> src/py/examples/simple-capture.py | 15 ++++++++++-----\n> 1 file changed, 10 insertions(+), 5 deletions(-)\n> \n> diff --git a/src/py/examples/simple-capture.py b/src/py/examples/simple-capture.py\n> index 4b85408f..4fa36d6f 100755\n> --- a/src/py/examples/simple-capture.py\n> +++ b/src/py/examples/simple-capture.py\n> @@ -107,17 +107,22 @@ def main():\n> sel.register(cm.event_fd, selectors.EVENT_READ)\n> \n> while frames_done < TOTAL_FRAMES:\n> - # cm.get_ready_requests() does not block, so we use a Selector to wait\n> - # for a camera event. Here we should almost always get a single\n> - # Request, but in some cases there could be multiple or none.\n> + # cm.get_events() does not block, so we use a Selector to wait for a\n> + # camera event. Here we should almost always get a single request\n> + # completion event, but in some cases there could be multiple ones,\n> + # other events, or no events at all.\n\nI think this will be confusing to people reading the code, as you\nmention there can be cases that depart from the norm, but don't explain\nwhy. simple-capture.py is meant as a simple example so I expect\ndevelopers to read the code. Let' make it crystal clear :-)\n\n> \n> events = sel.select()\n> if not events:\n> continue\n\nAlso, you're handling two types of events, the ones returned by the\nselector, and the ones returned by get_events(). It's not clear which of\nthose two the comment above relates to. For instance, when you say \"no\nevents at all\", is that for selector events, camera manager events, or\nboth ? This needs to be clarified.\n\n> \n> - reqs = cm.get_ready_requests()\n> + for ev in cm.get_events():\n> + # We are only interested in RequestCompleted events\n> + if ev.type != libcam.Event.Type.RequestCompleted:\n> + continue\n> +\n> + req = ev.request\n> \n> - for req in reqs:\n> frames_done += 1\n> \n> buffers = req.buffers","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 84624C3200\n\tfor ;\n\tMon, 5 Jun 2023 05:15:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E700462728;\n\tMon, 5 Jun 2023 07:15:11 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F2F2962722\n\tfor ;\n\tMon, 5 Jun 2023 07:15:10 +0200 (CEST)","from pendragon.ideasonboard.com (om126156242094.26.openmobile.ne.jp\n\t[126.156.242.94])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 427681BA;\n\tMon, 5 Jun 2023 07:14:44 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1685942112;\n\tbh=PzJsnWit7mPJ7lZbY6liRRE2JyPLE67pBbmL9c9VLrk=;\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=CGNlPdcoPnq7uJkUzuJyXO0Wyh9Fy53D5YR9pJ+e7m0Vo0ZMP89iIHFZQMJFT3ezR\n\t/mwf/8cdFeMQWfV/H5EFWrHgzMd/JoTWlR9chkBlZLamWL+6eEIqM0TKVhjUvDYvEb\n\tdcY9KnM8soNw5Fvfa6fad5eXZomaoRJB/8k7+lUjEk1vTkI5Ge8Gx523p9yj3YUaF6\n\tjnSdlMGT6ENHMqYzUEnmhXXKYrdForzWJ2aypEMFx/J/Xw1VW6BUlReWjMsc6hUKCi\n\tUjz2IB6dDaE57FeMvAxnfQ10OCesHrAh9B13TRASy+/AymAMWXqzZkUl94PtoznFFi\n\tjDEBn1LRg7q7Q==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1685942086;\n\tbh=PzJsnWit7mPJ7lZbY6liRRE2JyPLE67pBbmL9c9VLrk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=PL8FctjRddtWWyYVAzph0iRgSDFLdLXH4uQmxMLDUNSmVFAo8uQp/aBhygBScxxbI\n\tiUx+1NYaRg6WqBA183Fn4waloQRY/0VADQLPqEMWziRWsuhFpJ9jqS2iJ7+Ac5YCL/\n\t9hnFa0Mh8+DmcGCnrMF7kbqEeHZiq2Xlf5vga20I="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"PL8FctjR\"; dkim-atps=neutral","Date":"Mon, 5 Jun 2023 08:15:09 +0300","To":"Tomi Valkeinen ","Message-ID":"<20230605051509.GC22604@pendragon.ideasonboard.com>","References":"<20230603075615.20663-1-tomi.valkeinen@ideasonboard.com>\n\t<20230603075615.20663-7-tomi.valkeinen@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230603075615.20663-7-tomi.valkeinen@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v5 06/13] py: simple-capture.py: Use\n\tnew events support","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":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":27254,"web_url":"https://patchwork.libcamera.org/comment/27254/","msgid":"<3ca2709a-8165-0ccd-de59-0a888cc51ea5@ideasonboard.com>","date":"2023-06-05T10:43:15","subject":"Re: [libcamera-devel] [PATCH v5 06/13] py: simple-capture.py: Use\n\tnew events support","submitter":{"id":109,"url":"https://patchwork.libcamera.org/api/people/109/","name":"Tomi Valkeinen","email":"tomi.valkeinen@ideasonboard.com"},"content":"On 05/06/2023 08:15, Laurent Pinchart wrote:\n> Hi Tomi,\n> \n> Thank you for the patch.\n> \n> On Sat, Jun 03, 2023 at 10:56:08AM +0300, Tomi Valkeinen wrote:\n>> Update simple-capture.py to the new event model.\n>>\n>> Signed-off-by: Tomi Valkeinen \n>> Reviewed-by: Jacopo Mondi \n>> Reviewed-by: Laurent Pinchart \n>> ---\n>> src/py/examples/simple-capture.py | 15 ++++++++++-----\n>> 1 file changed, 10 insertions(+), 5 deletions(-)\n>>\n>> diff --git a/src/py/examples/simple-capture.py b/src/py/examples/simple-capture.py\n>> index 4b85408f..4fa36d6f 100755\n>> --- a/src/py/examples/simple-capture.py\n>> +++ b/src/py/examples/simple-capture.py\n>> @@ -107,17 +107,22 @@ def main():\n>> sel.register(cm.event_fd, selectors.EVENT_READ)\n>> \n>> while frames_done < TOTAL_FRAMES:\n>> - # cm.get_ready_requests() does not block, so we use a Selector to wait\n>> - # for a camera event. Here we should almost always get a single\n>> - # Request, but in some cases there could be multiple or none.\n>> + # cm.get_events() does not block, so we use a Selector to wait for a\n>> + # camera event. Here we should almost always get a single request\n>> + # completion event, but in some cases there could be multiple ones,\n>> + # other events, or no events at all.\n> \n> I think this will be confusing to people reading the code, as you\n> mention there can be cases that depart from the norm, but don't explain\n> why. simple-capture.py is meant as a simple example so I expect\n> developers to read the code. Let' make it crystal clear :-)\n\nYep. A bad comment is worse than no comment at all =).\n\n>> \n>> events = sel.select()\n>> if not events:\n>> continue\n> \n> Also, you're handling two types of events, the ones returned by the\n> selector, and the ones returned by get_events(). It's not clear which of\n> those two the comment above relates to. For instance, when you say \"no\n> events at all\", is that for selector events, camera manager events, or\n> both ? This needs to be clarified.\n\nRight, annoyingly we have two types of (very different) events. I'll try \nto make it clear which I'm referring to.\n\n>> \n>> - reqs = cm.get_ready_requests()\n>> + for ev in cm.get_events():\n>> + # We are only interested in RequestCompleted events\n>> + if ev.type != libcam.Event.Type.RequestCompleted:\n>> + continue\n>> +\n>> + req = ev.request\n>> \n>> - for req in reqs:\n>> frames_done += 1\n>> \n>> buffers = req.buffers\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 BB890C3200\n\tfor ;\n\tMon, 5 Jun 2023 10:43:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 295746287D;\n\tMon, 5 Jun 2023 12:43:21 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7927D62709\n\tfor ;\n\tMon, 5 Jun 2023 12:43:19 +0200 (CEST)","from [192.168.88.20] (91-154-35-171.elisa-laajakaista.fi\n\t[91.154.35.171])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 47FB383F;\n\tMon, 5 Jun 2023 12:42:54 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1685961801;\n\tbh=6wuHu6VWwdJht+XcfQqKQya1QaJDw+RgaCw8TpKCM9w=;\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=BPVpAlqqqnQ9fnlgUyqUYKqKoEATPQHOt9po+R/TYebBQhLuYCdWi3SBSozB52JWI\n\tHWdaxrhyVHtiwceEbFHjaxrndzHPcWKDB+2qPrvWzpX456A95PwJgbCP3iWlotUrm2\n\t+ALyWaz11wK4++hL/j78p1bG/aBd2lv5i/wNZwp/b5aM2M4pZglFOJHbJ+jd/OJ4af\n\tP/sztJjXZM5Gxe7tdrp2hrCU35A+WMZFpTKmz9MX4Kf0J1ICLGp+nkio5dQbnqGIue\n\tXzzwmZHZ/bBdVJypV2aKYCiHiZbSLswHrvdNTNLLZpjVJBsg0o0SYA+vpFWXCLAxYD\n\t0KTJ7fhJr03xA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1685961774;\n\tbh=6wuHu6VWwdJht+XcfQqKQya1QaJDw+RgaCw8TpKCM9w=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=aKBhOzNbzFAJp/mdW2krsRAD80LcJRfkf29c1QwjEtb8iqWHnIDWIrBjIt9XtAo+3\n\t2YqiWDZyV/RjmeC/S1HO8RZQjZJjhLN9R5j/gcXhtUEgNVpYFEKSiexgyMTFpzriXH\n\t4WEOdTuFXVIichVIPbE1xRFCo/ttosLM4MlcUbd4="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"aKBhOzNb\"; dkim-atps=neutral","Message-ID":"<3ca2709a-8165-0ccd-de59-0a888cc51ea5@ideasonboard.com>","Date":"Mon, 5 Jun 2023 13:43:15 +0300","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101\n\tThunderbird/102.11.0","Content-Language":"en-US","To":"Laurent Pinchart ","References":"<20230603075615.20663-1-tomi.valkeinen@ideasonboard.com>\n\t<20230603075615.20663-7-tomi.valkeinen@ideasonboard.com>\n\t<20230605051509.GC22604@pendragon.ideasonboard.com>","In-Reply-To":"<20230605051509.GC22604@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v5 06/13] py: simple-capture.py: Use\n\tnew events support","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\" "}}]