Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH] avcodec/itut35: always check the provider code and country code together
@ 2025-06-12 16:27 Maryla Ustarroz-Calonge via ffmpeg-devel
  2025-06-12 16:47 ` Devin Heitmueller
  0 siblings, 1 reply; 3+ messages in thread
From: Maryla Ustarroz-Calonge via ffmpeg-devel @ 2025-06-12 16:27 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Maryla Ustarroz-Calonge

[-- Attachment #1: Type: message/rfc822, Size: 11709 bytes --]

From: Maryla Ustarroz-Calonge <maryla@google.com>
To: ffmpeg-devel@ffmpeg.org
Subject: [PATCH] avcodec/itut35: always check the provider code and country code together
Date: Thu, 12 Jun 2025 18:27:43 +0200
Message-ID: <20250612162743.3475175-1-maryla@google.com>

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.
In the code this could be done by having an outer switch on the country code,
then an inner switch on the provider code, but this would add an extra level of
indentation and is not necessary as long as the codes used don't collide.

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       |  5 ++++-
 libavcodec/h2645_sei.c    | 27 +++++++++++++++++++++------
 libavcodec/itut35.h       | 21 ++++++++++++++++-----
 libavcodec/libdav1d.c     |  5 ++++-
 libavformat/matroskadec.c |  2 +-
 libavformat/matroskaenc.c |  2 +-
 6 files changed, 47 insertions(+), 15 deletions(-)

diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
index 8ff1bf394c..8618b9f015 100644
--- a/libavcodec/av1dec.c
+++ b/libavcodec/av1dec.c
@@ -972,6 +972,9 @@ static int export_itut_t35(AVCodecContext *avctx, AVFrame *frame,
     provider_code = bytestream2_get_be16(&gb);
     switch (provider_code) {
     case ITU_T_T35_PROVIDER_CODE_ATSC: {
+        if (itut_t35->itu_t_t35_country_code != ITU_T_T35_COUNTRY_CODE_US)
+            break;
+
         uint32_t user_identifier = bytestream2_get_be32(&gb);
         switch (user_identifier) {
         case MKBETAG('G', 'A', '9', '4'): { // closed captions
@@ -999,7 +1002,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
         }
         break;
     }
-    case ITU_T_T35_PROVIDER_CODE_SMTPE: {
+    case ITU_T_T35_PROVIDER_CODE_SAMSUNG: {
         AVDynamicHDRPlus *hdrplus;
         int provider_oriented_code = bytestream2_get_be16(&gb);
         int application_identifier = bytestream2_get_byte(&gb);
diff --git a/libavcodec/h2645_sei.c b/libavcodec/h2645_sei.c
index d17c4fb5f9..c1fb37042d 100644
--- a/libavcodec/h2645_sei.c
+++ b/libavcodec/h2645_sei.c
@@ -172,6 +172,9 @@ static int decode_registered_user_data(H2645SEI *h, GetByteContext *gb,
 
     switch (provider_code) {
     case ITU_T_T35_PROVIDER_CODE_ATSC: {
+        if (country_code != ITU_T_T35_COUNTRY_CODE_US)
+            goto unsupported_provider_code;
+
         uint32_t user_identifier;
 
         if (bytestream2_get_bytes_left(gb) < 4)
@@ -191,7 +194,10 @@ static int decode_registered_user_data(H2645SEI *h, GetByteContext *gb,
         }
         break;
     }
-    case ITU_T_T35_PROVIDER_CODE_LCEVC: {
+    case ITU_T_T35_PROVIDER_CODE_VNOVA: {
+        if (country_code != ITU_T_T35_COUNTRY_CODE_UK)
+            goto unsupported_provider_code;
+
         if (bytestream2_get_bytes_left(gb) < 2)
             return AVERROR_INVALIDDATA;
 
@@ -199,7 +205,10 @@ 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: {
+    case ITU_T_T35_PROVIDER_CODE_HDR_VIVID: {
+        if (country_code != ITU_T_T35_COUNTRY_CODE_CN)
+            goto unsupported_provider_code;
+
         const uint16_t cuva_provider_oriented_code = 0x0005;
         uint16_t provider_oriented_code;
 
@@ -215,7 +224,10 @@ static int decode_registered_user_data(H2645SEI *h, GetByteContext *gb,
         }
         break;
     }
-    case ITU_T_T35_PROVIDER_CODE_SMTPE: {
+    case ITU_T_T35_PROVIDER_CODE_SAMSUNG: {
+        if (country_code != ITU_T_T35_COUNTRY_CODE_US)
+            goto unsupported_provider_code;
+
         // A/341 Amendment - 2094-40
         const uint16_t smpte2094_40_provider_oriented_code = 0x0001;
         const uint8_t smpte2094_40_application_identifier = 0x04;
@@ -236,7 +248,10 @@ static int decode_registered_user_data(H2645SEI *h, GetByteContext *gb,
         }
         break;
     }
-    case 0x5890: { // aom_provider_code
+    case ITU_T_T35_PROVIDER_CODE_AOM: {
+        if (country_code != ITU_T_T35_COUNTRY_CODE_US)
+            goto unsupported_provider_code;
+
         const uint16_t aom_grain_provider_oriented_code = 0x0001;
         uint16_t provider_oriented_code;
 
@@ -258,8 +273,8 @@ static int decode_registered_user_data(H2645SEI *h, GetByteContext *gb,
 #endif
     default:
         av_log(logctx, AV_LOG_VERBOSE,
-               "Unsupported User Data Registered ITU-T T35 SEI message (provider_code = %d)\n",
-               provider_code);
+               "Unsupported User Data Registered ITU-T T35 SEI message (country_code = %d, provider_code = %d)\n",
+               country_code, provider_code);
         break;
     }
 
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..4678c29ac8 100644
--- a/libavcodec/libdav1d.c
+++ b/libavcodec/libdav1d.c
@@ -522,6 +522,9 @@ static int libdav1d_receive_frame(AVCodecContext *c, AVFrame *frame)
         provider_code = bytestream2_get_be16(&gb);
         switch (provider_code) {
         case ITU_T_T35_PROVIDER_CODE_ATSC: {
+            if (itut_t35->country_code != ITU_T_T35_COUNTRY_CODE_US)
+                break;
+
             uint32_t user_identifier = bytestream2_get_be32(&gb);
             switch (user_identifier) {
             case MKBETAG('G', 'A', '9', '4'): { // closed captions
@@ -549,7 +552,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
             }
             break;
         }
-        case ITU_T_T35_PROVIDER_CODE_SMTPE: {
+        case ITU_T_T35_PROVIDER_CODE_SAMSUNG: {
             AVDynamicHDRPlus *hdrplus;
             int provider_oriented_code = bytestream2_get_be16(&gb);
             int application_identifier = bytestream2_get_byte(&gb);
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.rc2.692.g299adb8693-goog


[-- 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".

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avcodec/itut35: always check the provider code and country code together
  2025-06-12 16:27 [FFmpeg-devel] [PATCH] avcodec/itut35: always check the provider code and country code together Maryla Ustarroz-Calonge via ffmpeg-devel
@ 2025-06-12 16:47 ` Devin Heitmueller
  2025-06-13 10:57   ` Maryla Ustarroz via ffmpeg-devel
  0 siblings, 1 reply; 3+ messages in thread
From: Devin Heitmueller @ 2025-06-12 16:47 UTC (permalink / raw)
  To: FFmpeg development discussions and patches; +Cc: Maryla Ustarroz-Calonge

On Thu, Jun 12, 2025 at 12:27 PM Maryla Ustarroz-Calonge via
ffmpeg-devel <ffmpeg-devel@ffmpeg.org> wrote:
> ---------- Forwarded message ----------
> From: Maryla Ustarroz-Calonge <maryla@google.com>
> To: ffmpeg-devel@ffmpeg.org
> Cc:
> Bcc:
> Date: Thu, 12 Jun 2025 18:27:43 +0200
> Subject: [PATCH] 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.
> In the code this could be done by having an outer switch on the country code,
> then an inner switch on the provider code, but this would add an extra level of
> indentation and is not necessary as long as the codes used don't collide.
>
> 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.

While it's a bit more annoying, I think the checks should actually be
inverted.  It should be looking at the country codes first, and then
within those country codes it should look at the provider.

Devin

-- 
Devin Heitmueller, Senior Software Engineer
LTN Global Communications
o: +1 (301) 363-1001
w: https://ltnglobal.com  e: devin.heitmueller@ltnglobal.com
_______________________________________________
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".

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [FFmpeg-devel] [PATCH] avcodec/itut35: always check the provider code and country code together
  2025-06-12 16:47 ` Devin Heitmueller
@ 2025-06-13 10:57   ` Maryla Ustarroz via ffmpeg-devel
  0 siblings, 0 replies; 3+ messages in thread
From: Maryla Ustarroz via ffmpeg-devel @ 2025-06-13 10:57 UTC (permalink / raw)
  To: Devin Heitmueller
  Cc: Maryla Ustarroz, FFmpeg development discussions and patches

[-- Attachment #1: Type: message/rfc822, Size: 5749 bytes --]

From: Maryla Ustarroz <maryla@google.com>
To: Devin Heitmueller <devin.heitmueller@ltnglobal.com>
Cc: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH] avcodec/itut35: always check the provider code and country code together
Date: Fri, 13 Jun 2025 12:57:55 +0200
Message-ID: <CA+yX6GEs_WYNv+nv5KnY4mzoqhToqmzdnHRpWkhKMXphbeH--g@mail.gmail.com>

On Thu, Jun 12, 2025 at 6:48 PM Devin Heitmueller
<devin.heitmueller@ltnglobal.com> wrote:
>
> On Thu, Jun 12, 2025 at 12:27 PM Maryla Ustarroz-Calonge via
> ffmpeg-devel <ffmpeg-devel@ffmpeg.org> wrote:
> > ---------- Forwarded message ----------
> > From: Maryla Ustarroz-Calonge <maryla@google.com>
> > To: ffmpeg-devel@ffmpeg.org
> > Cc:
> > Bcc:
> > Date: Thu, 12 Jun 2025 18:27:43 +0200
> > Subject: [PATCH] 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.
> > In the code this could be done by having an outer switch on the country code,
> > then an inner switch on the provider code, but this would add an extra level of
> > indentation and is not necessary as long as the codes used don't collide.
> >
> > 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.
>
> While it's a bit more annoying, I think the checks should actually be
> inverted.  It should be looking at the country codes first, and then
> within those country codes it should look at the provider.

Thank you for the feedback.
I agree this would be cleaner on paper, but I just tried it and I
actually find it really hard to read and follow with all the nested
switches.
I think that simple ifs like this would be clearer:

if (country_code == ITU_T_T35_COUNTRY_CODE_US && provider_code ==
ITU_T_T35_PROVIDER_CODE_ATSC) {
   ...
} else if (country_code == ITU_T_T35_COUNTRY_CODE_US && provider_code
== ITU_T_T35_PROVIDER_CODE_SAMSUNG) {
   ...
} else if (...) {
etc.

This would also avoid adding an extra indentation level.
Of course, nested ifs are also possible.
Let me know what you think.



> Devin
>
> --
> Devin Heitmueller, Senior Software Engineer
> LTN Global Communications
> o: +1 (301) 363-1001
> w: https://ltnglobal.com  e: devin.heitmueller@ltnglobal.com

[-- 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".

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-06-13 10:58 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-06-12 16:27 [FFmpeg-devel] [PATCH] avcodec/itut35: always check the provider code and country code together Maryla Ustarroz-Calonge via ffmpeg-devel
2025-06-12 16:47 ` Devin Heitmueller
2025-06-13 10:57   ` Maryla Ustarroz via ffmpeg-devel

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