Merge pull request 'Add an 'All Artists' index including tracks under multiple artists' (#144) from jqln/all-artists into main

Reviewed-on: https://codeberg.org/cool-tech-zone/tangara-fw/pulls/144

lib/libtags/id3v2.c

@@ -68,6 +68,10 @@ v2cb(Tagctx *ctx, char *k, char *v)
 			return 0;
 	}else if(strcmp(k-1, "COM") == 0 || strcmp(k-1, "COMM") == 0){
 		txtcb(ctx, Tcomment, k-1, v);
+	}else if(strcmp(k, "XXX") == 0){
+		k = v;
+		v += strlen(v) + 1;
+		txtcb(ctx, Tunknown, k, v);
 	}else{
 		txtcb(ctx, Tunknown, k-1, v);
 	}

lib/libtags/tags.h

@@ -10,6 +10,7 @@ enum
 {
 	Tunknown = -1,
 	Tartist,
+	Tmultiartists,
 	Talbumartist,
 	Talbum,
 	Ttitle,

lib/libtags/vorbis.c

@@ -11,6 +11,7 @@ static const struct {
 	{"album", Talbum},
 	{"title", Ttitle},
 	{"artist", Tartist},
+	{"artists", Tmultiartists},
 	{"albumartist", Talbumartist},
 	{"tracknumber", Ttrack},
 	{"date", Tdate},

src/tangara/database/database.cpp

@@ -292,7 +292,7 @@ auto Database::setTrackData(TrackId id, const TrackData& data) -> void {
 auto Database::getIndexes() -> std::vector<IndexInfo> {
   // TODO(jacqueline): This probably needs to be async? When we have runtime
   // configurable indexes, they will need to come from somewhere.
-  return {kAllTracks,     kAllAlbums, kAlbumsByArtist,
+  return {kAllTracks,     kAllAlbums, kAllArtists, kAlbumsByArtist,
           kTracksByGenre, kPodcasts,  kAudiobooks};
 }
 

src/tangara/database/database.hpp

@@ -38,7 +38,7 @@
 
 namespace database {
 
-const uint8_t kCurrentDbVersion = 8;
+const uint8_t kCurrentDbVersion = 9;
 
 struct SearchKey;
 class Record;

src/tangara/database/index.cpp

@@ -56,15 +56,22 @@ const IndexInfo kAllAlbums{
     .components = {Tag::kAlbum, Tag::kAlbumOrder},
 };
 
-const IndexInfo kPodcasts{
+const IndexInfo kAllArtists{
     .id = 5,
+    .type = MediaType::kMusic,
+    .name = "All Artists",
+    .components = {Tag::kAllArtists, Tag::kTitle},
+};
+
+const IndexInfo kPodcasts{
+    .id = 6,
     .type = MediaType::kPodcast,
     .name = "Podcasts",
     .components = {Tag::kAlbum, Tag::kTitle},
 };
 
 const IndexInfo kAudiobooks{
-    .id = 6,
+    .id = 7,
     .type = MediaType::kAudiobook,
     .name = "Audiobooks",
     .components = {Tag::kAlbum, Tag::kAlbumOrder},
@@ -109,11 +116,12 @@ class Indexer {
       case Tag::kTitle:
         return titleOrFilename(track_data_, track_tags_);
       case Tag::kArtist:
+      case Tag::kAlbumArtist:
         return "Unknown Artist";
       case Tag::kAlbum:
         return "Unknown Album";
-      case Tag::kAlbumArtist:
+      case Tag::kAllArtists:
-        return track_tags_.artist().value_or("Unknown Artist");
+        return std::pmr::vector<std::pmr::string>{};
       case Tag::kGenres:
         return std::pmr::vector<std::pmr::string>{};
       case Tag::kDisc:

src/tangara/database/index.hpp

@@ -78,6 +78,7 @@ extern const IndexInfo kAlbumsByArtist;
 extern const IndexInfo kTracksByGenre;
 extern const IndexInfo kAllTracks;
 extern const IndexInfo kAllAlbums;
+extern const IndexInfo kAllArtists;
 extern const IndexInfo kPodcasts;
 extern const IndexInfo kAudiobooks;
 

src/tangara/database/tag_parser.cpp

@@ -13,6 +13,7 @@
 #include <iomanip>
 #include <memory>
 #include <mutex>
+#include <optional>
 
 #include "database/track.hpp"
 #include "debug.hpp"
@@ -32,6 +33,8 @@ static auto convert_tag(int tag) -> std::optional<Tag> {
       return Tag::kTitle;
     case Tartist:
       return Tag::kArtist;
+    case Tmultiartists:
+      return Tag::kAllArtists;
     case Talbumartist:
       return Tag::kAlbumArtist;
     case Talbum:
@@ -45,6 +48,29 @@ static auto convert_tag(int tag) -> std::optional<Tag> {
   }
 }
 
+static std::unordered_map<std::string, Tag> sVorbisNameToTag{
+    {"TITLE", Tag::kTitle},
+    {"ALBUM", Tag::kAlbum},
+    {"ARTIST", Tag::kArtist},
+    {"ARTISTS", Tag::kAllArtists},
+    {"ALBUMARTIST", Tag::kAlbumArtist},
+    {"TRACK", Tag::kTrack},
+    {"TRACKNUMBER", Tag::kTrack},
+    {"GENRE", Tag::kGenres},
+    {"DISC", Tag::kDisc},
+    {"DISCNUMBER", Tag::kDisc},
+};
+
+static auto convert_vorbis_tag(const std::string_view name)
+    -> std::optional<Tag> {
+  std::string name_upper{name};
+  std::transform(name.begin(), name.end(), name_upper.begin(), ::toupper);
+  if (sVorbisNameToTag.contains(name_upper)) {
+    return sVorbisNameToTag[name_upper];
+  }
+  return {};
+}
+
 namespace libtags {
 
 struct Aux {
@@ -94,7 +120,14 @@ static void tag(Tagctx* ctx,
                 int size,
                 Tagread f) {
   Aux* aux = reinterpret_cast<Aux*>(ctx->aux);
-  auto tag = convert_tag(t);
+  std::optional<Tag> tag;
+  if (t == Tunknown && k && v) {
+    // Sometimes 'unknown' tags are vorbis comments shoved into a generic tag
+    // name in other containers.
+    tag = convert_vorbis_tag(k);
+  } else {
+    tag = convert_tag(t);
+  }
   if (!tag) {
     return;
   }
@@ -166,17 +199,7 @@ auto TagParserImpl::ReadAndParseTags(std::string_view path)
   return tags;
 }
 
-OggTagParser::OggTagParser() {
+OggTagParser::OggTagParser() {}
-  nameToTag_["TITLE"] = Tag::kTitle;
-  nameToTag_["ALBUM"] = Tag::kAlbum;
-  nameToTag_["ARTIST"] = Tag::kArtist;
-  nameToTag_["ALBUMARTIST"] = Tag::kAlbumArtist;
-  nameToTag_["TRACK"] = Tag::kTrack;
-  nameToTag_["TRACKNUMBER"] = Tag::kTrack;
-  nameToTag_["GENRE"] = Tag::kGenres;
-  nameToTag_["DISC"] = Tag::kDisc;
-  nameToTag_["DISCNUMBER"] = Tag::kDisc;
-}
 
 auto OggTagParser::ReadAndParseTags(std::string_view p)
     -> std::shared_ptr<TrackTags> {
@@ -292,8 +315,9 @@ auto OggTagParser::parseComments(TrackTags& res, std::span<unsigned char> data)
       std::string key_upper{key};
       std::transform(key.begin(), key.end(), key_upper.begin(), ::toupper);
 
-      if (nameToTag_.contains(key_upper) && !val.empty()) {
+      auto tag = convert_vorbis_tag(key);
-        res.set(nameToTag_[key_upper], val);
+      if (tag && !val.empty()) {
+        res.set(*tag, val);
       }
     }
 
@@ -313,14 +337,16 @@ auto GenericTagParser::ReadAndParseTags(std::string_view p)
   std::string path{p};
   libtags::Aux aux;
 
-  // Fail fast if trying to parse a file that doesn't appear to be a supported audio format
+  // Fail fast if trying to parse a file that doesn't appear to be a supported
-  // For context, see: https://codeberg.org/cool-tech-zone/tangara-fw/issues/149
+  // audio format For context, see:
+  // https://codeberg.org/cool-tech-zone/tangara-fw/issues/149
   bool found = false;
   for (const auto& ext : supported_exts) {
     // Case-insensitive file extension check
-    if (std::equal(ext.rbegin(), ext.rend(), path.rbegin(),
+    if (std::equal(ext.rbegin(), ext.rend(), path.rbegin(), [](char a, char b) {
-                   [](char a, char b) { return std::tolower(a) == std::tolower(b); })) {
+          return std::tolower(a) == std::tolower(b);
-      found=true;
+        })) {
+      found = true;
       break;
     }
   }

src/tangara/database/tag_parser.hpp

@@ -47,8 +47,6 @@ class OggTagParser : public ITagParser {
  private:
   auto parseComments(TrackTags&, std::span<unsigned char> data) -> void;
   auto parseLength(std::span<unsigned char> data) -> uint64_t;
-
-  std::unordered_map<std::string, Tag> nameToTag_;
 };
 
 class GenericTagParser : public ITagParser {

src/tangara/database/track.cpp

@@ -20,6 +20,7 @@
 
 namespace database {
 
+static constexpr char kAllArtistDelimiters[] = ";";
 static constexpr char kGenreDelimiters[] = ",;";
 
 auto tagName(Tag t) -> std::string {
@@ -28,6 +29,8 @@ auto tagName(Tag t) -> std::string {
       return "title";
     case Tag::kArtist:
       return "artist";
+    case Tag::kAllArtists:
+      return "all_artists";
     case Tag::kAlbum:
       return "album";
     case Tag::kAlbumArtist:
@@ -91,6 +94,50 @@ auto tagToString(const TagValue& val) -> std::string {
   return "";
 }
 
+/*
+ * Utility for taking a string containing delimited tags, and splitting it out
+ * into a vector of individual tags.
+ */
+auto parseDelimitedTags(const std::string_view s,
+                        const char* delimiters,
+                        std::pmr::vector<std::pmr::string>& out) -> void {
+  out.clear();
+  std::string src = {s.data(), s.size()};
+  char* token = std::strtok(src.data(), delimiters);
+
+  auto trim_and_add = [&](std::string_view s) {
+    std::string copy = {s.data(), s.size()};
+
+    // Trim the left
+    copy.erase(copy.begin(),
+               std::find_if(copy.begin(), copy.end(), [](unsigned char ch) {
+                 return !std::isspace(ch);
+               }));
+
+    // Trim the right
+    copy.erase(std::find_if(copy.rbegin(), copy.rend(),
+                            [](unsigned char ch) { return !std::isspace(ch); })
+                   .base(),
+               copy.end());
+
+    // Ignore empty strings.
+    if (!copy.empty()) {
+      out.push_back({copy.data(), copy.size()});
+    }
+  };
+
+  if (token == NULL) {
+    // No delimiters found in the input. Treat this as a single result.
+    trim_and_add(s);
+  } else {
+    while (token != NULL) {
+      // Add tokens until no more delimiters found.
+      trim_and_add(token);
+      token = std::strtok(NULL, delimiters);
+    }
+  }
+}
+
 auto TrackTags::create() -> std::shared_ptr<TrackTags> {
   return std::allocate_shared<TrackTags,
                               std::pmr::polymorphic_allocator<TrackTags>>(
@@ -108,21 +155,23 @@ auto valueOrMonostate(std::optional<T> t) -> TagValue {
 auto TrackTags::get(Tag t) const -> TagValue {
   switch (t) {
     case Tag::kTitle:
-      return valueOrMonostate(title_);
+      return valueOrMonostate(title());
     case Tag::kArtist:
-      return valueOrMonostate(artist_);
+      return valueOrMonostate(artist());
+    case Tag::kAllArtists:
+      return allArtists();
     case Tag::kAlbum:
-      return valueOrMonostate(album_);
+      return valueOrMonostate(album());
     case Tag::kAlbumArtist:
-      return valueOrMonostate(album_artist_);
+      return valueOrMonostate(albumArtist());
     case Tag::kDisc:
-      return valueOrMonostate(disc_);
+      return valueOrMonostate(disc());
     case Tag::kTrack:
-      return valueOrMonostate(track_);
+      return valueOrMonostate(track());
     case Tag::kAlbumOrder:
       return albumOrder();
     case Tag::kGenres:
-      return genres_;
+      return genres();
   }
   return std::monostate{};
 }
@@ -135,6 +184,9 @@ auto TrackTags::set(Tag t, std::string_view v) -> void {
     case Tag::kArtist:
       artist(v);
       break;
+    case Tag::kAllArtists:
+      allArtists(v);
+      break;
     case Tag::kAlbum:
       album(v);
       break;
@@ -165,6 +217,7 @@ auto TrackTags::allPresent() const -> std::vector<Tag> {
   };
   add_if_present(Tag::kTitle, title_);
   add_if_present(Tag::kArtist, artist_);
+  add_if_present(Tag::kAllArtists, !allArtists_.empty());
   add_if_present(Tag::kAlbum, album_);
   add_if_present(Tag::kAlbumArtist, album_artist_);
   add_if_present(Tag::kDisc, disc_);
@@ -187,6 +240,16 @@ auto TrackTags::artist() const -> const std::optional<std::pmr::string>& {
 
 auto TrackTags::artist(std::string_view s) -> void {
   artist_ = s;
+  maybeSynthesizeAllArtists();
+}
+
+auto TrackTags::allArtists() const -> std::span<const std::pmr::string> {
+  return allArtists_;
+}
+
+auto TrackTags::allArtists(const std::string_view s) -> void {
+  parseDelimitedTags(s, kAllArtistDelimiters, allArtists_);
+  maybeSynthesizeAllArtists();
 }
 
 auto TrackTags::album() const -> const std::optional<std::pmr::string>& {
@@ -198,6 +261,9 @@ auto TrackTags::album(std::string_view s) -> void {
 }
 
 auto TrackTags::albumArtist() const -> const std::optional<std::pmr::string>& {
+  if (!album_artist_) {
+    return artist_;
+  }
   return album_artist_;
 }
 
@@ -230,41 +296,7 @@ auto TrackTags::genres() const -> std::span<const std::pmr::string> {
 }
 
 auto TrackTags::genres(const std::string_view s) -> void {
-  genres_.clear();
+  parseDelimitedTags(s, kGenreDelimiters, genres_);
-  std::string src = {s.data(), s.size()};
-  char* token = std::strtok(src.data(), kGenreDelimiters);
-
-  auto trim_and_add = [this](std::string_view s) {
-    std::string copy = {s.data(), s.size()};
-
-    // Trim the left
-    copy.erase(copy.begin(),
-               std::find_if(copy.begin(), copy.end(), [](unsigned char ch) {
-                 return !std::isspace(ch);
-               }));
-
-    // Trim the right
-    copy.erase(std::find_if(copy.rbegin(), copy.rend(),
-                            [](unsigned char ch) { return !std::isspace(ch); })
-                   .base(),
-               copy.end());
-
-    // Ignore empty strings.
-    if (!copy.empty()) {
-      genres_.push_back({copy.data(), copy.size()});
-    }
-  };
-
-  if (token == NULL) {
-    // No delimiters found in the input. Treat this as a single genre.
-    trim_and_add(s);
-  } else {
-    while (token != NULL) {
-      // Add tokens until no more delimiters found.
-      trim_and_add(token);
-      token = std::strtok(NULL, kGenreDelimiters);
-    }
-  }
 }
 
 /*
@@ -293,6 +325,17 @@ auto TrackTags::Hash() const -> uint64_t {
   return komihash_stream_final(&stream);
 }
 
+/*
+ * Adds the current 'artist' tag to 'allArtists' if needed. Many tracks lack a
+ * fine-grained 'ARTISTS=' tag (or equivalent), but pushing down this nuance to
+ * consumers of TrackTags adds a lot of complexity.
+ */
+auto TrackTags::maybeSynthesizeAllArtists() -> void {
+  if (allArtists_.empty() && artist_) {
+    allArtists_.push_back(*artist_);
+  }
+}
+
 auto database::TrackData::clone() const -> std::shared_ptr<TrackData> {
   auto data = std::make_shared<TrackData>();
   data->id = id;

src/tangara/database/track.hpp

@@ -73,6 +73,7 @@ enum class Tag {
   kTrack = 5,
   kAlbumOrder = 6,
   kGenres = 7,
+  kAllArtists = 8,
 };
 
 using TagValue = std::variant<std::monostate,
@@ -114,6 +115,9 @@ class TrackTags {
   auto artist() const -> const std::optional<std::pmr::string>&;
   auto artist(std::string_view) -> void;
 
+  auto allArtists() const -> std::span<const std::pmr::string>;
+  auto allArtists(const std::string_view) -> void;
+
   auto album() const -> const std::optional<std::pmr::string>&;
   auto album(std::string_view) -> void;
 
@@ -140,10 +144,13 @@ class TrackTags {
   auto Hash() const -> uint64_t;
 
  private:
+  auto maybeSynthesizeAllArtists() -> void;
+
   Container encoding_;
 
   std::optional<std::pmr::string> title_;
   std::optional<std::pmr::string> artist_;
+  std::pmr::vector<std::pmr::string> allArtists_;
   std::optional<std::pmr::string> album_;
   std::optional<std::pmr::string> album_artist_;
   std::optional<uint8_t> disc_;