Re: [FFmpeg-devel] [PATCH] intreadwrite: Altivec implementations of AV_COPY128 and AV_ZERO128.
On Sun, Mar 08, 2015 at 09:59:21PM +0100, Reimar Döffinger wrote: On Sun, Mar 08, 2015 at 08:44:50PM +0100, Michael Niedermayer wrote: On Sun, Mar 08, 2015 at 07:43:29PM +0100, Reimar Döffinger wrote: On 08.03.2015, at 19:34, Michael Niedermayer michae...@gmx.at wrote: On Sun, Mar 08, 2015 at 05:16:43PM +0100, Reimar Döffinger wrote: Slightly (ca. 4%?) faster and smaller ff_h264_decode_mb_cavlc in my tests on a G4 7450. Signed-off-by: Reimar Döffinger reimar.doeffin...@gmx.de breaks build: CC libavcodec/aliaspixdec.o libavcodec/aliaspixdec.c: In function ‘decode_frame’: libavcodec/aliaspixdec.c:35:75: error: expected identifier or ‘(’ before ‘unsigned’ What compiler/os/... is this? I only tried on Linux, not OSX for example. Though maybe I just messed up the merge, sorry my PPC machine doesn't have email. gcc 4.6 (Debian 4.6.3-14) the problem is the pixel identifer, renaming it make it build the affected file Hm, I don't have that issue with 4.9. I guess something's broken with the altivec header? Looking at the 4.9 version a #undef pixel right after the altivec.h include might fix it? Assuming we don't actually use it anywhere as an altivec keyword. your patch with #undef pixel added after the include: CC libavcodec/dss_sp.o libavcodec/dss_sp.c: In function ‘dss_sp_gen_exc’: libavcodec/dss_sp.c:473:1: error: parameter name omitted libavcodec/dss_sp.c:482:13: error: expected expression before ‘__attribute__’ libavcodec/dss_sp.c:485:13: error: expected expression before ‘__attribute__’ libavcodec/dss_sp.c:488:26: error: expected expression before ‘__attribute__’ libavcodec/dss_sp.c:489:43: error: expected identifier or ‘(’ before ‘[’ token libavcodec/dss_sp.c:488:13: warning: unused variable ‘tmp’ [-Wunused-variable] libavcodec/dss_sp.c: In function ‘dss_sp_update_buf’: libavcodec/dss_sp.c:505:1: error: parameter name omitted libavcodec/dss_sp.c:510:9: error: expected expression before ‘__attribute__’ libavcodec/dss_sp.c:513:9: error: expected expression before ‘__attribute__’ libavcodec/dss_sp.c: In function ‘dss_sp_decode_one_frame’: libavcodec/dss_sp.c:720:24: warning: passing argument 1 of ‘dss_sp_gen_exc’ from incompatible pointer type [enabled by default] libavcodec/dss_sp.c:473:13: note: expected ‘__vector(4) int *’ but argument is of type ‘int32_t *’ libavcodec/dss_sp.c:724:9: warning: passing argument 2 of ‘dss_sp_update_buf’ from incompatible pointer type [enabled by default] libavcodec/dss_sp.c:505:13: note: expected ‘__vector(4) int *’ but argument is of type ‘int32_t *’ make: *** [libavcodec/dss_sp.o] Error 1 CC libavcodec/eatgv.o libavcodec/eatgv.c: In function ‘tgv_decode_inter’: libavcodec/eatgv.c:226:61: error: expected identifier or ‘(’ before ‘=’ token libavcodec/eatgv.c:230:17: error: expected expression before ‘__attribute__’ libavcodec/eatgv.c:231:49: error: expected expression before ‘__attribute__’ libavcodec/eatgv.c:232:49: error: expected expression before ‘__attribute__’ libavcodec/eatgv.c:243:30: error: expected expression before ‘__attribute__’ make: *** [libavcodec/eatgv.o] Error 1 CC libavcodec/g723_1.o libavcodec/g723_1.c: In function ‘scale_vector’: libavcodec/g723_1.c:249:1: error: parameter name omitted libavcodec/g723_1.c:255:57: error: expected expression before ‘=’ token libavcodec/g723_1.c:255:57: warning: type defaults to ‘int’ in type name [enabled by default] libavcodec/g723_1.c:255:57: error: variable length array is used [-Werror=vla] libavcodec/g723_1.c:261:18: error: expected expression before ‘__attribute__’ libavcodec/g723_1.c: In function ‘gen_dirac_train’: libavcodec/g723_1.c:445:47: error: expected identifier or ‘(’ before ‘[’ token libavcodec/g723_1.c:448:12: error: expected expression before ‘__attribute__’ libavcodec/g723_1.c:448:12: error: too few arguments to function ‘memcpy’ libavcodec/g723_1.c:451:27: error: expected expression before ‘__attribute__’ libavcodec/g723_1.c: In function ‘gen_fcb_excitation’: libavcodec/g723_1.c:464:1: error: parameter name omitted libavcodec/g723_1.c:469:12: error: expected expression before ‘__attribute__’ libavcodec/g723_1.c:469:12: error: too few arguments to function ‘memset’ libavcodec/g723_1.c:484:51: error: expected identifier or ‘(’ before ‘[’ token libavcodec/g723_1.c:487:51: error: expected identifier or ‘(’ before ‘[’ token libavcodec/g723_1.c:494:29: error: expected expression before ‘__attribute__’ libavcodec/g723_1.c:494:29: error: too few arguments to function ‘gen_dirac_train’ libavcodec/g723_1.c:443:13: note: declared here libavcodec/g723_1.c:504:47: error: expected identifier or ‘(’ before ‘[’ token libavcodec/g723_1.c:504:13: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement] libavcodec/g723_1.c:516:17: error: expected expression before ‘__attribute__’ libavcodec/g723_1.c:500:21: warning: variable ‘beta’ set but not used [-Wunused-but-set-variable]
Re: [FFmpeg-devel] [PATCH] intreadwrite: Altivec implementations of AV_COPY128 and AV_ZERO128.
On Sun, Mar 08, 2015 at 08:44:50PM +0100, Michael Niedermayer wrote: On Sun, Mar 08, 2015 at 07:43:29PM +0100, Reimar Döffinger wrote: On 08.03.2015, at 19:34, Michael Niedermayer michae...@gmx.at wrote: On Sun, Mar 08, 2015 at 05:16:43PM +0100, Reimar Döffinger wrote: Slightly (ca. 4%?) faster and smaller ff_h264_decode_mb_cavlc in my tests on a G4 7450. Signed-off-by: Reimar Döffinger reimar.doeffin...@gmx.de breaks build: CC libavcodec/aliaspixdec.o libavcodec/aliaspixdec.c: In function ‘decode_frame’: libavcodec/aliaspixdec.c:35:75: error: expected identifier or ‘(’ before ‘unsigned’ What compiler/os/... is this? I only tried on Linux, not OSX for example. Though maybe I just messed up the merge, sorry my PPC machine doesn't have email. gcc 4.6 (Debian 4.6.3-14) the problem is the pixel identifer, renaming it make it build the affected file Hm, I don't have that issue with 4.9. I guess something's broken with the altivec header? Looking at the 4.9 version a #undef pixel right after the altivec.h include might fix it? Assuming we don't actually use it anywhere as an altivec keyword. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] intreadwrite: Altivec implementations of AV_COPY128 and AV_ZERO128.
On Sun, Mar 08, 2015 at 05:16:43PM +0100, Reimar Döffinger wrote: Slightly (ca. 4%?) faster and smaller ff_h264_decode_mb_cavlc in my tests on a G4 7450. Signed-off-by: Reimar Döffinger reimar.doeffin...@gmx.de breaks build: CC libavcodec/aliaspixdec.o libavcodec/aliaspixdec.c: In function ‘decode_frame’: libavcodec/aliaspixdec.c:35:75: error: expected identifier or ‘(’ before ‘unsigned’ libavcodec/aliaspixdec.c:98:62: error: expected identifier or ‘(’ before ‘=’ token libavcodec/aliaspixdec.c:100:99: error: expected expression before ‘;’ token libavcodec/aliaspixdec.c:100:178: error: expected expression before ‘’ token libavcodec/aliaspixdec.c:100:260: error: expected expression before ‘’ token libavcodec/aliaspixdec.c:104:62: error: expected identifier or ‘(’ before ‘=’ token libavcodec/aliaspixdec.c:106:30: error: expected expression before ‘__attribute__’ [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB No snowflake in an avalanche ever feels responsible. -- Voltaire signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] intreadwrite: Altivec implementations of AV_COPY128 and AV_ZERO128.
On Sun, Mar 08, 2015 at 07:43:29PM +0100, Reimar Döffinger wrote: On 08.03.2015, at 19:34, Michael Niedermayer michae...@gmx.at wrote: On Sun, Mar 08, 2015 at 05:16:43PM +0100, Reimar Döffinger wrote: Slightly (ca. 4%?) faster and smaller ff_h264_decode_mb_cavlc in my tests on a G4 7450. Signed-off-by: Reimar Döffinger reimar.doeffin...@gmx.de breaks build: CC libavcodec/aliaspixdec.o libavcodec/aliaspixdec.c: In function ‘decode_frame’: libavcodec/aliaspixdec.c:35:75: error: expected identifier or ‘(’ before ‘unsigned’ What compiler/os/... is this? I only tried on Linux, not OSX for example. Though maybe I just messed up the merge, sorry my PPC machine doesn't have email. gcc 4.6 (Debian 4.6.3-14) the problem is the pixel identifer, renaming it make it build the affected file [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB It is dangerous to be right in matters on which the established authorities are wrong. -- Voltaire signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] intreadwrite: Altivec implementations of AV_COPY128 and AV_ZERO128.
On 08.03.2015, at 19:34, Michael Niedermayer michae...@gmx.at wrote: On Sun, Mar 08, 2015 at 05:16:43PM +0100, Reimar Döffinger wrote: Slightly (ca. 4%?) faster and smaller ff_h264_decode_mb_cavlc in my tests on a G4 7450. Signed-off-by: Reimar Döffinger reimar.doeffin...@gmx.de breaks build: CC libavcodec/aliaspixdec.o libavcodec/aliaspixdec.c: In function ‘decode_frame’: libavcodec/aliaspixdec.c:35:75: error: expected identifier or ‘(’ before ‘unsigned’ What compiler/os/... is this? I only tried on Linux, not OSX for example. Though maybe I just messed up the merge, sorry my PPC machine doesn't have email. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] intreadwrite: Altivec implementations of AV_COPY128 and AV_ZERO128.
On 08/03/15 1:16 PM, Reimar Döffinger wrote: Slightly (ca. 4%?) faster and smaller ff_h264_decode_mb_cavlc in my tests on a G4 7450. Signed-off-by: Reimar Döffinger reimar.doeffin...@gmx.de --- libavutil/ppc/intreadwrite.h | 10 ++ 1 file changed, 10 insertions(+) diff --git a/libavutil/ppc/intreadwrite.h b/libavutil/ppc/intreadwrite.h index 7671676..65b346e 100644 --- a/libavutil/ppc/intreadwrite.h +++ b/libavutil/ppc/intreadwrite.h @@ -24,6 +24,16 @@ #include stdint.h #include config.h +#if HAVE_ALTIVEC +#include util_altivec.h +#if HAVE_BIGENDIAN +#define AV_COPY128(d, s) vec_st(vec_ld(0, (const unsigned char *)(s)), 0, (unsigned char *)(d)) +#else +#define AV_COPY128(d, s) vec_vsx_st(vec_vsx_ld(0, (const unsigned char *)(s)), 0, (unsigned char *)(d)) +#endif +#define AV_ZERO128(d) VEC_ST(vec_splat_u8(0), 0, (unsigned char *)(d)) +#endif Why not use static av_always_inline functions, like it's done on other arches (and also for other defines in ppc)? + #if HAVE_XFORM_ASM #if HAVE_BIGENDIAN ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] intreadwrite: Altivec implementations of AV_COPY128 and AV_ZERO128.
On Sun, Mar 08, 2015 at 01:21:20PM -0300, James Almer wrote: On 08/03/15 1:16 PM, Reimar Döffinger wrote: Slightly (ca. 4%?) faster and smaller ff_h264_decode_mb_cavlc in my tests on a G4 7450. Signed-off-by: Reimar Döffinger reimar.doeffin...@gmx.de --- libavutil/ppc/intreadwrite.h | 10 ++ 1 file changed, 10 insertions(+) diff --git a/libavutil/ppc/intreadwrite.h b/libavutil/ppc/intreadwrite.h index 7671676..65b346e 100644 --- a/libavutil/ppc/intreadwrite.h +++ b/libavutil/ppc/intreadwrite.h @@ -24,6 +24,16 @@ #include stdint.h #include config.h +#if HAVE_ALTIVEC +#include util_altivec.h +#if HAVE_BIGENDIAN +#define AV_COPY128(d, s) vec_st(vec_ld(0, (const unsigned char *)(s)), 0, (unsigned char *)(d)) +#else +#define AV_COPY128(d, s) vec_vsx_st(vec_vsx_ld(0, (const unsigned char *)(s)), 0, (unsigned char *)(d)) +#endif +#define AV_ZERO128(d) VEC_ST(vec_splat_u8(0), 0, (unsigned char *)(d)) +#endif Why not use static av_always_inline functions, like it's done on other arches (and also for other defines in ppc)? Well, it would mean a define and the function itself for overall around 4 lines of code what like this is a single line. I don't have much of an opinion, I did that at first, but it just seemed fairly bloated for what it does. Also, not using a function is consistent with what the implementations in the non-arch-specific intreadwrite.h do. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] intreadwrite: Altivec implementations of AV_COPY128 and AV_ZERO128.
On 08/03/15 1:25 PM, Reimar Döffinger wrote: On Sun, Mar 08, 2015 at 01:21:20PM -0300, James Almer wrote: On 08/03/15 1:16 PM, Reimar Döffinger wrote: Slightly (ca. 4%?) faster and smaller ff_h264_decode_mb_cavlc in my tests on a G4 7450. Signed-off-by: Reimar Döffinger reimar.doeffin...@gmx.de --- libavutil/ppc/intreadwrite.h | 10 ++ 1 file changed, 10 insertions(+) diff --git a/libavutil/ppc/intreadwrite.h b/libavutil/ppc/intreadwrite.h index 7671676..65b346e 100644 --- a/libavutil/ppc/intreadwrite.h +++ b/libavutil/ppc/intreadwrite.h @@ -24,6 +24,16 @@ #include stdint.h #include config.h +#if HAVE_ALTIVEC +#include util_altivec.h +#if HAVE_BIGENDIAN +#define AV_COPY128(d, s) vec_st(vec_ld(0, (const unsigned char *)(s)), 0, (unsigned char *)(d)) +#else +#define AV_COPY128(d, s) vec_vsx_st(vec_vsx_ld(0, (const unsigned char *)(s)), 0, (unsigned char *)(d)) +#endif +#define AV_ZERO128(d) VEC_ST(vec_splat_u8(0), 0, (unsigned char *)(d)) +#endif Why not use static av_always_inline functions, like it's done on other arches (and also for other defines in ppc)? Well, it would mean a define and the function itself for overall around 4 lines of code what like this is a single line. I don't have much of an opinion, I did that at first, but it just seemed fairly bloated for what it does. Also, not using a function is consistent with what the implementations in the non-arch-specific intreadwrite.h do. Ah, for some reason i thought i saw a semicolon in one of the defines, meaning more than one line. Fair enough then. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
