[PATCH v7 0/4] MR359: mmdevapi: copy ContainerID property from audio driver if available

[Claire (＠ClearlyClaire)]

Some games with support for the haptic feedback and speaker features of the Sony DualSense controller select the controller's audio output by filtering on the ContainerId IMMDevice property to find one that matches the controller's HID's. This MR allows this information to be accessible from the IMMDevice's property store when the audio driver provides it (none for now, but I intend to add that feature to `winepulse.drv` and maybe `winealsa.drv`)

This is made specific to containerId rather than supporting VT_CLSID on every property because Wine currently stores VT_BLOBs directly in the registry value, which does not allow us to safely disambiguate between VT_CLSID and VT_BLOB values when reading from registry.

--
  v7: mmdevapi: Invalidate ContainerID of unavailable audio devices.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/359

[PATCH v7 1/4] mmdevapi: support VT_CLSID for containerId property in MMDevice_SetPropValue.

[Claire Girka]

From: Claire Girka <claire(a)sitedethib.com>

CLSID is special-cased to this property because we can't safely differentiate
an encoded VT_CLSID from an encoded VT_BLOB.
---
 dlls/mmdevapi/devenum.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/dlls/mmdevapi/devenum.c b/dlls/mmdevapi/devenum.c
index bfeb3f3ecd8..48f4c736217 100644
--- a/dlls/mmdevapi/devenum.c
+++ b/dlls/mmdevapi/devenum.c
@@ -225,6 +225,18 @@ static HRESULT MMDevice_SetPropValue(const GUID *devguid, DWORD flow, REFPROPERT
             ret = RegSetValueExW(regkey, buffer, 0, REG_SZ, (const BYTE*)pv->pwszVal, sizeof(WCHAR)*(1+lstrlenW(pv->pwszVal)));
             break;
         }
+        case VT_CLSID:
+        {
+            if (IsEqualPropertyKey(*key, DEVPKEY_Device_ContainerId)) {
+                BYTE value[24] = { VT_CLSID, 0, 0, 0, 1, 0, 0, 0 };
+                memcpy(value + 8, pv->puuid, sizeof(GUID));
+
+                ret = RegSetValueExW(regkey, buffer, 0, REG_BINARY, (const BYTE*)value, 24);
+                break;
+            }
+            /* If it's not containerId, fall through the default unsupported case as we can't
+               ensure it will be decoded as CLSID. */
+        }
         default:
             ret = 0;
             FIXME("Unhandled type %u\n", pv->vt);
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/359

[PATCH v7 2/4] mmdevapi: decode ContainerId property to CLSID in MMDevice_GetPropValue.

[Claire Girka]

From: Claire Girka <claire(a)sitedethib.com>

---
 dlls/mmdevapi/devenum.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/dlls/mmdevapi/devenum.c b/dlls/mmdevapi/devenum.c
index 48f4c736217..10bf76057ee 100644
--- a/dlls/mmdevapi/devenum.c
+++ b/dlls/mmdevapi/devenum.c
@@ -189,6 +189,21 @@ static HRESULT MMDevice_GetPropValue(const GUID *devguid, DWORD flow, REFPROPERT
             break;
     }
     RegCloseKey(regkey);
+
+    /* Special case ContainerID as CLSID */
+    if(pv->vt == VT_BLOB && pv->blob.pBlobData && pv->blob.cbSize == 24 && pv->blob.pBlobData[0] == VT_CLSID && IsEqualPropertyKey(*key, DEVPKEY_Device_ContainerId)) {
+        GUID *guid = CoTaskMemAlloc(sizeof(GUID));
+        if (!guid) {
+            PropVariantClear(pv);
+            hr = E_OUTOFMEMORY;
+        } else {
+            memcpy(guid, pv->blob.pBlobData + 8, sizeof(GUID));
+            CoTaskMemFree(pv->blob.pBlobData);
+            pv->vt = VT_CLSID;
+            pv->puuid = guid;
+        }
+    }
+
     return hr;
 }
 
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/359

[PATCH v7 3/4] mmdevapi: copy ContainerID from audio driver if available.

[Claire Girka]

From: Claire Girka <claire(a)sitedethib.com>

Some games with support for the haptic feedback and speaker features of the
Sony DualSense controller select the controller's audio output by filtering on
the ContainerId IMMDevice property to find one that matches the controller's
HID's.
---
 dlls/mmdevapi/devenum.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/dlls/mmdevapi/devenum.c b/dlls/mmdevapi/devenum.c
index 10bf76057ee..bf0fc4728d1 100644
--- a/dlls/mmdevapi/devenum.c
+++ b/dlls/mmdevapi/devenum.c
@@ -391,6 +391,8 @@ static MMDevice *MMDevice_Create(WCHAR *name, GUID *id, EDataFlow flow, DWORD st
             MMDevice_SetPropValue(id, flow, (const PROPERTYKEY*)&DEVPKEY_DeviceInterface_FriendlyName, &pv);
             MMDevice_SetPropValue(id, flow, (const PROPERTYKEY*)&DEVPKEY_Device_DeviceDesc, &pv);
 
+            set_driver_prop_value(id, flow, (const PROPERTYKEY*)&DEVPKEY_Device_ContainerId);
+
             pv.pwszVal = guidstr;
             MMDevice_SetPropValue(id, flow, &deviceinterface_key, &pv);
 
-- 
GitLab


https://gitlab.winehq.org/wine/wine/-/merge_requests/359

[PATCH v7 4/4] mmdevapi: Invalidate ContainerID of unavailable audio devices.

[Claire Girka]

From: Claire Girka <claire(a)sitedethib.com>

Some games, like Deathloop, enumerate all unavailable audio devices, including
unavailable ones, and stop at the first one matching the ContainerID.

Depending on how ContainerIDs end up being attributed, an active device may end
up sharing a ContainerID with an unplugged device that was previously plugged
in the same physical port. Depending on the audio backend, the same audio
device plugged in the same port may be seen as a different audio device by Wine
depending on things like in which order devices were discovered.

In those cases, a game might find an inactive matching device before the active
one, and thus fail to open the audio output.

By invalidating the ContainerID of unavailable devices, we this issue can be
avoided.
---
 dlls/mmdevapi/devenum.c | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/dlls/mmdevapi/devenum.c b/dlls/mmdevapi/devenum.c
index bf0fc4728d1..09fb3c44875 100644
--- a/dlls/mmdevapi/devenum.c
+++ b/dlls/mmdevapi/devenum.c
@@ -207,6 +207,26 @@ static HRESULT MMDevice_GetPropValue(const GUID *devguid, DWORD flow, REFPROPERT
     return hr;
 }
 
+static HRESULT MMDevice_DeletePropValue(const GUID *devguid, DWORD flow, REFPROPERTYKEY key)
+{
+    WCHAR buffer[80];
+    const GUID *id = &key->fmtid;
+    HRESULT hr;
+    HKEY regkey;
+    LONG ret;
+
+    hr = MMDevPropStore_OpenPropKey(devguid, flow, ®key);
+    if (FAILED(hr))
+        return hr;
+    wsprintfW( buffer, propkey_formatW, id->Data1, id->Data2, id->Data3,
+               id->Data4[0], id->Data4[1], id->Data4[2], id->Data4[3],
+               id->Data4[4], id->Data4[5], id->Data4[6], id->Data4[7], key->pid );
+    ret = RegDeleteValueW(regkey, buffer);
+    RegCloseKey(regkey);
+    TRACE("Deleting %s returned %lu\n", debugstr_w(buffer), ret);
+    return hr;
+}
+
 static HRESULT MMDevice_SetPropValue(const GUID *devguid, DWORD flow, REFPROPERTYKEY key, REFPROPVARIANT pv)
 {
     WCHAR buffer[80];
@@ -391,7 +411,14 @@ static MMDevice *MMDevice_Create(WCHAR *name, GUID *id, EDataFlow flow, DWORD st
             MMDevice_SetPropValue(id, flow, (const PROPERTYKEY*)&DEVPKEY_DeviceInterface_FriendlyName, &pv);
             MMDevice_SetPropValue(id, flow, (const PROPERTYKEY*)&DEVPKEY_Device_DeviceDesc, &pv);
 
-            set_driver_prop_value(id, flow, (const PROPERTYKEY*)&DEVPKEY_Device_ContainerId);
+            /* The mechanism we use to attribute Container IDs is not very robust and could end up making
+               an active device share a ContainerID with inactive devices, and some games enumerate even
+               inactive devices, stopping at the first matching one.
+               To avoid issues, invalidate the ContainerID of devices that are not present. */
+            if (state & DEVICE_STATE_ACTIVE)
+                set_driver_prop_value(id, flow, (const PROPERTYKEY*)&DEVPKEY_Device_ContainerId);
+            else if (state & DEVICE_STATE_NOTPRESENT)
+                MMDevice_DeletePropValue(id, flow, (const PROPERTYKEY*)&DEVPKEY_Device_ContainerId);
 
             pv.pwszVal = guidstr;
             MMDevice_SetPropValue(id, flow, &deviceinterface_key, &pv);
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/359

[Olivia/Isabelle Mahadevan (＠doubledge-spec)]

This seems nifty, any updates? @huw

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/359#note_26478

[Huw Davies (＠huw)]

The issue here is that Wine isn't currently saving binary data in the correct format (it looks like the data should be prefixed by the appropriate VT_ type and possibly a count value).  So the problem becomes how to fix the format while updating any existing saved data.

Special casing one specific property (as this MR does) isn't the right way to do this.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/359#note_26622

[Huw Davies (＠huw)]

> (it looks like the data should be prefixed by the appropriate VT\_ type and possibly a count value).

Or more likely just the binary dump of the whole PROPVARIANT.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/359#note_26624

[Jinoh Kang (＠iamahuman)]

Jinoh Kang (@iamahuman) commented about dlls/mmdevapi/devenum.c:
> break;
>      }
>      RegCloseKey(regkey);
> +
> +    /* Special case ContainerID as CLSID */
> +    if(pv->vt == VT_BLOB && pv->blob.pBlobData && pv->blob.cbSize == 24 && pv->blob.pBlobData[0] == VT_CLSID && IsEqualPropertyKey(*key, DEVPKEY_Device_ContainerId)) {

Avoid hard-coding constants wherever possible.

```suggestion:-0+0
    if(pv->vt == VT_BLOB && pv->blob.pBlobData && pv->blob.cbSize == sizeof(VARIANT) && pv->blob.pBlobData[0] == VT_CLSID && IsEqualPropertyKey(*key, DEVPKEY_Device_ContainerId)) {
```

(Assuming this is VARIANT; correct me if this is not the case.)

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/359#note_87710

[Jinoh Kang (＠iamahuman)]

Jinoh Kang (@iamahuman) commented about dlls/mmdevapi/devenum.c:
> ret = RegSetValueExW(regkey, buffer, 0, REG_SZ, (const BYTE*)pv->pwszVal, sizeof(WCHAR)*(1+lstrlenW(pv->pwszVal)));
>              break;
>          }
> +        case VT_CLSID:
> +        {
> +            if (IsEqualPropertyKey(*key, DEVPKEY_Device_ContainerId)) {
> +                BYTE value[24] = { VT_CLSID, 0, 0, 0, 1, 0, 0, 0 };
> +                memcpy(value + 8, pv->puuid, sizeof(GUID));
> +
> +                ret = RegSetValueExW(regkey, buffer, 0, REG_BINARY, (const BYTE*)value, 24);

```suggestion:-0+0
                ret = RegSetValueExW(regkey, buffer, 0, REG_BINARY, (const BYTE*)value, sizeof(value));
```

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/359#note_87711

[Jinoh Kang (＠iamahuman)]

The code looks careful enough, but just like any MR that adds special cases, it would be helpful to add conformance tests that cover the newly introduced cases.

Also, I'm not particularly well versed in mmdevapi. Some maintainers may have different opinions. Thanks!

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/359#note_87712

[Elizabeth Figura (＠zfigura)]

On Sat Nov 16 11:35:05 2024 +0000, Jinoh Kang wrote:
> Avoid hard-coding constants wherever possible.
> ```suggestion:-0+0
>     if(pv->vt == VT_BLOB && pv->blob.pBlobData && pv->blob.cbSize ==
> sizeof(VARIANT) && pv->blob.pBlobData[0] == VT_CLSID &&
> IsEqualPropertyKey(*key, DEVPKEY_Device_ContainerId)) {
> ```
> (Assuming this is VARIANT; correct me if this is not the case.)
> And if that's the case, you probably want to cast pv->blob.pBlobData to a VARIANT * and check its fields.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/359#note_87721

[David Akhmedbayev (＠davidakh)]

This should get implemented Proton has had this for some time now and its fixed some major DualSense haptics issues.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/359#note_128630