georgmartius / vid.stab

Video stabilization library
http://public.hronopik.de/vid.stab/
Other
842 stars 108 forks source link

1.1.0 breaks compilation under clang #65

Open mqudsi opened 5 years ago

mqudsi commented 5 years ago
mqudsi@Blitzkrieg /m/d/G/FFmpeg> ./configure --enable-libvidstab --enable-gpl
ERROR: vidstab >= 0.98 not found using pkg-config

If you think configure made a mistake, make sure you are using the latest
version from Git.  If the latest version fails, report the problem to the
ffmpeg-user@ffmpeg.org mailing list or IRC #ffmpeg on irc.freenode.net.
Include the log file "ffbuild/config.log" produced by configure as this will help
solve the problem.
mqudsi@Blitzkrieg /m/d/G/FFmpeg> pkg-config --cflags --libs "vidstab >= 0.98"
-I/usr/local/include -L/usr/local/lib -lvidstab -lm -lgomp -lpthread

ffmpeg's configure script tries to compile the following, which fails:

#include <vid.stab/libvidstab.h>
#include <stdint.h>
long check_vsMotionDetectInit(void) { return (long) vsMotionDetectInit; }
int main(void) { int ret = 0;
ret |= ((intptr_t)check_vsMotionDetectInit) & 0xFFFF;
return ret; }

compiling it myself:

mqudsi@Blitzkrieg /m/d/G/FFmpeg> clang (pkg-config --cflags --libs vidstab | string split ' ') ./test.c -o test
/usr/local/lib/libvidstab.so: undefined reference to `__kmpc_end_critical'
/usr/local/lib/libvidstab.so: undefined reference to `__kmpc_for_static_fini'
/usr/local/lib/libvidstab.so: undefined reference to `__kmpc_fork_call'
/usr/local/lib/libvidstab.so: undefined reference to `__kmpc_for_static_init_4'
/usr/local/lib/libvidstab.so: undefined reference to `__kmpc_critical'
clang: error: linker command failed with exit code 1 (use -v to see invocation)

Corrected by modifying vidstab.pc to include -fopenmp=libomp:

# file generated by vid.stab cmake build
prefix=/usr/local
libdir=/usr/local/lib
includedir=/usr/local/include

Name: vidstab
Description: Vid.Stab, a library for stabilizing video clips
Version: 1.10
Libs: -L${libdir} -lvidstab -lm -lgomp -lpthread -fopenmp=libomp
Cflags: -I${includedir}

I'm not familiar enough with OpenMP to determine if this is safe to add universally.

georgmartius commented 5 years ago

Hi, I think openmp should be fairly standard. I think you can safely add it universally.

mqudsi commented 5 years ago

Sure, but should the particular openmp library (=libomp) be hardcoded?

georgmartius commented 5 years ago

I don't know a better way. Not sure whether the dependencies in the packages have to be updated as well.

museum-future commented 4 years ago

I have this issue as well, when using clang > 7.0 you don't need to specify -lgomp

in the CmakeLists.txt change

if(USE_OMP)
  add_definitions(-fopenmp -DUSE_OMP)
endif()

to

if(USE_OMP)
  find_package(OpenMP)
  if(OpenMP_CXX_FOUND)
    target_link_libraries(OpenMP::OpenMP_CXX)
  endif()
endif()

and this

if(USE_OMP)
  target_link_libraries(vidstab gomp)
  set(PKG_EXTRA_LIBS "${PKG_EXTRA_LIBS} -lgomp -lpthread")
endif()

to

if(USE_OMP)
  target_link_libraries(vidstab)
  set(PKG_EXTRA_LIBS "${PKG_EXTRA_LIBS} -lpthread")
endif()
charles1018 commented 4 years ago

I have another way to solve this problem add CFLAGS to -fopenmp my build environment: linux mint Linux Mint 19.3 LLVM 9.0.0 Pre-Built Binaries: