release v0.21.6

Closes https://github.com/MusicPlayerDaemon/MPD/issues/465 and
https://github.com/MusicPlayerDaemon/MPD/pull/466

NEWS

@@ -1,3 +1,26 @@
+ver 0.21.6 (2019/03/17)
+* protocol
+  - allow loading playlists specified as absolute filesystem paths
+  - fix negated filter expressions with multiple tag values
+  - fix "list" with filter expression
+  - omit empty playlist names in "listplaylists"
+* input
+  - cdio_paranoia: fix build failure due to missing #include
+* decoder
+  - opus: fix replay gain when there are no other tags
+  - opus: fix seeking to beginning of song
+  - vorbis: fix Tremor conflict resulting in crash
+* output
+  - pulse: work around error with unusual channel count
+  - osx: fix build failure
+* playlist
+  - flac: fix use-after-free bug
+* support abstract sockets on Linux
+* Windows
+  - remove the unused libwinpthread-1.dll dependency
+* Android
+  - enable SLES power saving mode
+
 ver 0.21.5 (2019/02/22)
 * protocol
   - fix deadlock in "albumart" command

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="27"
-          android:versionName="0.21.5">
+          android:versionCode="28"
+          android:versionName="0.21.6">
 
   <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.5'
+version = '0.21.6'
 # The full version, including alpha/beta/rc tags.
 release = version
 

doc/protocol.rst

@@ -144,15 +144,20 @@ syntax::
 ``EXPRESSION`` is a string enclosed in parantheses which can be one
 of:
 
-- ``(TAG == 'VALUE')``: match a tag value.
-  ``(TAG != 'VALUE')``: mismatch a tag value.
-  The special tag "*any*" checks all
-  tag values.
-  *albumartist* looks for
+- ``(TAG == 'VALUE')``: match a tag value; if there are multiple
+  values of the given type, at least one must match.
+  ``(TAG != 'VALUE')``: mismatch a tag value; if there are multiple
+  values of the given type, none of them must match.
+  The special tag ``any`` checks all
+  tag types.
+  ``AlbumArtist`` looks for
   ``VALUE`` in ``AlbumArtist``
   and falls back to ``Artist`` tags if
   ``AlbumArtist`` does not exist.
   ``VALUE`` is what to find.
+  An empty value string means: match only if the given tag type does
+  not exist at all; this implies that negation with an empty value
+  checks for the existence of the given tag type.
 
 - ``(TAG contains 'VALUE')`` checks if the given value is a substring
   of the tag value.
@@ -178,7 +183,7 @@ of:
 
 - ``(AudioFormat =~ 'SAMPLERATE:BITS:CHANNELS')``:
   matches the audio format with the given mask (i.e. one
-  or more attributes may be "*").
+  or more attributes may be ``*``).
 
 - ``(!EXPRESSION)``: negate an expression.  Note that each expression
   must be enclosed in parantheses, e.g. :code:`(!(artist == 'VALUE'))`
@@ -207,11 +212,11 @@ backslash.
 
 Example expression which matches an artist named ``foo'bar"``::
 
- (artist "foo\'bar\"")
+ (Artist == "foo\'bar\"")
 
 At the protocol level, the command must look like this::
 
- find "(artist \"foo\\'bar\\\"\")"
+ find "(Artist == \"foo\\'bar\\\"\")"
 
 The double quotes enclosing the artist name must be escaped because
 they are inside a double-quoted ``find`` parameter.  The single quote
@@ -714,7 +719,9 @@ and without the `.m3u` suffix).
 Some of the commands described in this section can be used to
 run playlist plugins instead of the hard-coded simple
 `m3u` parser.  They can access playlists in
-the music directory (relative path including the suffix) or
+the music directory (relative path including the suffix),
+playlists in arbitrary location (absolute path including the suffix;
+allowed only for clients that are connected via UNIX domain socket), or
 remote playlists (absolute URI with a supported scheme).
 
 :command:`listplaylist {NAME}`

doc/user.rst

@@ -531,6 +531,12 @@ choice::
 
  bind_to_address "/var/run/mpd/socket"
 
+On Linux, local sockets can be bound to a name without a socket inode
+on the filesystem; MPD implements this by prepending ``@`` to the
+address::
+
+ bind_to_address "@mpd"
+
 If no port is specified, the default port is 6600.  This default can
 be changed with the port setting::
 

meson.build

@@ -1,7 +1,7 @@
 project(
   'mpd',
   ['c', 'cpp'],
-  version: '0.21.5',
+  version: '0.21.6',
   meson_version: '>= 0.47.2',
   default_options: [
     'c_std=c99',
@@ -20,7 +20,7 @@ conf.set_quoted('PACKAGE', meson.project_name())
 conf.set_quoted('PACKAGE_NAME', meson.project_name())
 conf.set_quoted('PACKAGE_VERSION', meson.project_version())
 conf.set_quoted('VERSION', meson.project_version())
-conf.set_quoted('PROTOCOL_VERSION', '0.21.4')
+conf.set_quoted('PROTOCOL_VERSION', '0.21.6')
 conf.set_quoted('SYSTEM_CONFIG_FILE_LOCATION', join_paths(get_option('prefix'), get_option('sysconfdir'), 'mpd.conf'))
 
 common_cppflags = [

src/PlaylistFile.cxx

@@ -134,7 +134,9 @@ LoadPlaylistFileInfo(PlaylistInfo &info,
 	const auto *const name_fs_end =
 		FindStringSuffix(name_fs_str,
 				 PATH_LITERAL(PLAYLIST_FILE_SUFFIX));
-	if (name_fs_end == nullptr)
+	if (name_fs_end == nullptr ||
+	    /* no empty playlist names (raw file name = ".m3u") */
+	    name_fs_end == name_fs_str)
 		return false;
 
 	FileInfo fi;

src/command/DatabaseCommands.cxx

@@ -268,7 +268,10 @@ handle_list(Client &client, Request args, Response &r)
 	std::unique_ptr<SongFilter> filter;
 	TagType group = TAG_NUM_OF_ITEM_TYPES;
 
-	if (args.size == 1) {
+	if (args.size == 1 &&
+	    /* parantheses are the syntax for filter expressions: no
+	       compatibility mode */
+	    args.front()[0] != '(') {
 		/* for compatibility with < 0.12.0 */
 		if (tagType != TAG_ALBUM) {
 			r.FormatError(ACK_ERROR_ARG,

src/command/PlaylistCommands.cxx

@@ -38,6 +38,7 @@
 #include "util/UriUtil.hxx"
 #include "util/ConstBuffer.hxx"
 #include "util/ChronoUtil.hxx"
+#include "LocateUri.hxx"
 
 bool
 playlist_commands_available() noexcept
@@ -66,12 +67,17 @@ handle_save(Client &client, Request args, gcc_unused Response &r)
 CommandResult
 handle_load(Client &client, Request args, gcc_unused Response &r)
 {
+	const auto uri = LocateUri(args.front(), &client
+#ifdef ENABLE_DATABASE
+					   , nullptr
+#endif
+					   );
 	RangeArg range = args.ParseOptional(1, RangeArg::All());
 
 	const ScopeBulkEdit bulk_edit(client.GetPartition());
 
 	const SongLoader loader(client);
-	playlist_open_into_queue(args.front(),
+	playlist_open_into_queue(uri,
 				 range.start, range.end,
 				 client.GetPlaylist(),
 				 client.GetPlayerControl(), loader);
@@ -81,7 +87,11 @@ handle_load(Client &client, Request args, gcc_unused Response &r)
 CommandResult
 handle_listplaylist(Client &client, Request args, Response &r)
 {
-	const char *const name = args.front();
+	const auto name = LocateUri(args.front(), &client
+#ifdef ENABLE_DATABASE
+					   , nullptr
+#endif
+					   );
 
 	if (playlist_file_print(r, client.GetPartition(), SongLoader(client),
 				name, false))
@@ -93,7 +103,11 @@ handle_listplaylist(Client &client, Request args, Response &r)
 CommandResult
 handle_listplaylistinfo(Client &client, Request args, Response &r)
 {
-	const char *const name = args.front();
+	const auto name = LocateUri(args.front(), &client
+#ifdef ENABLE_DATABASE
+					   , nullptr
+#endif
+					   );
 
 	if (playlist_file_print(r, client.GetPartition(), SongLoader(client),
 				name, true))

src/config/Net.cxx

@@ -29,6 +29,10 @@ ServerSocketAddGeneric(ServerSocket &server_socket, const char *address, unsigne
 		server_socket.AddPort(port);
 	} else if (address[0] == '/' || address[0] == '~') {
 		server_socket.AddPath(ParsePath(address));
+#ifdef __linux__
+	} else if (address[0] == '@') {
+		server_socket.AddAbstract(address);
+#endif
 	} else {
 		server_socket.AddHost(address, port);
 	}

src/decoder/plugins/OpusDecoderPlugin.cxx

@@ -208,10 +208,12 @@ MPDOpusDecoder::HandleTags(const ogg_packet &packet)
 	TagBuilder tag_builder;
 	AddTagHandler h(tag_builder);
 
-	if (ScanOpusTags(packet.packet, packet.bytes, &rgi, h) &&
-	    !tag_builder.empty()) {
-		client.SubmitReplayGain(&rgi);
+	if (!ScanOpusTags(packet.packet, packet.bytes, &rgi, h))
+		return;
 
+	client.SubmitReplayGain(&rgi);
+
+	if (!tag_builder.empty()) {
 		Tag tag = tag_builder.Commit();
 		auto cmd = client.SubmitTag(input_stream, std::move(tag));
 		if (cmd != DecoderCommand::NONE)

src/event/ServerSocket.cxx

@@ -396,3 +396,19 @@ ServerSocket::AddPath(AllocatedPath &&path)
 #endif /* !HAVE_UN */
 }
 
+
+#ifdef __linux__
+
+void
+ServerSocket::AddAbstract(const char *name)
+{
+	assert(name != nullptr);
+	assert(*name == '@');
+
+	AllocatedSocketAddress address;
+	address.SetLocal(name);
+
+	AddAddress(std::move(address));
+}
+
+#endif

src/event/ServerSocket.hxx

@@ -99,6 +99,18 @@ public:
 	 */
 	void AddPath(AllocatedPath &&path);
 
+#ifdef __linux__
+	/**
+	 * Add a listener on an abstract local socket (Linux specific).
+	 *
+	 * Throws on error.
+	 *
+	 * @param name the abstract socket name, starting with a '@'
+	 * instead of a null byte
+	 */
+	void AddAbstract(const char *name);
+#endif
+
 	/**
 	 * Add a socket descriptor that is accepting connections.  After this
 	 * has been called, don't call server_socket_open(), because the

src/lib/cdio/Paranoia.hxx

@@ -43,6 +43,8 @@
 #include <stdexcept>
 #include <utility>
 
+#include <cstdio>
+
 class CdromDrive {
 	cdrom_drive_t *drv = nullptr;
 

src/lib/xiph/OggVisitor.cxx

@@ -20,6 +20,7 @@
 #include "OggVisitor.hxx"
 
 #include <stdexcept>
+#include <utility>
 
 void
 OggVisitor::EndStream()
@@ -51,7 +52,13 @@ OggVisitor::ReadNextPage()
 inline void
 OggVisitor::HandlePacket(const ogg_packet &packet)
 {
+	const bool _post_seek = std::exchange(post_seek, false);
+
 	if (packet.b_o_s) {
+		if (_post_seek)
+			/* ignore the BOS packet after seeking */
+			return;
+
 		EndStream();
 		has_stream = true;
 		OnOggBeginning(packet);
@@ -97,4 +104,6 @@ OggVisitor::PostSeek()
 
 	/* find the next Ogg page and feed it into the stream */
 	sync.ExpectPageSeekIn(stream);
+
+	post_seek = true;
 }

src/lib/xiph/OggVisitor.hxx

@@ -39,6 +39,14 @@ class OggVisitor {
 
 	bool has_stream = false;
 
+	/**
+	 * This is true after seeking; its one-time effect is to
+	 * ignore the BOS packet, just in case we have been seeking to
+	 * the beginning of the file, because that would disrupt
+	 * playback.
+	 */
+	bool post_seek = false;
+
 public:
 	explicit OggVisitor(Reader &reader)
 		:sync(reader), stream(0) {}

src/lib/xiph/meson.build

@@ -1,7 +1,20 @@
 libflac_dep = dependency('flac', version: '>= 1.2', required: get_option('flac'))
 libopus_dep = dependency('opus', required: get_option('opus'))
-libvorbis_dep = dependency('vorbis', required: get_option('vorbis'))
-libvorbisidec_dep = dependency('vorbisidec', required: get_option('tremor'))
+
+if get_option('tremor').enabled()
+  # no libvorbis if Tremor was explicitly enabled
+  libvorbis_dep = dependency('', required: false)
+else
+  libvorbis_dep = dependency('vorbis', required: get_option('vorbis'))
+endif
+
+if libvorbis_dep.found()
+  # no Tremor if libvorbis is used
+  libvorbisidec_dep = dependency('', required: false)
+else
+  # attempt to auto-detect Tremor only if libvorbis was disabled or not found
+  libvorbisidec_dep = dependency('vorbisidec', required: get_option('tremor'))
+endif
 
 if get_option('vorbis').enabled() and get_option('tremor').enabled()
 	error('Cannot build both, the Vorbis decoder AND the Tremor (Vorbis fixed-point) decoder')

src/net/SocketDescriptor.cxx

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2012-2017 Max Kellermann <max.kellermann@gmail.com>
+ * Copyright 2012-2019 Max Kellermann <max.kellermann@gmail.com>
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -315,6 +315,13 @@ SocketDescriptor::SetTcpDeferAccept(const int &seconds) noexcept
 	return SetOption(IPPROTO_TCP, TCP_DEFER_ACCEPT, &seconds, sizeof(seconds));
 }
 
+bool
+SocketDescriptor::SetTcpUserTimeout(const unsigned &milliseconds) noexcept
+{
+	return SetOption(IPPROTO_TCP, TCP_USER_TIMEOUT,
+			 &milliseconds, sizeof(milliseconds));
+}
+
 bool
 SocketDescriptor::SetV6Only(bool value) noexcept
 {

src/net/SocketDescriptor.hxx

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2012-2017 Max Kellermann <max.kellermann@gmail.com>
+ * Copyright 2012-2019 Max Kellermann <max.kellermann@gmail.com>
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -188,6 +188,12 @@ public:
 	bool SetCork(bool value=true) noexcept;
 
 	bool SetTcpDeferAccept(const int &seconds) noexcept;
+
+	/**
+	 * Setter for TCP_USER_TIMEOUT.
+	 */
+	bool SetTcpUserTimeout(const unsigned &milliseconds) noexcept;
+
 	bool SetV6Only(bool value) noexcept;
 
 	/**

src/net/StaticSocketAddress.cxx

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2012-2017 Max Kellermann <max.kellermann@gmail.com>
+ * Copyright 2012-2019 Max Kellermann <max.kellermann@gmail.com>
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -29,6 +29,7 @@
 
 #include "config.h"
 #include "StaticSocketAddress.hxx"
+#include "util/StringView.hxx"
 
 #include <algorithm>
 
@@ -50,6 +51,16 @@ StaticSocketAddress::operator=(SocketAddress other) noexcept
 	return *this;
 }
 
+#ifdef HAVE_UN
+
+StringView
+StaticSocketAddress::GetLocalRaw() const noexcept
+{
+	return SocketAddress(*this).GetLocalRaw();
+}
+
+#endif
+
 #ifdef HAVE_TCP
 
 bool

src/net/StaticSocketAddress.hxx

@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2012-2017 Max Kellermann <max.kellermann@gmail.com>
+ * Copyright 2012-2019 Max Kellermann <max.kellermann@gmail.com>
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -66,14 +66,6 @@ public:
 		return reinterpret_cast<const struct sockaddr *>(&address);
 	}
 
-	struct sockaddr *GetAddress() noexcept {
-		return reinterpret_cast<struct sockaddr *>(&address);
-	}
-
-	const struct sockaddr *GetAddress() const noexcept {
-		return reinterpret_cast<const struct sockaddr *>(&address);
-	}
-
 	constexpr size_type GetCapacity() const noexcept {
 		return sizeof(address);
 	}
@@ -109,6 +101,14 @@ public:
 		address.ss_family = AF_UNSPEC;
 	}
 
+#ifdef HAVE_UN
+	/**
+	 * @see SocketAddress::GetLocalRaw()
+	 */
+	gcc_pure
+	StringView GetLocalRaw() const noexcept;
+#endif
+
 #ifdef HAVE_TCP
 	/**
 	 * Extract the port number.  Returns 0 if not applicable.

src/output/plugins/PulseOutputPlugin.cxx

@@ -581,8 +581,8 @@ PulseOutput::SetupStream(const pa_sample_spec &ss)
 
 	/* WAVE-EX is been adopted as the speaker map for most media files */
 	pa_channel_map chan_map;
-	pa_channel_map_init_auto(&chan_map, ss.channels,
-				 PA_CHANNEL_MAP_WAVEEX);
+	pa_channel_map_init_extend(&chan_map, ss.channels,
+				   PA_CHANNEL_MAP_WAVEEX);
 	stream = pa_stream_new(context, name, &ss, &chan_map);
 	if (stream == nullptr)
 		throw MakePulseError(context,

src/output/plugins/meson.build

@@ -76,7 +76,10 @@ if is_darwin
   audiounit_dep = declare_dependency(
     link_args: [
       '-framework', 'AudioUnit', '-framework', 'CoreAudio', '-framework', 'CoreServices',
-    ]
+    ],
+    dependencies: [
+      boost_dep,
+    ],
   )
 else
   audiounit_dep = dependency('', required: false)

src/output/plugins/sles/SlesOutputPlugin.cxx

@@ -229,6 +229,14 @@ SlesOutput::Open(AudioFormat &audio_format)
 						    SL_ANDROID_KEY_STREAM_TYPE,
 						    &stream_type,
 						    sizeof(stream_type));
+
+		/* MPD doesn't care much about latency, so let's
+		   configure power saving mode */
+		SLuint32 performance_mode = SL_ANDROID_PERFORMANCE_POWER_SAVING;
+		(*android_config)->SetConfiguration(android_config,
+						    SL_ANDROID_KEY_PERFORMANCE_MODE,
+						    &performance_mode,
+						    sizeof(performance_mode));
 	}
 
 	result = play_object.Realize(false);

src/playlist/PlaylistAny.cxx

@@ -17,6 +17,7 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  */
 
+#include "LocateUri.hxx"
 #include "PlaylistAny.hxx"
 #include "PlaylistStream.hxx"
 #include "PlaylistMapper.hxx"
@@ -25,17 +26,26 @@
 #include "config.h"
 
 std::unique_ptr<SongEnumerator>
-playlist_open_any(const char *uri,
+playlist_open_any(const LocatedUri &located_uri,
 #ifdef ENABLE_DATABASE
 		  const Storage *storage,
 #endif
 		  Mutex &mutex)
 {
-	return uri_has_scheme(uri)
-		? playlist_open_remote(uri, mutex)
-		: playlist_mapper_open(uri,
+	switch (located_uri.type) {
+	case LocatedUri::Type::ABSOLUTE:
+		return playlist_open_remote(located_uri.canonical_uri, mutex);
+
+	case LocatedUri::Type::PATH:
+		return playlist_open_path(located_uri.path, mutex);
+
+	case LocatedUri::Type::RELATIVE:
+		return playlist_mapper_open(located_uri.canonical_uri,
 #ifdef ENABLE_DATABASE
 				       storage,
 #endif
 				       mutex);
+	}
+
+	gcc_unreachable();
 }

src/playlist/PlaylistAny.hxx

@@ -34,7 +34,7 @@ class Storage;
  * music or playlist directory.
  */
 std::unique_ptr<SongEnumerator>
-playlist_open_any(const char *uri,
+playlist_open_any(const LocatedUri &located_uri,
 #ifdef ENABLE_DATABASE
 		  const Storage *storage,
 #endif

src/playlist/PlaylistQueue.cxx

@@ -18,6 +18,7 @@
  */
 
 #include "config.h"
+#include "LocateUri.hxx"
 #include "PlaylistQueue.hxx"
 #include "PlaylistAny.hxx"
 #include "PlaylistSong.hxx"
@@ -63,7 +64,7 @@ playlist_load_into_queue(const char *uri, SongEnumerator &e,
 }
 
 void
-playlist_open_into_queue(const char *uri,
+playlist_open_into_queue(const LocatedUri &uri,
 			 unsigned start_index, unsigned end_index,
 			 playlist &dest, PlayerControl &pc,
 			 const SongLoader &loader)
@@ -78,7 +79,7 @@ playlist_open_into_queue(const char *uri,
 	if (playlist == nullptr)
 		throw PlaylistError::NoSuchList();
 
-	playlist_load_into_queue(uri, *playlist,
+	playlist_load_into_queue(uri.canonical_uri, *playlist,
 				 start_index, end_index,
 				 dest, pc, loader);
 }

src/playlist/PlaylistQueue.hxx

@@ -49,7 +49,7 @@ playlist_load_into_queue(const char *uri, SongEnumerator &e,
  * play queue.
  */
 void
-playlist_open_into_queue(const char *uri,
+playlist_open_into_queue(const LocatedUri &uri,
 			 unsigned start_index, unsigned end_index,
 			 playlist &dest, PlayerControl &pc,
 			 const SongLoader &loader);

src/playlist/Print.cxx

@@ -18,6 +18,7 @@
  */
 
 #include "config.h"
+#include "LocateUri.hxx"
 #include "Print.hxx"
 #include "PlaylistAny.hxx"
 #include "PlaylistSong.hxx"
@@ -55,7 +56,7 @@ playlist_provider_print(Response &r,
 bool
 playlist_file_print(Response &r, Partition &partition,
 		    const SongLoader &loader,
-		    const char *uri, bool detail)
+		    const LocatedUri &uri, bool detail)
 {
 	Mutex mutex;
 
@@ -71,6 +72,6 @@ playlist_file_print(Response &r, Partition &partition,
 	if (playlist == nullptr)
 		return false;
 
-	playlist_provider_print(r, loader, uri, *playlist, detail);
+	playlist_provider_print(r, loader, uri.canonical_uri, *playlist, detail);
 	return true;
 }

src/playlist/Print.hxx

@@ -34,6 +34,6 @@ struct Partition;
 bool
 playlist_file_print(Response &r, Partition &partition,
 		    const SongLoader &loader,
-		    const char *uri, bool detail);
+		    const LocatedUri &uri, bool detail);
 
 #endif

src/playlist/plugins/FlacPlaylistPlugin.cxx

@@ -34,7 +34,7 @@
 #include <FLAC/metadata.h>
 
 class FlacPlaylist final : public SongEnumerator {
-	const char *const uri;
+	const std::string uri;
 
 	FLAC__StreamMetadata *const cuesheet;
 	const unsigned sample_rate;

src/song/StringFilter.cxx

@@ -22,13 +22,10 @@
 
 #include <assert.h>
 
-inline bool
+bool
 StringFilter::MatchWithoutNegation(const char *s) const noexcept
 {
-#if !CLANG_CHECK_VERSION(3,6)
-	/* disabled on clang due to -Wtautological-pointer-compare */
 	assert(s != nullptr);
-#endif
 
 #ifdef HAVE_PCRE
 	if (regex)

src/song/StringFilter.hxx

@@ -105,7 +105,9 @@ public:
 	gcc_pure
 	bool Match(const char *s) const noexcept;
 
-private:
+	/**
+	 * Like Match(), but ignore the "negated" flag.
+	 */
 	gcc_pure
 	bool MatchWithoutNegation(const char *s) const noexcept;
 };

src/song/TagSongFilter.cxx

@@ -35,43 +35,41 @@ TagSongFilter::ToExpression() const noexcept
 }
 
 bool
-TagSongFilter::MatchNN(const TagItem &item) const noexcept
+TagSongFilter::Match(const Tag &tag) const noexcept
 {
-	return (type == TAG_NUM_OF_ITEM_TYPES || item.type == type) &&
-		filter.Match(item.value);
-}
-
-bool
-TagSongFilter::MatchNN(const Tag &tag) const noexcept
-{
-	bool visited_types[TAG_NUM_OF_ITEM_TYPES];
-	std::fill_n(visited_types, size_t(TAG_NUM_OF_ITEM_TYPES), false);
+	bool visited_types[TAG_NUM_OF_ITEM_TYPES]{};
 
 	for (const auto &i : tag) {
 		visited_types[i.type] = true;
 
-		if (MatchNN(i))
-			return true;
+		if ((type == TAG_NUM_OF_ITEM_TYPES || i.type == type) &&
+		    filter.MatchWithoutNegation(i.value))
+			return !filter.IsNegated();
 	}
 
 	if (type < TAG_NUM_OF_ITEM_TYPES && !visited_types[type]) {
+		/* if the specified tag is not present, try the
+		   fallback tags */
+
 		bool result = false;
-		if (ApplyTagFallback(type,
-				     [&](TagType tag2) {
-			     if (!visited_types[tag2])
-				     return false;
+		if (ApplyTagFallback(type, [&](TagType tag2) {
+			if (!visited_types[tag2])
+				/* we already know that this tag type
+				   isn't present, so let's bail out
+				   without checking again */
+				return false;
 
-			     for (const auto &item : tag) {
-				     if (item.type == tag2 &&
-					 filter.Match(item.value)) {
-					     result = true;
-					     break;
-				     }
-			     }
+			for (const auto &item : tag) {
+				if (item.type == tag2 &&
+				    filter.MatchWithoutNegation(item.value)) {
+					result = true;
+					break;
+				}
+			}
 
-			     return true;
-		     }))
-			return result;
+			return true;
+		}))
+			return result != filter.IsNegated();
 
 		/* If the search critieron was not visited during the
 		   sweep through the song's tag, it means this field
@@ -80,14 +78,14 @@ TagSongFilter::MatchNN(const Tag &tag) const noexcept
 		   then it's a match as well and we should return
 		   true. */
 		if (filter.empty())
-			return true;
+			return !filter.IsNegated();
 	}
 
-	return false;
+	return filter.IsNegated();
 }
 
 bool
 TagSongFilter::Match(const LightSong &song) const noexcept
 {
-	return MatchNN(song.tag);
+	return Match(song.tag);
 }

src/song/TagSongFilter.hxx

@@ -68,8 +68,7 @@ public:
 	bool Match(const LightSong &song) const noexcept override;
 
 private:
-	bool MatchNN(const Tag &tag) const noexcept;
-	bool MatchNN(const TagItem &tag) const noexcept;
+	bool Match(const Tag &tag) const noexcept;
 };
 
 #endif

src/system/UniqueFileDescriptor.hxx

@@ -79,6 +79,11 @@ public:
 		return FileDescriptor::CreatePipe(r, w);
 	}
 
+	static bool CreatePipeNonBlock(UniqueFileDescriptor &r,
+				       UniqueFileDescriptor &w) noexcept {
+		return FileDescriptor::CreatePipeNonBlock(r, w);
+	}
+
 	static bool CreatePipe(FileDescriptor &r, FileDescriptor &w) noexcept;
 #endif
 

src/tag/Fallback.hxx

@@ -22,6 +22,11 @@
 
 #include <utility>
 
+/**
+ * Invoke the given function for all fallback tags of the given
+ * #TagType, until the function returns true (or until there are no
+ * more fallback tags).
+ */
 template<typename F>
 bool
 ApplyTagFallback(TagType type, F &&f) noexcept
@@ -43,6 +48,11 @@ ApplyTagFallback(TagType type, F &&f) noexcept
 	return false;
 }
 
+/**
+ * Invoke the given function for the given #TagType and all of its
+ * fallback tags, until the function returns true (or until there are
+ * no more fallback tags).
+ */
 template<typename F>
 bool
 ApplyTagWithFallback(TagType type, F &&f) noexcept

src/thread/meson.build

@@ -1,4 +1,11 @@
-threads_dep = dependency('threads')
+if is_windows
+  # avoid the unused libwinpthread-1.dll dependency on Windows; MPD
+  # doesn't use the pthread API on Windows, but this is what Meson
+  # unhelpfully detects for us
+  threads_dep = []
+else
+  threads_dep = dependency('threads')
+endif
 
 conf.set('HAVE_PTHREAD_SETNAME_NP', compiler.has_function('pthread_setname_np', dependencies: threads_dep))
 

test/MakeTag.hxx

@@ -0,0 +1,45 @@
+/*
+ * Copyright 2003-2019 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.
+ */
+
+#include "tag/Builder.hxx"
+#include "tag/Tag.hxx"
+#include "util/Compiler.h"
+
+inline void
+BuildTag(gcc_unused TagBuilder &tag) noexcept
+{
+}
+
+template<typename... Args>
+inline void
+BuildTag(TagBuilder &tag, TagType type, const char *value,
+	 Args&&... args) noexcept
+{
+	tag.AddItem(type, value);
+	BuildTag(tag, std::forward<Args>(args)...);
+}
+
+template<typename... Args>
+inline Tag
+MakeTag(Args&&... args) noexcept
+{
+	TagBuilder tag;
+	BuildTag(tag, std::forward<Args>(args)...);
+	return tag.Commit();
+}

test/TestTagSongFilter.cxx

@@ -0,0 +1,163 @@
+/*
+ * Copyright 2003-2019 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.
+ */
+
+#include "MakeTag.hxx"
+#include "song/TagSongFilter.hxx"
+#include "song/LightSong.hxx"
+#include "tag/Type.h"
+
+#include <gtest/gtest.h>
+
+static bool
+InvokeFilter(const TagSongFilter &f, const Tag &tag) noexcept
+{
+	return f.Match(LightSong("dummy", tag));
+}
+
+TEST(TagSongFilter, Basic)
+{
+	const TagSongFilter f(TAG_TITLE,
+			      StringFilter("needle", false, false, false));
+
+	EXPECT_TRUE(InvokeFilter(f, MakeTag(TAG_TITLE, "needle")));
+	EXPECT_TRUE(InvokeFilter(f, MakeTag(TAG_TITLE, "foo", TAG_TITLE, "needle")));
+	EXPECT_TRUE(InvokeFilter(f, MakeTag(TAG_TITLE, "needle", TAG_TITLE, "foo")));
+	EXPECT_TRUE(InvokeFilter(f, MakeTag(TAG_ARTIST, "foo", TAG_TITLE, "needle", TAG_ALBUM, "bar")));
+
+	EXPECT_FALSE(InvokeFilter(f, MakeTag()));
+	EXPECT_FALSE(InvokeFilter(f, MakeTag(TAG_TITLE, "foo")));
+	EXPECT_FALSE(InvokeFilter(f, MakeTag(TAG_TITLE, "foo", TAG_TITLE, "bar")));
+	EXPECT_FALSE(InvokeFilter(f, MakeTag(TAG_TITLE, "foo", TAG_ARTIST, "needle")));
+	EXPECT_FALSE(InvokeFilter(f, MakeTag(TAG_TITLE, "FOOneedleBAR")));
+}
+
+/**
+ * Test with empty string.  This matches tags where the given tag type
+ * does not exist.
+ */
+TEST(TagSongFilter, Empty)
+{
+	const TagSongFilter f(TAG_TITLE,
+			      StringFilter("", false, false, false));
+
+	EXPECT_TRUE(InvokeFilter(f, MakeTag()));
+
+	EXPECT_FALSE(InvokeFilter(f, MakeTag(TAG_TITLE, "foo")));
+	EXPECT_FALSE(InvokeFilter(f, MakeTag(TAG_TITLE, "foo", TAG_TITLE, "bar")));
+}
+
+TEST(TagSongFilter, Substring)
+{
+	const TagSongFilter f(TAG_TITLE,
+			      StringFilter("needle", false, true, false));
+
+	EXPECT_TRUE(InvokeFilter(f, MakeTag(TAG_TITLE, "needle")));
+	EXPECT_TRUE(InvokeFilter(f, MakeTag(TAG_TITLE, "needleBAR")));
+	EXPECT_TRUE(InvokeFilter(f, MakeTag(TAG_TITLE, "FOOneedle")));
+	EXPECT_TRUE(InvokeFilter(f, MakeTag(TAG_TITLE, "FOOneedleBAR")));
+
+	EXPECT_FALSE(InvokeFilter(f, MakeTag()));
+	EXPECT_FALSE(InvokeFilter(f, MakeTag(TAG_TITLE, "eedle")));
+}
+
+TEST(TagSongFilter, Negated)
+{
+	const TagSongFilter f(TAG_TITLE,
+			      StringFilter("needle", false, false, true));
+
+	EXPECT_TRUE(InvokeFilter(f, MakeTag()));
+	EXPECT_FALSE(InvokeFilter(f, MakeTag(TAG_TITLE, "needle")));
+	EXPECT_TRUE(InvokeFilter(f, MakeTag(TAG_TITLE, "foo")));
+}
+
+/**
+ * Combine the "Empty" and "Negated" tests.
+ */
+TEST(TagSongFilter, EmptyNegated)
+{
+	const TagSongFilter f(TAG_TITLE,
+			      StringFilter("", false, false, true));
+
+	EXPECT_FALSE(InvokeFilter(f, MakeTag()));
+	EXPECT_TRUE(InvokeFilter(f, MakeTag(TAG_TITLE, "foo")));
+}
+
+/**
+ * Negation with multiple tag values.
+ */
+TEST(TagSongFilter, MultiNegated)
+{
+	const TagSongFilter f(TAG_TITLE,
+			      StringFilter("needle", false, false, true));
+
+	EXPECT_TRUE(InvokeFilter(f, MakeTag(TAG_TITLE, "foo", TAG_TITLE, "bar")));
+	EXPECT_FALSE(InvokeFilter(f, MakeTag(TAG_TITLE, "needle", TAG_TITLE, "bar")));
+	EXPECT_FALSE(InvokeFilter(f, MakeTag(TAG_TITLE, "foo", TAG_TITLE, "needle")));
+}
+
+/**
+ * Check whether fallback tags work, e.g. AlbumArtist falls back to
+ * just Artist if there is no AlbumArtist.
+ */
+TEST(TagSongFilter, Fallback)
+{
+	const TagSongFilter f(TAG_ALBUM_ARTIST,
+			      StringFilter("needle", false, false, false));
+
+	EXPECT_TRUE(InvokeFilter(f, MakeTag(TAG_ALBUM_ARTIST, "needle")));
+	EXPECT_TRUE(InvokeFilter(f, MakeTag(TAG_ARTIST, "needle")));
+
+	EXPECT_FALSE(InvokeFilter(f, MakeTag()));
+	EXPECT_FALSE(InvokeFilter(f, MakeTag(TAG_ALBUM_ARTIST, "foo")));
+	EXPECT_FALSE(InvokeFilter(f, MakeTag(TAG_ARTIST, "foo")));
+
+	/* no fallback, thus the Artist tag isn't used and this must
+	   be a mismatch */
+	EXPECT_FALSE(InvokeFilter(f, MakeTag(TAG_ARTIST, "needle", TAG_ALBUM_ARTIST, "foo")));
+}
+
+/**
+ * Combine the "Empty" and "Fallback" tests.
+ */
+TEST(TagSongFilter, EmptyFallback)
+{
+	const TagSongFilter f(TAG_ALBUM_ARTIST,
+			      StringFilter("", false, false, false));
+
+	EXPECT_TRUE(InvokeFilter(f, MakeTag()));
+
+	EXPECT_FALSE(InvokeFilter(f, MakeTag(TAG_ALBUM_ARTIST, "foo")));
+	EXPECT_FALSE(InvokeFilter(f, MakeTag(TAG_ARTIST, "foo")));
+}
+
+/**
+ * Combine the "Negated" and "Fallback" tests.
+ */
+TEST(TagSongFilter, NegatedFallback)
+{
+	const TagSongFilter f(TAG_ALBUM_ARTIST,
+			      StringFilter("needle", false, false, true));
+
+	EXPECT_TRUE(InvokeFilter(f, MakeTag()));
+	EXPECT_TRUE(InvokeFilter(f, MakeTag(TAG_ALBUM_ARTIST, "foo")));
+	EXPECT_FALSE(InvokeFilter(f, MakeTag(TAG_ALBUM_ARTIST, "needle")));
+	EXPECT_TRUE(InvokeFilter(f, MakeTag(TAG_ARTIST, "foo")));
+	EXPECT_FALSE(InvokeFilter(f, MakeTag(TAG_ARTIST, "needle")));
+	EXPECT_TRUE(InvokeFilter(f, MakeTag(TAG_ARTIST, "needle", TAG_ALBUM_ARTIST, "foo")));
+}

test/meson.build

@@ -20,16 +20,6 @@ gtest_dep = declare_dependency(
 
 subdir('net')
 
-executable(
-  'ParseSongFilter',
-  'ParseSongFilter.cxx',
-  include_directories: inc,
-  dependencies: [
-    song_dep,
-    pcm_dep,
-  ],
-)
-
 executable(
   'read_conf',
   'read_conf.cxx',
@@ -211,6 +201,33 @@ if zlib_dep.found()
   )
 endif
 
+#
+# Filter
+#
+
+executable(
+  'ParseSongFilter',
+  'ParseSongFilter.cxx',
+  include_directories: inc,
+  dependencies: [
+    song_dep,
+    pcm_dep,
+  ],
+)
+
+test(
+  'TestSongFilter',
+  executable(
+    'TestSongFilter',
+    'TestTagSongFilter.cxx',
+    include_directories: inc,
+    dependencies: [
+      song_dep,
+      gtest_dep,
+    ],
+  )
+)
+
 #
 # Neighbor
 #

test/test_translate_song.cxx

@@ -2,6 +2,7 @@
  * Unit tests for playlist_check_translate_song().
  */
 
+#include "MakeTag.hxx"
 #include "playlist/PlaylistSong.hxx"
 #include "song/DetachedSong.hxx"
 #include "SongLoader.hxx"
@@ -38,28 +39,6 @@ uri_supported_scheme(const char *uri) noexcept
 static constexpr auto music_directory = PATH_LITERAL("/music");
 static Storage *storage;
 
-static void
-BuildTag(gcc_unused TagBuilder &tag)
-{
-}
-
-template<typename... Args>
-static void
-BuildTag(TagBuilder &tag, TagType type, const char *value, Args&&... args)
-{
-	tag.AddItem(type, value);
-	BuildTag(tag, std::forward<Args>(args)...);
-}
-
-template<typename... Args>
-static Tag
-MakeTag(Args&&... args)
-{
-	TagBuilder tag;
-	BuildTag(tag, std::forward<Args>(args)...);
-	return tag.Commit();
-}
-
 static Tag
 MakeTag1a()
 {