[libcamera-devel] [PATCH] libcamera: v4l2_videodevice: Identify non-zero stream starts

[Jacopo Mondi]

Hi Kieran

On Mon, Jun 13, 2022 at 12:35:28PM +0200, Kieran Bingham via libcamera-devel wrote:
> V4L2 Video devices should always start streaming from index zero.
> Identify and report a warning if they don't, and correct for any offset.
>
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>
> I'm not yet sure if the Warning print is perhaps just unhelpful.
> It doesn't really affect us if we have this work around in - so we can
> be silent, and I'm not yet sure if V4L2 has any 'requirement' that it
> should start at 0 at every stream on.
>

Let's keep the warning! As the requirement is documented, as reported
by Dave, I wonder if we should not "fix your kernel driver" as we do
for missing sensor driver features.

Anyway, the patch is good
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
   j

>  include/libcamera/internal/v4l2_videodevice.h |  1 +
>  src/libcamera/v4l2_videodevice.cpp            | 15 +++++++++++++++
>  2 files changed, 16 insertions(+)
>
> diff --git a/include/libcamera/internal/v4l2_videodevice.h b/include/libcamera/internal/v4l2_videodevice.h
> index 23f37f83f8e2..8525acbc558d 100644
> --- a/include/libcamera/internal/v4l2_videodevice.h
> +++ b/include/libcamera/internal/v4l2_videodevice.h
> @@ -276,6 +276,7 @@ private:
>  	EventNotifier *fdBufferNotifier_;
>
>  	State state_;
> +	std::optional<unsigned int> firstFrame_;
>
>  	Timer watchdog_;
>  	utils::Duration watchdogDuration_;
> diff --git a/src/libcamera/v4l2_videodevice.cpp b/src/libcamera/v4l2_videodevice.cpp
> index 430715afd554..63911339f96e 100644
> --- a/src/libcamera/v4l2_videodevice.cpp
> +++ b/src/libcamera/v4l2_videodevice.cpp
> @@ -1771,6 +1771,19 @@ FrameBuffer *V4L2VideoDevice::dequeueBuffer()
>  	if (V4L2_TYPE_IS_OUTPUT(buf.type))
>  		return buffer;
>
> +	/*
> +	 * Detect kernel drivers which do not reset the sequence number to zero
> +	 * on stream start.
> +	 */
> +	if (!firstFrame_) {
> +		if (buf.sequence)
> +			LOG(V4L2, Warning)
> +				<< "Zero sequence expected for first frame (got "
> +				<< buf.sequence << ")";
> +		firstFrame_ = buf.sequence;
> +	}
> +	buffer->metadata_.sequence -= firstFrame_.value();
> +
>  	unsigned int numV4l2Planes = multiPlanar ? buf.length : 1;
>  	FrameMetadata &metadata = buffer->metadata_;
>
> @@ -1847,6 +1860,8 @@ int V4L2VideoDevice::streamOn()
>  {
>  	int ret;
>
> +	firstFrame_.reset();
> +
>  	ret = ioctl(VIDIOC_STREAMON, &bufferType_);
>  	if (ret < 0) {
>  		LOG(V4L2, Error)
> --
> 2.25.1
>