Merge branch 'v0.21.x'

NEWS

@@ -5,9 +5,14 @@ ver 0.22 (not yet released)
 ver 0.21.6 (not yet released)
 * protocol
   - allow loading playlists specified as absolute filesystem paths
+  - fix negated filter expressions with multiple tag values
   - fix "list" with filter expression
 * input
   - cdio_paranoia: fix build failure due to missing #include
+* decoder
+  - opus: fix replay gain when there are no other tags
+* output
+  - pulse: work around error with unusual channel count
 * playlist
   - flac: fix use-after-free bug
 * support abstract sockets on Linux

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')``: match a tag value; if there are multiple
-  ``(TAG != 'VALUE')``: mismatch a tag value.
+  values of the given type, at least one must match.
-  The special tag "*any*" checks all
+  ``(TAG != 'VALUE')``: mismatch a tag value; if there are multiple
-  tag values.
+  values of the given type, none of them must match.
-  *albumartist* looks for
+  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

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) &&
+	if (!ScanOpusTags(packet.packet, packet.bytes, &rgi, h))
-	    !tag_builder.empty()) {
+		return;
-		client.SubmitReplayGain(&rgi);
 
+	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/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_init_extend(&chan_map, ss.channels,
-				 PA_CHANNEL_MAP_WAVEEX);
+				   PA_CHANNEL_MAP_WAVEEX);
 	stream = pa_stream_new(context, name, &ss, &chan_map);
 	if (stream == nullptr)
 		throw MakePulseError(context,

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,42 +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]{};
 
 	for (const auto &i : tag) {
 		visited_types[i.type] = true;
 
-		if (MatchNN(i))
+		if ((type == TAG_NUM_OF_ITEM_TYPES || i.type == type) &&
-			return true;
+		    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,
+		if (ApplyTagFallback(type, [&](TagType tag2) {
-				     [&](TagType tag2) {
+			if (!visited_types[tag2])
-			     if (!visited_types[tag2])
+				/* we already know that this tag type
-				     return false;
+				   isn't present, so let's bail out
+				   without checking again */
+				return false;
 
-			     for (const auto &item : tag) {
+			for (const auto &item : tag) {
-				     if (item.type == tag2 &&
+				if (item.type == tag2 &&
-					 filter.Match(item.value)) {
+				    filter.MatchWithoutNegation(item.value)) {
-					     result = true;
+					result = true;
-					     break;
+					break;
-				     }
+				}
-			     }
+			}
 
-			     return true;
+			return true;
-		     }))
+		}))
-			return result;
+			return result != filter.IsNegated();
 
 		/* If the search critieron was not visited during the
 		   sweep through the song's tag, it means this field
@@ -79,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 Match(const Tag &tag) const noexcept;
-	bool MatchNN(const TagItem &tag) const 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

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()
 {