[FFmpeg-devel] [PATCH 5/5] mpegtsenc: handle audio channel specific language metadata

[Anssi Hannula]

On 17.02.2011 00:43, Janne Grunau wrote:
> On Mon, Feb 14, 2011 at 08:43:42PM +0200, Anssi Hannula wrote:
>> ---
>>  libavformat/mpegtsenc.c |   29 ++++++++++++++++++++++++++++-
>>  1 files changed, 28 insertions(+), 1 deletions(-)
>>
>> diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
>> index 6945b4b..dac0e4e 100644
>> --- a/libavformat/mpegtsenc.c
>> +++ b/libavformat/mpegtsenc.c
>> @@ -283,7 +283,33 @@ static void mpegts_write_pmt(AVFormatContext *s, MpegTSService *service)
>>          /* write optional descriptors here */
>>          switch(st->codec->codec_type) {
>>          case AVMEDIA_TYPE_AUDIO:
>> -            if (lang && strlen(lang->value) == 3) {
>> +            {
> 
> why? I see that AVMEDIA_TYPE_SUBTITLE has it too but it is not requirred
> and it makes this patch much harder to read.

Due to the below ch_lang declaration?

Otherwise one gets:
libavformat/mpegtsenc.c:287: error: a label can only be part of a
statement and a declaration is not a statement

>> +                AVMetadataTag *ch_lang = av_metadata_get(st->metadata, "channel0/language", NULL, 0);
>> +                if (ch_lang && strlen(ch_lang->value) == 3) {
>> +                    /* add channel specific tags */
>> +                    uint8_t* lang_len_ptr;
>> +                    int ch;
>> +                    char *language_key = av_strdup("channel0/language");
>> +                    if (!language_key)
>> +                        break;
>> +                    *q++ = 0x0a; /* ISO 639 language descriptor */
>> +                    lang_len_ptr = q++;
>> +                    for (ch = 0; ch <= 9; ch++) {
>> +                        if (ch > 0) {
>> +                            language_key[7] = '0' + ch;
>> +                            ch_lang = av_metadata_get(st->metadata, language_key, NULL, 0);
>> +                            if (!ch_lang || strlen(ch_lang->value) != 3)
>> +                                break;
>> +                        }
>> +                        *q++ = ch_lang->value[0];
>> +                        *q++ = ch_lang->value[1];
>> +                        *q++ = ch_lang->value[2];
>> +                        *q++ = 0; /* undefined type */
>> +                    }
>> +                    *lang_len_ptr = ch * 4;
>> +                    av_freep(&language_key);
>> +                }
>> +            else if (lang && strlen(lang->value) == 3) {
> 
> wrongly indented

I was told before to not reindent existing lines even if I change them
(adding 'else' in this case).

>>                  *q++ = 0x0a; /* ISO 639 language descriptor */
>>                  *q++ = 4;
>>                  *q++ = lang->value[0];
>> @@ -298,6 +324,7 @@ static void mpegts_write_pmt(AVFormatContext *s, MpegTSService *service)
>>                  else
>>                  *q++ = 0; /* undefined type */
>>              }
>> +            }
>>              break;
>>          case AVMEDIA_TYPE_SUBTITLE:
>>              {
> 
> but I guess it's obsolete since you have change it after the mpeg ts
> demuxer changes.

Yep. Haven't written anything yet, but it should be somewhat simpler.

-- 
Anssi Hannula