[FFmpeg-cvslog] r27730 - in trunk/libswscale: swscale.c swscale_internal.h
Michael Niedermayer
michaelni
Wed Oct 22 03:22:28 CEST 2008
On Tue, Oct 21, 2008 at 10:06:58PM +0200, Vitor Sessak wrote:
> Michael Niedermayer wrote:
>> On Mon, Oct 20, 2008 at 08:05:26PM +0200, Vitor Sessak wrote:
>>> Michael Niedermayer wrote:
>>>> On Mon, Oct 20, 2008 at 06:27:36PM +0200, Vitor Sessak wrote:
>>>>> Michael Niedermayer wrote:
>>>>>> On Sun, Oct 19, 2008 at 08:25:41PM +0200, Vitor Sessak wrote:
>>>>>>> Michael Niedermayer wrote:
>>>>>>>> On Sat, Oct 18, 2008 at 11:55:58PM +0200, Vitor Sessak wrote:
>>>>>>>>> vitor wrote:
>>>>>>>>>> Author: vitor
>>>>>>>>>> Date: Wed Oct 8 19:46:22 2008
>>>>>>>>>> New Revision: 27730
>>>>>>>>>> Log:
>>>>>>>>>> Add a new unscaled PAL8 -> RGB converter.
>>>>>>>>> 1000l for me for this patch. I tested it, but without noticing that
>>>>>>>>> typing "make" do _not_ rebuild swscale-example. Actually it made a
>>>>>>>>> small regression in {PAL,RGB8,BGR8,RGB4,BGR4}->{RGB32, BGR32}.
>>>>>>>>>
>>>>>>>>> Three possibilities:
>>>>>>>>>
>>>>>>>>> 1- Revert it
>>>>>>>>> 2- sws_fix10l.diff: fix the actual bug (in rgb2rgb.c)
>>>>>>>>> 3- sws_hack.diff: just fix my breakage
>>>>>>>> i do not think 2 or 3 fix the code
>>>>>>>> the palette is in RGB32 format IIRC so one side of the #ifdef has to
>>>>>>>> be
>>>>>>>> wrong. Also the rgb24 would need ifdefs which it does not have, or
>>>>>>>> the
>>>>>>>> palette would have to be converted into a differnt byte order or the
>>>>>>>> switch case would need to be changed a little
>>>>>>> AFAIK, before the palette was either in BGR32 or RGB32 depending on
>>>>>>> alignment. Now I've made everything alignment independent. Also, if I
>>>>>>> understand correctly the code, I think that swscaler uses a slightly
>>>>>>> unusual definition of RGB/BGR.
>>>>>> the swscaler uses the same definition as lav* uses, and its very
>>>>>> similar
>>>>>> to mplayers except some rgb <-> bgr flips
>>>>>> These definitions have also been choosen that way for efficiency and
>>>>>> simplicity.
>>>>>> anyway, this patch doesnt look correct either
>>>>>> i suggest you read the definition of the formats in avutils.h
>>>>> Documented header files are a bliss =)
>>>>>
>>>>> Hopefully correct version attached.
>>>>>
>>>>> -Vitor
>>>> [...]
>>>>> void palette8tobgr32(const uint8_t *src, uint8_t *dst, long
>>>>> num_pixels, const uint8_t *palette)
>>>>> {
>>>>> long i;
>>>>> - for (i=0; i<num_pixels; i++)
>>>>> - {
>>>>> - #ifdef WORDS_BIGENDIAN
>>>>> - dst[3]= palette[src[i]*4+0];
>>>>> - dst[2]= palette[src[i]*4+1];
>>>>> - dst[1]= palette[src[i]*4+2];
>>>>> - #else
>>>>> - //FIXME slow?
>>>>> - dst[0]= palette[src[i]*4+0];
>>>>> - dst[1]= palette[src[i]*4+1];
>>>>> - dst[2]= palette[src[i]*4+2];
>>>>> - //dst[3]= 0; /* do we need this cleansing? */
>>>>> - #endif
>>>>> - dst+= 4;
>>>>> - }
>>>>> + for (i=0; i<num_pixels; i++)
>>>>> + ((uint32_t *) dst)[i] =
>>>>> + bswap_32(((const uint32_t *) palette)[src[i]]) >> 8;
>>>>> }
>>>> i think it would be faster to bswap 256 entries in palette than
>>>> width*height, besides this would allow the same code to be used for
>>>> rgb & bgr
>>> Yes, but it would have to be done in swscale(). Isn't having both
>>> pal_rgb[] and pal_bgr[] in the context a bit of an overkill?
>> yep, what about a pal_native ?
>
> ok.
>
>> there are btw 4 32bit rgb/bgr formats so its not 2 vs 1 but 4 vs 1
>>> Also palette8tobgr32() is a public function, so I think it should either
>>> be fixed or removed...
>> i dont mind removing broken and redundant functions
>
> Both done.
[...]
> /**
> - * Palette is assumed to contain BGR32.
> + * Palette is assumed to contain RGB32 (in CPU endianness).
> */
> void palette8torgb24(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette)
> {
> long i;
> -/*
> - Writes 1 byte too much and might cause alignment issues on some architectures?
> +
> for (i=0; i<num_pixels; i++)
> - ((unsigned *)(&dst[i*3])) = ((unsigned *)palette)[src[i]];
> -*/
> - for (i=0; i<num_pixels; i++)
> {
> //FIXME slow?
> +#ifdef WORDS_BIGENDIAN
> + dst[0]= palette[src[i]*4+1];
> + dst[1]= palette[src[i]*4+2];
> + dst[2]= palette[src[i]*4+3];
> +#else
> dst[0]= palette[src[i]*4+2];
> dst[1]= palette[src[i]*4+1];
> dst[2]= palette[src[i]*4+0];
> +#endif
> dst+= 3;
> }
> }
>
> +/**
> + * Palette is assumed to contain RGB32 (in CPU endianness).
> + */
> void palette8tobgr24(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette)
> {
> long i;
> -/*
> - Writes 1 byte too much and might cause alignment issues on some architectures?
> +
> for (i=0; i<num_pixels; i++)
> - ((unsigned *)(&dst[i*3])) = ((unsigned *)palette)[src[i]];
> -*/
> - for (i=0; i<num_pixels; i++)
> {
> //FIXME slow?
> +#ifdef WORDS_BIGENDIAN
> + dst[2]= palette[src[i]*4+1];
> + dst[1]= palette[src[i]*4+2];
> + dst[0]= palette[src[i]*4+3];
> +#else
> dst[0]= palette[src[i]*4+0];
> dst[1]= palette[src[i]*4+1];
> dst[2]= palette[src[i]*4+2];
> +#endif
> dst+= 3;
> }
> }
You could merge these by changing the palette as well, that also would
avoid the #ifdef WORDS_BIGENDIAN
> Index: libswscale/rgb2rgb.h
> ===================================================================
> --- libswscale/rgb2rgb.h (revision 27788)
> +++ libswscale/rgb2rgb.h (working copy)
> @@ -61,7 +61,7 @@
> extern void bgr8torgb8 (const uint8_t *src, uint8_t *dst, long src_size);
>
>
> -extern void palette8torgb32(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette);
> +extern void palette8topacked32(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette);
> extern void palette8tobgr32(const uint8_t *src, uint8_t *dst, long num_pixels, const uint8_t *palette);
this one has been removed ...
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/attachments/20081022/a26af335/attachment.pgp>
More information about the ffmpeg-cvslog
mailing list