LGTM in general, mostly minor comments below.
On Wed, Apr 10, 2013 at 07:24:31PM +0300, Martin Storsjö wrote:
> --- a/libavcodec/hpeldsp.c
> +++ b/libavcodec/hpeldsp.c
> @@ -54,5 +54,6 @@ av_cold void ff_hpeldsp_init(HpelDSPContext *c, int flags)
> hpel_funcs(avg, [3], 2);
> hpel_funcs(avg_no_rnd,, 16);
>
> + if (ARCH_PPC) ff_hpeldsp_init_ppc (c, flags);
> if (ARCH_X86) ff_hpeldsp_init_x86 (c, flags);
Break the line and eliminate the odd spacing please.
> --- a/libavcodec/hpeldsp.h
> +++ b/libavcodec/hpeldsp.h
> @@ -94,6 +94,7 @@ typedef struct HpelDSPContext {
>
> void ff_hpeldsp_init(HpelDSPContext *c, int flags);
>
> +void ff_hpeldsp_init_ppc(HpelDSPContext* c, int flags);
> void ff_hpeldsp_init_x86(HpelDSPContext* c, int flags);
*c
> --- a/libavcodec/ppc/dsputil_altivec.c
> +++ b/libavcodec/ppc/dsputil_altivec.c
> @@ -1367,16 +956,6 @@ av_cold void ff_dsputil_init_altivec(DSPContext *c,
> AVCodecContext *avctx)
> if (!high_bit_depth) {
> c->get_pixels = get_pixels_altivec;
> c->clear_block = clear_block_altivec;
> - c->put_pixels_tab[0][0] = ff_put_pixels16_altivec;
> - /* the two functions do the same thing, so use the same code */
> - c->put_no_rnd_pixels_tab[0][0] = ff_put_pixels16_altivec;
> - c->avg_pixels_tab[0][0] = ff_avg_pixels16_altivec;
> - c->avg_pixels_tab[1][0] = avg_pixels8_altivec;
> - c->avg_pixels_tab[1][3] = avg_pixels8_xy2_altivec;
> - c->put_pixels_tab[1][3] = put_pixels8_xy2_altivec;
> - c->put_no_rnd_pixels_tab[1][3] = put_no_rnd_pixels8_xy2_altivec;
> - c->put_pixels_tab[0][3] = put_pixels16_xy2_altivec;
> - c->put_no_rnd_pixels_tab[0][3] = put_no_rnd_pixels16_xy2_altivec;
The comment is dropped; not sure if we need to care.
> --- /dev/null
> +++ b/libavcodec/ppc/hpeldsp_altivec.c
> @@ -0,0 +1,464 @@
> +
> +void ff_hpeldsp_init_ppc(HpelDSPContext* c, int flags)
*c
> +{
> +#if HAVE_ALTIVEC
> + int mm_flags = av_get_cpu_flags();
> +
> + if (mm_flags & AV_CPU_FLAG_ALTIVEC) {
> + c->put_pixels_tab[0][0] = ff_put_pixels16_altivec;
> + c->put_no_rnd_pixels_tab[0][0] = ff_put_pixels16_altivec;
> + c->avg_pixels_tab[0][0] = ff_avg_pixels16_altivec;
> + c->avg_pixels_tab[1][0] = avg_pixels8_altivec;
> + c->avg_pixels_tab[1][3] = avg_pixels8_xy2_altivec;
> + c->put_pixels_tab[1][3] = put_pixels8_xy2_altivec;
> + c->put_no_rnd_pixels_tab[1][3] = put_no_rnd_pixels8_xy2_altivec;
> + c->put_pixels_tab[0][3] = put_pixels16_xy2_altivec;
> + c->put_no_rnd_pixels_tab[0][3] = put_no_rnd_pixels16_xy2_altivec;
I think the following would be more readable, with or without the
empty lines:
c->avg_pixels_tab[0][0] = ff_avg_pixels16_altivec;
c->avg_pixels_tab[1][0] = avg_pixels8_altivec;
c->avg_pixels_tab[1][3] = avg_pixels8_xy2_altivec;
c->put_pixels_tab[0][0] = ff_put_pixels16_altivec;
c->put_pixels_tab[0][3] = put_pixels16_xy2_altivec;
c->put_pixels_tab[1][3] = put_pixels8_xy2_altivec;
c->put_no_rnd_pixels_tab[0][0] = ff_put_pixels16_altivec;
c->put_no_rnd_pixels_tab[0][3] = put_no_rnd_pixels16_xy2_altivec;
c->put_no_rnd_pixels_tab[1][3] = put_no_rnd_pixels8_xy2_altivec;
Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
