Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

media-libs/libsdl2: add USE to enable hidapi-libusb flag #22558

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Tatsh
Copy link
Contributor

@Tatsh Tatsh commented Oct 11, 2021

Package-Manager: Portage-3.0.28, Repoman-3.0.3

Yuzu and other emulators need this flag enabled if SDL is to be used as a shared library. Without it, motion controls silently fail.

Yuzu is also in GURU but only builds from Git master and submodules for the time being.

My Yuzu ebuild

@gentoo-bot
Copy link

Pull Request assignment

Submitter: @Tatsh
Areas affected: ebuilds
Packages affected: media-libs/libsdl2

media-libs/libsdl2: @gentoo/games

Linked bugs

No bugs to link found. If your pull request references any of the Gentoo bug reports, please add appropriate GLEP 66 tags to the commit message and request reassignment.

If you do not receive any reply to this pull request, please open or link a bug to attract the attention of maintainers.

Missing GCO sign-off

Please read the terms of Gentoo Certificate of Origin and acknowledge them by adding a sign-off to all your commits.


In order to force reassignment and/or bug reference scan, please append [please reassign] to the pull request title.

Docs: Code of ConductCopyright policy (expl.) ● DevmanualGitHub PRsProxy-maint guide

@gentoo-bot gentoo-bot added assigned PR successfully assigned to the package maintainer(s). no bug found No Bug/Closes found in the commits. no signoff One or more commits do not indicate GCO sign-off. labels Oct 11, 2021
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-10-11 19:05 UTC
Newest commit scanned: a56ddfc
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/b385f3d743/output.html

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-10-11 19:35 UTC
Newest commit scanned: 0a929f6
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/36f3f83b92/output.html

@chewi
Copy link
Member

chewi commented Oct 11, 2021

Thanks for this, but could we call the flag raw-hid or something to reflect what it actually does?

@Tatsh
Copy link
Contributor Author

Tatsh commented Oct 11, 2021

Yes. Happy to change it.

There are two variants of HIDAPI usage: HIDAPI+libusb and HIDAPI without libusb. Do we want a USE flag for either of these? Currently without anything only HIDAPI without libusb support is built (based on auto detection).

I propose having +hidapi (+ because most people would want it and it is the default; precedent: app-misc/g810-led) and then specify USE="hidapi libusb" to this variant.

EDIT: libusb is preceded by several packages which is why I went with it.

@Tatsh
Copy link
Contributor Author

Tatsh commented Oct 13, 2021

This change would require setting matching KEYWORDS in dev-libs/hidapi ebuild, which would include stabilising SPARC and others. Not sure if that would be desired.

@ionenwks
Copy link
Contributor

This change would require setting matching KEYWORDS in dev-libs/hidapi ebuild, which would include stabilising SPARC and others. Not sure if that would be desired.

Can also mask the USE on these arches -- which can be temporary until it's keyworded, or permanent if it's broken or there's no interest. Not that had a close look at what it's good for, just saying.

hidapi to enable/disable hidapi support (which was missing)
libusb flag to enable libusb-based raw HID access on top of hidapi
profiles: mask media-libs/libsdl2[hidapi] for unsupported archs

Package-Manager: Portage-3.0.28, Repoman-3.0.3

Signed-off-by: Andrew Udvare <audvare@gmail.com>
@Tatsh
Copy link
Contributor Author

Tatsh commented Oct 13, 2021

This change would require setting matching KEYWORDS in dev-libs/hidapi ebuild, which would include stabilising SPARC and others. Not sure if that would be desired.

Can also mask the USE on these arches -- which can be temporary until it's keyworded, or permanent if it's broken or there's no interest. Not that had a close look at what it's good for, just saying.

Done I think (as far as I can tell editing in profiles/ is how that is done). I hope the descriptions are good enough.

@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2021-10-13 10:30 UTC
Newest commit scanned: ab0ab08
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/5a36bd0aee/output.html

@Tatsh
Copy link
Contributor Author

Tatsh commented Oct 25, 2021

bump?

@chewi
Copy link
Member

chewi commented Oct 25, 2021

For future reference, you can use package.use.stable.mask to mask flags on stable only. This is hopefully just temporary anyway. I'd request some keywording/stabilisation now but it looks like hidapi might be bumped soon.

@chewi
Copy link
Member

chewi commented Oct 25, 2021

Sorry, I realised afterwards that I'd overlooked some things here and had to revert it.

  1. This doesn't actually use the external hidapi package, but a bundled copy within SDL2.
  2. The libusb library is loaded dynamically though dlopen but is possibly unused.
  3. The changes were made to the stable ebuild instead of a new revision.

I'm not sure whether the first point can be easily addressed properly. It doesn't look like any consideration was made upstream for using an external copy of the library. A patch with an upstream submission would be much appreciated.

On the second point, most external libraries have a --disable-*-shared option but libusb does not. Setting require_hidapi_libusb=yes hacks around it, but the library seemingly gets dropped by -Wl,--as-needed anyway, as though it weren't being used at all. I'm not sure, but the impression I get is that libusb is only used on BSD. We don't really claim to support BSD like we used to, so this could be dropped, but I would accept changes for this if it were handled properly.

If it hadn't been for all the above, I might have let the change to a stable ebuild slide.

@chewi chewi reopened this Oct 25, 2021
@chewi
Copy link
Member

chewi commented Oct 25, 2021

Looking closer at libusb, I'm even more confused. SDL_hidapi.c suggests it only supports being loaded dynamically, but configure forces it to be explicitly linked on BSD. I think it's only outright required on BSD regardless. It's unclear whether there's any benefit in enabling it on other platforms.

@chewi
Copy link
Member

chewi commented Oct 25, 2021

Okay, I see that it allows you to use Nyko and EVORETRO GameCube adaptors, but I think that's all.

@Tatsh
Copy link
Contributor Author

Tatsh commented Oct 26, 2021

Thanks for responding.

The point of this change is to make motion (accelerometer, etc) work with the Nintendo Switch Pro Controller mainly.

Within Yuzu, without this flag, SDL will incorrectly see the Pro Controller as two devices rather than one, and motion will not work correctly. I am not sure what the magic is underneath.

I guess I will have to dig into making it work with external libusb.

Tatsh added a commit to Tatsh/tatsh-overlay that referenced this pull request Oct 26, 2021
SDL built with --enable-hidapi --enable-hidapi-libusb uses a bundled version of
libusb that needs to be patched out in media-libs/libsdl2 before the PR can be
accepted.

gentoo/gentoo#22558

Package-Manager: Portage-3.0.28, Repoman-3.0.3
RepoMan-Options: --force
@tiagodfer
Copy link

Hey guys, what's the current state of this request? Is there anyway to have motion detected by SDL2?

@Tatsh
Copy link
Contributor Author

Tatsh commented Apr 28, 2023

It's a pain to make this proper for Gentoo so it's on hold.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assigned PR successfully assigned to the package maintainer(s). no bug found No Bug/Closes found in the commits. no signoff One or more commits do not indicate GCO sign-off.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants