release v0.21.3

Works around a problem where MPD goes into a busy loop because
snd_pcm_drain() always returns `-EAGAIN` without making any progress
(fixes #425).

This problem was triggered by snd_pcm_drain() after snd_pcm_cancel()
and snd_pcm_prepare(), but without submitting any data with
snd_pcm_writei().

I believe this is a kernel bug: in non-blocking mode, the kernel's
snd_pcm_drain() function returns early.  In this mode, it only checks
whether snd_pcm_drain_done() has been called already, but
snd_pcm_drain_done() is never called if no data was submitted.

In blocking mode, the following `for` loop detects this condition, so
snd_pcm_drain_done() is not necessary, but without this extra check,
we get `-EAGAIN` forever.

NEWS

@@ -1,3 +1,11 @@
+ver 0.21.3 (2018/11/16)
+* output
+  - alsa: fix crash bug
+  - alsa: fix stuttering at start of playback
+  - alsa: fix discarded samples at end of song
+  - alsa: clear error after reopening device
+* log: default to journal if MPD was started as systemd service
+
 ver 0.21.2 (2018/11/12)
 * protocol
   - operator "=~" matches a regular expression

android/AndroidManifest.xml

@@ -2,8 +2,8 @@
 <manifest xmlns:android="http://schemas.android.com/apk/res/android"
           package="org.musicpd"
           android:installLocation="auto"
-          android:versionCode="24"
+          android:versionCode="25"
-          android:versionName="0.21.2">
+          android:versionName="0.21.3">
 
   <uses-sdk android:minSdkVersion="21" android:targetSdkVersion="26"/>
 

doc/conf.py

@@ -38,7 +38,7 @@ author = 'Max Kellermann'
 # built documents.
 #
 # The short X.Y version.
-version = '0.21.2'
+version = '0.21.3'
 # The full version, including alpha/beta/rc tags.
 release = version
 

doc/mpdconf.example

@@ -32,7 +32,7 @@
 # settings.
 #
 # The special value "syslog" makes MPD use the local syslog daemon. This
-# setting defaults to logging to syslog, otherwise logging is disabled.
+# setting defaults to logging to syslog.
 #
 #log_file			"~/.mpd/log"
 #

doc/protocol.rst

@@ -556,7 +556,7 @@ The Queue
 There are two ways to address songs within the queue: by their
 position and by their id.
 
-The position is a 1-based index.  It is unstable by design: if you
+The position is a 0-based index.  It is unstable by design: if you
 move, delete or insert songs, all following indices will change, and a
 client can never be sure what song is behind a given index/position.
 

meson.build

@@ -1,7 +1,7 @@
 project(
   'mpd',
   ['c', 'cpp'],
-  version: '0.21.2',
+  version: '0.21.3',
   meson_version: '>= 0.47.2',
   default_options: [
     'c_std=c99',

python/build/libs.py

@@ -112,8 +112,8 @@ liblame = AutotoolsProject(
 )
 
 ffmpeg = FfmpegProject(
-    'http://ffmpeg.org/releases/ffmpeg-4.0.2.tar.xz',
+    'http://ffmpeg.org/releases/ffmpeg-4.1.tar.xz',
-    'a95c0cc9eb990e94031d2183f2e6e444cc61c99f6f182d1575c433d62afb2f97',
+    'a38ec4d026efb58506a99ad5cd23d5a9793b4bf415f2c4c2e9c1bb444acd1994',
     'lib/libavcodec.a',
     [
         '--disable-shared', '--enable-static',
@@ -341,8 +341,8 @@ ffmpeg = FfmpegProject(
 )
 
 curl = AutotoolsProject(
-    'http://curl.haxx.se/download/curl-7.61.1.tar.xz',
+    'http://curl.haxx.se/download/curl-7.62.0.tar.xz',
-    '3d5913d6a39bd22e68e34dff697fd6e4c3c81563f580c76fca2009315cd81891',
+    'dab5643a5fe775ae92570b9f3df6b0ef4bc2a827a959361fb130c73b721275c1',
     'lib/libcurl.a',
     [
         '--disable-shared', '--enable-static',

src/LogInit.cxx

@@ -30,6 +30,10 @@
 #include "util/RuntimeError.hxx"
 #include "system/Error.hxx"
 
+#ifdef ENABLE_SYSTEMD_DAEMON
+#include <systemd/sd-daemon.h>
+#endif
+
 #include <assert.h>
 #include <string.h>
 #include <fcntl.h>
@@ -139,6 +143,16 @@ log_init(const ConfigData &config, bool verbose, bool use_stdout)
 		if (param == nullptr) {
 			/* no configuration: default to syslog (if
 			   available) */
+#ifdef ENABLE_SYSTEMD_DAEMON
+			if (sd_booted() &&
+			    getenv("NOTIFY_SOCKET") != nullptr) {
+				/* if MPD was started as a systemd
+				   service, default to journal (which
+				   is connected to fd=2) */
+				out_fd = STDOUT_FILENO;
+				return;
+			}
+#endif
 #ifndef HAVE_SYSLOG
 			throw std::runtime_error("config parameter 'log_file' not found");
 #endif

src/event/Thread.cxx

@@ -20,6 +20,7 @@
 #include "config.h"
 #include "Thread.hxx"
 #include "thread/Name.hxx"
+#include "thread/Slack.hxx"
 #include "thread/Util.hxx"
 #include "Log.hxx"
 
@@ -46,6 +47,8 @@ EventThread::Run() noexcept
 	SetThreadName(realtime ? "rtio" : "io");
 
 	if (realtime) {
+		SetThreadTimerSlackUS(10);
+
 		try {
 			SetThreadRealtime();
 		} catch (...) {

src/output/plugins/AlsaOutputPlugin.cxx

@@ -33,6 +33,7 @@
 #include "util/RuntimeError.hxx"
 #include "util/Domain.hxx"
 #include "util/ConstBuffer.hxx"
+#include "util/ScopeExit.hxx"
 #include "util/StringView.hxx"
 #include "event/MultiSocketMonitor.hxx"
 #include "event/DeferEvent.hxx"
@@ -108,6 +109,13 @@ class AlsaOutput final
 	 */
 	snd_pcm_uframes_t period_frames;
 
+	/**
+	 * If snd_pcm_avail() goes above this value and no more data
+	 * is available in the #ring_buffer, we need to play some
+	 * silence.
+	 */
+	snd_pcm_sframes_t max_avail_frames;
+
 	/**
 	 * Is this a buggy alsa-lib version, which needs a workaround
 	 * for the snd_pcm_drain() bug always returning -EAGAIN?  See
@@ -139,6 +147,16 @@ class AlsaOutput final
 	 */
 	bool must_prepare;
 
+	/**
+	 * Has snd_pcm_writei() been called successfully at least once
+	 * since the PCM was prepared?
+	 *
+	 * This is necessary to work around a kernel bug which causes
+	 * snd_pcm_drain() to return -EAGAIN forever in non-blocking
+	 * mode if snd_pcm_writei() was never called.
+	 */
+	bool written;
+
 	bool drain;
 
 	/**
@@ -257,10 +275,12 @@ private:
 	/**
 	 * Drain all buffers.  To be run in #EventLoop's thread.
 	 *
+	 * Throws on error.
+	 *
 	 * @return true if draining is complete, false if this method
 	 * needs to be called again later
 	 */
-	bool DrainInternal() noexcept;
+	bool DrainInternal();
 
 	/**
 	 * Stop playback immediately, dropping all buffers.  To be run
@@ -295,14 +315,18 @@ private:
 
 		auto frames_written = snd_pcm_writei(pcm, period_buffer.GetHead(),
 						     period_buffer.GetFrames(out_frame_size));
-		if (frames_written > 0)
+		if (frames_written > 0) {
+			written = true;
 			period_buffer.ConsumeFrames(frames_written,
 						    out_frame_size);
+		}
 
 		return frames_written;
 	}
 
 	void LockCaughtError() noexcept {
+		period_buffer.Clear();
+
 		const std::lock_guard<Mutex> lock(mutex);
 		error = std::current_exception();
 		active = false;
@@ -477,6 +501,10 @@ AlsaOutput::Setup(AudioFormat &audio_format,
 
 	period_frames = alsa_period_size;
 
+	/* generate silence if there's less than once period of data
+	   in the ALSA-PCM buffer */
+	max_avail_frames = hw_result.buffer_size - hw_result.period_size;
+
 	silence = new uint8_t[snd_pcm_frames_to_bytes(pcm, alsa_period_size)];
 	snd_pcm_format_set_silence(hw_result.format, silence,
 				   alsa_period_size * audio_format.channels);
@@ -657,6 +685,8 @@ AlsaOutput::Open(AudioFormat &audio_format)
 
 	active = false;
 	must_prepare = false;
+	written = false;
+	error = {};
 }
 
 inline int
@@ -688,6 +718,7 @@ AlsaOutput::Recover(int err) noexcept
 	case SND_PCM_STATE_SETUP:
 	case SND_PCM_STATE_XRUN:
 		period_buffer.Rewind();
+		written = false;
 		err = snd_pcm_prepare(pcm);
 		break;
 	case SND_PCM_STATE_DISCONNECTED:
@@ -710,13 +741,8 @@ AlsaOutput::Recover(int err) noexcept
 }
 
 inline bool
-AlsaOutput::DrainInternal() noexcept
+AlsaOutput::DrainInternal()
 {
-	if (snd_pcm_state(pcm) != SND_PCM_STATE_RUNNING) {
-		CancelInternal();
-		return true;
-	}
-
 	/* drain ring_buffer */
 	CopyRingToPeriodBuffer();
 
@@ -729,28 +755,42 @@ AlsaOutput::DrainInternal() noexcept
 	/* drain period_buffer */
 	if (!period_buffer.IsEmpty()) {
 		auto frames_written = WriteFromPeriodBuffer();
-		if (frames_written < 0 && errno != EAGAIN) {
+		if (frames_written < 0) {
-			CancelInternal();
+			if (frames_written == -EAGAIN)
-			return true;
+				return false;
+
+			throw FormatRuntimeError("snd_pcm_writei() failed: %s",
+						 snd_strerror(-frames_written));
 		}
 
-		if (!period_buffer.IsEmpty())
+		/* need to call CopyRingToPeriodBuffer() and
-			/* need to call WriteFromPeriodBuffer() again
+		   WriteFromPeriodBuffer() again in the next
-			   in the next iteration, so don't finish the
+		   iteration, so don't finish the drain just yet */
-			   drain just yet */
+		return period_buffer.IsEmpty();
-			return false;
 	}
 
+	if (!written)
+		/* if nothing has ever been written to the PCM, we
+		   don't need to drain it */
+		return true;
+
 	/* .. and finally drain the ALSA hardware buffer */
 
+	int result;
 	if (work_around_drain_bug) {
 		snd_pcm_nonblock(pcm, 0);
-		bool result = snd_pcm_drain(pcm) != -EAGAIN;
+		result = snd_pcm_drain(pcm);
 		snd_pcm_nonblock(pcm, 1);
-		return result;
+	} else
-	}
+		result = snd_pcm_drain(pcm);
 
-	return snd_pcm_drain(pcm) != -EAGAIN;
+	if (result == 0)
+		return true;
+	else if (result == -EAGAIN)
+		return false;
+	else
+		throw FormatRuntimeError("snd_pcm_drain() failed: %s",
+					 snd_strerror(-result));
 }
 
 void
@@ -775,6 +815,9 @@ AlsaOutput::Drain()
 inline void
 AlsaOutput::CancelInternal() noexcept
 {
+	/* this method doesn't need to lock the mutex because while it
+	   runs, the calling thread is blocked inside Cancel() */
+
 	must_prepare = true;
 
 	snd_pcm_drop(pcm);
@@ -783,10 +826,7 @@ AlsaOutput::CancelInternal() noexcept
 	period_buffer.Clear();
 	ring_buffer->reset();
 
-	{
+	active = false;
-		const std::lock_guard<Mutex> lock(mutex);
-		active = false;
-	}
 
 	MultiSocketMonitor::Reset();
 	defer_invalidate_sockets.Cancel();
@@ -891,6 +931,16 @@ AlsaOutput::DispatchSockets() noexcept
 try {
 	non_block.DispatchSockets(*this, pcm);
 
+	if (must_prepare) {
+		must_prepare = false;
+		written = false;
+
+		int err = snd_pcm_prepare(pcm);
+		if (err < 0)
+			throw FormatRuntimeError("snd_pcm_prepare() failed: %s",
+						 snd_strerror(-err));
+	}
+
 	{
 		const std::lock_guard<Mutex> lock(mutex);
 
@@ -911,19 +961,11 @@ try {
 		}
 	}
 
-	if (must_prepare) {
-		must_prepare = false;
-
-		int err = snd_pcm_prepare(pcm);
-		if (err < 0)
-			throw FormatRuntimeError("snd_pcm_prepare() failed: %s",
-						 snd_strerror(-err));
-	}
-
 	CopyRingToPeriodBuffer();
 
 	if (period_buffer.IsEmpty()) {
-		if (snd_pcm_state(pcm) == SND_PCM_STATE_PREPARED) {
+		if (snd_pcm_state(pcm) == SND_PCM_STATE_PREPARED ||
+		    snd_pcm_avail(pcm) <= max_avail_frames) {
 			/* at SND_PCM_STATE_PREPARED (not yet switched
 			   to SND_PCM_STATE_RUNNING), we have no
 			   pressure to fill the ALSA buffer, because
@@ -933,10 +975,16 @@ try {
 			   monitoring the ALSA file descriptor, and
 			   let it be reactivated by Play()/Activate()
 			   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
+			   start of playback, when our ring_buffer is
+			   smaller than the ALSA-PCM buffer */
 
 			{
 				const std::lock_guard<Mutex> lock(mutex);
 				active = false;
+				cond.signal();
 			}
 
 			/* avoid race condition: see if data has

test/meson.build

@@ -239,6 +239,7 @@ if enable_database
     '../src/LogBackend.cxx',
     include_directories: inc,
     dependencies: [
+      event_dep,
       storage_glue_dep,
     ],
   )