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

[Kieran Bingham]

Quoting Stefan Klug (2025-03-31 16:12:58)
> 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?

For what it's worth, I like this approach keeping the 'test' distinctly
separated.

I can see other events I might need to subscribe to in the future - and
I think I'd batch them so they all get checked at startup.

Definitely for sure - we can only 'test' if the event is available
before we subscribe, but with the current limitations, I think this is
the right thing to do.

--
Kieran

> 
> 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
> >