DatabaseCommands: "list" allows grouping

NEWS

@@ -6,6 +6,7 @@ ver 0.19 (not yet released)
   - "playlistadd" supports file:///
   - "idle" with unrecognized event name fails
   - "list" on album artist falls back to the artist tag
+  - "list" allows grouping
 * database
   - proxy: forward "idle" events
   - proxy: copy "Last-Modified" from remote directories

doc/protocol.xml

@@ -1559,6 +1559,9 @@ OK
               <arg choice="opt"><replaceable>FILTERTYPE</replaceable></arg>
               <arg choice="opt"><replaceable>FILTERWHAT</replaceable></arg>
               <arg choice="opt"><replaceable>...</replaceable></arg>
+              <arg choice="opt">group</arg>
+              <arg choice="opt"><replaceable>GROUPTYPE</replaceable></arg>
+              <arg choice="opt"><replaceable>...</replaceable></arg>
             </cmdsynopsis>
           </term>
           <listitem>
@@ -1573,6 +1576,13 @@ OK
               linkend="command_find"><command>find</command>
               command</link>.
             </para>
+            <para>
+              The <parameter>group</parameter> keyword may be used
+              (repeatedly) to group the results by one or more tags.
+              The following example lists all album names,
+              grouped by their respective (album) artist:
+            </para>
+            <programlisting>list album group albumartist</programlisting>
           </listitem>
         </varlistentry>
 

src/command/DatabaseCommands.cxx

@@ -32,6 +32,8 @@
 #include "SongFilter.hxx"
 #include "protocol/Result.hxx"
 
+#include <string.h>
+
 CommandResult
 handle_listfiles_db(Client &client, const char *uri)
 {
@@ -191,6 +193,7 @@ handle_list(Client &client, int argc, char *argv[])
 	}
 
 	SongFilter *filter = nullptr;
+	uint32_t group_mask = 0;
 
 	if (args.size == 1) {
 		/* for compatibility with < 0.12.0 */
@@ -204,6 +207,22 @@ handle_list(Client &client, int argc, char *argv[])
 		filter = new SongFilter((unsigned)TAG_ARTIST, args.shift());
 	}
 
+	while (args.size >= 2 &&
+	       strcmp(args[args.size - 2], "group") == 0) {
+		const char *s = args[args.size - 1];
+		TagType gt = tag_name_parse_i(s);
+		if (gt == TAG_NUM_OF_ITEM_TYPES) {
+			command_error(client, ACK_ERROR_ARG,
+				      "Unknown tag type: %s", s);
+			return CommandResult::ERROR;
+		}
+
+		group_mask |= 1u << unsigned(gt);
+
+		args.pop_back();
+		args.pop_back();
+	}
+
 	if (!args.IsEmpty()) {
 		filter = new SongFilter();
 		if (!filter->Parse(args, false)) {
@@ -216,7 +235,7 @@ handle_list(Client &client, int argc, char *argv[])
 
 	Error error;
 	CommandResult ret =
-		listAllUniqueTags(client, tagType, filter, error)
+		listAllUniqueTags(client, tagType, group_mask, filter, error)
 		? CommandResult::OK
 		: print_error(client, error);
 

src/db/DatabasePrint.cxx

@@ -238,14 +238,24 @@ PrintSongURIVisitor(Client &client, const LightSong &song)
 
 static bool
 PrintUniqueTag(Client &client, TagType tag_type,
-	       const char *value)
+	       const Tag &tag)
 {
+	const char *value = tag.GetValue(tag_type);
+	assert(value != nullptr);
 	client_printf(client, "%s: %s\n", tag_item_names[tag_type], value);
+
+	for (unsigned i = 0, n = tag.num_items; i < n; i++) {
+		const TagItem &item = *tag.items[i];
+		if (item.type != tag_type)
+			client_printf(client, "%s: %s\n",
+				      tag_item_names[item.type], item.value);
+	}
+
 	return true;
 }
 
 bool
-listAllUniqueTags(Client &client, unsigned type,
+listAllUniqueTags(Client &client, unsigned type, uint32_t group_mask,
 		  const SongFilter *filter,
 		  Error &error)
 {
@@ -267,6 +277,7 @@ listAllUniqueTags(Client &client, unsigned type,
 		const auto f = std::bind(PrintUniqueTag, std::ref(client),
 					 (TagType)type, _1);
 		return db->VisitUniqueTags(selection, (TagType)type,
+					   group_mask,
 					   f, error);
 	}
 }

src/db/DatabasePrint.hxx

@@ -22,6 +22,8 @@
 
 #include "Compiler.h"
 
+#include <stdint.h>
+
 class SongFilter;
 struct DatabaseSelection;
 class Client;
@@ -51,7 +53,7 @@ searchStatsForSongsIn(Client &client, const char *name,
 		      Error &error);
 
 bool
-listAllUniqueTags(Client &client, unsigned type,
+listAllUniqueTags(Client &client, unsigned type, uint32_t group_mask,
 		  const SongFilter *filter,
 		  Error &error);
 

src/db/Helpers.cxx

@@ -22,6 +22,7 @@
 #include "Interface.hxx"
 #include "LightSong.hxx"
 #include "tag/Tag.hxx"
+#include "tag/TagBuilder.hxx"
 
 #include <functional>
 #include <set>
@@ -38,13 +39,100 @@ struct StringLess {
 
 typedef std::set<const char *, StringLess> StringSet;
 
+struct TagLess {
+	gcc_pure
+	bool operator()(const Tag &a, const Tag &b) const {
+		if (a.num_items != b.num_items)
+			return a.num_items < b.num_items;
+
+		const unsigned n = a.num_items;
+		for (unsigned i = 0; i < n; ++i) {
+			const TagItem &ai = *a.items[i];
+			const TagItem &bi = *b.items[i];
+			if (ai.type != bi.type)
+				return unsigned(ai.type) < unsigned(bi.type);
+
+			const int cmp = strcmp(ai.value, bi.value);
+			if (cmp != 0)
+				return cmp < 0;
+		}
+
+		return false;
+	}
+};
+
+typedef std::set<Tag, TagLess> TagSet;
+
+/**
+ * Copy all tag items of the specified type.
+ */
 static bool
-CheckUniqueTag(StringSet &set, const Tag &tag, TagType type)
+CopyTagItem(TagBuilder &dest, TagType dest_type,
+	    const Tag &src, TagType src_type)
+{
+	bool found = false;
+	const unsigned n = src.num_items;
+	for (unsigned i = 0; i < n; ++i) {
+		if (src.items[i]->type == src_type) {
+			dest.AddItem(dest_type, src.items[i]->value);
+			found = true;
+		}
+	}
+
+	return found;
+}
+
+/**
+ * Copy all tag items of the specified type.  Fall back to "Artist" if
+ * there is no "AlbumArtist".
+ */
+static void
+CopyTagItem(TagBuilder &dest, const Tag &src, TagType type)
+{
+	if (!CopyTagItem(dest, type, src, type) &&
+	    type == TAG_ALBUM_ARTIST)
+		CopyTagItem(dest, type, src, TAG_ARTIST);
+}
+
+/**
+ * Copy all tag items of the types in the mask.
+ */
+static void
+CopyTagMask(TagBuilder &dest, const Tag &src, uint32_t mask)
+{
+	for (unsigned i = 0; i < TAG_NUM_OF_ITEM_TYPES; ++i)
+		if ((mask & (1u << i)) != 0)
+			CopyTagItem(dest, src, TagType(i));
+}
+
+static void
+InsertUniqueTag(TagSet &set, const Tag &src, TagType type, const char *value,
+		uint32_t group_mask)
+{
+	TagBuilder builder;
+	if (value == nullptr)
+		builder.AddEmptyItem(type);
+	else
+		builder.AddItem(type, value);
+	CopyTagMask(builder, src, group_mask);
+#if defined(__clang__) || GCC_CHECK_VERSION(4,8)
+	set.emplace(builder.Commit());
+#else
+	set.insert(builder.Commit());
+#endif
+}
+
+static bool
+CheckUniqueTag(TagSet &set, TagType dest_type,
+	       const Tag &tag, TagType src_type,
+	       uint32_t group_mask)
 {
 	bool found = false;
 	for (unsigned i = 0; i < tag.num_items; ++i) {
-		if (tag.items[i]->type == type) {
+		if (tag.items[i]->type == src_type) {
-			set.insert(tag.items[i]->value);
+			InsertUniqueTag(set, tag, dest_type,
+					tag.items[i]->value,
+					group_mask);
 			found = true;
 		}
 	}
@@ -53,35 +141,42 @@ CheckUniqueTag(StringSet &set, const Tag &tag, TagType type)
 }
 
 static bool
-CollectTags(StringSet &set, TagType tag_type, const LightSong &song)
+CollectTags(TagSet &set, TagType tag_type, uint32_t group_mask,
+	    const LightSong &song)
 {
+	static_assert(sizeof(group_mask) * 8 >= TAG_NUM_OF_ITEM_TYPES,
+		      "Mask is too small");
+
+	assert((group_mask & (1u << unsigned(tag_type))) == 0);
+
 	assert(song.tag != nullptr);
 	const Tag &tag = *song.tag;
 
-	if (!CheckUniqueTag(set, tag, tag_type) &&
+	if (!CheckUniqueTag(set, tag_type, tag, tag_type, group_mask) &&
 	    (tag_type != TAG_ALBUM_ARTIST ||
 	     /* fall back to "Artist" if no "AlbumArtist" was found */
-	     !CheckUniqueTag(set, tag, TAG_ARTIST)))
+	     !CheckUniqueTag(set, tag_type, tag, TAG_ARTIST, group_mask)))
-		set.insert("");
+		InsertUniqueTag(set, tag, tag_type, nullptr, group_mask);
 
 	return true;
 }
 
 bool
 VisitUniqueTags(const Database &db, const DatabaseSelection &selection,
-		TagType tag_type,
+		TagType tag_type, uint32_t group_mask,
-		VisitString visit_string,
+		VisitTag visit_tag,
 		Error &error)
 {
-	StringSet set;
+	TagSet set;
 
 	using namespace std::placeholders;
-	const auto f = std::bind(CollectTags, std::ref(set), tag_type, _1);
+	const auto f = std::bind(CollectTags, std::ref(set),
+				 tag_type, group_mask, _1);
 	if (!db.Visit(selection, f, error))
 		return false;
 
-	for (auto value : set)
+	for (const auto &value : set)
-		if (!visit_string(value, error))
+		if (!visit_tag(value, error))
 			return false;
 
 	return true;

src/db/Helpers.hxx

@@ -23,6 +23,8 @@
 #include "Visitor.hxx"
 #include "tag/TagType.h"
 
+#include <stdint.h>
+
 class Error;
 class Database;
 struct DatabaseSelection;
@@ -30,8 +32,8 @@ struct DatabaseStats;
 
 bool
 VisitUniqueTags(const Database &db, const DatabaseSelection &selection,
-		TagType tag_type,
+		TagType tag_type, uint32_t group_mask,
-		VisitString visit_string,
+		VisitTag visit_tag,
 		Error &error);
 
 bool

src/db/Interface.hxx

@@ -25,6 +25,7 @@
 #include "Compiler.h"
 
 #include <time.h>
+#include <stdint.h>
 
 struct DatabasePlugin;
 struct DatabaseStats;
@@ -106,8 +107,8 @@ public:
 	 * Visit all unique tag values.
 	 */
 	virtual bool VisitUniqueTags(const DatabaseSelection &selection,
-				     TagType tag_type,
+				     TagType tag_type, uint32_t group_mask,
-				     VisitString visit_string,
+				     VisitTag visit_tag,
 				     Error &error) const = 0;
 
 	virtual bool GetStats(const DatabaseSelection &selection,

src/db/Visitor.hxx

@@ -25,6 +25,7 @@
 struct LightDirectory;
 struct LightSong;
 struct PlaylistInfo;
+struct Tag;
 class Error;
 
 typedef std::function<bool(const LightDirectory &, Error &)> VisitDirectory;
@@ -32,6 +33,6 @@ typedef std::function<bool(const LightSong &, Error &)> VisitSong;
 typedef std::function<bool(const PlaylistInfo &, const LightDirectory &,
 			   Error &)> VisitPlaylist;
 
-typedef std::function<bool(const char *, Error &)> VisitString;
+typedef std::function<bool(const Tag &, Error &)> VisitTag;
 
 #endif

src/db/plugins/LazyDatabase.cxx

@@ -85,12 +85,13 @@ LazyDatabase::Visit(const DatabaseSelection &selection,
 
 bool
 LazyDatabase::VisitUniqueTags(const DatabaseSelection &selection,
-			      TagType tag_type,
+			      TagType tag_type, uint32_t group_mask,
-			      VisitString visit_string,
+			      VisitTag visit_tag,
 			      Error &error) const
 {
 	return EnsureOpen(error) &&
-		db->VisitUniqueTags(selection, tag_type, visit_string, error);
+		db->VisitUniqueTags(selection, tag_type, group_mask, visit_tag,
+				    error);
 }
 
 bool

src/db/plugins/LazyDatabase.hxx

@@ -52,8 +52,8 @@ public:
 			   Error &error) const override;
 
 	virtual bool VisitUniqueTags(const DatabaseSelection &selection,
-				     TagType tag_type,
+				     TagType tag_type, uint32_t group_mask,
-				     VisitString visit_string,
+				     VisitTag visit_tag,
 				     Error &error) const override;
 
 	virtual bool GetStats(const DatabaseSelection &selection,

src/db/plugins/ProxyDatabasePlugin.cxx

@@ -112,8 +112,8 @@ public:
 			   Error &error) const override;
 
 	virtual bool VisitUniqueTags(const DatabaseSelection &selection,
-				     TagType tag_type,
+				     TagType tag_type, uint32_t group_mask,
-				     VisitString visit_string,
+				     VisitTag visit_tag,
 				     Error &error) const override;
 
 	virtual bool GetStats(const DatabaseSelection &selection,
@@ -715,7 +715,8 @@ ProxyDatabase::Visit(const DatabaseSelection &selection,
 bool
 ProxyDatabase::VisitUniqueTags(const DatabaseSelection &selection,
 			       TagType tag_type,
-			       VisitString visit_string,
+			       gcc_unused uint32_t group_mask,
+			       VisitTag visit_tag,
 			       Error &error) const
 {
 	// TODO: eliminate the const_cast
@@ -734,6 +735,8 @@ ProxyDatabase::VisitUniqueTags(const DatabaseSelection &selection,
 	if (!SendConstraints(connection, selection))
 		return CheckError(connection, error);
 
+	// TODO: use group_mask
+
 	if (!mpd_search_commit(connection))
 		return CheckError(connection, error);
 
@@ -742,7 +745,9 @@ ProxyDatabase::VisitUniqueTags(const DatabaseSelection &selection,
 	struct mpd_pair *pair;
 	while (result &&
 	       (pair = mpd_recv_pair_tag(connection, tag_type2)) != nullptr) {
-		result = visit_string(pair->value, error);
+		TagBuilder tag;
+		tag.AddItem(tag_type, pair->value);
+		result = visit_tag(tag.Commit(), error);
 		mpd_return_pair(connection, pair);
 	}
 

src/db/plugins/simple/SimpleDatabasePlugin.cxx

@@ -334,11 +334,12 @@ SimpleDatabase::Visit(const DatabaseSelection &selection,
 
 bool
 SimpleDatabase::VisitUniqueTags(const DatabaseSelection &selection,
-				TagType tag_type,
+				TagType tag_type, uint32_t group_mask,
-				VisitString visit_string,
+				VisitTag visit_tag,
 				Error &error) const
 {
-	return ::VisitUniqueTags(*this, selection, tag_type, visit_string,
+	return ::VisitUniqueTags(*this, selection, tag_type, group_mask,
+				 visit_tag,
 				 error);
 }
 

src/db/plugins/simple/SimpleDatabasePlugin.hxx

@@ -116,8 +116,8 @@ public:
 			   Error &error) const override;
 
 	virtual bool VisitUniqueTags(const DatabaseSelection &selection,
-				     TagType tag_type,
+				     TagType tag_type, uint32_t group_mask,
-				     VisitString visit_string,
+				     VisitTag visit_tag,
 				     Error &error) const override;
 
 	virtual bool GetStats(const DatabaseSelection &selection,

src/db/plugins/upnp/UpnpDatabasePlugin.cxx

@@ -94,8 +94,8 @@ public:
 			   Error &error) const override;
 
 	virtual bool VisitUniqueTags(const DatabaseSelection &selection,
-				     TagType tag_type,
+				     TagType tag_type, uint32_t group_mask,
-				     VisitString visit_string,
+				     VisitTag visit_tag,
 				     Error &error) const override;
 
 	virtual bool GetStats(const DatabaseSelection &selection,
@@ -721,11 +721,13 @@ UpnpDatabase::Visit(const DatabaseSelection &selection,
 
 bool
 UpnpDatabase::VisitUniqueTags(const DatabaseSelection &selection,
-			      TagType tag,
+			      TagType tag, gcc_unused uint32_t group_mask,
-			      VisitString visit_string,
+			      VisitTag visit_tag,
 			      Error &error) const
 {
-	if (!visit_string)
+	// TODO: use group_mask
+
+	if (!visit_tag)
 		return true;
 
 	std::vector<ContentDirectoryService> servers;
@@ -754,9 +756,12 @@ UpnpDatabase::VisitUniqueTags(const DatabaseSelection &selection,
 		}
 	}
 
-	for (const auto& value : values)
+	for (const auto& value : values) {
-		if (!visit_string(value.c_str(), error))
+		TagBuilder builder;
+		builder.AddItem(tag, value.c_str());
+		if (!visit_tag(builder.Commit(), error))
 			return false;
+	}
 
 	return true;
 }