From 84a3c21beb2f3360e4fda13179846406b4a24f7c Mon Sep 17 00:00:00 2001 From: Kevin Mitchell Date: Fri, 26 Feb 2016 06:58:09 -0800 Subject: [PATCH] ao_wasapi: replace laggy COM messaging with mp_dispatch_queue A COM message loop is apparently totally inappropriate for a low latency thread. It leads to audio glitches because the thread doesn't wake up fast enough when it should. It also causes mysterious correlations between the vo and ao thread (i.e., toggling fullscreen delays audio feed events). Instead use an mp_dispatch_queue to set/get volume/mute/session display name from the audio thread. This has the added benefit of obviating the need to marshal the associated interfaces from the audio thread. --- audio/out/ao_wasapi.c | 146 +++++++++++++++++++----------------- audio/out/ao_wasapi.h | 16 +--- audio/out/ao_wasapi_utils.c | 93 ----------------------- 3 files changed, 80 insertions(+), 175 deletions(-) diff --git a/audio/out/ao_wasapi.c b/audio/out/ao_wasapi.c index 509a4d3e48..75090791b5 100644 --- a/audio/out/ao_wasapi.c +++ b/audio/out/ao_wasapi.c @@ -24,6 +24,7 @@ #include "options/m_option.h" #include "osdep/timer.h" #include "osdep/io.h" +#include "misc/dispatch.h" #include "ao_wasapi.h" // naive av_rescale for unsigned @@ -188,6 +189,21 @@ static void thread_reset(struct ao *ao) return; } +static void thread_wakeup(void *ptr) +{ + struct ao *ao = ptr; + struct wasapi_state *state = ao->priv; + SetEvent(state->hWake); +} + +static void set_thread_state(struct ao *ao, + enum wasapi_thread_state thread_state) +{ + struct wasapi_state *state = ao->priv; + atomic_store(&state->thread_state, thread_state); + thread_wakeup(ao); +} + static DWORD __stdcall AudioThread(void *lpParameter) { struct ao *ao = lpParameter; @@ -200,41 +216,31 @@ static DWORD __stdcall AudioThread(void *lpParameter) goto exit_label; MP_DBG(ao, "Entering dispatch loop\n"); - while (true) { // watch events - HANDLE events[] = {state->hWake}; - switch (MsgWaitForMultipleObjects(MP_ARRAY_SIZE(events), events, - FALSE, INFINITE, - QS_POSTMESSAGE | QS_SENDMESSAGE)) { - // AudioThread wakeup - case WAIT_OBJECT_0: - switch (atomic_load(&state->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); - break; - case WASAPI_THREAD_RESUME: - thread_reset(ao); - thread_resume(ao); - break; - case WASAPI_THREAD_SHUTDOWN: - thread_reset(ao); - goto exit_label; - default: - MP_ERR(ao, "Unhandled thread state\n"); - goto exit_label; - } + while (true) { + 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; - // messages to dispatch (COM marshalling) - case (WAIT_OBJECT_0 + MP_ARRAY_SIZE(events)): - wasapi_dispatch(ao); + case WASAPI_THREAD_RESET: + thread_reset(ao); break; - default: - MP_ERR(ao, "Unhandled thread event\n"); + case WASAPI_THREAD_RESUME: + thread_reset(ao); + thread_resume(ao); + break; + case WASAPI_THREAD_SHUTDOWN: + thread_reset(ao); goto exit_label; + default: + MP_ERR(ao, "Unhandled thread state: %d\n", thread_state); } } exit_label: @@ -245,19 +251,10 @@ exit_label: return 0; } -static void set_thread_state(struct ao *ao, - enum wasapi_thread_state thread_state) -{ - struct wasapi_state *state = ao->priv; - atomic_store(&state->thread_state, thread_state); - SetEvent(state->hWake); -} - static void uninit(struct ao *ao) { MP_DBG(ao, "Uninit wasapi\n"); struct wasapi_state *state = ao->priv; - wasapi_release_proxies(state); if (state->hWake) set_thread_state(ao, WASAPI_THREAD_SHUTDOWN); @@ -305,6 +302,9 @@ static int init(struct ao *ao) return -1; } + state->dispatch = mp_dispatch_create(state); + mp_dispatch_set_wakeup_fn(state->dispatch, thread_wakeup, ao); + state->init_ret = E_FAIL; state->hAudioThread = CreateThread(NULL, 0, &AudioThread, ao, 0, NULL); if (!state->hAudioThread) { @@ -322,19 +322,18 @@ static int init(struct ao *ao) return -1; } - wasapi_receive_proxies(state); MP_DBG(ao, "Init wasapi done\n"); return 0; } -static int control_exclusive(struct ao *ao, enum aocontrol cmd, void *arg) +static int thread_control_exclusive(struct ao *ao, enum aocontrol cmd, void *arg) { struct wasapi_state *state = ao->priv; switch (cmd) { case AOCONTROL_GET_VOLUME: case AOCONTROL_SET_VOLUME: - if (!state->pEndpointVolumeProxy || + if (!state->pEndpointVolume || !(state->vol_hw_support & ENDPOINT_HARDWARE_SUPPORT_VOLUME)) { return CONTROL_FALSE; } @@ -343,8 +342,7 @@ static int control_exclusive(struct ao *ao, enum aocontrol cmd, void *arg) switch (cmd) { case AOCONTROL_GET_VOLUME: IAudioEndpointVolume_GetMasterVolumeLevelScalar( - state->pEndpointVolumeProxy, - &volume); + state->pEndpointVolume, &volume); *(ao_control_vol_t *)arg = (ao_control_vol_t){ .left = 100.0f * volume, .right = 100.0f * volume, @@ -353,13 +351,12 @@ static int control_exclusive(struct ao *ao, enum aocontrol cmd, void *arg) case AOCONTROL_SET_VOLUME: volume = ((ao_control_vol_t *)arg)->left / 100.f; IAudioEndpointVolume_SetMasterVolumeLevelScalar( - state->pEndpointVolumeProxy, - volume, NULL); + state->pEndpointVolume, volume, NULL); return CONTROL_OK; } case AOCONTROL_GET_MUTE: case AOCONTROL_SET_MUTE: - if (!state->pEndpointVolumeProxy || + if (!state->pEndpointVolume || !(state->vol_hw_support & ENDPOINT_HARDWARE_SUPPORT_MUTE)) { return CONTROL_FALSE; } @@ -367,14 +364,12 @@ static int control_exclusive(struct ao *ao, enum aocontrol cmd, void *arg) BOOL mute; switch (cmd) { case AOCONTROL_GET_MUTE: - IAudioEndpointVolume_GetMute(state->pEndpointVolumeProxy, - &mute); + IAudioEndpointVolume_GetMute(state->pEndpointVolume, &mute); *(bool *)arg = mute; return CONTROL_OK; case AOCONTROL_SET_MUTE: mute = *(bool *)arg; - IAudioEndpointVolume_SetMute(state->pEndpointVolumeProxy, - mute, NULL); + IAudioEndpointVolume_SetMute(state->pEndpointVolume, mute, NULL); return CONTROL_OK; } case AOCONTROL_HAS_PER_APP_VOLUME: @@ -384,18 +379,17 @@ static int control_exclusive(struct ao *ao, enum aocontrol cmd, void *arg) } } -static int control_shared(struct ao *ao, enum aocontrol cmd, void *arg) +static int thread_control_shared(struct ao *ao, enum aocontrol cmd, void *arg) { struct wasapi_state *state = ao->priv; - if (!state->pAudioVolumeProxy) + if (!state->pAudioVolume) return CONTROL_UNKNOWN; float volume; BOOL mute; switch(cmd) { case AOCONTROL_GET_VOLUME: - ISimpleAudioVolume_GetMasterVolume(state->pAudioVolumeProxy, - &volume); + ISimpleAudioVolume_GetMasterVolume(state->pAudioVolume, &volume); *(ao_control_vol_t *)arg = (ao_control_vol_t){ .left = 100.0f * volume, .right = 100.0f * volume, @@ -403,16 +397,15 @@ static int control_shared(struct ao *ao, enum aocontrol cmd, void *arg) return CONTROL_OK; case AOCONTROL_SET_VOLUME: volume = ((ao_control_vol_t *)arg)->left / 100.f; - ISimpleAudioVolume_SetMasterVolume(state->pAudioVolumeProxy, - volume, NULL); + ISimpleAudioVolume_SetMasterVolume(state->pAudioVolume, volume, NULL); return CONTROL_OK; case AOCONTROL_GET_MUTE: - ISimpleAudioVolume_GetMute(state->pAudioVolumeProxy, &mute); + ISimpleAudioVolume_GetMute(state->pAudioVolume, &mute); *(bool *)arg = mute; return CONTROL_OK; case AOCONTROL_SET_MUTE: mute = *(bool *)arg; - ISimpleAudioVolume_SetMute(state->pAudioVolumeProxy, mute, NULL); + ISimpleAudioVolume_SetMute(state->pAudioVolume, mute, NULL); return CONTROL_OK; case AOCONTROL_HAS_PER_APP_VOLUME: return CONTROL_TRUE; @@ -421,14 +414,14 @@ static int control_shared(struct ao *ao, enum aocontrol cmd, void *arg) } } -static int control(struct ao *ao, enum aocontrol cmd, void *arg) +static int thread_control(struct ao *ao, enum aocontrol cmd, void *arg) { struct wasapi_state *state = ao->priv; // common to exclusive and shared switch (cmd) { case AOCONTROL_UPDATE_STREAM_TITLE: - if (!state->pSessionControlProxy) + if (!state->pSessionControl) return CONTROL_FALSE; wchar_t *title = mp_from_utf8(NULL, (char*)arg); @@ -437,12 +430,10 @@ static int control(struct ao *ao, enum aocontrol cmd, void *arg) // it seems that *sometimes* the SetDisplayName does not take effect and // it still shows the old title. Use this loop to insist until it works. do { - IAudioSessionControl_SetDisplayName(state->pSessionControlProxy, - title, NULL); + IAudioSessionControl_SetDisplayName(state->pSessionControl, title, NULL); SAFE_RELEASE(tmp, CoTaskMemFree(tmp)); - IAudioSessionControl_GetDisplayName(state->pSessionControlProxy, - &tmp); + IAudioSessionControl_GetDisplayName(state->pSessionControl, &tmp); } while (lstrcmpW(title, tmp)); SAFE_RELEASE(tmp, CoTaskMemFree(tmp)); talloc_free(title); @@ -450,7 +441,26 @@ static int control(struct ao *ao, enum aocontrol cmd, void *arg) } return state->share_mode == AUDCLNT_SHAREMODE_EXCLUSIVE ? - control_exclusive(ao, cmd, arg) : control_shared(ao, cmd, arg); + thread_control_exclusive(ao, cmd, arg) : + thread_control_shared(ao, cmd, arg); +} + +static void run_control(void *p) +{ + void **pp = p; + struct ao *ao = pp[0]; + enum aocontrol cmd = *(enum aocontrol *)pp[1]; + void *arg = pp[2]; + *(int *)pp[3] = thread_control(ao, cmd, arg); +} + +static int control(struct ao *ao, enum aocontrol cmd, void *arg) +{ + struct wasapi_state *state = ao->priv; + int ret; + void *p[] = {ao, &cmd, arg, &ret}; + mp_dispatch_run(state->dispatch, run_control, p); + return ret; } static void audio_reset(struct ao *ao) diff --git a/audio/out/ao_wasapi.h b/audio/out/ao_wasapi.h index c1181450a3..3ae50159b3 100644 --- a/audio/out/ao_wasapi.h +++ b/audio/out/ao_wasapi.h @@ -66,6 +66,7 @@ typedef struct wasapi_state { HANDLE hAudioThread; // the audio thread itself 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 // for setting the audio thread priority HANDLE hTask; @@ -83,25 +84,12 @@ typedef struct wasapi_state { UINT64 clock_frequency; // scale for position returned by GetPosition LARGE_INTEGER qpc_frequency; // frequency of Windows' high resolution timer - // WASAPI control (handles owned by audio thread but used by main thread) + // WASAPI control IAudioSessionControl *pSessionControl; // setting the stream title IAudioEndpointVolume *pEndpointVolume; // exclusive mode volume/mute ISimpleAudioVolume *pAudioVolume; // shared mode volume/mute DWORD vol_hw_support; // is hardware volume supported for exclusive-mode? - // Streams used to marshal the proxy objects. The thread owning the actual - // objects needs to marshal proxy objects into these streams, and the thread - // that wants the proxies unmarshals them from here. - IStream *sSessionControl; - IStream *sEndpointVolume; - IStream *sAudioVolume; - - // WASAPI proxy handles, for Single-Threaded Apartment communication. One is - // needed for each audio thread object that's accessed from the main thread. - IAudioSessionControl *pSessionControlProxy; - IAudioEndpointVolume *pEndpointVolumeProxy; - ISimpleAudioVolume *pAudioVolumeProxy; - // ao options int opt_exclusive; int opt_list; diff --git a/audio/out/ao_wasapi_utils.c b/audio/out/ao_wasapi_utils.c index 7816517bb8..ce084731e4 100644 --- a/audio/out/ao_wasapi_utils.c +++ b/audio/out/ao_wasapi_utils.c @@ -936,93 +936,6 @@ exit_label: return deviceID; } -static void *unmarshal(struct wasapi_state *state, REFIID type, IStream **from) -{ - if (!*from) - return NULL; - void *to_proxy = NULL; - HRESULT hr = CoGetInterfaceAndReleaseStream(*from, type, &to_proxy); - *from = NULL; // the stream is released even on failure - EXIT_ON_ERROR(hr); - return to_proxy; -exit_label: - MP_WARN(state, "Error reading COM proxy: %s\n", mp_HRESULT_to_str(hr)); - return to_proxy; -} - -void wasapi_receive_proxies(struct wasapi_state *state) { - state->pAudioVolumeProxy = unmarshal(state, &IID_ISimpleAudioVolume, - &state->sAudioVolume); - state->pEndpointVolumeProxy = unmarshal(state, &IID_IAudioEndpointVolume, - &state->sEndpointVolume); - state->pSessionControlProxy = unmarshal(state, &IID_IAudioSessionControl, - &state->sSessionControl); -} - -void wasapi_release_proxies(wasapi_state *state) { - SAFE_RELEASE(state->pAudioVolumeProxy, - ISimpleAudioVolume_Release(state->pAudioVolumeProxy)); - SAFE_RELEASE(state->pEndpointVolumeProxy, - IAudioEndpointVolume_Release(state->pEndpointVolumeProxy)); - SAFE_RELEASE(state->pSessionControlProxy, - IAudioSessionControl_Release(state->pSessionControlProxy)); -} - -// Must call CoReleaseMarshalData to decrement marshalled object's reference -// count. -#define SAFE_RELEASE_INTERFACE_STREAM(stream) do { \ - if ((stream) != NULL) { \ - CoReleaseMarshalData((stream)); \ - IStream_Release((stream)); \ - (stream) = NULL; \ - } \ - } while(0) - -static IStream *marshal(struct wasapi_state *state, - REFIID type, void *from_obj) -{ - if (!from_obj) - return NULL; - IStream *to; - HRESULT hr = CreateStreamOnHGlobal(NULL, TRUE, &to); - EXIT_ON_ERROR(hr); - hr = CoMarshalInterThreadInterfaceInStream(type, (IUnknown *)from_obj, &to); - EXIT_ON_ERROR(hr); - return to; -exit_label: - SAFE_RELEASE_INTERFACE_STREAM(to); - MP_WARN(state, "Error creating COM proxy stream: %s\n", - mp_HRESULT_to_str(hr)); - return to; -} - -static void create_proxy_streams(struct wasapi_state *state) { - state->sAudioVolume = marshal(state, &IID_ISimpleAudioVolume, - state->pAudioVolume); - state->sEndpointVolume = marshal(state, &IID_IAudioEndpointVolume, - state->pEndpointVolume); - state->sSessionControl = marshal(state, &IID_IAudioSessionControl, - state->pSessionControl); -} - -static void destroy_proxy_streams(struct wasapi_state *state) { - // This is only to handle error conditions. - // During normal operation, these will already have been released by - // unmarshaling. - SAFE_RELEASE_INTERFACE_STREAM(state->sAudioVolume); - SAFE_RELEASE_INTERFACE_STREAM(state->sEndpointVolume); - SAFE_RELEASE_INTERFACE_STREAM(state->sSessionControl); -} - -void wasapi_dispatch(struct ao *ao) -{ - MP_DBG(ao, "Dispatch\n"); - // dispatch any possible pending messages - MSG msg; - while (PeekMessage(&msg, NULL, 0, 0, PM_REMOVE)) - DispatchMessage(&msg); -} - HRESULT wasapi_thread_init(struct ao *ao) { struct wasapi_state *state = ao->priv; @@ -1057,9 +970,6 @@ retry: ; } EXIT_ON_ERROR(hr); - MP_DBG(ao, "Creating proxies\n"); - create_proxy_streams(state); - MP_DBG(ao, "Init wasapi thread done\n"); return S_OK; exit_label: @@ -1071,13 +981,10 @@ void wasapi_thread_uninit(struct ao *ao) { struct wasapi_state *state = ao->priv; MP_DBG(ao, "Thread shutdown\n"); - wasapi_dispatch(ao); if (state->pAudioClient) IAudioClient_Stop(state->pAudioClient); - destroy_proxy_streams(state); - SAFE_RELEASE(state->pRenderClient, IAudioRenderClient_Release(state->pRenderClient)); SAFE_RELEASE(state->pAudioClock, IAudioClock_Release(state->pAudioClock)); SAFE_RELEASE(state->pAudioVolume, ISimpleAudioVolume_Release(state->pAudioVolume));