nowrep / obs-vkcapture

OBS Linux Vulkan/OpenGL game capture
GNU General Public License v2.0
508 stars 25 forks source link

Overlay library paths #132

Closed onitake closed 1 year ago

onitake commented 1 year ago

Hi, obs-vkcapture is working very well for me. My system can reach a 4K@60Hz capture + realtime encoding rate in OBS without any performance impact on games.

I'm trying get the plugin packaged for Debian, but I ran into some Lintian errors on the overlay libraries libobs_glcapture.so and libVkLayer_obs_vkcapture.so. Since these aren't "proper" shared libraries (you're building them as "MODULE" according to CMakeLists.txt), Debian doesn't like them in /usr/lib/<architecture_triplet>. But even if I compile them with the "SHARED" flag, I'm running into other errors.

That made me wonder if /usr/lib/<architecture_triplet> is even the right place for them. Putting them into /usr/lib/<architecture_triplet>/obs-vkcapture (or some other plugin-like directory) leads to no problems. Obviously, the paths in obs_vkcapture.json and obs-gamecapture have to be changed as well, but that's easy to do.

On another note, I saw that obs-gamecapture contains a modification of the LD_LIBRARY_PATH. Is there a specific reason for that? I think it makes more sense to put the absolute path of the overlay library into the LD_PRELOAD variable, like this:

--- a/src/obs-gamecapture.in
+++ b/src/obs-gamecapture.in
@@ -8,7 +8,6 @@
     exit 1
 fi

-exec env LD_LIBRARY_PATH="${LD_LIBRARY_PATH}${LD_LIBRARY_PATH:+:}@CMAKE_INSTALL_PREFIX@/\$LIB" \
-       LD_PRELOAD="${LD_PRELOAD}${LD_PRELOAD:+:}libobs_glcapture.so" \
+exec env LD_PRELOAD="${LD_PRELOAD}${LD_PRELOAD:+:}@CMAKE_INSTALL_FULL_LIBDIR@/libobs_glcapture.so" \
        OBS_VKCAPTURE=1 \
        "$@"

Even so, I don't think the $LIB path makes much sense here. Different Linux distributions use different paths for shared libraries, and Debian uses /usr/lib/<architecture_triplet> (which is /usr/lib/x86_64-linux-gnu/ on amd64). I think you should simply use @CMAKE_INSTALL_FULL_LIBDIR@ instead of @CMAKE_INSTALL_PREFIX@/\$LIB here, and it will pick up whatever path was used as the installation path of libobs_glcapture.so.

nowrep commented 1 year ago

It doesn't really matter where you put the layer library, that's true. However, even debian packages puts it in /lib (see https://packages.debian.org/bullseye/amd64/vulkan-validationlayers/filelist).

I think you should simply use @CMAKE_INSTALL_FULL_LIBDIR@

This cannot be done because this is meant to work for both 32 and 64bit libraries. You just use obs-gamecapture ... and the loader will decide which library to load depending on if the binary is 32 or 64bit.

onitake commented 1 year ago

It doesn't really matter where you put the layer library, that's true. However, even debian packages puts it in /lib (see https://packages.debian.org/bullseye/amd64/vulkan-validationlayers/filelist).

I think you should simply use @CMAKE_INSTALL_FULL_LIBDIR@

This cannot be done because this is meant to work for both 32 and 64bit libraries. You just use obs-gamecapture ... and the loader will decide which library to load depending on if the binary is 32 or 64bit.

That is a very good point, I haven't thought about that. However, it makes me wonder what ld.so does if the system doesn't use /usr/lib64. Does /usr/$LIB resolve to /usr/lib/x86_64-linux-gnu on Debian amd64?

nowrep commented 1 year ago

Does /usr/$LIB resolve to /usr/lib/x86_64-linux-gnu on Debian amd64

Yes

onitake commented 1 year ago

All right, thanks for the clarification.

If I want to keep the plugin-like path to work around the shared library Lintian errors, I should probably use an LD_LIBRARY_PATH like this: @CMAKE_INSTALL_PREFIX@/\$LIB/obs-vkcapture. Does that make sense?

That still leaves obs_vkcapture.json. It references @CMAKE_INSTALL_FULL_LIBDIR@/libVkLayer_obs_vkcapture.so, which means that it will always resolve to the same file, independent of the architecture. And there is no architecture-specific version, it's always installed to /usr/share/vulkan/implicit_layer.d/. Is there anything that can be done to make it work for 32 and 64 bit programs?

nowrep commented 1 year ago

Is there anything that can be done to make it work for 32 and 64 bit programs?

It gets installed as obs_vkcapture_64.json (or 32 respectively), see https://github.com/nowrep/obs-vkcapture/blob/35e147b5695f53e7f2f5632ff37c56afa81e059d/CMakeLists.txt#L119

onitake commented 1 year ago

Right, I see. That means that if both are installed, Vulkan will simply skip the layer that can't be loaded for the active architecture, correct?

Although... now that I think about it, this isn't ideal. It will only work if the host architecture supports "32"-bit and "64"-bit binaries. In a hypothetical heterogenous case, it wouldn't be possible to install it for two completely distinct CPU architectures. Perhaps the name of the actual CPU architecture should be used instead of "32" and "64"? I.e., I would do this:

--- obs-vkcapture.orig/CMakeLists.txt   2023-01-12 19:32:57.202089004 +0100
+++ obs-vkcapture/CMakeLists.txt        2023-01-12 19:33:03.449893484 +0100
@@ -35,11 +35,7 @@

 option(BUILD_PLUGIN "Build OBS plugin" ON)

-if (${CMAKE_SIZEOF_VOID_P} EQUAL 4)
-    set(LAYER_SUFFIX "_32")
-else()
-    set(LAYER_SUFFIX "_64")
-endif()
+set(LAYER_SUFFIX "_${CMAKE_LIBRARY_ARCHITECTURE}")

 set(CMAKE_C_STANDARD 11)
nowrep commented 1 year ago

There is no hypothetical heterogeneous case in practice, so no :D But you're free to add custom patches when making the package.

onitake commented 1 year ago

Since my questions are answered and I got the plugin packaged in a sensible way (I think), the issue can be closed.

Although, would you accept a PR for my last comment? I think it makes sense to put the target architecture into the file name explicitly, as there may be other use cases than the amd64/i386 doublet (amd64/i386/x32 comes to mind, it's still supported on Debian).

nowrep commented 1 year ago

No, see https://vulkan.lunarg.com/doc/view/1.3.211.0/linux/LoaderLayerInterface.html#user-content-layer-conventions-and-rules

If anything, maybe would be a good idea to eliminate this duplicated layer json with setting "library_path": "/usr/$LIB/libVkLayer_obs_vkcapture.so", but definitely not making it even more complicated.

onitake commented 1 year ago

No, see https://vulkan.lunarg.com/doc/view/1.3.211.0/linux/LoaderLayerInterface.html#user-content-layer-conventions-and-rules

This document explicitly says:

The JSON file itself does not have any requirements for naming.

That indicates to me that there's nothing that would disallow putting the host triplet into the manifest file name.

If anything, maybe would be a good idea to eliminate this duplicated layer json with setting "library_path": "/usr/$LIB/libVkLayer_obs_vkcapture.so", but definitely not making it even more complicated.

I'll test this. I had assumed it wouldn't work, because $LIB is a feature of ld.so and I don't know how Vulkan implementations load overlays. But it would definitely be a better choice.