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

[Marton Balint]

On Mon, 19 Feb 2018, Aman Gupta wrote:

> On Fri, Feb 16, 2018 at 11:22 AM, Stefan Pöschel <basic.master at 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