[PATCH 1/2] mmdevapi: Truncate strings from PropertyStore's GetValue to 62 chars.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53010 Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- Not sure if this is the best solution, but... dlls/mmdevapi/devenum.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/dlls/mmdevapi/devenum.c b/dlls/mmdevapi/devenum.c index ed1daa8..f5799be 100644 --- a/dlls/mmdevapi/devenum.c +++ b/dlls/mmdevapi/devenum.c @@ -1378,6 +1378,8 @@ static HRESULT WINAPI MMDevPropStore_GetAt(IPropertyStore *iface, DWORD prop, PR static HRESULT WINAPI MMDevPropStore_GetValue(IPropertyStore *iface, REFPROPERTYKEY key, PROPVARIANT *pv) { MMDevPropStore *This = impl_from_IPropertyStore(iface); + HRESULT hres; + TRACE("(%p)->(\"%s,%lu\", %p)\n", This, key ? debugstr_guid(&key->fmtid) : NULL, key ? key->pid : 0, pv); if (!key || !pv) @@ -1397,7 +1399,16 @@ static HRESULT WINAPI MMDevPropStore_GetValue(IPropertyStore *iface, REFPROPERTY return S_OK; } - return MMDevice_GetPropValue(&This->parent->devguid, This->parent->flow, key, pv); + hres = MMDevice_GetPropValue(&This->parent->devguid, This->parent->flow, key, pv); + if (FAILED(hres)) + return hres; + + /* Some broken apps and libs (e.g. Split/Second with fmodex) + can't deal with strings longer than 62 characters + NUL. */ + if (pv->vt == VT_LPWSTR && wcslen(pv->pwszVal) > 62) + pv->pwszVal[62] = '\0'; + + return hres; } static HRESULT WINAPI MMDevPropStore_SetValue(IPropertyStore *iface, REFPROPERTYKEY key, REFPROPVARIANT pv) -- 2.34.1
Signed-off-by: Gabriel Ivăncescu <gabrielopcode(a)gmail.com> --- The rest of the code path assumes it's uninitialized since it doesn't clear it first. Furthermore, the variant from MMDevPropStore_GetValue is passed directly to here, and that one is called from 3rd party code, not wine's. dlls/mmdevapi/devenum.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/mmdevapi/devenum.c b/dlls/mmdevapi/devenum.c index f5799be..7767966 100644 --- a/dlls/mmdevapi/devenum.c +++ b/dlls/mmdevapi/devenum.c @@ -150,7 +150,7 @@ static HRESULT MMDevice_GetPropValue(const GUID *devguid, DWORD flow, REFPROPERT { WARN("Reading %s returned %ld\n", debugstr_w(buffer), ret); RegCloseKey(regkey); - PropVariantClear(pv); + pv->vt = VT_EMPTY; return S_OK; } -- 2.34.1
On Fri, May 20, 2022 at 08:40:18PM +0300, Gabriel Ivăncescu wrote:
@@ -1397,7 +1399,16 @@ static HRESULT WINAPI MMDevPropStore_GetValue(IPropertyStore *iface, REFPROPERTY return S_OK; }
- return MMDevice_GetPropValue(&This->parent->devguid, This->parent->flow, key, pv); + hres = MMDevice_GetPropValue(&This->parent->devguid, This->parent->flow, key, pv); + if (FAILED(hres)) + return hres; + + /* Some broken apps and libs (e.g. Split/Second with fmodex) + can't deal with strings longer than 62 characters + NUL. */ + if (pv->vt == VT_LPWSTR && wcslen(pv->pwszVal) > 62) + pv->pwszVal[62] = '\0'; +
I think we'd only want to do this for certain keys, not every returned string. But, Windows doesn't do this. You can edit the registry and put whatever you want into the device name, and it'll be returned unmodified: [HKLM\Software\Microsoft\Windows\CurrentVersion\MMDevices\Audio\Render\<device_guid>\Properties] "{b3f8fa53-0004-438e-9003-51a46e139bfc},6"="A Very Long String...." I don't immediately have the ability to check if the game crashes on Windows if I set the string very long, but that's what I would check next. Andrew
On 23/05/2022 18:03, Andrew Eikum wrote:
On Fri, May 20, 2022 at 08:40:18PM +0300, Gabriel Ivăncescu wrote:
@@ -1397,7 +1399,16 @@ static HRESULT WINAPI MMDevPropStore_GetValue(IPropertyStore *iface, REFPROPERTY return S_OK; }
- return MMDevice_GetPropValue(&This->parent->devguid, This->parent->flow, key, pv); + hres = MMDevice_GetPropValue(&This->parent->devguid, This->parent->flow, key, pv); + if (FAILED(hres)) + return hres; + + /* Some broken apps and libs (e.g. Split/Second with fmodex) + can't deal with strings longer than 62 characters + NUL. */ + if (pv->vt == VT_LPWSTR && wcslen(pv->pwszVal) > 62) + pv->pwszVal[62] = '\0'; +
I think we'd only want to do this for certain keys, not every returned string. But, Windows doesn't do this. You can edit the registry and put whatever you want into the device name, and it'll be returned unmodified:
[HKLM\Software\Microsoft\Windows\CurrentVersion\MMDevices\Audio\Render\<device_guid>\Properties] "{b3f8fa53-0004-438e-9003-51a46e139bfc},6"="A Very Long String...."
I don't immediately have the ability to check if the game crashes on Windows if I set the string very long, but that's what I would check next.
Andrew
Thanks. I tested it on Windows 7 VM. Unfortunately, it crashes in the same way on Windows if I make a device name longer than 63 chars (for some reason, 63 instead of 62, but might have been a lucky run?). So yes it's true that Windows doesn't do this, but it might also be that Windows doesn't have very long device names in the first place, so it's not a problem noticed "in the wild" there. I don't really like truncation either, but since it makes the game unplayable it's kind of tough decision. Maybe use a registry setting? But then most users wouldn't know to try it...
On Mon, May 23, 2022 at 06:50:16PM +0300, Gabriel Ivăncescu wrote:
On 23/05/2022 18:03, Andrew Eikum wrote:
On Fri, May 20, 2022 at 08:40:18PM +0300, Gabriel Ivăncescu wrote:
@@ -1397,7 +1399,16 @@ static HRESULT WINAPI MMDevPropStore_GetValue(IPropertyStore *iface, REFPROPERTY return S_OK; } - return MMDevice_GetPropValue(&This->parent->devguid, This->parent->flow, key, pv); + hres = MMDevice_GetPropValue(&This->parent->devguid, This->parent->flow, key, pv); + if (FAILED(hres)) + return hres; + + /* Some broken apps and libs (e.g. Split/Second with fmodex) + can't deal with strings longer than 62 characters + NUL. */ + if (pv->vt == VT_LPWSTR && wcslen(pv->pwszVal) > 62) + pv->pwszVal[62] = '\0'; +
I think we'd only want to do this for certain keys, not every returned string. But, Windows doesn't do this. You can edit the registry and put whatever you want into the device name, and it'll be returned unmodified:
[HKLM\Software\Microsoft\Windows\CurrentVersion\MMDevices\Audio\Render\<device_guid>\Properties] "{b3f8fa53-0004-438e-9003-51a46e139bfc},6"="A Very Long String...."
I don't immediately have the ability to check if the game crashes on Windows if I set the string very long, but that's what I would check next.
Andrew
Thanks. I tested it on Windows 7 VM. Unfortunately, it crashes in the same way on Windows if I make a device name longer than 63 chars (for some reason, 63 instead of 62, but might have been a lucky run?).
So yes it's true that Windows doesn't do this, but it might also be that Windows doesn't have very long device names in the first place, so it's not a problem noticed "in the wild" there.
I don't really like truncation either, but since it makes the game unplayable it's kind of tough decision. Maybe use a registry setting? But then most users wouldn't know to try it...
We could try to acquire a shorter name for the underlying device, somehow, but I'm not sure what options PA and/or ALSA give us. Maybe use PA_PROP_DEVICE_PRODUCT_NAME instead, for example. Andrew
On 23/05/2022 19:18, Andrew Eikum wrote:
On Mon, May 23, 2022 at 06:50:16PM +0300, Gabriel Ivăncescu wrote:
On 23/05/2022 18:03, Andrew Eikum wrote:
On Fri, May 20, 2022 at 08:40:18PM +0300, Gabriel Ivăncescu wrote:
@@ -1397,7 +1399,16 @@ static HRESULT WINAPI MMDevPropStore_GetValue(IPropertyStore *iface, REFPROPERTY return S_OK; } - return MMDevice_GetPropValue(&This->parent->devguid, This->parent->flow, key, pv); + hres = MMDevice_GetPropValue(&This->parent->devguid, This->parent->flow, key, pv); + if (FAILED(hres)) + return hres; + + /* Some broken apps and libs (e.g. Split/Second with fmodex) + can't deal with strings longer than 62 characters + NUL. */ + if (pv->vt == VT_LPWSTR && wcslen(pv->pwszVal) > 62) + pv->pwszVal[62] = '\0'; +
I think we'd only want to do this for certain keys, not every returned string. But, Windows doesn't do this. You can edit the registry and put whatever you want into the device name, and it'll be returned unmodified:
[HKLM\Software\Microsoft\Windows\CurrentVersion\MMDevices\Audio\Render\<device_guid>\Properties] "{b3f8fa53-0004-438e-9003-51a46e139bfc},6"="A Very Long String...."
I don't immediately have the ability to check if the game crashes on Windows if I set the string very long, but that's what I would check next.
Andrew
Thanks. I tested it on Windows 7 VM. Unfortunately, it crashes in the same way on Windows if I make a device name longer than 63 chars (for some reason, 63 instead of 62, but might have been a lucky run?).
So yes it's true that Windows doesn't do this, but it might also be that Windows doesn't have very long device names in the first place, so it's not a problem noticed "in the wild" there.
I don't really like truncation either, but since it makes the game unplayable it's kind of tough decision. Maybe use a registry setting? But then most users wouldn't know to try it...
We could try to acquire a shorter name for the underlying device, somehow, but I'm not sure what options PA and/or ALSA give us. Maybe use PA_PROP_DEVICE_PRODUCT_NAME instead, for example.
Andrew
I experimented with a few options. Unfortunately, they don't really work. PA_PROP_DEVICE_PRODUCT_NAME is either not available in most cases, or it's the same as the description (which is what is currently used), and when it is different, the returned name is way too short (e.g. just "D30"). However the problem is that it's not available for "virtual" sources (or sinks), and in this case those are the largest offenders, e.g. "Monitor of <some really long name>" which makes the long name even longer. In the case of the linked bug, it's the builtin soundcard (on motherboard) that is too long and it seems the product name won't help there at all anyway, even without the "Monitor of" source. I was thinking of using the pulse_name in case the description is longer than 62 chars, but apparently those can get pretty long too. Though I suppose using it as a fallback should work in most cases... And pick it if it's shorter in such case. Thoughts?
On Tue, May 24, 2022 at 05:48:58PM +0300, Gabriel Ivăncescu wrote:
On 23/05/2022 19:18, Andrew Eikum wrote:
We could try to acquire a shorter name for the underlying device, somehow, but I'm not sure what options PA and/or ALSA give us. Maybe use PA_PROP_DEVICE_PRODUCT_NAME instead, for example.
I experimented with a few options. Unfortunately, they don't really work.
PA_PROP_DEVICE_PRODUCT_NAME is either not available in most cases, or it's the same as the description (which is what is currently used), and when it is different, the returned name is way too short (e.g. just "D30"). However the problem is that it's not available for "virtual" sources (or sinks), and in this case those are the largest offenders, e.g. "Monitor of <some really long name>" which makes the long name even longer.
In the case of the linked bug, it's the builtin soundcard (on motherboard) that is too long and it seems the product name won't help there at all anyway, even without the "Monitor of" source.
I was thinking of using the pulse_name in case the description is longer than 62 chars, but apparently those can get pretty long too. Though I suppose using it as a fallback should work in most cases... And pick it if it's shorter in such case.
I don't know. I don't think it's great to truncate useful information just to work around one game bug. My best idea is to try to construct some useful string out of the information we can get from Pulse. Looking at the pulse properties for the hardware I happen to have, I see: device.description = "Starship/Matisse HD Audio Controller Digital Stereo (IEC958)" This is pretty long. Here are some other names: device.product.name = "Starship/Matisse HD Audio Controller" device.profile.description = "Digital Stereo (IEC958)" alsa.card_name = "HD-Audio Generic" Maybe we could use some combination of those? Maybe choose whichever of device.product.name and alsa.card_name is shorter, and append the profile description if it doesn't become too long? As for monitoring, we can use: device.class = "monitor" to build our own "Monitor of" string. Andrew
On 24/05/2022 18:24, Andrew Eikum wrote:
On Tue, May 24, 2022 at 05:48:58PM +0300, Gabriel Ivăncescu wrote:
On 23/05/2022 19:18, Andrew Eikum wrote:
We could try to acquire a shorter name for the underlying device, somehow, but I'm not sure what options PA and/or ALSA give us. Maybe use PA_PROP_DEVICE_PRODUCT_NAME instead, for example.
I experimented with a few options. Unfortunately, they don't really work.
PA_PROP_DEVICE_PRODUCT_NAME is either not available in most cases, or it's the same as the description (which is what is currently used), and when it is different, the returned name is way too short (e.g. just "D30"). However the problem is that it's not available for "virtual" sources (or sinks), and in this case those are the largest offenders, e.g. "Monitor of <some really long name>" which makes the long name even longer.
In the case of the linked bug, it's the builtin soundcard (on motherboard) that is too long and it seems the product name won't help there at all anyway, even without the "Monitor of" source.
I was thinking of using the pulse_name in case the description is longer than 62 chars, but apparently those can get pretty long too. Though I suppose using it as a fallback should work in most cases... And pick it if it's shorter in such case.
I don't know. I don't think it's great to truncate useful information just to work around one game bug.
My best idea is to try to construct some useful string out of the information we can get from Pulse. Looking at the pulse properties for the hardware I happen to have, I see:
device.description = "Starship/Matisse HD Audio Controller Digital Stereo (IEC958)"
This is pretty long. Here are some other names:
device.product.name = "Starship/Matisse HD Audio Controller" device.profile.description = "Digital Stereo (IEC958)" alsa.card_name = "HD-Audio Generic"
Maybe we could use some combination of those? Maybe choose whichever of device.product.name and alsa.card_name is shorter, and append the profile description if it doesn't become too long?
As for monitoring, we can use:
device.class = "monitor"
to build our own "Monitor of" string.
Andrew
Interesting. I'll probably try something that builds it incrementally, and stop if any stage makes it > 62 chars and just return what I built until then. First I'll try the entire description though (like now), and only fall back to this if it's > 62 chars (this is also the only way to build it for virtual sinks that only give description).
participants (2)
-
Andrew Eikum -
Gabriel Ivăncescu