[PATCH v10 0/7] 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). -- v10: 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: Remove unused pulse_config winepulse.drv: do not fallback to global periods in pulse_get_device_period winepulse.drv: do not fallback to global pulse_fmt in pulse_get_mix_format winepulse: Store device-specific format and periods 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 | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/dlls/winepulse.drv/pulse.c b/dlls/winepulse.drv/pulse.c index 42d73db45f9..04795854fbc 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; @@ -684,10 +689,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 +715,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 +750,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 +793,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 +819,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); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/337
From: Claire Girka <claire(a)sitedethib.com> pulse_name should always match a valid device, the default audio device with an empty name is one of them. --- 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 04795854fbc..4e32fa4093d 100644 --- a/dlls/winepulse.drv/pulse.c +++ b/dlls/winepulse.drv/pulse.c @@ -2029,6 +2029,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; @@ -2308,6 +2328,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, @@ -2463,6 +2484,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 @@ -2689,6 +2730,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, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/337
From: Claire Girka <claire(a)sitedethib.com> --- dlls/winepulse.drv/mmdevdrv.c | 13 +++++---- dlls/winepulse.drv/pulse.c | 52 +++++++++++++++++++++++++++++++++++ dlls/winepulse.drv/unixlib.h | 10 +++++++ 3 files changed, 70 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 4e32fa4093d..e46eb4d3b55 100644 --- a/dlls/winepulse.drv/pulse.c +++ b/dlls/winepulse.drv/pulse.c @@ -2049,6 +2049,34 @@ 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; + struct list *list = (params->flow == eRender) ? &g_phys_speakers : &g_phys_sources; + PhysDevice *dev; + + if (!params->def_period && !params->min_period) { + params->result = E_POINTER; + return STATUS_SUCCESS; + } + + LIST_FOR_EACH_ENTRY(dev, list, PhysDevice, entry) { + if (strcmp(params->pulse_name, dev->pulse_name)) + continue; + + if (params->def_period) + *params->def_period = dev->def_period; + if (params->min_period) + *params->min_period = dev->min_period; + 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; @@ -2329,6 +2357,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, @@ -2504,6 +2533,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 @@ -2731,6 +2782,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, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/337
From: Claire Girka <claire(a)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 e46eb4d3b55..920fde97e29 100644 --- a/dlls/winepulse.drv/pulse.c +++ b/dlls/winepulse.drv/pulse.c @@ -749,7 +749,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; @@ -832,13 +831,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; @@ -2710,12 +2702,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 -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/337
From: Claire Girka <claire(a)sitedethib.com> --- dlls/winepulse.drv/pulse.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/dlls/winepulse.drv/pulse.c b/dlls/winepulse.drv/pulse.c index 920fde97e29..b2d00fee3cc 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; @@ -1070,6 +1072,8 @@ static NTSTATUS pulse_create_stream(void *args) REFERENCE_TIME period, duration = params->duration; struct pulse_stream *stream; unsigned int i, bufsize_bytes; + struct list *list = (params->dataflow == eRender) ? &g_phys_speakers : &g_phys_sources; + PhysDevice *dev; HRESULT hr; pulse_lock(); @@ -1097,10 +1101,23 @@ static NTSTATUS pulse_create_stream(void *args) if (FAILED(hr)) goto exit; - period = pulse_def_period[stream->dataflow == eCapture]; + period = 0; + LIST_FOR_EACH_ENTRY(dev, list, PhysDevice, entry) { + if (strcmp(params->pulse_name, dev->pulse_name)) { + period = dev->def_period; + break; + } + } + if (!period) { + params->result = E_FAIL; + return STATUS_SUCCESS; + } + 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); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/337
From: Claire Girka <claire(a)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 b2d00fee3cc..2f36c6eda9b 100644 --- a/dlls/winepulse.drv/pulse.c +++ b/dlls/winepulse.drv/pulse.c @@ -2121,7 +2121,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; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/337
From: Claire Girka <claire(a)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 2f36c6eda9b..3449bfa13df 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); @@ -794,9 +790,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); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/337
On Tue Jul 5 11:07:26 2022 +0000, Claire wrote:
changed this line in [version 10 of the diff](/wine/wine/-/merge_requests/337/diffs?diff_id=4122&start_sha=dcf1172beeec3a3afa29372f6df94892822fb279#152f5af0ef9650fb7529b3638869ab8c7c4ff082_762_743) I believe I have addressed all these issues.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/337#note_3331
Huw Davies (@huw) commented about dlls/winepulse.drv/pulse.c:
if (FAILED(hr)) goto exit;
- period = pulse_def_period[stream->dataflow == eCapture]; + period = 0; + LIST_FOR_EACH_ENTRY(dev, list, PhysDevice, entry) { + if (strcmp(params->pulse_name, dev->pulse_name)) { + period = dev->def_period; + break;
The `if` condition needs inverting. However, it would be better to combine this logic with the implmentation of `pulse_get_device_period()`. I'd suggest having `pulse_get_device_period()` call `static HRESULT get_device_period(EDataFlow flow, const char *pulse_name, REFERENCE_TIME *def, REFERENCE_TIME *min)` which could then also be used in `create_stream()`. You'd add `get_device_period()` when you first implement `pulse_get_device_period()` and you'd put it above `create_stream()`. Also note the second and third commits still have lowercase letters starting the commit message (and are the only two with ".drv" in the prefix). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/337#note_3344
One last thing I noticed: in pulse_probe_settings, since we're passing the empty string pulse_name now for the default device, you'll have to convert it to NULL before handling it off to the pulse API. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/337#note_3347
On Tue Jul 5 13:21:23 2022 +0000, Huw Davies wrote:
The `if` condition needs inverting. However, it would be better to combine this logic with the implmentation of `pulse_get_device_period()`. I'd suggest having `pulse_get_device_period()` call `static HRESULT get_device_period(EDataFlow flow, const char *pulse_name, REFERENCE_TIME *def, REFERENCE_TIME *min)` which could then also be used in `create_stream()`. You'd add `get_device_period()` when you first implement `pulse_get_device_period()` and you'd put it above `create_stream()`. Also note the second and third commits still have lowercase letters starting the commit message (and are the only two with ".drv" in the prefix). Good catch. There's also a missing `pulse_unlock`, I'll add it.
The `get_device_period` name would clash with the `unix_funcs` enum. I could call it `_get_device_period` if that's an acceptable name. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/337#note_3348
On Tue Jul 5 13:47:44 2022 +0000, Gabriel Ivăncescu wrote:
One last thing I noticed: in pulse_probe_settings, since we're passing the empty string pulse_name now for the default device, you'll have to convert it to NULL before handling it off to the pulse API. Oh yuck, that's fragile.
Right, so probably near the start of `pulse_probe_settings()` you want ```c if (!pulse_name[0]) pulse_name = NULL; ``` -- https://gitlab.winehq.org/wine/wine/-/merge_requests/337#note_3349
On Tue Jul 5 13:47:44 2022 +0000, Huw Davies wrote:
Oh yuck, that's fragile. Right, so probably near the start of `pulse_probe_settings()` you want ```c if (!pulse_name[0]) pulse_name = NULL; ``` This does not appear to cause issues in practice, but I won't take a chance and I will apply that change.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/337#note_3350
On Tue Jul 5 13:42:13 2022 +0000, Claire wrote:
Good catch. There's also a missing `pulse_unlock`, I'll add it. The `get_device_period` name would clash with the `unix_funcs` enum. I could call it `_get_device_period` if that's an acceptable name. How about `get_device_period_helper()`?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/337#note_3351
participants (4)
-
Claire (＠ClearlyClaire) -
Claire Girka
-
Gabriel Ivăncescu
-
Huw Davies (＠huw)