From ae178c77bdc47c954fd9a4b32ffc07fe6c4a8a49 Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@duempel.org>
Date: Thu, 24 Apr 2014 10:20:24 +0200
Subject: [PATCH] DatabaseCommands: "list" allows grouping
---
NEWS | 1 +
doc/protocol.xml | 10 ++
src/command/DatabaseCommands.cxx | 21 ++-
src/db/DatabasePrint.cxx | 15 ++-
src/db/DatabasePrint.hxx | 4 +-
src/db/Helpers.cxx | 121 ++++++++++++++++--
src/db/Helpers.hxx | 6 +-
src/db/Interface.hxx | 5 +-
src/db/Visitor.hxx | 3 +-
src/db/plugins/LazyDatabase.cxx | 7 +-
src/db/plugins/LazyDatabase.hxx | 4 +-
src/db/plugins/ProxyDatabasePlugin.cxx | 13 +-
.../plugins/simple/SimpleDatabasePlugin.cxx | 7 +-
.../plugins/simple/SimpleDatabasePlugin.hxx | 4 +-
src/db/plugins/upnp/UpnpDatabasePlugin.cxx | 19 ++-
15 files changed, 197 insertions(+), 43 deletions(-)
diff --git a/NEWS b/NEWS
index a64400e32..d356b271c 100644
--- a/NEWS
+++ b/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
diff --git a/doc/protocol.xml b/doc/protocol.xml
index b67efd69b..e1fb08a84 100644
--- a/doc/protocol.xml
+++ b/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>
diff --git a/src/command/DatabaseCommands.cxx b/src/command/DatabaseCommands.cxx
index 71ae3ec8a..23ff593a4 100644
--- a/src/command/DatabaseCommands.cxx
+++ b/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);
diff --git a/src/db/DatabasePrint.cxx b/src/db/DatabasePrint.cxx
index 781866272..d84c26590 100644
--- a/src/db/DatabasePrint.cxx
+++ b/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);
}
}
diff --git a/src/db/DatabasePrint.hxx b/src/db/DatabasePrint.hxx
index da8fecece..c4c719ca0 100644
--- a/src/db/DatabasePrint.hxx
+++ b/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);
diff --git a/src/db/Helpers.cxx b/src/db/Helpers.cxx
index 43b0e8159..6259d4f40 100644
--- a/src/db/Helpers.cxx
+++ b/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) {
- set.insert(tag.items[i]->value);
+ if (tag.items[i]->type == src_type) {
+ 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)))
- set.insert("");
+ !CheckUniqueTag(set, tag_type, tag, TAG_ARTIST, group_mask)))
+ InsertUniqueTag(set, tag, tag_type, nullptr, group_mask);
return true;
}
bool
VisitUniqueTags(const Database &db, const DatabaseSelection &selection,
- TagType tag_type,
- VisitString visit_string,
+ TagType tag_type, uint32_t group_mask,
+ 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)
- if (!visit_string(value, error))
+ for (const auto &value : set)
+ if (!visit_tag(value, error))
return false;
return true;
diff --git a/src/db/Helpers.hxx b/src/db/Helpers.hxx
index 24db260c0..b9a7af4cf 100644
--- a/src/db/Helpers.hxx
+++ b/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,
- VisitString visit_string,
+ TagType tag_type, uint32_t group_mask,
+ VisitTag visit_tag,
Error &error);
bool
diff --git a/src/db/Interface.hxx b/src/db/Interface.hxx
index 11e2da12e..5fc9265e2 100644
--- a/src/db/Interface.hxx
+++ b/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,
- VisitString visit_string,
+ TagType tag_type, uint32_t group_mask,
+ VisitTag visit_tag,
Error &error) const = 0;
virtual bool GetStats(const DatabaseSelection &selection,
diff --git a/src/db/Visitor.hxx b/src/db/Visitor.hxx
index 0ec29bf49..c524f1722 100644
--- a/src/db/Visitor.hxx
+++ b/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
diff --git a/src/db/plugins/LazyDatabase.cxx b/src/db/plugins/LazyDatabase.cxx
index 2b576b1ee..bc52395c5 100644
--- a/src/db/plugins/LazyDatabase.cxx
+++ b/src/db/plugins/LazyDatabase.cxx
@@ -85,12 +85,13 @@ LazyDatabase::Visit(const DatabaseSelection &selection,
bool
LazyDatabase::VisitUniqueTags(const DatabaseSelection &selection,
- TagType tag_type,
- VisitString visit_string,
+ TagType tag_type, uint32_t group_mask,
+ 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
diff --git a/src/db/plugins/LazyDatabase.hxx b/src/db/plugins/LazyDatabase.hxx
index 000b63a98..ae1b961d0 100644
--- a/src/db/plugins/LazyDatabase.hxx
+++ b/src/db/plugins/LazyDatabase.hxx
@@ -52,8 +52,8 @@ public:
Error &error) const override;
virtual bool VisitUniqueTags(const DatabaseSelection &selection,
- TagType tag_type,
- VisitString visit_string,
+ TagType tag_type, uint32_t group_mask,
+ VisitTag visit_tag,
Error &error) const override;
virtual bool GetStats(const DatabaseSelection &selection,
diff --git a/src/db/plugins/ProxyDatabasePlugin.cxx b/src/db/plugins/ProxyDatabasePlugin.cxx
index 0ab25005a..a3cc640cf 100644
--- a/src/db/plugins/ProxyDatabasePlugin.cxx
+++ b/src/db/plugins/ProxyDatabasePlugin.cxx
@@ -112,8 +112,8 @@ public:
Error &error) const override;
virtual bool VisitUniqueTags(const DatabaseSelection &selection,
- TagType tag_type,
- VisitString visit_string,
+ TagType tag_type, uint32_t group_mask,
+ 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);
}
diff --git a/src/db/plugins/simple/SimpleDatabasePlugin.cxx b/src/db/plugins/simple/SimpleDatabasePlugin.cxx
index 68101f564..d616c793f 100644
--- a/src/db/plugins/simple/SimpleDatabasePlugin.cxx
+++ b/src/db/plugins/simple/SimpleDatabasePlugin.cxx
@@ -334,11 +334,12 @@ SimpleDatabase::Visit(const DatabaseSelection &selection,
bool
SimpleDatabase::VisitUniqueTags(const DatabaseSelection &selection,
- TagType tag_type,
- VisitString visit_string,
+ TagType tag_type, uint32_t group_mask,
+ 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);
}
diff --git a/src/db/plugins/simple/SimpleDatabasePlugin.hxx b/src/db/plugins/simple/SimpleDatabasePlugin.hxx
index a03969f92..e27b3d956 100644
--- a/src/db/plugins/simple/SimpleDatabasePlugin.hxx
+++ b/src/db/plugins/simple/SimpleDatabasePlugin.hxx
@@ -116,8 +116,8 @@ public:
Error &error) const override;
virtual bool VisitUniqueTags(const DatabaseSelection &selection,
- TagType tag_type,
- VisitString visit_string,
+ TagType tag_type, uint32_t group_mask,
+ VisitTag visit_tag,
Error &error) const override;
virtual bool GetStats(const DatabaseSelection &selection,
diff --git a/src/db/plugins/upnp/UpnpDatabasePlugin.cxx b/src/db/plugins/upnp/UpnpDatabasePlugin.cxx
index abc7638f8..66951f402 100644
--- a/src/db/plugins/upnp/UpnpDatabasePlugin.cxx
+++ b/src/db/plugins/upnp/UpnpDatabasePlugin.cxx
@@ -94,8 +94,8 @@ public:
Error &error) const override;
virtual bool VisitUniqueTags(const DatabaseSelection &selection,
- TagType tag_type,
- VisitString visit_string,
+ TagType tag_type, uint32_t group_mask,
+ 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,
- VisitString visit_string,
+ TagType tag, gcc_unused uint32_t group_mask,
+ 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)
- if (!visit_string(value.c_str(), error))
+ for (const auto& value : values) {
+ TagBuilder builder;
+ builder.AddItem(tag, value.c_str());
+ if (!visit_tag(builder.Commit(), error))
return false;
+ }
return true;
}