ao_wasapi: fix player core lockup when avoiding premature buffer fills

6863eefc3d 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: 6863eefc3d
This commit is contained in:
nanahi 2024-04-25 13:08:41 -04:00 committed by Kacper Michajłow
parent 7f0961479a
commit 51e01e9772
2 changed files with 20 additions and 9 deletions

View File

@ -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;
}

View File

@ -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;