[{"id":27237,"web_url":"https://patchwork.libcamera.org/comment/27237/","msgid":"<20230605051028.GB22604@pendragon.ideasonboard.com>","date":"2023-06-05T05:10:28","subject":"Re: [libcamera-devel] [PATCH v5 04/13] py: cam.py: Use new events\n\tsupport","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:06AM +0300, Tomi Valkeinen wrote:\n> Convert cam.py to use the new event dispatching. In addition to handling\n> the request-completed event, handle also disconnect, camera-added and\n> camera-removed events (which only do a simple print).\n> \n> Signed-off-by: Tomi Valkeinen \n> Reviewed-by: Jacopo Mondi \n> Reviewed-by: Laurent Pinchart \n> ---\n> src/py/cam/cam.py | 27 +++++++++++++++++++++------\n> 1 file changed, 21 insertions(+), 6 deletions(-)\n> \n> diff --git a/src/py/cam/cam.py b/src/py/cam/cam.py\n> index a2a115c1..1e2d1f69 100755\n> --- a/src/py/cam/cam.py\n> +++ b/src/py/cam/cam.py\n> @@ -230,11 +230,19 @@ class CaptureState:\n> # Called from renderer when there is a libcamera event\n> def event_handler(self):\n> try:\n> - reqs = self.cm.get_ready_requests()\n> -\n> - for req in reqs:\n> - ctx = next(ctx for ctx in self.contexts if ctx.idx == req.cookie)\n> - self.__request_handler(ctx, req)\n> + for ev in self.cm.get_events():\n> + type = ev.type\n> +\n> + if type == libcam.Event.Type.CameraAdded:\n> + print(f'Camera {ev.camera} added')\n> + elif type == libcam.Event.Type.CameraRemoved:\n> + print(f'Camera {ev.camera} removed')\n> + elif type == libcam.Event.Type.Disconnect:\n> + print(f'Camera {ev.camera} disconnected')\n> + elif type == libcam.Event.Type.RequestCompleted:\n> + self.__request_handler(ev.camera, ev.request)\n> + else:\n> + raise RuntimeError(\"Bad event type\")\n\nThis will cause issues if we later add new event types. Wouldn't it be\nbetter to ignore unknown event types, or possibly print a (one-time)\nwarning message ?\n\n> \n> running = any(ctx.reqs_completed < ctx.opt_capture for ctx in self.contexts)\n> return running\n> @@ -242,7 +250,9 @@ class CaptureState:\n> traceback.print_exc()\n> return False\n> \n> - def __request_handler(self, ctx, req):\n> + def __request_handler(self, cam, req):\n> + ctx = next(ctx for ctx in self.contexts if ctx.camera == cam)\n> +\n> if req.status != libcam.Request.Status.Complete:\n> raise Exception('{}: Request failed: {}'.format(ctx.id, req.status))\n> \n> @@ -447,6 +457,11 @@ def main():\n> \n> state.do_cmd_capture()\n> \n> + # This is not strictly needed, but it helps to do a proper cleanup as we\n> + # drop any unhandled events, and so makes it easier to use memory leak\n> + # detectors.\n> + cm.get_events()\n\nSomehow this feels like a hack, outlining a design issue. Is there any\nway we could do better ? How about clearing events in the camera manager\ndestructor ?\n\n> +\n> return 0\n> \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 6E31EC31E9\n\tfor ;\n\tMon, 5 Jun 2023 05:10:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D828661EA2;\n\tMon, 5 Jun 2023 07:10:35 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B66F561EA2\n\tfor ;\n\tMon, 5 Jun 2023 07:10:30 +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 D3A8A1BA;\n\tMon, 5 Jun 2023 07:10:04 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1685941835;\n\tbh=vdaffF3L0g0DHryhb5b3W1sOUQoDjT6uxot/iBzc+6w=;\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=g3PCHskEaV99PHvbOMPD97IUsm2qa3X/PZyPngSfN/fdjB0BJTmY6wG8i+ejJs+bG\n\tkKp5qpHzN+/D/L+CUvGhwn7HeHmmdPXPMKVO1EJ4HnD1FtDlS5TG7KwX6Mq7r5Q0lt\n\tzBO4i+SO1AC7/TeKia6AgNLgcV09kcZiJBsEEtJC9bzVKBnSa78et6ZiEWIRs8csf4\n\tK0FXSMuk/Bq6pRz88zJJtHBybusctWNmkQPVGl5iloz6HjJsVgbY43ZACFoobBZAWi\n\tgMNggqG9f7QjyeU4GV+PR2Ka3Zn2G0jLTOyehDZlKMS+lE/nRd/OivsLmItEeAFZ7e\n\tDXXKdwpatAJqw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1685941805;\n\tbh=vdaffF3L0g0DHryhb5b3W1sOUQoDjT6uxot/iBzc+6w=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=bH45i+e8kM2+jKYjDiGB0nlXI66JYUnXTZlWf6nV5zV+8di7q95WcHOCa/e6tRB51\n\tuSlHo2tVne/4jv+I/ixbH5Oj4aZsHWBUPAq1I1YJGgI6Po8dflj1LC/e+v6VY6l/O6\n\tQWQZV2rRv5GRduM86M7woc5HJ1CHQsuqd70+iuB0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"bH45i+e8\"; dkim-atps=neutral","Date":"Mon, 5 Jun 2023 08:10:28 +0300","To":"Tomi Valkeinen ","Message-ID":"<20230605051028.GB22604@pendragon.ideasonboard.com>","References":"<20230603075615.20663-1-tomi.valkeinen@ideasonboard.com>\n\t<20230603075615.20663-5-tomi.valkeinen@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230603075615.20663-5-tomi.valkeinen@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v5 04/13] py: cam.py: Use new events\n\tsupport","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":27250,"web_url":"https://patchwork.libcamera.org/comment/27250/","msgid":"","date":"2023-06-05T09:10:40","subject":"Re: [libcamera-devel] [PATCH v5 04/13] py: cam.py: Use new events\n\tsupport","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:10, Laurent Pinchart wrote:\n> Hi Tomi,\n> \n> Thank you for the patch.\n> \n> On Sat, Jun 03, 2023 at 10:56:06AM +0300, Tomi Valkeinen wrote:\n>> Convert cam.py to use the new event dispatching. In addition to handling\n>> the request-completed event, handle also disconnect, camera-added and\n>> camera-removed events (which only do a simple print).\n>>\n>> Signed-off-by: Tomi Valkeinen \n>> Reviewed-by: Jacopo Mondi \n>> Reviewed-by: Laurent Pinchart \n>> ---\n>> src/py/cam/cam.py | 27 +++++++++++++++++++++------\n>> 1 file changed, 21 insertions(+), 6 deletions(-)\n>>\n>> diff --git a/src/py/cam/cam.py b/src/py/cam/cam.py\n>> index a2a115c1..1e2d1f69 100755\n>> --- a/src/py/cam/cam.py\n>> +++ b/src/py/cam/cam.py\n>> @@ -230,11 +230,19 @@ class CaptureState:\n>> # Called from renderer when there is a libcamera event\n>> def event_handler(self):\n>> try:\n>> - reqs = self.cm.get_ready_requests()\n>> -\n>> - for req in reqs:\n>> - ctx = next(ctx for ctx in self.contexts if ctx.idx == req.cookie)\n>> - self.__request_handler(ctx, req)\n>> + for ev in self.cm.get_events():\n>> + type = ev.type\n>> +\n>> + if type == libcam.Event.Type.CameraAdded:\n>> + print(f'Camera {ev.camera} added')\n>> + elif type == libcam.Event.Type.CameraRemoved:\n>> + print(f'Camera {ev.camera} removed')\n>> + elif type == libcam.Event.Type.Disconnect:\n>> + print(f'Camera {ev.camera} disconnected')\n>> + elif type == libcam.Event.Type.RequestCompleted:\n>> + self.__request_handler(ev.camera, ev.request)\n>> + else:\n>> + raise RuntimeError(\"Bad event type\")\n> \n> This will cause issues if we later add new event types. Wouldn't it be\n> better to ignore unknown event types, or possibly print a (one-time)\n> warning message ?\n\nRight, this error should never happen. Maybe this again shows that we \nhave some unclear behavior in the bindings. If we make it so that all \nthe events (including CameraManager's events) must be explicitly \nenabled, we can only handle the ones we have enabled, and raise an error \nfor anything else (or assert). But if we do implicitly enable some \nevents, we have to ignore any unhandled ones.\n\nI'm leaning towards the explicit behavior, as these are somewhat low \nlevel bindings.\n\n>> \n>> running = any(ctx.reqs_completed < ctx.opt_capture for ctx in self.contexts)\n>> return running\n>> @@ -242,7 +250,9 @@ class CaptureState:\n>> traceback.print_exc()\n>> return False\n>> \n>> - def __request_handler(self, ctx, req):\n>> + def __request_handler(self, cam, req):\n>> + ctx = next(ctx for ctx in self.contexts if ctx.camera == cam)\n>> +\n>> if req.status != libcam.Request.Status.Complete:\n>> raise Exception('{}: Request failed: {}'.format(ctx.id, req.status))\n>> \n>> @@ -447,6 +457,11 @@ def main():\n>> \n>> state.do_cmd_capture()\n>> \n>> + # This is not strictly needed, but it helps to do a proper cleanup as we\n>> + # drop any unhandled events, and so makes it easier to use memory leak\n>> + # detectors.\n>> + cm.get_events()\n> \n> Somehow this feels like a hack, outlining a design issue. Is there any\n> way we could do better ? How about clearing events in the camera manager\n> destructor ?\n\nIf we have events in the queue, the events keep their respective cameras \nalive, and the cameras keep the camera manager alive, so there will \nnever be a destructor call. That said, python gc should detect circular \nreferences, but I'm not sure if it applies here.\n\nBut now that I test it, I don't see PyCameraManager destructor being \ncalled at all... So something has gotten broken. Hmm it's probably the \nnew signal subscription. We keep the requestCompleted subscribed, and we \nhave passed a shared_ptr to it...\n\nThis is odd, I'm pretty sure I tested this. And the unittests think that \nCameraManager does get cleaned up. Sigh...\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 A3947C3200\n\tfor ;\n\tMon, 5 Jun 2023 09:10:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 15D6561EA3;\n\tMon, 5 Jun 2023 11:10:46 +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 8AF1661EA2\n\tfor ;\n\tMon, 5 Jun 2023 11:10:44 +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 762052BC;\n\tMon, 5 Jun 2023 11:10:19 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1685956246;\n\tbh=7FsgTfhFfw1ru32foWHwXq7cYSfLXkYjcmzKrg+PvlE=;\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=E/BrbNpvsaYkBGYY67dP5bYxibXbLzjXdIM6hZakbAwyxUpk7XXa51ZA4C+uQWsVn\n\tibAi59sFTLKz4lRDSbNbz/ZxYGpFOC5JXSOkvc0gugjxsg03lSXQ0glKbYt998ODxN\n\tJpGIHPsOLKg0HXvGxNoE0Ha5fam+7Oao5LEjghCoPH+1SJnhLtz6EBIX4/83+GcoOp\n\tyKmw2wsCEqu07GTbgoRLfM378V3fl7G1KZDBoDnkENcpGOp7tI+6sxYXyuCcZxLIju\n\ts/meeyzz1QJIMw2adzspt9iYEJSr99fEMKj+MCeACqfm4smwXe27BusQAB6mn+OR3b\n\tz1gNu0S0ezvpA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1685956219;\n\tbh=7FsgTfhFfw1ru32foWHwXq7cYSfLXkYjcmzKrg+PvlE=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=rjFNmEXU94e37L8ad1vnpRgWtT3FldolR7D42WlQKTYqZXPKvT7hyEaW0qGKbIQzp\n\tmZQP0OEWBxOjQoQobWk9a/pntqrM0F9h+S4ZU1dqEuJgfUUmGgMrKBI7qAIEpIz//z\n\tKDBXWMfjrKZ668ARbF3YhMyo00tq0HfvhzGTNs8k="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"rjFNmEXU\"; dkim-atps=neutral","Message-ID":"","Date":"Mon, 5 Jun 2023 12:10:40 +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-5-tomi.valkeinen@ideasonboard.com>\n\t<20230605051028.GB22604@pendragon.ideasonboard.com>","In-Reply-To":"<20230605051028.GB22604@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v5 04/13] py: cam.py: Use new events\n\tsupport","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":27252,"web_url":"https://patchwork.libcamera.org/comment/27252/","msgid":"<9b2ae833-ef20-0a23-7580-82e68ebc2966@ideasonboard.com>","date":"2023-06-05T09:37:26","subject":"Re: [libcamera-devel] [PATCH v5 04/13] py: cam.py: Use new events\n\tsupport","submitter":{"id":109,"url":"https://patchwork.libcamera.org/api/people/109/","name":"Tomi Valkeinen","email":"tomi.valkeinen@ideasonboard.com"},"content":"On 05/06/2023 12:10, Tomi Valkeinen wrote:\n> On 05/06/2023 08:10, Laurent Pinchart wrote:\n>> Hi Tomi,\n>>\n>> Thank you for the patch.\n>>\n>> On Sat, Jun 03, 2023 at 10:56:06AM +0300, Tomi Valkeinen wrote:\n>>> Convert cam.py to use the new event dispatching. In addition to handling\n>>> the request-completed event, handle also disconnect, camera-added and\n>>> camera-removed events (which only do a simple print).\n>>>\n>>> Signed-off-by: Tomi Valkeinen \n>>> Reviewed-by: Jacopo Mondi \n>>> Reviewed-by: Laurent Pinchart \n>>> ---\n>>>   src/py/cam/cam.py | 27 +++++++++++++++++++++------\n>>>   1 file changed, 21 insertions(+), 6 deletions(-)\n>>>\n>>> diff --git a/src/py/cam/cam.py b/src/py/cam/cam.py\n>>> index a2a115c1..1e2d1f69 100755\n>>> --- a/src/py/cam/cam.py\n>>> +++ b/src/py/cam/cam.py\n>>> @@ -230,11 +230,19 @@ class CaptureState:\n>>>       # Called from renderer when there is a libcamera event\n>>>       def event_handler(self):\n>>>           try:\n>>> -            reqs = self.cm.get_ready_requests()\n>>> -\n>>> -            for req in reqs:\n>>> -                ctx = next(ctx for ctx in self.contexts if ctx.idx \n>>> == req.cookie)\n>>> -                self.__request_handler(ctx, req)\n>>> +            for ev in self.cm.get_events():\n>>> +                type = ev.type\n>>> +\n>>> +                if type == libcam.Event.Type.CameraAdded:\n>>> +                    print(f'Camera {ev.camera} added')\n>>> +                elif type == libcam.Event.Type.CameraRemoved:\n>>> +                    print(f'Camera {ev.camera} removed')\n>>> +                elif type == libcam.Event.Type.Disconnect:\n>>> +                    print(f'Camera {ev.camera} disconnected')\n>>> +                elif type == libcam.Event.Type.RequestCompleted:\n>>> +                    self.__request_handler(ev.camera, ev.request)\n>>> +                else:\n>>> +                    raise RuntimeError(\"Bad event type\")\n>>\n>> This will cause issues if we later add new event types. Wouldn't it be\n>> better to ignore unknown event types, or possibly print a (one-time)\n>> warning message ?\n> \n> Right, this error should never happen. Maybe this again shows that we \n> have some unclear behavior in the bindings. If we make it so that all \n> the events (including CameraManager's events) must be explicitly \n> enabled, we can only handle the ones we have enabled, and raise an error \n> for anything else (or assert). But if we do implicitly enable some \n> events, we have to ignore any unhandled ones.\n> \n> I'm leaning towards the explicit behavior, as these are somewhat low \n> level bindings.\n> \n>>>               running = any(ctx.reqs_completed < ctx.opt_capture for \n>>> ctx in self.contexts)\n>>>               return running\n>>> @@ -242,7 +250,9 @@ class CaptureState:\n>>>               traceback.print_exc()\n>>>               return False\n>>> -    def __request_handler(self, ctx, req):\n>>> +    def __request_handler(self, cam, req):\n>>> +        ctx = next(ctx for ctx in self.contexts if ctx.camera == cam)\n>>> +\n>>>           if req.status != libcam.Request.Status.Complete:\n>>>               raise Exception('{}: Request failed: {}'.format(ctx.id, \n>>> req.status))\n>>> @@ -447,6 +457,11 @@ def main():\n>>>           state.do_cmd_capture()\n>>> +        # This is not strictly needed, but it helps to do a proper \n>>> cleanup as we\n>>> +        # drop any unhandled events, and so makes it easier to use \n>>> memory leak\n>>> +        # detectors.\n>>> +        cm.get_events()\n>>\n>> Somehow this feels like a hack, outlining a design issue. Is there any\n>> way we could do better ? How about clearing events in the camera manager\n>> destructor ?\n> \n> If we have events in the queue, the events keep their respective cameras \n> alive, and the cameras keep the camera manager alive, so there will \n> never be a destructor call. That said, python gc should detect circular \n> references, but I'm not sure if it applies here.\n> \n> But now that I test it, I don't see PyCameraManager destructor being \n> called at all... So something has gotten broken. Hmm it's probably the \n> new signal subscription. We keep the requestCompleted subscribed, and we \n> have passed a shared_ptr to it...\n\nYes, that's the issue. It can be solved by passing a \nweak_ptr instead. And we also need to disconnect all \nthe signals in the PyCameraManager destructor, as otherwise libcamera \ncomplains about \"Removing media device /dev/media0 while still in use\".\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 2D008C31E9\n\tfor ;\n\tMon, 5 Jun 2023 09:37:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7342962880;\n\tMon, 5 Jun 2023 11:37:31 +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 5423A61EA3\n\tfor ;\n\tMon, 5 Jun 2023 11:37:29 +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 1C12C2BC;\n\tMon, 5 Jun 2023 11:37:04 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1685957851;\n\tbh=SE7axUxz0QEeM677AccO6F5l1oB8gSIsghJBYfgj3Es=;\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=AUSaf/epaHshv1s87nPSV8MJ9xK4Cen1jE7Wh8x3w7ZppjnYDlz/Qb0FlGKcSea1O\n\tdj9wYfog0VhfKTLhGhwE2K7hPVGXAhMyibh7EjRLod0WtwAx1Teg+OFpjQcJO8bvzz\n\tbB36an2kj8sRw5h9DfnZqczDG6AxPeYt36FczFbFMMyC9DFw3/9SB1KC8ZNleoOSRF\n\t+xsvnr4grdaOcfYt7DeSjnLvcyiI7uosO0yXjXVuUQhUn8hrPhN57EL3cjrBHNYmrD\n\tCIP1EKEZ/BYr8NT4atSpUDQMUy/AGCCx8Y4V1XW3nLPGQGHzEqSPMRe1RUAnQ25kzq\n\t/3TTAjJg63ERQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1685957824;\n\tbh=SE7axUxz0QEeM677AccO6F5l1oB8gSIsghJBYfgj3Es=;\n\th=Date:Subject:From:To:Cc:References:In-Reply-To:From;\n\tb=EflmjLYb5GoW3huPdd1X+VdGJQ0DVtOIqkCeGH8YtALzQOc7w0j81n/qGkDWVvjTw\n\tZMHl6yUOL7zSPoF7CgL4fakXcDzDbTJdW5ItLFtDmCJwxWr7fVh4in33nabnkMITrH\n\tPiaDe4ZafyUYKA+i+KnVyiuvNo0V9ZnIWlXmQmMk="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"EflmjLYb\"; dkim-atps=neutral","Message-ID":"<9b2ae833-ef20-0a23-7580-82e68ebc2966@ideasonboard.com>","Date":"Mon, 5 Jun 2023 12:37:26 +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-5-tomi.valkeinen@ideasonboard.com>\n\t<20230605051028.GB22604@pendragon.ideasonboard.com>\n\t","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v5 04/13] py: cam.py: Use new events\n\tsupport","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":27294,"web_url":"https://patchwork.libcamera.org/comment/27294/","msgid":"<20230607073124.GL14101@pendragon.ideasonboard.com>","date":"2023-06-07T07:31:24","subject":"Re: [libcamera-devel] [PATCH v5 04/13] py: cam.py: Use new events\n\tsupport","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Tomi,\n\nOn Mon, Jun 05, 2023 at 12:37:26PM +0300, Tomi Valkeinen wrote:\n> On 05/06/2023 12:10, Tomi Valkeinen wrote:\n> > On 05/06/2023 08:10, Laurent Pinchart wrote:\n> >> On Sat, Jun 03, 2023 at 10:56:06AM +0300, Tomi Valkeinen wrote:\n> >>> Convert cam.py to use the new event dispatching. In addition to handling\n> >>> the request-completed event, handle also disconnect, camera-added and\n> >>> camera-removed events (which only do a simple print).\n> >>>\n> >>> Signed-off-by: Tomi Valkeinen \n> >>> Reviewed-by: Jacopo Mondi \n> >>> Reviewed-by: Laurent Pinchart \n> >>> ---\n> >>>   src/py/cam/cam.py | 27 +++++++++++++++++++++------\n> >>>   1 file changed, 21 insertions(+), 6 deletions(-)\n> >>>\n> >>> diff --git a/src/py/cam/cam.py b/src/py/cam/cam.py\n> >>> index a2a115c1..1e2d1f69 100755\n> >>> --- a/src/py/cam/cam.py\n> >>> +++ b/src/py/cam/cam.py\n> >>> @@ -230,11 +230,19 @@ class CaptureState:\n> >>>       # Called from renderer when there is a libcamera event\n> >>>       def event_handler(self):\n> >>>           try:\n> >>> -            reqs = self.cm.get_ready_requests()\n> >>> -\n> >>> -            for req in reqs:\n> >>> -                ctx = next(ctx for ctx in self.contexts if ctx.idx == req.cookie)\n> >>> -                self.__request_handler(ctx, req)\n> >>> +            for ev in self.cm.get_events():\n> >>> +                type = ev.type\n> >>> +\n> >>> +                if type == libcam.Event.Type.CameraAdded:\n> >>> +                    print(f'Camera {ev.camera} added')\n> >>> +                elif type == libcam.Event.Type.CameraRemoved:\n> >>> +                    print(f'Camera {ev.camera} removed')\n> >>> +                elif type == libcam.Event.Type.Disconnect:\n> >>> +                    print(f'Camera {ev.camera} disconnected')\n> >>> +                elif type == libcam.Event.Type.RequestCompleted:\n> >>> +                    self.__request_handler(ev.camera, ev.request)\n> >>> +                else:\n> >>> +                    raise RuntimeError(\"Bad event type\")\n> >>\n> >> This will cause issues if we later add new event types. Wouldn't it be\n> >> better to ignore unknown event types, or possibly print a (one-time)\n> >> warning message ?\n> > \n> > Right, this error should never happen. Maybe this again shows that we \n> > have some unclear behavior in the bindings. If we make it so that all \n> > the events (including CameraManager's events) must be explicitly \n> > enabled, we can only handle the ones we have enabled, and raise an error \n> > for anything else (or assert). But if we do implicitly enable some \n> > events, we have to ignore any unhandled ones.\n\nAgreed.\n\n> > I'm leaning towards the explicit behavior, as these are somewhat low \n> > level bindings.\n\nAonther option would be to always emit all events. I think I would\nprefer that, an event subscription mechanism seems a bit over-engineered\nto me, it's additional complexity for unclear gains. Could we skip it\nfor now and introduce it later if needed ?\n\n> >>>               running = any(ctx.reqs_completed < ctx.opt_capture for \n> >>> ctx in self.contexts)\n> >>>               return running\n> >>> @@ -242,7 +250,9 @@ class CaptureState:\n> >>>               traceback.print_exc()\n> >>>               return False\n> >>> -    def __request_handler(self, ctx, req):\n> >>> +    def __request_handler(self, cam, req):\n> >>> +        ctx = next(ctx for ctx in self.contexts if ctx.camera == cam)\n> >>> +\n> >>>           if req.status != libcam.Request.Status.Complete:\n> >>>               raise Exception('{}: Request failed: {}'.format(ctx.id, req.status))\n> >>> @@ -447,6 +457,11 @@ def main():\n> >>>           state.do_cmd_capture()\n> >>> +        # This is not strictly needed, but it helps to do a proper cleanup as we\n> >>> +        # drop any unhandled events, and so makes it easier to use memory leak\n> >>> +        # detectors.\n> >>> +        cm.get_events()\n> >>\n> >> Somehow this feels like a hack, outlining a design issue. Is there any\n> >> way we could do better ? How about clearing events in the camera manager\n> >> destructor ?\n> > \n> > If we have events in the queue, the events keep their respective cameras \n> > alive, and the cameras keep the camera manager alive, so there will \n> > never be a destructor call. That said, python gc should detect circular \n> > references, but I'm not sure if it applies here.\n> > \n> > But now that I test it, I don't see PyCameraManager destructor being \n> > called at all... So something has gotten broken. Hmm it's probably the \n> > new signal subscription. We keep the requestCompleted subscribed, and we \n> > have passed a shared_ptr to it...\n> \n> Yes, that's the issue. It can be solved by passing a \n> weak_ptr instead. And we also need to disconnect all \n> the signals in the PyCameraManager destructor, as otherwise libcamera \n> complains about \"Removing media device /dev/media0 while still in use\".","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 76944C31E9\n\tfor ;\n\tWed, 7 Jun 2023 07:31:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0590462886;\n\tWed, 7 Jun 2023 09:31:31 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D615D62728\n\tfor ;\n\tWed, 7 Jun 2023 09:31:28 +0200 (CEST)","from pendragon.ideasonboard.com (om126233170111.36.openmobile.ne.jp\n\t[126.233.170.111])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9457F75B;\n\tWed, 7 Jun 2023 09:31:01 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686123091;\n\tbh=rM59fu/bCzoGfesENzrqAXPVeoU6dpbCqfn0vwLsjdc=;\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=VM+TOwyUqB9hY3eX26T6+sNwtcK/ohhDAHp6gYjsUUh0kw687rzLjDMb7eQFWSnMt\n\tz/ZSsmWUk2i/TVO64xgUkY+JWqBW2u4j5fH/pvvAJLMAwbPBn2qypnavEVgbXeHRBl\n\tIzKQfFkkcrmWE6SmRoYY2WRXSfhbxKF4WuebrAdWqBIjzRZFZiLQh6Q4rc8bESIwkq\n\tHrbzk2hQzGNiGzoFqcSVolLPeP7x3DK8mNE9WwU8N7DWxfN+bMsaX0f0X3PR2mkHjv\n\tKzwCW6hBcAVIpQLotONPKkgolgFHHHOhKePP3A5+024ZSwBk//QMDzrPQVHFc3ato/\n\tiaUx98X8EqfZw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1686123062;\n\tbh=rM59fu/bCzoGfesENzrqAXPVeoU6dpbCqfn0vwLsjdc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Yg8p2rXw4sNXWi6H19rtRZmKT199Va3TUAHX0dTD1aPQdQbuP0Nu1AYbiNzusiK/V\n\tS3y/YRO7+sLS20V6+Oz/Af2hVKJAjaFTJkL3x3ILhPCGhHyawnD7gQsP6DFozPntmx\n\trTRnFyjp07sQR1x8cRZmjbWSWuoeOEI5L3X3GGsk="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Yg8p2rXw\"; dkim-atps=neutral","Date":"Wed, 7 Jun 2023 10:31:24 +0300","To":"Tomi Valkeinen ","Message-ID":"<20230607073124.GL14101@pendragon.ideasonboard.com>","References":"<20230603075615.20663-1-tomi.valkeinen@ideasonboard.com>\n\t<20230603075615.20663-5-tomi.valkeinen@ideasonboard.com>\n\t<20230605051028.GB22604@pendragon.ideasonboard.com>\n\t\n\t<9b2ae833-ef20-0a23-7580-82e68ebc2966@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<9b2ae833-ef20-0a23-7580-82e68ebc2966@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v5 04/13] py: cam.py: Use new events\n\tsupport","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":27296,"web_url":"https://patchwork.libcamera.org/comment/27296/","msgid":"<2817571e-8814-aab5-b578-c779ad31bf4e@ideasonboard.com>","date":"2023-06-07T09:35:47","subject":"Re: [libcamera-devel] [PATCH v5 04/13] py: cam.py: Use new events\n\tsupport","submitter":{"id":109,"url":"https://patchwork.libcamera.org/api/people/109/","name":"Tomi Valkeinen","email":"tomi.valkeinen@ideasonboard.com"},"content":"On 07/06/2023 10:31, Laurent Pinchart wrote:\n> Hi Tomi,\n> \n> On Mon, Jun 05, 2023 at 12:37:26PM +0300, Tomi Valkeinen wrote:\n>> On 05/06/2023 12:10, Tomi Valkeinen wrote:\n>>> On 05/06/2023 08:10, Laurent Pinchart wrote:\n>>>> On Sat, Jun 03, 2023 at 10:56:06AM +0300, Tomi Valkeinen wrote:\n>>>>> Convert cam.py to use the new event dispatching. In addition to handling\n>>>>> the request-completed event, handle also disconnect, camera-added and\n>>>>> camera-removed events (which only do a simple print).\n>>>>>\n>>>>> Signed-off-by: Tomi Valkeinen \n>>>>> Reviewed-by: Jacopo Mondi \n>>>>> Reviewed-by: Laurent Pinchart \n>>>>> ---\n>>>>>   src/py/cam/cam.py | 27 +++++++++++++++++++++------\n>>>>>   1 file changed, 21 insertions(+), 6 deletions(-)\n>>>>>\n>>>>> diff --git a/src/py/cam/cam.py b/src/py/cam/cam.py\n>>>>> index a2a115c1..1e2d1f69 100755\n>>>>> --- a/src/py/cam/cam.py\n>>>>> +++ b/src/py/cam/cam.py\n>>>>> @@ -230,11 +230,19 @@ class CaptureState:\n>>>>>       # Called from renderer when there is a libcamera event\n>>>>>       def event_handler(self):\n>>>>>           try:\n>>>>> -            reqs = self.cm.get_ready_requests()\n>>>>> -\n>>>>> -            for req in reqs:\n>>>>> -                ctx = next(ctx for ctx in self.contexts if ctx.idx == req.cookie)\n>>>>> -                self.__request_handler(ctx, req)\n>>>>> +            for ev in self.cm.get_events():\n>>>>> +                type = ev.type\n>>>>> +\n>>>>> +                if type == libcam.Event.Type.CameraAdded:\n>>>>> +                    print(f'Camera {ev.camera} added')\n>>>>> +                elif type == libcam.Event.Type.CameraRemoved:\n>>>>> +                    print(f'Camera {ev.camera} removed')\n>>>>> +                elif type == libcam.Event.Type.Disconnect:\n>>>>> +                    print(f'Camera {ev.camera} disconnected')\n>>>>> +                elif type == libcam.Event.Type.RequestCompleted:\n>>>>> +                    self.__request_handler(ev.camera, ev.request)\n>>>>> +                else:\n>>>>> +                    raise RuntimeError(\"Bad event type\")\n>>>>\n>>>> This will cause issues if we later add new event types. Wouldn't it be\n>>>> better to ignore unknown event types, or possibly print a (one-time)\n>>>> warning message ?\n>>>\n>>> Right, this error should never happen. Maybe this again shows that we\n>>> have some unclear behavior in the bindings. If we make it so that all\n>>> the events (including CameraManager's events) must be explicitly\n>>> enabled, we can only handle the ones we have enabled, and raise an error\n>>> for anything else (or assert). But if we do implicitly enable some\n>>> events, we have to ignore any unhandled ones.\n> \n> Agreed.\n> \n>>> I'm leaning towards the explicit behavior, as these are somewhat low\n>>> level bindings.\n> \n> Aonther option would be to always emit all events. I think I would\n> prefer that, an event subscription mechanism seems a bit over-engineered\n> to me, it's additional complexity for unclear gains. Could we skip it\n> for now and introduce it later if needed ?\n\nPerhaps that's the best option. It does rub me the wrong way, though, \ndoing at least a double amount of allocations (presuming the buffer \ncompleted event is not used), but in the bigger picture it's perhaps in \nthe who-cares category.\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 6F782C31E9\n\tfor ;\n\tWed, 7 Jun 2023 09:35:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E318562886;\n\tWed, 7 Jun 2023 11:35:53 +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 4D9F862728\n\tfor ;\n\tWed, 7 Jun 2023 11:35:52 +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 A4A1C2B6;\n\tWed, 7 Jun 2023 11:35:25 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1686130553;\n\tbh=W7my6CzLRgelSzCWOZn73NYTn/ETjtnhtVzp3sQ65t8=;\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=B1aGNvM3AdkRtocbUm7RXVxF3GmiLhyP61EPMleMUMYcvaalnjw9ACNtXt8ZOuUbm\n\t8QDlixa9+N+1C2MCH9AN2pb9nQe6av3SV7EilQRF3zd2+1XRY1rm5Tl7j4CobOkgTw\n\tj/pFQoqpjY4+5cqIFkpQcRfh8rrYXnNUTRjGTwCj5ron4tfzVsv7SERIKrrezYgYsI\n\tWtpmkg6BkSnNouUixA+Rw4+PvvpKC3vsvjnkJ2SQtCIElICSNd1jdp8ZFfTvmJwvRW\n\t5vQkNB6yaNh70jCOXhbNJ1gj4vobBzYGOuGxgpnLOXhpFT6UZtjBf1mvOoyiCxCSeW\n\twLZVcol2mRzZw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1686130526;\n\tbh=W7my6CzLRgelSzCWOZn73NYTn/ETjtnhtVzp3sQ65t8=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=QSla2HXizX0EyhgCjLpv7q2omcf6r4wAmg2jWAaTTHI0Y6a06mDR8eYSL23K+pnBH\n\tpeddXzfeQRZpnfXWiHEZ3SrvFUWbWAlGW0lwBjD5l5NSqRgSyW3mX/r0zf+oQdFebV\n\tQychj6JhsPgjv2DjnNpmX3gvYeK1Z0ZZx+dLkR58="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"QSla2HXi\"; dkim-atps=neutral","Message-ID":"<2817571e-8814-aab5-b578-c779ad31bf4e@ideasonboard.com>","Date":"Wed, 7 Jun 2023 12:35:47 +0300","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101\n\tThunderbird/102.11.0","To":"Laurent Pinchart ","References":"<20230603075615.20663-1-tomi.valkeinen@ideasonboard.com>\n\t<20230603075615.20663-5-tomi.valkeinen@ideasonboard.com>\n\t<20230605051028.GB22604@pendragon.ideasonboard.com>\n\t\n\t<9b2ae833-ef20-0a23-7580-82e68ebc2966@ideasonboard.com>\n\t<20230607073124.GL14101@pendragon.ideasonboard.com>","Content-Language":"en-US","In-Reply-To":"<20230607073124.GL14101@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v5 04/13] py: cam.py: Use new events\n\tsupport","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\" "}}]