On Tue, Nov 01, 2016 at 09:37:50AM +0100, Alexandra Hájková wrote: > From: Alexandra Hajkova <alexandra.khirn...@gmail.com>
Your name lost some accents here.. > --- > libavcodec/hevcdsp.c | 2 + > libavcodec/hevcdsp.h | 1 + > libavcodec/ppc/Makefile | 1 + > libavcodec/ppc/hevcdsp_ppc.c | 111 > ++++++++++++++++++++++++++++++++++++++ > libavcodec/ppc/hevcdsp_template.c | 52 ++++++++++++++++++ > 5 files changed, 167 insertions(+) > create mode 100644 libavcodec/ppc/hevcdsp_ppc.c > create mode 100644 libavcodec/ppc/hevcdsp_template.c > > --- a/libavcodec/hevcdsp.c > +++ b/libavcodec/hevcdsp.c > @@ -247,4 +247,6 @@ void ff_hevc_dsp_init(HEVCDSPContext *hevcdsp, int > bit_depth) > > if (ARCH_X86) > ff_hevc_dsp_init_x86(hevcdsp, bit_depth); > + if (ARCH_PPC) > + ff_hevc_dsp_init_altivec(hevcdsp, bit_depth); order Look at how other such init functions are named. > --- a/libavcodec/hevcdsp.h > +++ b/libavcodec/hevcdsp.h > @@ -116,6 +116,7 @@ typedef struct HEVCDSPContext { > void ff_hevc_dsp_init(HEVCDSPContext *hpc, int bit_depth); > > void ff_hevc_dsp_init_x86(HEVCDSPContext *c, const int bit_depth); > +void ff_hevc_dsp_init_altivec(HEVCDSPContext *c, const int bit_depth); order > --- /dev/null > +++ b/libavcodec/ppc/hevcdsp_ppc.c > @@ -0,0 +1,111 @@ > + > +/* > + * Copyright (c) Alexandra Hajkova stray empty line > +static const vector int16_t trans4[4] = { > + { 64, 64, 64, 64, 64, 64, 64, 64 }, > + { 83, 36, 83, 36, 83, 36, 83, 36 }, > + { 64, -64, 64, -64, 64, -64, 64, -64 }, > + { 36, -83, 36, -83, 36, -83, 36, -83 }, > +}; This fits in int8_t, is there a reason to have it int16_t? > +static const vec_u8 mask[2] = { > + { 0x00, 0x01, 0x08, 0x09, 0x10, 0x11, 0x18, 0x19, 0x02, 0x03, 0x0A, > 0x0B, 0x12, 0x13, 0x1A, 0x1B }, > + { 0x04, 0x05, 0x0C, 0x0D, 0x14, 0x15, 0x1C, 0x1D, 0x06, 0x07, 0x0E, > 0x0F, 0x16, 0x17, 0x1E, 0x1F }, > +}; Where do these tables come from? I would expect them to be shared across arches. > +#if HAVE_ALTIVEC > +static void transform4x4(vector int16_t src_01, vector int16_t src_23, > + vector int32_t res[4], const int shift, int16_t > *coeffs) long line > + // if is not used by the other transform This sentence lacks a proper subject and is therefore confusing. > +av_cold void ff_hevc_dsp_init_altivec(HEVCDSPContext *c, const int bit_depth) > +{ > +#undef FUNC > +#define FUNC(a, bit_depth) a ## _ ## bit_depth This macro seems pretty pointless. It adds one level of indirection and makes the expression longer. > +#if HAVE_ALTIVEC What about endianness? > --- /dev/null > +++ b/libavcodec/ppc/hevcdsp_template.c > @@ -0,0 +1,52 @@ > + > +#include "libavcodec/bit_depth_template.c" > + > +#if HAVE_ALTIVEC Moving this ifdef out of the template seems simpler and is how the other template files are handled. > +static void FUNC(ff_hevc_idct_4x4)(int16_t *coeffs, int col_limit) > +{ > + const int shift = 7; > + const int shift2 = 20 - BIT_DEPTH; > + vector int16_t src_01, src_23; > + vector int32_t res[4]; > + vector int16_t res_packed[2]; > + > + src_01 = vec_ld(0, (short *) coeffs); > + src_23 = vec_ld(16, (short *) coeffs); Is the cast required? Diego _______________________________________________ libav-devel mailing list libav-devel@libav.org https://lists.libav.org/mailman/listinfo/libav-devel