From: Maryla Ustarroz via ffmpeg-devel <ffmpeg-devel@ffmpeg.org>
To: ffmpeg-devel@ffmpeg.org
Cc: Maryla Ustarroz <maryla@google.com>
Subject: Re: [FFmpeg-devel] [PATCH v2] avcodec/itut35: always check the provider code and country code together
Date: Wed, 30 Jul 2025 17:49:44 +0200
Message-ID: <CA+yX6GFyoV3A=iNhY_ee0sawHio21g9L_5NspK36SV3aH0xj5Q@mail.gmail.com> (raw)
In-Reply-To: <20250625155545.933182-1-maryla@google.com>
On Wed, Jun 25, 2025 at 5:55 PM Maryla Ustarroz-Calonge
<maryla@google.com> wrote:
>
> 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.
> 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.
>
> Signed-off-by: Maryla Ustarroz-Calonge <maryla@google.com>
> ---
> libavcodec/av1dec.c | 32 +++-----
> libavcodec/h2645_sei.c | 36 +++------
> libavcodec/itut35.h | 21 ++++--
> libavcodec/libdav1d.c | 154 +++++++++++++++++++-------------------
> libavformat/matroskadec.c | 2 +-
> libavformat/matroskaenc.c | 2 +-
> 6 files changed, 116 insertions(+), 131 deletions(-)
>
> diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
> index 8ff1bf394c..be595484d1 100644
> --- a/libavcodec/av1dec.c
> +++ b/libavcodec/av1dec.c
> @@ -965,13 +965,13 @@ static int export_itut_t35(AVCodecContext *avctx, AVFrame *frame,
> {
> GetByteContext gb;
> AV1DecContext *s = avctx->priv_data;
> - int ret, provider_code;
> + int ret, provider_code, country_code;
>
> bytestream2_init(&gb, itut_t35->payload, itut_t35->payload_size);
>
> provider_code = bytestream2_get_be16(&gb);
> - switch (provider_code) {
> - case ITU_T_T35_PROVIDER_CODE_ATSC: {
> + country_code = itut_t35->itu_t_t35_country_code ;
> + if (country_code == ITU_T_T35_COUNTRY_CODE_US && provider_code == ITU_T_T35_PROVIDER_CODE_ATSC) {
> uint32_t user_identifier = bytestream2_get_be32(&gb);
> switch (user_identifier) {
> case MKBETAG('G', 'A', '9', '4'): { // closed captions
> @@ -997,16 +997,13 @@ FF_ENABLE_DEPRECATION_WARNINGS
> default: // ignore unsupported identifiers
> break;
> }
> - break;
> - }
> - case ITU_T_T35_PROVIDER_CODE_SMTPE: {
> + } else if (country_code == ITU_T_T35_COUNTRY_CODE_US && provider_code == ITU_T_T35_PROVIDER_CODE_SAMSUNG) {
> AVDynamicHDRPlus *hdrplus;
> int provider_oriented_code = bytestream2_get_be16(&gb);
> int application_identifier = bytestream2_get_byte(&gb);
>
> - if (itut_t35->itu_t_t35_country_code != ITU_T_T35_COUNTRY_CODE_US ||
> - provider_oriented_code != 1 || application_identifier != 4)
> - break;
> + if (provider_oriented_code != 1 || application_identifier != 4)
> + return 0; // ignore
>
> hdrplus = av_dynamic_hdr_plus_create_side_data(frame);
> if (!hdrplus)
> @@ -1016,28 +1013,23 @@ FF_ENABLE_DEPRECATION_WARNINGS
> bytestream2_get_bytes_left(&gb));
> if (ret < 0)
> return ret;
> - break;
> - }
> - case ITU_T_T35_PROVIDER_CODE_DOLBY: {
> + } else if (country_code == ITU_T_T35_COUNTRY_CODE_US && provider_code == ITU_T_T35_PROVIDER_CODE_DOLBY) {
> int provider_oriented_code = bytestream2_get_be32(&gb);
> - if (itut_t35->itu_t_t35_country_code != ITU_T_T35_COUNTRY_CODE_US ||
> - provider_oriented_code != 0x800)
> - break;
> + if (provider_oriented_code != 0x800)
> + return 0; // ignore
>
> ret = ff_dovi_rpu_parse(&s->dovi, gb.buffer, gb.buffer_end - gb.buffer,
> avctx->err_recognition);
> if (ret < 0) {
> av_log(avctx, AV_LOG_WARNING, "Error parsing DOVI OBU.\n");
> - break; // ignore
> + return 0; // ignore
> }
>
> ret = ff_dovi_attach_side_data(&s->dovi, frame);
> if (ret < 0)
> return ret;
> - break;
> - }
> - default: // ignore unsupported provider codes
> - break;
> + } else {
> + // ignore unsupported provider codes
> }
>
> return 0;
> diff --git a/libavcodec/h2645_sei.c b/libavcodec/h2645_sei.c
> index d17c4fb5f9..04432b6a8d 100644
> --- a/libavcodec/h2645_sei.c
> +++ b/libavcodec/h2645_sei.c
> @@ -158,20 +158,10 @@ static int decode_registered_user_data(H2645SEI *h, GetByteContext *gb,
> bytestream2_skipu(gb, 1); // itu_t_t35_country_code_extension_byte
> }
>
> - if (country_code != ITU_T_T35_COUNTRY_CODE_US &&
> - country_code != ITU_T_T35_COUNTRY_CODE_UK &&
> - country_code != ITU_T_T35_COUNTRY_CODE_CN) {
> - av_log(logctx, AV_LOG_VERBOSE,
> - "Unsupported User Data Registered ITU-T T35 SEI message (country_code = %d)\n",
> - country_code);
> - return 0;
> - }
> -
> /* itu_t_t35_payload_byte follows */
> provider_code = bytestream2_get_be16u(gb);
>
> - switch (provider_code) {
> - case ITU_T_T35_PROVIDER_CODE_ATSC: {
> + if (country_code == ITU_T_T35_COUNTRY_CODE_US && provider_code == ITU_T_T35_PROVIDER_CODE_ATSC) {
> uint32_t user_identifier;
>
> if (bytestream2_get_bytes_left(gb) < 4)
> @@ -189,9 +179,7 @@ static int decode_registered_user_data(H2645SEI *h, GetByteContext *gb,
> user_identifier);
> break;
> }
> - break;
> - }
> - case ITU_T_T35_PROVIDER_CODE_LCEVC: {
> + } else if (country_code == ITU_T_T35_COUNTRY_CODE_UK && provider_code == ITU_T_T35_PROVIDER_CODE_VNOVA) {
> if (bytestream2_get_bytes_left(gb) < 2)
> return AVERROR_INVALIDDATA;
>
> @@ -199,7 +187,7 @@ static int decode_registered_user_data(H2645SEI *h, GetByteContext *gb,
> return decode_registered_user_data_lcevc(&h->lcevc, gb);
> }
> #if CONFIG_HEVC_SEI
> - case ITU_T_T35_PROVIDER_CODE_CUVA: {
> + else if (country_code == ITU_T_T35_COUNTRY_CODE_CN && provider_code == ITU_T_T35_PROVIDER_CODE_HDR_VIVID) {
> const uint16_t cuva_provider_oriented_code = 0x0005;
> uint16_t provider_oriented_code;
>
> @@ -213,9 +201,7 @@ static int decode_registered_user_data(H2645SEI *h, GetByteContext *gb,
> if (provider_oriented_code == cuva_provider_oriented_code) {
> return decode_registered_user_data_dynamic_hdr_vivid(&h->dynamic_hdr_vivid, gb);
> }
> - break;
> - }
> - case ITU_T_T35_PROVIDER_CODE_SMTPE: {
> + } else if(country_code == ITU_T_T35_COUNTRY_CODE_US && provider_code == ITU_T_T35_PROVIDER_CODE_SAMSUNG) {
> // A/341 Amendment - 2094-40
> const uint16_t smpte2094_40_provider_oriented_code = 0x0001;
> const uint8_t smpte2094_40_application_identifier = 0x04;
> @@ -234,9 +220,7 @@ static int decode_registered_user_data(H2645SEI *h, GetByteContext *gb,
> application_identifier == smpte2094_40_application_identifier) {
> return decode_registered_user_data_dynamic_hdr_plus(&h->dynamic_hdr_plus, gb);
> }
> - break;
> - }
> - case 0x5890: { // aom_provider_code
> + } else if (country_code == ITU_T_T35_COUNTRY_CODE_US && provider_code == ITU_T_T35_PROVIDER_CODE_AOM) {
> const uint16_t aom_grain_provider_oriented_code = 0x0001;
> uint16_t provider_oriented_code;
>
> @@ -252,15 +236,13 @@ static int decode_registered_user_data(H2645SEI *h, GetByteContext *gb,
> gb->buffer,
> bytestream2_get_bytes_left(gb));
> }
> - break;
> }
> - unsupported_provider_code:
> #endif
> - default:
> + else {
> + unsupported_provider_code:
> av_log(logctx, AV_LOG_VERBOSE,
> - "Unsupported User Data Registered ITU-T T35 SEI message (provider_code = %d)\n",
> - provider_code);
> - break;
> + "Unsupported User Data Registered ITU-T T35 SEI message (country_code = %d, provider_code = %d)\n",
> + country_code, provider_code);
> }
>
> return 0;
> diff --git a/libavcodec/itut35.h b/libavcodec/itut35.h
> index a75ef37929..b8987d0b01 100644
> --- a/libavcodec/itut35.h
> +++ b/libavcodec/itut35.h
> @@ -23,10 +23,21 @@
> #define ITU_T_T35_COUNTRY_CODE_UK 0xB4
> #define ITU_T_T35_COUNTRY_CODE_US 0xB5
>
> -#define ITU_T_T35_PROVIDER_CODE_ATSC 0x31
> -#define ITU_T_T35_PROVIDER_CODE_CUVA 0x04
> -#define ITU_T_T35_PROVIDER_CODE_DOLBY 0x3B
> -#define ITU_T_T35_PROVIDER_CODE_LCEVC 0x50
> -#define ITU_T_T35_PROVIDER_CODE_SMTPE 0x3C
> +// The Terminal Provider Code (or "Manufacturer Code") identifies the
> +// manufacturer within a country. An Assignment Authority appointed by the
> +// national body assigns this code nationally. The manufacturer code is always
> +// used in conjunction with a country code.
> +// - CN providers
> +#define ITU_T_T35_PROVIDER_CODE_HDR_VIVID 0x0004
> +// - UK providers
> +// V-Nova should be 0x5000 according to UK Register of Manufacturer Codes
> +// https://www.cix.co.uk/~bpechey/H221/h221code.htm
> +// but FFmpeg has been using 0x0050
> +#define ITU_T_T35_PROVIDER_CODE_VNOVA 0x0050
> +// - US providers
> +#define ITU_T_T35_PROVIDER_CODE_ATSC 0x0031
> +#define ITU_T_T35_PROVIDER_CODE_DOLBY 0x003B
> +#define ITU_T_T35_PROVIDER_CODE_AOM 0x5890
> +#define ITU_T_T35_PROVIDER_CODE_SAMSUNG 0x003C
>
> #endif /* AVCODEC_ITUT35_H */
> 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;
> +
> + bytestream2_init(&gb, itut_t35->payload, itut_t35->payload_size);
> +
> + provider_code = bytestream2_get_be16(&gb);
> + country_code = itut_t35->country_code;
> + if (country_code == ITU_T_T35_COUNTRY_CODE_US && provider_code == ITU_T_T35_PROVIDER_CODE_ATSC) {
> + uint32_t user_identifier = bytestream2_get_be32(&gb);
> + switch (user_identifier) {
> + case MKBETAG('G', 'A', '9', '4'): { // closed captions
> + AVBufferRef *buf = NULL;
> +
> + res = ff_parse_a53_cc(&buf, gb.buffer, bytestream2_get_bytes_left(&gb));
> + if (res < 0)
> + return res;
> + if (!res)
> + return 0; // no cc found, ignore
> +
> + res = ff_frame_new_side_data_from_buf(c, frame, AV_FRAME_DATA_A53_CC, &buf);
> + if (res < 0)
> + return res;
> +
> +#if FF_API_CODEC_PROPS
> +FF_DISABLE_DEPRECATION_WARNINGS
> + c->properties |= FF_CODEC_PROPERTY_CLOSED_CAPTIONS;
> +FF_ENABLE_DEPRECATION_WARNINGS
> +#endif
> + break;
> + }
> + default: // ignore unsupported identifiers
> + break;
> + }
> + } else if (country_code == ITU_T_T35_COUNTRY_CODE_US && provider_code == ITU_T_T35_PROVIDER_CODE_SAMSUNG) {
> + AVDynamicHDRPlus *hdrplus;
> + int provider_oriented_code = bytestream2_get_be16(&gb);
> + int application_identifier = bytestream2_get_byte(&gb);
> +
> + if (provider_oriented_code != 1 || application_identifier != 4)
> + return 0; // ignore
> +
> + hdrplus = av_dynamic_hdr_plus_create_side_data(frame);
> + if (!hdrplus)
> + return AVERROR(ENOMEM);
> +
> + res = av_dynamic_hdr_plus_from_t35(hdrplus, gb.buffer,
> + bytestream2_get_bytes_left(&gb));
> + if (res < 0)
> + return res;
> + } else if (country_code == ITU_T_T35_COUNTRY_CODE_US && provider_code == ITU_T_T35_PROVIDER_CODE_DOLBY) {
> + int provider_oriented_code = bytestream2_get_be32(&gb);
> + if (provider_oriented_code != 0x800)
> + return 0; // ignore
> +
> + res = ff_dovi_rpu_parse(&dav1d->dovi, gb.buffer, gb.buffer_end - gb.buffer,
> + c->err_recognition);
> + if (res < 0) {
> + av_log(c, AV_LOG_WARNING, "Error parsing DOVI OBU.\n");
> + return 0; // ignore
> + }
> +
> + res = ff_dovi_attach_side_data(&dav1d->dovi, frame);
> + if (res < 0)
> + return res;
> + } else {
> + // ignore unsupported provider codes
> + }
> + return 0;
> +}
> +
> static int libdav1d_receive_frame(AVCodecContext *c, AVFrame *frame)
> {
> Libdav1dContext *dav1d = c->priv_data;
> @@ -514,83 +588,9 @@ static int libdav1d_receive_frame(AVCodecContext *c, AVFrame *frame)
> #else
> const Dav1dITUTT35 *itut_t35 = p->itut_t35;
> #endif
> - GetByteContext gb;
> - int provider_code;
> -
> - bytestream2_init(&gb, itut_t35->payload, itut_t35->payload_size);
> -
> - provider_code = bytestream2_get_be16(&gb);
> - switch (provider_code) {
> - case ITU_T_T35_PROVIDER_CODE_ATSC: {
> - uint32_t user_identifier = bytestream2_get_be32(&gb);
> - switch (user_identifier) {
> - case MKBETAG('G', 'A', '9', '4'): { // closed captions
> - AVBufferRef *buf = NULL;
> -
> - res = ff_parse_a53_cc(&buf, gb.buffer, bytestream2_get_bytes_left(&gb));
> - if (res < 0)
> - goto fail;
> - if (!res)
> - break;
> -
> - res = ff_frame_new_side_data_from_buf(c, frame, AV_FRAME_DATA_A53_CC, &buf);
> - if (res < 0)
> - goto fail;
> -
> -#if FF_API_CODEC_PROPS
> -FF_DISABLE_DEPRECATION_WARNINGS
> - c->properties |= FF_CODEC_PROPERTY_CLOSED_CAPTIONS;
> -FF_ENABLE_DEPRECATION_WARNINGS
> -#endif
> - break;
> - }
> - default: // ignore unsupported identifiers
> - break;
> - }
> - break;
> - }
> - case ITU_T_T35_PROVIDER_CODE_SMTPE: {
> - AVDynamicHDRPlus *hdrplus;
> - int provider_oriented_code = bytestream2_get_be16(&gb);
> - int application_identifier = bytestream2_get_byte(&gb);
> -
> - if (itut_t35->country_code != ITU_T_T35_COUNTRY_CODE_US ||
> - provider_oriented_code != 1 || application_identifier != 4)
> - break;
> -
> - hdrplus = av_dynamic_hdr_plus_create_side_data(frame);
> - if (!hdrplus) {
> - res = AVERROR(ENOMEM);
> - goto fail;
> - }
> -
> - res = av_dynamic_hdr_plus_from_t35(hdrplus, gb.buffer,
> - bytestream2_get_bytes_left(&gb));
> - if (res < 0)
> - goto fail;
> - break;
> - }
> - case ITU_T_T35_PROVIDER_CODE_DOLBY: {
> - int provider_oriented_code = bytestream2_get_be32(&gb);
> - if (itut_t35->country_code != ITU_T_T35_COUNTRY_CODE_US ||
> - provider_oriented_code != 0x800)
> - break;
> -
> - res = ff_dovi_rpu_parse(&dav1d->dovi, gb.buffer, gb.buffer_end - gb.buffer,
> - c->err_recognition);
> - if (res < 0) {
> - av_log(c, AV_LOG_WARNING, "Error parsing DOVI OBU.\n");
> - break; // ignore
> - }
> -
> - res = ff_dovi_attach_side_data(&dav1d->dovi, frame);
> - if (res < 0)
> - goto fail;
> - break;
> - }
> - default: // ignore unsupported provider codes
> - break;
> - }
> + res = parse_itut_t35_metadata(dav1d, p, itut_t35, c, frame);
> + if (res < 0)
> + goto fail;
> #if FF_DAV1D_VERSION_AT_LEAST(6,9)
> }
> #endif
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index da5166319e..27baf3f18e 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -3931,7 +3931,7 @@ static int matroska_parse_block_additional(MatroskaDemuxContext *matroska,
> provider_code = bytestream2_get_be16u(&bc);
>
> if (country_code != ITU_T_T35_COUNTRY_CODE_US ||
> - provider_code != ITU_T_T35_PROVIDER_CODE_SMTPE)
> + provider_code != ITU_T_T35_PROVIDER_CODE_SAMSUNG)
> break; // ignore
>
> provider_oriented_code = bytestream2_get_be16u(&bc);
> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
> index 408890fa89..8142d9125e 100644
> --- a/libavformat/matroskaenc.c
> +++ b/libavformat/matroskaenc.c
> @@ -2869,7 +2869,7 @@ static int mkv_write_block(void *logctx, MatroskaMuxContext *mkv,
> size_t payload_size = sizeof(t35_buf) - 6;
>
> bytestream_put_byte(&payload, ITU_T_T35_COUNTRY_CODE_US);
> - bytestream_put_be16(&payload, ITU_T_T35_PROVIDER_CODE_SMTPE);
> + bytestream_put_be16(&payload, ITU_T_T35_PROVIDER_CODE_SAMSUNG);
> bytestream_put_be16(&payload, 0x01); // provider_oriented_code
> bytestream_put_byte(&payload, 0x04); // application_identifier
>
> --
> 2.50.0.714.g196bf9f422-goog
>
Friendly ping.
This is v2 where I changed from switch statements to if/else
statements, which I think is easier to read and reduces nesting.
_______________________________________________
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".
next parent reply other threads:[~2025-07-30 15:50 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20250625155545.933182-1-maryla@google.com>
2025-07-30 15:49 ` Maryla Ustarroz via ffmpeg-devel [this message]
2025-06-25 15:55 Maryla Ustarroz-Calonge via ffmpeg-devel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CA+yX6GFyoV3A=iNhY_ee0sawHio21g9L_5NspK36SV3aH0xj5Q@mail.gmail.com' \
--to=ffmpeg-devel@ffmpeg.org \
--cc=maryla@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git