gas-preprocessor icon indicating copy to clipboard operation
gas-preprocessor copied to clipboard

Fix use of Visual Studio compiler values "-w..." or "-FS"

Open AlwinEsch opened this issue 5 years ago • 6 comments

This was needed by a Kodi addon project and ffmpeg Windows ARM compile, where the flag "-FS" was needed, to become AppVeyor and Jenkins more happy and the "-w..." to prevent one warning, which was given by config.h on every compile file.

Without comes on configure:

BEGIN ./ffconf.KEHnLLrW/test.S
    1
    2   .macro m n, y:vararg=0
    3   \n: .int \y
    4   .endm
    5   m x
END ./ffconf.KEHnLLrW/test.S
gas-preprocessor.pl -arch aarch64 -as-type armasm -- armasm64.exe -I/d/Dev/Kodi64/NEW/inputstream.ffmpegdirect/build2/build/depends/include -D_ISOC99_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_USE_MATH_DEFINES -D_CRT_SECURE_NO_WARNINGS -D_CRT_NONSTDC_NO_WARNINGS -nologo -ignore 4509 -DWIN32 -D_WINDOWS -W3 -DUNICODE -D_UNICODE -DTARGET_WINDOWS -DTARGET_WINDOWS_STORE -DNOMINMAX -D_CRT_SECURE_NO_WARNINGS -D_WINSOCKAPI_ -ID:/Dev/Kodi64/NEW/inputstream.ffmpegdirect/build2/build/depends/include -ID:/Dev/Kodi64/NEW/inputstream.ffmpegdirect/build2/build/depends/include/libxml2 -FS -DWINAPI_FAMILY=WINAPI_FAMILY_APP -D_WIN32_WINNT=0x0A00 -wd4828 -mcpu=aarch64 -c -o ./ffconf.KEHnLLrW/test.o ./ffconf.KEHnLLrW/test.S
cpp.exe: error: unrecognized command line option '-wd4828'
GNU assembler not found, install/update gas-preprocessor

I'm not sure if OK, just thought that if I bring a patch into the project (https://github.com/AlwinEsch/inputstream.ffmpegdirect/tree/build-fixes/depends/common/gas-preprocessor), I could also ask and try to bring it in directly.

AlwinEsch avatar Jan 26 '20 12:01 AlwinEsch

@mstorsjo what do you think?

phunkyfish avatar Apr 17 '20 14:04 phunkyfish

In general, the change probably looks good - but I wonder why this is needed - the normal build also uses a number of -wd parameters, and they are only passed when building C sources, not when building assembly files.

What does the configure line look like, how do you pass the -wd4828 and -FS options to the build? If they would be passed via --extra-cflags, they wouldn't end up passed to the assembler at all.

mstorsjo avatar Apr 17 '20 17:04 mstorsjo

The related flags are set for here https://github.com/xbmc/inputstream.ffmpegdirect/blob/Matrix/depends/common/ffmpeg/CMakeLists.txt#L215-L235, what we use to create the ffmpeg for our Kodi addon


About the -FS was needed for here to have the multiprocess build fixed.

Here his build without value: https://jenkins.kodi.tv/blue/organizations/jenkins/xbmc%2Finputstream.ffmpegdirect/detail/PR-36/5/pipeline/307

libavfilter/af_acontrast.c: fatal error C1041: cannot open program database 'F:\builds\workspace\binary-addons\kodi-windows-x86_64-Matrix\cmake\addons\build\ffmpeg\src\ffmpeg\vc140.pdb'; if multiple CL.EXE write to the same .PDB file, please use /FS
libavfilter/aeval.c: fatal error C1041: cannot open program database 'F:\builds\workspace\binary-addons\kodi-windows-x86_64-Matrix\cmake\addons\build\ffmpeg\src\ffmpeg\vc140.pdb'; if multiple CL.EXE write to the same .PDB file, please use /FS

This with the gas-preprocessor comes then by the Windows UWP ARM build, where it bring, without patch: https://dev.azure.com/teamkodi/binary-addons/_build/results?buildId=936&view=logs&j=f1a74db1-652b-553f-2cc0-a946453fa7a5&t=07a7f212-cab5-52f8-bce6-fbf845a5d175&l=1868

   GNU assembler not found, install/update gas-preprocessor

Seems by a test build inside configure.


The other for -w without the previous needed -wd4828, was currently not reproducable. But was coming by VS where it has a problem to handle a string inside config.h (if I'm not wrong now by this line):

#define CC_IDENT "Microsoft (R) C/C++-Optimierungscompiler Version 19.16.27034 f.r x64"

The german für was there on ü wrong and has generated on every compiled file a 4828 warning, which was by ffmpeg amount of source files, much, much warning messages. On begin I seen this warning also on a test build, like Azure, where the amount of messages is limited to 5 MByte, where then goes over it.

Also can there other flags on CMAKE_C_FLAGS given from parent.

There was then on configure this, too:

   GNU assembler not found, install/update gas-preprocessor

AlwinEsch avatar Apr 19 '20 04:04 AlwinEsch

Right, thanks for the links.

After rechecking - options added via --extra-cflags do end up passed to armasm - the reason why I didn't think that happened was because the one flag I was passing to --extra-cflags, -MD, is being filtered out by configure here: https://github.com/FFmpeg/FFmpeg/blob/master/configure#L4397-L4406

One could in theory add these flags there as well, but it's probably better and more robust (and useful to more projects) to add the flags in gas-preprocessor instead.

So the patch looks fine, but development of gas-preprocessor happens over at https://github.com/ffmpeg/gas-preprocessor nowadays, and contributions should be sent to the ffmpeg-devel mailing list - once sent there I'll acknowledge and merge it.

mstorsjo avatar Apr 19 '20 17:04 mstorsjo

Thank you very much, had overlooked the fact that it is there now.

Will then bring the request there.

AlwinEsch avatar Apr 19 '20 18:04 AlwinEsch

@mstorsjo I submitted the patch on Alwin's behalf here: https://patchwork.ffmpeg.org/project/ffmpeg/patch/[email protected]/

But appears to fail on applying it in patchwork.

phunkyfish avatar Apr 20 '20 12:04 phunkyfish