On Wed, Jun 25, 2025 at 5:55 PM Maryla Ustarroz-Calonge <mar...@google.com> wrote: > > From: Maryla <mar...@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 <mar...@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-requ...@ffmpeg.org with subject "unsubscribe".
