[Ffmpeg-devel] [PATCH] Convert ffmpeg.c to use the swscaleinterface

[Michael Niedermayer]

Hi

On Wed, Aug 02, 2006 at 09:53:49AM +0200, Luca Abeni wrote:
> Hi Michael,
> 
> On Tue, 2006-08-01 at 23:44 +0200, Michael Niedermayer wrote:
> [...]
> > > So, I think the patch does not introduce any obvious regression... Is it
> > > ok for commit?
> > 
> > do the regression tests pass? if so ive no objections to the patch ...
> Opsss... There's a failure in "make libavtest":
> --- ./data/libav.regression     2006-08-02 08:14:52.867521352 +0200
> +++ /tmp/ffmpeg/tests/libav.regression.ref      2006-08-02 07:36:00.492096312 +0200
> @@ -40,7 +40,7 @@
>  0c5fe86621b7377705837f304d4ba1e9 *./data/b-libav.ppm
>  7603575 ./data/b-libav.ppm
>  ./data/b-libav.ppm CRC=0xb2bb8e92
> -83c96fdc22006750e8c14afd3cf21b30 *./data/b-libav.gif
> +88a98269295fbfce7816558ad84e1259 *./data/b-libav.gif
>  2906382 ./data/b-libav.gif
>  b977a4fedff90a79baf70c8e02986820 *./data/b-libav.y4m
>  3801810 ./data/b-libav.y4m
> 
> Regression test: Error.
> make: *** [libavtest] Error 1
> 
> This is happening because libavformat/gif.c is setting the codec pixel
> format in gif_write_header(). I do not know if this is valid (in fact,
> there is a "XXX: is it allowed ? seems to work so far..." comment in the
> code). And since the swscale context is created before writing the
> header, the YUV420P ---> RGB24 conversion is lost.
> 
> Now, I can see some possible fixes:
> 1) Deciding the setting the codec pixel format in the format
> write_header() function is not allowed. In this case, the pixel format
> will have to be set by the user, and gif_write_header() will simply
> check pix_fmt == PIX_FMT_RGB24, failing if false (this is what
> libavformat/yuv4mpeg.c is doing)
> 2) Setting the pixel format to PIX_FMT_RGB24 somewhere before
> write_header()
> 3) modify ffmpeg.c to allocate the sws context after having called
> write_header()
> 4) repeat the check on enc->pix_fmt every time that we encode a frame,
> as ffmpeg.c is currently doing (dynamically allocating a new sws context
> if needed)
> 5) maybe some other solution that I am not seeing now...

moving the gif stuff to libavcodec / image2, sadly thats probably not
a reasonable solution now as it would delay the swscaler stuff unneeded ...

> 
> Which one do you prefer?

1. seems easiest, and as gif shouldnt be in libavformat but libavcodec this
would just be a temporary solution until some volunteer moves gif support
to libavcodec

4. (allowing the pix_fmt to change midway?) would be nice too, independant
of gif or the swscaler move ...

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

In the past you could go to a library and read, borrow or copy any book
Today you'd get arrested for mere telling someone where the library is