Re: [PATCH] media: v4l2-ctrls: add full AV1 profile validation in validate_av1_sequence()

[Nicolas Dufresne <nicolas.dufresne@collabora.com>]

[-- Attachment #1: Type: text/plain, Size: 4685 bytes --]

Hi,

Le vendredi 12 septembre 2025 à 18:44 +0530, Pavan Bobba a écrit :
> The AV1 stateless decoder API provides the control
> V4L2_CID_STATELESS_AV1_SEQUENCE to pass sequence header parameters
> from userspace. The current validator only checked that seq_profile
> ≤ 2 and that monochrome was not signaled in profile 1.
> 
> This patch completes the "TODO: PROFILES" by enforcing all
> profile-specific constraints as defined by the AV1 specification
> (Section 5.5.2, "Color config syntax"):
> 
> - Profile 0: 8/10-bit only, 4:2:0 subsampling, no monochrome
> - Profile 1: 8/10-bit only, 4:4:4 only, no monochrome
> - Profile 2: 8/10/12-bit, 4:2:0 / 4:2:2 / 4:4:4 allowed, monochrome allowed
> 
> Additionally, when the MONO_CHROME flag is set:
> - subsampling_x and subsampling_y must both be 1
> - separate_uv_delta_q must be 0
> 
> These checks prevent userspace from providing invalid AV1 sequence
> headers that would otherwise be accepted, leading to undefined
> driver or hardware behavior.
> 
> Signed-off-by: Pavan Bobba <opensource206@gmail.com>

The changes looks good and seems safer. I will have to run some tests to make
sure we don't regress anything. About your commit message, there is a push to
make things more imperative, so that would mean reformatting to the following
and dropping the first paragraph:

   Complete the "TODO: PROFILES" by enforcing all profile-specific constraints
   as defined by the AV1 specification (Section 5.5.2, "Color config syntax"):
   
   - Profile 0: 8/10-bit only, 4:2:0 subsampling, no monochrome
   - Profile 1: 8/10-bit only, 4:4:4 only, no monochrome
   - Profile 2: 8/10/12-bit, 4:2:0 / 4:2:2 / 4:4:4 allowed, monochrome allowed
   
   Additionally, when the MONO_CHROME flag is set:
   - subsampling_x and subsampling_y must both be 1
   - separate_uv_delta_q must be 0
   
   These checks prevent userspace from providing invalid AV1 sequence
   headers that would otherwise be accepted, leading to undefined
   driver or hardware behavior.

If you are fine with this change I can apply. Otherwise please include my Rb in
your v2.

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

regards,
Nicolas

> ---
>  drivers/media/v4l2-core/v4l2-ctrls-core.c | 54 +++++++++++++++++++++--
>  1 file changed, 50 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c b/drivers/media/v4l2-
> core/v4l2-ctrls-core.c
> index 98b960775e87..3283ed04cc36 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls-core.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c
> @@ -852,14 +852,60 @@ static int validate_av1_sequence(struct
> v4l2_ctrl_av1_sequence *s)
>  	 V4L2_AV1_SEQUENCE_FLAG_SEPARATE_UV_DELTA_Q))
>  		return -EINVAL;
>  
> -	if (s->seq_profile == 1 && s->flags &
> V4L2_AV1_SEQUENCE_FLAG_MONO_CHROME)
> -		return -EINVAL;
> -
>  	/* reserved */
>  	if (s->seq_profile > 2)
>  		return -EINVAL;
>  
> -	/* TODO: PROFILES */
> +	/* Profile-specific checks */
> +	switch (s->seq_profile) {
> +	case 0:
> +		/* Bit depth: 8 or 10 */
> +		if (s->bit_depth != 8 && s->bit_depth != 10)
> +			return -EINVAL;
> +
> +		/* Subsampling must be 4:2:0 → x=1, y=1 */
> +		if (!(s->flags & V4L2_AV1_SEQUENCE_FLAG_SUBSAMPLING_X) ||
> +		    !(s->flags & V4L2_AV1_SEQUENCE_FLAG_SUBSAMPLING_Y))
> +			return -EINVAL;
> +		break;
> +
> +	case 1:
> +		/* Monochrome is forbidden in profile 1 */
> +		if (s->flags & V4L2_AV1_SEQUENCE_FLAG_MONO_CHROME)
> +			return -EINVAL;
> +
> +		/* Bit depth: 8 or 10 */
> +		if (s->bit_depth != 8 && s->bit_depth != 10)
> +			return -EINVAL;
> +
> +		/* Subsampling must be 4:4:4 → x=0, y=0 */
> +		if ((s->flags & V4L2_AV1_SEQUENCE_FLAG_SUBSAMPLING_X) ||
> +		    (s->flags & V4L2_AV1_SEQUENCE_FLAG_SUBSAMPLING_Y))
> +			return -EINVAL;
> +		break;
> +
> +	case 2:
> +		/* Bit depth: 8, 10, or 12 */
> +		if (s->bit_depth != 8 && s->bit_depth != 10 &&
> +		    s->bit_depth != 12)
> +			return -EINVAL;
> +
> +		/* Subsampling: 4:2:0, 4:2:2, or 4:4:4 allowed → no extra
> check */
> +		break;
> +	}
> +
> +	/* If monochrome flag is set, enforce spec rules */
> +	if (s->flags & V4L2_AV1_SEQUENCE_FLAG_MONO_CHROME) {
> +		/* Must signal subsampling_x=1, subsampling_y=1 */
> +		if (!(s->flags & V4L2_AV1_SEQUENCE_FLAG_SUBSAMPLING_X) ||
> +		    !(s->flags & V4L2_AV1_SEQUENCE_FLAG_SUBSAMPLING_Y))
> +			return -EINVAL;
> +
> +		/* separate_uv_delta_q must be 0 in monochrome */
> +		if (s->flags & V4L2_AV1_SEQUENCE_FLAG_SEPARATE_UV_DELTA_Q)
> +			return -EINVAL;
> +	}
> +
>  	return 0;
>  }
>  

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]