[FFmpeg-devel] Patch (1/2) Decoding of Teletext Descriptor (0x56)

[JULIAN GARDNER]

>________________________________
> From: Clément Bœsch <u at pkh.me>
>To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org> 
>Sent: Monday, 23 September 2013, 14:21
>Subject: Re: [FFmpeg-devel] Patch (1/2) Decoding of Teletext Descriptor (0x56)
> 
>
>On Mon, Sep 23, 2013 at 11:52:31AM +0100, JULIAN GARDNER wrote:
>> Hope these work, please let me know of any problems
>
>> From 650e40c5e2856656bf2f807ae81a275ee0bc647a Mon Sep 17 00:00:00 2001
>> From: Julian Gardner <joolzg at btinternet.com>
>> Date: Mon, 23 Sep 2013 12:44:41 +0200
>> Subject: [PATCH 2/3] Add in decoding of Teletext Subtitles Descriptor (0x56)
>> 
>> ---
>>  libavformat/mpegts.c | 32 ++++++++++++++++++++++++++------
>>  1 file changed, 26 insertions(+), 6 deletions(-)
>> 
>> diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
>> index 2e438fd..1eab5e2 100644
>> --- a/libavformat/mpegts.c
>> +++ b/libavformat/mpegts.c
>> @@ -1404,7 +1404,7 @@ int ff_parse_mpeg2_descriptor(AVFormatContext *fc, AVStream *st, int stream_type
>>      const uint8_t *desc_end;
>>      int desc_len, desc_tag, desc_es_id;
>>      char language[252];
>> -    int i;
>> +    int i, l;
>>  
>>      desc_tag = get8(pp, desc_list_end);
>>      if (desc_tag < 0)
>> @@ -1457,10 +1457,30 @@ int ff_parse_mpeg2_descriptor(AVFormatContext *fc, AVStream *st, int stream_type
>>          }
>>          break;
>>      case 0x56: /* DVB teletext descriptor */
>> -        language[0] = get8(pp, desc_end);
>> -        language[1] = get8(pp, desc_end);
>> -        language[2] = get8(pp, desc_end);
>> -        language[3] = 0;
>> +        for (i = l = 0; i + 5 <= desc_len; i += 5) {
>> +        int type, mag, page;
>> +        int l0, l1, l2;
>> +        static const char types[] = "RISAPH";
>> +
>> +            l0 = get8(pp, desc_end);
>> +            l1 = get8(pp, desc_end);
>> +            l2 = get8(pp, desc_end);
>> +            type = get8(pp, desc_end);
>> +            page = get8(pp, desc_end);
>> +            if (l<(sizeof( language) - 9)) {
>> +                mag = type;
>> +                type >>= 3;
>> +                if (type < sizeof( types)) {
>> +                    mag &= 7; if (!mag) mag = 8;
>> +                    sprintf( language+l, "%c%c%c,%c%d%02x,", l0, l1,
>> +                        l2, types[ type], mag, page);
>
>use snprintf, the your if don't look safe enougth to me.
>


> +            if (l<(sizeof( language) - 9)) {
Hmm this protects against a buffer overrun


>> +                    l += 9;
>> +                }
>> +            }
>> +        }
>
>> +        if( l) {
>> +            language[ l - 1] = 0;
>> +        }
>
>Is this necessary?


removes last ','

>
>Also, please fix your style it's all broken; indent, random spaces
>inconsistently badly placed, etc.
>
>>          av_dict_set(&st->metadata, "language", language, 0);
>>          break;
>>      case 0x59: /* subtitling descriptor */
>> @@ -1979,7 +1999,7 @@ static int read_packet(AVFormatContext *s, uint8_t *buf, int raw_packet_size, co
>>      return 0;
>>  }
>>  


new editor, will check

>
>> -static void finished_reading_packet(AVFormatContext *s, int raw_packet_size)
>> +static inline void finished_reading_packet(AVFormatContext *s, int raw_packet_size)
>
>Why?


that crept in, make code a little faster and shorter