Refactor C++ wrappers for libOBS #806
Conversation
Xaymar
force-pushed
the
refactor/obs
branch
6 times, most recently
from
b08bdae
to
336c758
Compare
Xaymar
force-pushed
the
refactor/obs
branch
7 times, most recently
from
db309e1
to
61848a6
Compare
- Moved all auto-dependencies to a uniform subdirectory for easier caching and cleanup. - Add an option to download or specify a path for libOBS+obs-frontend-api. - Remove the dependency on the non-standard obs-frontend-apiConfig.cmake file. - Add an option to download or specify a path for OBS Dependencies. - Add an option to download or specify a path for Qt. - Add an option to download or specify a path for AOM. - Fix and improve architecture and platform detection. - Fix some messages having two :, or no prefix at all. - Fix detection of obs-frontend-api. - Fix applying custom compiler and linker flags for MSVC and GNU-style builds. - Use target_compile_options over add_compile_options for compatibility.
The bug fix to this was applied upstream, so we no longer need to support the broken behavior
In some rare cases, a bug is observed where some sources end up missing despite being visible in the OBS Studio UI. This is most likely related to us actually missing the events due to plugin load order. We can fix this by explicitly enumerating sources in the constructor. Additionally in order to reduce the human error factor, we should avoid explicit initialize() and finalize() calls for our singleton. Instead the get() function should do all of the heavy lifting, including thread safety, so that the human writing the code will have next to no chances to break it.
Also improves the functionality logic slightly to be more in line with real behavior.
As the recursion checking code is somewhat broken in libOBS, we need something to prevent accidental recursion from occurring. While the alternative fix is to simply make all of libOBS support recursion, unfortunately that endeavor would be too large for a single person to take on.
| if (NOT TARGET obs-frontend-api) | ||
| if(EXISTS "${${PREFIX}OBS_PATH}/cmake/obs-frontend-api/obs-frontend-apiConfig.cmake") | ||
| include("${${PREFIX}OBS_PATH}/cmake/obs-frontend-api/obs-frontend-apiConfig.cmake") | ||
| elseif((EXISTS "${${PREFIX}OBS_PATH}/bin/${D_PLATFORM_BITS}bit/obs-frontend-api.lib") AND (EXISTS "${${PREFIX}OBS_PATH}/include/obs-frontend-api.h")) |
There was a problem hiding this comment.
What is the obs-frontend-api.lib file? If it's the obs-frontend-api shared library, its path for Linux should be: ${${PREFIX}OBS_PATH}/lib/libobs-frontend-api.so
There was a problem hiding this comment.
The .lib file is the linker file for obs-frontend-api.so/.dll. It should be present in your build.
There was a problem hiding this comment.
Nevermind, it directly links to the .so file instead. The .lib file is only with some compilers and flags.
There was a problem hiding this comment.
Hmm how are configurations that have the libobs-frontend-api.so and the obs-frontend-api.h files but not the obs-frontend-apiConfig.cmake or the obs-frontend-api.lib files getting the UI?
There was a problem hiding this comment.
All my builds have the ...Config.cmake file, as well as a .lib file. That OBS still doesn't export a .pc or .cmake file for the frontend-api is probably a bug.
There was a problem hiding this comment.
From what I read, the .lib files are Windows specific.
I've opened obsproject/obs-studio#6556 for the CMake files.
There was a problem hiding this comment.
It depends on the compiler/linker used. I'll adjust it to check for .so on unix, just to be on the safer side. Generally these cmake files should be present, and any project not exporting them (as well as a pc file for pkg-config) in a package step is technically broken or not meant to have plugins.
| if(NOT EXISTS "${${PREFIX}OBS_PATH}/cmake/LibObs/LibObsConfig.cmake") | ||
| message(FATAL_ERROR "${LOGPREFIX} The provided path for libOBS is invalid as it did not contain '/cmake/LibObs/LibObsConfig.cmake'.") | ||
| return() |
There was a problem hiding this comment.
Builds from the master branch of OBS don't ship this file anymore, is it possible to check if the headers are present instead?
There was a problem hiding this comment.
No, this file is required as it sets up everything. If your builds do not ship this anymore, then your builds are broken and should be fixed.
There was a problem hiding this comment.
I fixed the OBS build upstream, the CMake config files will be included in the OBS 28 Flatpak. Do you plan on releasing v0.12 before it is released?
There was a problem hiding this comment.
Also, OBS CMake (at least on the master branch) generates a $PREFIX/cmake/libobs/libobsConfig.cmake, not a $PREFIX/cmake/LibObs/LibObsConfig.cmake one (different capitalization).
There was a problem hiding this comment.
The planned release for 0.12 is somewhere around end of July/start of August. Exact dates are not set, since StreamFX is not a net-positive product and as such is more of a "when I have time" thing.
There was a problem hiding this comment.
Also, OBS CMake (at least on the master branch) generates a $PREFIX/cmake/libobs/libobsConfig.cmake
StreamFX builds against tagged versions, and the change is for 27.2.4. 27.2.4 exports a "LibObsConfig.cmake", not a "libobsConfig.cmake". I do not know how you are building your CMake packages, but something is clearly wrong.
There was a problem hiding this comment.
The planned release for 0.12 is somewhere around end of July/start of August. Exact dates are not set, since StreamFX is not a net-positive product and as such is more of a "when I have time" thing.
No worries, a time frame and knowing that you don't align to OBS releases is enough to give me an indication.
StreamFX builds against tagged versions, and the change is for 27.2.4. 27.2.4 exports a "LibObsConfig.cmake", not a "libobsConfig.cmake". I do not know how you are building your CMake packages, but something is clearly wrong.
Yes, I was talking about the master branch, which will become OBS 28, so that you're aware of the change. Also targets will be prefixed with the OBS:: namespace.
By the way, I'm not building OBS myself, I'm just grabbing the official builds from upstream CI.
There was a problem hiding this comment.
Yes, that's what I'm saying: the commit is obsproject/obs-studio@df94b02, it is present in the master branch and will be present in OBS 28, and is not present in OBS 27.2.4. So when OBS 28 is released, you'll have to take the aforementioned changes into account to build against it.
|
For the record, the command |
Explain the Pull Request
This is a refactor of most libOBS wrapper classes to ensure improved stability and crash safety, as well as to ensure that future behavior becomes unified across all features. As a result of past failures to refactor this, a lot of StreamFX code is currently duplicated across multiple features, and sometimes even has different behavior.
Related Issues
Checklist