Re: [PATCH] media: v4l2-subdev: Remove stream support for the crop API

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

On Thu, Apr 04, 2024 at 01:42:33AM +0300, Laurent Pinchart wrote:
> When support for streams was added to the V4L2 subdev API, the
> v4l2_subdev_crop structure was extended with a stream field, but the
> field was not handled in the core code that translates the
> VIDIOC_SUBDEV_[GS]_CROP ioctls to the selection API. This could be
> fixed, but the crop API is deprecated and shouldn't be used by new
> userspace code. It's therefore best to avoid extending it with new
> features. Drop the stream field from the v4l2_subdev_crop structure, and
> update the documentation and kernel code accordingly.
> 
> Fixes: 2f91e10ee6fd ("media: subdev: add stream based configuration")
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> This supersedes the "[PATCH] media: v4l2-subdev: Fix stream handling for
> crop API" patch ([1]). I'll submit matching patches for v4l2-compliance.

Done, see https://lore.kernel.org/linux-media/20240403232425.22304-1-laurent.pinchart@ideasonboard.com/

> 
> [1] https://patchwork.linuxtv.org/project/linux-media/patch/20240401233725.2401-1-laurent.pinchart@ideasonboard.com/
> ---
>  .../userspace-api/media/v4l/vidioc-subdev-g-crop.rst        | 5 +----
>  drivers/media/v4l2-core/v4l2-subdev.c                       | 6 ------
>  include/uapi/linux/v4l2-subdev.h                            | 4 +---
>  3 files changed, 2 insertions(+), 13 deletions(-)
> 
> diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-g-crop.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-crop.rst
> index 92d933631fda..7eeb7b553abf 100644
> --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-g-crop.rst
> +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-crop.rst
> @@ -96,10 +96,7 @@ modified format should be as close as possible to the original request.
>        - ``rect``
>        - Crop rectangle boundaries, in pixels.
>      * - __u32
> -      - ``stream``
> -      - Stream identifier.
> -    * - __u32
> -      - ``reserved``\ [7]
> +      - ``reserved``\ [8]
>        - Reserved for future extensions. Applications and drivers must set
>  	the array to zero.
>  
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index 4c6198c48dd6..02c2a2b472df 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -725,9 +725,6 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>  		struct v4l2_subdev_crop *crop = arg;
>  		struct v4l2_subdev_selection sel;
>  
> -		if (!client_supports_streams)
> -			crop->stream = 0;
> -
>  		memset(crop->reserved, 0, sizeof(crop->reserved));
>  		memset(&sel, 0, sizeof(sel));
>  		sel.which = crop->which;
> @@ -749,9 +746,6 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>  		if (crop->which != V4L2_SUBDEV_FORMAT_TRY && ro_subdev)
>  			return -EPERM;
>  
> -		if (!client_supports_streams)
> -			crop->stream = 0;
> -
>  		memset(crop->reserved, 0, sizeof(crop->reserved));
>  		memset(&sel, 0, sizeof(sel));
>  		sel.which = crop->which;
> diff --git a/include/uapi/linux/v4l2-subdev.h b/include/uapi/linux/v4l2-subdev.h
> index 7048c51581c6..f7eea12d8a2c 100644
> --- a/include/uapi/linux/v4l2-subdev.h
> +++ b/include/uapi/linux/v4l2-subdev.h
> @@ -48,15 +48,13 @@ struct v4l2_subdev_format {
>   * @which: format type (from enum v4l2_subdev_format_whence)
>   * @pad: pad number, as reported by the media API
>   * @rect: pad crop rectangle boundaries
> - * @stream: stream number, defined in subdev routing
>   * @reserved: drivers and applications must zero this array
>   */
>  struct v4l2_subdev_crop {
>  	__u32 which;
>  	__u32 pad;
>  	struct v4l2_rect rect;
> -	__u32 stream;
> -	__u32 reserved[7];
> +	__u32 reserved[8];
>  };
>  
>  #define V4L2_SUBDEV_MBUS_CODE_CSC_COLORSPACE	0x00000001
> 
> base-commit: 39cd87c4eb2b893354f3b850f916353f2658ae6f

-- 
Regards,

Laurent Pinchart