search

luxonis / depthai-core

DepthAI C++ Library
MIT License
238 stars 128 forks source link

[BUG] XLink without XLINK_LIBUSB_SYSTEM=ON breaks other libusb usages #898

Open 0y8w1x opened 1 year ago

0y8w1x commented 1 year ago

Describe the bug linking other libraries that use libusb, as for example the monado project (https://gitlab.freedesktop.org/monado/monado) breaks those libraries if XLink wasn't built with XLINK_LIBUSB_SYSTEM=ON. reason for that is that the linker uses the depthai libusb build for every other libusb linkage instead of the system libusb.

Expected behavior installing depthai-core shouldn't by default break other libraries using libusb.

Suggested fix in depthai-core/cmake/Hunter/config.cmake I suggest to add CMAKE_ARGS XLINK_LIBUSB_SYSTEM=ON by default, at least for linux builds, as it is often the case that libusb is already installed. so there is no need to introduce another version of it.

themarpe commented 1 year ago

@0y8w1x Having a custom version of libusb is unfortunately required due to Android support. We can extend this as an option to DepthAI option(DEPTHAI_LIBUSB_SYSTEM) and document it appropriately

This is a bit of an "upstream library capabilities issue", where the versions diverged. One option is to make other dependencies rely on "dependencies that DepthAI brings" (under [install]/lib/cmake/depthai/dependencies)

Wallbraker commented 1 year ago

So it's technically only needed on Android? If it's a requirement for Android, then only use it on Android. As a rule you really should be using libraries from the system. . And you should upstream the needed change to libusb then only use your bundled one if you encounter a old version for Android.

Rebuilding the dependencies against depthAI isn't always a option, and is very annoying to users.

themarpe commented 1 year ago

DepthAI uses Hunter as package manager - on Windows there is no "system" libusb available, so it uses Hunter for that as well.

As a rule you really should be using libraries from the system

Being a shared library, being a system one would be better indeed - preferably we'd have a private linking here such that it would not clash with other programs using "other" libusb. Otherwise for private libraries I don't agree with this statement.

And you should upstream the needed change to libusb then only use your bundled one if you encounter a old version for Android.

https://github.com/libusb/libusb/pull/1164 in the PR state for ~3 years, 1y clean - I would love that it was upstreamed to not require a custom fork however

Rebuilding the dependencies against depthAI isn't always a option, and is very annoying to users.

Agreed - perhaps we should explore making all dependencies private (including libusb) - for static libraries utilizing private linking & for shared the auditwheel approach of "namespacing" the so/dll. In case system dependencies would rather be used, one can simply disable use of package manager & provide "common/system" libraries instead.

Though, the latter is already the case for libusb and system version can be picked instead