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

IOS: USB fixes #8070

Closed
wants to merge 13 commits into from

Conversation

7 participants
@leoetlino
Copy link
Member

commented May 2, 2019

Updates libusb and fixes several bugs both in libusb and in Dolphin's IOS HLE.

A huge thanks to JMC47 for his patience testing the many changes over the past three days.

Note: all libusb patches have been submitted to upstream (libusb/libusb#567, libusb/libusb#568, libusb/libusb#569).

Usage notes:

  • libusbK is now the recommended backend on Windows. usbdk should still work and is now supported by upstream libusb, but libusbK should be preferred over it as it is more mature and does not require an extra dependency.
  • For microphones and cameras, and any other device that has several USB interfaces, if you're using the libusbK backend, you must install the driver over the parent composite device in Zadig.
@JMC47

This comment has been minimized.

Copy link
Contributor

commented May 2, 2019

After reviewing the submission text carefully I agree.

@JMC47

This comment has been minimized.

Copy link
Contributor

commented May 2, 2019

Your Shape and Racquet Sports camera functionality now work in this PR

image

@BhaaLseN

This comment has been minimized.

Copy link
Member

commented May 2, 2019

...do we really need all those extra libusb files in there? I mean, we stripped them out before, and it's like 12k extra LoC that basically don't do anything.

}
if (AttachInterface(interface) != 0)
if (AttachInterface(0) != 0)

This comment has been minimized.

Copy link
@BhaaLseN

BhaaLseN May 2, 2019

Member

Had to look twice to notice that 0 (or any other value) is necessary here now since the interface parameter is gone. But is zero really a good value for this? Feels like it should either just fail, or pick some sort of default that might be indistinguishable from magic. Can't tell which one it is, or whether it's a third option.

This comment has been minimized.

Copy link
@leoetlino

leoetlino May 3, 2019

Author Member

Hmmm yeah, it might be better to just not claim any interface by default since the function is supposed to just attach the device... Better to make it explicit than implicit. I'll change this tomorrow

This comment has been minimized.

Copy link
@leoetlino

leoetlino May 4, 2019

Author Member

I've changed the way interfaces are claimed completely (as part of some bug fixes). Now all interfaces are claimed ahead-of-time as it's difficult to predict which interfaces are going to be used and errorprone to change them when needed. Claiming them ahead-of-time ensures the emulated software will always be able to use the interface it wants, just like on an actual Wii.

@leoetlino leoetlino force-pushed the leoetlino:usb-fixes branch from 3d1637c to a842d96 May 3, 2019

@leoetlino leoetlino changed the title IOS: USB fixes [WIP] IOS: USB fixes May 4, 2019

@leoetlino leoetlino force-pushed the leoetlino:usb-fixes branch from a842d96 to 6d73468 May 4, 2019

@leoetlino leoetlino changed the title [WIP] IOS: USB fixes IOS: USB fixes May 4, 2019

@leoetlino leoetlino added the QA done label May 4, 2019

@leoetlino

This comment has been minimized.

Copy link
Member Author

commented May 4, 2019

For the sake of making code review easier, the new/modified commits are:

  • 8b604ed (Fix initial device scan)
  • e8829e9 (Claim all interfaces ahead-of-time)
  • 6d73468 (Externals/libusb: Set policy ISO_ALWAYS_START_ASAP for libusbK). This patch will be submitted to upstream too.

JMC47 and I have tested OH0 (microphone, Wii Speak), VEN (microphone, camera) and HIDv4 (Internet Channel, Wii Shop Channel). HIDv5 wasn't tested but is very likely to work because it is pretty much a simpler version of VEN.

This fixes random crashes that were caused by libusb on Windows, random transfer failures, Your Shape on Windows, and Your Shape hanging on shutdown and Racquet Sports on all platforms.

@JMC47

This comment has been minimized.

Copy link
Contributor

commented May 5, 2019

Everything is confirmed to be working. I think we need to drop the usbdk recommendation (and maybe support for it altogether unless some of the bugs are fixed.)

libusbK needs to be recommended over WinUSB in Zadig in all places on the wiki and guides, as it's just superior at this point in time.

@mbc07

This comment has been minimized.

Copy link
Contributor

commented May 5, 2019

Any chance of the changes of this PR also impacting BT Passthrough communication? Some (non-Wii) BT Adapters have different results depending of whether you're using WinUSB/libusbK/UsbDk on Windows...

@JMC47

This comment has been minimized.

Copy link
Contributor

commented May 5, 2019

libUSBK should remove any crashes involved with using bluetooth passthrough.

@leoetlino

This comment has been minimized.

Copy link
Member Author

commented May 5, 2019

BT passthrough shouldn't be affected much, but yeah the libusb update might fix some random crashes.

@leoetlino leoetlino force-pushed the leoetlino:usb-fixes branch from 6d73468 to f450e34 May 8, 2019

@psennermann

This comment has been minimized.

Copy link

commented May 11, 2019

I've uninstalled usbdk 1.19, then followed these instructions:
Download and open Zadig, then:

  1. In the "Options" menu in Zadig, make sure "List All Devices" is enabled.
  2. Look through the devices for the device to passthrough.
  3. On the right column, select "libusbk" then click "Replace Driver". Select "Yes" to modify the system driver.

but now with both master 5.0-10188 and this PR, my Logitech microphone doesn't get recognised in game (We Sing keeps on asking to connect the microphone)...any suggestions?
Thanks

P.S. I also noticed that there's a brand new version of usbdk (v.1.21 - released 11 days ago)...any chances that this new version could be used instead of libusbk?

@JMC47

This comment has been minimized.

Copy link
Contributor

commented May 11, 2019

You have to install the libusbK driver to the parent device, you have to make sure composite devices are enabled in zadig and find the composite device and overwrite the driver. You'll get a warning but it'll be fine.

@JMC47

This comment has been minimized.

Copy link
Contributor

commented May 11, 2019

usbdk should be working again but libusbK is the most mature driver by far. It is the only one with good enough support for us to guarantee consistent support of the microphone, wiispeak, and camera peripherals.

@psennermann

This comment has been minimized.

Copy link

commented May 11, 2019

ok, great work, "We Sing" doesn't seem to crash anymore...

@JMC47

This comment has been minimized.

Copy link
Contributor

commented May 12, 2019

The last thing to do is to verify usbdk.

leoetlino added some commits May 1, 2019

Externals: Update libusb to 1.0.23-rc1
Now has support for isochronous transfers in the WinUSB backend,
which may or may not work better than the UsbDk backend.
IOS/USB: Add debug logging for all transfers
This makes debugging USB issues easier.
IOS/USB: Claim all interfaces ahead-of-time
To avoid having to claim/release interfaces all the time, and having to
trigger interface changes from several places, all interfaces are now
claimed ahead of time.

This commit also makes us avoid changing the active interface when it's
not necessary.

Changing the active interface has side effects such as resetting the
active alternate setting -- which is extremely undesirable because it
would require the emulated software to change the alternate setting
again, which isn't supposed to be necessary at all.

This fixes Your Shape, which submits isochronous transfers on an
endpoint that only exists in alt setting 6 right after submitting
control transfers (which would have reset to alt setting 0 prior to
this fix).
IOS/VEN: Read cancel endpoint correctly
Fixes an embarrassing bug that made the implementation utterly useless.

This fixes Your Shape hanging on shutdown. The game was waiting for an
interrupt transfer to be cancelled, and Dolphin wasn't cancelling
transfers on the correct endpoint.
IOS/USB: Fix TransferCommand length type
The total buffer size for isochronous transfers should be a u32,
not a u16. It is easy to hit the bug with devices such as cameras,
which require larger buffers.

This fixes Your Shape.

This also fixes the length type for bulk and interrupt transfers,
which should be u32 as that's what IOS supports. I'm not sure why
I made them u16... probably because OH0 uses u16 for most lengths...

leoetlino added some commits May 2, 2019

Externals: remove unused libusb files and add them to gitignore
This prevents them from being re-added in the future whenever someone
updates libusb. Also removes the need to manually remove those files.
IOS/USB: Fix initial device scan
Even though libusb is supposed to be thread-safe, in practice
it's not (at least on Windows); getting a list of devices from two
different threads can result in libusb crashes. This is easily
fixed by waiting for the scan thread to complete scanning instead
of running the scan on the CPU thread.

This also fixes an issue that I had overlooked in the initial
implementation: IOS interfaces such as OH0 are sometimes opened
every frame, in which case we were doing a full device scan every
single frame on the CPU thread!
Externals/libusb: Set policy ISO_ALWAYS_START_ASAP for libusbK
In some cases, this is required to avoid eventually getting a
USBD_STATUS_BAD_START_FRAME error back from the Windows USB stack.

This makes the libusbK code match the behaviour of the Linux backend.

It appears that the libusbK backend tried to get this behaviour by
setting StartFrame to 0. However, libusbK docs state that:

"Specifing 0 for KISO_CONTEXT::StartFrame (start transfer ASAP) is
restricted to the first transaction on a newly opened or reset pipe."

@leoetlino leoetlino force-pushed the leoetlino:usb-fixes branch from f450e34 to b601de7 May 12, 2019

@leoetlino

This comment has been minimized.

Copy link
Member Author

commented May 12, 2019

Added a commit to re-enable usbdk support.

@JMC47

This comment has been minimized.

Copy link
Contributor

commented May 12, 2019

usbdk is working in all titles with USB Passthrough. Going to test Bluetooth Passthrough next.

No Zadig required, no driver overriding required. It's nice.

RYRE41-3

@leoetlino leoetlino added the WIP label May 12, 2019

@leoetlino leoetlino force-pushed the leoetlino:usb-fixes branch 7 times, most recently from 7e4cdbd to 3e226ae May 12, 2019

@JMC47

This comment has been minimized.

Copy link
Contributor

commented May 12, 2019

Okay, usbdk testing finished

usbdk
Rockband Controllers = Working
Tony Hawk Ride Skateboard = Working
Wii Speak = Working
Microphone = Working
Camera = Working
Wii Bluetooth Adapter = Partially working - Fails on ES_Launch*
Native GameCube Controller Support = Unknown (Maybe hasn't been hooked up yet?)

libusbK
Rockband Controllers = Working
Tony Hawk Ride Skateboard = Working
Wii Speak = Working
Microphone = Working
Camera = Working
Wii Bluetooth Adapter Passthrough = Working
Native GameCube Controller Support = Working

WinUSB
Rockband Controllers = Working
Tony Hawk Ride Skateboard = Working
Wii Speak = Partially Working - Detected but audio doesn't seem to always work
Microphone = Not Working - Detected but no input
Camera = Not Working - No Image
Wii Bluetooth Adapter Passthrough = Working
Native GameCube Controller Support = Working

*Because ES_Launch disconnects Bluetooth for a split second, if you have a Windows Bluetooth driver installed, it will lose connection for too long. Having the Bluetooth be completely uninstalled fixes this. Dolphin no longer crashes at least. Maybe we could popup a better error message? Could we change behavior to avoid this?

@psennermann

This comment has been minimized.

Copy link

commented May 13, 2019

Perfect, I tried the new version with usbdk and it also doesn't crash anymore

@psennermann

This comment has been minimized.

Copy link

commented May 18, 2019

With this PR I got better scores in every song I've tried...so or microphone emulation has improved or I've become a better singer or both :-)

@JMC47

This comment has been minimized.

Copy link
Contributor

commented May 18, 2019

Microphone Passthrough is way better. Even I'm getting better scores.

@Hacsev

This comment has been minimized.

Copy link

commented May 20, 2019

Okay, usbdk testing finished

usbdk
Rockband Controllers = Working
Tony Hawk Ride Skateboard = Working
Wii Speak = Working
Microphone = Working
Camera = Working
Wii Bluetooth Adapter = Partially working - Fails on ES_Launch*
Native GameCube Controller Support = Unknown (Maybe hasn't been hooked up yet?)

libusbK
Rockband Controllers = Working
Tony Hawk Ride Skateboard = Working
Wii Speak = Working
Microphone = Working
Camera = Working
Wii Bluetooth Adapter Passthrough = Working
Native GameCube Controller Support = Working

WinUSB
Rockband Controllers = Working
Tony Hawk Ride Skateboard = Working
Wii Speak = Partially Working - Detected but audio doesn't seem to always work
Microphone = Not Working - Detected but no input
Camera = Not Working - No Image
Wii Bluetooth Adapter Passthrough = Working
Native GameCube Controller Support = Working

*Because ES_Launch disconnects Bluetooth for a split second, if you have a Windows Bluetooth driver installed, it will lose connection for too long. Having the Bluetooth be completely uninstalled fixes this. Dolphin no longer crashes at least. Maybe we could popup a better error message? Could we change behavior to avoid this?

What exactly is the procedure you are doing for your tests? I updated Dolphin and installed the latest libusbK and it still crashes. I even downloaded Zadig 2.4 and replaced the driver for the Logitech Microphone and it crashes as well. Both crashes occur a few minutes into the game, usually in less than 5 minutes.

@JMC47

This comment has been minimized.

Copy link
Contributor

commented May 20, 2019

I'm using the test builds here, I'm using libUSBK installed to the composite device. If you're having crashes, it's likely user error.

@leoetlino leoetlino force-pushed the leoetlino:usb-fixes branch 2 times, most recently from 2c5517f to 626e60d May 20, 2019

Move libusb utilities to LibusbUtils
* Simplifies libusb context usage and allows us to set options for
all contexts easily. Notably, this lets us enable usbdk support
in libusb, which is now opt-in in the latest version.

* Moves the libusb config descriptor wrapper class to LibusbUtils too
since that could easily be reused.

* Moves device listing to LibusbUtils too and add a lock around it
as some libusb backends are not thread safe.

* Consequences: only a single context and a single event handling
thread is used now, which is more efficient.

@leoetlino leoetlino force-pushed the leoetlino:usb-fixes branch from 626e60d to 23e57c2 May 20, 2019

@leoetlino leoetlino closed this May 21, 2019

@leoetlino leoetlino referenced this pull request May 21, 2019

Merged

IOS: USB fixes #8109

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.