[FFmpeg-devel] [PATCH] Read ogg stream language

Reimar Döffinger Reimar.Doeffinger
Sat Mar 1 23:00:10 CET 2008


On Sat, Mar 01, 2008 at 09:44:06PM +0000, M?ns Rullg?rd wrote:
> > @@ -96,6 +96,8 @@
> >                  as->track = atoi(ct);
> >              else if (!strcmp(tt, "ALBUM"))
> >                  av_strlcpy(as->album, ct, sizeof(as->album));
> > +            else if (st && !strcmp(tt, "LANGUAGE"))
> > +                av_lang_str2id(st->language, ct);
> 
> I think it might be more logical writing this as
> 
>     else if (!strcmp(tt, "LANGUAGE"))
>         if (st)
>             av_lang_str2id(st->language, ct);
> 
> especially if more tags are added.  It's not really important though.

Not such a good idea, at least it would need {} then (which would be a
bit ugly) or the next else will be unclear where it belongs.
If anything I'd propose adding before the new code a
// stream-specific tags

> > +} langmap [] = {
> > +    {"eng", "English"},
> > +    {"und", NULL}
> > +};
> > +
> > +void av_lang_str2id(char langid[4], const char *name) {
> > +    int i;
> > +    for (i = 0; langmap[i].name; i++)
> > +        if (!strcasecmp(name, langmap[i].name))
> 
> Although strcasecmp() is standard, I'm certain some systems will be
> missing it.  Not that I care...

I doubt it will be missing somewhere, also I was suspicious of it as
well so I checked that we already use it (mostly for network stuff, but
also in mp3.c and utils.c).
The much bigger problem is that it is locale-dependant (i.e.
strcasecmp("p", "P") != 0 is possible and actually for tr_TR locale
strcasecmp("i", "I") != 0 is a fact), and thus it actually does not do at
all what it is supposed to do in all the cases
where it is used currently - so IOW every single of the ca. 15 + lots in
ffserver uses of it is bug. But I don't want to fix that right now.

Greetings,
Reimar D?ffinger




More information about the ffmpeg-devel mailing list