[{"id":33769,"web_url":"https://patchwork.libcamera.org/comment/33769/","msgid":"","date":"2025-03-31T15:12:58","subject":"Re: [PATCH v6 1/5] libcamera: v4l2_device: add frame start event\n\thelpers","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Stanislaw,\n\nThank you for the patch. \n\nOn Wed, Mar 05, 2025 at 02:52:52PM +0100, Stanislaw Gruszka wrote:\n> Add helper to check if frame start event are supported by subdevice.\n> Since kernel does not have interface to query supported events\n> use subscribe interface.\n> \n> Reviewed-by: Kieran Bingham # v3\n> Reviewed-by: Laurent Pinchart # v5\n> Signed-off-by: Stanislaw Gruszka \n> ---\n> include/libcamera/internal/v4l2_device.h | 1 +\n> src/libcamera/v4l2_device.cpp | 18 ++++++++++++++++++\n> 2 files changed, 19 insertions(+)\n> \n> diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h\n> index affe52c2..a647c96a 100644\n> --- a/include/libcamera/internal/v4l2_device.h\n> +++ b/include/libcamera/internal/v4l2_device.h\n> @@ -45,6 +45,7 @@ public:\n> \tconst std::string &deviceNode() const { return deviceNode_; }\n> \tstd::string devicePath() const;\n> \n> +\tbool supportsFrameStartEvent();\n> \tint setFrameStartEnabled(bool enable);\n> \tSignal frameStart;\n> \n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index 2f65a43a..c3e9cd4a 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -449,6 +449,24 @@ std::string V4L2Device::devicePath() const\n> \treturn path;\n> }\n> \n> +/**\n> + * \\brief Check if frame start event is supported\n> + * \\return True if frame start event is supported, false otherwise\n> + */\n> +\n> +bool V4L2Device::supportsFrameStartEvent()\n> +{\n> +\tstruct v4l2_event_subscription event{};\n> +\tevent.type = V4L2_EVENT_FRAME_SYNC;\n> +\n> +\tint ret = ioctl(VIDIOC_SUBSCRIBE_EVENT, &event);\n> +\tif (ret)\n> +\t\treturn false;\n> +\n> +\tioctl(VIDIOC_UNSUBSCRIBE_EVENT, &event);\n\nMaybe I'm missing something. To me it appears that I must not call this\nfunction while frame start events are enabled, otherwise these events\nwould then also be unsubscribed as the subscription is per fd - or am I\nwrong here? In the current use-cases that does no harm because\nsupportFrameStart() is always called before enabling frame start events\nand never after. But it is an unexpected restriction on the class\ninterface. Should we mention that in the docs? Caching the result\ninternally and also filling it from within setFrameStartEnabled() might\nbe an overkill?\n\nBest regards,\nStefan\n\n> +\treturn true;\n> +}\n> +\n> /**\n> * \\brief Enable or disable frame start event notification\n> * \\param[in] enable True to enable frame start events, false to disable them\n> -- \n> 2.43.0\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 32CCDC323E\n\tfor ;\n\tMon, 31 Mar 2025 15:13:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9494B68986;\n\tMon, 31 Mar 2025 17:13:02 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 546B768967\n\tfor ;\n\tMon, 31 Mar 2025 17:13:01 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:823a:c275:e8b5:b937])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AB1C9703;\n\tMon, 31 Mar 2025 17:11:09 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Xv3DEuO7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1743433869;\n\tbh=ZePLaZ9p/JqIekouAmipFTiKIV8dARQziBXiSi9FWtg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Xv3DEuO7nw0WK8iocOW8egf3/v1yEYEpRqx/9SYbRfyRWSK1tVljk6tiRdyfubivN\n\tssbuZAZjQj382QhM0rxwxt7NMpyDLtzuPJyHUIjh2rdeL/yGU9FDMs2bLfxZmFwnm3\n\tkbZZFU2/osiCwb03ThC4pZlV/u09tXVPTakjc/cQ=","Date":"Mon, 31 Mar 2025 17:12:58 +0200","From":"Stefan Klug ","To":"Stanislaw Gruszka ","Cc":"libcamera-devel@lists.libcamera.org, Milan Zamazal ,\n\tLaurent Pinchart , \n\tKieran Bingham ,\n\tNaushir Patuck , \n\tSakari Ailus ,\n\tHans de Goede ","Subject":"Re: [PATCH v6 1/5] libcamera: v4l2_device: add frame start event\n\thelpers","Message-ID":"","References":"<20250305135256.801351-1-stanislaw.gruszka@linux.intel.com>\n\t<20250305135256.801351-2-stanislaw.gruszka@linux.intel.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250305135256.801351-2-stanislaw.gruszka@linux.intel.com>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":33787,"web_url":"https://patchwork.libcamera.org/comment/33787/","msgid":"<174344000138.3394313.15480616072522679555@ping.linuxembedded.co.uk>","date":"2025-03-31T16:53:21","subject":"Re: [PATCH v6 1/5] libcamera: v4l2_device: add frame start event\n\thelpers","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Stefan Klug (2025-03-31 16:12:58)\n> Hi Stanislaw,\n> \n> Thank you for the patch. \n> \n> On Wed, Mar 05, 2025 at 02:52:52PM +0100, Stanislaw Gruszka wrote:\n> > Add helper to check if frame start event are supported by subdevice.\n> > Since kernel does not have interface to query supported events\n> > use subscribe interface.\n> > \n> > Reviewed-by: Kieran Bingham # v3\n> > Reviewed-by: Laurent Pinchart # v5\n> > Signed-off-by: Stanislaw Gruszka \n> > ---\n> > include/libcamera/internal/v4l2_device.h | 1 +\n> > src/libcamera/v4l2_device.cpp | 18 ++++++++++++++++++\n> > 2 files changed, 19 insertions(+)\n> > \n> > diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h\n> > index affe52c2..a647c96a 100644\n> > --- a/include/libcamera/internal/v4l2_device.h\n> > +++ b/include/libcamera/internal/v4l2_device.h\n> > @@ -45,6 +45,7 @@ public:\n> > const std::string &deviceNode() const { return deviceNode_; }\n> > std::string devicePath() const;\n> > \n> > + bool supportsFrameStartEvent();\n> > int setFrameStartEnabled(bool enable);\n> > Signal frameStart;\n> > \n> > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> > index 2f65a43a..c3e9cd4a 100644\n> > --- a/src/libcamera/v4l2_device.cpp\n> > +++ b/src/libcamera/v4l2_device.cpp\n> > @@ -449,6 +449,24 @@ std::string V4L2Device::devicePath() const\n> > return path;\n> > }\n> > \n> > +/**\n> > + * \\brief Check if frame start event is supported\n> > + * \\return True if frame start event is supported, false otherwise\n> > + */\n> > +\n> > +bool V4L2Device::supportsFrameStartEvent()\n> > +{\n> > + struct v4l2_event_subscription event{};\n> > + event.type = V4L2_EVENT_FRAME_SYNC;\n> > +\n> > + int ret = ioctl(VIDIOC_SUBSCRIBE_EVENT, &event);\n> > + if (ret)\n> > + return false;\n> > +\n> > + ioctl(VIDIOC_UNSUBSCRIBE_EVENT, &event);\n> \n> Maybe I'm missing something. To me it appears that I must not call this\n> function while frame start events are enabled, otherwise these events\n> would then also be unsubscribed as the subscription is per fd - or am I\n> wrong here? In the current use-cases that does no harm because\n> supportFrameStart() is always called before enabling frame start events\n> and never after. But it is an unexpected restriction on the class\n> interface. Should we mention that in the docs? Caching the result\n> internally and also filling it from within setFrameStartEnabled() might\n> be an overkill?\n\nFor what it's worth, I like this approach keeping the 'test' distinctly\nseparated.\n\nI can see other events I might need to subscribe to in the future - and\nI think I'd batch them so they all get checked at startup.\n\nDefinitely for sure - we can only 'test' if the event is available\nbefore we subscribe, but with the current limitations, I think this is\nthe right thing to do.\n\n--\nKieran\n\n> \n> Best regards,\n> Stefan\n> \n> > + return true;\n> > +}\n> > +\n> > /**\n> > * \\brief Enable or disable frame start event notification\n> > * \\param[in] enable True to enable frame start events, false to disable them\n> > -- \n> > 2.43.0\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 A8657C3213\n\tfor ;\n\tMon, 31 Mar 2025 16:53:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 99D9568985;\n\tMon, 31 Mar 2025 18:53:25 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 93A9868967\n\tfor ;\n\tMon, 31 Mar 2025 18:53:24 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BEA18725;\n\tMon, 31 Mar 2025 18:51:32 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"VKUaPlin\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1743439892;\n\tbh=dXfb/z1eT3JxW8hmTnAFMJ+A71qQS9vG9bFMFjiOqc4=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=VKUaPlinhm5rJJYbfxx+Kh+Gr7tYJSRUGtICavzJp1fw6Z19rnr9deiJWbOy7nHS0\n\tpQu9ZmGytTtgsULmYdyA0a/dGCb8rGzTb5XkTg8GsieZcZjxikpQ3amBLZWSjaOV6l\n\tSkJLzPuJGwkYdUq5+2qGZ35/CtsYqhFSwzGNc6iA=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20250305135256.801351-1-stanislaw.gruszka@linux.intel.com>\n\t<20250305135256.801351-2-stanislaw.gruszka@linux.intel.com>\n\t","Subject":"Re: [PATCH v6 1/5] libcamera: v4l2_device: add frame start event\n\thelpers","From":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org, Milan Zamazal ,\n\tLaurent Pinchart ,\n\tNaushir Patuck ,\n\tSakari Ailus ,\n\tHans de Goede ","To":"Stanislaw Gruszka ,\n\tStefan Klug ","Date":"Mon, 31 Mar 2025 17:53:21 +0100","Message-ID":"<174344000138.3394313.15480616072522679555@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":33837,"web_url":"https://patchwork.libcamera.org/comment/33837/","msgid":"<20250401022007.GC26315@pendragon.ideasonboard.com>","date":"2025-04-01T02:20:07","subject":"Re: [PATCH v6 1/5] libcamera: v4l2_device: add frame start event\n\thelpers","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Mar 31, 2025 at 05:53:21PM +0100, Kieran Bingham wrote:\n> Quoting Stefan Klug (2025-03-31 16:12:58)\n> > Hi Stanislaw,\n> > \n> > Thank you for the patch. \n> > \n> > On Wed, Mar 05, 2025 at 02:52:52PM +0100, Stanislaw Gruszka wrote:\n> > > Add helper to check if frame start event are supported by subdevice.\n> > > Since kernel does not have interface to query supported events\n> > > use subscribe interface.\n> > > \n> > > Reviewed-by: Kieran Bingham # v3\n> > > Reviewed-by: Laurent Pinchart # v5\n> > > Signed-off-by: Stanislaw Gruszka \n> > > ---\n> > > include/libcamera/internal/v4l2_device.h | 1 +\n> > > src/libcamera/v4l2_device.cpp | 18 ++++++++++++++++++\n> > > 2 files changed, 19 insertions(+)\n> > > \n> > > diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h\n> > > index affe52c2..a647c96a 100644\n> > > --- a/include/libcamera/internal/v4l2_device.h\n> > > +++ b/include/libcamera/internal/v4l2_device.h\n> > > @@ -45,6 +45,7 @@ public:\n> > > const std::string &deviceNode() const { return deviceNode_; }\n> > > std::string devicePath() const;\n> > > \n> > > + bool supportsFrameStartEvent();\n> > > int setFrameStartEnabled(bool enable);\n> > > Signal frameStart;\n> > > \n> > > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> > > index 2f65a43a..c3e9cd4a 100644\n> > > --- a/src/libcamera/v4l2_device.cpp\n> > > +++ b/src/libcamera/v4l2_device.cpp\n> > > @@ -449,6 +449,24 @@ std::string V4L2Device::devicePath() const\n> > > return path;\n> > > }\n> > > \n> > > +/**\n> > > + * \\brief Check if frame start event is supported\n> > > + * \\return True if frame start event is supported, false otherwise\n> > > + */\n> > > +\n> > > +bool V4L2Device::supportsFrameStartEvent()\n> > > +{\n> > > + struct v4l2_event_subscription event{};\n> > > + event.type = V4L2_EVENT_FRAME_SYNC;\n> > > +\n> > > + int ret = ioctl(VIDIOC_SUBSCRIBE_EVENT, &event);\n> > > + if (ret)\n> > > + return false;\n> > > +\n> > > + ioctl(VIDIOC_UNSUBSCRIBE_EVENT, &event);\n> > \n> > Maybe I'm missing something. To me it appears that I must not call this\n> > function while frame start events are enabled, otherwise these events\n> > would then also be unsubscribed as the subscription is per fd - or am I\n> > wrong here? In the current use-cases that does no harm because\n> > supportFrameStart() is always called before enabling frame start events\n> > and never after. But it is an unexpected restriction on the class\n> > interface. Should we mention that in the docs? Caching the result\n> > internally and also filling it from within setFrameStartEnabled() might\n> > be an overkill?\n> \n> For what it's worth, I like this approach keeping the 'test' distinctly\n> separated.\n> \n> I can see other events I might need to subscribe to in the future - and\n> I think I'd batch them so they all get checked at startup.\n> \n> Definitely for sure - we can only 'test' if the event is available\n> before we subscribe, but with the current limitations, I think this is\n> the right thing to do.\n\nShould we try to add an enum event API extension to V4L2 in parallel to\nthis ?\n\n> > > + return true;\n> > > +}\n> > > +\n> > > /**\n> > > * \\brief Enable or disable frame start event notification\n> > > * \\param[in] enable True to enable frame start events, false to disable them","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 F2F03C3213\n\tfor ;\n\tTue, 1 Apr 2025 02:20:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1C96D6897A;\n\tTue, 1 Apr 2025 04:20: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 06CE762C66\n\tfor ;\n\tTue, 1 Apr 2025 04:20:33 +0200 (CEST)","from pendragon.ideasonboard.com (85-76-147-224-nat.elisa-mobile.fi\n\t[85.76.147.224])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7A657250;\n\tTue, 1 Apr 2025 04:18:40 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Uu2SWW13\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1743473920;\n\tbh=ZMviwZZZUhGPFIuSIeWzx5DW6eHAJIp66YdWLdpZlpc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Uu2SWW13nvLl+r7fvjgg5pUWPnWBHPH4v2phPJD6IVo1vYBet0M6hlNk9u6dKlKD3\n\tpn8kqAEWHDJTuLGTIqtoitMclxL9w/jkauE6BcGjL6kWiLQie9hj5P9E+O/2vaHRzQ\n\tBVw+q8uOmCDfGs0sOOWj943uuFoOKTEFDARBOfLY=","Date":"Tue, 1 Apr 2025 05:20:07 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"Stanislaw Gruszka ,\n\tStefan Klug ,\n\tlibcamera-devel@lists.libcamera.org,\n\tMilan Zamazal , \n\tNaushir Patuck ,\n\tSakari Ailus ,\n\tHans de Goede ","Subject":"Re: [PATCH v6 1/5] libcamera: v4l2_device: add frame start event\n\thelpers","Message-ID":"<20250401022007.GC26315@pendragon.ideasonboard.com>","References":"<20250305135256.801351-1-stanislaw.gruszka@linux.intel.com>\n\t<20250305135256.801351-2-stanislaw.gruszka@linux.intel.com>\n\t\n\t<174344000138.3394313.15480616072522679555@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<174344000138.3394313.15480616072522679555@ping.linuxembedded.co.uk>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":33838,"web_url":"https://patchwork.libcamera.org/comment/33838/","msgid":"<20250401091856.GA31480@pendragon.ideasonboard.com>","date":"2025-04-01T09:18:56","subject":"Re: [PATCH v6 1/5] libcamera: v4l2_device: add frame start event\n\thelpers","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Mar 31, 2025 at 05:12:58PM +0200, Stefan Klug wrote:\n> Hi Stanislaw,\n> \n> Thank you for the patch. \n> \n> On Wed, Mar 05, 2025 at 02:52:52PM +0100, Stanislaw Gruszka wrote:\n> > Add helper to check if frame start event are supported by subdevice.\n> > Since kernel does not have interface to query supported events\n> > use subscribe interface.\n> > \n> > Reviewed-by: Kieran Bingham # v3\n> > Reviewed-by: Laurent Pinchart # v5\n> > Signed-off-by: Stanislaw Gruszka \n> > ---\n> > include/libcamera/internal/v4l2_device.h | 1 +\n> > src/libcamera/v4l2_device.cpp | 18 ++++++++++++++++++\n> > 2 files changed, 19 insertions(+)\n> > \n> > diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h\n> > index affe52c2..a647c96a 100644\n> > --- a/include/libcamera/internal/v4l2_device.h\n> > +++ b/include/libcamera/internal/v4l2_device.h\n> > @@ -45,6 +45,7 @@ public:\n> > \tconst std::string &deviceNode() const { return deviceNode_; }\n> > \tstd::string devicePath() const;\n> > \n> > +\tbool supportsFrameStartEvent();\n> > \tint setFrameStartEnabled(bool enable);\n> > \tSignal frameStart;\n> > \n> > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> > index 2f65a43a..c3e9cd4a 100644\n> > --- a/src/libcamera/v4l2_device.cpp\n> > +++ b/src/libcamera/v4l2_device.cpp\n> > @@ -449,6 +449,24 @@ std::string V4L2Device::devicePath() const\n> > \treturn path;\n> > }\n> > \n> > +/**\n> > + * \\brief Check if frame start event is supported\n> > + * \\return True if frame start event is supported, false otherwise\n> > + */\n> > +\n\nStray empty line.\n\n> > +bool V4L2Device::supportsFrameStartEvent()\n> > +{\n> > +\tstruct v4l2_event_subscription event{};\n> > +\tevent.type = V4L2_EVENT_FRAME_SYNC;\n> > +\n> > +\tint ret = ioctl(VIDIOC_SUBSCRIBE_EVENT, &event);\n> > +\tif (ret)\n> > +\t\treturn false;\n> > +\n> > +\tioctl(VIDIOC_UNSUBSCRIBE_EVENT, &event);\n> \n> Maybe I'm missing something. To me it appears that I must not call this\n> function while frame start events are enabled, otherwise these events\n> would then also be unsubscribed as the subscription is per fd - or am I\n> wrong here? In the current use-cases that does no harm because\n> supportFrameStart() is always called before enabling frame start events\n> and never after. But it is an unexpected restriction on the class\n> interface. Should we mention that in the docs? Caching the result\n> internally and also filling it from within setFrameStartEnabled() might\n> be an overkill?\n\nLet's at least expand the documentation a bit, as that's easy:\n\n/**\n * \\brief Check if frame start event is supported\n *\n * Due to limitations in the kernel API, this function may disable the frame\n * start event as a side effect. It should only be called during initialization,\n * before enabling the frame start event with setFrameStartEnabled().\n *\n * \\return True if frame start event is supported, false otherwise\n */\n\nThis will improve the user experience a lot, they will now be able to\nget a weird behaviour because they didn't read the documentation,\ninstead of getting the same behaviour because it was undocumented :-)\n\n> > +\treturn true;\n> > +}\n> > +\n> > /**\n> > * \\brief Enable or disable frame start event notification\n> > * \\param[in] enable True to enable frame start events, false to disable them","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 CDAC7C3213\n\tfor ;\n\tTue, 1 Apr 2025 09:19:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id ECA0762C68;\n\tTue, 1 Apr 2025 11:19:22 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0071262C68\n\tfor ;\n\tTue, 1 Apr 2025 11:19:20 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BC8488DB;\n\tTue, 1 Apr 2025 11:17:28 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"dEnqIpAv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1743499048;\n\tbh=Yn1C2cohpWpuSbxIu7BV5B9OElu0Idfswuiwu0ovkj4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=dEnqIpAvJR/QrjWTBDqADhZAw1uNvqo5G8gQOM9v52ZAYbw7sBJTFS+MbSt6Bpbbe\n\tt/drbh2i9nmymZKIXCerltw1PL0+zL9Q8bV2YyCS1O/UYIEr5rOj803K3UEbOYYPap\n\tkV56O2rsE3g7wlLaO6C0DUwT/q25B982OH8yG988=","Date":"Tue, 1 Apr 2025 12:18:56 +0300","From":"Laurent Pinchart ","To":"Stefan Klug ","Cc":"Stanislaw Gruszka ,\n\tlibcamera-devel@lists.libcamera.org,\n\tMilan Zamazal , \n\tKieran Bingham ,\n\tNaushir Patuck ,\n\tSakari Ailus ,\n\tHans de Goede ","Subject":"Re: [PATCH v6 1/5] libcamera: v4l2_device: add frame start event\n\thelpers","Message-ID":"<20250401091856.GA31480@pendragon.ideasonboard.com>","References":"<20250305135256.801351-1-stanislaw.gruszka@linux.intel.com>\n\t<20250305135256.801351-2-stanislaw.gruszka@linux.intel.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]