diff --git a/Makefile.am b/Makefile.am index 1061d838d..50b98da76 100644 --- a/Makefile.am +++ b/Makefile.am @@ -985,7 +985,6 @@ libtag_a_SOURCES =\ src/tag/TagString.cxx src/tag/TagString.hxx \ src/tag/TagPool.cxx src/tag/TagPool.hxx \ src/tag/TagTable.cxx src/tag/TagTable.hxx \ - src/tag/Set.cxx src/tag/Set.hxx \ src/tag/Format.cxx src/tag/Format.hxx \ src/tag/VorbisComment.cxx src/tag/VorbisComment.hxx \ src/tag/ReplayGain.cxx src/tag/ReplayGain.hxx \ diff --git a/NEWS b/NEWS index 5b8a3b48c..8b0029784 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,7 @@ ver 0.20.22 (not yet released) * protocol - add tag fallbacks for AlbumArtistSort, ArtistSort - "count group ..." can print an empty group + - fix broken command "list ... group" * storage - curl: URL-encode paths * Android diff --git a/configure.ac b/configure.ac index 4e9f8b460..687823dc4 100644 --- a/configure.ac +++ b/configure.ac @@ -14,7 +14,7 @@ AM_SILENT_RULES AC_CONFIG_HEADERS(config.h) AC_CONFIG_MACRO_DIR([m4]) -AC_DEFINE(PROTOCOL_VERSION, "0.20.0", [The MPD protocol version]) +AC_DEFINE(PROTOCOL_VERSION, "0.20.22", [The MPD protocol version]) GIT_COMMIT=`cd "$srcdir" && git describe --dirty --always 2>/dev/null` if test x$GIT_COMMIT != x; then diff --git a/src/command/DatabaseCommands.cxx b/src/command/DatabaseCommands.cxx index 42f79b8e2..3f7d18324 100644 --- a/src/command/DatabaseCommands.cxx +++ b/src/command/DatabaseCommands.cxx @@ -191,7 +191,7 @@ handle_list(Client &client, Request args, Response &r) } std::unique_ptr filter; - tag_mask_t group_mask = 0; + TagType group = TAG_NUM_OF_ITEM_TYPES; if (args.size == 1) { /* for compatibility with < 0.12.0 */ @@ -206,18 +206,16 @@ handle_list(Client &client, Request args, Response &r) args.shift())); } - while (args.size >= 2 && - StringIsEqual(args[args.size - 2], "group")) { + if (args.size >= 2 && + StringIsEqual(args[args.size - 2], "group")) { const char *s = args[args.size - 1]; - TagType gt = tag_name_parse_i(s); - if (gt == TAG_NUM_OF_ITEM_TYPES) { + group = tag_name_parse_i(s); + if (group == TAG_NUM_OF_ITEM_TYPES) { r.FormatError(ACK_ERROR_ARG, "Unknown tag type: %s", s); return CommandResult::ERROR; } - group_mask |= tag_mask_t(1) << unsigned(gt); - args.pop_back(); args.pop_back(); } @@ -230,14 +228,13 @@ handle_list(Client &client, Request args, Response &r) } } - if (tagType < TAG_NUM_OF_ITEM_TYPES && - group_mask & (tag_mask_t(1) << tagType)) { + if (tagType < TAG_NUM_OF_ITEM_TYPES && tagType == group) { r.Error(ACK_ERROR_ARG, "Conflicting group"); return CommandResult::ERROR; } PrintUniqueTags(r, client.partition, - tagType, group_mask, filter.get()); + tagType, group, filter.get()); return CommandResult::OK; } diff --git a/src/db/DatabasePrint.cxx b/src/db/DatabasePrint.cxx index 87d441d20..d4d386912 100644 --- a/src/db/DatabasePrint.cxx +++ b/src/db/DatabasePrint.cxx @@ -187,22 +187,34 @@ PrintSongURIVisitor(Response &r, Partition &partition, const LightSong &song) } static void -PrintUniqueTag(Response &r, TagType tag_type, - const Tag &tag) +PrintUniqueTags(Response &r, TagType tag_type, + const std::set &values) { - const char *value = tag.GetValue(tag_type); - assert(value != nullptr); - r.Format("%s: %s\n", tag_item_names[tag_type], value); + const char *const name = tag_item_names[tag_type]; + for (const auto &i : values) + r.Format("%s: %s\n", name, i.c_str()); +} - for (const auto &item : tag) - if (item.type != tag_type) - r.Format("%s: %s\n", - tag_item_names[item.type], item.value); +static void +PrintGroupedUniqueTags(Response &r, TagType tag_type, TagType group, + const std::map> &groups) +{ + if (group == TAG_NUM_OF_ITEM_TYPES) { + for (const auto &i : groups) + PrintUniqueTags(r, tag_type, i.second); + return; + } + + const char *const group_name = tag_item_names[group]; + for (const auto &i : groups) { + r.Format("%s: %s\n", group_name, i.first.c_str()); + PrintUniqueTags(r, tag_type, i.second); + } } void PrintUniqueTags(Response &r, Partition &partition, - unsigned type, tag_mask_t group_mask, + unsigned type, TagType group, const SongFilter *filter) { const Database &db = partition.GetDatabaseOrThrow(); @@ -217,10 +229,9 @@ PrintUniqueTags(Response &r, Partition &partition, } else { assert(type < TAG_NUM_OF_ITEM_TYPES); - using namespace std::placeholders; - const auto f = std::bind(PrintUniqueTag, std::ref(r), - (TagType)type, _1); - db.VisitUniqueTags(selection, (TagType)type, - group_mask, f); + PrintGroupedUniqueTags(r, TagType(type), group, + db.CollectUniqueTags(selection, + TagType(type), + group)); } } diff --git a/src/db/DatabasePrint.hxx b/src/db/DatabasePrint.hxx index 51b9a92cd..bbb52b101 100644 --- a/src/db/DatabasePrint.hxx +++ b/src/db/DatabasePrint.hxx @@ -20,7 +20,7 @@ #ifndef MPD_DB_PRINT_H #define MPD_DB_PRINT_H -#include "tag/Mask.hxx" +#include "tag/TagType.h" class SongFilter; struct DatabaseSelection; @@ -44,7 +44,7 @@ db_selection_print(Response &r, Partition &partition, void PrintUniqueTags(Response &r, Partition &partition, - unsigned type, tag_mask_t group_mask, + unsigned type, TagType group, const SongFilter *filter); #endif diff --git a/src/db/Interface.hxx b/src/db/Interface.hxx index a61c323d3..656dfaa76 100644 --- a/src/db/Interface.hxx +++ b/src/db/Interface.hxx @@ -22,9 +22,12 @@ #include "Visitor.hxx" #include "tag/TagType.h" -#include "tag/Mask.hxx" #include "Compiler.h" +#include +#include +#include + #include struct DatabasePlugin; @@ -99,12 +102,9 @@ public: return Visit(selection, VisitDirectory(), visit_song); } - /** - * Visit all unique tag values. - */ - virtual void VisitUniqueTags(const DatabaseSelection &selection, - TagType tag_type, tag_mask_t group_mask, - VisitTag visit_tag) const = 0; + virtual std::map> CollectUniqueTags(const DatabaseSelection &selection, + TagType tag_type, + TagType group=TAG_NUM_OF_ITEM_TYPES) const = 0; virtual DatabaseStats GetStats(const DatabaseSelection &selection) const = 0; diff --git a/src/db/UniqueTags.cxx b/src/db/UniqueTags.cxx index c888fe7b4..4e33f5df6 100644 --- a/src/db/UniqueTags.cxx +++ b/src/db/UniqueTags.cxx @@ -20,34 +20,42 @@ #include "UniqueTags.hxx" #include "Interface.hxx" #include "LightSong.hxx" -#include "tag/Set.hxx" +#include "tag/VisitFallback.hxx" #include #include static void -CollectTags(TagSet &set, TagType tag_type, tag_mask_t group_mask, - const LightSong &song) +CollectTags(std::set &result, + const Tag &tag, + TagType tag_type) noexcept { - assert(song.tag != nullptr); - const Tag &tag = *song.tag; - - set.InsertUnique(tag, tag_type, group_mask); + VisitTagWithFallbackOrEmpty(tag, tag_type, [&result](const char *value){ + result.emplace(value); + }); } -void -VisitUniqueTags(const Database &db, const DatabaseSelection &selection, - TagType tag_type, tag_mask_t group_mask, - VisitTag visit_tag) +static void +CollectGroupTags(std::map> &result, + const Tag &tag, + TagType tag_type, + TagType group) noexcept { - TagSet set; - - using namespace std::placeholders; - const auto f = std::bind(CollectTags, std::ref(set), - tag_type, group_mask, _1); - db.Visit(selection, f); - - for (const auto &value : set) - visit_tag(value); + VisitTagWithFallbackOrEmpty(tag, group, [&](const char *group_name){ + CollectTags(result[group_name], tag, tag_type); + }); +} + +std::map> +CollectUniqueTags(const Database &db, const DatabaseSelection &selection, + TagType tag_type, TagType group) +{ + std::map> result; + + db.Visit(selection, [&result, tag_type, group](const LightSong &song){ + CollectGroupTags(result, *song.tag, tag_type, group); + }); + + return result; } diff --git a/src/db/UniqueTags.hxx b/src/db/UniqueTags.hxx index efb2a089d..83e6e7f09 100644 --- a/src/db/UniqueTags.hxx +++ b/src/db/UniqueTags.hxx @@ -20,16 +20,19 @@ #ifndef MPD_DB_UNIQUE_TAGS_HXX #define MPD_DB_UNIQUE_TAGS_HXX -#include "Visitor.hxx" #include "tag/TagType.h" -#include "tag/Mask.hxx" +#include "Compiler.h" + +#include +#include +#include class Database; struct DatabaseSelection; -void -VisitUniqueTags(const Database &db, const DatabaseSelection &selection, - TagType tag_type, tag_mask_t group_mask, - VisitTag visit_tag); +gcc_pure +std::map> +CollectUniqueTags(const Database &db, const DatabaseSelection &selection, + TagType tag_type, TagType group); #endif diff --git a/src/db/plugins/ProxyDatabasePlugin.cxx b/src/db/plugins/ProxyDatabasePlugin.cxx index b3d2d0a05..d6954968f 100644 --- a/src/db/plugins/ProxyDatabasePlugin.cxx +++ b/src/db/plugins/ProxyDatabasePlugin.cxx @@ -120,9 +120,9 @@ public: VisitSong visit_song, VisitPlaylist visit_playlist) const override; - void VisitUniqueTags(const DatabaseSelection &selection, - TagType tag_type, tag_mask_t group_mask, - VisitTag visit_tag) const override; + std::map> CollectUniqueTags(const DatabaseSelection &selection, + TagType tag_type, + TagType group) const override; DatabaseStats GetStats(const DatabaseSelection &selection) const override; @@ -334,28 +334,19 @@ SendConstraints(mpd_connection *connection, const DatabaseSelection &selection) } static bool -SendGroupMask(mpd_connection *connection, tag_mask_t mask) +SendGroup(mpd_connection *connection, TagType group) { + if (group == TAG_NUM_OF_ITEM_TYPES) + return true; + #if LIBMPDCLIENT_CHECK_VERSION(2,12,0) - for (unsigned i = 0; i < TAG_NUM_OF_ITEM_TYPES; ++i) { - if ((mask & (tag_mask_t(1) << i)) == 0) - continue; + const auto tag = Convert(group); + if (tag == MPD_TAG_COUNT) + throw std::runtime_error("Unsupported tag"); - const auto tag = Convert(TagType(i)); - if (tag == MPD_TAG_COUNT) - throw std::runtime_error("Unsupported tag"); - - if (!mpd_search_add_group_tag(connection, tag)) - return false; - } - - return true; + return mpd_search_add_group_tag(connection, tag); #else (void)connection; - (void)mask; - - if (mask != 0) - throw std::runtime_error("Grouping requires libmpdclient 2.12"); return true; #endif @@ -799,11 +790,9 @@ ProxyDatabase::Visit(const DatabaseSelection &selection, visit_directory, visit_song, visit_playlist); } -void -ProxyDatabase::VisitUniqueTags(const DatabaseSelection &selection, - TagType tag_type, - tag_mask_t group_mask, - VisitTag visit_tag) const +std::map> +ProxyDatabase::CollectUniqueTags(const DatabaseSelection &selection, + TagType tag_type, TagType group) const try { // TODO: eliminate the const_cast const_cast(this)->EnsureConnected(); @@ -814,54 +803,56 @@ try { if (!mpd_search_db_tags(connection, tag_type2) || !SendConstraints(connection, selection) || - !SendGroupMask(connection, group_mask)) + !SendGroup(connection, group)) ThrowError(connection); if (!mpd_search_commit(connection)) ThrowError(connection); - TagBuilder builder; + std::map> result; - while (auto *pair = mpd_recv_pair(connection)) { - AtScopeExit(this, pair) { - mpd_return_pair(connection, pair); - }; + if (group == TAG_NUM_OF_ITEM_TYPES) { + auto &values = result[std::string()]; - const auto current_type = tag_name_parse_i(pair->name); - if (current_type == TAG_NUM_OF_ITEM_TYPES) - continue; + while (auto *pair = mpd_recv_pair(connection)) { + AtScopeExit(this, pair) { + mpd_return_pair(connection, pair); + }; - if (current_type == tag_type && !builder.IsEmpty()) { - try { - visit_tag(builder.Commit()); - } catch (...) { - mpd_response_finish(connection); - throw; - } + const auto current_type = tag_name_parse_i(pair->name); + if (current_type == TAG_NUM_OF_ITEM_TYPES) + continue; + + if (current_type == tag_type) + values.emplace(pair->value); } + } else { + std::set *current_group = nullptr; - builder.AddItem(current_type, pair->value); + while (auto *pair = mpd_recv_pair(connection)) { + AtScopeExit(this, pair) { + mpd_return_pair(connection, pair); + }; - if (!builder.HasType(current_type)) - /* if no tag item has been added, then the - given value was not acceptable - (e.g. empty); forcefully insert an empty - tag in this case, as the caller expects the - given tag type to be present */ - builder.AddEmptyItem(current_type); - } + const auto current_type = tag_name_parse_i(pair->name); + if (current_type == TAG_NUM_OF_ITEM_TYPES) + continue; - if (!builder.IsEmpty()) { - try { - visit_tag(builder.Commit()); - } catch (...) { - mpd_response_finish(connection); - throw; + if (current_type == tag_type) { + if (current_group == nullptr) + current_group = &result[std::string()]; + + current_group->emplace(pair->value); + } else if (current_type == group) { + current_group = &result[pair->value]; + } } } if (!mpd_response_finish(connection)) ThrowError(connection); + + return result; } catch (...) { if (connection != nullptr) mpd_search_cancel(connection); diff --git a/src/db/plugins/simple/SimpleDatabasePlugin.cxx b/src/db/plugins/simple/SimpleDatabasePlugin.cxx index a27c070c6..1e5bf8f7e 100644 --- a/src/db/plugins/simple/SimpleDatabasePlugin.cxx +++ b/src/db/plugins/simple/SimpleDatabasePlugin.cxx @@ -312,12 +312,11 @@ SimpleDatabase::Visit(const DatabaseSelection &selection, "No such directory"); } -void -SimpleDatabase::VisitUniqueTags(const DatabaseSelection &selection, - TagType tag_type, tag_mask_t group_mask, - VisitTag visit_tag) const +std::map> +SimpleDatabase::CollectUniqueTags(const DatabaseSelection &selection, + TagType tag_type, TagType group) const { - ::VisitUniqueTags(*this, selection, tag_type, group_mask, visit_tag); + return ::CollectUniqueTags(*this, selection, tag_type, group); } DatabaseStats diff --git a/src/db/plugins/simple/SimpleDatabasePlugin.hxx b/src/db/plugins/simple/SimpleDatabasePlugin.hxx index 4336fb58b..4c58c73d3 100644 --- a/src/db/plugins/simple/SimpleDatabasePlugin.hxx +++ b/src/db/plugins/simple/SimpleDatabasePlugin.hxx @@ -119,9 +119,9 @@ public: VisitSong visit_song, VisitPlaylist visit_playlist) const override; - void VisitUniqueTags(const DatabaseSelection &selection, - TagType tag_type, tag_mask_t group_mask, - VisitTag visit_tag) const override; + std::map> CollectUniqueTags(const DatabaseSelection &selection, + TagType tag_type, + TagType group) const override; DatabaseStats GetStats(const DatabaseSelection &selection) const override; diff --git a/src/db/plugins/upnp/UpnpDatabasePlugin.cxx b/src/db/plugins/upnp/UpnpDatabasePlugin.cxx index c49d8a725..81548f98e 100644 --- a/src/db/plugins/upnp/UpnpDatabasePlugin.cxx +++ b/src/db/plugins/upnp/UpnpDatabasePlugin.cxx @@ -87,9 +87,9 @@ public: VisitSong visit_song, VisitPlaylist visit_playlist) const override; - void VisitUniqueTags(const DatabaseSelection &selection, - TagType tag_type, tag_mask_t group_mask, - VisitTag visit_tag) const override; + std::map> CollectUniqueTags(const DatabaseSelection &selection, + TagType tag_type, + TagType group) const override; DatabaseStats GetStats(const DatabaseSelection &selection) const override; @@ -603,17 +603,15 @@ UpnpDatabase::Visit(const DatabaseSelection &selection, visit_directory, visit_song, visit_playlist); } -void -UpnpDatabase::VisitUniqueTags(const DatabaseSelection &selection, - TagType tag, gcc_unused tag_mask_t group_mask, - VisitTag visit_tag) const +std::map> +UpnpDatabase::CollectUniqueTags(const DatabaseSelection &selection, + TagType tag, TagType group) const { - // TODO: use group_mask + (void)group; // TODO: use group - if (!visit_tag) - return; + std::map> result; + auto &values = result[std::string()]; - std::set values; for (auto& server : discovery->GetDirectories()) { const auto dirbuf = SearchSongs(server, rootid, selection); @@ -633,11 +631,7 @@ UpnpDatabase::VisitUniqueTags(const DatabaseSelection &selection, } } - for (const auto& value : values) { - TagBuilder builder; - builder.AddItem(tag, value.c_str()); - visit_tag(builder.Commit()); - } + return result; } DatabaseStats diff --git a/src/tag/Set.cxx b/src/tag/Set.cxx deleted file mode 100644 index d52d517aa..000000000 --- a/src/tag/Set.cxx +++ /dev/null @@ -1,117 +0,0 @@ -/* - * 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. - */ - -#include "Set.hxx" -#include "Fallback.hxx" -#include "TagBuilder.hxx" -#include "util/StringView.hxx" - -#include - -#include - -/** - * Copy all tag items of the specified type. - */ -static bool -CopyTagItem2(TagBuilder &dest, TagType dest_type, - const Tag &src, TagType src_type) -{ - bool found = false; - - for (const auto &item : src) { - if (item.type == src_type) { - dest.AddItem(dest_type, item.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) -{ - ApplyTagWithFallback(type, - std::bind(CopyTagItem2, std::ref(dest), type, - std::cref(src), std::placeholders::_1)); -} - -/** - * Copy all tag items of the types in the mask. - */ -static void -CopyTagMask(TagBuilder &dest, const Tag &src, tag_mask_t mask) -{ - for (unsigned i = 0; i < TAG_NUM_OF_ITEM_TYPES; ++i) - if ((mask & (tag_mask_t(1) << i)) != 0) - CopyTagItem(dest, src, TagType(i)); -} - -void -TagSet::InsertUnique(const Tag &src, TagType type, const char *value, - tag_mask_t group_mask) noexcept -{ - TagBuilder builder; - builder.AddItemUnchecked(type, value); - CopyTagMask(builder, src, group_mask); -#if CLANG_OR_GCC_VERSION(4,8) - emplace(builder.Commit()); -#else - insert(builder.Commit()); -#endif -} - -bool -TagSet::CheckUnique(TagType dest_type, - const Tag &tag, TagType src_type, - tag_mask_t group_mask) noexcept -{ - bool found = false; - - for (const auto &item : tag) { - if (item.type == src_type) { - InsertUnique(tag, dest_type, item.value, group_mask); - found = true; - } - } - - return found; -} - -void -TagSet::InsertUnique(const Tag &tag, - TagType type, tag_mask_t group_mask) noexcept -{ - static_assert(sizeof(group_mask) * 8 >= TAG_NUM_OF_ITEM_TYPES, - "Mask is too small"); - - assert((group_mask & (tag_mask_t(1) << unsigned(type))) == 0); - - if (!ApplyTagWithFallback(type, - std::bind(&TagSet::CheckUnique, this, - type, std::cref(tag), - std::placeholders::_1, - group_mask))) - InsertUnique(tag, type, "", group_mask); -} diff --git a/src/tag/Set.hxx b/src/tag/Set.hxx deleted file mode 100644 index 8e19b7ca2..000000000 --- a/src/tag/Set.hxx +++ /dev/null @@ -1,73 +0,0 @@ -/* - * 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_TAG_SET_HXX -#define MPD_TAG_SET_HXX - -#include "Compiler.h" -#include "Tag.hxx" -#include "Mask.hxx" - -#include - -#include - -/** - * Helper class for #TagSet which compares two #Tag objects. - */ -struct TagLess { - gcc_pure - bool operator()(const Tag &a, const Tag &b) const noexcept { - 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; - } -}; - -/** - * A set of #Tag objects. - */ -class TagSet : public std::set { -public: - void InsertUnique(const Tag &tag, - TagType type, tag_mask_t group_mask) noexcept; - -private: - void InsertUnique(const Tag &src, TagType type, const char *value, - tag_mask_t group_mask) noexcept; - - bool CheckUnique(TagType dest_type, - const Tag &tag, TagType src_type, - tag_mask_t group_mask) noexcept; -}; - -#endif