[FFmpeg-devel] [PATCH] get rid of nonsense palette pointer for RGB8, GRAY8 etc.

[Daniel Verkamp]

On Fri, Mar 20, 2009 at 12:40 PM, Reimar D?ffinger
<Reimar.Doeffinger at gmx.de> wrote:
> Hello,
> I have no idea why that code came to be like that, but I know for sure
> it causes regression tests to fail because the silly copyPlanar function
> things that palette pointer is the U plane it needs to copy.
> Probably that function should be fixed anyway, but this change still
> seems right:
> Index: libavcodec/imgconvert.c
> ===================================================================
> --- libavcodec/imgconvert.c ? ? (revision 18076)
> +++ libavcodec/imgconvert.c ? ? (working copy)
> @@ -681,6 +681,11 @@
> ? ? case PIX_FMT_YUYV422:
> ? ? case PIX_FMT_UYVY422:
> ? ? case PIX_FMT_UYYVYY411:
> + ? ?case PIX_FMT_RGB8:
> + ? ?case PIX_FMT_BGR8:
> + ? ?case PIX_FMT_GRAY8:
> + ? ?case PIX_FMT_RGB4_BYTE:
> + ? ?case PIX_FMT_BGR4_BYTE:
> ? ? case PIX_FMT_RGB4:
> ? ? case PIX_FMT_BGR4:
> ? ? case PIX_FMT_MONOWHITE:
> @@ -691,11 +696,6 @@
> ? ? ? ? picture->data[3] = NULL;
> ? ? ? ? return size;
> ? ? case PIX_FMT_PAL8:
> - ? ?case PIX_FMT_RGB8:
> - ? ?case PIX_FMT_BGR8:
> - ? ?case PIX_FMT_RGB4_BYTE:
> - ? ?case PIX_FMT_BGR4_BYTE:
> - ? ?case PIX_FMT_GRAY8:
> ? ? ? ? size2 = (size + 3) & ~3;
> ? ? ? ? picture->data[0] = ptr;
> ? ? ? ? picture->data[1] = ptr + size2; /* palette is stored here as 256 32 bit words */
>
>
> Interestingly it causes some regression tests to fail, which I have not
> yet investigated:
> diff -u -w "/data/ffmpeg"/tests/libav.regression.ref tests/data/lavf.regression
> --- /data/ffmpeg/tests/libav.regression.ref ? ? 2009-03-20
> 12:04:26.000000000 +0100
> +++ tests/data/lavf.regression ?2009-03-20 18:39:06.000000000 +0100
> @@ -47,7 +47,7 @@
> ?./tests/data/b-pbmpipe.pbm CRC=0x806e17d8
> ?aff140ce80a1c86c1bf54118ad23da7b *./tests/data/b-pgmpipe.pgm
> ?2534775 ./tests/data/b-pgmpipe.pgm
> -./tests/data/b-pgmpipe.pgm CRC=0x0e82c482
> +./tests/data/b-pgmpipe.pgm CRC=0xf485870f
> ?9169b1f1ca56f01a6e1f5041572aa1d4 *./tests/data/b-ppmpipe.ppm
> ?7603575 ./tests/data/b-ppmpipe.ppm
> ?./tests/data/b-ppmpipe.ppm CRC=0x79bd6ce6
> @@ -56,7 +56,7 @@
> ?b977a4fedff90a79baf70c8e02986820 *./tests/data/b-libav.y4m
> ?3801810 ./tests/data/b-libav.y4m
> ?0a6d74b54396884f117669965b57d3b5 *./tests/data/b-libav02.pgm
> -./tests/data/b-libav%02d.pgm CRC=0xc8032eb1
> +./tests/data/b-libav%02d.pgm CRC=0x7e552eb1
> ?101391 ./tests/data/b-libav02.pgm
> ?dbe42bd8d9ca0acbd2673bd739705f0f *./tests/data/b-libav02.ppm
> ?./tests/data/b-libav%02d.ppm CRC=0x6f775c0d
>

Some codecs assume these pixfmts do have a palette (gif.c, bmpenc.c,
pcxenc.c at least).

Perhaps it would be better to make these codecs only accept pal8 and
automatically convert rgb8 etc. to pal8 when desired, since there is
no extra work except to set the palette pointer? (I don't know how
this is all hooked up internally, so maybe this is not so easy...)

Thanks,
-- Daniel Verkamp