[FFmpeg-devel] [PATCH] Coremake support - ffmpeg_nommx.patch (1/1) - ffmpeg-nommx.patch (1/1)
Michael Niedermayer
michaelni
Mon May 21 19:13:40 CEST 2007
Hi
On Mon, May 21, 2007 at 09:40:19AM -0400, Ronald S. Bultje wrote:
> Hi,
>
> In article <20070520202630.GU16391 at MichaelsNB>,
> Michael Niedermayer <michaelni at gmx.at> wrote:
> > On Sun, May 20, 2007 at 02:05:44PM -0400, Ronald S. Bultje wrote:
> > [...]
> > > 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 :)
>
> Ok, how?
so that no 2 unrelated changes are in 1 patch, emms->emms_c and
HAVE_GPL for example are unrelated
> I can submit several parts for the emms->emms_c, HAVE_GPL
> changes, those are very small, then the rest of the HAVE_MMX stuff and
> then API changes in various tests?
>
> I took your comments for dct-test.c into account (less changes and not
> for tables / function declarations atop of the file). For the rest:
>
> > > 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
>
> In file included from dct-test.c:33:
> dsputil.h: In function 'copy_block2':
> dsputil.h:688: warning: implicit declaration of function 'ST16'
> dsputil.h:688: warning: implicit declaration of function 'LD16'
> dsputil.h: In function 'copy_block4':
> dsputil.h:699: warning: implicit declaration of function 'ST32'
> dsputil.h:699: warning: implicit declaration of function 'LD32'
> dct-test.c: In function 'idct248_ref':
> dct-test.c:333: warning: implicit declaration of function 'sqrt'
> dct-test.c:333: warning: incompatible implicit declaration of built-in
> function 'sqrt'
> dct-test.c:334: warning: implicit declaration of function 'cos'
> dct-test.c:334: warning: incompatible implicit declaration of built-in
> function 'cos'
> dct-test.c:334: error: 'M_PI' undeclared (first use in this function)
> dct-test.c:334: error: (Each undeclared identifier is reported only once
> dct-test.c:334: error: for each function it appears in.)
> dct-test.c:342: warning: incompatible implicit declaration of built-in
> function 'sqrt'
> dct-test.c:343: warning: incompatible implicit declaration of built-in
> function 'cos'
> dct-test.c:350: warning: incompatible implicit declaration of built-in
> function 'sqrt'
> dct-test.c:393: warning: implicit declaration of function 'rint'
> dct-test.c:393: warning: incompatible implicit declaration of built-in
> function 'rint'
>
> I can work around this by adding an include for math.h.
>
> In file included from fft-test.c:26:
> dsputil.h: In function 'copy_block2':
> dsputil.h:688: warning: implicit declaration of function 'ST16'
> dsputil.h:688: warning: implicit declaration of function 'LD16'
> dsputil.h: In function 'copy_block4':
> dsputil.h:699: warning: implicit declaration of function 'ST32'
> dsputil.h:699: warning: implicit declaration of function 'LD32'
> fft-test.c: In function 'frandom':
> fft-test.c:129: warning: implicit declaration of function 'random'
> fft-test.c: In function 'help':
> fft-test.c:161: warning: implicit declaration of function 'exit'
> fft-test.c:161: warning: incompatible implicit declaration of built-in
> function 'exit'
> fft-test.c: In function 'main':
> fft-test.c:198: warning: implicit declaration of function 'atoi'
> fft-test.c:249: warning: implicit declaration of function 'memcpy'
> fft-test.c:249: warning: incompatible implicit declaration of built-in
> function 'memcpy'
> fft-test.c:276: warning: incompatible implicit declaration of built-in
> function 'memcpy'
>
> I can work around this by adding includes for stdlib.h and string.h to
> fft-test.c.
>
> apiexample / motion_test give the dsputil warnings for ST/LD16/32 but
> compile fine otherwise. I don't really know how to prevent the warnings,
> since dsputil.h is clearly a file that is internal and supposed to be
> used only internally (or well, such is my impression). So the fact that
> it used internal API seems normal. You probably don't want to obfuscate
> it with all sort of #ifdef HAVE_AV_CONFIG_H, do you?
no
> (I mean, I can just
> protect all copy_blockx() functions with it to make the warnings go
> away, but that's not a good idea.)
>
> libswscale's cs_test compiles fine without, so removed that also. For
> all of the above, I could remove it.
the more of the test utils work without HAVE_AV_CONFIG_H the better
messing with the internal API is never a good idea if it can be
avoided, they also would serve as a bad example on how to use lavc
>
> dsptest in tests/ and cputest in lavc/i386/ can't be compiled, since
> they use internaly API (dsptest is about ten pages of warnings, so
> nevermind there), cputest is below:
>
> cputest.c: In function 'mm_support':
> cputest.c:83: error: 'MM_MMX' undeclared (first use in this function)
> cputest.c:83: error: (Each undeclared identifier is reported only once
> cputest.c:83: error: for each function it appears in.)
> cputest.c:85: error: 'MM_MMXEXT' undeclared (first use in this function)
> cputest.c:85: error: 'MM_SSE' undeclared (first use in this function)
> cputest.c:87: error: 'MM_SSE2' undeclared (first use in this function)
> cputest.c:89: error: 'MM_SSE3' undeclared (first use in this function)
> cputest.c:91: error: 'MM_SSSE3' undeclared (first use in this function)
> cputest.c:99: error: 'MM_3DNOW' undeclared (first use in this function)
> cputest.c:101: error: 'MM_3DNOWEXT' undeclared (first use in this
> function)
this should be useing FF_MM_MMX and similar
> cputest.c: In function 'main':
> cputest.c:128: warning: implicit declaration of function 'printf'
> cputest.c:128: warning: incompatible implicit declaration of built-in
> function 'printf'
>
> I left those as-is.
>
> > > 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?
>
> dsptest.c includes several .c files, each of which has these macros
> without undef'ing them the first time, i.e. assuming they're yet
> undeclared. This means if you include two such files, the second causes
> warnings because of redeclarations of all of those variables. undef'ing
> them at the end of the source file seems the right thing to do, at least
> in one of them.
i disagree, the fact that dsptest.c #includes several random .c files
is the problem, hacking the c files to make that possible is not a good
solution
i guess i wouldnt be opposed to svn remove dsptest.c ...
[...]
> For reference, new patch attached. If you like it, I'll split it in
> whatever way you prefer and submit parts.
i like it in principle but the individual changes need to be reviewed
and ill do that after you submitted them in a splitted fashion as its
much easier
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin
-------------- 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/20070521/0a55eaa6/attachment.pgp>
More information about the ffmpeg-devel
mailing list