[MPlayer-dev-eng] [PATCH] handle vobsub idx information from lavf demuxer

[Aurelien Jacobs]

Reimar Döffinger wrote:

> Hello,
> 
> Sorry for being nit-picky, but...
> 
> On Thu, Aug 28, 2008 at 03:45:54PM +0200, Aurelien Jacobs wrote:
> > +static void lavf_parse_idx(sh_sub_t* sh) {
> > +    char *ptr = sh->extradata;
> 
> Wouldn't it be better to use const here?

I guess so. Fixed locally.

> > +    do {
> > +        if (!strncmp(ptr, "size:", 5))
> > +            sscanf(ptr, "size: %dx%d", &sh->width, &sh->height);
> 
> Also, I have some doubts it is okay to assume that extradata is
> 0-terminated.

It is not.
That's why the loop check that ptr won't overflow extradata:
    ptr < sh->extradata+sh->extradata_len
Now if extradata is truncated in the middle of a line (ie. corrupted
extradata), sscanf could read wrong values. But what would you expect
from reading corrupted extradata ?

> Unfortunately changing it would be an API change, but am I the only one
> who thinks this extradata format is not just a bit but actually
> _extremely_ stupid?

What do you consider stupid here ? The lack of 0-termination or the
text format I'm parsing here ?
Adding a zero termination could be nice.
About the text format parsed here, it is the format of .idx dvd files
IIRC, and that's exactly what is stored in matroska files for example.
And it have to be kept for remuxing, etc...

Aurel