[FFmpeg-devel] Patch (2/2) Make MPEGTSENC understand Teletext Encoding Language String

[JULIAN GARDNER]

----- Original Message -----
> From: Clément Bœsch <u at pkh.me>
> To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> Cc: 
> Sent: Monday, 23 September 2013, 14:26
> Subject: Re: [FFmpeg-devel] Patch (2/2) Make MPEGTSENC understand Teletext Encoding Language String
> 
> On Mon, Sep 23, 2013 at 11:52:45AM +0100, JULIAN GARDNER wrote:
>> 
> 
>>  From 488017a2dbc1b2489f161c7b6933fad94c1987af Mon Sep 17 00:00:00 2001
>>  From: Julian Gardner <joolzg at btinternet.com>
>>  Date: Mon, 23 Sep 2013 12:45:20 +0200
>>  Subject: [PATCH 3/3] Add in decoding of language field which contains the 
> list
>>   of Teletext Pages being signalled
> 
> "Subject: Re: [FFmpeg-devel] Patch (2/2) Make MPEGTSENC understand 
> Teletext"
> 
> Hint: you can use git-send-email to avoid manually re-writing the subject
> which doesn't match the title of your commit.
> 
>> 
>>  ---
>>   libavformat/mpegtsenc.c | 60 
> ++++++++++++++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 59 insertions(+), 1 deletion(-)
>> 
>>  diff --git a/libavformat/mpegtsenc.c b/libavformat/mpegtsenc.c
>>  index 1d51b97..55fd2f9 100644
>>  --- a/libavformat/mpegtsenc.c
>>  +++ b/libavformat/mpegtsenc.c
>>  @@ -372,7 +372,65 @@ static void mpegts_write_pmt(AVFormatContext *s, 
> MpegTSService *service)
>>               }
>>               break;
>>           case AVMEDIA_TYPE_SUBTITLE:
>>  -            {
>>  +        av_log( s, AV_LOG_WARNING, "SUBTITLE %p %d %s\n",
>>  +            st->codec->extradata, st->codec->extradata_size, 
> lang->value);
> 
> Why a warning?
> 

again so sorry, missed it

>>  +            if (!st->codec->extradata_size) {
>>  +                const char *language = lang->value;
>>  +                uint8_t *sizep = q + 1;
>>  +                uint8_t *ss;
>>  +                int type, page;
>>  +
> 
>>  +                *q++ = 0x56;
>>  +                *q++ = 0;
>>  +                ss = q;
>>  +                while (*language) {
>>  +                    *q++ = *language++;
>>  +                    *q++ = *language++;
>>  +                    *q++ = *language++;
> 
> Is this safe for overflows (both for read and write)?

It depends on my previous patch, so unless you mess with the mpegts.c patch this is safe, as they have to match each other.

> 
>>  +                    language++;             /* ignore comma */
>>  +                    switch (*language++) {
>>  +                    case 'R':
>>  +                        type = 0;   /* Reserved */
>>  +                        break;
>>  +
>>  +                    case 'I':       /* Inital Page */
>>  +                        type = 1;
>>  +                        break;
>>  +
>>  +                    case 'S':       /* Subtitle Page */
>>  +                        type = 2;
>>  +                        break;
>>  +
>>  +                    case 'A':       /* Additional Info Page */
>>  +                        type = 3;
>>  +                        break;
>>  +
>>  +                    case 'P':       /* Programme Schedule Page */
>>  +                        type = 4;
>>  +                        break;
>>  +
>>  +                    case 'H':       /* Subtitle For Hard Of 
> Hearing */
>>  +                        type = 5;
>>  +                        break;
>>  +
>>  +                    default:
>>  +                        type = 31;   /* reserved */
>>  +                        break;
>>  +                    }
>>  +                    sscanf( language, "%x", &page);
>>  +                    language += 3;
>>  +                    /* 100-799 is as is, 800-899 is mapped to 000-099 */
>>  +                    if( page>0x7ff)
>>  +                        page -= 0x800;
>>  +                    *q++ = (type<<3) | page/256;
>>  +                    *q++ = page & 255;
>>  +                    if (*language!=',')
>>  +                        break;
>>  +                    language++;
>>  +                }
>>  +                *sizep = (q-ss);
>>  +            }
>>  +            else {
> 
> Note: your coding style is completely off in that patch as well.
> 

what am i missing, coding style wise? some clues please