Re: [FFmpeg-devel] [PATCH] intreadwrite: Altivec implementations of AV_COPY128 and AV_ZERO128.
On Sun, Mar 08, 2015 at 10:37:53PM +0100, Michael Niedermayer wrote: > 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 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 > > > > > > > > > > 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 That seems to be due to the vector define. I don't think we can avoid that, to my knowledge using __vector instead doesn't work with some OSX compilers (plus it's kind of ugly). It would be possible to undefine them only in intreadwrite.h, but that would mean altivec code that includes it before altivec.h will break. Does anyone have a suggestion on how to solve this that isn't completely horrible? I guess a gcc version check might be an option, assuming that's what makes the difference... ___ 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 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 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 > > > > > > > > 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
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 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 > > > > > > 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 07:43:29PM +0100, Reimar Döffinger wrote: > On 08.03.2015, at 19:34, Michael Niedermayer 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 > > > > 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 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 > > 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 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 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 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 >>> --- >>> 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 >>> #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
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 > > --- > > 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 > > #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: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 > --- > 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 > #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
[FFmpeg-devel] [PATCH] intreadwrite: Altivec implementations of AV_COPY128 and AV_ZERO128.
Slightly (ca. 4%?) faster and smaller ff_h264_decode_mb_cavlc in my tests on a G4 7450. Signed-off-by: Reimar Döffinger --- 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 #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 + #if HAVE_XFORM_ASM #if HAVE_BIGENDIAN -- 2.1.4 ___ 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 12:09:32AM +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 > --- > libavutil/ppc/intreadwrite.h | 10 ++ > 1 file changed, 10 insertions(+) this does not apply cleanly [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB No human being will ever know the Truth, for even if they happen to say it by chance, they would not even known they had done so. -- Xenophanes signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] intreadwrite: Altivec implementations of AV_COPY128 and AV_ZERO128.
Slightly (ca. 4%?) faster and smaller ff_h264_decode_mb_cavlc in my tests on a G4 7450. Signed-off-by: Reimar Döffinger --- libavutil/ppc/intreadwrite.h | 10 ++ 1 file changed, 10 insertions(+) diff --git a/libavutil/ppc/intreadwrite.h b/libavutil/ppc/intreadwrite.h index 1ec349e..b68b40e 100644 --- a/libavutil/ppc/intreadwrite.h +++ b/libavutil/ppc/intreadwrite.h @@ -38,6 +38,16 @@ #endif +#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 + #if HAVE_XFORM_ASM #if HAVE_BIGENDIAN -- 2.1.4 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
