[FFmpeg-devel] [PATCH] compilation error in libavcodec/x86/mpegaudiodsp.c

[Matt Oliver]

On 6 May 2014 04:31, Derek Buitenhuis <derek.buitenhuis at gmail.com> wrote:

> On 5/5/2014 12:42 PM, Paweł Witas wrote:
> > +#if HAVE_AVX_EXTERNAL
> >       if (EXTERNAL_AVX(cpu_flags)) {
> >           s->imdct36_blocks_float = imdct36_blocks_avx;
> >       }
> > +#endif
>
> I am incline not to accept this. This is a hack to fix improper DCE.
>
> > I use MinGW/MSys tools.
> > My ffmpeg compilation configuration for Pentium III and above, that
> causes this error:
> >
> > ./configure --toolchain=msvc --prefix=/usr/local \
> >   --extra-cflags="-arch:SSE -O2 -MT -LD -I/usr/local/include
> -D_BIND_TO_CURRENT_CRT_VERSION=1 -D_BIND_TO_CURRENT_MFC_VERSION=1" \
> >   --arch=x86 --cpu=c3-2 --disable-runtime-cpudetect --disable-sse2
> --disable-sse3 --disable-ssse3 --disable-sse4 --disable-sse42 \
> >   --disable-amd3dnow --disable-amd3dnowext --disable-avx --disable-avx2
> --disable-xop --disable-fma3 --disable-fma4 \
> >   --disable-gpl --disable-debug --enable-shared --enable-swscale \
> >   --disable-decoder=aac --disable-encoder=aac --enable-decoder=aac_latm \
> >   --enable-libfdk-aac --disable-libaacplus --enable-nonfree
>
> Let's break this down, as it seems you do not know what you are doing.
>
> > --extra-cflags="-arch:SSE -O2 -MT -LD -I/usr/local/include
> -D_BIND_TO_CURRENT_CRT_VERSION=1 -D_BIND_TO_CURRENT_MFC_VERSION=1"
>
> This makes no sense. Also, please try a build without all the custom flags.
>
> In order:
>
> > -arch:SSE
>
> All recent MSVC compiles default to SSE (SSE2 even I think). This is
> superfluous.
>
> > -O2 -MT
>
> All of these are default for FFmpeg. Why are you specifying them twice?
>
> > -LD
>
> Incorrect. If you want DLLs, use --enable-shared alone.
>
> > -I/usr/local/include
>
> Very wrong. Do NOT pass MSVC your *MinGW* includes folder.
>
> >  -D_BIND_TO_CURRENT_CRT_VERSION=1 -D_BIND_TO_CURRENT_MFC_VERSION=1
>
> Why?
>
> > --cpu=c3-2
>
> Welcome to 15 years ago I guess.
>
> > --disable-runtime-cpudetect
>
> This is the exact opposite of what you want. You *want* it to see what the
> CPU
> can do at runtime, and in fact this is pretty much the only way we support
> nowadays.
>
> > --disable-sse2 --disable-sse3 --disable-ssse3 --disable-sse4
> --disable-sse42 \
> >   --disable-amd3dnow --disable-amd3dnowext --disable-avx --disable-avx2
> --disable-xop --disable-fma3 --disable-fma4
>
> Do not do this. You are crippling the build. The binary will run perfectly
> fine on a P3,
> since we detect the CPU capabilities at runtime.
>
> > --disable-gpl
>
> Pointless. LGPL is default.
>
> > --enable-swscale
>
> Pointless. It is enabled by default.
>
> > --enable-decoder=aac_latm
>
> Ditto.
>
> > --disable-libaacplus
>
> Pointless, as you have to explicitly enable this.
>
> After trying a build with default settings, please tell us your compiler
> version.
>
> - Derek
>

I agree with Derek that your configure options could do with some serious
cleanups. That said however based on the error given I was able to
reproduce the problem using nothing more than "--disable-avx" which causes
compilation errors for msvc and icl. It appears msvc will generate the
error before the dead code elimination stage (gcc is smart enough to just
give a warning).
The same issue will arise if compiling with --disable-sse as well. So i
added to the patch to handle this possible situation as well. I wouldnt
recommend anyone compile without sse but then I wouldnt compile without avx
either and this patch report is evidence that some people do. So ive made
an updated patch that fixes both situations just in case.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: x86-mpegaudiodsp.c-msvc-compilation-error-without-ss.patch
Type: application/octet-stream
Size: 1087 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140506/ef14276c/attachment.obj>