Re: [PATCH 1/3] media: videobuf2: Add a transfer error event

[Nicolas Dufresne <nicolas@ndufresne.ca>]

Hi Micheal,

thanks for your work, I have some questions below ...

Le mardi 28 juin 2022 à 20:00 +0200, Michael Rodin a écrit :
> From: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> 
> Add a new V4L2_EVENT_XFER_ERROR event to signal if an error happens during
> video transfer.
> 
> The use-case that sparked this new event is to signal to the video
> device driver that an error has happen on the CSI-2 bus from the CSI-2
> receiver subdevice.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> [mrodin@de.adit-jv.com: adapted information what to do if this new event is received]
> Signed-off-by: Michael Rodin <mrodin@de.adit-jv.com>
> ---
>  .../userspace-api/media/v4l/vidioc-dqevent.rst         | 10 ++++++++++
>  .../userspace-api/media/videodev2.h.rst.exceptions     |  1 +
>  include/uapi/linux/videodev2.h                         |  1 +
>  3 files changed, 12 insertions(+)
> 
> diff --git a/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst b/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
> index 6eb40073c906..3cf0b4859784 100644
> --- a/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
> +++ b/Documentation/userspace-api/media/v4l/vidioc-dqevent.rst
> @@ -182,6 +182,16 @@ call.
>  	the regions changes. This event has a struct
>  	:c:type:`v4l2_event_motion_det`
>  	associated with it.
> +    * - ``V4L2_EVENT_XFER_ERROR``

I'm not sure why this event is specific to XFER. Is there uses cases were a
future implementation would have both XFER and RECEIVER error ?

> +      - 7
> +      - This event is triggered when an transfer error is detected while
> +	streaming. For example if an error is detected on a video bus in
> +	the pipeline. If a driver receives this event from an upstream
> +	subdevice, it has to forward the event to userspace. The streaming
> +	application has to check if the transfer error is unrecoverable,
> +	i.e. no new buffers can be dequeued from the kernel after the
> +	expected time. If the error is unrecoverable, the streaming
> +	application should restart streaming if it wants to continue.

The process to determine if an error is recoverable or not isn't clear to me. As
an application developer, I would not know what to do here. Recoverable error
already have a designed mechanism, it consist of marking done a buffer with the
flag V4L2_BUF_FLAG_ERROR. I would like to understand what the existing mechanism
needed to be replaced, and the placement should be documented.

Nicolas

>      * - ``V4L2_EVENT_PRIVATE_START``
>        - 0x08000000
>        - Base event number for driver-private events.
> diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> index 9cbb7a0c354a..25bde61a1519 100644
> --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions
> @@ -500,6 +500,7 @@ replace define V4L2_EVENT_CTRL event-type
>  replace define V4L2_EVENT_FRAME_SYNC event-type
>  replace define V4L2_EVENT_SOURCE_CHANGE event-type
>  replace define V4L2_EVENT_MOTION_DET event-type
> +replace define V4L2_EVENT_XFER_ERROR event-type
>  replace define V4L2_EVENT_PRIVATE_START event-type
>  
>  replace define V4L2_EVENT_CTRL_CH_VALUE ctrl-changes-flags
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 5311ac4fde35..44db724d4541 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -2385,6 +2385,7 @@ struct v4l2_streamparm {
>  #define V4L2_EVENT_FRAME_SYNC			4
>  #define V4L2_EVENT_SOURCE_CHANGE		5
>  #define V4L2_EVENT_MOTION_DET			6
> +#define V4L2_EVENT_XFER_ERROR			7
>  #define V4L2_EVENT_PRIVATE_START		0x08000000
>  
>  /* Payload for V4L2_EVENT_VSYNC */