SDL
SDL copied to clipboard
libsdl-org
Add support for libusb device whitelisting
When combining both libusb and an OS backend for SDL_hidapi, we want to ensure that libusb is only used when absolutely necessary. At the same time, we don't want to interfere with platforms that only have libusb. So, implement a whitelist but only use it when another backend exists.
With this in place, it should now be safe to enable libusb support by default.
Affects SDL_hidapi (Sam), configure.ac (Ozkan), and CMake (Anon).
Fixes #6005
This doesn't bring back https://github.com/libsdl-org/SDL/issues/5722 does it?
No, it should only affect devices that aren't visible to the Linux hid.c backend. There's logic in the libusb hid.c that takes over devices during enumeration but it's disabled (with good reason).
I believe this will break Xbox controllers on Steam Link, where we use linux/hid.c, but xpad is largely unused and we rely on libusb for access to those devices.
I'm putting this in the next milestone so we can smoke out issues like these early in the milestone.
Pushed the SDL_bool change.
For Link, it should be possible to make the list user-modifiable, not sure how that might be done though.
Moved the check a little closer to the whitelist declaration, to make it easier to keep the whitelist work in one place (in case we do end up adding variables or something).
For Link, it should be possible to make the list user-modifiable, not sure how that might be done though.
For systems where exclusive access to the device isn't an issue, like Steam Link, we probably want to turn this off. Maybe we can make it a configure option?
It's almost like we want 3 passes:
- Devices where libusb is preferred (e.g. GameCube adapter)
- Devices where platform support is preferred (e.g. almost everything else)
- Devices which platform support doesn't pick up (e.g. Xbox controllers)
If it's possible to prioritize, switching the order to have libusb as the last pass would definitely be enough, since the adapter doesn't show up as a regular device anyhow.
It looks like the backend priority was flipped to libusb first in this commit: https://github.com/libsdl-org/SDL/commit/69a8c8468c211d66ad547a352c4762470298aa36
libusb enforces exclusive access to the device, which isn't generally useful on desktop systems. I don't mind a define for custom systems that enables this, but I don't think we should do it by default.
Feel free to submit another PR that implements an option for this, but I'll go ahead and close this one.
Just to be sure: Does it enforce exclusive access during enumeration time? The main goal of this patch was to enumerate first and avoid any device that wasn't in the list, so that libusb doesn't take it over - if anything this should make it take devices less often, but that will only be true if enumeration doesn't grab the device to get the product information.
No, enumeration is fine, it's only if you open it. I'm going to reopen this, I think I misunderstood the intent.
Is this blocked on something in particular? or is it held up by just a lack of time?
I'm going to change it to merge lists based on device priority, but I haven't gotten time to do that yet.
Actually... this is pretty good as-is. We just need to enable an option to change the whitelist behavior from CMake.
Rebase is done - CMake should now enable libusb by default when available for all targets, but at the same time it will only affect builds that have libusb and another hidapi backend available, so the *BSDs shouldn't be affected here.
Just to be clear:
- would this make the pre-built SDL for windows support things like the GameCube controller automatically?
- would an SDL built from source also need to get a copy of libusb for windows during the build for that to be enabled, or is the necessary libusb stuff dynamically linked or whatever?
Updated to "fix" the CMake changes, but my solution is really clumsy - feel free to redo it if it's not too hard to fix! (This may affect the premade project files as well, not sure if that's in CI...)
Just to be clear:
- would this make the pre-built SDL for windows support things like the GameCube controller automatically?
- would an SDL built from source also need to get a copy of libusb for windows during the build for that to be enabled, or is the necessary libusb stuff dynamically linked or whatever?
I don't think we're planning on adding libusb to the Windows build. That may change by the time SDL 3.0 ships though.
Ah. It was suggested in https://github.com/libsdl-org/SDL/issues/7481 that this PR would be the most likely path to using gamecube controllers on Windows. Unfortunate, but I guess that can be resolved in the future.
Took a different approach that mostly avoids CMake altogether - instead of determining the use as a build config, I've changed it to a hint, which is enabled by default only when there are other hidapi backends available. This seemed like a better way to allow enabling/disabling this for a particular application without having to rebuild SDL or redirect dynapi to a custom file.
Assuming https://github.com/libsdl-org/SDL/pull/6014#issuecomment-1207251777 holds, the cmake changes are fine: libusb will be enabled for most platforms.
This did indeed break Steam Link hardware: https://steamcommunity.com/app/353380/discussions/0/5992658048661696817/
The problem is that opening a device with libusb unbinds it from the HID subsystem, which is what SDL uses to detect and open Steam Controllers on Steam Link hardware.
What we really need to do is enumerate the devices using all the back ends and then add them to the enumeration list based on driver priority, which is controlled by the whitelisting process.