From 6526de024a4da15ff1b0019b5bbe5048dce9e0f2 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Mon, 24 Feb 2014 20:29:01 +0100 Subject: [PATCH 1/6] output/pulse: remove bogus g_free() call --- src/output/PulseOutputPlugin.cxx | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/output/PulseOutputPlugin.cxx b/src/output/PulseOutputPlugin.cxx index ab797387d..1eece448a 100644 --- a/src/output/PulseOutputPlugin.cxx +++ b/src/output/PulseOutputPlugin.cxx @@ -369,8 +369,6 @@ pulse_output_enable(struct audio_output *ao, Error &error) po->mainloop = pa_threaded_mainloop_new(); if (po->mainloop == nullptr) { - g_free(po); - error.Set(pulse_output_domain, "pa_threaded_mainloop_new() has failed"); return false; From d34ae0850cf8ebde4b39e6865f21e50b74fe6c94 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 27 Feb 2014 23:08:22 +0100 Subject: [PATCH 2/6] AllCommands: "findadd" requires the "add" permission --- NEWS | 2 ++ src/command/AllCommands.cxx | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index df6b14373..d23544ac3 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,6 @@ ver 0.18.9 (not yet released) +* protocol + - "findadd" requires the "add" permission * decoder - vorbis: fix linker failure when libvorbis/libogg are static * encoder diff --git a/src/command/AllCommands.cxx b/src/command/AllCommands.cxx index 74802ced4..36f6fb97c 100644 --- a/src/command/AllCommands.cxx +++ b/src/command/AllCommands.cxx @@ -90,7 +90,7 @@ static const struct command commands[] = { { "disableoutput", PERMISSION_ADMIN, 1, 1, handle_disableoutput }, { "enableoutput", PERMISSION_ADMIN, 1, 1, handle_enableoutput }, { "find", PERMISSION_READ, 2, -1, handle_find }, - { "findadd", PERMISSION_READ, 2, -1, handle_findadd}, + { "findadd", PERMISSION_ADD, 2, -1, handle_findadd}, { "idle", PERMISSION_READ, 0, -1, handle_idle }, { "kill", PERMISSION_ADMIN, -1, -1, handle_kill }, { "list", PERMISSION_READ, 1, -1, handle_list }, From 0102a8665ae23f3e7807f711abbe461e58359f61 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 2 Mar 2014 10:21:31 +0100 Subject: [PATCH 3/6] event/SignalMonitor: fix build failure due to missing signal.h include --- NEWS | 1 + src/event/SignalMonitor.cxx | 2 ++ 2 files changed, 3 insertions(+) diff --git a/NEWS b/NEWS index d23544ac3..52b999a3a 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,7 @@ ver 0.18.9 (not yet released) - vorbis: fix another linker failure * output - pipe: fix hanging child process due to blocked signals +* fix build failure due to missing signal.h include ver 0.18.8 (2014/02/07) * decoder diff --git a/src/event/SignalMonitor.cxx b/src/event/SignalMonitor.cxx index eda52aba1..4f5174377 100644 --- a/src/event/SignalMonitor.cxx +++ b/src/event/SignalMonitor.cxx @@ -43,6 +43,8 @@ #include #endif +#include + class SignalMonitor final : private SocketMonitor { #ifdef USE_SIGNALFD SignalFD fd; From a884e37de16fba2dddbe61e34520860f8b88c15d Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 2 Mar 2014 11:12:09 +0100 Subject: [PATCH 4/6] output/alsa: call snd_pcm_prepare() after snd_pcm_drop() Don't wait for an optimistic write to fail. This is an improved workaround for the infamous Raspberry Pi bug (see commit af991765). It works much better and comes without the negative side effects. The old workaround is now obsolete. --- NEWS | 2 ++ src/output/AlsaOutputPlugin.cxx | 25 +++++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/NEWS b/NEWS index 52b999a3a..6d7d13de1 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,8 @@ ver 0.18.9 (not yet released) * protocol - "findadd" requires the "add" permission +* output + - alsa: improved workaround for noise after manual song change * decoder - vorbis: fix linker failure when libvorbis/libogg are static * encoder diff --git a/src/output/AlsaOutputPlugin.cxx b/src/output/AlsaOutputPlugin.cxx index 4877d3a46..c5db2320d 100644 --- a/src/output/AlsaOutputPlugin.cxx +++ b/src/output/AlsaOutputPlugin.cxx @@ -113,6 +113,18 @@ struct AlsaOutput { */ unsigned pi_workaround; + /** + * 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 + * calling snd_pcm_drop(). + * + * Without this flag, we could easily recover after a failed + * optimistic write (returning -EBADFD), but the Raspberry Pi + * audio driver is infamous for generating ugly artefacts from + * this. + */ + bool must_prepare; + /** * This buffer gets allocated after opening the ALSA device. * It contains silence samples, enough to fill one period (see @@ -699,6 +711,8 @@ alsa_open(struct audio_output *ao, AudioFormat &audio_format, Error &error) ad->in_frame_size = audio_format.GetFrameSize(); ad->out_frame_size = ad->pcm_export->GetFrameSize(audio_format); + ad->must_prepare = false; + return true; } @@ -801,6 +815,7 @@ alsa_cancel(struct audio_output *ao) AlsaOutput *ad = (AlsaOutput *)ao; ad->period_position = 0; + ad->must_prepare = true; snd_pcm_drop(ad->pcm); } @@ -822,6 +837,16 @@ alsa_play(struct audio_output *ao, const void *chunk, size_t size, assert(size % ad->in_frame_size == 0); + if (ad->must_prepare) { + ad->must_prepare = false; + + int err = snd_pcm_prepare(ad->pcm); + if (err < 0) { + error.Set(alsa_output_domain, err, snd_strerror(-err)); + return 0; + } + } + chunk = ad->pcm_export->Export(chunk, size, size); assert(size % ad->out_frame_size == 0); From 47ea69233bdf6eaef55c3120247462ae2c6f5691 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 2 Mar 2014 11:11:40 +0100 Subject: [PATCH 5/6] output/alsa: remove the obsolete Raspberry Pi workaround Has been superseded by the previous commit. --- src/output/AlsaOutputPlugin.cxx | 36 --------------------------------- 1 file changed, 36 deletions(-) diff --git a/src/output/AlsaOutputPlugin.cxx b/src/output/AlsaOutputPlugin.cxx index c5db2320d..6668c920f 100644 --- a/src/output/AlsaOutputPlugin.cxx +++ b/src/output/AlsaOutputPlugin.cxx @@ -105,14 +105,6 @@ struct AlsaOutput { */ snd_pcm_uframes_t period_position; - /** - * Set to non-zero when the Raspberry Pi workaround has been - * activated in alsa_recover(); decremented by each write. - * This will avoid activating it again, leading to an endless - * loop. This problem was observed with a "RME Digi9636/52". - */ - unsigned pi_workaround; - /** * 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 @@ -688,8 +680,6 @@ alsa_open(struct audio_output *ao, AudioFormat &audio_format, Error &error) { AlsaOutput *ad = (AlsaOutput *)ao; - ad->pi_workaround = 0; - int err = snd_pcm_open(&ad->pcm, alsa_device(ad), SND_PCM_STREAM_PLAYBACK, ad->mode); if (err < 0) { @@ -750,29 +740,6 @@ alsa_recover(AlsaOutput *ad, int err) case SND_PCM_STATE_XRUN: ad->period_position = 0; err = snd_pcm_prepare(ad->pcm); - - if (err == 0 && ad->pi_workaround == 0) { - /* this works around a driver bug observed on - the Raspberry Pi: after snd_pcm_drop(), the - whole ring buffer must be invalidated, but - the snd_pcm_prepare() call above makes the - driver play random data that just happens - to be still in the buffer; by adding and - cancelling some silence, this bug does not - occur */ - alsa_write_silence(ad, ad->period_frames); - - /* cancel the silence data right away to avoid - increasing latency; even though this - function call invalidates the portion of - silence, the driver seems to avoid the - bug */ - snd_pcm_reset(ad->pcm); - - /* disable the workaround for some time */ - ad->pi_workaround = 8; - } - break; case SND_PCM_STATE_DISCONNECTED: break; @@ -859,9 +826,6 @@ alsa_play(struct audio_output *ao, const void *chunk, size_t size, ad->period_position = (ad->period_position + ret) % ad->period_frames; - if (ad->pi_workaround > 0) - --ad->pi_workaround; - size_t bytes_written = ret * ad->out_frame_size; return ad->pcm_export->CalcSourceSize(bytes_written); } From 2784d656181230e3536d7260f3b16652602399d8 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 2 Mar 2014 11:25:01 +0100 Subject: [PATCH 6/6] release v0.18.9 --- NEWS | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 6d7d13de1..7a3173f57 100644 --- a/NEWS +++ b/NEWS @@ -1,4 +1,4 @@ -ver 0.18.9 (not yet released) +ver 0.18.9 (2014/03/02) * protocol - "findadd" requires the "add" permission * output