release v0.18.11

Fixes missing song length on high-latency Opus files.

According to tests with 320 kbit/s opus files with 60ms packets, we
need to skip up to 29 kB.

NEWS

@@ -1,3 +1,29 @@
+ver 0.18.11 (2014/05/12)
+* decoder
+  - opus: fix missing song length on high-latency files
+* fix race condition when using GLib event loop (non-Linux)
+
+ver 0.18.10 (2014/04/10)
+* decoder
+  - ffmpeg: fix seeking bug
+  - ffmpeg: handle unknown stream start time
+  - gme: fix memory leak
+  - sndfile: work around libsndfile bug on partial read
+* don't interrupt playback when current song gets deleted
+
+ver 0.18.9 (2014/03/02)
+* 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
+  - 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
   - ffmpeg: support libav v10_alpha1

configure.ac

@@ -1,6 +1,6 @@
 AC_PREREQ(2.60)
 
-AC_INIT(mpd, 0.18.8, mpd-devel@musicpd.org)
+AC_INIT(mpd, 0.18.11, mpd-devel@musicpd.org)
 
 VERSION_MAJOR=0
 VERSION_MINOR=18
@@ -1023,7 +1023,7 @@ if test x$enable_tremor = xyes; then
 	fi
 fi
 
-MPD_AUTO_PKG(vorbis, VORBIS, [vorbis vorbisfile ogg],
+MPD_AUTO_PKG(vorbis, VORBIS, [vorbisfile vorbis ogg],
 	[Ogg Vorbis decoder], [libvorbis not found])
 if test x$enable_vorbis = xyes; then
 	AC_DEFINE(ENABLE_VORBIS_DECODER, 1, [Define for Ogg Vorbis support])
@@ -1139,7 +1139,7 @@ fi
 AM_CONDITIONAL(ENABLE_FLAC_ENCODER, test x$enable_flac_encoder = xyes)
 
 dnl ---------------------------- Ogg Vorbis Encoder ---------------------------
-MPD_AUTO_PKG(vorbis_encoder, VORBISENC, [vorbisenc vorbis],
+MPD_AUTO_PKG(vorbis_encoder, VORBISENC, [vorbisenc vorbis ogg],
 	[Ogg Vorbis encoder], [libvorbisenc not found])
 
 if test x$enable_vorbis_encoder = xyes; then

src/PlaylistEdit.cxx

@@ -234,12 +234,8 @@ playlist::DeleteInternal(PlayerControl &pc,
 	if (playing && current == (int)songOrder) {
 		const bool paused = pc.GetState() == PlayerState::PAUSE;
 
-		/* the current song is going to be deleted: stop the player */
+		/* the current song is going to be deleted: see which
-
+		   song is going to be played instead */
-		pc.Stop();
-		playing = false;
-
-		/* see which song is going to be played instead */
 
 		current = queue.GetNextOrder(current);
 		if (current == (int)songOrder)
@@ -248,10 +244,12 @@ playlist::DeleteInternal(PlayerControl &pc,
 		if (current >= 0 && !paused)
 			/* play the song after the deleted one */
 			PlayOrder(pc, current);
-		else
+		else {
-			/* no songs left to play, stop playback
+			/* stop the player */
-			   completely */
+
-			Stop(pc);
+			pc.Stop();
+			playing = false;
+		}
 
 		*queued_p = nullptr;
 	} else if (current == (int)songOrder)

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 },

src/decoder/FfmpegDecoderPlugin.cxx

@@ -197,6 +197,29 @@ time_to_ffmpeg(double t, const AVRational time_base)
 			    time_base);
 }
 
+/**
+ * Replace #AV_NOPTS_VALUE with the given fallback.
+ */
+static constexpr int64_t
+timestamp_fallback(int64_t t, int64_t fallback)
+{
+	return gcc_likely(t != int64_t(AV_NOPTS_VALUE))
+		? t
+		: fallback;
+}
+
+/**
+ * Accessor for AVStream::start_time that replaces AV_NOPTS_VALUE with
+ * zero.  We can't use AV_NOPTS_VALUE in calculations, and we simply
+ * assume that the stream's start time is zero, which appears to be
+ * the best way out of that situation.
+ */
+static int64_t
+start_time_fallback(const AVStream &stream)
+{
+	return timestamp_fallback(stream.start_time, 0);
+}
+
 static void
 copy_interleave_frame2(uint8_t *dest, uint8_t **src,
 		       unsigned nframes, unsigned nchannels,
@@ -263,7 +286,7 @@ ffmpeg_send_packet(Decoder &decoder, InputStream &is,
 {
 	if (packet->pts >= 0 && packet->pts != (int64_t)AV_NOPTS_VALUE)
 		decoder_timestamp(decoder,
-				  time_from_ffmpeg(packet->pts - stream->start_time,
+				  time_from_ffmpeg(packet->pts - start_time_fallback(*stream),
 				  stream->time_base));
 
 	AVPacket packet2 = *packet;
@@ -493,10 +516,10 @@ ffmpeg_decode(Decoder &decoder, InputStream &input)
 			int64_t where =
 				time_to_ffmpeg(decoder_seek_where(decoder),
 					       av_stream->time_base) +
-				av_stream->start_time;
+				start_time_fallback(*av_stream);
 
 			if (av_seek_frame(format_context, audio_stream, where,
-					  AV_TIME_BASE) < 0)
+					  AVSEEK_FLAG_ANY) < 0)
 				decoder_seek_error(decoder);
 			else {
 				avcodec_flush_buffers(codec_context);

src/decoder/GmeDecoderPlugin.cxx

@@ -117,6 +117,7 @@ gme_container_scan(const char *path_fs, const unsigned int tnum)
 	}
 
 	const unsigned num_songs = gme_track_count(emu);
+	gme_delete(emu);
 	/* if it only contains a single tune, don't treat as container */
 	if (num_songs < 2)
 		return nullptr;

src/decoder/OggUtil.cxx

@@ -81,7 +81,7 @@ bool
 OggExpectPageSeek(ogg_sync_state &oy, ogg_page &page,
 		  Decoder *decoder, InputStream &input_stream)
 {
-	size_t remaining_skipped = 16384;
+	size_t remaining_skipped = 32768;
 
 	while (true) {
 		int r = ogg_sync_pageseek(&oy, &page);

src/decoder/SndfileDecoderPlugin.cxx

@@ -55,14 +55,28 @@ sndfile_vio_read(void *ptr, sf_count_t count, void *user_data)
 {
 	InputStream &is = *(InputStream *)user_data;
 
+	sf_count_t total_bytes = 0;
 	Error error;
-	size_t nbytes = is.LockRead(ptr, count, error);
+
-	if (nbytes == 0 && error.IsDefined()) {
+	/* this loop is necessary because libsndfile chokes on partial
+	   reads */
+
+	do {
+		size_t nbytes = is.LockRead((char *)ptr + total_bytes,
+					    count - total_bytes, error);
+		if (nbytes == 0) {
+			if (error.IsDefined()) {
 				LogError(error);
 				return -1;
 			}
 
-	return nbytes;
+			break;
+		}
+
+		total_bytes += nbytes;
+	} while (total_bytes < count);
+
+	return total_bytes;
 }
 
 static sf_count_t

src/event/DeferredMonitor.cxx

@@ -27,9 +27,11 @@ DeferredMonitor::Cancel()
 #ifdef USE_EPOLL
 	pending = false;
 #else
-	const auto id = source_id.exchange(0);
+	const ScopeLock protect(mutex);
-	if (id != 0)
+	if (source_id != 0) {
-		g_source_remove(id);
+		g_source_remove(source_id);
+		source_id = 0;
+	}
 #endif
 }
 
@@ -40,10 +42,9 @@ DeferredMonitor::Schedule()
 	if (!pending.exchange(true))
 		fd.Write();
 #else
-	const unsigned id = loop.AddIdle(Callback, this);
+	const ScopeLock protect(mutex);
-	const auto old_id = source_id.exchange(id);
+	if (source_id == 0)
-	if (old_id != 0)
+		source_id = loop.AddIdle(Callback, this);
-		g_source_remove(old_id);
 #endif
 }
 
@@ -65,8 +66,15 @@ DeferredMonitor::OnSocketReady(unsigned)
 void
 DeferredMonitor::Run()
 {
-	const auto id = source_id.exchange(0);
+	{
-	if (id != 0)
+		const ScopeLock protect(mutex);
+		if (source_id == 0)
+			/* cancelled */
+			return;
+
+		source_id = 0;
+	}
+
 	RunDeferred();
 }
 

src/event/DeferredMonitor.hxx

@@ -27,6 +27,7 @@
 #include "SocketMonitor.hxx"
 #include "WakeFD.hxx"
 #else
+#include "thread/Mutex.hxx"
 #include <glib.h>
 #endif
 
@@ -48,7 +49,9 @@ class DeferredMonitor
 #else
 	EventLoop &loop;
 
-	std::atomic<guint> source_id;
+	Mutex mutex;
+
+	guint source_id;
 #endif
 
 public:

src/event/SignalMonitor.cxx

@@ -39,6 +39,12 @@
 
 #include <algorithm>
 
+#ifdef USE_SIGNALFD
+#include <pthread.h>
+#endif
+
+#include <signal.h>
+
 class SignalMonitor final : private SocketMonitor {
 #ifdef USE_SIGNALFD
 	SignalFD fd;
@@ -99,7 +105,21 @@ static std::atomic_bool signal_pending[MAX_SIGNAL];
 
 static Manual<SignalMonitor> monitor;
 
-#ifndef USE_SIGNALFD
+#ifdef USE_SIGNALFD
+
+/**
+ * This is a pthread_atfork() callback that unblocks the signals that
+ * were blocked for our signalfd().  Without this, our child processes
+ * would inherit the blocked signals.
+ */
+static void
+at_fork_child()
+{
+	sigprocmask(SIG_UNBLOCK, &signal_mask, nullptr);
+}
+
+#else
+
 static void
 SignalCallback(int signo)
 {
@@ -108,6 +128,7 @@ SignalCallback(int signo)
 	if (!signal_pending[signo].exchange(true))
 		monitor->WakeUp();
 }
+
 #endif
 
 void
@@ -115,6 +136,8 @@ SignalMonitorInit(EventLoop &loop)
 {
 #ifdef USE_SIGNALFD
 	sigemptyset(&signal_mask);
+
+	pthread_atfork(nullptr, nullptr, at_fork_child);
 #endif
 
 	monitor.Construct(loop);

src/output/AlsaOutputPlugin.cxx

@@ -106,12 +106,16 @@ struct AlsaOutput {
 	snd_pcm_uframes_t period_position;
 
 	/**
-	 * Set to non-zero when the Raspberry Pi workaround has been
+	 * Do we need to call snd_pcm_prepare() before the next write?
-	 * activated in alsa_recover(); decremented by each write.
+	 * It means that we put the device to SND_PCM_STATE_SETUP by
-	 * This will avoid activating it again, leading to an endless
+	 * calling snd_pcm_drop().
-	 * loop.  This problem was observed with a "RME Digi9636/52".
+	 *
+	 * 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.
 	 */
-	unsigned pi_workaround;
+	bool must_prepare;
 
 	/**
 	 * This buffer gets allocated after opening the ALSA device.
@@ -676,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) {
@@ -699,6 +701,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;
 }
 
@@ -736,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;
@@ -801,6 +782,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 +804,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);
@@ -834,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);
 		}

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;