[FFmpeg-devel] [PATCH] Video decoder and demuxer for AMV files

[Aurelien Jacobs]

Vladimir Voroshilov wrote:

> 2007/9/24, Aurelien Jacobs <aurel at gnuage.org>:
> > On Mon, 24 Sep 2007 20:14:36 +0700
> > "Vladimir Voroshilov" <voroshil at gmail.com> wrote:
> 
> [...]
> 
> > > amv_demux_ffmpeg.patch: AMV file demuxer.
> >
> > Better post one email per patch.
> >
> > > AMV files contains ugly headers: strh sections are filled by zero,
> > > strf sections contains movie duration, image size and frame duration
> > > for video and waveformatex structure for audio respectively.
> > > Index is absent.
> > > Patch uses simplified version of avi_read_header routine for
> > > extracting needed info.
> >
> > I didn't looked carefully, but according to your description, this
> > looks mostly like duplication of avi_read_header. This is not
> > acceptable. You should modify avi_read_header directly instead.
> 
> Is attached version better ?

It is indeed much better.

> Index: mplayer/libavformat/avidec.c
> ===================================================================
> --- mplayer/libavformat/avidec.c	(revision 10543)
> +++ mplayer/libavformat/avidec.c	(working copy)
> @@ -86,7 +86,8 @@
>      if(tag == MKTAG('A', 'V', 'I', 0x19))
>          av_log(NULL, AV_LOG_INFO, "file has been generated with a totally broken muxer\n"); else
> -    if (tag != MKTAG('A', 'V', 'I', ' ') && tag != MKTAG('A', 'V', 'I', 'X'))
> +    if (tag != MKTAG('A', 'V', 'I', ' ') && tag != MKTAG('A', 'V', 'I', 'X')
> +    && tag != MKTAG('A', 'M', 'V', ' '))

Alignment is not nice. Maybe use something like this:

+    if (   tag != MKTAG('A', 'V', 'I', ' ') && tag != MKTAG('A', 'V', 'I', 'X')
+        && tag != MKTAG('A', 'M', 'V', ' '))

> @@ -265,6 +268,8 @@
>              avi->is_odml = 1;
>              url_fskip(pb, size + (size & 1));
>              break;
> +        case MKTAG('a', 'm', 'v', 'h'):
> +	    amv_file_format=1;

Bad indentation (TABs are forbidden, and there are more of them
in your patch).

> @@ -298,6 +306,16 @@
>                      goto fail;
>                  st->priv_data = ast;
>              }
> +            if(amv_file_format){
> +	        switch(stream_index){
> +		case 0:
> +		    tag1=MKTAG('v','i','d','s');
> +		    break;
> +		case 1:
> +		    tag1=MKTAG('a','u','d','s');
> +		    break;
> +		}
> +	    }

(some more TABs...)
I haven't checked the surrounding code but if stream_index can
only be 0 or 1, you can replace your whole switch() by:

  tag1 = stream_index ? MKTAG('a','u','d','s') : MKTAG('v','i','d','s');

> @@ -1005,6 +1036,11 @@
>          p->buf[8] == 'A' && p->buf[9] == 'V' &&
>          p->buf[10] == 'I' && (p->buf[11] == ' ' || p->buf[11] == 0x19))
>          return AVPROBE_SCORE_MAX;
> +    if (p->buf[0] == 'R' && p->buf[1] == 'I' &&
> +        p->buf[2] == 'F' && p->buf[3] == 'F' &&
> +        p->buf[8] == 'A' && p->buf[9] == 'M' &&
> +        p->buf[10] == 'V' && p->buf[11] == ' ')
> +        return AVPROBE_SCORE_MAX;
>      else
>          return 0;
>  }

BTW: this 'else' should be removed, but this is not related to this patch.

> > > P.S. I also use new fourcc (AMVV) to allow further interaction with
> > > mplayer.
> >
> > IIRC, this is not needed.
> 
> When i set codec_tag to MJPG, mjpeg decoder is loaded instead of amvv
> (in MPlayer of course). codec_tag==AMVV plus apropriate section in codecs.conf
> works fine.

I'm not really sure about this issue, but my guess is that either:
  - AMVV is a normal AVI fourcc, then it should be placed in riff.c
    (and you should probably use codec_get_id())
  - AMVV is *not* a normal fourcc, then you shouldn't set codec_tag
    at all (see: http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/2007-September/035453.html )

Aurel