[Ffmpeg-devel] [PATCH] remove special nuv wav tags

[Måns Rullgård]

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

> Hello,
> attached patch does $subj and is tested. Feel free to either apply,
> ignore or think of something better.
>
> Greetings,
> Reimar D?ffinger
>
> Index: libavformat/nuv.c
> ===================================================================
> --- libavformat/nuv.c	(revision 6906)
> +++ libavformat/nuv.c	(working copy)
> @@ -90,12 +90,18 @@
>                      url_fskip(pb, 4);
>
>                  if (ast) {
> +                    int lookuptag;
>                      ast->codec->codec_tag = get_le32(pb);
>                      ast->codec->sample_rate = get_le32(pb);
>                      ast->codec->bits_per_sample = get_le32(pb);
>                      ast->codec->channels = get_le32(pb);
> +                    lookuptag = ast->codec->codec_tag;
> +                    switch (lookuptag) {
> +                      case MKTAG('R', 'A', 'W', 'A'): lookuptag = 1; break;
> +                      case MKTAG('L', 'A', 'M', 'E'): lookuptag = 0x55; break;
> +                    }
>                      ast->codec->codec_id =
> -                        wav_codec_get_id(ast->codec->codec_tag,
> +                        wav_codec_get_id(lookuptag,
>                                           ast->codec->bits_per_sample);
>                  } else
>                      url_fskip(pb, 4 * 4);
> Index: libavformat/riff.c
> ===================================================================
> --- libavformat/riff.c	(revision 6906)
> +++ libavformat/riff.c	(working copy)
> @@ -200,10 +200,6 @@
>      { CODEC_ID_TTA, MKTAG('T', 'T', 'A', '1') },
>      { CODEC_ID_WAVPACK, MKTAG('W', 'V', 'P', 'K') },
>      { CODEC_ID_SHORTEN, MKTAG('s', 'h', 'r', 'n') },
> -
> -    // for NuppelVideo (nuv.c)
> -    { CODEC_ID_PCM_S16LE, MKTAG('R', 'A', 'W', 'A') },
> -    { CODEC_ID_MP3, MKTAG('L', 'A', 'M', 'E') },
>      { 0, 0 },
>  };

Are those two the only allowed codecs?  Then something like this seems
much more appropriate:

    int tag = get_le32(pb);
    switch(tag){
    case MKTAG('R', 'A', 'W', 'A'):
        ast->codec->codec_id = CODEC_ID_PCM_S16LE;
        break;
    case MKTAG('L', 'A', 'M', 'E'):
        ast->codec->codec_id = CODEC_ID_MP3;
        break;
    }

See, no need to involve RIFF tags at all.  RIFF tags are *not* the
bloody center of the universe, even if mplayer appears to have been
written with that assumption.

Also, setting AVCodecContext.codec_tag in nuv.c seems a little
questionable, given that most of the code treats it as RIFF codec tag,
which it is not in this case.

-- 
M?ns Rullg?rd
mru at inprovide.com