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

[Vladimir Voroshilov]

2007/9/26, Vitor Sessak <vitor1001 at gmail.com>:
> 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...

Fixed.

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

Shouldn't be Changelog updated after all three patches applied ?
And... Can i ask you to update docs ?

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

I'll fix it

-- 
Regards,
Vladimir Voroshilov     mailto:voroshil at gmail.com
JID: voroshil at gmail.com, voroshil at jabber.ru
ICQ: 95587719
-------------- next part --------------
A non-text attachment was scrubbed...
Name: amv_demux_ffmpeg_as_avi2.diff
Type: text/x-diff
Size: 2830 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070926/a1ce7eb9/attachment.diff>