#9859 closed art (worksforme)

Compilation warnings ATOMIC_VAR_INIT

Reported by: Jozef Chutka Owned by:
Priority: normal Component: undetermined
Version: unspecified Keywords:
Cc: Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description (last modified by Jozef Chutka)

While compiling ffmpeg (commit 1368b5a) with emscripten 3.1.17, there are two warnings, I thinks maintainers might want to look into:

fftools/ffmpeg.c:339:41: warning: macro 'ATOMIC_VAR_INIT' has been marked as deprecated [-Wdeprecated-pragma]
static atomic_int transcode_init_done = ATOMIC_VAR_INIT(0);
                                        ^
/home/XYZ/ffmpeg-wasm/modules/emsdk/upstream/lib/clang/15.0.0/include/stdatomic.h:50:41: note: macro marked 'deprecated' here
#pragma clang deprecated(ATOMIC_VAR_INIT)
                                        ^

To my knowledge, this macro was a part of early draft design for C11 atomic types. It is not needed in C11, and is deprecated in C17 and removed in C23.

The other warning is also interesting:

fftools/ffmpeg_filter.c:898:35: warning: floating-point comparison is always true; constant cannot be represented exactly in type 'float' [-Wliteral-range]
        if (audio_drift_threshold != 0.1)
            ~~~~~~~~~~~~~~~~~~~~~ ^  ~~~

It turns out that 0.1 is one of the numbers that it is impossible to encode in binary floating point. And according to the message the condition is as good as having if(true) in place. Consider using epsilon:

if (abs(audio_drift_threshold - 0.1) < epsilon)

SO helped me with understanding the details https://stackoverflow.com/questions/73203857/ffmpeg-compilation-warnings-atomic-var-init

Change History (13)

comment:1 by Jozef Chutka, 22 months ago

Description: modified (diff)

comment:2 by Jozef Chutka, 22 months ago

Description: modified (diff)

comment:3 by Carl Eugen Hoyos, 21 months ago

ATOMIC_VAR_INIT() is part of C11 and FFmpeg explicitely requests C11 with -std=c11
So while this ticket is missing all necessary information to reproduce (like at least the used configure line), I do not immediately see an issue.

comment:4 by Balling, 21 months ago

ATOMIC_VAR_INIT() is not needed by C11 and deprecated by C17, stop reading drafts and download normal standard on libgen. But since when do we use C11? I thought it was C90 (so all "declaration before code" applies) with only some parts C99, but should be frowned upon.

https://stackoverflow.com/questions/81656/where-do-i-find-the-current-c-or-c-standard-documents

See that C11 has one TC, where they modify constands for version, public page: https://www.iso.org/obp/ui/#iso:std:iso-iec:9899:ed-3:v1:cor:1:v1:en

Deprecation may not be obvious, since that is just two words removal in C17 standard... https://en.cppreference.com/mwiki/index.php?title=c/atomic/ATOMIC_VAR_INIT&diff=97502&oldid=77535

Last edited 21 months ago by Balling (previous) (diff)

comment:5 by Jozef Chutka, 21 months ago

Attaching emscripten configure and build commands. Its based on https://github.com/ffmpegwasm/ffmpeg.wasm-core :

emconfigure
  ./configure 
  --target-os=none
  --arch=x86_32
  --enable-cross-compile
  --enable-gpl
  --enable-version3
  --enable-zlib
  --enable-libaom
  --enable-libx264
  --enable-libx265
  --enable-libvpx
  --enable-libmp3lame
  --enable-libtheora
  --enable-libvorbis
  --enable-libopus
  --enable-libwebp
  --enable-librubberband
  --disable-x86asm
  --disable-inline-asm
  --disable-stripping
  --disable-programs
  --disable-doc
  --disable-debug
  --disable-runtime-cpudetect
  --disable-autodetect
  --extra-cflags="-O3 -I$BUILD_DIR/include -s USE_PTHREADS=1 -msimd128"
  --extra-cxxflags="-O3 -I$BUILD_DIR/include -s USE_PTHREADS=1 -msimd128"
  --extra-ldflags="-O3 -I$BUILD_DIR/include -s USE_PTHREADS=1 -msimd128"
  --pkg-config-flags="--static"
  --nm="llvm-nm"
  --ar=emar
  --ranlib=emranlib
  --cc=emcc
  --cxx=em++
  --objcc=emcc
  --dep-cc=emcc
emcc
  -O3 -I$BUILD_DIR/include -s USE_PTHREADS=1 -msimd128
  -I. -I./fftools -I$BUILD_DIR/include
  -Llibavcodec -Llibavdevice -Llibavfilter -Llibavformat -Llibavresample -Llibavutil -Llibpostproc -Llibswscale -Llibswresample -Lrubberband -Lsamplerate -Lflite -L$BUILD_DIR/lib
  -Wno-deprecated-declarations -Wno-pointer-sign -Wno-implicit-int-float-conversion -Wno-switch -Wno-parentheses -Qunused-arguments
  -lavdevice -lavfilter -lavformat -lavcodec -lswresample -lswscale -lavutil -lpostproc -lm -laom -lx264 -lx265 -lvpx -lmp3lame -lvorbis -lvorbisenc -lvorbisfile -logg -ltheora -ltheoraenc -ltheoradec -lz -lopus -lwebp -lwebpmux -lrubberband -lsamplerate
  fftools/cmdutils.c fftools/ffmpeg.c fftools/ffmpeg_filter.c fftools/ffmpeg_hw.c fftools/ffmpeg_mux.c fftools/ffmpeg_opt.c fftools/objpool.c fftools/opt_common.c fftools/sync_queue.c fftools/thread_queue.c
  -lworkerfs.js
  -s USE_SDL=2
  -s INVOKE_RUN=0
  -s EXIT_RUNTIME=1
  -s MODULARIZE=1
  -s EXPORT_NAME="createFFmpeg"
  -s EXPORTED_FUNCTIONS="[_main, ___wasm_init_memory_flag]"
  -s EXPORTED_RUNTIME_METHODS="[callMain, FS, WORKERFS]"
  -s INITIAL_MEMORY=128mb
  -s ALLOW_MEMORY_GROWTH=1
  -s MAXIMUM_MEMORY=4gb
  -s ENVIRONMENT=worker
  -s PROXY_TO_PTHREAD=1
  -pthread
  -o $WASM_DIR/ffmpeg.js

With this configuration I observe the reported issues (ATOMIC_VAR_INIT and audio_drift_threshold != 0.1). Please advise if you see something wrong in my commands

Last edited 21 months ago by Jozef Chutka (previous) (diff)

comment:6 by Balling, 21 months ago

So this happened with https://github.com/llvm/llvm-project/commit/0d459444e5105d689e6388612784b89a93054944

Yet they say DR 485 was closed, but it was not: https://www.open-std.org/jtc1/sc22/wg14/www/docs/summary.htm

It is still in review. Very shady.

Last edited 21 months ago by Balling (previous) (diff)

comment:8 by Balling, 20 months ago

One warning fixed in 686096739b129c7e3ea26be29c875e0887e58c49, the other please somehow force emscripten to use older C standard. Please verify.

Last edited 20 months ago by Carl Eugen Hoyos (previous) (diff)

comment:9 by Jozef Chutka, 20 months ago

Thanks for the fix.

In regards to C standard, there are a few numbers mentioned in this thread, which is the right one to be used?

comment:10 by Balling, 20 months ago

It is C90 with parts of C99. So it should be c99, I imagine.

comment:11 by mkver, 20 months ago

No, the atomics are only part of C11. FFmpeg has not been C90 for a long time; currently it is "C99 excluding variable-length arrays, with C11 atomics very much encouraged". (There is a fallback in case atomics are unavailable, but you should really not use it.)

comment:12 by Balling, 20 months ago

ATOMIC_VAR_INIT() is not needed by C11 though, but it says that with deleting two words.

comment:13 by Carl Eugen Hoyos, 19 months ago

Resolution: worksforme
Status: newclosed
Type: enhancementart

I cannot reproduce this with emscripten 3.1.22

Note: See TracTickets for help on using tickets.