Re: [FFmpeg-devel] [PATCH v2] avcodec/itut35: always check the provider code and country code together

[Michael Niedermayer <michael@niedermayer.cc>]

[-- Attachment #1.1: Type: text/plain, Size: 2289 bytes --]

Hi Maryla

On Wed, Jun 25, 2025 at 05:55:45PM +0200, Maryla Ustarroz-Calonge via ffmpeg-devel wrote:
> Date: Wed, 25 Jun 2025 17:55:45 +0200
> From: Maryla Ustarroz-Calonge <maryla@google.com>
> To: ffmpeg-devel@ffmpeg.org
> Subject: [PATCH v2] avcodec/itut35: always check the provider code and country code together
> 
> From: Maryla <maryla@google.com>
> 
> ITU-T T.35 provider codes are attributed by national bodies and it's
> possible to have collisions across countries. This is why the country code
> must always be checked as well.
> 
> Use if statements rather than nested switches which would be unreadable.
> 

> Rename some of the constants to match the corresponding organization.

Renaming should be a seperate patch


> Add a constant for AOM.
> Write all constants with 4 hex digits to make it clear that they are 2-byte ids.
> 

> The code for V-Nova should be 0x5000 instead of 0x0050 according to the
> UK Register of Manufacturer Codes https://www.cix.co.uk/~bpechey/H221/h221code.htm
> but I have been unable to find any reference for what code should be used for
> LCEVC, and the author of the original change has not replied to my emails.

this should be a seperate patch

in general, please split this up a bit to make it easier to review


[...]

> diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
> index f4cbc927b5..a1158a23c4 100644
> --- a/libavcodec/libdav1d.c
> +++ b/libavcodec/libdav1d.c
> @@ -386,6 +386,80 @@ static int libdav1d_receive_frame_internal(AVCodecContext *c, Dav1dPicture *p)
>      return res;
>  }
>  
> +static int parse_itut_t35_metadata(Libdav1dContext *dav1d, Dav1dPicture *p,
> +                                   const Dav1dITUTT35 *itut_t35, AVCodecContext *c,
> +                                   AVFrame *frame) {
> +    GetByteContext gb;
> +    int provider_code, country_code;
> +    int res;

please split the factorization of code from changes to the code.
That way the changes are clearer vissible in the patch


thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Some people wanted to paint the bikeshed green, some blue and some pink.
People argued and fought, when they finally agreed, only rust was left.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".