[PATCH v12 0/8] 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).
-- v12: winepulse: Don't probe default devices twice winepulse: Use stream-specific period in pulse_get_latency winepulse: Store and use device period on stream creation winepulse: Fix memory leak in pulse_create_stream in some error cases winepulse: Remove unused pulse_config winepulse: Return device-specific values for GetDevicePeriod
From: Claire Girka claire@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 | 37 +++++++++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 10 deletions(-)
diff --git a/dlls/winepulse.drv/pulse.c b/dlls/winepulse.drv/pulse.c index 42d73db45f9..7535a423bee 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;
@@ -542,6 +544,8 @@ static void pulse_add_device(struct list *list, pa_proplist *proplist, int index dev->form = form; dev->index = index; dev->channel_mask = channel_mask; + dev->def_period = 0; + dev->min_period = 0; fill_device_info(dev, proplist); memcpy(dev->pulse_name, pulse_name, len + 1);
@@ -659,7 +663,8 @@ 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, const char *pulse_name, WAVEFORMATEXTENSIBLE *fmt, REFERENCE_TIME *def_period, REFERENCE_TIME *min_period) +{ WAVEFORMATEX *wfx = &fmt->Format; pa_stream *stream; pa_channel_map map; @@ -668,6 +673,9 @@ static void pulse_probe_settings(int render, WAVEFORMATEXTENSIBLE *fmt) { int ret; unsigned int length = 0;
+ if (pulse_name && !pulse_name[0]) + pulse_name = NULL; + pa_channel_map_init_auto(&map, 2, PA_CHANNEL_MAP_ALSA); ss.rate = 48000; ss.format = PA_SAMPLE_FLOAT32LE; @@ -684,10 +692,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 +718,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 +753,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 +796,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, NULL, &pulse_fmt[0], &pulse_def_period[0], &pulse_min_period[0]); + pulse_probe_settings(0, NULL, &pulse_fmt[1], &pulse_def_period[1], &pulse_min_period[1]);
free_phys_device_lists(); list_init(&g_phys_speakers); @@ -813,6 +822,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->pulse_name, &dev->fmt, &dev->def_period, &dev->min_period); + } + + LIST_FOR_EACH_ENTRY(dev, &g_phys_sources, PhysDevice, entry) { + pulse_probe_settings(0, dev->pulse_name, &dev->fmt, &dev->def_period, &dev->min_period); + } + pa_context_unref(pulse_ctx); pulse_ctx = NULL; pa_mainloop_free(pulse_ml);
From: Claire Girka claire@sitedethib.com
When GetMixFormat 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 | 21 ++++++++++++++---- dlls/winepulse.drv/pulse.c | 42 +++++++++++++++++++++++++++++++++++ dlls/winepulse.drv/unixlib.h | 9 ++++++++ 3 files changed, 68 insertions(+), 4 deletions(-)
diff --git a/dlls/winepulse.drv/mmdevdrv.c b/dlls/winepulse.drv/mmdevdrv.c index 3cbbc1d8115..61875b7352d 100644 --- a/dlls/winepulse.drv/mmdevdrv.c +++ b/dlls/winepulse.drv/mmdevdrv.c @@ -1136,17 +1136,30 @@ static HRESULT WINAPI AudioClient_GetMixFormat(IAudioClient3 *iface, WAVEFORMATEX **pwfx) { ACImpl *This = impl_from_IAudioClient3(iface); + struct get_mix_format_params params;
TRACE("(%p)->(%p)\n", This, pwfx);
if (!pwfx) return E_POINTER; + *pwfx = NULL;
- *pwfx = clone_format(&pulse_config.modes[This->dataflow == eCapture].format.Format); - if (!*pwfx) + params.pulse_name = This->pulse_name; + params.flow = This->dataflow; + params.fmt = CoTaskMemAlloc(sizeof(WAVEFORMATEXTENSIBLE)); + if (!params.fmt) return E_OUTOFMEMORY; - dump_fmt(*pwfx); - return S_OK; + + pulse_call(get_mix_format, ¶ms); + + if (SUCCEEDED(params.result)) { + *pwfx = ¶ms.fmt->Format; + dump_fmt(*pwfx); + } else { + CoTaskMemFree(params.fmt); + } + + return params.result; }
static HRESULT WINAPI AudioClient_GetDevicePeriod(IAudioClient3 *iface, diff --git a/dlls/winepulse.drv/pulse.c b/dlls/winepulse.drv/pulse.c index 7535a423bee..2e2d94d2d1f 100644 --- a/dlls/winepulse.drv/pulse.c +++ b/dlls/winepulse.drv/pulse.c @@ -2032,6 +2032,26 @@ static NTSTATUS pulse_release_capture_buffer(void *args) return STATUS_SUCCESS; }
+static NTSTATUS pulse_get_mix_format(void *args) +{ + struct get_mix_format_params *params = args; + struct list *list = (params->flow == eRender) ? &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->result = S_OK; + + return STATUS_SUCCESS; + } + + params->result = E_FAIL; + return STATUS_SUCCESS; +} + static NTSTATUS pulse_get_buffer_size(void *args) { struct get_buffer_size_params *params = args; @@ -2311,6 +2331,7 @@ const unixlib_entry_t __wine_unix_call_funcs[] = pulse_release_render_buffer, pulse_get_capture_buffer, pulse_release_capture_buffer, + pulse_get_mix_format, pulse_get_buffer_size, pulse_get_latency, pulse_get_current_padding, @@ -2466,6 +2487,26 @@ static NTSTATUS pulse_wow64_get_capture_buffer(void *args) return STATUS_SUCCESS; };
+static NTSTATUS pulse_wow64_get_mix_format(void *args) +{ + struct + { + PTR32 pulse_name; + EDataFlow flow; + PTR32 fmt; + HRESULT result; + } *params32 = args; + struct get_mix_format_params params = + { + .pulse_name = ULongToPtr(params32->pulse_name), + .flow = params32->flow, + .fmt = ULongToPtr(params32->fmt), + }; + pulse_get_mix_format(¶ms); + params32->result = params.result; + return STATUS_SUCCESS; +} + static NTSTATUS pulse_wow64_get_buffer_size(void *args) { struct @@ -2692,6 +2733,7 @@ const unixlib_entry_t __wine_unix_call_wow64_funcs[] = pulse_release_render_buffer, pulse_wow64_get_capture_buffer, pulse_release_capture_buffer, + pulse_wow64_get_mix_format, pulse_wow64_get_buffer_size, pulse_wow64_get_latency, pulse_wow64_get_current_padding, diff --git a/dlls/winepulse.drv/unixlib.h b/dlls/winepulse.drv/unixlib.h index f224f26c909..b6a19ddeb96 100644 --- a/dlls/winepulse.drv/unixlib.h +++ b/dlls/winepulse.drv/unixlib.h @@ -138,6 +138,14 @@ struct release_capture_buffer_params HRESULT result; };
+struct get_mix_format_params +{ + const char *pulse_name; + EDataFlow flow; + WAVEFORMATEXTENSIBLE *fmt; + HRESULT result; +}; + struct get_buffer_size_params { stream_handle stream; @@ -241,6 +249,7 @@ enum unix_funcs release_render_buffer, get_capture_buffer, release_capture_buffer, + get_mix_format, get_buffer_size, get_latency, get_current_padding,
From: Claire Girka claire@sitedethib.com
--- dlls/winepulse.drv/mmdevdrv.c | 13 +++++---- dlls/winepulse.drv/pulse.c | 55 +++++++++++++++++++++++++++++++++++ dlls/winepulse.drv/unixlib.h | 10 +++++++ 3 files changed, 73 insertions(+), 5 deletions(-)
diff --git a/dlls/winepulse.drv/mmdevdrv.c b/dlls/winepulse.drv/mmdevdrv.c index 61875b7352d..6b78a56388b 100644 --- a/dlls/winepulse.drv/mmdevdrv.c +++ b/dlls/winepulse.drv/mmdevdrv.c @@ -1165,6 +1165,7 @@ static HRESULT WINAPI AudioClient_GetMixFormat(IAudioClient3 *iface, static HRESULT WINAPI AudioClient_GetDevicePeriod(IAudioClient3 *iface, REFERENCE_TIME *defperiod, REFERENCE_TIME *minperiod) { + struct get_device_period_params params; ACImpl *This = impl_from_IAudioClient3(iface);
TRACE("(%p)->(%p, %p)\n", This, defperiod, minperiod); @@ -1172,12 +1173,14 @@ static HRESULT WINAPI AudioClient_GetDevicePeriod(IAudioClient3 *iface, if (!defperiod && !minperiod) return E_POINTER;
- if (defperiod) - *defperiod = pulse_config.modes[This->dataflow == eCapture].def_period; - if (minperiod) - *minperiod = pulse_config.modes[This->dataflow == eCapture].min_period; + params.flow = This->dataflow; + params.pulse_name = This->pulse_name; + params.def_period = defperiod; + params.min_period = minperiod;
- return S_OK; + pulse_call(get_device_period, ¶ms); + + return params.result; }
static HRESULT WINAPI AudioClient_Start(IAudioClient3 *iface) diff --git a/dlls/winepulse.drv/pulse.c b/dlls/winepulse.drv/pulse.c index 2e2d94d2d1f..70a05656e5e 100644 --- a/dlls/winepulse.drv/pulse.c +++ b/dlls/winepulse.drv/pulse.c @@ -1075,6 +1075,29 @@ static ULONG_PTR zero_bits(void) #endif }
+static HRESULT get_device_period_helper(EDataFlow flow, const char *pulse_name, REFERENCE_TIME *def, REFERENCE_TIME *min) +{ + struct list *list = (flow == eRender) ? &g_phys_speakers : &g_phys_sources; + PhysDevice *dev; + + if (!def && !min) { + return E_POINTER; + } + + LIST_FOR_EACH_ENTRY(dev, list, PhysDevice, entry) { + if (strcmp(pulse_name, dev->pulse_name)) + continue; + + if (def) + *def = dev->def_period; + if (min) + *min = dev->min_period; + return S_OK; + } + + return E_FAIL; +} + static NTSTATUS pulse_create_stream(void *args) { struct create_stream_params *params = args; @@ -2052,6 +2075,14 @@ static NTSTATUS pulse_get_mix_format(void *args) return STATUS_SUCCESS; }
+static NTSTATUS pulse_get_device_period(void *args) +{ + struct get_device_period_params *params = args; + + params->result = get_device_period_helper(params->flow, params->pulse_name, params->def_period, params->min_period); + return STATUS_SUCCESS; +} + static NTSTATUS pulse_get_buffer_size(void *args) { struct get_buffer_size_params *params = args; @@ -2332,6 +2363,7 @@ const unixlib_entry_t __wine_unix_call_funcs[] = pulse_get_capture_buffer, pulse_release_capture_buffer, pulse_get_mix_format, + pulse_get_device_period, pulse_get_buffer_size, pulse_get_latency, pulse_get_current_padding, @@ -2507,6 +2539,28 @@ static NTSTATUS pulse_wow64_get_mix_format(void *args) return STATUS_SUCCESS; }
+static NTSTATUS pulse_wow64_get_device_period(void *args) +{ + struct + { + PTR32 pulse_name; + EDataFlow flow; + HRESULT result; + PTR32 def_period; + PTR32 min_period; + } *params32 = args; + struct get_device_period_params params = + { + .pulse_name = ULongToPtr(params32->pulse_name), + .flow = params32->flow, + .def_period = ULongToPtr(params32->def_period), + .min_period = ULongToPtr(params32->min_period), + }; + pulse_get_device_period(¶ms); + params32->result = params.result; + return STATUS_SUCCESS; +} + static NTSTATUS pulse_wow64_get_buffer_size(void *args) { struct @@ -2734,6 +2788,7 @@ const unixlib_entry_t __wine_unix_call_wow64_funcs[] = pulse_wow64_get_capture_buffer, pulse_release_capture_buffer, pulse_wow64_get_mix_format, + pulse_wow64_get_device_period, pulse_wow64_get_buffer_size, pulse_wow64_get_latency, pulse_wow64_get_current_padding, diff --git a/dlls/winepulse.drv/unixlib.h b/dlls/winepulse.drv/unixlib.h index b6a19ddeb96..2eebc20297c 100644 --- a/dlls/winepulse.drv/unixlib.h +++ b/dlls/winepulse.drv/unixlib.h @@ -146,6 +146,15 @@ struct get_mix_format_params HRESULT result; };
+struct get_device_period_params +{ + const char *pulse_name; + EDataFlow flow; + HRESULT result; + REFERENCE_TIME *def_period; + REFERENCE_TIME *min_period; +}; + struct get_buffer_size_params { stream_handle stream; @@ -250,6 +259,7 @@ enum unix_funcs get_capture_buffer, release_capture_buffer, get_mix_format, + get_device_period, get_buffer_size, get_latency, get_current_padding,
From: Claire Girka claire@sitedethib.com
--- dlls/winepulse.drv/mmdevdrv.c | 3 --- dlls/winepulse.drv/pulse.c | 10 ---------- dlls/winepulse.drv/unixlib.h | 11 ----------- 3 files changed, 24 deletions(-)
diff --git a/dlls/winepulse.drv/mmdevdrv.c b/dlls/winepulse.drv/mmdevdrv.c index 6b78a56388b..446354fb9a5 100644 --- a/dlls/winepulse.drv/mmdevdrv.c +++ b/dlls/winepulse.drv/mmdevdrv.c @@ -60,8 +60,6 @@ enum DriverPriority { Priority_Preferred };
-static struct pulse_config pulse_config; - static HANDLE pulse_thread; static struct list g_sessions = LIST_INIT(g_sessions); static struct list g_devices_cache = LIST_INIT(g_devices_cache); @@ -492,7 +490,6 @@ int WINAPI AUDDRV_GetPriority(void) char *name;
params.name = name = get_application_name(FALSE); - params.config = &pulse_config; pulse_call(test_connect, ¶ms); free(name); return SUCCEEDED(params.result) ? Priority_Preferred : Priority_Unavailable; diff --git a/dlls/winepulse.drv/pulse.c b/dlls/winepulse.drv/pulse.c index 70a05656e5e..a7f7cceb873 100644 --- a/dlls/winepulse.drv/pulse.c +++ b/dlls/winepulse.drv/pulse.c @@ -752,7 +752,6 @@ static void pulse_probe_settings(int render, const char *pulse_name, WAVEFORMATE 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; @@ -835,13 +834,6 @@ static NTSTATUS pulse_test_connect(void *args) pa_mainloop_free(pulse_ml); pulse_ml = NULL;
- config->modes[0].format = pulse_fmt[0]; - config->modes[0].def_period = pulse_def_period[0]; - config->modes[0].min_period = pulse_min_period[0]; - config->modes[1].format = pulse_fmt[1]; - config->modes[1].def_period = pulse_def_period[1]; - config->modes[1].min_period = pulse_min_period[1]; - pulse_unlock();
params->result = S_OK; @@ -2716,12 +2708,10 @@ static NTSTATUS pulse_wow64_test_connect(void *args) { PTR32 name; HRESULT result; - PTR32 config; } *params32 = args; struct test_connect_params params = { .name = ULongToPtr(params32->name), - .config = ULongToPtr(params32->config), /* struct pulse_config is identical */ }; pulse_test_connect(¶ms); params32->result = params.result; diff --git a/dlls/winepulse.drv/unixlib.h b/dlls/winepulse.drv/unixlib.h index 2eebc20297c..1481a5db4b8 100644 --- a/dlls/winepulse.drv/unixlib.h +++ b/dlls/winepulse.drv/unixlib.h @@ -29,16 +29,6 @@ enum phys_device_bus_type { phys_device_bus_usb };
-struct pulse_config -{ - struct - { - WAVEFORMATEXTENSIBLE format; - REFERENCE_TIME def_period; - REFERENCE_TIME min_period; - } modes[2]; -}; - struct endpoint { unsigned int name; @@ -218,7 +208,6 @@ struct test_connect_params { const char *name; HRESULT result; - struct pulse_config *config; };
struct is_started_params
From: Claire Girka claire@sitedethib.com
--- dlls/winepulse.drv/pulse.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlls/winepulse.drv/pulse.c b/dlls/winepulse.drv/pulse.c index a7f7cceb873..40f5f66c129 100644 --- a/dlls/winepulse.drv/pulse.c +++ b/dlls/winepulse.drv/pulse.c @@ -1188,8 +1188,8 @@ exit: if (stream->stream) { pa_stream_disconnect(stream->stream); pa_stream_unref(stream->stream); - free(stream); } + free(stream); }
pulse_unlock();
From: Claire Girka claire@sitedethib.com
--- dlls/winepulse.drv/pulse.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/dlls/winepulse.drv/pulse.c b/dlls/winepulse.drv/pulse.c index 40f5f66c129..caf9a1c9c51 100644 --- a/dlls/winepulse.drv/pulse.c +++ b/dlls/winepulse.drv/pulse.c @@ -57,6 +57,8 @@ struct pulse_stream HANDLE event; float vol[PA_CHANNELS_MAX];
+ REFERENCE_TIME def_period; + INT32 locked; BOOL started; SIZE_T bufsize_frames, real_bufsize_bytes, period_bytes; @@ -1123,10 +1125,16 @@ static NTSTATUS pulse_create_stream(void *args) if (FAILED(hr)) goto exit;
- period = pulse_def_period[stream->dataflow == eCapture]; + period = 0; + hr = get_device_period_helper(params->dataflow, params->pulse_name, &period, NULL); + if (FAILED(hr)) + goto exit; + if (duration < 3 * period) duration = 3 * period;
+ stream->def_period = period; + stream->period_bytes = pa_frame_size(&stream->ss) * muldiv(period, stream->ss.rate, 10000000);
stream->bufsize_frames = ceil((duration / 10000000.) * params->fmt->nSamplesPerSec);
From: Claire Girka claire@sitedethib.com
--- dlls/winepulse.drv/pulse.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/dlls/winepulse.drv/pulse.c b/dlls/winepulse.drv/pulse.c index caf9a1c9c51..097c65a282d 100644 --- a/dlls/winepulse.drv/pulse.c +++ b/dlls/winepulse.drv/pulse.c @@ -2118,7 +2118,7 @@ static NTSTATUS pulse_get_latency(void *args) lat = attr->minreq / pa_frame_size(&stream->ss); else lat = attr->fragsize / pa_frame_size(&stream->ss); - *params->latency = (lat * 10000000) / stream->ss.rate + pulse_def_period[0]; + *params->latency = (lat * 10000000) / stream->ss.rate + stream->def_period; pulse_unlock(); TRACE("Latency: %u ms\n", (unsigned)(*params->latency / 10000)); params->result = S_OK;
From: Claire Girka claire@sitedethib.com
Also drop global format and period variables --- dlls/winepulse.drv/pulse.c | 7 ------- 1 file changed, 7 deletions(-)
diff --git a/dlls/winepulse.drv/pulse.c b/dlls/winepulse.drv/pulse.c index 097c65a282d..bd918b6f260 100644 --- a/dlls/winepulse.drv/pulse.c +++ b/dlls/winepulse.drv/pulse.c @@ -99,10 +99,6 @@ typedef struct _PhysDevice { static pa_context *pulse_ctx; static pa_mainloop *pulse_ml;
-/* Mixer format + period times */ -static WAVEFORMATEXTENSIBLE pulse_fmt[2]; -static REFERENCE_TIME pulse_min_period[2], pulse_def_period[2]; - static struct list g_phys_speakers = LIST_INIT(g_phys_speakers); static struct list g_phys_sources = LIST_INIT(g_phys_sources);
@@ -797,9 +793,6 @@ 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, NULL, &pulse_fmt[0], &pulse_def_period[0], &pulse_min_period[0]); - pulse_probe_settings(0, NULL, &pulse_fmt[1], &pulse_def_period[1], &pulse_min_period[1]); - free_phys_device_lists(); list_init(&g_phys_speakers); list_init(&g_phys_sources);
Huw Davies (@huw) commented about dlls/winepulse.drv/pulse.c:
pa_context_get_server(pulse_ctx), pa_context_get_server_protocol_version(pulse_ctx));
- pulse_probe_settings(1, NULL, &pulse_fmt[0], &pulse_def_period[0], &pulse_min_period[0]);
- pulse_probe_settings(0, NULL, &pulse_fmt[1], &pulse_def_period[1], &pulse_min_period[1]);
Note sure why, but this commit is causing test failures for me: ``` tools/runtest -q -P wine -T . -M mmdevapi.dll -p dlls/mmdevapi/tests/mmdevapi_test.exe capture && touch dlls/mmdevapi/tests/capture.ok 014c:fixme:pulse:AudioClient_Initialize Unknown flags: ffffffff capture.c:187: Test failed: GetNextPacketSize 0 vs. GCP 0 capture.c:190: Test failed: Valid IAudioCaptureClient_GetBuffer returns 08890001 capture.c:259: Test failed: GetNextPacketSize 0 vs. GetDevicePeriod 441 capture.c:288: Test failed: GCP 17199 vs. BufferSize 22050 make[1]: *** [Makefile:66651: dlls/mmdevapi/tests/capture.ok] Error 4 ```
On Tue Jul 5 19:57:12 2022 +0000, Huw Davies wrote:
Note sure why, but this commit is causing test failures for me:
tools/runtest -q -P wine -T . -M mmdevapi.dll -p dlls/mmdevapi/tests/mmdevapi_test.exe capture && touch dlls/mmdevapi/tests/capture.ok 014c:fixme:pulse:AudioClient_Initialize Unknown flags: ffffffff capture.c:187: Test failed: GetNextPacketSize 0 vs. GCP 0 capture.c:190: Test failed: Valid IAudioCaptureClient_GetBuffer returns 08890001 capture.c:259: Test failed: GetNextPacketSize 0 vs. GetDevicePeriod 441 capture.c:288: Test failed: GCP 17199 vs. BufferSize 22050 make[1]: *** [Makefile:66651: dlls/mmdevapi/tests/capture.ok] Error 4
Hm, I can't replicate that, and I'm not sure what could be causing this… it is perhaps possible that the PulseAudio context/main loop/connection itself could have some lingering state between `pulse_probe_settings` calls that would make the order in which devices are probed matter?
On Tue Jul 5 20:24:29 2022 +0000, Claire wrote:
Hm, I can't replicate that, and I'm not sure what could be causing this… it is perhaps possible that the PulseAudio context/main loop/connection itself could have some lingering state between `pulse_probe_settings` calls that would make the order in which devices are probed matter?
Though the order of calls to `pulse_probe_settings` wouldn't actually change either…
On Tue Jul 5 20:34:08 2022 +0000, Claire wrote:
Though the order of calls to `pulse_probe_settings` wouldn't actually change either…
After restarting PulseAudio a bunch of times (after trying to reproduce the issue with pipewire-pulse too), it eventually ended up in a state where I got the exact same test failures. But those were reproduced on `master` too, and audio input did not work in any other application either, so while it's still possible I introduced a regression, I suspect this might just have been PulseAudio being stuck in a weird state, and might not be related to the code changes at all…
On Tue Jul 5 22:14:01 2022 +0000, Claire wrote:
After restarting PulseAudio a bunch of times (after trying to reproduce the issue with pipewire-pulse too), it eventually ended up in a state where I got the exact same test failures. But those were reproduced on `master` too, and audio input did not work in any other application either, so while it's still possible I introduced a regression, I suspect this might just have been PulseAudio being stuck in a weird state, and might not be related to the code changes at all…
It's definitely this patch that causes a regression for me. Specifically, removal of only the second `pulse_probe_settings()` call is enough to trigger it.
On Wed Jul 6 06:17:09 2022 +0000, Huw Davies wrote:
It's definitely this patch that causes a regression for me. Specifically, removal of only the second `pulse_probe_settings()` call is enough to trigger it.
ok… that is definitely weird, and I'm not sure how that could happen. Are you using PulseAudio our pipewire-pulse? Could you apply something like the following and see if/how the output differs when doing the second `pulse_probe_settings` vs not doing it?
```patch diff --git a/dlls/winepulse.drv/pulse.c b/dlls/winepulse.drv/pulse.c index bd918b6f260..f3cf3a2c012 100644 --- a/dlls/winepulse.drv/pulse.c +++ b/dlls/winepulse.drv/pulse.c @@ -699,12 +699,20 @@ static void pulse_probe_settings(int render, const char *pulse_name, WAVEFORMATE pa_stream_get_state(stream) == PA_STREAM_CREATING) {} if (pa_stream_get_state(stream) == PA_STREAM_READY) { + if (!pulse_name) + FIXME("Stream ready (render? %d): %s\n", render, pa_stream_get_device_name(stream)); ss = *pa_stream_get_sample_spec(stream); map = *pa_stream_get_channel_map(stream); if (render) length = pa_stream_get_buffer_attr(stream)->minreq; else length = pa_stream_get_buffer_attr(stream)->fragsize; + if (!pulse_name) { + FIXME(" Sample format (size, rate, format): %ld, %u\n", pa_sample_size_of_format(ss.format), ss.rate); + FIXME(" Nb channels: %d\n", map.channels); + FIXME(" Period (bytes): %u\n", length); + FIXME(" Def period: %ld\n", pa_bytes_to_usec(10 * length, &ss)); + } pa_stream_disconnect(stream); while (pa_mainloop_iterate(pulse_ml, 1, &ret) >= 0 && pa_stream_get_state(stream) == PA_STREAM_READY) ```
Hi,
It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated.
The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=118336
Your paranoid android.
=== debian11 (build log) ===
../wine/dlls/winepulse.drv/pulse.c:696:18: error: ‘pulse_name’ undeclared (first use in this function); did you mean ‘pulse_fmt’? Task: The win32 Wine build failed
=== debian11 (build log) ===
../wine/dlls/winepulse.drv/pulse.c:696:18: error: ‘pulse_name’ undeclared (first use in this function); did you mean ‘pulse_fmt’? Task: The wow64 Wine build failed
On Wed Jul 6 06:50:31 2022 +0000, **** wrote:
Marvin replied on the mailing list:
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=118336 Your paranoid android. === debian11 (build log) === ../wine/dlls/winepulse.drv/pulse.c:696:18: error: ‘pulse_name’ undeclared (first use in this function); did you mean ‘pulse_fmt’? Task: The win32 Wine build failed === debian11 (build log) === ../wine/dlls/winepulse.drv/pulse.c:696:18: error: ‘pulse_name’ undeclared (first use in this function); did you mean ‘pulse_fmt’? Task: The wow64 Wine build failed
So, from your last comment, probing the default audio source (even when not doing anything with the result…) a first time before enumerating devices and probing the audio sinks works, but if the first probe of the default source happens after enumerating devices and probing the discovered sinks, it fails.
I cannot reproduce this issue, and I don't have a definite explanation on what may be going on.
My only guesses are that either the following happens: 1. probing sinks alters the way in which PulseAudio will subsequently chose a source when no device name is specified (the documentation states “Name of the source to connect to, or NULL to let the server decide”, it does not explain how PulseAudio selects the source, and https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/issues/767 suggests it's more complicated) 2. probing one of the sinks causes subsequent issues when probing other devices, and the error handling is lacking
For these reasons, I have chosen to special-case the probing of the default sink and source to be performed before any enumeration and probing of other devices. I have also added error logging in case that change would obscure errors when probing other devices.
It may also make sense considering using the `@DEFAULT_SINK@` and `@DEFAULT_SOURCE` names instead of blank names, though this would require more investigation and if possible i would like to defer that to another PR.
-
Claire (＠ClearlyClaire) -
Claire Girka
-
Huw Davies (＠huw)
-
Marvin