From 51e01e9772c157da02afe5a018c66f06231d732f Mon Sep 17 00:00:00 2001 From: nanahi <130121847+na-na-hi@users.noreply.github.com> Date: Thu, 25 Apr 2024 13:08:41 -0400 Subject: [PATCH] ao_wasapi: fix player core lockup when avoiding premature buffer fills 6863eefc3d1d683a08f56aeaca0b4706672f2447 handled this situation by using an atomic variable to express the state for which the wakeup is caused by AO control, and the dispatch queue is only processed at this state. However, this can cause permanent lockup of the player core when the following happens: - AO control sets the thread state to WASAPI_THREAD_DISPATCH, and sets the wakeup handle. - WASAPI thread reads the WASAPI_THREAD_DISPATCH state and processes the dispatch queue. - Another AO control happens. A dispatch item is enqueued, and the state stays at WASAPI_THREAD_DISPATCH. - WASAPI thread resets the thread state to WASAPI_THREAD_FEED since the state has not changed. - WaitForSingleObject() returns in the WASAPI thread, sees this state, and does not process the dispatch queue. - The player core locks permanently because it is waiting for the dispatch to be processed. This has been experimentally verified on a system under high contention: The easiest way to trigger this lockup is to continuously hold down "i", which rapidly issues AO get volume/mute controls. To properly handle this, use separate handles for system and user wakeup requests. Only feed audio when woke up by system and only process the dispatch queue when woke up by user. Fixes: 6863eefc3d1d683a08f56aeaca0b4706672f2447 --- audio/out/ao_wasapi.c | 28 +++++++++++++++++++--------- audio/out/ao_wasapi.h | 1 + 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/audio/out/ao_wasapi.c b/audio/out/ao_wasapi.c index bc41309ec6..34c3b6193f 100644 --- a/audio/out/ao_wasapi.c +++ b/audio/out/ao_wasapi.c @@ -197,7 +197,7 @@ static void thread_wakeup(void *ptr) { struct ao *ao = ptr; struct wasapi_state *state = ao->priv; - SetEvent(state->hWake); + SetEvent(state->hUserWake); } static void set_thread_state(struct ao *ao, @@ -222,17 +222,25 @@ static DWORD __stdcall AudioThread(void *lpParameter) MP_DBG(ao, "Entering dispatch loop\n"); while (true) { - if (WaitForSingleObject(state->hWake, INFINITE) != WAIT_OBJECT_0) - MP_ERR(ao, "Unexpected return value from WaitForSingleObject\n"); + HANDLE handles[] = {state->hWake, state->hUserWake}; + switch (WaitForMultipleObjects(MP_ARRAY_SIZE(handles), handles, FALSE, INFINITE)) { + case WAIT_OBJECT_0: + // fill twice on under-full buffer (see comment in thread_feed) + if (thread_feed(ao) && thread_feed(ao)) + MP_ERR(ao, "Unable to fill buffer fast enough\n"); + continue; + case WAIT_OBJECT_0 + 1: + break; + default: + MP_ERR(ao, "Unexpected return value from WaitForMultipleObjects\n"); + break; + } mp_dispatch_queue_process(state->dispatch, 0); int thread_state = atomic_load(&state->thread_state); switch (thread_state) { case WASAPI_THREAD_FEED: - // fill twice on under-full buffer (see comment in thread_feed) - if (thread_feed(ao) && thread_feed(ao)) - MP_ERR(ao, "Unable to fill buffer fast enough\n"); break; case WASAPI_THREAD_RESET: thread_reset(ao); @@ -268,7 +276,7 @@ static void uninit(struct ao *ao) { MP_DBG(ao, "Uninit wasapi\n"); struct wasapi_state *state = ao->priv; - if (state->hWake) + if (state->hWake && state->hUserWake) set_thread_state(ao, WASAPI_THREAD_SHUTDOWN); if (state->hAudioThread && @@ -280,6 +288,7 @@ static void uninit(struct ao *ao) SAFE_DESTROY(state->hInitDone, CloseHandle(state->hInitDone)); SAFE_DESTROY(state->hWake, CloseHandle(state->hWake)); + SAFE_DESTROY(state->hUserWake, CloseHandle(state->hUserWake)); SAFE_DESTROY(state->hAudioThread,CloseHandle(state->hAudioThread)); wasapi_change_uninit(ao); @@ -313,7 +322,8 @@ static int init(struct ao *ao) state->hInitDone = CreateEventW(NULL, FALSE, FALSE, NULL); state->hWake = CreateEventW(NULL, FALSE, FALSE, NULL); - if (!state->hInitDone || !state->hWake) { + state->hUserWake = CreateEventW(NULL, FALSE, FALSE, NULL); + if (!state->hInitDone || !state->hWake || !state->hUserWake) { MP_FATAL(ao, "Error creating events\n"); uninit(ao); return -1; @@ -485,7 +495,7 @@ static void audio_resume(struct ao *ao) static bool audio_set_pause(struct ao *ao, bool paused) { - set_state_and_wakeup_thread(ao, paused ? WASAPI_THREAD_PAUSE : WASAPI_THREAD_UNPAUSE); + set_thread_state(ao, paused ? WASAPI_THREAD_PAUSE : WASAPI_THREAD_UNPAUSE); return true; } diff --git a/audio/out/ao_wasapi.h b/audio/out/ao_wasapi.h index 333412bcfe..cbd4a3f933 100644 --- a/audio/out/ao_wasapi.h +++ b/audio/out/ao_wasapi.h @@ -65,6 +65,7 @@ typedef struct wasapi_state { HANDLE hWake; // thread wakeup event atomic_int thread_state; // enum wasapi_thread_state (what to do on wakeup) struct mp_dispatch_queue *dispatch; // for volume/mute/session display + HANDLE hUserWake; // mpv-requested wakeup event // for setting the audio thread priority HANDLE hTask;