[libcamera-devel,1/9] libcamera: v4l2_device: Move start of frame detection to V4L2Device

10274 diff mbox series

The V4L2_EVENT_FRAME_SYNC event may occur on both V4L2 video-devices
(V4L2VideoDevice) and sub-devices (V4L2Subdevice). Move the start of
frame detection to the common base class of the two, V4L2Device.

There is no functional change.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
* Changes since v1
- Mark eventAvailable() private instead of protected.
- Remove a blank line.
- Call setFd() instead of duplicating it.
---
 include/libcamera/internal/v4l2_device.h      | 13 +++-
 include/libcamera/internal/v4l2_videodevice.h |  8 --
 src/libcamera/v4l2_device.cpp                 | 76 ++++++++++++++++++-
 src/libcamera/v4l2_videodevice.cpp            | 75 +-----------------
 4 files changed, 87 insertions(+), 85 deletions(-)

Hi Niklas,

On Wed, Oct 28, 2020 at 02:00:43AM +0100, Niklas Söderlund wrote:
> The V4L2_EVENT_FRAME_SYNC event may occur on both V4L2 video-devices
> (V4L2VideoDevice) and sub-devices (V4L2Subdevice). Move the start of
> frame detection to the common base class of the two, V4L2Device.
>
> There is no functional change.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> * Changes since v1
> - Mark eventAvailable() private instead of protected.
> - Remove a blank line.
> - Call setFd() instead of duplicating it.
> ---
>  include/libcamera/internal/v4l2_device.h      | 13 +++-
>  include/libcamera/internal/v4l2_videodevice.h |  8 --
>  src/libcamera/v4l2_device.cpp                 | 76 ++++++++++++++++++-
>  src/libcamera/v4l2_videodevice.cpp            | 75 +-----------------
>  4 files changed, 87 insertions(+), 85 deletions(-)
>
> diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h
> index 722fb72207d74111..295f08f0be6f1980 100644
> --- a/include/libcamera/internal/v4l2_device.h
> +++ b/include/libcamera/internal/v4l2_device.h
> @@ -13,11 +13,15 @@
>
>  #include <linux/videodev2.h>
>
> +#include <libcamera/signal.h>
> +
>  #include "libcamera/internal/log.h"
>  #include "libcamera/internal/v4l2_controls.h"
>
>  namespace libcamera {
>
> +class EventNotifier;
> +
>  class V4L2Device : protected Loggable
>  {
>  public:
> @@ -34,6 +38,9 @@ public:
>  	const std::string &deviceNode() const { return deviceNode_; }
>  	std::string devicePath() const;
>
> +	int setFrameStartEnabled(bool enable);
> +	Signal<uint32_t> frameStart;
> +
>  protected:
>  	V4L2Device(const std::string &deviceNode);
>  	~V4L2Device();
> @@ -44,18 +51,22 @@ protected:
>  	int ioctl(unsigned long request, void *argp);
>
>  	int fd() { return fd_; }
> -

Intentional ?
I see we have an empty line between the different sections in other
headers

>  private:
>  	void listControls();
>  	void updateControls(ControlList *ctrls,
>  			    const struct v4l2_ext_control *v4l2Ctrls,
>  			    unsigned int count);
>
> +	void eventAvailable(EventNotifier *notifier);
> +
>  	std::map<unsigned int, struct v4l2_query_ext_ctrl> controlInfo_;
>  	std::vector<std::unique_ptr<V4L2ControlId>> controlIds_;
>  	ControlInfoMap controls_;
>  	std::string deviceNode_;
>  	int fd_;
> +
> +	EventNotifier *fdEventNotifier_;
> +	bool frameStartEnabled_;
>  };
>
>  } /* namespace libcamera */
> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h
> index 53f6a2d5515b9bb3..cf705ec9cd6ad118 100644
> --- a/include/libcamera/internal/v4l2_videodevice.h
> +++ b/include/libcamera/internal/v4l2_videodevice.h
> @@ -204,9 +204,6 @@ public:
>  	int queueBuffer(FrameBuffer *buffer);
>  	Signal<FrameBuffer *> bufferReady;
>
> -	int setFrameStartEnabled(bool enable);
> -	Signal<uint32_t> frameStart;
> -
>  	int streamOn();
>  	int streamOff();
>
> @@ -240,8 +237,6 @@ private:
>  	void bufferAvailable(EventNotifier *notifier);
>  	FrameBuffer *dequeueBuffer();
>
> -	void eventAvailable(EventNotifier *notifier);
> -
>  	V4L2Capability caps_;
>
>  	enum v4l2_buf_type bufferType_;
> @@ -251,9 +246,6 @@ private:
>  	std::map<unsigned int, FrameBuffer *> queuedBuffers_;
>
>  	EventNotifier *fdBufferNotifier_;
> -	EventNotifier *fdEventNotifier_;
> -
> -	bool frameStartEnabled_;
>  };
>
>  class V4L2M2MDevice
> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> index 31d4dad0ee47b734..36fe62b04003551e 100644
> --- a/src/libcamera/v4l2_device.cpp
> +++ b/src/libcamera/v4l2_device.cpp
> @@ -16,6 +16,8 @@
>  #include <sys/syscall.h>
>  #include <unistd.h>
>
> +#include <libcamera/event_notifier.h>
> +
>  #include "libcamera/internal/log.h"
>  #include "libcamera/internal/sysfs.h"
>  #include "libcamera/internal/utils.h"
> @@ -52,7 +54,8 @@ LOG_DEFINE_CATEGORY(V4L2)
>   * at open() time, and the \a logTag to prefix log messages with.
>   */
>  V4L2Device::V4L2Device(const std::string &deviceNode)
> -	: deviceNode_(deviceNode), fd_(-1)
> +	: deviceNode_(deviceNode), fd_(-1), fdEventNotifier_(nullptr),
> +	  frameStartEnabled_(false)
>  {
>  }
>
> @@ -87,7 +90,7 @@ int V4L2Device::open(unsigned int flags)
>  		return ret;
>  	}
>
> -	fd_ = ret;
> +	setFd(ret);

Is this related to this patch ?
Anyway, the change looks good:
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
   j

>
>  	listControls();
>
> @@ -117,6 +120,10 @@ int V4L2Device::setFd(int fd)
>
>  	fd_ = fd;
>
> +	fdEventNotifier_ = new EventNotifier(fd_, EventNotifier::Exception);
> +	fdEventNotifier_->activated.connect(this, &V4L2Device::eventAvailable);
> +	fdEventNotifier_->setEnabled(false);
> +
>  	return 0;
>  }
>
> @@ -130,6 +137,8 @@ void V4L2Device::close()
>  	if (!isOpen())
>  		return;
>
> +	delete fdEventNotifier_;
> +
>  	if (::close(fd_) < 0)
>  		LOG(V4L2, Error) << "Failed to close V4L2 device: "
>  				 << strerror(errno);
> @@ -396,6 +405,40 @@ std::string V4L2Device::devicePath() const
>  	return path;
>  }
>
> +/**
> + * \brief Enable or disable frame start event notification
> + * \param[in] enable True to enable frame start events, false to disable them
> + *
> + * This function enables or disables generation of frame start events. Once
> + * enabled, the events are signalled through the frameStart signal.
> + *
> + * \return 0 on success, a negative error code otherwise
> + */
> +int V4L2Device::setFrameStartEnabled(bool enable)
> +{
> +	if (frameStartEnabled_ == enable)
> +		return 0;
> +
> +	struct v4l2_event_subscription event{};
> +	event.type = V4L2_EVENT_FRAME_SYNC;
> +
> +	unsigned long request = enable ? VIDIOC_SUBSCRIBE_EVENT
> +			      : VIDIOC_UNSUBSCRIBE_EVENT;
> +	int ret = ioctl(request, &event);
> +	if (enable && ret)
> +		return ret;
> +
> +	fdEventNotifier_->setEnabled(enable);
> +	frameStartEnabled_ = enable;
> +
> +	return ret;
> +}
> +
> +/**
> + * \var V4L2Device::frameStart
> + * \brief A Signal emitted when capture of a frame has started
> + */
> +
>  /**
>   * \brief Perform an IOCTL system call on the device node
>   * \param[in] request The IOCTL request code
> @@ -519,4 +562,33 @@ void V4L2Device::updateControls(ControlList *ctrls,
>  	}
>  }
>
> +/**
> + * \brief Slot to handle V4L2 events from the V4L2 device
> + * \param[in] notifier The event notifier
> + *
> + * When this slot is called, a V4L2 event is available to be dequeued from the
> + * device.
> + */
> +void V4L2Device::eventAvailable([[maybe_unused]] EventNotifier *notifier)
> +{
> +	struct v4l2_event event{};
> +	int ret = ioctl(VIDIOC_DQEVENT, &event);
> +	if (ret < 0) {
> +		LOG(V4L2, Error)
> +			<< "Failed to dequeue event, disabling event notifier";
> +		fdEventNotifier_->setEnabled(false);
> +		return;
> +	}
> +
> +	if (event.type != V4L2_EVENT_FRAME_SYNC) {
> +		LOG(V4L2, Error)
> +			<< "Spurious event (" << event.type
> +			<< "), disabling event notifier";
> +		fdEventNotifier_->setEnabled(false);
> +		return;
> +	}
> +
> +	frameStart.emit(event.u.frame_sync.frame_sequence);
> +}
> +
>  } /* namespace libcamera */
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 36d7d9a0f27a103f..012d9bc73f30ad09 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -472,8 +472,7 @@ const std::string V4L2DeviceFormat::toString() const
>   * \param[in] deviceNode The file-system path to the video device node
>   */
>  V4L2VideoDevice::V4L2VideoDevice(const std::string &deviceNode)
> -	: V4L2Device(deviceNode), cache_(nullptr), fdBufferNotifier_(nullptr),
> -	  fdEventNotifier_(nullptr), frameStartEnabled_(false)
> +	: V4L2Device(deviceNode), cache_(nullptr), fdBufferNotifier_(nullptr)
>  {
>  	/*
>  	 * We default to an MMAP based CAPTURE video device, however this will
> @@ -566,10 +565,6 @@ int V4L2VideoDevice::open()
>  	fdBufferNotifier_->activated.connect(this, &V4L2VideoDevice::bufferAvailable);
>  	fdBufferNotifier_->setEnabled(false);
>
> -	fdEventNotifier_ = new EventNotifier(fd(), EventNotifier::Exception);
> -	fdEventNotifier_->activated.connect(this, &V4L2VideoDevice::eventAvailable);
> -	fdEventNotifier_->setEnabled(false);
> -
>  	LOG(V4L2, Debug)
>  		<< "Opened device " << caps_.bus_info() << ": "
>  		<< caps_.driver() << ": " << caps_.card();
> @@ -659,10 +654,6 @@ int V4L2VideoDevice::open(int handle, enum v4l2_buf_type type)
>  	fdBufferNotifier_->activated.connect(this, &V4L2VideoDevice::bufferAvailable);
>  	fdBufferNotifier_->setEnabled(false);
>
> -	fdEventNotifier_ = new EventNotifier(fd(), EventNotifier::Exception);
> -	fdEventNotifier_->activated.connect(this, &V4L2VideoDevice::eventAvailable);
> -	fdEventNotifier_->setEnabled(false);
> -
>  	LOG(V4L2, Debug)
>  		<< "Opened device " << caps_.bus_info() << ": "
>  		<< caps_.driver() << ": " << caps_.card();
> @@ -680,7 +671,6 @@ void V4L2VideoDevice::close()
>
>  	releaseBuffers();
>  	delete fdBufferNotifier_;
> -	delete fdEventNotifier_;
>
>  	V4L2Device::close();
>  }
> @@ -1533,74 +1523,11 @@ FrameBuffer *V4L2VideoDevice::dequeueBuffer()
>  	return buffer;
>  }
>
> -/**
> - * \brief Slot to handle V4L2 events from the V4L2 video device
> - * \param[in] notifier The event notifier
> - *
> - * When this slot is called, a V4L2 event is available to be dequeued from the
> - * device.
> - */
> -void V4L2VideoDevice::eventAvailable([[maybe_unused]] EventNotifier *notifier)
> -{
> -	struct v4l2_event event{};
> -	int ret = ioctl(VIDIOC_DQEVENT, &event);
> -	if (ret < 0) {
> -		LOG(V4L2, Error)
> -			<< "Failed to dequeue event, disabling event notifier";
> -		fdEventNotifier_->setEnabled(false);
> -		return;
> -	}
> -
> -	if (event.type != V4L2_EVENT_FRAME_SYNC) {
> -		LOG(V4L2, Error)
> -			<< "Spurious event (" << event.type
> -			<< "), disabling event notifier";
> -		fdEventNotifier_->setEnabled(false);
> -		return;
> -	}
> -
> -	frameStart.emit(event.u.frame_sync.frame_sequence);
> -}
> -
>  /**
>   * \var V4L2VideoDevice::bufferReady
>   * \brief A Signal emitted when a framebuffer completes
>   */
>
> -/**
> - * \brief Enable or disable frame start event notification
> - * \param[in] enable True to enable frame start events, false to disable them
> - *
> - * This function enables or disables generation of frame start events. Once
> - * enabled, the events are signalled through the frameStart signal.
> - *
> - * \return 0 on success, a negative error code otherwise
> - */
> -int V4L2VideoDevice::setFrameStartEnabled(bool enable)
> -{
> -	if (frameStartEnabled_ == enable)
> -		return 0;
> -
> -	struct v4l2_event_subscription event{};
> -	event.type = V4L2_EVENT_FRAME_SYNC;
> -
> -	unsigned long request = enable ? VIDIOC_SUBSCRIBE_EVENT
> -			      : VIDIOC_UNSUBSCRIBE_EVENT;
> -	int ret = ioctl(request, &event);
> -	if (enable && ret)
> -		return ret;
> -
> -	fdEventNotifier_->setEnabled(enable);
> -	frameStartEnabled_ = enable;
> -
> -	return ret;
> -}
> -
> -/**
> - * \var V4L2VideoDevice::frameStart
> - * \brief A Signal emitted when capture of a frame has started
> - */
> -
>  /**
>   * \brief Start the video stream
>   * \return 0 on success or a negative error code otherwise
> --
> 2.29.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Hi Jacopo,

Thanks for your comments.

On 2020-11-04 16:33:24 +0100, Jacopo Mondi wrote:
> Hi Niklas,
> 
> On Wed, Oct 28, 2020 at 02:00:43AM +0100, Niklas Söderlund wrote:
> > The V4L2_EVENT_FRAME_SYNC event may occur on both V4L2 video-devices
> > (V4L2VideoDevice) and sub-devices (V4L2Subdevice). Move the start of
> > frame detection to the common base class of the two, V4L2Device.
> >
> > There is no functional change.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> > * Changes since v1
> > - Mark eventAvailable() private instead of protected.
> > - Remove a blank line.
> > - Call setFd() instead of duplicating it.
> > ---
> >  include/libcamera/internal/v4l2_device.h      | 13 +++-
> >  include/libcamera/internal/v4l2_videodevice.h |  8 --
> >  src/libcamera/v4l2_device.cpp                 | 76 ++++++++++++++++++-
> >  src/libcamera/v4l2_videodevice.cpp            | 75 +-----------------
> >  4 files changed, 87 insertions(+), 85 deletions(-)
> >
> > diff --git a/include/libcamera/internal/v4l2_device.h b/include/libcamera/internal/v4l2_device.h
> > index 722fb72207d74111..295f08f0be6f1980 100644
> > --- a/include/libcamera/internal/v4l2_device.h
> > +++ b/include/libcamera/internal/v4l2_device.h
> > @@ -13,11 +13,15 @@
> >
> >  #include <linux/videodev2.h>
> >
> > +#include <libcamera/signal.h>
> > +
> >  #include "libcamera/internal/log.h"
> >  #include "libcamera/internal/v4l2_controls.h"
> >
> >  namespace libcamera {
> >
> > +class EventNotifier;
> > +
> >  class V4L2Device : protected Loggable
> >  {
> >  public:
> > @@ -34,6 +38,9 @@ public:
> >  	const std::string &deviceNode() const { return deviceNode_; }
> >  	std::string devicePath() const;
> >
> > +	int setFrameStartEnabled(bool enable);
> > +	Signal<uint32_t> frameStart;
> > +
> >  protected:
> >  	V4L2Device(const std::string &deviceNode);
> >  	~V4L2Device();
> > @@ -44,18 +51,22 @@ protected:
> >  	int ioctl(unsigned long request, void *argp);
> >
> >  	int fd() { return fd_; }
> > -
> 
> Intentional ?
> I see we have an empty line between the different sections in other
> headers

Nope, this is a typo, will fix.

> 
> >  private:
> >  	void listControls();
> >  	void updateControls(ControlList *ctrls,
> >  			    const struct v4l2_ext_control *v4l2Ctrls,
> >  			    unsigned int count);
> >
> > +	void eventAvailable(EventNotifier *notifier);
> > +
> >  	std::map<unsigned int, struct v4l2_query_ext_ctrl> controlInfo_;
> >  	std::vector<std::unique_ptr<V4L2ControlId>> controlIds_;
> >  	ControlInfoMap controls_;
> >  	std::string deviceNode_;
> >  	int fd_;
> > +
> > +	EventNotifier *fdEventNotifier_;
> > +	bool frameStartEnabled_;
> >  };
> >
> >  } /* namespace libcamera */
> > diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h
> > index 53f6a2d5515b9bb3..cf705ec9cd6ad118 100644
> > --- a/include/libcamera/internal/v4l2_videodevice.h
> > +++ b/include/libcamera/internal/v4l2_videodevice.h
> > @@ -204,9 +204,6 @@ public:
> >  	int queueBuffer(FrameBuffer *buffer);
> >  	Signal<FrameBuffer *> bufferReady;
> >
> > -	int setFrameStartEnabled(bool enable);
> > -	Signal<uint32_t> frameStart;
> > -
> >  	int streamOn();
> >  	int streamOff();
> >
> > @@ -240,8 +237,6 @@ private:
> >  	void bufferAvailable(EventNotifier *notifier);
> >  	FrameBuffer *dequeueBuffer();
> >
> > -	void eventAvailable(EventNotifier *notifier);
> > -
> >  	V4L2Capability caps_;
> >
> >  	enum v4l2_buf_type bufferType_;
> > @@ -251,9 +246,6 @@ private:
> >  	std::map<unsigned int, FrameBuffer *> queuedBuffers_;
> >
> >  	EventNotifier *fdBufferNotifier_;
> > -	EventNotifier *fdEventNotifier_;
> > -
> > -	bool frameStartEnabled_;
> >  };
> >
> >  class V4L2M2MDevice
> > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp
> > index 31d4dad0ee47b734..36fe62b04003551e 100644
> > --- a/src/libcamera/v4l2_device.cpp
> > +++ b/src/libcamera/v4l2_device.cpp
> > @@ -16,6 +16,8 @@
> >  #include <sys/syscall.h>
> >  #include <unistd.h>
> >
> > +#include <libcamera/event_notifier.h>
> > +
> >  #include "libcamera/internal/log.h"
> >  #include "libcamera/internal/sysfs.h"
> >  #include "libcamera/internal/utils.h"
> > @@ -52,7 +54,8 @@ LOG_DEFINE_CATEGORY(V4L2)
> >   * at open() time, and the \a logTag to prefix log messages with.
> >   */
> >  V4L2Device::V4L2Device(const std::string &deviceNode)
> > -	: deviceNode_(deviceNode), fd_(-1)
> > +	: deviceNode_(deviceNode), fd_(-1), fdEventNotifier_(nullptr),
> > +	  frameStartEnabled_(false)
> >  {
> >  }
> >
> > @@ -87,7 +90,7 @@ int V4L2Device::open(unsigned int flags)
> >  		return ret;
> >  	}
> >
> > -	fd_ = ret;
> > +	setFd(ret);
> 
> Is this related to this patch ?

Yes, as setFd() is extended below to create and enable the notifier we 
can either duplicate that code here or call setFd().

> Anyway, the change looks good:
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks!

> 
> Thanks
>    j
> 
> >
> >  	listControls();
> >
> > @@ -117,6 +120,10 @@ int V4L2Device::setFd(int fd)
> >
> >  	fd_ = fd;
> >
> > +	fdEventNotifier_ = new EventNotifier(fd_, EventNotifier::Exception);
> > +	fdEventNotifier_->activated.connect(this, &V4L2Device::eventAvailable);
> > +	fdEventNotifier_->setEnabled(false);
> > +
> >  	return 0;
> >  }
> >
> > @@ -130,6 +137,8 @@ void V4L2Device::close()
> >  	if (!isOpen())
> >  		return;
> >
> > +	delete fdEventNotifier_;
> > +
> >  	if (::close(fd_) < 0)
> >  		LOG(V4L2, Error) << "Failed to close V4L2 device: "
> >  				 << strerror(errno);
> > @@ -396,6 +405,40 @@ std::string V4L2Device::devicePath() const
> >  	return path;
> >  }
> >
> > +/**
> > + * \brief Enable or disable frame start event notification
> > + * \param[in] enable True to enable frame start events, false to disable them
> > + *
> > + * This function enables or disables generation of frame start events. Once
> > + * enabled, the events are signalled through the frameStart signal.
> > + *
> > + * \return 0 on success, a negative error code otherwise
> > + */
> > +int V4L2Device::setFrameStartEnabled(bool enable)
> > +{
> > +	if (frameStartEnabled_ == enable)
> > +		return 0;
> > +
> > +	struct v4l2_event_subscription event{};
> > +	event.type = V4L2_EVENT_FRAME_SYNC;
> > +
> > +	unsigned long request = enable ? VIDIOC_SUBSCRIBE_EVENT
> > +			      : VIDIOC_UNSUBSCRIBE_EVENT;
> > +	int ret = ioctl(request, &event);
> > +	if (enable && ret)
> > +		return ret;
> > +
> > +	fdEventNotifier_->setEnabled(enable);
> > +	frameStartEnabled_ = enable;
> > +
> > +	return ret;
> > +}
> > +
> > +/**
> > + * \var V4L2Device::frameStart
> > + * \brief A Signal emitted when capture of a frame has started
> > + */
> > +
> >  /**
> >   * \brief Perform an IOCTL system call on the device node
> >   * \param[in] request The IOCTL request code
> > @@ -519,4 +562,33 @@ void V4L2Device::updateControls(ControlList *ctrls,
> >  	}
> >  }
> >
> > +/**
> > + * \brief Slot to handle V4L2 events from the V4L2 device
> > + * \param[in] notifier The event notifier
> > + *
> > + * When this slot is called, a V4L2 event is available to be dequeued from the
> > + * device.
> > + */
> > +void V4L2Device::eventAvailable([[maybe_unused]] EventNotifier *notifier)
> > +{
> > +	struct v4l2_event event{};
> > +	int ret = ioctl(VIDIOC_DQEVENT, &event);
> > +	if (ret < 0) {
> > +		LOG(V4L2, Error)
> > +			<< "Failed to dequeue event, disabling event notifier";
> > +		fdEventNotifier_->setEnabled(false);
> > +		return;
> > +	}
> > +
> > +	if (event.type != V4L2_EVENT_FRAME_SYNC) {
> > +		LOG(V4L2, Error)
> > +			<< "Spurious event (" << event.type
> > +			<< "), disabling event notifier";
> > +		fdEventNotifier_->setEnabled(false);
> > +		return;
> > +	}
> > +
> > +	frameStart.emit(event.u.frame_sync.frame_sequence);
> > +}
> > +
> >  } /* namespace libcamera */
> > diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> > index 36d7d9a0f27a103f..012d9bc73f30ad09 100644
> > --- a/src/libcamera/v4l2_videodevice.cpp
> > +++ b/src/libcamera/v4l2_videodevice.cpp
> > @@ -472,8 +472,7 @@ const std::string V4L2DeviceFormat::toString() const
> >   * \param[in] deviceNode The file-system path to the video device node
> >   */
> >  V4L2VideoDevice::V4L2VideoDevice(const std::string &deviceNode)
> > -	: V4L2Device(deviceNode), cache_(nullptr), fdBufferNotifier_(nullptr),
> > -	  fdEventNotifier_(nullptr), frameStartEnabled_(false)
> > +	: V4L2Device(deviceNode), cache_(nullptr), fdBufferNotifier_(nullptr)
> >  {
> >  	/*
> >  	 * We default to an MMAP based CAPTURE video device, however this will
> > @@ -566,10 +565,6 @@ int V4L2VideoDevice::open()
> >  	fdBufferNotifier_->activated.connect(this, &V4L2VideoDevice::bufferAvailable);
> >  	fdBufferNotifier_->setEnabled(false);
> >
> > -	fdEventNotifier_ = new EventNotifier(fd(), EventNotifier::Exception);
> > -	fdEventNotifier_->activated.connect(this, &V4L2VideoDevice::eventAvailable);
> > -	fdEventNotifier_->setEnabled(false);
> > -
> >  	LOG(V4L2, Debug)
> >  		<< "Opened device " << caps_.bus_info() << ": "
> >  		<< caps_.driver() << ": " << caps_.card();
> > @@ -659,10 +654,6 @@ int V4L2VideoDevice::open(int handle, enum v4l2_buf_type type)
> >  	fdBufferNotifier_->activated.connect(this, &V4L2VideoDevice::bufferAvailable);
> >  	fdBufferNotifier_->setEnabled(false);
> >
> > -	fdEventNotifier_ = new EventNotifier(fd(), EventNotifier::Exception);
> > -	fdEventNotifier_->activated.connect(this, &V4L2VideoDevice::eventAvailable);
> > -	fdEventNotifier_->setEnabled(false);
> > -
> >  	LOG(V4L2, Debug)
> >  		<< "Opened device " << caps_.bus_info() << ": "
> >  		<< caps_.driver() << ": " << caps_.card();
> > @@ -680,7 +671,6 @@ void V4L2VideoDevice::close()
> >
> >  	releaseBuffers();
> >  	delete fdBufferNotifier_;
> > -	delete fdEventNotifier_;
> >
> >  	V4L2Device::close();
> >  }
> > @@ -1533,74 +1523,11 @@ FrameBuffer *V4L2VideoDevice::dequeueBuffer()
> >  	return buffer;
> >  }
> >
> > -/**
> > - * \brief Slot to handle V4L2 events from the V4L2 video device
> > - * \param[in] notifier The event notifier
> > - *
> > - * When this slot is called, a V4L2 event is available to be dequeued from the
> > - * device.
> > - */
> > -void V4L2VideoDevice::eventAvailable([[maybe_unused]] EventNotifier *notifier)
> > -{
> > -	struct v4l2_event event{};
> > -	int ret = ioctl(VIDIOC_DQEVENT, &event);
> > -	if (ret < 0) {
> > -		LOG(V4L2, Error)
> > -			<< "Failed to dequeue event, disabling event notifier";
> > -		fdEventNotifier_->setEnabled(false);
> > -		return;
> > -	}
> > -
> > -	if (event.type != V4L2_EVENT_FRAME_SYNC) {
> > -		LOG(V4L2, Error)
> > -			<< "Spurious event (" << event.type
> > -			<< "), disabling event notifier";
> > -		fdEventNotifier_->setEnabled(false);
> > -		return;
> > -	}
> > -
> > -	frameStart.emit(event.u.frame_sync.frame_sequence);
> > -}
> > -
> >  /**
> >   * \var V4L2VideoDevice::bufferReady
> >   * \brief A Signal emitted when a framebuffer completes
> >   */
> >
> > -/**
> > - * \brief Enable or disable frame start event notification
> > - * \param[in] enable True to enable frame start events, false to disable them
> > - *
> > - * This function enables or disables generation of frame start events. Once
> > - * enabled, the events are signalled through the frameStart signal.
> > - *
> > - * \return 0 on success, a negative error code otherwise
> > - */
> > -int V4L2VideoDevice::setFrameStartEnabled(bool enable)
> > -{
> > -	if (frameStartEnabled_ == enable)
> > -		return 0;
> > -
> > -	struct v4l2_event_subscription event{};
> > -	event.type = V4L2_EVENT_FRAME_SYNC;
> > -
> > -	unsigned long request = enable ? VIDIOC_SUBSCRIBE_EVENT
> > -			      : VIDIOC_UNSUBSCRIBE_EVENT;
> > -	int ret = ioctl(request, &event);
> > -	if (enable && ret)
> > -		return ret;
> > -
> > -	fdEventNotifier_->setEnabled(enable);
> > -	frameStartEnabled_ = enable;
> > -
> > -	return ret;
> > -}
> > -
> > -/**
> > - * \var V4L2VideoDevice::frameStart
> > - * \brief A Signal emitted when capture of a frame has started
> > - */
> > -
> >  /**
> >   * \brief Start the video stream
> >   * \return 0 on success or a negative error code otherwise
> > --
> > 2.29.1
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel@lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel