1. Patchew
  2. linux
  3. View series

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

Pavan Bobba posted 1 patch 2 weeks, 5 days ago 
drivers/media/v4l2-core/v4l2-ctrls-core.c | 125 +++++++++++++++++-----
1 file changed, 100 insertions(+), 25 deletions(-)
[PATCH v3] media: v4l2-ctrls: add full AV1 profile validation in validate_av1_sequence()
Posted by Pavan Bobba 2 weeks, 5 days ago 
Complete the "TODO: PROFILES" by enforcing profile-specific and
monochrome constraints as defined by the AV1 specification
(Section 5.5.2, "Color config syntax").

The validator now checks:

 - Flags: reject any unknown bits set in sequence->flags
 - Profile range: only profiles 0..2 are valid
 - Profile 0: 8/10-bit only, subsampling must be 4:2:0 (sx=1, sy=1),
   monochrome allowed
 - Profile 1: 8/10-bit only, subsampling must be 4:4:4 (sx=0, sy=0),
   monochrome forbidden
 - Profile 2:
    * 8/10-bit: only 4:2:2 allowed (sx=1, sy=0)
    * 12-bit: 4:4:4 (sx=0, sy=0), 4:2:2 (sx=1, sy=0), or 4:2:0 (sx=1, sy=1)
      allowed
 - Monochrome path (all profiles except 1): forces subsampling_x=1,
   subsampling_y=1, separate_uv_delta_q=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>
---
v1 -> v2 : Added more checks for subsampling combinations per profile.
         : Added a TODO note in the function header for checks to be implemented later.

v2 -> v3 : Patch generated properly with all the changes

 drivers/media/v4l2-core/v4l2-ctrls-core.c | 125 +++++++++++++++++-----
 1 file changed, 100 insertions(+), 25 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c b/drivers/media/v4l2-core/v4l2-ctrls-core.c
index 98b960775e87..fa03341588e4 100644
--- a/drivers/media/v4l2-core/v4l2-ctrls-core.c
+++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c
@@ -827,39 +827,114 @@ static int validate_av1_frame(struct v4l2_ctrl_av1_frame *f)
 	return 0;
 }
 
+/**
+ * validate_av1_sequence - validate AV1 sequence header fields
+ * @s: control struct from userspace
+ *
+ * Implements AV1 spec §5.5.2 color_config() checks that are
+ * possible with the current v4l2_ctrl_av1_sequence definition.
+ *
+ * TODO: extend validation once additional fields such as
+ *       color_primaries, transfer_characteristics,
+ *       matrix_coefficients, and chroma_sample_position
+ *       are added to the uAPI.
+ *
+ * Returns 0 if valid, -EINVAL otherwise.
+ */
 static int validate_av1_sequence(struct v4l2_ctrl_av1_sequence *s)
 {
-	if (s->flags &
-	~(V4L2_AV1_SEQUENCE_FLAG_STILL_PICTURE |
-	 V4L2_AV1_SEQUENCE_FLAG_USE_128X128_SUPERBLOCK |
-	 V4L2_AV1_SEQUENCE_FLAG_ENABLE_FILTER_INTRA |
-	 V4L2_AV1_SEQUENCE_FLAG_ENABLE_INTRA_EDGE_FILTER |
-	 V4L2_AV1_SEQUENCE_FLAG_ENABLE_INTERINTRA_COMPOUND |
-	 V4L2_AV1_SEQUENCE_FLAG_ENABLE_MASKED_COMPOUND |
-	 V4L2_AV1_SEQUENCE_FLAG_ENABLE_WARPED_MOTION |
-	 V4L2_AV1_SEQUENCE_FLAG_ENABLE_DUAL_FILTER |
-	 V4L2_AV1_SEQUENCE_FLAG_ENABLE_ORDER_HINT |
-	 V4L2_AV1_SEQUENCE_FLAG_ENABLE_JNT_COMP |
-	 V4L2_AV1_SEQUENCE_FLAG_ENABLE_REF_FRAME_MVS |
-	 V4L2_AV1_SEQUENCE_FLAG_ENABLE_SUPERRES |
-	 V4L2_AV1_SEQUENCE_FLAG_ENABLE_CDEF |
-	 V4L2_AV1_SEQUENCE_FLAG_ENABLE_RESTORATION |
-	 V4L2_AV1_SEQUENCE_FLAG_MONO_CHROME |
-	 V4L2_AV1_SEQUENCE_FLAG_COLOR_RANGE |
-	 V4L2_AV1_SEQUENCE_FLAG_SUBSAMPLING_X |
-	 V4L2_AV1_SEQUENCE_FLAG_SUBSAMPLING_Y |
-	 V4L2_AV1_SEQUENCE_FLAG_FILM_GRAIN_PARAMS_PRESENT |
-	 V4L2_AV1_SEQUENCE_FLAG_SEPARATE_UV_DELTA_Q))
-		return -EINVAL;
+	const bool mono  = s->flags & V4L2_AV1_SEQUENCE_FLAG_MONO_CHROME;
+	const bool sx    = s->flags & V4L2_AV1_SEQUENCE_FLAG_SUBSAMPLING_X;
+	const bool sy    = s->flags & V4L2_AV1_SEQUENCE_FLAG_SUBSAMPLING_Y;
+	const bool uv_dq = s->flags & V4L2_AV1_SEQUENCE_FLAG_SEPARATE_UV_DELTA_Q;
 
-	if (s->seq_profile == 1 && s->flags & V4L2_AV1_SEQUENCE_FLAG_MONO_CHROME)
+	/* 1. Reject unknown flags */
+	if (s->flags &
+	    ~(V4L2_AV1_SEQUENCE_FLAG_STILL_PICTURE |
+	      V4L2_AV1_SEQUENCE_FLAG_USE_128X128_SUPERBLOCK |
+	      V4L2_AV1_SEQUENCE_FLAG_ENABLE_FILTER_INTRA |
+	      V4L2_AV1_SEQUENCE_FLAG_ENABLE_INTRA_EDGE_FILTER |
+	      V4L2_AV1_SEQUENCE_FLAG_ENABLE_INTERINTRA_COMPOUND |
+	      V4L2_AV1_SEQUENCE_FLAG_ENABLE_MASKED_COMPOUND |
+	      V4L2_AV1_SEQUENCE_FLAG_ENABLE_WARPED_MOTION |
+	      V4L2_AV1_SEQUENCE_FLAG_ENABLE_DUAL_FILTER |
+	      V4L2_AV1_SEQUENCE_FLAG_ENABLE_ORDER_HINT |
+	      V4L2_AV1_SEQUENCE_FLAG_ENABLE_JNT_COMP |
+	      V4L2_AV1_SEQUENCE_FLAG_ENABLE_REF_FRAME_MVS |
+	      V4L2_AV1_SEQUENCE_FLAG_ENABLE_SUPERRES |
+	      V4L2_AV1_SEQUENCE_FLAG_ENABLE_CDEF |
+	      V4L2_AV1_SEQUENCE_FLAG_ENABLE_RESTORATION |
+	      V4L2_AV1_SEQUENCE_FLAG_MONO_CHROME |
+	      V4L2_AV1_SEQUENCE_FLAG_COLOR_RANGE |
+	      V4L2_AV1_SEQUENCE_FLAG_SUBSAMPLING_X |
+	      V4L2_AV1_SEQUENCE_FLAG_SUBSAMPLING_Y |
+	      V4L2_AV1_SEQUENCE_FLAG_FILM_GRAIN_PARAMS_PRESENT |
+	      V4L2_AV1_SEQUENCE_FLAG_SEPARATE_UV_DELTA_Q))
 		return -EINVAL;
 
-	/* reserved */
+	/* 2. Profile range */
 	if (s->seq_profile > 2)
 		return -EINVAL;
 
-	/* TODO: PROFILES */
+	/* 3. Monochrome shortcut */
+	if (mono) {
+		/* Profile 1 forbids monochrome */
+		if (s->seq_profile == 1)
+			return -EINVAL;
+
+		/* Mono → subsampling must look like 4:0:0: sx=1, sy=1 */
+		if (!sx || !sy)
+			return -EINVAL;
+
+		/* separate_uv_delta_q must be 0 */
+		if (uv_dq)
+			return -EINVAL;
+
+		return 0;
+	}
+
+	/* 4. Profile-specific rules */
+	switch (s->seq_profile) {
+	case 0:
+		/* Profile 0: only 8/10-bit, subsampling=4:2:0 (sx=1, sy=1) */
+		if (s->bit_depth != 8 && s->bit_depth != 10)
+			return -EINVAL;
+		if (!(sx && sy))
+			return -EINVAL;
+		break;
+
+	case 1:
+		/* Profile 1: only 8/10-bit, subsampling=4:4:4 (sx=0, sy=0) */
+		if (s->bit_depth != 8 && s->bit_depth != 10)
+			return -EINVAL;
+		if (sx || sy)
+			return -EINVAL;
+		break;
+
+	case 2:
+		/* Profile 2: 8/10/12-bit allowed */
+		if (s->bit_depth != 8 && s->bit_depth != 10 &&
+		    s->bit_depth != 12)
+			return -EINVAL;
+
+		if (s->bit_depth == 12) {
+			if (!sx) {
+				/* 4:4:4 → sy must be 0 */
+				if (sy)
+					return -EINVAL;
+			} else {
+				/* sx=1 → sy=0 (4:2:2) or sy=1 (4:2:0) */
+				if (sy != 0 && sy != 1)
+					return -EINVAL;
+			}
+		} else {
+			/* 8/10-bit → only 4:2:2 allowed (sx=1, sy=0) */
+			if (!(sx && !sy))
+				return -EINVAL;
+		}
+		break;
+	}
+
 	return 0;
 }
 
-- 
2.43.0
Re: [PATCH v3] media: v4l2-ctrls: add full AV1 profile validation in validate_av1_sequence()
Posted by opensource india 5 days, 10 hours ago 
On Sat, Sep 13, 2025 at 4:23 PM Pavan Bobba <opensource206@gmail.com> wrote:
>
> Complete the "TODO: PROFILES" by enforcing profile-specific and
> monochrome constraints as defined by the AV1 specification
> (Section 5.5.2, "Color config syntax").
>
> The validator now checks:
>
>  - Flags: reject any unknown bits set in sequence->flags
>  - Profile range: only profiles 0..2 are valid
>  - Profile 0: 8/10-bit only, subsampling must be 4:2:0 (sx=1, sy=1),
>    monochrome allowed
>  - Profile 1: 8/10-bit only, subsampling must be 4:4:4 (sx=0, sy=0),
>    monochrome forbidden
>  - Profile 2:
>     * 8/10-bit: only 4:2:2 allowed (sx=1, sy=0)
>     * 12-bit: 4:4:4 (sx=0, sy=0), 4:2:2 (sx=1, sy=0), or 4:2:0 (sx=1, sy=1)
>       allowed
>  - Monochrome path (all profiles except 1): forces subsampling_x=1,
>    subsampling_y=1, separate_uv_delta_q=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>
> ---
> v1 -> v2 : Added more checks for subsampling combinations per profile.
>          : Added a TODO note in the function header for checks to be implemented later.
>
> v2 -> v3 : Patch generated properly with all the changes
>
>  drivers/media/v4l2-core/v4l2-ctrls-core.c | 125 +++++++++++++++++-----
>  1 file changed, 100 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c b/drivers/media/v4l2-core/v4l2-ctrls-core.c
> index 98b960775e87..fa03341588e4 100644
> --- a/drivers/media/v4l2-core/v4l2-ctrls-core.c
> +++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c
> @@ -827,39 +827,114 @@ static int validate_av1_frame(struct v4l2_ctrl_av1_frame *f)
>         return 0;
>  }
>
> +/**
> + * validate_av1_sequence - validate AV1 sequence header fields
> + * @s: control struct from userspace
> + *
> + * Implements AV1 spec §5.5.2 color_config() checks that are
> + * possible with the current v4l2_ctrl_av1_sequence definition.
> + *
> + * TODO: extend validation once additional fields such as
> + *       color_primaries, transfer_characteristics,
> + *       matrix_coefficients, and chroma_sample_position
> + *       are added to the uAPI.
> + *
> + * Returns 0 if valid, -EINVAL otherwise.
> + */
>  static int validate_av1_sequence(struct v4l2_ctrl_av1_sequence *s)
>  {
> -       if (s->flags &
> -       ~(V4L2_AV1_SEQUENCE_FLAG_STILL_PICTURE |
> -        V4L2_AV1_SEQUENCE_FLAG_USE_128X128_SUPERBLOCK |
> -        V4L2_AV1_SEQUENCE_FLAG_ENABLE_FILTER_INTRA |
> -        V4L2_AV1_SEQUENCE_FLAG_ENABLE_INTRA_EDGE_FILTER |
> -        V4L2_AV1_SEQUENCE_FLAG_ENABLE_INTERINTRA_COMPOUND |
> -        V4L2_AV1_SEQUENCE_FLAG_ENABLE_MASKED_COMPOUND |
> -        V4L2_AV1_SEQUENCE_FLAG_ENABLE_WARPED_MOTION |
> -        V4L2_AV1_SEQUENCE_FLAG_ENABLE_DUAL_FILTER |
> -        V4L2_AV1_SEQUENCE_FLAG_ENABLE_ORDER_HINT |
> -        V4L2_AV1_SEQUENCE_FLAG_ENABLE_JNT_COMP |
> -        V4L2_AV1_SEQUENCE_FLAG_ENABLE_REF_FRAME_MVS |
> -        V4L2_AV1_SEQUENCE_FLAG_ENABLE_SUPERRES |
> -        V4L2_AV1_SEQUENCE_FLAG_ENABLE_CDEF |
> -        V4L2_AV1_SEQUENCE_FLAG_ENABLE_RESTORATION |
> -        V4L2_AV1_SEQUENCE_FLAG_MONO_CHROME |
> -        V4L2_AV1_SEQUENCE_FLAG_COLOR_RANGE |
> -        V4L2_AV1_SEQUENCE_FLAG_SUBSAMPLING_X |
> -        V4L2_AV1_SEQUENCE_FLAG_SUBSAMPLING_Y |
> -        V4L2_AV1_SEQUENCE_FLAG_FILM_GRAIN_PARAMS_PRESENT |
> -        V4L2_AV1_SEQUENCE_FLAG_SEPARATE_UV_DELTA_Q))
> -               return -EINVAL;
> +       const bool mono  = s->flags & V4L2_AV1_SEQUENCE_FLAG_MONO_CHROME;
> +       const bool sx    = s->flags & V4L2_AV1_SEQUENCE_FLAG_SUBSAMPLING_X;
> +       const bool sy    = s->flags & V4L2_AV1_SEQUENCE_FLAG_SUBSAMPLING_Y;
> +       const bool uv_dq = s->flags & V4L2_AV1_SEQUENCE_FLAG_SEPARATE_UV_DELTA_Q;
>
> -       if (s->seq_profile == 1 && s->flags & V4L2_AV1_SEQUENCE_FLAG_MONO_CHROME)
> +       /* 1. Reject unknown flags */
> +       if (s->flags &
> +           ~(V4L2_AV1_SEQUENCE_FLAG_STILL_PICTURE |
> +             V4L2_AV1_SEQUENCE_FLAG_USE_128X128_SUPERBLOCK |
> +             V4L2_AV1_SEQUENCE_FLAG_ENABLE_FILTER_INTRA |
> +             V4L2_AV1_SEQUENCE_FLAG_ENABLE_INTRA_EDGE_FILTER |
> +             V4L2_AV1_SEQUENCE_FLAG_ENABLE_INTERINTRA_COMPOUND |
> +             V4L2_AV1_SEQUENCE_FLAG_ENABLE_MASKED_COMPOUND |
> +             V4L2_AV1_SEQUENCE_FLAG_ENABLE_WARPED_MOTION |
> +             V4L2_AV1_SEQUENCE_FLAG_ENABLE_DUAL_FILTER |
> +             V4L2_AV1_SEQUENCE_FLAG_ENABLE_ORDER_HINT |
> +             V4L2_AV1_SEQUENCE_FLAG_ENABLE_JNT_COMP |
> +             V4L2_AV1_SEQUENCE_FLAG_ENABLE_REF_FRAME_MVS |
> +             V4L2_AV1_SEQUENCE_FLAG_ENABLE_SUPERRES |
> +             V4L2_AV1_SEQUENCE_FLAG_ENABLE_CDEF |
> +             V4L2_AV1_SEQUENCE_FLAG_ENABLE_RESTORATION |
> +             V4L2_AV1_SEQUENCE_FLAG_MONO_CHROME |
> +             V4L2_AV1_SEQUENCE_FLAG_COLOR_RANGE |
> +             V4L2_AV1_SEQUENCE_FLAG_SUBSAMPLING_X |
> +             V4L2_AV1_SEQUENCE_FLAG_SUBSAMPLING_Y |
> +             V4L2_AV1_SEQUENCE_FLAG_FILM_GRAIN_PARAMS_PRESENT |
> +             V4L2_AV1_SEQUENCE_FLAG_SEPARATE_UV_DELTA_Q))
>                 return -EINVAL;
>
> -       /* reserved */
> +       /* 2. Profile range */
>         if (s->seq_profile > 2)
>                 return -EINVAL;
>
> -       /* TODO: PROFILES */
> +       /* 3. Monochrome shortcut */
> +       if (mono) {
> +               /* Profile 1 forbids monochrome */
> +               if (s->seq_profile == 1)
> +                       return -EINVAL;
> +
> +               /* Mono → subsampling must look like 4:0:0: sx=1, sy=1 */
> +               if (!sx || !sy)
> +                       return -EINVAL;
> +
> +               /* separate_uv_delta_q must be 0 */
> +               if (uv_dq)
> +                       return -EINVAL;
> +
> +               return 0;
> +       }
> +
> +       /* 4. Profile-specific rules */
> +       switch (s->seq_profile) {
> +       case 0:
> +               /* Profile 0: only 8/10-bit, subsampling=4:2:0 (sx=1, sy=1) */
> +               if (s->bit_depth != 8 && s->bit_depth != 10)
> +                       return -EINVAL;
> +               if (!(sx && sy))
> +                       return -EINVAL;
> +               break;
> +
> +       case 1:
> +               /* Profile 1: only 8/10-bit, subsampling=4:4:4 (sx=0, sy=0) */
> +               if (s->bit_depth != 8 && s->bit_depth != 10)
> +                       return -EINVAL;
> +               if (sx || sy)
> +                       return -EINVAL;
> +               break;
> +
> +       case 2:
> +               /* Profile 2: 8/10/12-bit allowed */
> +               if (s->bit_depth != 8 && s->bit_depth != 10 &&
> +                   s->bit_depth != 12)
> +                       return -EINVAL;
> +
> +               if (s->bit_depth == 12) {
> +                       if (!sx) {
> +                               /* 4:4:4 → sy must be 0 */
> +                               if (sy)
> +                                       return -EINVAL;
> +                       } else {
> +                               /* sx=1 → sy=0 (4:2:2) or sy=1 (4:2:0) */
> +                               if (sy != 0 && sy != 1)
> +                                       return -EINVAL;
> +                       }
> +               } else {
> +                       /* 8/10-bit → only 4:2:2 allowed (sx=1, sy=0) */
> +                       if (!(sx && !sy))
> +                               return -EINVAL;
> +               }
> +               break;
> +       }
> +
>         return 0;
>  }
>
> --
> 2.43.0
>
Hi all,
  It has been a couple of weeks since I sent this patch. Could anyone
please review it?
Re: [PATCH v3] media: v4l2-ctrls: add full AV1 profile validation in validate_av1_sequence()
Posted by Daniel Almeida 5 days, 6 hours ago 
Hi, I’ll review this in the coming weeks.

> On 27 Sep 2025, at 06:51, opensource india <opensource206@gmail.com> wrote:
> 
> On Sat, Sep 13, 2025 at 4:23 PM Pavan Bobba <opensource206@gmail.com> wrote:
>> 
>> Complete the "TODO: PROFILES" by enforcing profile-specific and
>> monochrome constraints as defined by the AV1 specification
>> (Section 5.5.2, "Color config syntax").
>> 
>> The validator now checks:
>> 
>> - Flags: reject any unknown bits set in sequence->flags
>> - Profile range: only profiles 0..2 are valid
>> - Profile 0: 8/10-bit only, subsampling must be 4:2:0 (sx=1, sy=1),
>>   monochrome allowed
>> - Profile 1: 8/10-bit only, subsampling must be 4:4:4 (sx=0, sy=0),
>>   monochrome forbidden
>> - Profile 2:
>>    * 8/10-bit: only 4:2:2 allowed (sx=1, sy=0)
>>    * 12-bit: 4:4:4 (sx=0, sy=0), 4:2:2 (sx=1, sy=0), or 4:2:0 (sx=1, sy=1)
>>      allowed
>> - Monochrome path (all profiles except 1): forces subsampling_x=1,
>>   subsampling_y=1, separate_uv_delta_q=0
>> 
>> These checks prevent userspace from providing invalid AV1 sequence
>> headers that would otherwise be accepted, leading to undefined driver
>> or hardware behavior.

Mauro,

A reminder that I have been warning about this for quite a while [0], which
includes mentioning that patches like this, although welcome, do not solve the
root issue completely.

I keep working on what I believe to be the solution [1][2]. I would appreciate if
we could restart this discussion.

[0]: https://lwn.net/Articles/970565/
[1]: https://lore.kernel.org/linux-media/20250818-v4l2-v1-0-6887e772aac2@collabora.com/
[2]: https://lwn.net/Articles/963966/

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.