[PATCH v6 0/3] MR5585: 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`. -- v6: winepulse.drv: Implement set_sample_rate. mmdevapi: Add stub IAudioClockAdjustment implementation. mmdevapi/tests: Add more IAudioClock tests. https://gitlab.winehq.org/wine/wine/-/merge_requests/5585
From: Arkadiusz Hiler <ahiler(a)codeweavers.com> --- dlls/mmdevapi/tests/render.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/dlls/mmdevapi/tests/render.c b/dlls/mmdevapi/tests/render.c index b8ef4b5a549..dac047b98ab 100644 --- a/dlls/mmdevapi/tests/render.c +++ b/dlls/mmdevapi/tests/render.c @@ -135,6 +135,7 @@ static void test_audioclient(void) IAudioClient *ac; IAudioClient2 *ac2; IAudioClient3 *ac3; + IAudioClock *acl; IUnknown *unk; HRESULT hr; ULONG ref; @@ -203,6 +204,8 @@ static void test_audioclient(void) ref = IUnknown_Release(unk); ok(ref == 1, "Released count is %lu\n", ref); } + hr = IAudioClient_QueryInterface(ac, &IID_IAudioClock, (void**)&acl); + ok(hr == E_NOINTERFACE, "QueryInterface(IID_IAudioClock) returned %08lx\n", hr); hr = IAudioClient_GetDevicePeriod(ac, NULL, NULL); ok(hr == E_POINTER, "Invalid GetDevicePeriod call returns %08lx\n", hr); @@ -598,7 +601,7 @@ static void test_formats(AUDCLNT_SHAREMODE mode) static void test_references(void) { - IAudioClient *ac; + IAudioClient *ac, *ac2; IAudioRenderClient *rc; ISimpleAudioVolume *sav; IAudioStreamVolume *asv; @@ -692,6 +695,9 @@ static void test_references(void) ref = IAudioClock_Release(acl); ok(ref != 0, "AudioClock_Release gave wrong refcount: %lu\n", ref); + hr = IAudioClock_QueryInterface(acl, &IID_IAudioClient, (void**)&ac2); + ok(hr == E_NOINTERFACE, "QueryInterface(IID_IAudioClient) returned %08lx\n", hr); + ref = IAudioClient_Release(ac); ok(ref != 0, "Client_Release gave wrong refcount: %lu\n", ref); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5585
From: Andrew Eikum <aeikum(a)codeweavers.com> with tests by Arkadiusz Hiler. --- dlls/mmdevapi/client.c | 71 +++++++++++++++++++++++++++--- dlls/mmdevapi/mmdevdrv.h | 1 + dlls/mmdevapi/tests/render.c | 64 +++++++++++++++++++++++++++ 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 + 8 files changed, 146 insertions(+), 6 deletions(-) diff --git a/dlls/mmdevapi/client.c b/dlls/mmdevapi/client.c index 860c31f4b49..d29969b80c5 100644 --- a/dlls/mmdevapi/client.c +++ b/dlls/mmdevapi/client.c @@ -92,6 +92,11 @@ static inline struct audio_client *impl_from_IAudioClock2(IAudioClock2 *iface) return CONTAINING_RECORD(iface, struct audio_client, IAudioClock2_iface); } +static inline struct audio_client *impl_from_IAudioClockAdjustment(IAudioClockAdjustment *iface) +{ + return CONTAINING_RECORD(iface, struct audio_client, IAudioClockAdjustment_iface); +} + static inline struct audio_client *impl_from_IAudioRenderClient(IAudioRenderClient *iface) { return CONTAINING_RECORD(iface, struct audio_client, IAudioRenderClient_iface); @@ -607,6 +612,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) @@ -617,6 +624,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); @@ -966,6 +975,9 @@ static HRESULT WINAPI client_GetService(IAudioClient3 *iface, REFIID riid, void if (!new_session) IUnknown_AddRef((IUnknown *)*ppv); + } else if (IsEqualIID(riid, &IID_IAudioClockAdjustment)) { + IAudioClockAdjustment_AddRef(&This->IAudioClockAdjustment_iface); + *ppv = &This->IAudioClockAdjustment_iface; } else { FIXME("stub %s\n", debugstr_guid(riid)); hr = E_NOINTERFACE; @@ -1273,6 +1285,52 @@ const IAudioClock2Vtbl AudioClock2_Vtbl = clock2_GetDevicePosition }; +static HRESULT WINAPI AudioClockAdjustment_QueryInterface(IAudioClockAdjustment *iface, + REFIID riid, void **ppv) +{ + struct audio_client *This = impl_from_IAudioClockAdjustment(iface); + return IAudioClient3_QueryInterface(&This->IAudioClient3_iface, riid, ppv); +} + +static ULONG WINAPI AudioClockAdjustment_AddRef(IAudioClockAdjustment *iface) +{ + struct audio_client *This = impl_from_IAudioClockAdjustment(iface); + return IAudioClient3_AddRef(&This->IAudioClient3_iface); +} + +static ULONG WINAPI AudioClockAdjustment_Release(IAudioClockAdjustment *iface) +{ + struct audio_client *This = impl_from_IAudioClockAdjustment(iface); + return IAudioClient3_Release(&This->IAudioClient3_iface); +} + +static HRESULT WINAPI AudioClockAdjustment_SetSampleRate(IAudioClockAdjustment *iface, float rate) +{ + struct audio_client *This = impl_from_IAudioClockAdjustment(iface); + struct set_sample_rate_params params; + + TRACE("(%p)->(%f)\n", This, rate); + + if (!This->stream) + return AUDCLNT_E_NOT_INITIALIZED; + + params.stream = This->stream; + params.rate = rate; + params.result = E_NOTIMPL; + + wine_unix_call(set_sample_rate, ¶ms); + + return params.result; +} + +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); @@ -1552,12 +1610,13 @@ HRESULT AudioClient_Create(GUID *guid, IMMDevice *device, IAudioClient **out) This->device_name = name; - This->IAudioCaptureClient_iface.lpVtbl = &AudioCaptureClient_Vtbl; - This->IAudioClient3_iface.lpVtbl = &AudioClient3_Vtbl; - This->IAudioClock_iface.lpVtbl = &AudioClock_Vtbl; - This->IAudioClock2_iface.lpVtbl = &AudioClock2_Vtbl; - This->IAudioRenderClient_iface.lpVtbl = &AudioRenderClient_Vtbl; - This->IAudioStreamVolume_iface.lpVtbl = &AudioStreamVolume_Vtbl; + This->IAudioCaptureClient_iface.lpVtbl = &AudioCaptureClient_Vtbl; + 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; This->dataflow = dataflow; This->parent = device; diff --git a/dlls/mmdevapi/mmdevdrv.h b/dlls/mmdevapi/mmdevdrv.h index 7683867a9c1..a580043f577 100644 --- a/dlls/mmdevapi/mmdevdrv.h +++ b/dlls/mmdevapi/mmdevdrv.h @@ -62,6 +62,7 @@ struct audio_client { IAudioCaptureClient IAudioCaptureClient_iface; IAudioClock IAudioClock_iface; IAudioClock2 IAudioClock2_iface; + IAudioClockAdjustment IAudioClockAdjustment_iface; IAudioStreamVolume IAudioStreamVolume_iface; LONG ref; diff --git a/dlls/mmdevapi/tests/render.c b/dlls/mmdevapi/tests/render.c index dac047b98ab..64d24fac695 100644 --- a/dlls/mmdevapi/tests/render.c +++ b/dlls/mmdevapi/tests/render.c @@ -137,6 +137,7 @@ static void test_audioclient(void) IAudioClient3 *ac3; IAudioClock *acl; IUnknown *unk; + IAudioClockAdjustment *aca; HRESULT hr; ULONG ref; WAVEFORMATEX *pwfx, *pwfx2; @@ -207,6 +208,16 @@ static void test_audioclient(void) hr = IAudioClient_QueryInterface(ac, &IID_IAudioClock, (void**)&acl); ok(hr == E_NOINTERFACE, "QueryInterface(IID_IAudioClock) returned %08lx\n", hr); + hr = IAudioClient_QueryInterface(ac, &IID_IAudioClockAdjustment, (void**)&aca); + ok(hr == S_OK, "QueryInterface(IID_IAudioClockAdjustment) returned %08lx\n", hr); + if (aca) + { + hr = IAudioClockAdjustment_QueryInterface(aca, &IID_IAudioClock, (void**)&acl); + ok(hr == E_NOINTERFACE, "QueryInterface(IID_IAudioClock) returned %08lx\n", hr); + ref = IAudioClockAdjustment_Release(aca); + ok(ref == 1, "Released count is %lu\n", ref); + } + hr = IAudioClient_GetDevicePeriod(ac, NULL, NULL); ok(hr == E_POINTER, "Invalid GetDevicePeriod call returns %08lx\n", hr); @@ -603,6 +614,7 @@ static void test_references(void) { IAudioClient *ac, *ac2; IAudioRenderClient *rc; + IAudioClockAdjustment *aca; ISimpleAudioVolume *sav; IAudioStreamVolume *asv; IAudioClock *acl; @@ -643,6 +655,58 @@ static void test_references(void) ref = IAudioRenderClient_Release(rc); ok(ref == 0, "RenderClient_Release gave wrong refcount: %lu\n", ref); + /* IAudioClockAdjustment */ + hr = IMMDevice_Activate(dev, &IID_IAudioClient, CLSCTX_INPROC_SERVER, + NULL, (void**)&ac); + ok(hr == S_OK, "Activation failed with %08lx\n", hr); + if(hr != S_OK) + return; + + hr = IAudioClient_GetMixFormat(ac, &pwfx); + ok(hr == S_OK, "GetMixFormat failed: %08lx\n", hr); + + hr = IAudioClient_Initialize(ac, AUDCLNT_SHAREMODE_SHARED, 0, 5000000, + 0, pwfx, NULL); + ok(hr == S_OK, "Initialize failed: %08lx\n", hr); + + CoTaskMemFree(pwfx); + + hr = IAudioClient_GetService(ac, &IID_IAudioClockAdjustment, (void**)&aca); + todo_wine ok(hr == E_INVALIDARG, "IAudioClient_GetService(IID_IAudioClockAdjustment) returned %08lx\n", hr); + + if (hr == S_OK) { + ref = IAudioClockAdjustment_Release(aca); + ok(ref == 1, "AudioClockAdjustment_Release gave wrong refcount: %lu\n", ref); + } + + ref = IAudioClient_Release(ac); + ok(ref == 0, "Client_Release gave wrong refcount: %lu\n", ref); + + /* IAudioClockAdjustment */ + hr = IMMDevice_Activate(dev, &IID_IAudioClient, CLSCTX_INPROC_SERVER, + NULL, (void**)&ac); + ok(hr == S_OK, "Activation failed with %08lx\n", hr); + if(hr != S_OK) + return; + + hr = IAudioClient_GetMixFormat(ac, &pwfx); + ok(hr == S_OK, "GetMixFormat failed: %08lx\n", hr); + + hr = IAudioClient_Initialize(ac, AUDCLNT_SHAREMODE_SHARED, AUDCLNT_STREAMFLAGS_RATEADJUST, 5000000, + 0, pwfx, NULL); + ok(hr == S_OK, "Initialize failed: %08lx\n", hr); + + CoTaskMemFree(pwfx); + + hr = IAudioClient_GetService(ac, &IID_IAudioClockAdjustment, (void**)&aca); + ok(hr == S_OK, "IAudioClient_GetService(IID_IAudioClockAdjustment) returned %08lx\n", hr); + ref = IAudioClockAdjustment_Release(aca); + ok(ref == 1, "AudioClockAdjustment_Release gave wrong refcount: %lu\n", ref); + + ref = IAudioClient_Release(ac); + ok(ref == 0, "Client_Release gave wrong refcount: %lu\n", ref); + + /* ISimpleAudioVolume */ hr = IMMDevice_Activate(dev, &IID_IAudioClient, CLSCTX_INPROC_SERVER, NULL, (void**)&ac); diff --git a/dlls/mmdevapi/unixlib.h b/dlls/mmdevapi/unixlib.h index 4cb4c881bcf..097b129f564 100644 --- a/dlls/mmdevapi/unixlib.h +++ b/dlls/mmdevapi/unixlib.h @@ -225,6 +225,13 @@ struct set_event_handle_params HRESULT result; }; +struct set_sample_rate_params +{ + stream_handle stream; + float rate; + HRESULT result; +}; + struct test_connect_params { const WCHAR *name; @@ -333,6 +340,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 970db59ad12..046b447aafd 100644 --- a/dlls/winealsa.drv/alsa.c +++ b/dlls/winealsa.drv/alsa.c @@ -2503,6 +2503,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, @@ -2960,6 +2961,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 db5e1116bf8..377f3f5f035 100644 --- a/dlls/winecoreaudio.drv/coreaudio.c +++ b/dlls/winecoreaudio.drv/coreaudio.c @@ -1849,6 +1849,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, @@ -2305,6 +2306,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 a017d242a98..cac8e272c8c 100644 --- a/dlls/wineoss.drv/oss.c +++ b/dlls/wineoss.drv/oss.c @@ -1702,6 +1702,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, @@ -2198,6 +2199,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 263d0f1a45a..26b44e79f0c 100644 --- a/dlls/winepulse.drv/pulse.c +++ b/dlls/winepulse.drv/pulse.c @@ -2611,6 +2611,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, @@ -3108,6 +3109,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: Arkadiusz Hiler <ahiler(a)codeweavers.com> --- dlls/winepulse.drv/pulse.c | 65 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 62 insertions(+), 3 deletions(-) diff --git a/dlls/winepulse.drv/pulse.c b/dlls/winepulse.drv/pulse.c index 26b44e79f0c..d1cb238b7f7 100644 --- a/dlls/winepulse.drv/pulse.c +++ b/dlls/winepulse.drv/pulse.c @@ -69,6 +69,7 @@ struct pulse_stream float vol[PA_CHANNELS_MAX]; REFERENCE_TIME def_period; + REFERENCE_TIME duration; INT32 locked; BOOL started; @@ -1073,7 +1074,7 @@ static HRESULT pulse_stream_connect(struct pulse_stream *stream, const char *pul pulse_name = NULL; /* use default */ if (stream->dataflow == eRender) - ret = pa_stream_connect_playback(stream->stream, pulse_name, &attr, flags, NULL, NULL); + ret = pa_stream_connect_playback(stream->stream, pulse_name, &attr, flags|PA_STREAM_VARIABLE_RATE, NULL, NULL); else ret = pa_stream_connect_record(stream->stream, pulse_name, &attr, flags); if (ret < 0) { @@ -1158,6 +1159,7 @@ static NTSTATUS pulse_create_stream(void *args) goto exit; stream->def_period = params->period; + stream->duration = params->duration; stream->period_bytes = pa_frame_size(&stream->ss) * muldiv(params->period, stream->ss.rate, @@ -2486,6 +2488,63 @@ 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; + int success; + SIZE_T size, new_bufsize_frames; + BYTE *new_buffer = NULL; + pa_sample_spec new_ss; + + pulse_lock(); + if (!pulse_stream_valid(stream)) + hr = AUDCLNT_E_DEVICE_INVALIDATED; + else if (stream->dataflow != eRender) + hr = E_NOTIMPL; + else { + new_ss = stream->ss; + new_ss.rate = params->rate; + new_bufsize_frames = ceil((stream->duration / 10000000.) * new_ss.rate); + size = new_bufsize_frames * 2 * pa_frame_size(&stream->ss); + + if (NtAllocateVirtualMemory(GetCurrentProcess(), (void **)&new_buffer, zero_bits, &size, MEM_COMMIT, PAGE_READWRITE)) + hr = E_OUTOFMEMORY; + else { + if (!wait_pa_operation_complete(pa_stream_update_sample_rate(stream->stream, params->rate, pulse_op_cb, &success))) + success = 0; + + if (!success) { + hr = E_OUTOFMEMORY; + size = 0; + NtFreeVirtualMemory(GetCurrentProcess(), (void **)&new_buffer, &size, MEM_RELEASE); + } else { + if (stream->held_bytes) + wait_pa_operation_complete(pa_stream_flush(stream->stream, pulse_op_cb, &success)); + + stream->clock_lastpos = stream->clock_written = 0; + stream->pa_offs_bytes = stream->lcl_offs_bytes = 0; + stream->held_bytes = stream->pa_held_bytes = 0; + stream->period_bytes = pa_frame_size(&new_ss) * muldiv(stream->mmdev_period_usec, new_ss.rate, 1000000); + stream->real_bufsize_bytes = size; + stream->bufsize_frames = new_bufsize_frames; + stream->ss = new_ss; + + size = 0; + NtFreeVirtualMemory(GetCurrentProcess(), (void **)&stream->local_buffer, &size, MEM_RELEASE); + + silence_buffer(new_ss.format, new_buffer, size); + stream->local_buffer = new_buffer; + } + } + } + pulse_unlock(); + + params->result = hr; + return STATUS_SUCCESS; +} + static NTSTATUS pulse_is_started(void *args) { struct is_started_params *params = args; @@ -2611,7 +2670,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, @@ -3109,7 +3168,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
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 tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=147011 Your paranoid android. === w8adm (32 bit report) === mmdevapi: render.c:1446: Test failed: GetBuffer large (20671) at iteration 5 === w1064v1507 (32 bit report) === mmdevapi: render.c:1446: Test failed: GetBuffer large (20671) at iteration 1 render.c:1446: Test failed: GetBuffer large (20671) at iteration 4 render.c:1446: Test failed: GetBuffer large (20671) at iteration 5 render.c:1446: Test failed: GetBuffer large (20671) at iteration 8 === w1064_tsign (32 bit report) === mmdevapi: render.c:1446: Test failed: GetBuffer large (22500) at iteration 2 render.c:1446: Test failed: GetBuffer large (22500) at iteration 3 === w10pro64 (32 bit report) === mmdevapi: render.c:1446: Test failed: GetBuffer large (22500) at iteration 4 render.c:1446: Test failed: GetBuffer large (22500) at iteration 8 render.c:1446: Test failed: GetBuffer large (22500) at iteration 7 === w1064v1507 (64 bit report) === mmdevapi: render.c:1446: Test failed: GetBuffer large (20671) at iteration 7 === w1064_tsign (64 bit report) === mmdevapi: render.c:1446: Test failed: GetBuffer large (22500) at iteration 1
A bunch of ddraw failures: https://gitlab.winehq.org/ivyl/wine/-/jobs/86614#L789 `render.c:1446: Test failed: GetBuffer large (22500) at iteration 1`: https://gitlab.winehq.org/ivyl/wine/-/jobs/86615#L129 unrelated to the changes - the test seems to be just flaky. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5585#note_75922
Huw Davies (@huw) commented about dlls/winepulse.drv/pulse.c:
+ + stream->clock_lastpos = stream->clock_written = 0; + stream->pa_offs_bytes = stream->lcl_offs_bytes = 0; + stream->held_bytes = stream->pa_held_bytes = 0; + stream->period_bytes = pa_frame_size(&new_ss) * muldiv(stream->mmdev_period_usec, new_ss.rate, 1000000); + stream->real_bufsize_bytes = size; + stream->bufsize_frames = new_bufsize_frames; + stream->ss = new_ss; + + size = 0; + NtFreeVirtualMemory(GetCurrentProcess(), (void **)&stream->local_buffer, &size, MEM_RELEASE); + + silence_buffer(new_ss.format, new_buffer, size); + stream->local_buffer = new_buffer; + } + } The nested `if`s here are rather unfortunate. I think it would look cleaner if we used `goto`s in the failure cases.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5585#note_78965
On Wed Jul 10 14:34:41 2024 +0000, Arek Hiler wrote:
I've been looking into this when originally implementing the function. I had another look now. The only shared parts that I think we could extract are ```c new_bufsize_frames = ceil((stream->duration / 10000000.) * new_ss.rate); size = new_bufsize_frames * 2 * pa_frame_size(&stream->ss); if (NtAllocateVirtualMemory(GetCurrentProcess(), (void **)&new_buffer, zero_bits, &size, MEM_COMMIT, PAGE_READWRITE)) hr = E_OUTOFMEMORY; ``` but I couldn't make it not overly awkward, mostly because of when the variables have to be set and the failure paths / cleanup. I can do two helpers, one for `bufsize_frames` and `size` each unless you have a more detailed suggestion. @huw What do you think?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/5585#note_78969
@huw What do you think?
Not sure yet. I was going to wait until the control flow was a bit simpler. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5585#note_78971
participants (6)
-
Andrew Eikum -
Arek Hiler (＠ivyl)
-
Arkadiusz Hiler
-
Davide Beatrici (＠davidebeatrici)
-
Huw Davies (＠huw)
-
Marvin