Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Compile plugin on Fedora 37 #393

Closed
MagnoBooter opened this issue Jul 18, 2023 · 5 comments · Fixed by #398
Closed

Compile plugin on Fedora 37 #393

MagnoBooter opened this issue Jul 18, 2023 · 5 comments · Fixed by #398
Assignees
Labels
bug Something isn't working

Comments

@MagnoBooter
Copy link
Contributor

Describe the bug

Compilation errors on Fedora 37

To Reproduce

Steps to reproduce the behavior:

  1. Clone source
  2. Run .github/scripts/build-linux --skip-deps on Fedora 37

Expected behavior

The plugin should compile without errors.

Compile Log

Last messages before compilation stops:

[...]
[7/8] Building CXX object CMakeFiles/obs-backgroundremoval.dir/src/ort-utils/ort-session-utils.cpp.o
FAILED: CMakeFiles/obs-backgroundremoval.dir/src/ort-utils/ort-session-utils.cpp.o 
/usr/bin/clang++ -DCURL_STATICLIB -DHAVE_OBSCONFIG_H -DQT_CORE_LIB -DQT_GUI_LIB -DQT_NO_DEBUG -DQT_WIDGETS_LIB -DSIMDE_ENABLE_OPENMP -Dobs_backgroundremoval_EXPORTS -I/home/user/obs-backgroundremoval/build_x86_64 -I/home/user/obs-backgroundremoval -I/home/user/obs-backgroundremoval/build_x86_64/obs-backgroundremoval_autogen/include -I/home/user/obs-backgroundremoval/src -isystem /home/user/obs-backgroundremoval/build_x86_64/_deps/onnxruntime-src/include -isystem /home/user/obs-backgroundremoval/vendor/curl/include -isystem /usr/include/obs -isystem /usr/include/qt6/QtCore -isystem /usr/include/qt6 -isystem /usr/lib64/qt6/mkspecs/linux-g++ -isystem /usr/include/qt6/QtWidgets -isystem /usr/include/qt6/QtGui -isystem /home/user/obs-backgroundremoval/build_x86_64/OpenCV_Build-prefix/include/opencv4 -Wno-sign-conversion -O2 -g -DNDEBUG -std=c++17 -fPIC -fvisibility=hidden -fvisibility-inlines-hidden -fcolor-diagnostics -fopenmp-simd -fno-strict-aliasing -Wbool-conversion -Wcomma -Wconstant-conversion -Wdeprecated-declarations -Wempty-body -Wenum-conversion -Werror=return-type -Wextra -Wformat -Wformat-security -Wfour-char-constants -Winfinite-recursion -Wint-conversion -Wnewline-eof -Wno-conversion -Wno-float-conversion -Wno-implicit-fallthrough -Wno-missing-braces -Wno-missing-field-initializers -Wno-missing-prototypes -Wno-semicolon-before-method-body -Wno-shadow -Wno-sign-conversion -Wno-strict-prototypes -Wno-trigraphs -Wno-unknown-pragmas -Wno-unused-function -Wno-unused-label -Wnon-literal-null-conversion -Wobjc-literal-conversion -Wparentheses -Wpointer-sign -Wquoted-include-in-framework-header -Wshadow -Wshorten-64-to-32 -Wuninitialized -Wunreachable-code -Wunused-parameter -Wunused-value -Wunused-variable -Wvla -Wconversion -Wdeprecated-implementations -Wduplicate-method-match -Wfloat-conversion -Wimplicit-retain-self -Winvalid-offsetof -Wmove -Wno-c++11-extensions -Wno-exit-time-destructors -Wno-implicit-atomic-properties -Wno-objc-interface-ivars -Wno-overloaded-virtual -Wrange-loop-analysis -Wno-quoted-include-in-framework-header -Wno-comma -Wno-error=unused-command-line-argument -mmmx -msse -msse2 -Werror -MD -MT CMakeFiles/obs-backgroundremoval.dir/src/ort-utils/ort-session-utils.cpp.o -MF CMakeFiles/obs-backgroundremoval.dir/src/ort-utils/ort-session-utils.cpp.o.d -o CMakeFiles/obs-backgroundremoval.dir/src/ort-utils/ort-session-utils.cpp.o -c /home/user/obs-backgroundremoval/src/ort-utils/ort-session-utils.cpp
In file included from /home/user/obs-backgroundremoval/src/ort-utils/ort-session-utils.cpp:19:
In file included from /home/user/obs-backgroundremoval/src/ort-utils/ort-session-utils.h:6:
In file included from /home/user/obs-backgroundremoval/src/FilterData.h:6:
/home/user/obs-backgroundremoval/src/models/Model.h:60:35: error: implicit conversion changes signedness: 'const int' to 'std::vector::size_type' (aka 'unsigned long') [-Werror,-Wsign-conversion]
        std::vector<cv::Mat> channelsVec(channels);
                             ~~~~~~~~~~~ ^~~~~~~~
[...repeated "implicit conversion" errors from Model.h...]
/home/user/obs-backgroundremoval/src/models/Model.h:360:18: error: implicit conversion changes signedness: 'uint32_t' (aka 'unsigned int') to 'int' [-Werror,-Wsign-conversion]
                return cv::Mat(outputHeight, outputWidth, outputChannels,
                       ~~      ^~~~~~~~~~~~
/home/user/obs-backgroundremoval/src/ort-utils/ort-session-utils.cpp:37:43: error: implicit conversion changes signedness: 'uint32_t' (aka 'unsigned int') to 'int' [-Werror,-Wsign-conversion]
                sessionOptions.SetInterOpNumThreads(tf->numThreads);
                               ~~~~~~~~~~~~~~~~~~~~ ~~~~^~~~~~~~~~
fatal error: too many errors emitted, stopping now [-ferror-limit=]
20 errors generated.
ninja: build stopped: subcommand failed.

Desktop (please complete the following information):

  • OS: Fedora 37
  • Plugin Version: git repo - main branch
  • OBS Version: 29.1.2
  • CMake: 3.26.4
  • CLang: 15.0.7

Additional context

Why does the error complain about change in signedness, when clang++ seems to be invoked with -Wno-sign-conversion ?
I'd be happy to contribute instructions on how to compile on Fedora.. if I could just get it to compile.. 😄
Can anyone skilled in CMake or compilers help me?

Additional question:
I see the compile process downloads Curl and OpenCV, and compiles these.
Is there any particular reason why this is done?
Curl 7.85.0 and OpenCV 4.6.0 already comes with Fedora, are these versions insufficient/incompatible, is that why they need to be compiled?

@MagnoBooter MagnoBooter added the bug Something isn't working label Jul 18, 2023
@umireon umireon self-assigned this Jul 18, 2023
@umireon
Copy link
Member

umireon commented Jul 21, 2023

Thanks for reporting! I will fix this later.

I see the compile process downloads Curl and OpenCV, and compiles these.
Is there any particular reason why this is done?
Curl 7.85.0 and OpenCV 4.6.0 already comes with Fedora, are these versions insufficient/incompatible, is that why they need to be compiled?

I'm planning to link curl on the system if we are building on Linux, though OpenCV 4.6.0 is too old and we have to keep bundling it.

@MagnoBooter
Copy link
Contributor Author

MagnoBooter commented Jul 21, 2023

Nice! Looking forward to that. 🙂
Well, I now see Fedora 38 comes with OpenCV 4.7.0.
Is that sufficient? Or are you applying some kind of special patch/modification to OpenCV that requires you to still bundle it?
Maybe the build process could look for OpenCV, check the version, and only compile it if the OpenCV version in the OS is too old?

After looking further into this, I made the plugin compile on my Fedora release (37), simply by changing cmake/common/compiler_common.cmake:

--- a/cmake/common/compiler_common.cmake
+++ b/cmake/common/compiler_common.cmake
@@ -21,6 +21,7 @@ set(CMAKE_VISIBILITY_INLINES_HIDDEN TRUE)
 set(_obs_clang_c_options
     # cmake-format: sortable
     -fno-strict-aliasing
+    -fPIC
     -Wbool-conversion
     -Wcomma
     -Wconstant-conversion
@@ -67,10 +68,8 @@ set(_obs_clang_c_options
 set(_obs_clang_cxx_options
     # cmake-format: sortable
     ${_obs_clang_c_options}
-    -Wconversion
     -Wdeprecated-implementations
     -Wduplicate-method-match
-    -Wfloat-conversion
     -Wfour-char-constants
     -Wimplicit-retain-self
     -Winvalid-offsetof

Had to add the -fPIC flag or I would get this linker error:

/usr/bin/ld: libplugin-support.a(plugin-support.c.o): relocation R_X86_64_32 against `.rodata.str1.1' can not be used when making a shared object; recompile with -fPIC
/usr/bin/ld: failed to set dynamic section sizes: bad value
clang-15: error: linker command failed with exit code 1 (use -v to see invocation)
ninja: build stopped: subcommand failed.

Never mind the following, it was my bad, forgot to put the contents of the data directory into the share OBS plugin directory:

Now I see the filter in OBS, and I can add it to my webcam video source, but the background is still not removed. 😞
And OBS constantly outputs this error:
error: OpenCV(4.7.0) /home/user/obs-backgroundremoval/build_x86_64/OpenCV_Build-prefix/src/OpenCV_Build/modules/imgproc/src/stackblur.cpp:1197: error: (-215:Assertion failed) !_src.empty() in function 'stackBlur'

@umireon
Copy link
Member

umireon commented Jul 21, 2023

You can use cmake --install build_x86_64 --prefix /usr to place all the files correctly for your Fedora.

@umireon
Copy link
Member

umireon commented Jul 21, 2023

Is that sufficient? Or are you applying some kind of special patch/modification to OpenCV that requires you to still bundle it?

We cannot assume all the Linux we support has the latest OpenCV (4.7.0) so we have to continue to bundle OpenCV in our plugin.

@umireon
Copy link
Member

umireon commented Jul 21, 2023

It seems that you are using clang for building but we don't support clang on Fedora

This was referenced Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants