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 }))
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 }))
?
> As for the cast, maybe add a T typedef as above to make it shorter?
Hmm, I'm not overly fond of that idea yet.
Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
