[FFmpeg-cvslog] r27730 - in trunk/libswscale: swscale.c swscale_internal.h

[Vitor Sessak]

Baptiste Coudurier wrote:
> Hi Vitor,
> 
> 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.
>>
>> [...]
>  >
>> -    }
>> +        ((uint32_t *) dst)[i] = ((const uint32_t *) palette)[src[i]];
> 
> I'll nitpick on style for once :>
> When using casts, I tend to prefer avoiding useless spaces like:
> ((const uint32_t*)dst)
> 
> Just my 2 cents, how others prefer it ?

I have no preference, so I don't mind following those who do.

-Vitor