diff mbox

[FFmpeg-devel,v2] lavf/mpegts: add supplementary audio descriptor

Message ID c5c5b49f-bf2d-0d49-8a9c-9e336c8440a7@gmx.de
State Superseded
Headers show

Commit Message

Stefan Pöschel Feb. 16, 2018, 7:22 p.m. UTC 
The supplementary audio descriptor is defined in ETSI EN 300 468 and
provides more details regarding accessibility audio tracks, especially
the normative annex J contains a detailed description of its use.

Its language code (if present) overrides the language code of an also
present ISO 639 language descriptor.

Note that this also changes the priority of multiple descriptors with
language from "the last descriptor with language within the ES loop" to
"specific descriptor over general ISO 639 descriptor".
---
 libavformat/mpegts.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

Comments

Aman Karmani Feb. 19, 2018, 10:54 p.m. UTC | #1 
On Fri, Feb 16, 2018 at 11:22 AM, Stefan Pöschel <basic.master@gmx.de>
wrote:

> The supplementary audio descriptor is defined in ETSI EN 300 468 and
> provides more details regarding accessibility audio tracks, especially
> the normative annex J contains a detailed description of its use.
>
> Its language code (if present) overrides the language code of an also
> present ISO 639 language descriptor.
>
> Note that this also changes the priority of multiple descriptors with
> language from "the last descriptor with language within the ES loop" to
> "specific descriptor over general ISO 639 descriptor".
> ---
>  libavformat/mpegts.c | 37 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
> index 0a3ad05..c3b522b 100644
> --- a/libavformat/mpegts.c
> +++ b/libavformat/mpegts.c
> @@ -1840,7 +1840,9 @@ int ff_parse_mpeg2_descriptor(AVFormatContext *fc,
> AVStream *st, int stream_type
>          }
>          if (i && language[0]) {
>              language[i - 1] = 0;
> -            av_dict_set(&st->metadata, "language", language, 0);
> +            /* don't overwrite language, as it may already have been set
> by
> +             * another, more specific descriptor (e.g. supplementary
> audio) */
> +            av_dict_set(&st->metadata, "language", language,
> AV_DICT_DONT_OVERWRITE);
>          }
>          break;
>      case 0x05: /* registration descriptor */
> @@ -1895,6 +1897,39 @@ int ff_parse_mpeg2_descriptor(AVFormatContext *fc,
> AVStream *st, int stream_type
>                  st->internal->need_context_update = 1;
>              }
>          }
> +        if (ext_desc_tag == 0x06) { /* supplementary audio descriptor */
> +            char flags;
> +
> +            if (desc_len < 1)
> +                return AVERROR_INVALIDDATA;
> +            flags = get8(pp, desc_end);
> +
> +            switch ((flags >> 2) & 0x1F) { /* editorial_classification */
> +            case 0x01:
> +                st->disposition |= AV_DISPOSITION_VISUAL_IMPAIRED;
> +                break;
> +            case 0x02:
> +                st->disposition |= AV_DISPOSITION_HEARING_IMPAIRED;
> +                break;
> +            case 0x03:
> +                st->disposition |= AV_DISPOSITION_VISUAL_IMPAIRED;
> +                break;
> +            }
> +
> +            if (flags & 0x01) { /* language_code_present */
> +                if (desc_len < 4)
> +                    return AVERROR_INVALIDDATA;
> +                language[0] = get8(pp, desc_end);
> +                language[1] = get8(pp, desc_end);
> +                language[2] = get8(pp, desc_end);
> +                language[3] = 0;
> +
> +                /* This language always has to override a possible
> +                 * ISO 639 language descriptor language */
> +                if (language[0])
> +                    av_dict_set(&st->metadata, "language", language, 0);
> +            }
> +        }
>          break;
>      default:
>          break;
>

LGTM. Will commit in a couple days along with my AV_DISPOSITION_DEPENDENT
if no one objects.

Aman


> --
> 2.7.4
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Marton Balint Feb. 19, 2018, 11:13 p.m. UTC | #2 
On Mon, 19 Feb 2018, Aman Gupta wrote:

> On Fri, Feb 16, 2018 at 11:22 AM, Stefan Pöschel <basic.master@gmx.de>
> wrote:
>
>> The supplementary audio descriptor is defined in ETSI EN 300 468 and
>> provides more details regarding accessibility audio tracks, especially
>> the normative annex J contains a detailed description of its use.
>>
>> Its language code (if present) overrides the language code of an also
>> present ISO 639 language descriptor.
>>
>> Note that this also changes the priority of multiple descriptors with
>> language from "the last descriptor with language within the ES loop" to
>> "specific descriptor over general ISO 639 descriptor".
>> ---
>>  libavformat/mpegts.c | 37 ++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 36 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
>> index 0a3ad05..c3b522b 100644
>> --- a/libavformat/mpegts.c
>> +++ b/libavformat/mpegts.c
>> @@ -1840,7 +1840,9 @@ int ff_parse_mpeg2_descriptor(AVFormatContext *fc,
>> AVStream *st, int stream_type
>>          }
>>          if (i && language[0]) {
>>              language[i - 1] = 0;
>> -            av_dict_set(&st->metadata, "language", language, 0);
>> +            /* don't overwrite language, as it may already have been set
>> by
>> +             * another, more specific descriptor (e.g. supplementary
>> audio) */
>> +            av_dict_set(&st->metadata, "language", language,
>> AV_DICT_DONT_OVERWRITE);
>>          }
>>          break;
>>      case 0x05: /* registration descriptor */
>> @@ -1895,6 +1897,39 @@ int ff_parse_mpeg2_descriptor(AVFormatContext *fc,
>> AVStream *st, int stream_type
>>                  st->internal->need_context_update = 1;
>>              }
>>          }
>> +        if (ext_desc_tag == 0x06) { /* supplementary audio descriptor */
>> +            char flags;

Change this to int, I see no reason why this is a char.

>> +
>> +            if (desc_len < 1)
>> +                return AVERROR_INVALIDDATA;
>> +            flags = get8(pp, desc_end);
>> +
>> +            switch ((flags >> 2) & 0x1F) { /* editorial_classification */
>> +            case 0x01:
>> +                st->disposition |= AV_DISPOSITION_VISUAL_IMPAIRED;
>> +                break;
>> +            case 0x02:
>> +                st->disposition |= AV_DISPOSITION_HEARING_IMPAIRED;
>> +                break;
>> +            case 0x03:
>> +                st->disposition |= AV_DISPOSITION_VISUAL_IMPAIRED;
>> +                break;
>> +            }
>> +
>> +            if (flags & 0x01) { /* language_code_present */
>> +                if (desc_len < 4)
>> +                    return AVERROR_INVALIDDATA;
>> +                language[0] = get8(pp, desc_end);
>> +                language[1] = get8(pp, desc_end);
>> +                language[2] = get8(pp, desc_end);
>> +                language[3] = 0;
>> +
>> +                /* This language always has to override a possible
>> +                 * ISO 639 language descriptor language */
>> +                if (language[0])
>> +                    av_dict_set(&st->metadata, "language", language, 0);
>> +            }
>> +        }
>>          break;
>>      default:
>>          break;
>>
>
> LGTM. Will commit in a couple days along with my AV_DISPOSITION_DEPENDENT
> if no one objects.

LGTM too otherwise.

Thanks,
Marton
Stefan Pöschel Feb. 20, 2018, 5:11 p.m. UTC | #3 
Am 20.02.2018 um 00:13 schrieb Marton Balint:

>>> +        if (ext_desc_tag == 0x06) { /* supplementary audio descriptor */
>>> +            char flags;
> 
> Change this to int, I see no reason why this is a char.

OK, I will fix that with v3.

>> LGTM. Will commit in a couple days along with my AV_DISPOSITION_DEPENDENT
>> if no one objects.
> 
> LGTM too otherwise.

Great, thanks!

Regards,
	Stefan
diff mbox

Patch

diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
index 0a3ad05..c3b522b 100644
--- a/libavformat/mpegts.c
+++ b/libavformat/mpegts.c
@@ -1840,7 +1840,9 @@  int ff_parse_mpeg2_descriptor(AVFormatContext *fc, AVStream *st, int stream_type
         }
         if (i && language[0]) {
             language[i - 1] = 0;
-            av_dict_set(&st->metadata, "language", language, 0);
+            /* don't overwrite language, as it may already have been set by
+             * another, more specific descriptor (e.g. supplementary audio) */
+            av_dict_set(&st->metadata, "language", language, AV_DICT_DONT_OVERWRITE);
         }
         break;
     case 0x05: /* registration descriptor */
@@ -1895,6 +1897,39 @@  int ff_parse_mpeg2_descriptor(AVFormatContext *fc, AVStream *st, int stream_type
                 st->internal->need_context_update = 1;
             }
         }
+        if (ext_desc_tag == 0x06) { /* supplementary audio descriptor */
+            char flags;
+
+            if (desc_len < 1)
+                return AVERROR_INVALIDDATA;
+            flags = get8(pp, desc_end);
+
+            switch ((flags >> 2) & 0x1F) { /* editorial_classification */
+            case 0x01:
+                st->disposition |= AV_DISPOSITION_VISUAL_IMPAIRED;
+                break;
+            case 0x02:
+                st->disposition |= AV_DISPOSITION_HEARING_IMPAIRED;
+                break;
+            case 0x03:
+                st->disposition |= AV_DISPOSITION_VISUAL_IMPAIRED;
+                break;
+            }
+
+            if (flags & 0x01) { /* language_code_present */
+                if (desc_len < 4)
+                    return AVERROR_INVALIDDATA;
+                language[0] = get8(pp, desc_end);
+                language[1] = get8(pp, desc_end);
+                language[2] = get8(pp, desc_end);
+                language[3] = 0;
+
+                /* This language always has to override a possible
+                 * ISO 639 language descriptor language */
+                if (language[0])
+                    av_dict_set(&st->metadata, "language", language, 0);
+            }
+        }
         break;
     default:
         break;