From fac15aaffbc2a9016c013188fb062407e3cebe20 Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@musicpd.org>
Date: Fri, 28 Jun 2019 14:39:52 +0200
Subject: [PATCH 1/7] output/alsa: fix comment typo
---
src/output/plugins/AlsaOutputPlugin.cxx | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/output/plugins/AlsaOutputPlugin.cxx b/src/output/plugins/AlsaOutputPlugin.cxx
index 16832a1bc..e9e68c0ce 100644
--- a/src/output/plugins/AlsaOutputPlugin.cxx
+++ b/src/output/plugins/AlsaOutputPlugin.cxx
@@ -995,7 +995,7 @@ try {
whenever more data arrives */
/* the same applies when there is still enough
data in the ALSA-PCM buffer (determined by
- snd_pcm_avail()); this can happend at the
+ snd_pcm_avail()); this can happen at the
start of playback, when our ring_buffer is
smaller than the ALSA-PCM buffer */
From 3da1fa88d0688c32f5db4948c3d86f3b0933bd4d Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@musicpd.org>
Date: Fri, 28 Jun 2019 18:04:45 +0200
Subject: [PATCH 2/7] output/alsa: fix comment typo
---
src/output/plugins/AlsaOutputPlugin.cxx | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/output/plugins/AlsaOutputPlugin.cxx b/src/output/plugins/AlsaOutputPlugin.cxx
index e9e68c0ce..eac424622 100644
--- a/src/output/plugins/AlsaOutputPlugin.cxx
+++ b/src/output/plugins/AlsaOutputPlugin.cxx
@@ -501,7 +501,7 @@ AlsaOutput::Setup(AudioFormat &audio_format,
period_frames = alsa_period_size;
- /* generate silence if there's less than once period of data
+ /* generate silence if there's less than one period of data
in the ALSA-PCM buffer */
max_avail_frames = hw_result.buffer_size - hw_result.period_size;
From 3c5f860fb8164ccc4bb714cc39f3e6a3e982862a Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@musicpd.org>
Date: Fri, 28 Jun 2019 17:03:26 +0200
Subject: [PATCH 3/7] output/alsa: Cancel() also affects "active"
(documentation)
---
src/output/plugins/AlsaOutputPlugin.cxx | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/output/plugins/AlsaOutputPlugin.cxx b/src/output/plugins/AlsaOutputPlugin.cxx
index eac424622..54aa6d9e0 100644
--- a/src/output/plugins/AlsaOutputPlugin.cxx
+++ b/src/output/plugins/AlsaOutputPlugin.cxx
@@ -128,8 +128,8 @@ class AlsaOutput final
bool work_around_drain_bug;
/**
- * After Open(), has this output been activated by a Play()
- * command?
+ * After Open() or Cancel(), has this output been activated by
+ * a Play() command?
*
* Protected by #mutex.
*/
From 0c0a35475313c6bb96c7cdf6059d245746f3843d Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@musicpd.org>
Date: Fri, 28 Jun 2019 15:30:26 +0200
Subject: [PATCH 4/7] 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<Mutex> 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<Mutex> 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<Mutex> 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();
From 61a72a5d131e89f069d2d2de9411de8fe4c21f23 Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@musicpd.org>
Date: Fri, 28 Jun 2019 14:51:27 +0200
Subject: [PATCH 5/7] output/alsa: schedule a timer to generate silence
Without this timer, DispatchSockets() may disable the
MultiSocketMonitor and if Play() doesn't get called soon, it never
gets a chance to generate silence. However if Play() gets called,
generating silence isn't necessary anymore...
Resulting from this misdesign (added by commit ccafe3f3cf3 in 0.21.3),
the silence generator didn't work reliably.
---
NEWS | 1 +
src/output/plugins/AlsaOutputPlugin.cxx | 39 +++++++++++++++++++++++++
2 files changed, 40 insertions(+)
diff --git a/NEWS b/NEWS
index 542cd5d04..354491ea5 100644
--- a/NEWS
+++ b/NEWS
@@ -6,6 +6,7 @@ ver 0.21.11 (not yet released)
* output
- alsa: fix busy loop while draining
- alsa: fix missing drain call
+ - alsa: improve xrun-avoiding silence generator
- alsa, osx: fix distortions with DSD_U32 and DoP on 32 bit CPUs
* protocol
- fix "list" with multiple "group" levels
diff --git a/src/output/plugins/AlsaOutputPlugin.cxx b/src/output/plugins/AlsaOutputPlugin.cxx
index 96daac192..7e1919e43 100644
--- a/src/output/plugins/AlsaOutputPlugin.cxx
+++ b/src/output/plugins/AlsaOutputPlugin.cxx
@@ -56,6 +56,15 @@ class AlsaOutput final
DeferEvent defer_invalidate_sockets;
+ /**
+ * This timer is used to re-schedule the #MultiSocketMonitor
+ * after it had been disabled to wait for the next Play() call
+ * to deliver more data. This timer is necessary to start
+ * generating silence if Play() doesn't get called soon enough
+ * to avoid the xrun.
+ */
+ TimerEvent silence_timer;
+
Manual<PcmExport> pcm_export;
/**
@@ -109,6 +118,8 @@ class AlsaOutput final
*/
snd_pcm_uframes_t period_frames;
+ std::chrono::steady_clock::duration effective_period_duration;
+
/**
* If snd_pcm_avail() goes above this value and no more data
* is available in the #ring_buffer, we need to play some
@@ -348,6 +359,19 @@ private:
cond.signal();
}
+ /**
+ * Callback for @silence_timer
+ */
+ void OnSilenceTimer() noexcept {
+ {
+ const std::lock_guard<Mutex> lock(mutex);
+ assert(active);
+ waiting = false;
+ }
+
+ MultiSocketMonitor::InvalidateSockets();
+ }
+
/* virtual methods from class MultiSocketMonitor */
std::chrono::steady_clock::duration PrepareSockets() noexcept override;
void DispatchSockets() noexcept override;
@@ -359,6 +383,7 @@ AlsaOutput::AlsaOutput(EventLoop &_loop, const ConfigBlock &block)
:AudioOutput(FLAG_ENABLE_DISABLE),
MultiSocketMonitor(_loop),
defer_invalidate_sockets(_loop, BIND_THIS_METHOD(InvalidateSockets)),
+ silence_timer(_loop, BIND_THIS_METHOD(OnSilenceTimer)),
device(block.GetBlockValue("device", "")),
#ifdef ENABLE_DSD
dop_setting(block.GetBlockValue("dop", false) ||
@@ -515,6 +540,7 @@ AlsaOutput::Setup(AudioFormat &audio_format,
alsa_period_size = 1;
period_frames = alsa_period_size;
+ effective_period_duration = audio_format.FramesToTime<decltype(effective_period_duration)>(period_frames);
/* generate silence if there's less than one period of data
in the ALSA-PCM buffer */
@@ -865,6 +891,7 @@ AlsaOutput::CancelInternal() noexcept
MultiSocketMonitor::Reset();
defer_invalidate_sockets.Cancel();
+ silence_timer.Cancel();
}
void
@@ -893,6 +920,7 @@ AlsaOutput::Close() noexcept
BlockingCall(GetEventLoop(), [this](){
MultiSocketMonitor::Reset();
defer_invalidate_sockets.Cancel();
+ silence_timer.Cancel();
});
period_buffer.Free();
@@ -1029,6 +1057,17 @@ try {
if (!CopyRingToPeriodBuffer()) {
MultiSocketMonitor::Reset();
defer_invalidate_sockets.Cancel();
+
+ /* just in case Play() doesn't get
+ called soon enough, schedule a
+ timer which generates silence
+ before the xrun occurs */
+ /* the timer fires in half of a
+ period; this short duration may
+ produce a few more wakeups than
+ necessary, but should be small
+ enough to avoid the xrun */
+ silence_timer.Schedule(effective_period_duration / 2);
}
return;
From f780ac418a38a0a86ffcead364a27367b9210710 Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@musicpd.org>
Date: Fri, 28 Jun 2019 14:35:56 +0200
Subject: [PATCH 6/7] output/alsa: log when generating silence due to slow
decoder
MPD used to do that when this code lived in the player thread, but it
was removed by commit 98a7c62d7a4f716d90af6d78e18d1a3b10bc54b3; and
the replacement code in the ALSA output plugin didn't have it.
---
NEWS | 1 +
src/output/plugins/AlsaOutputPlugin.cxx | 6 ++++++
2 files changed, 7 insertions(+)
diff --git a/NEWS b/NEWS
index 354491ea5..8c8f514a5 100644
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,7 @@ ver 0.21.11 (not yet released)
- alsa: fix busy loop while draining
- alsa: fix missing drain call
- alsa: improve xrun-avoiding silence generator
+ - alsa: log when generating silence due to slow decoder
- alsa, osx: fix distortions with DSD_U32 and DoP on 32 bit CPUs
* protocol
- fix "list" with multiple "group" levels
diff --git a/src/output/plugins/AlsaOutputPlugin.cxx b/src/output/plugins/AlsaOutputPlugin.cxx
index 7e1919e43..8ccdd2121 100644
--- a/src/output/plugins/AlsaOutputPlugin.cxx
+++ b/src/output/plugins/AlsaOutputPlugin.cxx
@@ -27,6 +27,7 @@
#include "../OutputAPI.hxx"
#include "mixer/MixerList.hxx"
#include "pcm/PcmExport.hxx"
+#include "system/PeriodClock.hxx"
#include "thread/Mutex.hxx"
#include "thread/Cond.hxx"
#include "util/Manual.hxx"
@@ -65,6 +66,8 @@ class AlsaOutput final
*/
TimerEvent silence_timer;
+ PeriodClock throttle_silence_log;
+
Manual<PcmExport> pcm_export;
/**
@@ -1073,6 +1076,9 @@ try {
return;
}
+ if (throttle_silence_log.CheckUpdate(std::chrono::seconds(5)))
+ FormatWarning(alsa_output_domain, "Decoder is too slow; playing silence to avoid xrun");
+
/* insert some silence if the buffer has not enough
data yet, to avoid ALSA xrun */
period_buffer.FillWithSilence(silence, out_frame_size);
From f6125f0c354491c47c854cd061af211bd8716b09 Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@musicpd.org>
Date: Wed, 3 Jul 2019 15:16:27 +0200
Subject: [PATCH 7/7] release v0.21.11
---
NEWS | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/NEWS b/NEWS
index 8c8f514a5..b5972e4b5 100644
--- a/NEWS
+++ b/NEWS
@@ -1,4 +1,4 @@
-ver 0.21.11 (not yet released)
+ver 0.21.11 (2019/07/03)
* input
- tidal: deprecated because Tidal has changed the protocol
* decoder