[PATCH v3 0/2] MR337: winepulse: Return device-specific values for GetMixFormat and GetDevicePeriod
When `GetMixFormat` or `GetDevicePeriod` are used, return values specific to the selected device instead of those of the default one. This is especially useful when the audio devices features less channels than one specifically selected by the application. This patch is part of an attempt to get Sony's DualSense controllers to work with Windows games through Wine. This is not enough by itself, but getting the proper mix format is required for most of those games to correctly use the haptic feedback features of the DualSense (which go through an audio output with 4 audio devices). -- v3: winepulse: Return device-specific values for GetMixFormat and GetDevicePeriod https://gitlab.winehq.org/wine/wine/-/merge_requests/337
From: Claire Girka <claire(a)sitedethib.com> In addition to those of the default device, also store device-specific format and periods so that they can be returned on GetMixFormat and GetPeriod calls. --- dlls/winepulse.drv/pulse.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/dlls/winepulse.drv/pulse.c b/dlls/winepulse.drv/pulse.c index 42d73db45f9..1701b9c281c 100644 --- a/dlls/winepulse.drv/pulse.c +++ b/dlls/winepulse.drv/pulse.c @@ -89,6 +89,8 @@ typedef struct _PhysDevice { EndpointFormFactor form; UINT channel_mask; UINT index; + REFERENCE_TIME min_period, def_period; + WAVEFORMATEXTENSIBLE fmt; char pulse_name[0]; } PhysDevice; @@ -659,7 +661,7 @@ static void convert_channel_map(const pa_channel_map *pa_map, WAVEFORMATEXTENSIB fmt->dwChannelMask = pa_mask; } -static void pulse_probe_settings(int render, WAVEFORMATEXTENSIBLE *fmt) { +static void pulse_probe_settings(int render, WAVEFORMATEXTENSIBLE *fmt, REFERENCE_TIME *def_period, REFERENCE_TIME *min_period, CHAR *pulse_name) { WAVEFORMATEX *wfx = &fmt->Format; pa_stream *stream; pa_channel_map map; @@ -684,10 +686,10 @@ static void pulse_probe_settings(int render, WAVEFORMATEXTENSIBLE *fmt) { if (!stream) ret = -1; else if (render) - ret = pa_stream_connect_playback(stream, NULL, &attr, + ret = pa_stream_connect_playback(stream, pulse_name, &attr, PA_STREAM_START_CORKED|PA_STREAM_FIX_RATE|PA_STREAM_FIX_CHANNELS|PA_STREAM_EARLY_REQUESTS, NULL, NULL); else - ret = pa_stream_connect_record(stream, NULL, &attr, PA_STREAM_START_CORKED|PA_STREAM_FIX_RATE|PA_STREAM_FIX_CHANNELS|PA_STREAM_EARLY_REQUESTS); + ret = pa_stream_connect_record(stream, pulse_name, &attr, PA_STREAM_START_CORKED|PA_STREAM_FIX_RATE|PA_STREAM_FIX_CHANNELS|PA_STREAM_EARLY_REQUESTS); if (ret >= 0) { while (pa_mainloop_iterate(pulse_ml, 1, &ret) >= 0 && pa_stream_get_state(stream) == PA_STREAM_CREATING) @@ -710,13 +712,13 @@ static void pulse_probe_settings(int render, WAVEFORMATEXTENSIBLE *fmt) { pa_stream_unref(stream); if (length) - pulse_def_period[!render] = pulse_min_period[!render] = pa_bytes_to_usec(10 * length, &ss); + *def_period = *min_period = pa_bytes_to_usec(10 * length, &ss); - if (pulse_min_period[!render] < MinimumPeriod) - pulse_min_period[!render] = MinimumPeriod; + if (*min_period < MinimumPeriod) + *min_period = MinimumPeriod; - if (pulse_def_period[!render] < DefaultPeriod) - pulse_def_period[!render] = DefaultPeriod; + if (*def_period < DefaultPeriod) + *def_period = DefaultPeriod; wfx->wFormatTag = WAVE_FORMAT_EXTENSIBLE; wfx->cbSize = sizeof(WAVEFORMATEXTENSIBLE) - sizeof(WAVEFORMATEX); @@ -745,6 +747,7 @@ static NTSTATUS pulse_test_connect(void *args) { struct test_connect_params *params = args; struct pulse_config *config = params->config; + PhysDevice *dev; pa_operation *o; int ret; @@ -787,8 +790,8 @@ static NTSTATUS pulse_test_connect(void *args) pa_context_get_server(pulse_ctx), pa_context_get_server_protocol_version(pulse_ctx)); - pulse_probe_settings(1, &pulse_fmt[0]); - pulse_probe_settings(0, &pulse_fmt[1]); + pulse_probe_settings(1, &pulse_fmt[0], &pulse_def_period[0], &pulse_min_period[0], NULL); + pulse_probe_settings(0, &pulse_fmt[1], &pulse_def_period[1], &pulse_min_period[1], NULL); free_phys_device_lists(); list_init(&g_phys_speakers); @@ -813,6 +816,14 @@ static NTSTATUS pulse_test_connect(void *args) pa_operation_unref(o); } + LIST_FOR_EACH_ENTRY(dev, &g_phys_speakers, PhysDevice, entry) { + pulse_probe_settings(1, &dev->fmt, &dev->def_period, &dev->min_period, dev->pulse_name); + } + + LIST_FOR_EACH_ENTRY(dev, &g_phys_sources, PhysDevice, entry) { + pulse_probe_settings(0, &dev->fmt, &dev->def_period, &dev->min_period, dev->pulse_name); + } + pa_context_unref(pulse_ctx); pulse_ctx = NULL; pa_mainloop_free(pulse_ml); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/337
From: Claire Girka <claire(a)sitedethib.com> When GetMixFormat or GetDevicePeriod is used, return values specific to the selected device instead of those of the default one. This is especially useful when the default audio device features less channels than one specifically selected by the application. --- dlls/winepulse.drv/mmdevdrv.c | 32 +++++++++++++++++++++++++++++++- dlls/winepulse.drv/pulse.c | 23 +++++++++++++++++++++++ dlls/winepulse.drv/unixlib.h | 11 +++++++++++ 3 files changed, 65 insertions(+), 1 deletion(-) diff --git a/dlls/winepulse.drv/mmdevdrv.c b/dlls/winepulse.drv/mmdevdrv.c index 3cbbc1d8115..a300ec517c4 100644 --- a/dlls/winepulse.drv/mmdevdrv.c +++ b/dlls/winepulse.drv/mmdevdrv.c @@ -1136,13 +1136,27 @@ static HRESULT WINAPI AudioClient_GetMixFormat(IAudioClient3 *iface, WAVEFORMATEX **pwfx) { ACImpl *This = impl_from_IAudioClient3(iface); + const WAVEFORMATEX *fmt; TRACE("(%p)->(%p)\n", This, pwfx); if (!pwfx) return E_POINTER; - *pwfx = clone_format(&pulse_config.modes[This->dataflow == eCapture].format.Format); + fmt = &pulse_config.modes[This->dataflow == eCapture].format.Format; + + if (This->pulse_name[0]) { + struct get_device_settings_params params; + params.render = This->dataflow == eRender; + params.pulse_name = This->pulse_name; + pulse_call(get_device_settings, ¶ms); + + if (SUCCEEDED(params.result)) + fmt = ¶ms.fmt.Format; + } + + *pwfx = clone_format(fmt); + if (!*pwfx) return E_OUTOFMEMORY; dump_fmt(*pwfx); @@ -1159,6 +1173,22 @@ static HRESULT WINAPI AudioClient_GetDevicePeriod(IAudioClient3 *iface, if (!defperiod && !minperiod) return E_POINTER; + if (This->pulse_name[0]) { + struct get_device_settings_params params; + params.render = This->dataflow == eRender; + params.pulse_name = This->pulse_name; + pulse_call(get_device_settings, ¶ms); + + if (SUCCEEDED(params.result)) { + if (defperiod) + *defperiod = params.def_period; + if (minperiod) + *minperiod = params.min_period; + + return S_OK; + } + } + if (defperiod) *defperiod = pulse_config.modes[This->dataflow == eCapture].def_period; if (minperiod) diff --git a/dlls/winepulse.drv/pulse.c b/dlls/winepulse.drv/pulse.c index 1701b9c281c..4e7703c4128 100644 --- a/dlls/winepulse.drv/pulse.c +++ b/dlls/winepulse.drv/pulse.c @@ -739,6 +739,28 @@ static void pulse_probe_settings(int render, WAVEFORMATEXTENSIBLE *fmt, REFERENC fmt->SubFormat = KSDATAFORMAT_SUBTYPE_PCM; } +static NTSTATUS pulse_get_device_settings(void *args) +{ + struct get_device_settings_params *params = args; + struct list *list = params->render ? &g_phys_speakers : &g_phys_sources; + PhysDevice *dev; + + LIST_FOR_EACH_ENTRY(dev, list, PhysDevice, entry) { + if (strcmp(params->pulse_name, dev->pulse_name)) + continue; + + params->fmt = dev->fmt; + params->def_period = dev->def_period; + params->min_period = dev->min_period; + params->result = S_OK; + + return STATUS_SUCCESS; + } + + params->result = E_FAIL; + return STATUS_SUCCESS; +} + /* some poorly-behaved applications call audio functions during DllMain, so we * have to do as much as possible without creating a new thread. this function * sets up a synchronous connection to verify the server is running and query @@ -2316,6 +2338,7 @@ const unixlib_entry_t __wine_unix_call_funcs[] = pulse_test_connect, pulse_is_started, pulse_get_prop_value, + pulse_get_device_settings, }; #ifdef _WIN64 diff --git a/dlls/winepulse.drv/unixlib.h b/dlls/winepulse.drv/unixlib.h index f224f26c909..0c340e601d6 100644 --- a/dlls/winepulse.drv/unixlib.h +++ b/dlls/winepulse.drv/unixlib.h @@ -159,6 +159,16 @@ struct get_current_padding_params UINT32 *padding; }; +struct get_device_settings_params +{ + const char *pulse_name; + HRESULT result; + BOOL render; + WAVEFORMATEXTENSIBLE fmt; + REFERENCE_TIME def_period; + REFERENCE_TIME min_period; +}; + struct get_next_packet_size_params { stream_handle stream; @@ -252,4 +262,5 @@ enum unix_funcs test_connect, is_started, get_prop_value, + get_device_settings, }; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/337
Current version (1e8b1b3195c832aafa4e6c2ab37d6d5b24e7b317) is failing to build: ``` /home/aeikum/src/wine/dlls/winepulse.drv/pulse.c: In function ‘pulse_get_device_settings’: /home/aeikum/src/wine/dlls/winepulse.drv/pulse.c:750:26: error: ‘PhysDevice’ {aka ‘struct _PhysDevice’} has no member named ‘fmt’ 750 | params->fmt = dev->fmt; | ^~ /home/aeikum/src/wine/dlls/winepulse.drv/pulse.c:751:33: error: ‘PhysDevice’ {aka ‘struct _PhysDevice’} has no member named ‘def_period’ 751 | params->def_period = dev->def_period; | ^~ /home/aeikum/src/wine/dlls/winepulse.drv/pulse.c:752:33: error: ‘PhysDevice’ {aka ‘struct _PhysDevice’} has no member named ‘min_period’ 752 | params->min_period = dev->min_period; | ^~ make[1]: *** [Makefile:231696: dlls/winepulse.drv/pulse.o] Error 1 ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/337#note_3044
On Thu Jun 30 13:49:45 2022 +0000, Andrew Eikum wrote:
Current version (1e8b1b3195c832aafa4e6c2ab37d6d5b24e7b317) is failing to build: ``` /home/aeikum/src/wine/dlls/winepulse.drv/pulse.c: In function ‘pulse_get_device_settings’: /home/aeikum/src/wine/dlls/winepulse.drv/pulse.c:750:26: error: ‘PhysDevice’ {aka ‘struct _PhysDevice’} has no member named ‘fmt’ 750 | params->fmt = dev->fmt; | ^~ /home/aeikum/src/wine/dlls/winepulse.drv/pulse.c:751:33: error: ‘PhysDevice’ {aka ‘struct _PhysDevice’} has no member named ‘def_period’ 751 | params->def_period = dev->def_period; | ^~ /home/aeikum/src/wine/dlls/winepulse.drv/pulse.c:752:33: error: ‘PhysDevice’ {aka ‘struct _PhysDevice’} has no member named ‘min_period’ 752 | params->min_period = dev->min_period; | ^~ make[1]: *** [Makefile:231696: dlls/winepulse.drv/pulse.o] Error 1 ``` Woops, my mistake. Missed the first commit. Still getting used to this gitlab thing :)
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/337#note_3045
This series looks fine to me. @huw should review it, too. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/337#note_3056
This merge request was approved by Andrew Eikum. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/337
I’m still not convinced we need two more unix calls, but I’m looking at this on a phone while out this week, but perhaps I’ll be more persuaded when I see the changes properly next week. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/337#note_3067
On Mon Jul 4 07:52:00 2022 +0000, Claire wrote:
I'm not sure using a `BOOL device` field is really an option, since there is currently no existing unix call to get those values, they are all computed in the one-shot `pulse_test_connect` then retrieved from a global object. With regards to splitting this into multiple commits, should I force-push to this branch, or make a new Merge Request? Ah right, in the pulse driver we cache these at the start. I'd suggest making this more like the alsa driver, where we unconditionally call into the unixlib for both `GetMixFormat()` and `GetDevicePeriod()`.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/337#note_3186
Huw Davies (@huw) commented about dlls/winepulse.drv/pulse.c:
fmt->dwChannelMask = pa_mask; }
-static void pulse_probe_settings(int render, WAVEFORMATEXTENSIBLE *fmt) { +static void pulse_probe_settings(int render, WAVEFORMATEXTENSIBLE *fmt, REFERENCE_TIME *def_period, REFERENCE_TIME *min_period, CHAR *pulse_name) {
If you end up using this in the next version, then please use something like: ```suggestion:-0+0 static void pulse_probe_settings(int render, const char *pulse_name, WAVEFORMATEXTENSIBLE *fmt, REFERENCE_TIME *def_period, REFERENCE_TIME *min_period) { ``` so that the [out] parameters are at the end and let's put that opening brace on a new line. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/337#note_3187
On Mon Jul 4 07:52:00 2022 +0000, Huw Davies wrote:
Ah right, in the pulse driver we cache these at the start. I'd suggest making this more like the alsa driver, where we unconditionally call into the unixlib for both `GetMixFormat()` and `GetDevicePeriod()`. You mean moving the `pulse_name[0]` check to the unixlib, or probing the device each time? If the latter, I am not sure how to do that, as the PulseAudio APIs are asynchronous and the GetMixFormat/GetDevicePeriod APIs are synchronous.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/337#note_3188
On Mon Jul 4 08:28:46 2022 +0000, Claire wrote:
You mean moving the `pulse_name[0]` check to the unixlib, or probing the device each time? If the latter, I am not sure how to do that, as the PulseAudio APIs are asynchronous and the GetMixFormat/GetDevicePeriod APIs are synchronous. The former. Keep the first patch in the series, then add a `get_mix_format` unixcall that `GetMixFormat()` always calls and use `pulse_name` to determine the correct values in the unixlib. Then do a similar thing for `GetDevicePeriod()`.
Note, when you add or change a unixcall, you also need to add (or change) the appropriate wow64 handler (see e.g. `wow64_get_mix_format()` in the alsa driver. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/337#note_3189
participants (4)
-
Andrew Eikum (＠aeikum) -
Claire (＠ClearlyClaire)
-
Claire Girka
-
Huw Davies (＠huw)