[PATCH v3 0/1] MR2603: winecoreaudio: Move timer logic to unixlib, matching other drivers.
-- v3: winecoreaudio: Move timer logic to unixlib, matching other drivers. https://gitlab.winehq.org/wine/wine/-/merge_requests/2603
From: Davide Beatrici <git(a)davidebeatrici.dev> --- dlls/winecoreaudio.drv/coreaudio.c | 86 +++++++++++++++++++++++++---- dlls/winecoreaudio.drv/mmdevdrv.c | 87 +++++++++--------------------- 2 files changed, 102 insertions(+), 71 deletions(-) diff --git a/dlls/winecoreaudio.drv/coreaudio.c b/dlls/winecoreaudio.drv/coreaudio.c index 073dacf4bad..87f132b20cf 100644 --- a/dlls/winecoreaudio.drv/coreaudio.c +++ b/dlls/winecoreaudio.drv/coreaudio.c @@ -81,11 +81,15 @@ struct coreaudio_stream AudioConverterRef converter; AudioStreamBasicDescription dev_desc; /* audio unit format, not necessarily the same as fmt */ AudioDeviceID dev_id; + EDataFlow flow; + DWORD flags; AUDCLNT_SHAREMODE share; + HANDLE event; - BOOL playing; - UINT32 period_ms, period_frames; + BOOL playing, please_quit; + REFERENCE_TIME period; + UINT32 period_frames; UINT32 bufsize_frames, resamp_bufsize_frames; UINT32 lcl_offs_frames, held_frames, wri_offs_frames, tmp_buffer_frames; UINT32 cap_bufsize_frames, cap_offs_frames, cap_held_frames; @@ -658,10 +662,11 @@ static NTSTATUS unix_create_stream(void *args) goto end; } - stream->period_ms = params->period / 10000; + stream->period = params->period; stream->period_frames = muldiv(params->period, stream->fmt->nSamplesPerSec, 10000000); stream->dev_id = dev_id_from_device(params->device); stream->flow = params->flow; + stream->flags = params->flags; stream->share = params->share; stream->bufsize_frames = muldiv(params->duration, stream->fmt->nSamplesPerSec, 10000000); @@ -740,6 +745,12 @@ static NTSTATUS unix_release_stream( void *args ) struct coreaudio_stream *stream = handle_get_stream(params->stream); SIZE_T size; + if(params->timer_thread) { + stream->please_quit = TRUE; + NtWaitForSingleObject(params->timer_thread, FALSE, NULL); + NtClose(params->timer_thread); + } + if(stream->unit){ AudioOutputUnitStop(stream->unit); AudioComponentInstanceDispose(stream->unit); @@ -1272,8 +1283,7 @@ static NTSTATUS unix_get_latency(void *args) latency += stream_latency; /* pretend we process audio in Period chunks, so max latency includes * the period time */ - *params->latency = muldiv(latency, 10000000, stream->fmt->nSamplesPerSec) - + stream->period_ms * 10000; + *params->latency = muldiv(latency, 10000000, stream->fmt->nSamplesPerSec) + stream->period; OSSpinLockUnlock(&stream->lock); params->result = S_OK; @@ -1305,7 +1315,9 @@ static NTSTATUS unix_start(void *args) OSSpinLockLock(&stream->lock); - if(stream->playing) + if((stream->flags & AUDCLNT_STREAMFLAGS_EVENTCALLBACK) && !stream->event) + params->result = AUDCLNT_E_EVENTHANDLE_NOT_SET; + else if(stream->playing) params->result = AUDCLNT_E_NOT_STOPPED; else{ stream->playing = TRUE; @@ -1364,6 +1376,21 @@ static NTSTATUS unix_reset(void *args) return STATUS_SUCCESS; } +static NTSTATUS unix_timer_loop(void *args) +{ + struct timer_loop_params *params = args; + struct coreaudio_stream *stream = handle_get_stream(params->stream); + const LARGE_INTEGER delay = {{ -stream->period }}; + + while(!stream->please_quit) + { + NtDelayExecution(FALSE, &delay); + NtSetEvent(stream->event, NULL); + } + + return STATUS_SUCCESS; +} + static NTSTATUS unix_get_render_buffer(void *args) { struct get_render_buffer_params *params = args; @@ -1650,6 +1677,27 @@ static NTSTATUS unix_set_volumes(void *args) return STATUS_SUCCESS; } +static NTSTATUS unix_set_event_handle(void *args) +{ + struct set_event_handle_params *params = args; + struct coreaudio_stream *stream = handle_get_stream(params->stream); + HRESULT hr = S_OK; + + OSSpinLockLock(&stream->lock); + if(!stream->unit) + hr = AUDCLNT_E_DEVICE_INVALIDATED; + else if(!(stream->flags & AUDCLNT_STREAMFLAGS_EVENTCALLBACK)) + hr = AUDCLNT_E_EVENTHANDLE_NOT_EXPECTED; + else if(stream->event) + hr = HRESULT_FROM_WIN32(ERROR_INVALID_NAME); + else + stream->event = params->event; + OSSpinLockUnlock(&stream->lock); + + params->result = hr; + return STATUS_SUCCESS; +} + unixlib_entry_t __wine_unix_call_funcs[] = { unix_not_implemented, @@ -1661,7 +1709,7 @@ unixlib_entry_t __wine_unix_call_funcs[] = unix_start, unix_stop, unix_reset, - unix_not_implemented, + unix_timer_loop, unix_get_render_buffer, unix_release_render_buffer, unix_get_capture_buffer, @@ -1676,7 +1724,7 @@ unixlib_entry_t __wine_unix_call_funcs[] = unix_get_frequency, unix_get_position, unix_set_volumes, - unix_not_implemented, + unix_set_event_handle, unix_not_implemented, unix_is_started, unix_not_implemented, @@ -1994,6 +2042,24 @@ static NTSTATUS unix_wow64_set_volumes(void *args) return unix_set_volumes(¶ms); } +static NTSTATUS unix_wow64_set_event_handle(void *args) +{ + struct + { + stream_handle stream; + PTR32 event; + HRESULT result; + } *params32 = args; + struct set_event_handle_params params = + { + .stream = params32->stream, + .event = ULongToHandle(params32->event) + }; + unix_set_event_handle(¶ms); + params32->result = params.result; + return STATUS_SUCCESS; +} + unixlib_entry_t __wine_unix_call_wow64_funcs[] = { unix_not_implemented, @@ -2005,7 +2071,7 @@ unixlib_entry_t __wine_unix_call_wow64_funcs[] = unix_start, unix_stop, unix_reset, - unix_not_implemented, + unix_timer_loop, unix_wow64_get_render_buffer, unix_release_render_buffer, unix_wow64_get_capture_buffer, @@ -2020,7 +2086,7 @@ unixlib_entry_t __wine_unix_call_wow64_funcs[] = unix_wow64_get_frequency, unix_wow64_get_position, unix_wow64_set_volumes, - unix_not_implemented, + unix_wow64_set_event_handle, unix_not_implemented, unix_is_started, unix_not_implemented, diff --git a/dlls/winecoreaudio.drv/mmdevdrv.c b/dlls/winecoreaudio.drv/mmdevdrv.c index 8d1d14aa26f..5f6928ba655 100644 --- a/dlls/winecoreaudio.drv/mmdevdrv.c +++ b/dlls/winecoreaudio.drv/mmdevdrv.c @@ -91,9 +91,7 @@ struct ACImpl { IUnknown *pUnkFTMarshal; EDataFlow dataflow; - UINT32 channel_count, period_ms; - DWORD flags; - HANDLE event; + UINT32 channel_count; float *vols; HANDLE timer; @@ -128,8 +126,6 @@ typedef struct _SessionMgr { static WCHAR drv_key_devicesW[256]; -static HANDLE g_timer_q; - static CRITICAL_SECTION g_sessions_lock; static CRITICAL_SECTION_DEBUG g_sessions_lock_debug = { @@ -213,15 +209,11 @@ BOOL WINAPI DllMain(HINSTANCE dll, DWORD reason, void *reserved) swprintf(drv_key_devicesW, ARRAY_SIZE(drv_key_devicesW), L"Software\\Wine\\Drivers\\%s\\devices", filename); - g_timer_q = CreateTimerQueue(); - if(!g_timer_q) - return FALSE; break; } case DLL_PROCESS_DETACH: if (reserved) break; DeleteCriticalSection(&g_sessions_lock); - CloseHandle(g_timer_q); break; } return TRUE; @@ -517,28 +509,17 @@ static ULONG WINAPI AudioClient_AddRef(IAudioClient3 *iface) static ULONG WINAPI AudioClient_Release(IAudioClient3 *iface) { ACImpl *This = impl_from_IAudioClient3(iface); - struct release_stream_params params; ULONG ref; - ref = InterlockedDecrement(&This->ref); TRACE("(%p) Refcount now %lu\n", This, ref); - if(!ref){ - if(This->timer){ - HANDLE event; - BOOL wait; - event = CreateEventW(NULL, TRUE, FALSE, NULL); - wait = !DeleteTimerQueueTimer(g_timer_q, This->timer, event); - wait = wait && GetLastError() == ERROR_IO_PENDING; - if(event && wait) - WaitForSingleObject(event, INFINITE); - CloseHandle(event); - } - if(This->stream){ + if(!ref) { + if(This->stream) { + struct release_stream_params params; params.stream = This->stream; - params.timer_thread = NULL; + params.timer_thread = This->timer; UNIX_CALL(release_stream, ¶ms); - } - if(This->session){ + This->stream = 0; + EnterCriticalSection(&g_sessions_lock); list_remove(&This->entry); LeaveCriticalSection(&g_sessions_lock); @@ -752,9 +733,7 @@ static HRESULT WINAPI AudioClient_Initialize(IAudioClient3 *iface, return params.result; } - This->flags = flags; This->channel_count = fmt->nChannels; - This->period_ms = period / 10000; This->vols = HeapAlloc(GetProcessHeap(), 0, This->channel_count * sizeof(float)); if(!This->vols){ @@ -924,42 +903,38 @@ static HRESULT WINAPI AudioClient_GetDevicePeriod(IAudioClient3 *iface, return S_OK; } -void CALLBACK ca_period_cb(void *user, BOOLEAN timer) +static DWORD WINAPI ca_period_cb(void *user) { + struct timer_loop_params params; ACImpl *This = user; - - if(This->event) - SetEvent(This->event); + params.stream = This->stream; + SetThreadDescription(GetCurrentThread(), L"winecoreaudio_timer_loop"); + UNIX_CALL(timer_loop, ¶ms); + return 0; } static HRESULT WINAPI AudioClient_Start(IAudioClient3 *iface) { ACImpl *This = impl_from_IAudioClient3(iface); struct start_params params; + HRESULT hr; TRACE("(%p)\n", This); if(!This->stream) return AUDCLNT_E_NOT_INITIALIZED; - if((This->flags & AUDCLNT_STREAMFLAGS_EVENTCALLBACK) && !This->event) - return AUDCLNT_E_EVENTHANDLE_NOT_SET; - params.stream = This->stream; UNIX_CALL(start, ¶ms); + if(FAILED(hr = params.result)) + return hr; - if(SUCCEEDED(params.result)){ - if(This->event && !This->timer){ - if(!CreateTimerQueueTimer(&This->timer, g_timer_q, ca_period_cb, This, 0, - This->period_ms, WT_EXECUTEINTIMERTHREAD)){ - This->timer = NULL; - IAudioClient3_Stop(iface); - WARN("Unable to create timer: %lu\n", GetLastError()); - return E_OUTOFMEMORY; - } - } + if(!This->timer) { + This->timer = CreateThread(NULL, 0, ca_period_cb, This, 0, NULL); + SetThreadPriority(This->timer, THREAD_PRIORITY_TIME_CRITICAL); } - return params.result; + + return S_OK; } static HRESULT WINAPI AudioClient_Stop(IAudioClient3 *iface) @@ -996,29 +971,19 @@ static HRESULT WINAPI AudioClient_SetEventHandle(IAudioClient3 *iface, HANDLE event) { ACImpl *This = impl_from_IAudioClient3(iface); - HRESULT hr = S_OK; + struct set_event_handle_params params; TRACE("(%p)->(%p)\n", This, event); if(!event) return E_INVALIDARG; - if(!This->stream) return AUDCLNT_E_NOT_INITIALIZED; - EnterCriticalSection(&g_sessions_lock); - - if(!(This->flags & AUDCLNT_STREAMFLAGS_EVENTCALLBACK)) - hr = AUDCLNT_E_EVENTHANDLE_NOT_EXPECTED; - else if(This->event){ - FIXME("called twice\n"); - hr = HRESULT_FROM_WIN32(ERROR_INVALID_NAME); - }else - This->event = event; - - LeaveCriticalSection(&g_sessions_lock); - - return hr; + params.stream = This->stream; + params.event = event; + UNIX_CALL(set_event_handle, ¶ms); + return params.result; } static HRESULT WINAPI AudioClient_GetService(IAudioClient3 *iface, REFIID riid, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2603
Huw Davies (@huw) commented about dlls/winecoreaudio.drv/coreaudio.c:
return STATUS_SUCCESS; }
+static NTSTATUS unix_timer_loop(void *args) +{ + struct timer_loop_params *params = args; + struct coreaudio_stream *stream = handle_get_stream(params->stream); + const LARGE_INTEGER delay = {{ -stream->period }}; + + while(!stream->please_quit) + { + NtDelayExecution(FALSE, &delay); + NtSetEvent(stream->event, NULL); + }
This will cause problems for some apps. We need to keep track of the delta between the required and actual delay so that the events don't drift. See the alsa driver for how we do it there. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2603#note_29277
Huw Davies (@huw) commented about dlls/winecoreaudio.drv/mmdevdrv.c:
return S_OK; }
-void CALLBACK ca_period_cb(void *user, BOOLEAN timer) +static DWORD WINAPI ca_period_cb(void *user)
Could we change this name to `ca_timer_thread`? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2603#note_29278
This could be split into a few smaller patches. Moving from storing `period_ms` to `period` would be an obvious single change. Also you could add `set_event_handle` before the change to the timer logic (while continuing to keep the event on the PE-side until everything is moved across). Likewise the test on `flags` in `_Start` could be moved to the unixlib earlier. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2603#note_29279
participants (3)
-
Davide Beatrici -
Davide Beatrici (＠davidebeatrici)
-
Huw Davies (＠huw)