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

[Måns Rullgård]

Reimar D?ffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de> writes:

> On Thu, Feb 14, 2008 at 03:31:58PM +0100, Reimar D?ffinger wrote:
>> On Thu, Feb 14, 2008 at 02:06:37PM -0000, M?ns Rullg?rd wrote:
>> > Reimar D?ffinger wrote:
>> > > On Thu, Jan 31, 2008 at 11:57:35PM +0100, Reimar D?ffinger wrote:
>> > >> attached patch makes vorbis_comment (which lacks a ff_ prefix btw) parse
>> > >> the language tag into AVStream.language (if a mapping is known).
>> > >> It also adds vorbis comment parsing to oggparseogm.c (I would apply as a
>> > >> second step).
>> > >> The code is probably improvable, the mapping list even more so.
>> > >
>> > > So is this rejected, should it be done differently or just applied?
>> > 
>> > I'd prefer if a mapping of common language names to codes were kept
>> > outside the ogg demuxer.  It could be useful to someone else.
>> 
>> Sure, where? libavformat/utils.c? A new file (seems like overkill)?
>> ff_ or av_ (i.e. just internal or public)?
>
> Updated.
>
> Greetings,
> Reimar D?ffinger
>
> Index: libavformat/oggparseflac.c
> ===================================================================
> --- libavformat/oggparseflac.c	(revision 12287)
> +++ libavformat/oggparseflac.c	(working copy)
> @@ -69,7 +69,7 @@
>          st->time_base.num = 1;
>          st->time_base.den = st->codec->sample_rate;
>      } else if (mdt == 4) {
> -        vorbis_comment (s, os->buf + os->pstart + 4, os->psize - 4);
> +        vorbis_comment (s, os->buf + os->pstart + 4, os->psize - 4, st);
>      }
>
>      return 1;

OK

> Index: libavformat/oggparseogm.c
> ===================================================================
> --- libavformat/oggparseogm.c	(revision 12287)
> +++ libavformat/oggparseogm.c	(working copy)
> @@ -43,6 +43,9 @@
>
>      if(!(*p & 1))
>          return 0;
> +    if (*p == 3)
> +        if (os->psize > 8)
> +            vorbis_comment (s, p + 7, os->psize - 8, st);
>      if(*p != 1)
>          return 1;

OK

> Index: libavformat/oggparsetheora.c
> ===================================================================
> --- libavformat/oggparsetheora.c	(revision 12287)
> +++ libavformat/oggparsetheora.c	(working copy)
> @@ -102,7 +102,7 @@
>          st->codec->codec_id = CODEC_ID_THEORA;
>
>      } else if (os->buf[os->pstart] == 0x83) {
> -        vorbis_comment (s, os->buf + os->pstart + 7, os->psize - 8);
> +        vorbis_comment (s, os->buf + os->pstart + 7, os->psize - 8, st);
>      }
>
>      st->codec->extradata = av_realloc (st->codec->extradata, cds);

OK

> Index: libavformat/oggparsevorbis.c
> ===================================================================
> --- libavformat/oggparsevorbis.c	(revision 12287)
> +++ libavformat/oggparsevorbis.c	(working copy)
> @@ -31,7 +31,7 @@
>  #include "avstring.h"
>
>  extern int
> -vorbis_comment(AVFormatContext * as, uint8_t *buf, int size)
> +vorbis_comment(AVFormatContext * as, uint8_t *buf, int size, AVStream *st)
>  {
>      const uint8_t *p = buf;
>      const uint8_t *end = buf + size;
> @@ -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.

>          }
>      }
>
> @@ -210,7 +212,7 @@
>          st->time_base.den = st->codec->sample_rate;
>      } else if (os->buf[os->pstart] == 3) {
>          if (os->psize > 8)
> -            vorbis_comment (s, os->buf + os->pstart + 7, os->psize - 8);
> +            vorbis_comment (s, os->buf + os->pstart + 7, os->psize - 8, st);
>      } else {
>          st->codec->extradata_size =
>              fixup_vorbis_headers(s, priv, &st->codec->extradata);
> Index: libavformat/oggdec.h
> ===================================================================
> --- libavformat/oggdec.h	(revision 12287)
> +++ libavformat/oggdec.h	(working copy)
> @@ -85,6 +85,6 @@
>  extern ogg_codec_t theora_codec;
>  extern ogg_codec_t vorbis_codec;
>
> -extern int vorbis_comment(AVFormatContext *ms, uint8_t *buf, int size);
> +extern int vorbis_comment(AVFormatContext *ms, uint8_t *buf, int size, AVStream *st);

Please break that line.

>  #endif /* FFMPEG_OGGDEC_H */
> Index: libavformat/avformat.h
> ===================================================================
> --- libavformat/avformat.h	(revision 12287)
> +++ libavformat/avformat.h	(working copy)
> @@ -871,6 +871,8 @@
>                   const char *url,
>                   int is_output);
>
> +void av_lang_str2id(char langid[4], const char *name);
> +
>  /**
>   * parses width and height out of string str.
>   * @deprecated Use av_parse_video_frame_size instead.
> Index: libavformat/utils.c
> ===================================================================
> --- libavformat/utils.c	(revision 12287)
> +++ libavformat/utils.c	(working copy)
> @@ -2666,6 +2666,22 @@
>          dump_stream_format(ic, i, index, is_output);
>  }
>
> +static const struct {
> +    const char langid[4];
> +    const char * const name;

Two of those consts are unnecessary since the entire thing is const.

> +} 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...

Someone else will have to OK the inclusion of this in utils.c.

-- 
M?ns Rullg?rd
mans at mansr.com