On Wed, Feb 15, 2012 at 08:39:41PM -0800, Ronald S. Bultje wrote: > On Tue, Feb 7, 2012 at 7:08 AM, Diego Biurrun <[email protected]> wrote: > > --- > > > > Now with bells and whistles like compilation testing and without a brown > > paperbag idiocy that slipped in and I don't want to even talked about. > > > > libswscale/ppc/swscale_altivec.c | 444 ++++++++-------- > > libswscale/ppc/yuv2rgb_altivec.c | 1145 > > ++++++++++++++++++++------------------ > > libswscale/ppc/yuv2rgb_altivec.h | 22 +- > > libswscale/ppc/yuv2yuv_altivec.c | 173 +++--- > > 4 files changed, 914 insertions(+), 870 deletions(-) > > You know, this is __MASSIVE__, I really can't review this. It's simply too > much. > > I've reviewed swscale_altivec.c and that looks fine, you can commit that. > > yuv2rgb_altivec.c I'm not quite happy with, especially the way you > format the macros becomes quite unreadable (check e.g. vec_packclp()),
I saw no better way to format that with all the casts while keeping line length below 80 characters... > but also stuff like ... > > > typedef unsigned char ubyte; > > -typedef signed char sbyte; > > - > > +typedef signed char sbyte; > > ... makes me almost wonder if this is script-generated. Of course it is, what do you think? It's uncrustify with my tuned configuration file and some manual improvements on top of that. > Don't get me wrong, most of it looks OK, but I don't think it's > perfect yet. I'd like that to be split in its own patch so I can > review that separately. Coming up in a moment ... > The changes to yuv2yuv_altivec.h are weird, why is that a net benefit? > > > -#define YUV2PACKEDX_HEADER(suffix) \ > > - void ff_yuv2 ## suffix ## _X_altivec(SwsContext *c, \ > [..] > > +#define YUV2PACKEDX_HEADER(suffix) \ > > + void ff_yuv2 ## suffix ## _X_altivec(SwsContext *c, \ That's the standard \-placement position. It's not important, I'll just drop it. Diego _______________________________________________ libav-devel mailing list [email protected] https://lists.libav.org/mailman/listinfo/libav-devel
