Re: [PATCH 5/7] media: v4l2: Sanitize colorspace values in the framework

[Hans Verkuil <hverkuil-cisco@xs4all.nl>]

On 16/06/2022 20:36, Laurent Pinchart wrote:
> Extend the format sanitization code in the framework to handle invalid
> values for the colorspace-related fields.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>

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

Regards,

	Hans

> ---
>  drivers/media/v4l2-core/v4l2-ioctl.c | 65 +++++++++++++++++++++++-----
>  1 file changed, 55 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 18ed2227255a..24b5968e8f32 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1008,6 +1008,31 @@ static int check_fmt(struct file *file, enum v4l2_buf_type type)
>  	return -EINVAL;
>  }
>  
> +static void v4l_sanitize_colorspace(u32 pixelformat, u32 *colorspace,
> +				    u32 *encoding, u32 *quantization,
> +				    u32 *xfer_func)
> +{
> +	bool is_hsv = pixelformat == V4L2_PIX_FMT_HSV24 ||
> +		      pixelformat == V4L2_PIX_FMT_HSV32;
> +
> +	if (!v4l2_is_colorspace_valid(*colorspace)) {
> +		*colorspace = V4L2_COLORSPACE_DEFAULT;
> +		*encoding = V4L2_YCBCR_ENC_DEFAULT;
> +		*quantization = V4L2_QUANTIZATION_DEFAULT;
> +		*xfer_func = V4L2_XFER_FUNC_DEFAULT;
> +	}
> +
> +	if ((!is_hsv && !v4l2_is_ycbcr_enc_valid(*encoding)) ||
> +	    (is_hsv && !v4l2_is_hsv_enc_valid(*encoding)))
> +		*encoding = V4L2_YCBCR_ENC_DEFAULT;
> +
> +	if (!v4l2_is_quant_valid(*quantization))
> +		*quantization = V4L2_QUANTIZATION_DEFAULT;
> +
> +	if (!v4l2_is_xfer_func_valid(*xfer_func))
> +		*xfer_func = V4L2_XFER_FUNC_DEFAULT;
> +}
> +
>  static void v4l_sanitize_format(struct v4l2_format *fmt)
>  {
>  	unsigned int offset;
> @@ -1027,20 +1052,40 @@ static void v4l_sanitize_format(struct v4l2_format *fmt)
>  	 * field to the magic value when the extended pixel format structure
>  	 * isn't used by applications.
>  	 */
> +	if (fmt->type == V4L2_BUF_TYPE_VIDEO_CAPTURE ||
> +	    fmt->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
> +		if (fmt->fmt.pix.priv != V4L2_PIX_FMT_PRIV_MAGIC) {
> +			fmt->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;
>  
> -	if (fmt->type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
> -	    fmt->type != V4L2_BUF_TYPE_VIDEO_OUTPUT)
> -		return;
> +			offset = offsetof(struct v4l2_pix_format, priv)
> +			       + sizeof(fmt->fmt.pix.priv);
> +			memset(((void *)&fmt->fmt.pix) + offset, 0,
> +			       sizeof(fmt->fmt.pix) - offset);
> +		}
> +	}
>  
> -	if (fmt->fmt.pix.priv == V4L2_PIX_FMT_PRIV_MAGIC)
> -		return;
> +	/* Replace invalid colorspace values with defaults. */
> +	if (fmt->type == V4L2_BUF_TYPE_VIDEO_CAPTURE ||
> +	    fmt->type == V4L2_BUF_TYPE_VIDEO_OUTPUT) {
> +		v4l_sanitize_colorspace(fmt->fmt.pix.pixelformat,
> +					&fmt->fmt.pix.colorspace,
> +					&fmt->fmt.pix.ycbcr_enc,
> +					&fmt->fmt.pix.quantization,
> +					&fmt->fmt.pix.xfer_func);
> +	} else if (fmt->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE ||
> +		   fmt->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
> +		u32 ycbcr_enc = fmt->fmt.pix_mp.ycbcr_enc;
> +		u32 quantization = fmt->fmt.pix_mp.quantization;
> +		u32 xfer_func = fmt->fmt.pix_mp.xfer_func;
>  
> -	fmt->fmt.pix.priv = V4L2_PIX_FMT_PRIV_MAGIC;
> +		v4l_sanitize_colorspace(fmt->fmt.pix_mp.pixelformat,
> +					&fmt->fmt.pix_mp.colorspace, &ycbcr_enc,
> +					&quantization, &xfer_func);
>  
> -	offset = offsetof(struct v4l2_pix_format, priv)
> -	       + sizeof(fmt->fmt.pix.priv);
> -	memset(((void *)&fmt->fmt.pix) + offset, 0,
> -	       sizeof(fmt->fmt.pix) - offset);
> +		fmt->fmt.pix_mp.ycbcr_enc = ycbcr_enc;
> +		fmt->fmt.pix_mp.quantization = quantization;
> +		fmt->fmt.pix_mp.xfer_func = xfer_func;
> +	}
>  }
>  
>  static int v4l_querycap(const struct v4l2_ioctl_ops *ops,