[libcamera-devel] [PATCH 2/2] libcamera: v4l2_videodevice: Add support for frame start events

[Jacopo Mondi]

Hi Laurent,

On Sun, Apr 26, 2020 at 03:43:09AM +0300, Laurent Pinchart wrote:
> Extend the V4L2VideoDevice to support notifying of frame start events.
> The events are received from the device through the V4L2 event API, and
> passed to users of the class through a signal.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/libcamera/include/v4l2_videodevice.h |  8 +++
>  src/libcamera/v4l2_videodevice.cpp       | 75 +++++++++++++++++++++++-
>  2 files changed, 82 insertions(+), 1 deletion(-)
>
> diff --git a/src/libcamera/include/v4l2_videodevice.h b/src/libcamera/include/v4l2_videodevice.h
> index ee63d28da734..a0409e59c08f 100644
> --- a/src/libcamera/include/v4l2_videodevice.h
> +++ b/src/libcamera/include/v4l2_videodevice.h
> @@ -224,6 +224,9 @@ public:
>  	int queueBuffer(FrameBuffer *buffer);
>  	Signal<FrameBuffer *> bufferReady;
>
> +	int setFrameStartEnabled(bool enable);
> +	Signal<uint32_t> frameStart;
> +
>  	int streamOn();
>  	int streamOff();
>
> @@ -262,6 +265,8 @@ private:
>  	void bufferAvailable(EventNotifier *notifier);
>  	FrameBuffer *dequeueBuffer();
>
> +	void eventAvailable(EventNotifier *notifier);
> +
>  	V4L2Capability caps_;
>
>  	enum v4l2_buf_type bufferType_;
> @@ -271,6 +276,9 @@ private:
>  	std::map<unsigned int, FrameBuffer *> queuedBuffers_;
>
>  	EventNotifier *fdBufferNotifier_;
> +	EventNotifier *fdEventNotifier_;
> +
> +	bool frameStartEnabled_;
>  };
>
>  class V4L2M2MDevice
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 18a71e4f8a7f..45aedb1a9490 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -544,7 +544,8 @@ 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)
> +	: V4L2Device(deviceNode), cache_(nullptr), fdBufferNotifier_(nullptr),
> +	  fdEventNotifier_(nullptr), frameStartEnabled_(false)
>  {
>  	/*
>  	 * We default to an MMAP based CAPTURE video device, however this will
> @@ -637,6 +638,10 @@ 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();
> @@ -726,6 +731,10 @@ 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();
> @@ -743,6 +752,7 @@ void V4L2VideoDevice::close()
>
>  	releaseBuffers();
>  	delete fdBufferNotifier_;
> +	delete fdEventNotifier_;
>
>  	V4L2Device::close();
>  }
> @@ -1570,11 +1580,74 @@ 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(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);

Should we stop emitting events if a spurious one is received ?

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

to disable 'it' ?

> + *
> + * 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;

There's a very tiny window here that could make us miss a signal.
Enabling the notifier before instructing the video device to report
events would solve it. It will need handling the error path a bit more
carefully though.

With this considered
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j

> +
> +	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
> --
> Regards,
>
> Laurent Pinchart
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel