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

[Vitor Sessak]

Hi

Vladimir Voroshilov wrote:
> 2007/9/26, Aurelien Jacobs <aurel at gnuage.org>:
>> On Wed, 26 Sep 2007 17:40:21 +0700
>> "Vladimir Voroshilov" <voroshil at gmail.com> wrote:
>>
>>> 2007/9/25, Vladimir Voroshilov <voroshil at gmail.com>:
>>> [...]
>>>> Fake fourcc code removed.
>>>> I've also changed CODEC_ID_ADPCM_IMA_WS to CODEC_ID_ADPCM_IMA_AMV,
>>>> thus Vitor's patch is required too (his codec provides better sound).
>>> Small nit: AMV files are currently reported as "avi", but strictly
>>> speaking they are broken AVI files.
>>> Thus, i suggest to add separate AVInputFormat structure especially for
>>> AMV files (with apropriate name and comment) and
>>> register it with AVI's config variable (IOW, REGISTER_DEMUXER (AVI, amv))
>> I don't think it's worth. I much prefer your previous patch.
>> Note that you will have to update it (simplify it) according to
>> my recent commit on avidec.c.
> 
> Attached file is previous patch, synced with latest svn.
> I suppose it is latest version.
> If separate format will be really required it can be easily
> implemented sometime in the future.
> 
>>>> 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');
>>> I have no info about streams count, so kept as is
>> I gave a deeper look at the source. My proposition is safe.
>> tag1 will always be video for the first stream and audio for
>> the second stream (and all subsequent stream if they happen
>> to exist).
> 
> Ok. Got suggested line.
> 
> P.S. Can anybody commit all three AMV patches ? Michael?

I guess Michael is the maintainer for adding new codecs/formats. So 
after he review and ok the patches (he will do probably one at a time), 
I (or anyone else with commit rights) can commit it.

> ------------------------------------------------------------------------
> 
> Index: mplayer/libavformat/avienc.c
> ===================================================================
> --- mplayer/libavformat/avienc.c	(revision 10592)
> +++ mplayer/libavformat/avienc.c	(working copy)
> @@ -572,4 +572,5 @@
>      avi_write_trailer,
>      .codec_tag= (const AVCodecTag*[]){codec_bmp_tags, codec_wav_tags, 0},
>  };
> +

I guess that shouldn't be there...

[...]
> 
> ------------------------------------------------------------------------

The documentation changes are missing (Changelog, doc/ffmpeg-doc.texi).

Also, maybe it would be nice to add a line to libavcodec/Makefile like
OBJS-$(CONFIG_AMVVIDEO_DECODER)            += sp5xdec.o mjpegdec.o mjpeg.o

but that should be in the decoder patch, not in the demuxer one...

-Vitor