[PATCH 2/4] winebus.sys: Process quirky DualSense bluetooth reports.

[Roderick Colenbrander]

On Mon, Jan 24, 2022 at 10:49 AM Rémi Bernon <rbernon at codeweavers.com> wrote:
>
> On 1/24/22 17:37, Roderick Colenbrander wrote:
> > On Mon, Jan 24, 2022 at 2:25 AM Arkadiusz Hiler <ahiler at codeweavers.com> wrote:
> >>
> >> On Mon, Jan 24, 2022 at 12:48:21AM +0100, Rémi Bernon wrote:
> >>> On 1/22/22 21:01, Roderick Colenbrander wrote:
> >>>> Hi Arkadiusz,
> >>>>
> >>>> I'm not a big fan of this type of fixup here (and the same for DS4 to
> >>>> be honest). The DS4/DualSense have unusual HID reports, but this type
> >>>> of fixup causes issues for applications which properly parse the HID
> >>>> data. For example various applications for example do use our
> >>>> libScePad.dll for games on Windows. They go through the Windows HID
> >>>> APIs. This breaks such applications.
> >>>>
> >>>> Thanks,
> >>>>
> >>>> Roderick Colenbrander
> >>>> Sony Interactive Entertainment, LLC
> >>>
> >>> Hi Roderick,
> >>>
> >>> Thanks for giving some feedback. The fixups have been added for games and
> >>> applications which do not expect to receive the extended reports, and only
> >>> support the reports that are openly described in the HID descriptors.
> >>>
> >>> As Arek has found already, and as you say, some other applications or
> >>> middlewares are explicitly requesting them, so we were considering adding
> >>> some more logic to remove the fixups and send the extended reports in the
> >>> same way as on Windows, when they are requested.
> >>>
> >>> I understand that these extended reports are triggered by some feature
> >>> report requests, and this is automatically done on Linux by the kernel and
> >>> evdev driver, to the contrary to what happens on Windows, or maybe there's
> >>> something we've missed?
> >>
> >> Hi folks,
> >>
> >> As Rémi has mentioned this is only to "undo" the mode switch that
> >> hid_playstation/hid_sony does on Linux for Sony controllers when they
> >> are used over Bluetooth.
> >>
> >> On Windows those use the basic reports (#1) by default and require
> >> getting calibration feature report (#5) to switch modes.
> >>
> >> The next patch in this series handles the mode switch:
> >> https://source.winehq.org/patches/data/224003
> >>
> >> That's at least how SDL does that. If there are more ways to trigger
> >> this please let us know so we can support it.
> >
> > I believe feature report 5 is the main way.
> >
> >>
> >> Also after switching the modes the controller no longer works through
> >> DirectInput and requires to be reconnected, as it seems to be a one way
> >> transition.
> >>
> >> I've found a fairly new game that comes with libScePad.dll - God of War.
> >> The game does handle DualSense and DualShock 4 over USB only. When those
> >> contollers are connected over Bluetooth the game does not detect them.
> >
> > Correct, libScePad.dll doesn't handle Bluetooth devices and I'm not
> > sure why that decision was made (on older Windows version there
> > inconsistent Bluetooth stacks, so may have been related).
> >
> >> If you have any suggestions for a game that I could use for testing the
> >> Bluetooth extended mode switching please let me know.
> >
> > Unfortunately I don't know other games by head as I'm not part of the
> > Windows / game support teams. A good starting point would probably be
> > Sony games e.g. Horizon. There have been some other published as well.
> >
> >>
> >> --
> >> Cheers,
> >> Arek
> >
> > Overall I'm not sure what the best thing to do is. As I shared in the
> > other email various other applications (e.g. Chrome) do handle the
> > extended report data properly and would thus break.
> >
> > One method, but also not ideal at all. Would be to override the device
> > descriptors and report it as a USB device. However that would mean
> > spoofing various other reports as well and would make more like
> > hid-sony/hid-playstation. It would be the most compatible way and not
> > break other applications.
> >
>
> Well that's mostly what we are doing here, although we don't change the
> descriptor, and we fixup the minimum amount of data by using and filling
> the default input report which is already described, dropping all the
> extended data.
>
> We have other backends (SDL / evdev) which are actually doing what you
> suggest, and which expose their own HID descriptor with the data they
> get from the backend, so there too only basic gamepad data. That's no
> different imho, and it would not match the extended reports either, and
> so not provide the full feature to Chrome or other middleware which
> would be expecting it.
>
> Even worse, some games or applications are hardcoding the HID reports
> from the DS4 and other controllers and expect them, when they detect the
> controller VID/PID, to exactly match what the device is usually sending
> on Windows, whether it is the basic or the extended reports that depends
> on what they support.
>
> Crafting our own reports here, like we do with SDL or evdev backends,
> completely break these games. That's why Proton is using hidraw as a
> default instead, and then why we need to do these fixups.
>
> Faking VID/PID otoh isn't great either, as we won't get the expected
> controller glyphs in game anymore, and they would display some generic
> ones, or even worse again, not support the controller at all if they
> hardcode a few VID/PID.
>
> Last, not doing any kind of fixup will just make the DS4 completely
> unusable in Wine with the hidraw backend, as Steam (I suspect it comes
> from it most of the time) is already requesting calibration and thus
> enabling the extended input reports by default. We certainly don't want
> to have vendor-specific parsing logic in DInput.
>
> > I really hate HID so bad it is such a poor abstraction and since every
> > application accesses the device directly any state is lost (e.g. for
> > our devices you can change power management, reporting speed, light
> > colors and more).
> >
>
> Sure, it's maybe hard to describe advanced data, but imho for the basic
> things it just works fine, and it's actually supposed to do so in order
> for application not to have to hardcode the packet structures. Not
> having anything described just makes third-party and application
> hardcode these structures again and lose all the flexibility HID could
> have provided.
>
>
> Overall, and trying to move forward on this I'd like to better
> understand what you are worried about exactly. Our intention here is to
> mimic Windows behavior as close as possible, regardless of what the
> Linux environment is doing around, so we intended to:
>
> 1) Have the basic input reports sent by default, translated from the
> extended ones if needed, and until they are supposed to be sent, from
> the Windows world point of view.
>
> 2) Watch and support whatever mechanism is supposed to trigger the
> extended input reports and pass them through after that point.
>
> Are you worried that 2) isn't well defined and that it would be hard to
> decide whether the extended input reports we receive are actually
> supposed to be passed through or not?
>
> In that case instead of trying to figure which report numbers matter
> here, we could just assume that any kind of output or feature reports
> seen for these devices are potentially supposed to trigger the extended
> reports, would that be better?
>
>
> Cheers,
> --
> Rémi Bernon <rbernon at codeweavers.com>

I have been thinking about it a bit more. Maybe the hack if paired
with the feature report 5 filter is not too bad.

Though be aware that this fixup would also need to be done probably
for other platforms e.g. Mac. I do not know what applications all
support DS4/DualSense there. I assume at least SIE's RemotePlay app,
but probably other applications as well. I do not know what level of
support there is in native OSX, but there might be some (as Apple did
implement a lot of code for our controllers in iOS).

Thanks,
Roderick