[PATCH v6 1/5] libcamera: v4l2_device: add frame start event helpers

[Stefan Klug]

Hi Stanislaw,

Thank you for the patch. 

On Wed, Mar 05, 2025 at 02:52:52PM +0100, Stanislaw Gruszka wrote:
> Add helper to check if frame start event are supported by subdevice.
> Since kernel does not have interface to query supported events
> use subscribe interface.
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com> # v3
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com> # v5
> Signed-off-by: Stanislaw Gruszka <stanislaw.gruszka at linux.intel.com>
> ---
>  include/libcamera/internal/v4l2_device.h |  1 +
>  src/libcamera/v4l2_device.cpp            | 18 ++++++++++++++++++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h
> index affe52c2..a647c96a 100644
> --- a/include/libcamera/internal/v4l2_device.h
> +++ b/include/libcamera/internal/v4l2_device.h
> @@ -45,6 +45,7 @@ public:
>  	const std::string &deviceNode() const { return deviceNode_; }
>  	std::string devicePath() const;
>  
> +	bool supportsFrameStartEvent();
>  	int setFrameStartEnabled(bool enable);
>  	Signal<uint32_t> frameStart;
>  
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 2f65a43a..c3e9cd4a 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -449,6 +449,24 @@ std::string V4L2Device::devicePath() const
>  	return path;
>  }
>  
> +/**
> + * \brief Check if frame start event is supported
> + * \return True if frame start event is supported, false otherwise
> + */
> +
> +bool V4L2Device::supportsFrameStartEvent()
> +{
> +	struct v4l2_event_subscription event{};
> +	event.type = V4L2_EVENT_FRAME_SYNC;
> +
> +	int ret = ioctl(VIDIOC_SUBSCRIBE_EVENT, &event);
> +	if (ret)
> +		return false;
> +
> +	ioctl(VIDIOC_UNSUBSCRIBE_EVENT, &event);

Maybe I'm missing something. To me it appears that I must not call this
function while frame start events are enabled, otherwise these events
would then also be unsubscribed as the subscription is per fd - or am I
wrong here? In the current use-cases that does no harm because
supportFrameStart() is always called before enabling frame start events
and never after. But it is an unexpected restriction on the class
interface. Should we mention that in the docs? Caching the result
internally and also filling it from within setFrameStartEnabled() might
be an overkill?

Best regards,
Stefan

> +	return true;
> +}
> +
>  /**
>   * \brief Enable or disable frame start event notification
>   * \param[in] enable True to enable frame start events, false to disable them
> -- 
> 2.43.0
>