Re: [PATCH v3 08/13] media: controls: Add the stateless codec control class

From: Hans Verkuil <hverkuil@xs4all.nl>
To: Ezequiel Garcia <ezequiel@collabora.com>,
	linux-media@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: kernel@collabora.com, Jonas Karlman <jonas@kwiboo.se>,
	Nicolas Dufresne <nicolas.dufresne@collabora.com>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	Maxime Ripard <mripard@kernel.org>,
	Paul Kocialkowski <paul.kocialkowski@bootlin.com>,
	Jernej Skrabec <jernej.skrabec@siol.net>
Subject: Re: [PATCH v3 08/13] media: controls: Add the stateless codec control class
Date: Fri, 20 Nov 2020 10:33:23 +0100	[thread overview]
Message-ID: <a1b68f63-4e7c-2573-adf7-9e4533cbd3af@xs4all.nl> (raw)
In-Reply-To: <20201118184700.331213-9-ezequiel@collabora.com>

On 18/11/2020 19:46, Ezequiel Garcia wrote:
> Add a new control class to hold the stateless codecs controls
> that are ready to be moved out of staging.
> 
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
>  drivers/media/v4l2-core/v4l2-ctrls.c | 5 +++++
>  include/uapi/linux/v4l2-controls.h   | 7 +++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c
> index adcf47bddbe3..472b46e8290b 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls.c
> @@ -1181,6 +1181,10 @@ const char *v4l2_ctrl_get_name(u32 id)
>  	case V4L2_CID_DETECT_MD_GLOBAL_THRESHOLD: return "MD Global Threshold";
>  	case V4L2_CID_DETECT_MD_THRESHOLD_GRID:	return "MD Threshold Grid";
>  	case V4L2_CID_DETECT_MD_REGION_GRID:	return "MD Region Grid";
> +
> +	/* Codec controls */

Shouldn't that be: /* Stateless Codec controls */ ?

Regards,

	Hans

> +	/* Keep the order of the 'case's the same as in v4l2-controls.h! */
> +	case V4L2_CID_CODEC_STATELESS_CLASS:	return "Stateless Codec Controls";
>  	default:
>  		return NULL;
>  	}
> @@ -1368,6 +1372,7 @@ void v4l2_ctrl_fill(u32 id, const char **name, enum v4l2_ctrl_type *type,
>  	case V4L2_CID_FM_RX_CLASS:
>  	case V4L2_CID_RF_TUNER_CLASS:
>  	case V4L2_CID_DETECT_CLASS:
> +	case V4L2_CID_CODEC_STATELESS_CLASS:
>  		*type = V4L2_CTRL_TYPE_CTRL_CLASS;
>  		/* You can neither read not write these */
>  		*flags |= V4L2_CTRL_FLAG_READ_ONLY | V4L2_CTRL_FLAG_WRITE_ONLY;
> diff --git a/include/uapi/linux/v4l2-controls.h b/include/uapi/linux/v4l2-controls.h
> index 75cf87b9a377..90478ecc2f81 100644
> --- a/include/uapi/linux/v4l2-controls.h
> +++ b/include/uapi/linux/v4l2-controls.h
> @@ -65,6 +65,7 @@
>  #define V4L2_CTRL_CLASS_FM_RX		0x00a10000	/* FM Receiver controls */
>  #define V4L2_CTRL_CLASS_RF_TUNER	0x00a20000	/* RF tuner controls */
>  #define V4L2_CTRL_CLASS_DETECT		0x00a30000	/* Detection controls */
> +#define V4L2_CTRL_CLASS_CODEC_STATELESS 0x00a40000	/* Stateless codecs controls */
>  
>  /* User-class control IDs */
>  
> @@ -1177,6 +1178,12 @@ enum v4l2_detect_md_mode {
>  #define V4L2_CID_DETECT_MD_THRESHOLD_GRID	(V4L2_CID_DETECT_CLASS_BASE + 3)
>  #define V4L2_CID_DETECT_MD_REGION_GRID		(V4L2_CID_DETECT_CLASS_BASE + 4)
>  
> +
> +/*  Stateless CODECs controls */
> +#define V4L2_CID_CODEC_STATELESS_BASE          (V4L2_CTRL_CLASS_CODEC_STATELESS | 0x900)
> +#define V4L2_CID_CODEC_STATELESS_CLASS         (V4L2_CTRL_CLASS_CODEC_STATELESS | 1)
> +
> +
>  /* MPEG-compression definitions kept for backwards compatibility */
>  #ifndef __KERNEL__
>  #define V4L2_CTRL_CLASS_MPEG            V4L2_CTRL_CLASS_CODEC
>