From 0c0a35475313c6bb96c7cdf6059d245746f3843d Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 28 Jun 2019 15:30:26 +0200 Subject: [PATCH] output/alsa: add a new flag "waiting" for xrun management In DispatchSockets(), when there was not enough data, but enough for current playback, the method would disable the "active" flag so the next Play() call would re-enable the MultiSocketMonitor. This was an abuse of the flag which could result in a crash in Cancel(), because that method asserts that the period_buffer is empty, which it may be not. The solution is to add anther flag called "waiting" which shares some behavior with the old flag. --- src/output/plugins/AlsaOutputPlugin.cxx | 27 ++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/src/output/plugins/AlsaOutputPlugin.cxx b/src/output/plugins/AlsaOutputPlugin.cxx index 54aa6d9e0..96daac192 100644 --- a/src/output/plugins/AlsaOutputPlugin.cxx +++ b/src/output/plugins/AlsaOutputPlugin.cxx @@ -135,6 +135,13 @@ class AlsaOutput final */ bool active; + /** + * Is this output waiting for more data? + * + * Protected by #mutex. + */ + bool waiting; + /** * Do we need to call snd_pcm_prepare() before the next write? * It means that we put the device to SND_PCM_STATE_SETUP by @@ -176,7 +183,7 @@ class AlsaOutput final Alsa::PeriodBuffer period_buffer; /** - * Protects #cond, #error, #active, #drain. + * Protects #cond, #error, #active, #waiting, #drain. */ mutable Mutex mutex; @@ -248,6 +255,12 @@ private: return active; } + gcc_pure + bool LockIsActiveAndNotWaiting() const noexcept { + const std::lock_guard lock(mutex); + return active && !waiting; + } + /** * Activate the output by registering the sockets in the * #EventLoop. Before calling this, filling the ring buffer @@ -260,10 +273,11 @@ private: * was never unlocked */ bool Activate() noexcept { - if (active) + if (active && !waiting) return false; active = true; + waiting = false; const ScopeUnlock unlock(mutex); defer_invalidate_sockets.Schedule(); @@ -330,6 +344,7 @@ private: const std::lock_guard lock(mutex); error = std::current_exception(); active = false; + waiting = false; cond.signal(); } @@ -684,6 +699,7 @@ AlsaOutput::Open(AudioFormat &audio_format) period_buffer.Allocate(period_frames, out_frame_size); active = false; + waiting = false; must_prepare = false; written = false; error = {}; @@ -845,6 +861,7 @@ AlsaOutput::CancelInternal() noexcept ring_buffer->reset(); active = false; + waiting = false; MultiSocketMonitor::Reset(); defer_invalidate_sockets.Cancel(); @@ -930,7 +947,7 @@ AlsaOutput::Play(const void *chunk, size_t size) std::chrono::steady_clock::duration AlsaOutput::PrepareSockets() noexcept { - if (!LockIsActive()) { + if (!LockIsActiveAndNotWaiting()) { ClearSocketList(); return std::chrono::steady_clock::duration(-1); } @@ -1001,13 +1018,13 @@ try { { const std::lock_guard lock(mutex); - active = false; + waiting = true; cond.signal(); } /* avoid race condition: see if data has arrived meanwhile before disabling the - event (but after clearing the "active" + event (but after setting the "waiting" flag) */ if (!CopyRingToPeriodBuffer()) { MultiSocketMonitor::Reset();