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

[Dave Stevenson]

Hi Kieran

On Mon, 13 Jun 2022 at 11:34, Kieran Bingham via libcamera-devel
<libcamera-devel at lists.libcamera.org> 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.

[1] sequence
"The count starts at zero and includes dropped or repeated frames"

So if the sensor driver has set a value via subdev sensor ops
g_skip_frames[2], shouldn't it include that count? Or is skipping
considered different from dropping?

(I probably ought to add support for g_skip_frames to the Unicam
driver at some point).

  Dave

[1] https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/buffer.html#struct-v4l2-buffer
[2] https://www.kernel.org/doc/html/latest/driver-api/media/v4l2-subdev.html#c.v4l2_subdev_sensor_ops

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