We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

We don't currently include it because it libusb requires root permission to access devices on many systems, also a C++ dependency wouldn't work for SDL.

Created attachment 3897 [details]
[PATCH] hidapi: Support multiple hidapi backends

Here's a new version that is working for me. Note that it depends on the patches from Bug 4736, Bug 4737, and Bug 4738. This definitely needs cross-platform review, I've only tested it on my Arch Linux desktop.

This patch fixes the problems Ethan mentioned. It makes both the libusb and platform-specific backends optional at both compile- and run-time. The platform-specific backend can be missing at run-time if libudev cannot be loaded on Linux. For other platforms where the platform backend will always be present, the runtime check is replaced with an if-check for 1, so hopefully the compiler will just remove that entirely at build-time.

I also changed it to use trivial pointer-aliasing with a vtable, instead of traversing a linked list and writing a switch statement for every hidapi device call.

The patch contains changes to both configure and cmake build systems, but more effort may be required for other build systems.

Cameron pointed out on Bug 4738 that hid_enumerate should call hid_init. We need to add that to our implementation here.

Created attachment 3913 [details]
SDL_hidapi support

The patch has been updated and cleaned up; the build should now work on all supported targets (even those that don't use SDL_hidapi.c). This patch still depends on Bug 4737 and Bug 4738.

The only builds that will change are autoconf/CMake builds on Linux/macOS, all other builds should be exactly the same. MinGW builds will eventually join them, but only after somebody ports libusb/hid.c to be OS-agnostic. That's for another day!

Patch added!
https://hg.libsdl.org/SDL/rev/0fef4b21fa1d

I'm leaving this bug open to collect feedback and cleanup on this change.

How will this integrate into existing Xcode project files?

For now it won't affect Xcode projects at all, those files and builds should be exactly the same as before. To add support for the Xcode project we'd have to get libusb directly involved in the SDL packages, which didn't seem like a good idea. The make builds will treat libusb like any other dependency; it'll either include SDL_hidapi and dynamically load libusb, or it'll disable the feature if it's not found.

And for the autofoo builds to catch SDL_LIBUSB_DYNAMIC,
I need to manually build libusb-1.0 beforehand and put
it in my toolchain-seeable paths, yes?

Yes. You can also add to this block here, if it makes life any easier:

https://hg.libsdl.org/SDL/file/0fef4b21fa1d/src/hidapi/SDL_hidapi.c#l132

Created attachment 3915 [details]
libusb library names patch

(In reply to Ethan Lee from comment #10)
> Yes. You can also add to this block here, if it makes life any easier:
> 
> https://hg.libsdl.org/SDL/file/0fef4b21fa1d/src/hidapi/SDL_hidapi.c#l132

libusb from github current master (1.0.23-rc1 + some more commits)
builds for osx to this:
  libusb-1.0.dylib -> libusb-1.0.1.dylib

... and for linux to this:
  libusb-1.0.so -> libusb-1.0.so.1 -> libusb-1.0.so.1.1.0

An older version (1.0.9-rc1 from CentOS-6.10 / i386) is like this:
  libusb-1.0.so.0 -> libusb-1.0.so.0.1.0
 (libusb-1.0.so symlink comes only with the -devel rpm.)

So I gather that, we need to rely on the presence of libusb-1.0.so
symlink (autofoo should actually detect it correctly) for linux,
and libusb-1.0.dylib symlink for osx (brew should itstall it???).

Is the attached patch OK?

Interesting, I didn't know they had incremented the version for their next release. I was assuming it was still 1.0.0; I tested with CentOS 7, Fedora 30, and Homebrew for macOS, which are all on 1.0.22 or lower. It looks like the only changes are new APIs, so as long as that still holds up the patch should be safe. We'll probably want to do the same thing for Linux and friends as well:

https://hg.libsdl.org/SDL/file/0fef4b21fa1d/configure.ac#l3268

Looks like even the libusb team isn't sure about the change:

https://github.com/libusb/libusb/issues/602

(In reply to Ethan Lee from comment #12)
>  We'll probably want to do the same thing for Linux and
> friends as well:
> 
> https://hg.libsdl.org/SDL/file/0fef4b21fa1d/configure.ac#l3268

You suggest that we change libusb-1.0.so.* to libusb-1.0.so* ?

(In reply to Ethan Lee from comment #13)
> Looks like even the libusb team isn't sure about the change:
> 
> https://github.com/libusb/libusb/issues/602

Huh..

What is the final verdict?

For now let's wait and see what the libusb team has to say about the ABI increment - my hope is that they'll undo the change so we can avoid having to gamble on a symlink, but if they decide to increment then we'll need to apply the patch for this to keep working when 1.0.23 is released.

(In reply to Ethan Lee from comment #15)
> For now let's wait and see what the libusb team has to say about the ABI
> increment - my hope is that they'll undo the change so we can avoid having
> to gamble on a symlink, but if they decide to increment then we'll need to
> apply the patch for this to keep working when 1.0.23 is released.

That sounds reasonable at first, but what happens when they
do make a release (something like 1.1.x) with library versions
like in current 1.0.23-rc?

I suppose it depends on how SDL has dealt with dependency breakages in the past, which I don't have experience with... other than the nasty libudev.so.0/1 issue, which is just a hardcoded list of names. Ideally the reason for an increment is a legitimate breakage, so in that case we'd still depend on a specific version and have the version-specific code based on the `LIBUSB_API_VERSION` in libusb.h, unless we really needed to support both at once (like udev).

Ryan, Sam, any insight on this?

BTW, this breaks osx builds against 10.8 SDK, because libusb/hid.c
uses clock_gettime() & co, and 10.8 doesn't have it.  IIRC, SDL2.0
still supports building against 10.7+ SDKs.  Needs fixing.

Seems like clock_gettime() is a 10.12+ thing.
libusb has replacement code in libusb/os/darwin_usb.c,
maybe it helps.

A little googling shows clock_gettime() Darwin alternatives here:
https://stackoverflow.com/questions/5167269/
https://developer.apple.com/library/archive/qa/qa1398/_index.html

libusb's hid.c definitely needs to be SDL-ified, both for macOS and an eventual Windows build. That in particular will probably be replaced with something like SDL_GetTicks, along with other stuff like the explicit use of pthread instead of SDL_Thread.

Configury cannot enable libusb-less hidapi for osx like Xcode
since this merge.  Cmake is _possibly_ affected too (didn't try
it.)  Messing with autofoo now.

Created attachment 3917 [details]
configury patch to allow libusb-less hidapi for macosx

(In reply to Ozkan Sezer from comment #22)
> Configury cannot enable libusb-less hidapi for osx like Xcode
> since this merge.  Cmake is _possibly_ affected too (didn't try
> it.)  Messing with autofoo now.

Attached patch OK?

Looks good to me! I'll try to work on cleaning up libusb hid.c today.

(In reply to Ethan Lee from comment #24)
> Looks good to me!

Applied: https://hg.libsdl.org/SDL/rev/d2e027c5a389
Cmake will need similar love & care (I won't do that..)

>  I'll try to work on cleaning up libusb hid.c today.

Thanks.

Created attachment 3919 [details]
libusb hid.c Portability Patch

Attached is a... very pretty patch to make libusb's hid.c portable. This should fix the Mac build, and it also fixes the problems that Windows had, so it's also been added to the MinGW build.

I wrote it in this way because I don't know how far off SDL's hidapi copy is from upstream, so I figured this would be the safest way to compare the two (i.e. just delete all the SDL blocks and diff). If we're sync'd to upstream, we could just nuke all the unportable stuff and put a date/commit at the top of the file that says when we last synchronized with them.

Created attachment 3921 [details]
Port libusb hid.c to SDL

Patch updated with the changes from upstream hidapi's libusb hid.c (basically some explicit type casts and typo fixes), an added date to mark when their version was last updated, and removed all the non-SDL code paths. The file is now MUCH more legible and it will be easier to check for upstream changes in the future.

Okay, I think this is complete!

Ethan, I added your patch so libusb.c works on Windows and Mac OS X:
https://hg.libsdl.org/SDL/rev/a7ca9815bc79

We'll wait on changing libusb library names, that can go in as a separate bug.

Are we waiting on anything else for this bug?

I believe that was the last thing! I've posted/subscribed to the libusb issue re: the ABI change, may be worth subbing to that for anyone who has a GitHub account:

https://github.com/libusb/libusb/issues/602

If they decide to keep the increment I'll file a new issue for that.

Great, thanks!