[FFmpeg-devel] [PATCH 8/8] avidec: demux ASS and SRT tracks

[Aurelien Jacobs]

On Wed, Jul 21, 2010 at 07:46:51PM +0200, Reimar D?ffinger wrote:
> On Wed, Jul 21, 2010 at 02:53:08PM +0200, Aurelien Jacobs wrote:
> > +static int read_gab2_sub(AVStream *st, AVPacket *pkt) {
> > +    if (!strcmp(pkt->data, "GAB2") && AV_RL16(pkt->data+5) == 2) {
> > +        uint8_t tmp, desc[256], *d = desc;
> > +        const uint8_t *ptr = pkt->data+7;
> > +        unsigned int size, name_size = bytestream_get_le32(&ptr);
> > +        int i, score = AVPROBE_SCORE_MAX / 2;
> > +        AVIStream *ast = st->priv_data;
> > +        AVInputFormat *sub_demuxer;
> > +        AVRational time_base;
> > +        ByteIOContext *pb;
> > +        AVProbeData pd;
> > +
> > +        if (name_size > pkt->size-17)
> > +            return 0;
> 
> What about pkt->size < 17?

Humm... Indeed... Will fix.

> > +        for (i=0; i<name_size; i+=2)
> > +            PUT_UTF8(bytestream_get_le16(&ptr), tmp,
> > +                     if(d < desc+sizeof(desc)-1)  *d++ = tmp;);
> 
> tmp declaration could be moved in here.

OK.

> Also are you sure bytestream_get_le16 is correct (i.e. it is UCS-2)
> and it shouldn't be something based on GET_UTF16 instead?

Indeed, it seems to be UTF-16 and not UCS-2. I will fix this.

> Also it might be nicer to malloc the destination buffer to
> 7*name_size+1 (after limiting name_size) to avoid the check
> in the inner loop and allow longer metadata.

I will give this a try.

> > +        memmove(pkt->data, ptr, size);
> 
> Seems like a very inefficient way to do
> pkt->data += pkt->size - size;

But this is a way that won't crash when doing av_free_packet(pkt).
pkt->data points to av_malloced memory, so we don't want to loose this
pointer.

Aurel