Hi,
On Tue, Feb 21, 2012 at 1:21 PM, Diego Biurrun <[email protected]> wrote:
> On Tue, Feb 21, 2012 at 11:31:04AM -0800, Ronald S. Bultje wrote:
>> On Tue, Feb 21, 2012 at 11:12 AM, Diego Biurrun <[email protected]> wrote:
>> > --- a/libswscale/ppc/yuv2rgb_altivec.c
>> > +++ b/libswscale/ppc/yuv2rgb_altivec.c
>> > @@ -103,200 +104,201 @@ adjustment.
>> > -#define vec_unh(x) \
>> > - (vector signed short) \
>> > - vec_perm(x,(__typeof__(x)){0}, \
>> > - ((vector unsigned
>> > char){0x10,0x00,0x10,0x01,0x10,0x02,0x10,0x03,\
>> > -
>> > 0x10,0x04,0x10,0x05,0x10,0x06,0x10,0x07}))
>> > +#define vec_unh(x) \
>> > + (vector signed short) \
>> > + vec_perm(x, (__typeof__(x)) { 0 }, \
>> > + ((vector unsigned char) { \
>> > + 0x10, 0x00, 0x10, 0x01, 0x10, 0x02, 0x10, 0x03, \
>> > + 0x10, 0x04, 0x10, 0x05, 0x10, 0x06, 0x10, 0x07 }))
>>
>> I don't like this at all. First, it looks like the cast is a separate
>> statement because the next line is unindented. Second, the { <newline>
>> <values> suggests that we're dealing with a if (..) { <newline> <code>
>> here, whereas it really is an array. I'd prefer if the line was too
>> long and the thing was not broken up so oddly.
>
> How about
>
> #define vec_unh(x) \
> (vector signed short)vec_perm(x, (__typeof__(x)) { 0 }, \
> ((vector unsigned char) { \
> 0x10, 0x00, 0x10, 0x01, \
> 0x10, 0x02, 0x10, 0x03, \
> 0x10, 0x04, 0x10, 0x05, \
> 0x10, 0x06, 0x10, 0x07 }))
That looks quite pretty.
> or
>
> #define vec_unh(x) \
> (vector signed short) \
> vec_perm(x, (__typeof__(x)) { 0 }, \
> ((vector unsigned char) { \
> 0x10, 0x00, 0x10, 0x01, 0x10, 0x02, 0x10, 0x03, \
> 0x10, 0x04, 0x10, 0x05, 0x10, 0x06, 0x10, 0x07 }))
I probably like the first one better, but this is still better than
the original and addresses my main (indenting) concern, so I can live
with this if others prefer this.
Ronald
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
