On Mon, Jul 16, 2012 at 03:28:03PM +0100, Måns Rullgård wrote:
> Diego Biurrun <[email protected]> writes:
>
> > On Mon, Jul 16, 2012 at 02:17:06PM +0100, Mans Rullgard wrote:
> >>
> >> --- a/libavcodec/arm/Makefile
> >> +++ b/libavcodec/arm/Makefile
> >> @@ -14,6 +14,7 @@ OBJS-$(CONFIG_FLAC_DECODER) +=
> >> arm/flacdsp_init_arm.o \
> >> OBJS-$(CONFIG_MPEGAUDIODSP) += arm/mpegaudiodsp_init_arm.o
> >> ARMV6-OBJS-$(CONFIG_MPEGAUDIODSP) += arm/mpegaudiodsp_fixed_armv6.o
> >>
> >> +OBJS-$(CONFIG_VP3DSP) += arm/vp3dsp_init_arm.o
> >> OBJS-$(CONFIG_VP5_DECODER) += arm/vp56dsp_init_arm.o
> >
> > This would fit better at the top with the other parts used by many codecs.
>
> There is no such part in that makefile.
Sorry, I confused the libavcodec/arm/ Makefile with the one in libavcodec/.
> Perhaps it could use a cleanup,
> but that's for a different patch.
I have a patch for this that I'll rebase.
> >> --- a/libavcodec/ppc/Makefile
> >> +++ b/libavcodec/ppc/Makefile
> >> @@ -1,14 +1,13 @@
> >> OBJS += ppc/dsputil_ppc.o \
> >>
> >> +OBJS-$(CONFIG_VP3DSP) += ppc/vp3dsp_altivec.o
> >> +
> >> FFT-OBJS-$(HAVE_GNU_AS) += ppc/fft_altivec_s.o
> >> ALTIVEC-OBJS-$(CONFIG_FFT) += ppc/fft_altivec.o \
> >> $(FFT-OBJS-yes)
> >> ALTIVEC-OBJS-$(CONFIG_H264DSP) += ppc/h264_altivec.o
> >> ALTIVEC-OBJS-$(CONFIG_MPEGAUDIODSP) += ppc/mpegaudiodec_altivec.o
> >> ALTIVEC-OBJS-$(CONFIG_VC1_DECODER) += ppc/vc1dsp_altivec.o
> >> -ALTIVEC-OBJS-$(CONFIG_VP3_DECODER) += ppc/vp3dsp_altivec.o
> >> -ALTIVEC-OBJS-$(CONFIG_VP5_DECODER) += ppc/vp3dsp_altivec.o
> >> -ALTIVEC-OBJS-$(CONFIG_VP6_DECODER) += ppc/vp3dsp_altivec.o
> >> ALTIVEC-OBJS-$(CONFIG_VP8_DECODER) += ppc/vp8dsp_altivec.o
> >>
> >> --- a/libavcodec/ppc/vp3dsp_altivec.c
> >> +++ b/libavcodec/ppc/vp3dsp_altivec.c
> >> @@ -18,6 +18,12 @@
> >>
> >> +#include "config.h"
> >> +#include "libavutil/cpu.h"
> >> +#include "libavcodec/vp3dsp.h"
> >> +
> >> +#if HAVE_ALTIVEC
> >> +
> >> #include "libavutil/ppc/types_altivec.h"
> >> @@ -183,3 +171,14 @@ void ff_vp3_idct_add_altivec(uint8_t *dst, int
> >> stride, DCTELEM block[64])
> >> +
> >> +#endif /* HAVE_ALTIVEC */
> >> +
> >> +void ff_vp3dsp_init_ppc(VP3DSPContext *c, int flags)
> >> +{
> >> + if (HAVE_ALTIVEC && av_get_cpu_flags() & AV_CPU_FLAG_ALTIVEC) {
> >> + c->idct_put = vp3_idct_put_altivec;
> >> + c->idct_add = vp3_idct_add_altivec;
> >> + c->idct_perm = FF_TRANSPOSE_IDCT_PERM;
> >> + }
> >> +}
> >
> > So the general strategy is to compile files containing optimizations
> > unconditionally and then have ifdefs in the files?
>
> There is no "general strategy." In this specific case, the choice is
> either adding another file just for the init function, or placing the
> altivec code under ifdef. The ARM and x86 VP3 functions are in separate
> .S/.asm files anyway, so there is only one option there.
>
> The init function has to be built unconditionally to allow calling it
> based on ARCH_FOO only.
My point is precisely to find a general strategy - there is a lot of
inline x86 asm that will need to undergo a similar set of changes.
In the VP3 case we have incongruency between x86/arm and ppc in that the
former has the code in separate files, the latter under ifdef. What we
need to decide on is how to handle such situations so that it's clear
how to proceed in the x86 inline asm case.
> > Previously we did not compile the VP3 optimizations w/o MMX
> > available...
>
> MMX? This is PPC code...
s/mmx/altivec/
> All the optimised functions are built under exactly the same conditions
> as before.
Yes, but we moved from conditional compilation to ifdefs.
Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
