[{"id":33394,"web_url":"https://patchwork.libcamera.org/comment/33394/","msgid":"<173987977025.3276240.4232474153203292983@ping.linuxembedded.co.uk>","date":"2025-02-18T11:56:10","subject":"Re: [PATCH v3 1/2] 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 Stanislaw Gruszka (2025-02-18 09:19:50)\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> Additionally add helper for checking if the event enabled.\n> Will be needed later to proper code flow if enabling the event\n> fail for some reason.\n> \n\nI've started contemplating tackling this too - with a 'V4L2Event'\nwrapper ... but that's overkill while we have only a single event to\nmanage, so I think this is fine for now.\n\nReviewed-by: Kieran Bingham \n\n> Signed-off-by: Stanislaw Gruszka \n> ---\n> include/libcamera/internal/v4l2_device.h | 2 ++\n> src/libcamera/v4l2_device.cpp | 24 ++++++++++++++++++++++++\n> 2 files changed, 26 insertions(+)\n> \n> diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h\n> index affe52c2..0d807209 100644\n> --- a/include/libcamera/internal/v4l2_device.h\n> +++ b/include/libcamera/internal/v4l2_device.h\n> @@ -45,6 +45,8 @@ public:\n> const std::string &deviceNode() const { return deviceNode_; }\n> std::string devicePath() const;\n> \n> + bool supportsFrameStartEvent();\n> + bool frameStartEnabled() { return frameStartEnabled_; }\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..219c82f6 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -449,6 +449,30 @@ 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> + };\n> + event.type = V4L2_EVENT_FRAME_SYNC;\n> +\n> + int ret = ioctl(VIDIOC_SUBSCRIBE_EVENT, &event);\n> + if (ret == 0)\n> + ioctl(VIDIOC_UNSUBSCRIBE_EVENT, &event);\n> +\n> + return ret == 0;\n> +}\n> +\n> +/**\n> + * \\fn V4L2Device::frameStartEnabled()\n> + * \\brief Check if frame start event is enabled\n> + * \\return True if frame start event is enabled, false otherwise\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 EA2D1BD78E\n\tfor ;\n\tTue, 18 Feb 2025 11:56:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5E65968687;\n\tTue, 18 Feb 2025 12:56:15 +0100 (CET)","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 A755E6863B\n\tfor ;\n\tTue, 18 Feb 2025 12:56:13 +0100 (CET)","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 737B022F;\n\tTue, 18 Feb 2025 12:54:51 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"EliVLIEB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1739879691;\n\tbh=cC47duMdRRr/5aWP9X98iHDrl0h2ZLjYF+YGXvl7+aY=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=EliVLIEB97rOnXKgrBHyAGjR6HpFkIpHzh3cbeD92EeYJvJWIjyIclTOR6RyZQNQf\n\tBD+SyjHQuRyR8R1GFWxExB4MNcASfTYeEA4sy0bwaM4lzhHIENBhuZ/kM4rbQY/4Lr\n\tKyVivTKQl2V3NowfDM11W7fYtCDYh+F09Jd98cUo=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20250218091951.13017-2-stanislaw.gruszka@linux.intel.com>","References":"<20250218091951.13017-1-stanislaw.gruszka@linux.intel.com>\n\t<20250218091951.13017-2-stanislaw.gruszka@linux.intel.com>","Subject":"Re: [PATCH v3 1/2] libcamera: v4l2_device: add frame start event\n\thelpers","From":"Kieran Bingham ","Cc":"Milan Zamazal ,\n\tLaurent Pinchart ,\n\tNaushir Patuck ,\n\tSakari Ailus ,\n\tHans de Goede ","To":"Stanislaw Gruszka ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 18 Feb 2025 11:56:10 +0000","Message-ID":"<173987977025.3276240.4232474153203292983@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":33396,"web_url":"https://patchwork.libcamera.org/comment/33396/","msgid":"<20250219123507.GA15114@pendragon.ideasonboard.com>","date":"2025-02-19T12:35:07","subject":"Re: [PATCH v3 1/2] 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":"Hello,\n\nOn Tue, Feb 18, 2025 at 11:56:10AM +0000, Kieran Bingham wrote:\n> Quoting Stanislaw Gruszka (2025-02-18 09:19:50)\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> > Additionally add helper for checking if the event enabled.\n> > Will be needed later to proper code flow if enabling the event\n> > fail for some reason.\n> \n> I've started contemplating tackling this too - with a 'V4L2Event'\n> wrapper ... but that's overkill while we have only a single event to\n> manage, so I think this is fine for now.\n\nWe don't need a V4L2Event type, I think we can use an uint32_t to make\nthe function more generic, it should be an easy change.\n\n> Reviewed-by: Kieran Bingham \n> \n> > Signed-off-by: Stanislaw Gruszka \n> > ---\n> > include/libcamera/internal/v4l2_device.h | 2 ++\n> > src/libcamera/v4l2_device.cpp | 24 ++++++++++++++++++++++++\n> > 2 files changed, 26 insertions(+)\n> > \n> > diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h\n> > index affe52c2..0d807209 100644\n> > --- a/include/libcamera/internal/v4l2_device.h\n> > +++ b/include/libcamera/internal/v4l2_device.h\n> > @@ -45,6 +45,8 @@ public:\n> > const std::string &deviceNode() const { return deviceNode_; }\n> > std::string devicePath() const;\n> > \n> > + bool supportsFrameStartEvent();\n> > + bool frameStartEnabled() { return frameStartEnabled_; }\n\nThe function should be const.\n\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..219c82f6 100644\n> > --- a/src/libcamera/v4l2_device.cpp\n> > +++ b/src/libcamera/v4l2_device.cpp\n> > @@ -449,6 +449,30 @@ 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> > + };\n> > + event.type = V4L2_EVENT_FRAME_SYNC;\n> > +\n> > + int ret = ioctl(VIDIOC_SUBSCRIBE_EVENT, &event);\n> > + if (ret == 0)\n> > + ioctl(VIDIOC_UNSUBSCRIBE_EVENT, &event);\n> > +\n> > + return ret == 0;\n\nI think the following would be a bit more readable.\n\n \tint ret = ioctl(VIDIOC_SUBSCRIBE_EVENT, &event);\n\tif (ret)\n\t\treturn false;\n\n\tioctl(VIDIOC_UNSUBSCRIBE_EVENT, &event);\n\treturn true;\n\n> > +}\n> > +\n> > +/**\n> > + * \\fn V4L2Device::frameStartEnabled()\n> > + * \\brief Check if frame start event is enabled\n> > + * \\return True if frame start event is enabled, false otherwise\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 8F685C3237\n\tfor ;\n\tWed, 19 Feb 2025 12:35:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A35D068687;\n\tWed, 19 Feb 2025 13:35:24 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D002161861\n\tfor ;\n\tWed, 19 Feb 2025 13:35:22 +0100 (CET)","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 9DE2918D;\n\tWed, 19 Feb 2025 13:33:59 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"nF7onUFd\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1739968439;\n\tbh=vFYilmECUUraSGj76va+DaOYGy+yUrNnhXgOGpCtQRQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=nF7onUFdhX6ANxMOUlHCf72W+PkdKpsMNis1SO9Mzeg53YLSgEh5+X+oHq1ODnLmI\n\tZQaWytOU/dXzjop7570cKHPNe0OzUZJOFgJGqa492p7Tk6fmogsy9emxz73RwCIkT/\n\tUrKG0C7ebChxMqOfkZQd3PVwNC2tIwdJXD++hTyg=","Date":"Wed, 19 Feb 2025 14:35:07 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Cc":"Stanislaw Gruszka ,\n\tlibcamera-devel@lists.libcamera.org,\n\tMilan Zamazal , \n\tNaushir Patuck ,\n\tSakari Ailus ,\n\tHans de Goede ","Subject":"Re: [PATCH v3 1/2] libcamera: v4l2_device: add frame start event\n\thelpers","Message-ID":"<20250219123507.GA15114@pendragon.ideasonboard.com>","References":"<20250218091951.13017-1-stanislaw.gruszka@linux.intel.com>\n\t<20250218091951.13017-2-stanislaw.gruszka@linux.intel.com>\n\t<173987977025.3276240.4232474153203292983@ping.linuxembedded.co.uk>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<173987977025.3276240.4232474153203292983@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":33400,"web_url":"https://patchwork.libcamera.org/comment/33400/","msgid":"","date":"2025-02-20T08:14:49","subject":"Re: [PATCH v3 1/2] libcamera: v4l2_device: add frame start event\n\thelpers","submitter":{"id":211,"url":"https://patchwork.libcamera.org/api/people/211/","name":"Stanislaw Gruszka","email":"stanislaw.gruszka@linux.intel.com"},"content":"Hi Laurent,\n\nOn Wed, Feb 19, 2025 at 02:35:07PM +0200, Laurent Pinchart wrote:\n> > > + bool frameStartEnabled() { return frameStartEnabled_; }\n> \n> The function should be const.\nRight. \n\n> > > +bool V4L2Device::supportsFrameStartEvent()\n> > > +{\n> > > + struct v4l2_event_subscription event {\n> > > + };\n> > > + event.type = V4L2_EVENT_FRAME_SYNC;\n> > > +\n> > > + int ret = ioctl(VIDIOC_SUBSCRIBE_EVENT, &event);\n> > > + if (ret == 0)\n> > > + ioctl(VIDIOC_UNSUBSCRIBE_EVENT, &event);\n> > > +\n> > > + return ret == 0;\n> \n> I think the following would be a bit more readable.\n> \n> \tint ret = ioctl(VIDIOC_SUBSCRIBE_EVENT, &event);\n> \tif (ret)\n> \t\treturn false;\n> \n> \tioctl(VIDIOC_UNSUBSCRIBE_EVENT, &event);\n> \treturn true;\n\nFine, will recode this way in next version of the patches.\n\nRegards\nStanislaw","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 3B4F3BE175\n\tfor ;\n\tThu, 20 Feb 2025 08:15:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 42DA468692;\n\tThu, 20 Feb 2025 09:14:59 +0100 (CET)","from mgamail.intel.com (mgamail.intel.com [192.198.163.17])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EB55D6185D\n\tfor ;\n\tThu, 20 Feb 2025 09:14:55 +0100 (CET)","from orviesa002.jf.intel.com ([10.64.159.142])\n\tby fmvoesa111.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;\n\t20 Feb 2025 00:14:54 -0800","from sgruszka-mobl.ger.corp.intel.com (HELO localhost)\n\t([10.246.8.237]) by orviesa002-auth.jf.intel.com with\n\tESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Feb 2025 00:14:51 -0800"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=intel.com header.i=@intel.com\n\theader.b=\"L3qzs13/\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple;\n\td=intel.com; i=@intel.com; q=dns/txt; s=Intel;\n\tt=1740039296; x=1771575296;\n\th=date:from:to:cc:subject:message-id:references:\n\tmime-version:in-reply-to;\n\tbh=HvuXvX1DmMlHl25ZLJngXkofNyICdxFjYQKHnL2KW+w=;\n\tb=L3qzs13/On6i5CdWEIOpEDT+nAi6aPOsowABX5Wf01y7yugRiBkLm2en\n\tyWpSdzaz5B3r/nqG4EE8rP4rfKsZwfkm25symS2IR8fr7rLIUy2nmhmkJ\n\tBYNb1Tt1rkUQgFEd+8HMsHa8UT6E/QVuCHvgKlAoYEmk+0EIgjuyHJV1D\n\tUfWYwhCdClfHau0a47MMjmeGbsLS73vdg0M5PoxrI/wU5mrfTkx0bnH+4\n\tOoiU/JgO+jW1KmJ+5VZwWsloXDv3tcctkfVoZ1ykaV3KQYedju3vusaQ5\n\tSOGJ7Q/mTTNAvP96iFI71VyWDNqZ7rE/b2u/r5jiwzlUxPbfFoQNaFDJi Q==;","X-CSE-ConnectionGUID":["hHE2h/kfReiI/3pRdvhdtw==","wr/66tLsQXSLxCU3IESHvw=="],"X-CSE-MsgGUID":["4w0uVlB1RIOmG+bASRHmWQ==","Y9YRwVPMQOqAtR2/VzUyuw=="],"X-IronPort-AV":["E=McAfee;i=\"6700,10204,11350\"; a=\"40673726\"","E=Sophos;i=\"6.13,301,1732608000\"; d=\"scan'208\";a=\"40673726\"","E=Sophos;i=\"6.13,301,1732608000\"; d=\"scan'208\";a=\"145824922\""],"X-ExtLoop1":"1","Date":"Thu, 20 Feb 2025 09:14:49 +0100","From":"Stanislaw Gruszka ","To":"Laurent Pinchart ","Cc":"Kieran Bingham ,\n\tlibcamera-devel@lists.libcamera.org,\n\tMilan Zamazal , \n\tNaushir Patuck ,\n\tSakari Ailus ,\n\tHans de Goede ","Subject":"Re: [PATCH v3 1/2] libcamera: v4l2_device: add frame start event\n\thelpers","Message-ID":"","References":"<20250218091951.13017-1-stanislaw.gruszka@linux.intel.com>\n\t<20250218091951.13017-2-stanislaw.gruszka@linux.intel.com>\n\t<173987977025.3276240.4232474153203292983@ping.linuxembedded.co.uk>\n\t<20250219123507.GA15114@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20250219123507.GA15114@pendragon.ideasonboard.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\" "}}]