From cb1f84a33321ce4c76794e6867e337e8c9a1e947 Mon Sep 17 00:00:00 2001 From: Thomas Guillem Date: Tue, 12 Jul 2022 14:31:27 +0200 Subject: [PATCH 1/5] wasapi: call vlc_timer_disarm directly --- modules/audio_output/wasapi.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/modules/audio_output/wasapi.c b/modules/audio_output/wasapi.c index 3cd7fb67965..34293af2861 100644 --- a/modules/audio_output/wasapi.c +++ b/modules/audio_output/wasapi.c @@ -125,12 +125,6 @@ typedef struct aout_stream_sys bool s24s32; /**< Output configured as S24N, but input as S32N */ } aout_stream_sys_t; -static void ResetTimer(aout_stream_t *s) -{ - aout_stream_sys_t *sys = s->sys; - vlc_timer_disarm(sys->timer); -} - /*** VLC audio output callbacks ***/ static HRESULT TimeGet(aout_stream_t *s, vlc_tick_t *restrict delay) { @@ -206,7 +200,7 @@ static HRESULT StartDeferred(aout_stream_t *s, vlc_tick_t date) timer_updated = true; } else - ResetTimer(s); + vlc_timer_disarm(sys->timer); if (!timer_updated) { @@ -332,7 +326,7 @@ static HRESULT Pause(aout_stream_t *s, bool paused) if (paused) { - ResetTimer(s); + vlc_timer_disarm(sys->timer); if (atomic_load(&sys->started_state) == STARTED_STATE_OK) hr = IAudioClient_Stop(sys->client); else @@ -352,7 +346,7 @@ static HRESULT Flush(aout_stream_t *s) aout_stream_sys_t *sys = s->sys; HRESULT hr; - ResetTimer(s); + vlc_timer_disarm(sys->timer); /* Reset the timer state, the next start need to be deferred. */ if (atomic_exchange(&sys->started_state, STARTED_STATE_INIT) == STARTED_STATE_OK) { @@ -610,7 +604,7 @@ static void Stop(aout_stream_t *s) { aout_stream_sys_t *sys = s->sys; - ResetTimer(s); + vlc_timer_disarm(sys->timer); if (atomic_load(&sys->started_state) == STARTED_STATE_OK) IAudioClient_Stop(sys->client); -- GitLab From b71f96256ac1dba8675b7f15b4fcd2d42b39be77 Mon Sep 17 00:00:00 2001 From: Thomas Guillem Date: Tue, 12 Jul 2022 14:42:55 +0200 Subject: [PATCH 2/5] wasapi: fix double start leading to invalid state Check that the stream is not started after disarming the timer. This could lead to a double call to IAudioClient_Start(). The second call to IAudioClient_Start() fails (not critical) but the started_state is set to STARTED_STATE_ERROR preventing any future playback. Fixes #27128 --- modules/audio_output/wasapi.c | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/modules/audio_output/wasapi.c b/modules/audio_output/wasapi.c index 34293af2861..9b734d2e879 100644 --- a/modules/audio_output/wasapi.c +++ b/modules/audio_output/wasapi.c @@ -191,29 +191,30 @@ static HRESULT StartDeferred(aout_stream_t *s, vlc_tick_t date) aout_stream_sys_t *sys = s->sys; vlc_tick_t written = vlc_tick_from_frac(sys->written, sys->rate); vlc_tick_t start_delay = date - vlc_tick_now() - written; - BOOL timer_updated = false; /* Create or update the current timer */ if (start_delay > 0) { vlc_timer_schedule( sys->timer, false, start_delay, 0); - timer_updated = true; + msg_Dbg(s, "deferring start (%"PRId64" us)", start_delay); } else + { vlc_timer_disarm(sys->timer); - if (!timer_updated) - { - HRESULT hr = IAudioClient_Start(sys->client); - if (FAILED(hr)) + /* Check started_state again, since the timer callback could have been + * called before or while disarming the timer */ + if (atomic_load(&sys->started_state) == STARTED_STATE_INIT) { - atomic_store(&sys->started_state, STARTED_STATE_ERROR); - return hr; + HRESULT hr = IAudioClient_Start(sys->client); + if (FAILED(hr)) + { + atomic_store(&sys->started_state, STARTED_STATE_ERROR); + return hr; + } + atomic_store(&sys->started_state, STARTED_STATE_OK); } - atomic_store(&sys->started_state, STARTED_STATE_OK); } - else - msg_Dbg(s, "deferring start (%"PRId64" us)", start_delay); return S_OK; } -- GitLab From c0267647397952fdded29940cdcb7b6c030fca27 Mon Sep 17 00:00:00 2001 From: Thomas Guillem Date: Tue, 12 Jul 2022 14:47:31 +0200 Subject: [PATCH 3/5] wasapi: refactor start --- modules/audio_output/wasapi.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/modules/audio_output/wasapi.c b/modules/audio_output/wasapi.c index 9b734d2e879..2fd05fc8755 100644 --- a/modules/audio_output/wasapi.c +++ b/modules/audio_output/wasapi.c @@ -166,10 +166,21 @@ static HRESULT TimeGet(aout_stream_t *s, vlc_tick_t *restrict delay) return hr; } +static HRESULT StartNow(aout_stream_t *s) +{ + aout_stream_sys_t *sys = s->sys; + + HRESULT hr = IAudioClient_Start(sys->client); + + atomic_store(&sys->started_state, + SUCCEEDED(hr) ? STARTED_STATE_OK : STARTED_STATE_ERROR); + + return hr; +} + static void StartDeferredCallback(void *val) { aout_stream_t *s = val; - aout_stream_sys_t *sys = s->sys; HRESULT hr = CoInitializeEx(NULL, COINIT_MULTITHREADED); if (unlikely(FAILED(hr))) @@ -178,12 +189,9 @@ static void StartDeferredCallback(void *val) return; } - hr = IAudioClient_Start(sys->client); + StartNow(s); CoUninitialize(); - - atomic_store(&sys->started_state, - SUCCEEDED(hr) ? STARTED_STATE_OK : STARTED_STATE_ERROR); } static HRESULT StartDeferred(aout_stream_t *s, vlc_tick_t date) @@ -205,15 +213,7 @@ static HRESULT StartDeferred(aout_stream_t *s, vlc_tick_t date) /* Check started_state again, since the timer callback could have been * called before or while disarming the timer */ if (atomic_load(&sys->started_state) == STARTED_STATE_INIT) - { - HRESULT hr = IAudioClient_Start(sys->client); - if (FAILED(hr)) - { - atomic_store(&sys->started_state, STARTED_STATE_ERROR); - return hr; - } - atomic_store(&sys->started_state, STARTED_STATE_OK); - } + return StartNow(s); } return S_OK; -- GitLab From 42819895d5a35b33279c5ad59e0144bc17f4eab5 Mon Sep 17 00:00:00 2001 From: Thomas Guillem Date: Tue, 12 Jul 2022 16:02:22 +0200 Subject: [PATCH 4/5] wasapi: log stream start error No need to log success since it's already done by mmdevice. --- modules/audio_output/wasapi.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/modules/audio_output/wasapi.c b/modules/audio_output/wasapi.c index 2fd05fc8755..2d261dabbe0 100644 --- a/modules/audio_output/wasapi.c +++ b/modules/audio_output/wasapi.c @@ -175,6 +175,9 @@ static HRESULT StartNow(aout_stream_t *s) atomic_store(&sys->started_state, SUCCEEDED(hr) ? STARTED_STATE_OK : STARTED_STATE_ERROR); + if (FAILED(hr)) + msg_Err(s, "stream failed to start: 0x%lX", hr); + return hr; } -- GitLab From cb9014af76be77267ff6928ece63dd8e986e0f96 Mon Sep 17 00:00:00 2001 From: Thomas Guillem Date: Wed, 13 Jul 2022 09:43:55 +0200 Subject: [PATCH 5/5] wasapi: use StartNow() from Pause() This will set the started_state in case of error, preventing any future playback. --- modules/audio_output/wasapi.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/modules/audio_output/wasapi.c b/modules/audio_output/wasapi.c index 2d261dabbe0..95e187792e1 100644 --- a/modules/audio_output/wasapi.c +++ b/modules/audio_output/wasapi.c @@ -332,16 +332,17 @@ static HRESULT Pause(aout_stream_t *s, bool paused) { vlc_timer_disarm(sys->timer); if (atomic_load(&sys->started_state) == STARTED_STATE_OK) + { hr = IAudioClient_Stop(sys->client); + if (FAILED(hr)) + msg_Warn(s, "cannot stop stream (error 0x%lX)", hr); + } else hr = S_OK; /* Don't reset the timer state, we won't have to start deferred again. */ } else - hr = IAudioClient_Start(sys->client); - if (FAILED(hr)) - msg_Warn(s, "cannot %s stream (error 0x%lX)", - paused ? "stop" : "start", hr); + hr = StartNow(s); return hr; } -- GitLab