[PATCH 0/4] MR5585: Arek Hiler's avatar mmdevapi: Implement AudioClockAdjustment_SetSampleRate.
This is based on work done by Andrew Eikum. It has been in some form in Proton for the last 4 years. If server's sampling rate is not 48kHz **DOOM Eternal** will try to set it 48kHz for the streams using the implemented interface. There's a whole class of audio devices that use 44.1kHz sampling rate and at least PulseAudio / PipeWire tends to inherit the value from the hardware to avoid resampling. The value can also be overridden by the user via the audio server's config files. In such cases, if the interface is not present or stubbed, this results in **audio underruns and noticeable crackling**. It's easy to test with pipewire-pulse: ``` $ cat /etc/pipewire/pipewire.conf.d/sample-rate.conf context.properties = { default.clock.rate = 41100 } ``` With PulseAudio this should be doable via setting `default-sample-rate = 41100` in `/etc/pulse/daemon.conf`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5585
From: Arkadiusz Hiler <ahiler(a)codeweavers.com> --- dlls/mmdevapi/unixlib.h | 8 ++++++++ dlls/winealsa.drv/alsa.c | 2 ++ dlls/winecoreaudio.drv/coreaudio.c | 2 ++ dlls/wineoss.drv/oss.c | 2 ++ dlls/winepulse.drv/pulse.c | 2 ++ 5 files changed, 16 insertions(+) diff --git a/dlls/mmdevapi/unixlib.h b/dlls/mmdevapi/unixlib.h index d83ed918a51..2b29f6441ce 100644 --- a/dlls/mmdevapi/unixlib.h +++ b/dlls/mmdevapi/unixlib.h @@ -216,6 +216,13 @@ struct set_event_handle_params HRESULT result; }; +struct set_sample_rate_params +{ + stream_handle stream; + float new_rate; + HRESULT result; +}; + struct test_connect_params { const WCHAR *name; @@ -323,6 +330,7 @@ enum unix_funcs get_position, set_volumes, set_event_handle, + set_sample_rate, test_connect, is_started, get_prop_value, diff --git a/dlls/winealsa.drv/alsa.c b/dlls/winealsa.drv/alsa.c index aa1ad4abe6c..919bf2b198a 100644 --- a/dlls/winealsa.drv/alsa.c +++ b/dlls/winealsa.drv/alsa.c @@ -2532,6 +2532,7 @@ const unixlib_entry_t __wine_unix_call_funcs[] = alsa_set_volumes, alsa_set_event_handle, alsa_not_implemented, + alsa_not_implemented, alsa_is_started, alsa_get_prop_value, alsa_not_implemented, @@ -2988,6 +2989,7 @@ const unixlib_entry_t __wine_unix_call_wow64_funcs[] = alsa_wow64_set_volumes, alsa_wow64_set_event_handle, alsa_not_implemented, + alsa_not_implemented, alsa_is_started, alsa_wow64_get_prop_value, alsa_not_implemented, diff --git a/dlls/winecoreaudio.drv/coreaudio.c b/dlls/winecoreaudio.drv/coreaudio.c index a79a0d16839..f7daef1ed27 100644 --- a/dlls/winecoreaudio.drv/coreaudio.c +++ b/dlls/winecoreaudio.drv/coreaudio.c @@ -1879,6 +1879,7 @@ const unixlib_entry_t __wine_unix_call_funcs[] = unix_set_volumes, unix_set_event_handle, unix_not_implemented, + unix_not_implemented, unix_is_started, unix_get_prop_value, unix_midi_init, @@ -2334,6 +2335,7 @@ const unixlib_entry_t __wine_unix_call_wow64_funcs[] = unix_wow64_set_volumes, unix_wow64_set_event_handle, unix_not_implemented, + unix_not_implemented, unix_is_started, unix_wow64_get_prop_value, unix_wow64_midi_init, diff --git a/dlls/wineoss.drv/oss.c b/dlls/wineoss.drv/oss.c index 89e626f39d6..f7396847577 100644 --- a/dlls/wineoss.drv/oss.c +++ b/dlls/wineoss.drv/oss.c @@ -1731,6 +1731,7 @@ const unixlib_entry_t __wine_unix_call_funcs[] = oss_get_position, oss_set_volumes, oss_set_event_handle, + oss_not_implemented, oss_test_connect, oss_is_started, oss_get_prop_value, @@ -2226,6 +2227,7 @@ const unixlib_entry_t __wine_unix_call_wow64_funcs[] = oss_wow64_get_position, oss_wow64_set_volumes, oss_wow64_set_event_handle, + oss_not_implemented, oss_wow64_test_connect, oss_is_started, oss_wow64_get_prop_value, diff --git a/dlls/winepulse.drv/pulse.c b/dlls/winepulse.drv/pulse.c index 62658fc98e6..328b84c7ca4 100644 --- a/dlls/winepulse.drv/pulse.c +++ b/dlls/winepulse.drv/pulse.c @@ -2562,6 +2562,7 @@ const unixlib_entry_t __wine_unix_call_funcs[] = pulse_get_position, pulse_set_volumes, pulse_set_event_handle, + pulse_not_implemented, pulse_test_connect, pulse_is_started, pulse_get_prop_value, @@ -3033,6 +3034,7 @@ const unixlib_entry_t __wine_unix_call_wow64_funcs[] = pulse_wow64_get_position, pulse_wow64_set_volumes, pulse_wow64_set_event_handle, + pulse_not_implemented, pulse_wow64_test_connect, pulse_is_started, pulse_wow64_get_prop_value, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5585
From: Andrew Eikum <aeikum(a)codeweavers.com> --- dlls/mmdevapi/client.c | 47 ++++++++++++++++++++++++++++++++++++++++ dlls/mmdevapi/mmdevdrv.h | 1 + 2 files changed, 48 insertions(+) diff --git a/dlls/mmdevapi/client.c b/dlls/mmdevapi/client.c index e0c12b8e534..1ff5aa94a79 100644 --- a/dlls/mmdevapi/client.c +++ b/dlls/mmdevapi/client.c @@ -89,6 +89,11 @@ static inline struct audio_client *impl_from_IAudioClock2(IAudioClock2 *iface) return CONTAINING_RECORD(iface, struct audio_client, IAudioClock2_iface); } +static inline ACImpl *impl_from_IAudioClockAdjustment(IAudioClockAdjustment *iface) +{ + return CONTAINING_RECORD(iface, ACImpl, IAudioClockAdjustment_iface); +} + static inline struct audio_client *impl_from_IAudioRenderClient(IAudioRenderClient *iface) { return CONTAINING_RECORD(iface, struct audio_client, IAudioRenderClient_iface); @@ -407,6 +412,8 @@ const IAudioCaptureClientVtbl AudioCaptureClient_Vtbl = static HRESULT WINAPI client_QueryInterface(IAudioClient3 *iface, REFIID riid, void **ppv) { + struct audio_client *This = impl_from_IAudioClient3(iface); + TRACE("(%p)->(%s, %p)\n", iface, debugstr_guid(riid), ppv); if (!ppv) @@ -417,6 +424,8 @@ static HRESULT WINAPI client_QueryInterface(IAudioClient3 *iface, REFIID riid, v IsEqualIID(riid, &IID_IAudioClient2) || IsEqualIID(riid, &IID_IAudioClient3)) *ppv = iface; + else if (IsEqualIID(riid, &IID_IAudioClockAdjustment)) + *ppv = &This->IAudioClockAdjustment_iface; else if(IsEqualIID(riid, &IID_IMarshal)) { struct audio_client *This = impl_from_IAudioClient3(iface); return IUnknown_QueryInterface(This->marshal, riid, ppv); @@ -1144,6 +1153,43 @@ const IAudioClock2Vtbl AudioClock2_Vtbl = clock2_GetDevicePosition }; +static HRESULT WINAPI AudioClockAdjustment_QueryInterface(IAudioClockAdjustment *iface, + REFIID riid, void **ppv) +{ + ACImpl *This = impl_from_IAudioClockAdjustment(iface); + return IAudioClock_QueryInterface(&This->IAudioClock_iface, riid, ppv); +} + +static ULONG WINAPI AudioClockAdjustment_AddRef(IAudioClockAdjustment *iface) +{ + ACImpl *This = impl_from_IAudioClockAdjustment(iface); + return IAudioClient_AddRef((IAudioClient *)&This->IAudioClient3_iface); +} + +static ULONG WINAPI AudioClockAdjustment_Release(IAudioClockAdjustment *iface) +{ + ACImpl *This = impl_from_IAudioClockAdjustment(iface); + return IAudioClient_Release((IAudioClient *)&This->IAudioClient3_iface); +} + +static HRESULT WINAPI AudioClockAdjustment_SetSampleRate(IAudioClockAdjustment *iface, + float new_rate) +{ + ACImpl *This = impl_from_IAudioClockAdjustment(iface); + + FIXME("(%p)->(%f) stub\n", This, new_rate); + + return E_NOTIMPL; +} + +const IAudioClockAdjustmentVtbl AudioClockAdjustment_Vtbl = +{ + AudioClockAdjustment_QueryInterface, + AudioClockAdjustment_AddRef, + AudioClockAdjustment_Release, + AudioClockAdjustment_SetSampleRate +}; + static HRESULT WINAPI render_QueryInterface(IAudioRenderClient *iface, REFIID riid, void **ppv) { struct audio_client *This = impl_from_IAudioRenderClient(iface); @@ -1430,6 +1476,7 @@ HRESULT AudioClient_Create(GUID *guid, IMMDevice *device, IAudioClient **out) This->IAudioClient3_iface.lpVtbl = &AudioClient3_Vtbl; This->IAudioClock_iface.lpVtbl = &AudioClock_Vtbl; This->IAudioClock2_iface.lpVtbl = &AudioClock2_Vtbl; + This->IAudioClockAdjustment_iface.lpVtbl = &AudioClockAdjustment_Vtbl; This->IAudioRenderClient_iface.lpVtbl = &AudioRenderClient_Vtbl; This->IAudioStreamVolume_iface.lpVtbl = &AudioStreamVolume_Vtbl; diff --git a/dlls/mmdevapi/mmdevdrv.h b/dlls/mmdevapi/mmdevdrv.h index df21859cbad..004de87f11c 100644 --- a/dlls/mmdevapi/mmdevdrv.h +++ b/dlls/mmdevapi/mmdevdrv.h @@ -60,6 +60,7 @@ struct audio_client { IAudioCaptureClient IAudioCaptureClient_iface; IAudioClock IAudioClock_iface; IAudioClock2 IAudioClock2_iface; + IAudioClockAdjustment IAudioClockAdjustment_iface; IAudioStreamVolume IAudioStreamVolume_iface; LONG ref; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5585
From: Arkadiusz Hiler <ahiler(a)codeweavers.com> --- dlls/mmdevapi/client.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/dlls/mmdevapi/client.c b/dlls/mmdevapi/client.c index 1ff5aa94a79..ca8adf3bf46 100644 --- a/dlls/mmdevapi/client.c +++ b/dlls/mmdevapi/client.c @@ -1176,10 +1176,19 @@ static HRESULT WINAPI AudioClockAdjustment_SetSampleRate(IAudioClockAdjustment * float new_rate) { ACImpl *This = impl_from_IAudioClockAdjustment(iface); + struct set_sample_rate_params params; - FIXME("(%p)->(%f) stub\n", This, new_rate); + TRACE("(%p)->(%f)\n", This, new_rate); - return E_NOTIMPL; + if (!This->stream) + return AUDCLNT_E_NOT_INITIALIZED; + + params.stream = This->stream; + params.new_rate = new_rate; + + wine_unix_call(set_sample_rate, ¶ms); + + return params.result; } const IAudioClockAdjustmentVtbl AudioClockAdjustment_Vtbl = -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5585
From: Arkadiusz Hiler <ahiler(a)codeweavers.com> --- dlls/winepulse.drv/pulse.c | 41 +++++++++++++++++++++++++++++++++++--- 1 file changed, 38 insertions(+), 3 deletions(-) diff --git a/dlls/winepulse.drv/pulse.c b/dlls/winepulse.drv/pulse.c index 328b84c7ca4..d72ede13909 100644 --- a/dlls/winepulse.drv/pulse.c +++ b/dlls/winepulse.drv/pulse.c @@ -723,7 +723,7 @@ static void pulse_probe_settings(int render, const char *pulse_name, WAVEFORMATE ret = -1; else if (render) 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); + PA_STREAM_START_CORKED|PA_STREAM_FIX_RATE|PA_STREAM_FIX_CHANNELS|PA_STREAM_EARLY_REQUESTS|PA_STREAM_VARIABLE_RATE, NULL, NULL); else 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) { @@ -2438,6 +2438,41 @@ static NTSTATUS pulse_set_event_handle(void *args) return STATUS_SUCCESS; } +static NTSTATUS pulse_set_sample_rate(void *args) +{ + struct set_sample_rate_params *params = args; + struct pulse_stream *stream = handle_get_stream(params->stream); + HRESULT hr = S_OK; + pa_operation *o; + int success; + + pulse_lock(); + if (!pulse_stream_valid(stream)) + hr = AUDCLNT_E_DEVICE_INVALIDATED; + else + { + if (!(o = pa_stream_update_sample_rate(stream->stream, params->new_rate, pulse_op_cb, &success))) + success = 0; + else + { + while (pa_operation_get_state(o) == PA_OPERATION_RUNNING) + pthread_cond_wait(&pulse_cond, &pulse_mutex); + pa_operation_unref(o); + } + + if (!success) hr = E_FAIL; + else + { + stream->ss.rate = params->new_rate; + stream->period_bytes = pa_frame_size(&stream->ss) * muldiv(stream->mmdev_period_usec, stream->ss.rate, 1000000); + } + } + pulse_unlock(); + + params->result = hr; + return STATUS_SUCCESS; +} + static NTSTATUS pulse_is_started(void *args) { struct is_started_params *params = args; @@ -2562,7 +2597,7 @@ const unixlib_entry_t __wine_unix_call_funcs[] = pulse_get_position, pulse_set_volumes, pulse_set_event_handle, - pulse_not_implemented, + pulse_set_sample_rate, pulse_test_connect, pulse_is_started, pulse_get_prop_value, @@ -3034,7 +3069,7 @@ const unixlib_entry_t __wine_unix_call_wow64_funcs[] = pulse_wow64_get_position, pulse_wow64_set_volumes, pulse_wow64_set_event_handle, - pulse_not_implemented, + pulse_set_sample_rate, pulse_wow64_test_connect, pulse_is_started, pulse_wow64_get_prop_value, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5585
Let's just call this `rate`. I'd prefer if this commit were squashed with the third commit and come after what is now the second commit, so the new order would be 2,{1+3},4. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5585#note_69866
This could use `IAudioCLient3_AddRef()` and drop the cast. Likewise for `_Release()`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5585#note_69867
`params.result` isn't initialised (as the drivers' `not_implemented` fns don't touch `params`). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5585#note_69868
I haven't really looked through this commit, but this should use `pulse_cond_wait()`. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5585#note_69869
(Hmm, looks like I forgot to add this to the review) [MSDN](https://learn.microsoft.com/en-us/windows/win32/api/audioclient/nn-audioclie...) says that this interface is obtained via a call to `GetService()` rather than `QueryInterface()` (rather like `IAudioClock`). It would be nice to have some tests that confirm this behaviour. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5585#note_69870
@ivyl added you as a reviewer since you're trying to upstream one of @aeikum's commits. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5585#note_69873
participants (4)
-
Andrew Eikum -
Arek Hiler (＠ivyl)
-
Arkadiusz Hiler
-
Huw Davies (＠huw)