Hello, the following patch adds yasm support to the build system. In particular, this allows to use a benefit from mmx-like optimizations in libavcodec/fft.c Comments? -- Best regards, Andrew
On Thu, Nov 06, 2008 at 11:00:10AM +0300, Andrew Savchenko wrote:
the following patch adds yasm support to the build system. In particular, this allows to use a benefit from mmx-like optimizations in libavcodec/fft.c
Comments?
--- configure.orig 2008-11-06 07:06:20.000000000 +0300 +++ configure 2008-11-06 10:31:48.000000000 +0300
You don't use Subversion?
@@ -408,6 +421,7 @@ --as=ASSEMBLER assembler to build MPlayer [as] + --yasm=YASM yasm assembler to build MPlayer [yasm]
Yasm
@@ -2385,6 +2403,43 @@
+echocheck "yasm" +if test -z "$YASMFLAGS" ; then + [...] +else + _warn_CFLAGS=yes +fi + +if test $_yasm ; then + [...] +fi
inconsistent indentation, use two spaces
+ # check for mmx in order to enable FFT_MMX
MMX
+ _def_makefile_yasm='HAVE_YASM=yes'
You do not need _def_makefile_yasm...
+ test "$_mmx" == "yes" && _def_makefile_fft_mmx="CONFIG_FFT_MMX=yes"
... nor _def_makefile_fft_mmx.
@@ -7839,6 +7896,7 @@ `echo $_cpuexts | tr '[a-z] ' '[A-Z]\n' | sed 's/^/HAVE_/;s/$/=yes/'` +$_def_makefile_yasm
This belongs in the FFmpeg section.
@@ -7983,6 +8041,7 @@
# Some FFmpeg codecs depend on these. Enable them unconditionally for now. CONFIG_FFT=yes +$_def_makefile_fft_mmx
Just write the definition here, do not use a variable.
--- common.mak.orig 2008-05-12 21:50:30.000000000 +0400 +++ common.mak 2008-11-06 09:57:20.000000000 +0300 @@ -80,6 +80,12 @@
Syncing this file from FFmpeg should be done separately. Diego
Hi, On Thursday 06 November 2008 13:32, Diego Biurrun wrote: [...]
--- configure.orig 2008-11-06 07:06:20.000000000 +0300 +++ configure 2008-11-06 10:31:48.000000000 +0300
You don't use Subversion?
I do, but not svn diff. Just an old habit. Fixed.
@@ -408,6 +421,7 @@ --as=ASSEMBLER assembler to build MPlayer [as] + --yasm=YASM yasm assembler to build MPlayer [yasm]
Yasm
I suppose you mean "Yasm assembler to ...".
@@ -2385,6 +2403,43 @@
+echocheck "yasm" +if test -z "$YASMFLAGS" ; then + [...] +else + _warn_CFLAGS=yes +fi + +if test $_yasm ; then + [...] +fi
inconsistent indentation, use two spaces
Done.
+ # check for mmx in order to enable FFT_MMX
MMX
This comment will be purged due to further changes.
+ _def_makefile_yasm='HAVE_YASM=yes'
You do not need _def_makefile_yasm...
+ test "$_mmx" == "yes" && _def_makefile_fft_mmx="CONFIG_FFT_MMX=yes"
... nor _def_makefile_fft_mmx.
@@ -7839,6 +7896,7 @@ `echo $_cpuexts | tr '[a-z] ' '[A-Z]\n' | sed +$_def_makefile_yasm
This belongs in the FFmpeg section.
@@ -7983,6 +8041,7 @@
# Some FFmpeg codecs depend on these. Enable them unconditionally for now. CONFIG_FFT=yes +$_def_makefile_fft_mmx
Just write the definition here, do not use a variable.
OK
--- common.mak.orig 2008-05-12 21:50:30.000000000 +0400 +++ common.mak 2008-11-06 09:57:20.000000000 +0300 @@ -80,6 +80,12 @@
Syncing this file from FFmpeg should be done separately.
Full sync is done. (In the first version it was only partial.) Of course, this commit should be done in first place, otherwise compilation will be broken in between. -- Best regards, Andrew
On Fri, Nov 07, 2008 at 01:16:51AM +0300, Andrew Savchenko wrote:
On Thursday 06 November 2008 13:32, Diego Biurrun wrote: [...]
--- configure.orig 2008-11-06 07:06:20.000000000 +0300 +++ configure 2008-11-06 10:31:48.000000000 +0300
You don't use Subversion?
I do, but not svn diff. Just an old habit. Fixed.
Good.
@@ -408,6 +421,7 @@ --as=ASSEMBLER assembler to build MPlayer [as] + --yasm=YASM yasm assembler to build MPlayer [yasm]
Yasm
I suppose you mean "Yasm assembler to ...".
Yes.
--- common.mak.orig 2008-05-12 21:50:30.000000000 +0400 +++ common.mak 2008-11-06 09:57:20.000000000 +0300 @@ -80,6 +80,12 @@
Syncing this file from FFmpeg should be done separately.
Full sync is done. (In the first version it was only partial.) Of course, this commit should be done in first place, otherwise compilation will be broken in between.
I have committed this in the meantime, it was high time for a sync again.
--- configure (revision 27899) +++ configure (working copy) @@ -2385,7 +2403,41 @@
+echocheck "yasm" +if test -z "$YASMFLAGS" ; then + if darwin ; then + if x86_64 ; then + objformat="macho64" + else + objformat="macho" + fi
x86_64 && objformat="macho64" || objformat="macho"
@@ -7797,6 +7849,7 @@ CC = $_cc CXX = $_cc +YASM = $_yasm HOST_CC = $_host_cc INSTALL = $_install
Put it after HOST_CC.
@@ -7983,6 +8037,7 @@
# Some FFmpeg codecs depend on these. Enable them unconditionally for now. CONFIG_FFT=yes +CONFIG_FFT_MMX=$(test $_yasm && test "$_mmx" == "yes" && echo yes)
This is not the way to go. There should be no logic in this place.
@@ -8007,6 +8062,7 @@
HAVE_PTHREADS = $_pthreads HAVE_W32THREADS = $_w32threads +HAVE_YASM=$(test $_yasm && echo yes)
Ditto, set a variable that you can use here above. Diego
On 11 Nov 2008, at 15:25, Diego Biurrun wrote:
+echocheck "yasm" +if test -z "$YASMFLAGS" ; then + if darwin ; then + if x86_64 ; then + objformat="macho64" + else + objformat="macho" + fi
x86_64 && objformat="macho64" || objformat="macho"
I recently had problems with x86_32 and -f macho, so I'd suggest: x86_64 && objformat="macho64" || objformat="macho32"
Hi, On Tuesday 11 November 2008 18:25, Diego Biurrun wrote: [...]
--- configure (revision 27899) +++ configure (working copy) @@ -2385,7 +2403,41 @@
+echocheck "yasm" +if test -z "$YASMFLAGS" ; then + if darwin ; then + if x86_64 ; then + objformat="macho64" + else + objformat="macho" + fi
x86_64 && objformat="macho64" || objformat="macho"
Fixed as macho32 instead of macho as suggested by vmrsss.
@@ -7797,6 +7849,7 @@ CC = $_cc CXX = $_cc +YASM = $_yasm HOST_CC = $_host_cc INSTALL = $_install
Put it after HOST_CC.
Done.
@@ -7983,6 +8037,7 @@
# Some FFmpeg codecs depend on these. Enable them unconditionally for now. CONFIG_FFT=yes +CONFIG_FFT_MMX=$(test $_yasm && test "$_mmx" == "yes" && echo yes)
This is not the way to go. There should be no logic in this place.
@@ -8007,6 +8062,7 @@
HAVE_PTHREADS = $_pthreads HAVE_W32THREADS = $_w32threads +HAVE_YASM=$(test $_yasm && echo yes)
Ditto, set a variable that you can use here above.
I hope these issues are fixed now. P.S. Sorry for delay, my old mail server was badly broken... -- Best regards, Andrew
On Fri, Nov 14, 2008 at 01:39:30AM +0300, Andrew Savchenko wrote:
I hope these issues are fixed now.
--- configure (revision 27907) +++ configure (working copy) @@ -2385,7 +2403,39 @@
+yasm_check || _yasm="" +if test $_yasm ; then + _def_yasm='#define HAVE_YASM 1' + _have_yasm="yes" + test "$_mmx" == "yes" && _fft_mmx="yes" +fi
@@ -7983,6 +8035,7 @@
# Some FFmpeg codecs depend on these. Enable them unconditionally for now. CONFIG_FFT=yes +CONFIG_FFT_MMX=$_fft_mmx
This indirection is pointless, just use _mmx directly. Other than that, commit the patch anytime. Diego
Hi, On Sunday 16 November 2008 15:07, Diego Biurrun wrote: [...]
# Some FFmpeg codecs depend on these. Enable them unconditionally for now. CONFIG_FFT=yes +CONFIG_FFT_MMX=$_fft_mmx
This indirection is pointless, just use _mmx directly.
Fixed.
Other than that, commit the patch anytime.
Patch is applied. Thank you for review. -- Best regards, Andrew
participants (4)
-
Andrew Savchenko -
Andrew Savchenko -
Diego Biurrun -
vmrsss