Hi,

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()),
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. 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.

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,                 \

The changes to yuv2yuv_altivec.c are OK and can be committed.

Ronald
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to