release v0.20.12

Closes #155

Makefile.am

@@ -243,6 +243,7 @@ CURL_SOURCES = \
 	src/lib/curl/Slist.hxx
 
 UPNP_SOURCES = \
+	src/lib/upnp/Compat.hxx \
 	src/lib/upnp/Init.cxx src/lib/upnp/Init.hxx \
 	src/lib/upnp/ClientInit.cxx src/lib/upnp/ClientInit.hxx \
 	src/lib/upnp/Device.cxx src/lib/upnp/Device.hxx \

NEWS

@@ -1,3 +1,21 @@
+ver 0.20.12 (2017/11/25)
+* database
+  - upnp: adapt to libupnp 1.8 API changes
+* input
+  - cdio_paranoia, ffmpeg, file, smbclient: reduce lock contention,
+    fixing lots of xrun problems
+  - curl: fix seeking
+* decoder
+  - ffmpeg: fix GCC 8 warning
+  - vorbis: fix Tremor support
+* player
+  - log message when decoder is too slow
+* encoder
+  - vorbis: default to quality 3
+* output
+  - fix hanging playback with soxr resampler
+  - httpd: flush encoder after tag; fixes corrupt Vorbis stream
+
 ver 0.20.11 (2017/10/18)
 * storage
   - curl: support Content-Type application/xml

configure.ac

@@ -1,10 +1,10 @@
 AC_PREREQ(2.60)
 
-AC_INIT(mpd, 0.20.11, musicpd-dev-team@lists.sourceforge.net)
+AC_INIT(mpd, 0.20.12, musicpd-dev-team@lists.sourceforge.net)
 
 VERSION_MAJOR=0
 VERSION_MINOR=20
-VERSION_REVISION=11
+VERSION_REVISION=12
 VERSION_EXTRA=0
 
 AC_CONFIG_SRCDIR([src/Main.cxx])
@@ -1385,6 +1385,11 @@ then
 	AX_APPEND_COMPILE_FLAGS([-Wcast-qual])
 	AX_APPEND_COMPILE_FLAGS([-Wwrite-strings])
 	AX_APPEND_COMPILE_FLAGS([-Wsign-compare])
+
+	dnl This GCC8 warning for C++17 ABI compatibility is of no
+	dnl interest for us, because we're not a shared library.
+	AX_APPEND_COMPILE_FLAGS([-Wno-noexcept-type])
+
 	AC_LANG_POP
 fi
 

doc/user.xml

@@ -3068,8 +3068,8 @@ run</programlisting>
                 </entry>
                 <entry>
                   Sets the quality for VBR.  -1 is the lowest quality,
-                  10 is the highest quality.  Cannot be used with
-                  <varname>bitrate</varname>.
+                  10 is the highest quality.  Defaults to 3.  Cannot
+                  be used with <varname>bitrate</varname>.
                 </entry>
               </row>
               <row>

src/decoder/plugins/FfmpegDecoderPlugin.cxx

@@ -258,7 +258,7 @@ FfmpegSendFrame(DecoderClient &client, InputStream &is,
 	try {
 		output_buffer = copy_interleave_frame(codec_context, frame,
 						      buffer);
-	} catch (const std::exception e) {
+	} catch (const std::exception &e) {
 		/* this must be a serious error, e.g. OOM */
 		LogError(e);
 		return DecoderCommand::STOP;

src/decoder/plugins/VorbisDecoderPlugin.cxx

@@ -178,6 +178,20 @@ VorbisDecoder::SubmitInit()
 	client.Ready(audio_format, eos_granulepos > 0, duration);
 }
 
+#ifdef HAVE_TREMOR
+static inline int16_t tremor_clip_sample(int32_t x)
+{
+	x >>= 9;
+
+	if (x < INT16_MIN)
+		return INT16_MIN;
+	if (x > INT16_MAX)
+		return INT16_MAX;
+
+	return x;
+}
+#endif
+
 bool
 VorbisDecoder::SubmitSomePcm()
 {
@@ -197,7 +211,7 @@ VorbisDecoder::SubmitSomePcm()
 		auto *dest = &buffer[c];
 
 		for (size_t i = 0; i < n_frames; ++i) {
-			*dest = *src++;
+			*dest = tremor_clip_sample(*src++);
 			dest += channels;
 		}
 	}

src/encoder/plugins/VorbisEncoderPlugin.cxx

@@ -62,7 +62,7 @@ private:
 };
 
 class PreparedVorbisEncoder final : public PreparedEncoder {
-	float quality;
+	float quality = 3;
 	int bitrate;
 
 public:
@@ -97,7 +97,7 @@ PreparedVorbisEncoder::PreparedVorbisEncoder(const ConfigBlock &block)
 
 		value = block.GetBlockValue("bitrate");
 		if (value == nullptr)
-			throw std::runtime_error("neither bitrate nor quality defined");
+			return;
 
 		quality = -2.0;
 

src/input/plugins/CdioParanoiaInputPlugin.cxx

@@ -270,7 +270,10 @@ CdioParanoiaInputStream::Seek(offset_type new_offset)
 	lsn_relofs = new_offset / CDIO_CD_FRAMESIZE_RAW;
 	offset = new_offset;
 
-	cdio_paranoia_seek(para, lsn_from + lsn_relofs, SEEK_SET);
+	{
+		const ScopeUnlock unlock(mutex);
+		cdio_paranoia_seek(para, lsn_from + lsn_relofs, SEEK_SET);
+	}
 }
 
 size_t
@@ -292,6 +295,8 @@ CdioParanoiaInputStream::Read(void *ptr, size_t length)
 
 		//current sector was changed ?
 		if (lsn_relofs != buffer_lsn) {
+			const ScopeUnlock unlock(mutex);
+
 			rbuf = cdio_paranoia_read(para, nullptr);
 
 			s_err = cdda_errors(drv);

src/input/plugins/CurlInputPlugin.cxx

@@ -64,7 +64,6 @@ static const size_t CURL_RESUME_AT = 384 * 1024;
 struct CurlInputStream final : public AsyncInputStream, CurlResponseHandler {
 	/* some buffers which were passed to libcurl, which we have
 	   too free */
-	char range[32];
 	CurlSlist request_headers;
 
 	CurlRequest *request = nullptr;
@@ -86,8 +85,19 @@ struct CurlInputStream final : public AsyncInputStream, CurlResponseHandler {
 
 	static InputStream *Open(const char *url, Mutex &mutex, Cond &cond);
 
+	/**
+	 * Create and initialize a new #CurlRequest instance.  After
+	 * this, you may add more request headers and set options.  To
+	 * actually start the request, call StartRequest().
+	 */
 	void InitEasy();
 
+	/**
+	 * Start the request after having called InitEasy().  After
+	 * this, you must not set any CURL options.
+	 */
+	void StartRequest();
+
 	/**
 	 * Frees the current "libcurl easy" handle, and everything
 	 * associated with it.
@@ -372,6 +382,11 @@ CurlInputStream::InitEasy()
 
 	request_headers.Clear();
 	request_headers.Append("Icy-Metadata: 1");
+}
+
+void
+CurlInputStream::StartRequest()
+{
 	request->SetOption(CURLOPT_HTTPHEADER, request_headers.Get());
 
 	request->Start();
@@ -398,6 +413,7 @@ CurlInputStream::SeekInternal(offset_type new_offset)
 	/* send the "Range" header */
 
 	if (offset > 0) {
+		char range[32];
 #ifdef WIN32
 		// TODO: what can we use on Windows to format 64 bit?
 		sprintf(range, "%lu-", (long)offset);
@@ -406,6 +422,8 @@ CurlInputStream::SeekInternal(offset_type new_offset)
 #endif
 		request->SetOption(CURLOPT_RANGE, range);
 	}
+
+	StartRequest();
 }
 
 void
@@ -428,6 +446,7 @@ CurlInputStream::Open(const char *url, Mutex &mutex, Cond &cond)
 	try {
 		BlockingCall(io_thread_get(), [c](){
 				c->InitEasy();
+				c->StartRequest();
 			});
 	} catch (...) {
 		delete c;

src/input/plugins/FfmpegInputPlugin.cxx

@@ -104,7 +104,13 @@ input_ffmpeg_open(const char *uri,
 size_t
 FfmpegInputStream::Read(void *ptr, size_t read_size)
 {
-	auto result = avio_read(h, (unsigned char *)ptr, read_size);
+	int result;
+
+	{
+		const ScopeUnlock unlock(mutex);
+		result = avio_read(h, (unsigned char *)ptr, read_size);
+	}
+
 	if (result <= 0) {
 		if (result < 0)
 			throw MakeFfmpegError(result, "avio_read() failed");
@@ -126,7 +132,12 @@ FfmpegInputStream::IsEOF() noexcept
 void
 FfmpegInputStream::Seek(offset_type new_offset)
 {
-	auto result = avio_seek(h, new_offset, SEEK_SET);
+	int64_t result;
+
+	{
+		const ScopeUnlock unlock(mutex);
+		result = avio_seek(h, new_offset, SEEK_SET);
+	}
 
 	if (result < 0)
 		throw MakeFfmpegError(result, "avio_seek() failed");

src/input/plugins/FileInputPlugin.cxx

@@ -87,14 +87,24 @@ input_file_open(gcc_unused const char *filename,
 void
 FileInputStream::Seek(offset_type new_offset)
 {
-	reader.Seek((off_t)new_offset);
+	{
+		const ScopeUnlock unlock(mutex);
+		reader.Seek((off_t)new_offset);
+	}
+
 	offset = new_offset;
 }
 
 size_t
 FileInputStream::Read(void *ptr, size_t read_size)
 {
-	size_t nbytes = reader.Read(ptr, read_size);
+	size_t nbytes;
+
+	{
+		const ScopeUnlock unlock(mutex);
+		nbytes = reader.Read(ptr, read_size);
+	}
+
 	offset += nbytes;
 	return nbytes;
 }

src/input/plugins/SmbclientInputPlugin.cxx

@@ -125,9 +125,14 @@ input_smbclient_open(const char *uri,
 size_t
 SmbclientInputStream::Read(void *ptr, size_t read_size)
 {
-	smbclient_mutex.lock();
-	ssize_t nbytes = smbc_read(fd, ptr, read_size);
-	smbclient_mutex.unlock();
+	ssize_t nbytes;
+
+	{
+		const ScopeUnlock unlock(mutex);
+		const std::lock_guard<Mutex> lock(smbclient_mutex);
+		nbytes = smbc_read(fd, ptr, read_size);
+	}
+
 	if (nbytes < 0)
 		throw MakeErrno("smbc_read() failed");
 
@@ -138,9 +143,14 @@ SmbclientInputStream::Read(void *ptr, size_t read_size)
 void
 SmbclientInputStream::Seek(offset_type new_offset)
 {
-	smbclient_mutex.lock();
-	off_t result = smbc_lseek(fd, new_offset, SEEK_SET);
-	smbclient_mutex.unlock();
+	off_t result;
+
+	{
+		const ScopeUnlock unlock(mutex);
+		const std::lock_guard<Mutex> lock(smbclient_mutex);
+		result = smbc_lseek(fd, new_offset, SEEK_SET);
+	}
+
 	if (result < 0)
 		throw MakeErrno("smbc_lseek() failed");
 

src/lib/upnp/Callback.hxx

@@ -40,7 +40,7 @@ public:
 		return *(UpnpCallback *)cookie;
 	}
 
-	virtual int Invoke(Upnp_EventType et, void *evp) = 0;
+	virtual int Invoke(Upnp_EventType et, const void *evp) = 0;
 };
 
 #endif

src/lib/upnp/ClientInit.cxx

@@ -33,7 +33,12 @@ static unsigned upnp_client_ref;
 static UpnpClient_Handle upnp_client_handle;
 
 static int
-UpnpClientCallback(Upnp_EventType et, void *evp, void *cookie)
+UpnpClientCallback(Upnp_EventType et,
+#if UPNP_VERSION >= 10800
+		   const
+#endif
+		   void *evp,
+		   void *cookie)
 {
 	if (cookie == nullptr)
 		/* this is the cookie passed to UpnpRegisterClient();

src/lib/upnp/Compat.hxx

@@ -0,0 +1,69 @@
+/*
+ * Copyright 2003-2017 The Music Player Daemon Project
+ * http://www.musicpd.org
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#ifndef MPD_UPNP_COMPAT_HXX
+#define MPD_UPNP_COMPAT_HXX
+
+#include <upnp/upnp.h>
+
+#if UPNP_VERSION < 10800
+#include "Compiler.h"
+
+/* emulate the libupnp 1.8 API with older versions */
+
+using UpnpDiscovery = Upnp_Discovery;
+
+gcc_pure
+static inline int
+UpnpDiscovery_get_Expires(const UpnpDiscovery *disco) noexcept
+{
+  return disco->Expires;
+}
+
+gcc_pure
+static inline const char *
+UpnpDiscovery_get_DeviceID_cstr(const UpnpDiscovery *disco) noexcept
+{
+  return disco->DeviceId;
+}
+
+gcc_pure
+static inline const char *
+UpnpDiscovery_get_DeviceType_cstr(const UpnpDiscovery *disco) noexcept
+{
+  return disco->DeviceType;
+}
+
+gcc_pure
+static inline const char *
+UpnpDiscovery_get_ServiceType_cstr(const UpnpDiscovery *disco) noexcept
+{
+  return disco->ServiceType;
+}
+
+gcc_pure
+static inline const char *
+UpnpDiscovery_get_Location_cstr(const UpnpDiscovery *disco) noexcept
+{
+  return disco->Location;
+}
+
+#endif
+
+#endif

src/lib/upnp/Discovery.cxx

@@ -153,10 +153,10 @@ UPnPDeviceDirectory::Explore(void *ctx)
 }
 
 inline int
-UPnPDeviceDirectory::OnAlive(Upnp_Discovery *disco)
+UPnPDeviceDirectory::OnAlive(const UpnpDiscovery *disco)
 {
-	if (isMSDevice(disco->DeviceType) ||
-	    isCDService(disco->ServiceType)) {
+	if (isMSDevice(UpnpDiscovery_get_DeviceType_cstr(disco)) ||
+	    isCDService(UpnpDiscovery_get_ServiceType_cstr(disco))) {
 		DiscoveredTask *tp = new DiscoveredTask(disco);
 		if (queue.put(tp))
 			return UPNP_E_FINISH;
@@ -166,12 +166,12 @@ UPnPDeviceDirectory::OnAlive(Upnp_Discovery *disco)
 }
 
 inline int
-UPnPDeviceDirectory::OnByeBye(Upnp_Discovery *disco)
+UPnPDeviceDirectory::OnByeBye(const UpnpDiscovery *disco)
 {
-	if (isMSDevice(disco->DeviceType) ||
-	    isCDService(disco->ServiceType)) {
+	if (isMSDevice(UpnpDiscovery_get_DeviceType_cstr(disco)) ||
+	    isCDService(UpnpDiscovery_get_ServiceType_cstr(disco))) {
 		// Device signals it is going off.
-		LockRemove(disco->DeviceId);
+		LockRemove(UpnpDiscovery_get_DeviceID_cstr(disco));
 	}
 
 	return UPNP_E_SUCCESS;
@@ -182,19 +182,19 @@ UPnPDeviceDirectory::OnByeBye(Upnp_Discovery *disco)
 // Example: ContentDirectories appearing and disappearing from the network
 // We queue a task for our worker thread(s)
 int
-UPnPDeviceDirectory::Invoke(Upnp_EventType et, void *evp)
+UPnPDeviceDirectory::Invoke(Upnp_EventType et, const void *evp)
 {
 	switch (et) {
 	case UPNP_DISCOVERY_SEARCH_RESULT:
 	case UPNP_DISCOVERY_ADVERTISEMENT_ALIVE:
 		{
-			Upnp_Discovery *disco = (Upnp_Discovery *)evp;
+			auto *disco = (const UpnpDiscovery *)evp;
 			return OnAlive(disco);
 		}
 
 	case UPNP_DISCOVERY_ADVERTISEMENT_BYEBYE:
 		{
-			Upnp_Discovery *disco = (Upnp_Discovery *)evp;
+			auto *disco = (const UpnpDiscovery *)evp;
 			return OnByeBye(disco);
 		}
 

src/lib/upnp/Discovery.hxx

@@ -20,6 +20,7 @@
 #ifndef _UPNPPDISC_H_X_INCLUDED_
 #define _UPNPPDISC_H_X_INCLUDED_
 
+#include "Compat.hxx"
 #include "Callback.hxx"
 #include "Device.hxx"
 #include "WorkQueue.hxx"
@@ -34,6 +35,10 @@
 #include <memory>
 #include <chrono>
 
+#if UPNP_VERSION < 10800
+#define UpnpDiscovery Upnp_Discovery
+#endif
+
 class ContentDirectoryService;
 
 class UPnPDiscoveryListener {
@@ -59,10 +64,10 @@ class UPnPDeviceDirectory final : UpnpCallback {
 		std::string device_id;
 		std::chrono::steady_clock::duration expires;
 
-		DiscoveredTask(const Upnp_Discovery *disco)
-			:url(disco->Location),
-			 device_id(disco->DeviceId),
-			 expires(std::chrono::seconds(disco->Expires)) {}
+		DiscoveredTask(const UpnpDiscovery *disco)
+			:url(UpnpDiscovery_get_Location_cstr(disco)),
+			 device_id(UpnpDiscovery_get_DeviceID_cstr(disco)),
+			 expires(std::chrono::seconds(UpnpDiscovery_get_Expires(disco))) {}
 	};
 
 	/**
@@ -153,11 +158,11 @@ private:
 	static void *Explore(void *);
 	void Explore();
 
-	int OnAlive(Upnp_Discovery *disco);
-	int OnByeBye(Upnp_Discovery *disco);
+	int OnAlive(const UpnpDiscovery *disco);
+	int OnByeBye(const UpnpDiscovery *disco);
 
 	/* virtual methods from class UpnpCallback */
-	virtual int Invoke(Upnp_EventType et, void *evp) override;
+	virtual int Invoke(Upnp_EventType et, const void *evp) override;
 };
 
 

src/output/OutputThread.cxx

@@ -271,16 +271,15 @@ try {
 inline bool
 AudioOutput::PlayChunk()
 {
-	if (tags) {
-		const auto *tag = source.ReadTag();
-		if (tag != nullptr) {
-			const ScopeUnlock unlock(mutex);
-			try {
-				ao_plugin_send_tag(this, *tag);
-			} catch (const std::runtime_error &e) {
-				FormatError(e, "Failed to send tag to \"%s\" [%s]",
-					    name, plugin.name);
-			}
+	// ensure pending tags are flushed in all cases
+	const auto *tag = source.ReadTag();
+	if (tags && tag != nullptr) {
+		const ScopeUnlock unlock(mutex);
+		try {
+			ao_plugin_send_tag(this, *tag);
+		} catch (const std::runtime_error &e) {
+			FormatError(e, "Failed to send tag to \"%s\" [%s]",
+				    name, plugin.name);
 		}
 	}
 

src/output/plugins/httpd/HttpdOutputPlugin.cxx

@@ -468,6 +468,7 @@ HttpdOutput::SendTag(const Tag &tag)
 
 		try {
 			encoder->SendTag(tag);
+			encoder->Flush();
 		} catch (const std::runtime_error &) {
 			/* ignore */
 		}

src/player/Thread.cxx

@@ -32,6 +32,7 @@
 #include "output/MultipleOutputs.hxx"
 #include "tag/Tag.hxx"
 #include "Idle.hxx"
+#include "system/PeriodClock.hxx"
 #include "util/Domain.hxx"
 #include "thread/Name.hxx"
 #include "Log.hxx"
@@ -146,6 +147,8 @@ class Player {
 	 */
 	SongTime elapsed_time;
 
+	PeriodClock throttle_silence_log;
+
 public:
 	Player(PlayerControl &_pc, DecoderControl &_dc,
 	       MusicBuffer &_buffer)
@@ -934,6 +937,8 @@ Player::SongBorder()
 {
 	FormatDefault(player_domain, "played \"%s\"", song->GetURI());
 
+	throttle_silence_log.Reset();
+
 	ReplacePipe(dc.pipe);
 
 	pc.outputs.SongBorder();
@@ -1095,6 +1100,10 @@ Player::Run()
 			/* the decoder is too busy and hasn't provided
 			   new PCM data in time: send silence (if the
 			   output pipe is empty) */
+
+			if (throttle_silence_log.CheckUpdate(std::chrono::seconds(5)))
+				FormatWarning(player_domain, "Decoder is too slow; playing silence to avoid xrun");
+
 			if (!SendSilence())
 				break;
 		}

src/playlist/PlaylistRegistry.cxx

@@ -195,7 +195,7 @@ playlist_list_open_stream_mime2(InputStreamPtr &&is, const char *mime)
 			/* rewind the stream, so each plugin gets a
 			   fresh start */
 			try {
-				is->Rewind();
+				is->LockRewind();
 			} catch (const std::runtime_error &) {
 			}
 
@@ -239,7 +239,7 @@ playlist_list_open_stream_suffix(InputStreamPtr &&is, const char *suffix)
 			/* rewind the stream, so each plugin gets a
 			   fresh start */
 			try {
-				is->Rewind();
+				is->LockRewind();
 			} catch (const std::runtime_error &) {
 			}
 

test/test_protocol.cxx

@@ -37,7 +37,7 @@ ArgParserTest::TestRange()
 	try {
 		range = ParseCommandArgRange("-2");
 		CPPUNIT_ASSERT(false);
-	} catch (ProtocolError) {
+	} catch (const ProtocolError &) {
 		CPPUNIT_ASSERT(true);
 	}
 }