[FFmpeg-devel] [PATCH] Coremake support - ffmpeg_nommx.patch (1/1)

Michael Niedermayer michaelni
Sun May 20 22:26:30 CEST 2007


Hi

On Sun, May 20, 2007 at 02:05:44PM -0400, Ronald S. Bultje wrote:
> Hi,
> 
> Diego Biurrun <diego <at> biurrun.de> writes:
> > On Mon, May 14, 2007 at 05:38:44PM -0400, Ronald S. Bultje wrote:
> > > 
> > > first, sorry for the threading, if you know of a way to fix that in  
> > > digest mode, please let me know...
> > 
> > Not use digest mode?  Try gmane if the volume of ffmpeg-devel drowns
> > you.
> 
> Let's see how this works. I tried webinterface, didn't work (too much 
> quoted text, no attachments), then tried nntp: with xnntp, didn't work 

gmane sucks :)
ive had a few problems with it posting to the git ML too, a single "hi" at
the top was enough to reject the mail as top posting ...


[...]
> Anyway, on to the relevant part of the patch for you. Attached, or if 
> that doesn't work on 
> http://people.freedesktop.org/~rbultje/ffmpeg_nommx.patch (still 
> testing...), you'll find those parts of the patch that you referenced 
> that I should submit separately. The patch does a bunch of things. First 
> of all, the gains: it allows most of the tests to be compiled (by 
> default, w/o mmx and w/o gpl). The changes that I made:
> 
> * most tests don't link to lav[ufc] and thus don't use av_log() but 
> printf(). However, for utility macros, they do include avutil.h, and 
> thus fail to compile b/c of the redefinition of av_log(). Thus, most 
> tests need a #undef printf/fprintf to compile. Similar for malloc in 
> swscale (last part of the patch).
> * several tests reference mmx/gpl code w/o checking for whether this is 
> enabled. Those parts have been marked with appropriate compile 
> conditionals.
> * as Mans suggested, emms -> emms_c
> * in dsputil.c and dsputil_mmx.c/h264dsp_mmx.c, macros with the same 
> names are used. dsptest.c in tests/ includes both of those, and thus the 
> compile will give warnings. It's probably a good idea to #undef each of 
> them or use similar names. Both already use #undefs internally several 
> times for those variables (e.g. C[0-7]), since they're reused in various 
> places with different values within the same files. I simply added 
> #undefs at the end of where they're used also, so that multiple files 
> can use the same macro names. H264_{WEIGHT,MC} same story.
> * fastmemcpy buggage, see above, remove if unwanted (I don't care if it 
> goes upstream, but I'll leave it in in my copy regardless).
> * motion_test.c and dsptest.c had various API changes and I updated it 
> for those API changes. Worksforme[tm].
> 
> It's various changes together, but all of it is needed to make the tests 
> work, hence one big patch.

split it :)

a quick review is below anyway but spliting it is needed if you want it to
be applied


[...]
> @@ -46,13 +47,17 @@
>  /* reference fdct/idct */
>  extern void fdct(DCTELEM *block);
>  extern void idct(DCTELEM *block);
> +#if defined(HAVE_MMX) && defined(CONFIG_GPL)
>  extern void ff_idct_xvid_mmx(DCTELEM *block);
>  extern void ff_idct_xvid_mmx2(DCTELEM *block);
> +#endif
>  extern void init_fdct();
>  
>  extern void j_rev_dct(DCTELEM *data);
> +#ifdef HAVE_MMX
>  extern void ff_mmx_idct(DCTELEM *data);
>  extern void ff_mmxext_idct(DCTELEM *data);
> +#endif
>  

sensless


>  extern void odivx_idct_c (short *block);
>  
> @@ -83,6 +88,7 @@
>  
>  static short idct_mmx_perm[64];
>  
> +#ifdef HAVE_MMX
>  static short idct_simple_mmx_perm[64]={
>          0x00, 0x08, 0x04, 0x09, 0x01, 0x0C, 0x05, 0x0D,
>          0x10, 0x18, 0x14, 0x19, 0x11, 0x1C, 0x15, 0x1D,
> @@ -93,6 +99,7 @@
>          0x22, 0x2A, 0x26, 0x2B, 0x23, 0x2E, 0x27, 0x2F,
>          0x32, 0x3A, 0x36, 0x3B, 0x33, 0x3E, 0x37, 0x3F,
>  };
> +#endif

this too is senseless, its not as if 128 byte would matter for a test
which isnt compiled or used during normal use


>  
>  void idct_mmx_init(void)
>  {
> @@ -162,14 +169,19 @@
>          for(i=0; i<64; i++)
>              block_org[i]= block1[i];
>  
> +#ifdef HAVE_MMX
>          if (fdct_func == ff_mmx_idct ||
>              fdct_func == j_rev_dct || fdct_func == ff_mmxext_idct) {
> +#else
> +	if (fdct_func == j_rev_dct) {
> +#endif

tab
and
finding a solution which doesnt spice up all the code with #ifdefs would
be highly prefered, that is
#define ff_mmx_idct NULL ifn MMX
or some totally different way of checking the type


[...]
> @@ -508,18 +526,26 @@
>              dct_error("REF-DBL", 0, fdct, fdct, test); /* only to verify code ! */
>              dct_error("IJG-AAN-INT", 0, fdct_ifast, fdct, test);
>              dct_error("IJG-LLM-INT", 0, ff_jpeg_fdct_islow, fdct, test);
> +#ifdef HAVE_MMX
>              dct_error("MMX", 0, ff_fdct_mmx, fdct, test);
>              dct_error("MMX2", 0, ff_fdct_mmx2, fdct, test);
> +#endif
>              dct_error("FAAN", 0, ff_faandct, fdct, test);
>          } else {
>              dct_error("REF-DBL", 1, idct, idct, test);
>              dct_error("INT", 1, j_rev_dct, idct, test);
> +#ifdef HAVE_MMX
>              dct_error("LIBMPEG2-MMX", 1, ff_mmx_idct, idct, test);
>              dct_error("LIBMPEG2-MMXEXT", 1, ff_mmxext_idct, idct, test);
> +#endif
>              dct_error("SIMPLE-C", 1, simple_idct, idct, test);
> +#ifdef HAVE_MMX
>              dct_error("SIMPLE-MMX", 1, ff_simple_idct_mmx, idct, test);
> +#ifdef CONFIG_GPL
>              dct_error("XVID-MMX", 1, ff_idct_xvid_mmx, idct, test);
>              dct_error("XVID-MMX2", 1, ff_idct_xvid_mmx2, idct, test);
> +#endif
> +#endif

sort them so as to minimize the #ifdef please


>              //        dct_error("ODIVX-C", 1, odivx_idct_c, idct);
>              //printf(" test against odivx idct\n");
>              //        dct_error("REF", 1, idct, odivx_idct_c);
> Index: ffmpeg/libavcodec/fft-test.c
> ===================================================================
> --- ffmpeg.orig/libavcodec/fft-test.c	2007-03-22 01:00:48.000000000 -0400
> +++ ffmpeg/libavcodec/fft-test.c	2007-03-22 01:20:53.000000000 -0400
> @@ -28,6 +28,9 @@
>  #include <unistd.h>
>  #include <sys/time.h>
>  
> +#undef fprintf
> +#undef printf
> +
>  int mm_flags;
>  
>  /* reference fft */

there is no *printf in fft-test.c
and ive not checked that the other undefs are needed but that must be checked
before such changes can be accpeted also teh question remains why 
HAVE_AV_CONFIG_H is defined at all for these files

[...]

> Index: ffmpeg/libavcodec/i386/fdct_mmx.c
> ===================================================================
> --- ffmpeg.orig/libavcodec/i386/fdct_mmx.c	2007-03-22 01:00:40.000000000 -0400
> +++ ffmpeg/libavcodec/i386/fdct_mmx.c	2007-05-20 12:02:36.000000000 -0400
> @@ -281,6 +281,13 @@
>  #define C6 12299
>  #define C7 6270
>  TABLE_SSE2
> +#undef C1
> +#undef C2
> +#undef C3
> +#undef C4
> +#undef C5
> +#undef C6
> +#undef C7
>  }};

these dont seem to be #defined after the undef so what is this good for?

[...]

> Index: ffmpeg/libavcodec/imgresample.c
> ===================================================================
> --- ffmpeg.orig/libavcodec/imgresample.c	2007-03-22 01:00:47.000000000 -0400
> +++ ffmpeg/libavcodec/imgresample.c	2007-03-22 01:20:53.000000000 -0400
> @@ -28,8 +28,10 @@
>  #include "swscale.h"
>  #include "dsputil.h"
>  
> -#ifdef USE_FASTMEMCPY
> -#include "libvo/fastmemcpy.h"
> +#ifdef TEST
> +#undef printf
> +#undef fprintf
> +#define av_log(p,l,...) fprintf(stderr,__VA_ARGS__)

ugly


-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Concerning the gods, I have no means of knowing whether they exist or not
or of what sort they may be, because of the obscurity of the subject, and
the brevity of human life -- Protagoras
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070520/80f9b0aa/attachment.pgp>



More information about the ffmpeg-devel mailing list