Skip to content

Commit

Permalink
ao_wasapi: address premature buffer fills in exclusive mode
Browse files Browse the repository at this point in the history
Currently, running AO control wakes up the WASAPI renderer thread in the
`WASAPI_THREAD_FEED` state, where `thread_feed` will be called. However,
it seems that in recent Windows versions (tested on Windows 10 build
19044.3930 and Windows 11 build 22631.3007) we can't know if it is safe
to feed more audio data in event-driven exclusive mode:
- `IAudioClient_GetCurrentPadding` always returns `bufferFrameCount`,
  even if *NO* data has ever been written. This means we don't know how
  much free space we have that is available for writing. This is not the
  case in shared mode, where the return value correctly reflects the
  size of data waiting to be processed. As a sidenote, MS did not
  document the precise definition of the return value for an
  event-driven, exclusive stream [1].
- `IAudioRenderClient_GetBuffer` never fails. We can call it for 10
  times in a roll, each time requesting an entire buffer (the unit at
  which data is exchanged in exclusive mode using event-driven
  buffering; there are 2 such buffers) and get a successful return code
  everytime. In shared mode, we get `AUDCLNT_E_BUFFER_TOO_LARGE` if we
  request a buffer larger than that currently available.

As a result, `thread_feed` will always write `bufferFrameCount` frames
of audio in exclusive mode. There will therefore be glitches each time
`thread_control` is called due to the subsequent `thread_feed`
overwriting frames yet to be processed. Also, an irreversible error is
accumulated to `sample_count` as long as there is no AO reset, leading
to eventual, unbounded A/V desync.

As a fix to the issue, add a dedicated state for dispatch queue
processing so that `thread_feed` is only called when signaled by the OS.
The buffer checks in `thread_feed` that use `GetCurrentPadding` in
exclusive mode are kept in case there are older versions where the two
APIs behave differently.

Closes mpv-player#12615.

[1] https://learn.microsoft.com/en-us/windows/win32/api/audioclient/nf-audioclient-iaudioclient-getcurrentpadding
  • Loading branch information
sunpenghao authored and Dudemanguy committed Feb 24, 2024
1 parent 8cd678b commit 6863eef
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 13 deletions.
25 changes: 12 additions & 13 deletions audio/out/ao_wasapi.c
Original file line number Diff line number Diff line change
Expand Up @@ -180,19 +180,17 @@ static void thread_resume(struct ao *ao)
}
}

static void thread_wakeup(void *ptr)
static void set_state_and_wakeup_thread(struct ao *ao,
enum wasapi_thread_state thread_state)
{
struct ao *ao = ptr;
struct wasapi_state *state = ao->priv;
atomic_store(&state->thread_state, thread_state);
SetEvent(state->hWake);
}

static void set_thread_state(struct ao *ao,
enum wasapi_thread_state thread_state)
static void thread_process_dispatch(void *ptr)
{
struct wasapi_state *state = ao->priv;
atomic_store(&state->thread_state, thread_state);
thread_wakeup(ao);
set_state_and_wakeup_thread(ptr, WASAPI_THREAD_DISPATCH);
}

static DWORD __stdcall AudioThread(void *lpParameter)
Expand All @@ -212,15 +210,16 @@ static DWORD __stdcall AudioThread(void *lpParameter)
if (WaitForSingleObject(state->hWake, INFINITE) != WAIT_OBJECT_0)
MP_ERR(ao, "Unexpected return value from WaitForSingleObject\n");

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_DISPATCH:
mp_dispatch_queue_process(state->dispatch, 0);
break;
case WASAPI_THREAD_RESET:
thread_reset(ao);
break;
Expand Down Expand Up @@ -250,7 +249,7 @@ static void uninit(struct ao *ao)
MP_DBG(ao, "Uninit wasapi\n");
struct wasapi_state *state = ao->priv;
if (state->hWake)
set_thread_state(ao, WASAPI_THREAD_SHUTDOWN);
set_state_and_wakeup_thread(ao, WASAPI_THREAD_SHUTDOWN);

if (state->hAudioThread &&
WaitForSingleObject(state->hAudioThread, INFINITE) != WAIT_OBJECT_0)
Expand Down Expand Up @@ -301,7 +300,7 @@ static int init(struct ao *ao)
}

state->dispatch = mp_dispatch_create(state);
mp_dispatch_set_wakeup_fn(state->dispatch, thread_wakeup, ao);
mp_dispatch_set_wakeup_fn(state->dispatch, thread_process_dispatch, ao);

state->init_ok = false;
state->hAudioThread = CreateThread(NULL, 0, &AudioThread, ao, 0, NULL);
Expand Down Expand Up @@ -456,12 +455,12 @@ static int control(struct ao *ao, enum aocontrol cmd, void *arg)

static void audio_reset(struct ao *ao)
{
set_thread_state(ao, WASAPI_THREAD_RESET);
set_state_and_wakeup_thread(ao, WASAPI_THREAD_RESET);
}

static void audio_resume(struct ao *ao)
{
set_thread_state(ao, WASAPI_THREAD_RESUME);
set_state_and_wakeup_thread(ao, WASAPI_THREAD_RESUME);
}

static void hotplug_uninit(struct ao *ao)
Expand Down
1 change: 1 addition & 0 deletions audio/out/ao_wasapi.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ void wasapi_change_uninit(struct ao* ao);

enum wasapi_thread_state {
WASAPI_THREAD_FEED = 0,
WASAPI_THREAD_DISPATCH,
WASAPI_THREAD_RESUME,
WASAPI_THREAD_RESET,
WASAPI_THREAD_SHUTDOWN
Expand Down

0 comments on commit 6863eef

Please sign in to comment.