[RFC PATCH] media: Documentation: Drop deprecated bytesused == 0

[RFC PATCH] media: Documentation: Drop deprecated bytesused == 0

[Laurent Pinchart]

From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

The V4L2 API historically allowed buffers to be queued with bytesused
set to 0 on output devices, in which case the driver would use the
buffer length. This behaviour is deprecated, and videobuf2 prints a
warning message in the kernel log. Drop it from the documentation.

Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---

This patch aims at starting (or continuing) the discussion related to
handling of bytesused == 0 on video output devices. videobuf2 currently
printes a warning that indicates the behaviour is deprecated, but the
API documentation allows it explicitly. This discrepency bothers me and
I think we should fix it.

We probably won't be able to drop support for the deprecated behaviour
in the near future, if ever. If we want to push applications to get
fixed, we may want to replace the dev_warn() with a WARN_ONCE(), which
could be done in a v2 of this patch, or in another patch.

Another option would be to consider that the behaviour is fine and keep
official support for it in the documentation, in which case vb2 should
stop warning about it. This wouldn't be my preference.

---
 Documentation/userspace-api/media/v4l/buffer.rst | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/Documentation/userspace-api/media/v4l/buffer.rst b/Documentation/userspace-api/media/v4l/buffer.rst
index 4638ec64db00..04dec3e570ed 100644
--- a/Documentation/userspace-api/media/v4l/buffer.rst
+++ b/Documentation/userspace-api/media/v4l/buffer.rst
@@ -187,10 +187,8 @@ struct v4l2_buffer
 	on the negotiated data format and may change with each buffer for
 	compressed variable size data like JPEG images. Drivers must set
 	this field when ``type`` refers to a capture stream, applications
-	when it refers to an output stream. If the application sets this
-	to 0 for an output stream, then ``bytesused`` will be set to the
-	size of the buffer (see the ``length`` field of this struct) by
-	the driver. For multiplanar formats this field is ignored and the
+	when it refers to an output stream. For multiplanar formats this field
+        is ignored and the
 	``planes`` pointer is used instead.
     * - __u32
       - ``flags``
@@ -327,10 +325,7 @@ struct v4l2_plane
       - ``bytesused``
       - The number of bytes occupied by data in the plane (its payload).
 	Drivers must set this field when ``type`` refers to a capture
-	stream, applications when it refers to an output stream. If the
-	application sets this to 0 for an output stream, then
-	``bytesused`` will be set to the size of the plane (see the
-	``length`` field of this struct) by the driver.
+	stream, applications when it refers to an output stream.
 
 	.. note::
 
-- 
Regards,

Laurent Pinchart

Re: [RFC PATCH] media: Documentation: Drop deprecated bytesused == 0

[Hans Verkuil]

Hi Laurent,

On 10/9/22 23:35, Laurent Pinchart wrote:
> From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> 
> The V4L2 API historically allowed buffers to be queued with bytesused
> set to 0 on output devices, in which case the driver would use the
> buffer length. This behaviour is deprecated, and videobuf2 prints a
> warning message in the kernel log. Drop it from the documentation.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Reviewed-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Regards,

	Hans

> ---
> 
> This patch aims at starting (or continuing) the discussion related to
> handling of bytesused == 0 on video output devices. videobuf2 currently
> printes a warning that indicates the behaviour is deprecated, but the
> API documentation allows it explicitly. This discrepency bothers me and
> I think we should fix it.
> 
> We probably won't be able to drop support for the deprecated behaviour
> in the near future, if ever. If we want to push applications to get
> fixed, we may want to replace the dev_warn() with a WARN_ONCE(), which
> could be done in a v2 of this patch, or in another patch.
> 
> Another option would be to consider that the behaviour is fine and keep
> official support for it in the documentation, in which case vb2 should
> stop warning about it. This wouldn't be my preference.
> 
> ---
>  Documentation/userspace-api/media/v4l/buffer.rst | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/userspace-api/media/v4l/buffer.rst b/Documentation/userspace-api/media/v4l/buffer.rst
> index 4638ec64db00..04dec3e570ed 100644
> --- a/Documentation/userspace-api/media/v4l/buffer.rst
> +++ b/Documentation/userspace-api/media/v4l/buffer.rst
> @@ -187,10 +187,8 @@ struct v4l2_buffer
>  	on the negotiated data format and may change with each buffer for
>  	compressed variable size data like JPEG images. Drivers must set
>  	this field when ``type`` refers to a capture stream, applications
> -	when it refers to an output stream. If the application sets this
> -	to 0 for an output stream, then ``bytesused`` will be set to the
> -	size of the buffer (see the ``length`` field of this struct) by
> -	the driver. For multiplanar formats this field is ignored and the
> +	when it refers to an output stream. For multiplanar formats this field
> +        is ignored and the
>  	``planes`` pointer is used instead.
>      * - __u32
>        - ``flags``
> @@ -327,10 +325,7 @@ struct v4l2_plane
>        - ``bytesused``
>        - The number of bytes occupied by data in the plane (its payload).
>  	Drivers must set this field when ``type`` refers to a capture
> -	stream, applications when it refers to an output stream. If the
> -	application sets this to 0 for an output stream, then
> -	``bytesused`` will be set to the size of the plane (see the
> -	``length`` field of this struct) by the driver.
> +	stream, applications when it refers to an output stream.
>  
>  	.. note::
>  

Re: [RFC PATCH] media: Documentation: Drop deprecated bytesused == 0

[Sakari Ailus]

Hi Laurent,

On Mon, Oct 10, 2022 at 12:35:09AM +0300, Laurent Pinchart wrote:
> From: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> 
> The V4L2 API historically allowed buffers to be queued with bytesused
> set to 0 on output devices, in which case the driver would use the
> buffer length. This behaviour is deprecated, and videobuf2 prints a
> warning message in the kernel log. Drop it from the documentation.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>

Thanks!

Reviewed-by: Sakari Ailus <sakari.ailus@linux.intel.com>

> ---
> 
> This patch aims at starting (or continuing) the discussion related to
> handling of bytesused == 0 on video output devices. videobuf2 currently
> printes a warning that indicates the behaviour is deprecated, but the
> API documentation allows it explicitly. This discrepency bothers me and
> I think we should fix it.
> 
> We probably won't be able to drop support for the deprecated behaviour
> in the near future, if ever. If we want to push applications to get
> fixed, we may want to replace the dev_warn() with a WARN_ONCE(), which
> could be done in a v2 of this patch, or in another patch.
> 
> Another option would be to consider that the behaviour is fine and keep
> official support for it in the documentation, in which case vb2 should
> stop warning about it. This wouldn't be my preference.
> 
> ---
>  Documentation/userspace-api/media/v4l/buffer.rst | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/userspace-api/media/v4l/buffer.rst b/Documentation/userspace-api/media/v4l/buffer.rst
> index 4638ec64db00..04dec3e570ed 100644
> --- a/Documentation/userspace-api/media/v4l/buffer.rst
> +++ b/Documentation/userspace-api/media/v4l/buffer.rst
> @@ -187,10 +187,8 @@ struct v4l2_buffer
>  	on the negotiated data format and may change with each buffer for
>  	compressed variable size data like JPEG images. Drivers must set
>  	this field when ``type`` refers to a capture stream, applications
> -	when it refers to an output stream. If the application sets this
> -	to 0 for an output stream, then ``bytesused`` will be set to the
> -	size of the buffer (see the ``length`` field of this struct) by
> -	the driver. For multiplanar formats this field is ignored and the
> +	when it refers to an output stream. For multiplanar formats this field
> +        is ignored and the
>  	``planes`` pointer is used instead.
>      * - __u32
>        - ``flags``
> @@ -327,10 +325,7 @@ struct v4l2_plane
>        - ``bytesused``
>        - The number of bytes occupied by data in the plane (its payload).
>  	Drivers must set this field when ``type`` refers to a capture
> -	stream, applications when it refers to an output stream. If the
> -	application sets this to 0 for an output stream, then
> -	``bytesused`` will be set to the size of the plane (see the
> -	``length`` field of this struct) by the driver.
> +	stream, applications when it refers to an output stream.
>  
>  	.. note::
>  
> -- 
> Regards,
> 
> Laurent Pinchart
> 

-- 
Sakari Ailus