Support more datatypes in track tags

src/database/database.cpp

@@ -13,6 +13,7 @@
 #include <cstdint>
 #include <functional>
 #include <iomanip>
+#include <iostream>
 #include <memory>
 #include <optional>
 #include <sstream>
@@ -53,7 +54,7 @@ static SingletonEnv<leveldb::EspEnv> sEnv;
 static const char kDbPath[] = "/.tangara-db";
 
 static const char kKeyDbVersion[] = "schema_version";
-static const uint8_t kCurrentDbVersion = 3;
+static const uint8_t kCurrentDbVersion = 4;
 
 static const char kKeyCustom[] = "U\0";
 static const char kKeyCollator[] = "collator";
@@ -129,13 +130,11 @@ auto Database::Open(IFileGatherer& gatherer,
   }
 
   if (!leveldb::sBackgroundThread) {
-    leveldb::sBackgroundThread = &bg_worker;
+    leveldb::sBackgroundThread = tasks::Worker::Start<tasks::Type::kDatabase>();
   }
 
-  std::shared_ptr<tasks::Worker> worker(
+  return bg_worker
-      tasks::Worker::Start<tasks::Type::kDatabase>());
+      .Dispatch<cpp::result<Database*, DatabaseError>>(
-  return worker
-      ->Dispatch<cpp::result<Database*, DatabaseError>>(
           [&]() -> cpp::result<Database*, DatabaseError> {
             leveldb::DB* db;
             std::unique_ptr<leveldb::Cache> cache{
@@ -167,8 +166,8 @@ auto Database::Open(IFileGatherer& gatherer,
             }
 
             ESP_LOGI(kTag, "Database opened successfully");
-            return new Database(db, cache.release(), gatherer, parser, collator,
+            return new Database(db, cache.release(), gatherer, parser,
-                                worker);
+                                collator);
           })
       .get();
 }
@@ -183,11 +182,9 @@ Database::Database(leveldb::DB* db,
                    leveldb::Cache* cache,
                    IFileGatherer& file_gatherer,
                    ITagParser& tag_parser,
-                   locale::ICollator& collator,
+                   locale::ICollator& collator)
-                   std::shared_ptr<tasks::Worker> worker)
     : db_(db),
       cache_(cache),
-      worker_task_(worker),
       file_gatherer_(file_gatherer),
       tag_parser_(tag_parser),
       collator_(collator) {}
@@ -412,9 +409,9 @@ auto Database::updateIndexes() -> void {
       ESP_LOGW(kTag, "tag hash collision for %s and %s",
                existing_data->filepath.c_str(), path.c_str());
       ESP_LOGI(kTag, "hash components: %s, %s, %s",
-               tags->at(Tag::kTitle).value_or("no title").c_str(),
+               tags->title().value_or("no title").c_str(),
-               tags->at(Tag::kArtist).value_or("no artist").c_str(),
+               tags->artist().value_or("no artist").c_str(),
-               tags->at(Tag::kAlbum).value_or("no album").c_str());
+               tags->album().value_or("no album").c_str());
     }
   });
   events::Ui().Dispatch(event::UpdateFinished{});
@@ -533,11 +530,11 @@ auto Database::dbIngestTagHashes(const TrackTags& tags,
                                  std::pmr::unordered_map<Tag, uint64_t>& out)
     -> void {
   leveldb::WriteBatch batch{};
-  for (auto& entry : tags.tags()) {
+  for (const auto& tag : tags.allPresent()) {
-    auto hash =
+    auto val = tags.get(tag);
-        komihash_stream_oneshot(entry.second.data(), entry.second.size(), 0);
+    auto hash = tagHash(val);
-    batch.Put(EncodeTagHashKey(hash), entry.second.c_str());
+    batch.Put(EncodeTagHashKey(hash), tagToString(val));
-    out[entry.first] = hash;
+    out[tag] = hash;
   }
   db_->Write(leveldb::WriteOptions{}, &batch);
 }

src/database/include/database.hpp

@@ -87,8 +87,6 @@ class Database {
   leveldb::DB* db_;
   leveldb::Cache* cache_;
 
-  std::shared_ptr<tasks::Worker> worker_task_;
-
   // Not owned.
   IFileGatherer& file_gatherer_;
   ITagParser& tag_parser_;
@@ -98,8 +96,7 @@ class Database {
            leveldb::Cache* cache,
            IFileGatherer& file_gatherer,
            ITagParser& tag_parser,
-           locale::ICollator& collator,
+           locale::ICollator& collator);
-           std::shared_ptr<tasks::Worker> worker);
 
   auto dbMintNewTrackId() -> TrackId;
 

src/database/include/index.hpp

@@ -62,7 +62,7 @@ struct IndexKey {
 };
 
 auto Index(locale::ICollator&, const IndexInfo&, const Track&)
-    -> std::vector<std::pair<IndexKey, std::pmr::string>>;
+    -> std::vector<std::pair<IndexKey, std::string>>;
 
 auto ExpandHeader(const IndexKey::Header&,
                   const std::optional<std::pmr::string>&) -> IndexKey::Header;

src/database/include/track.hpp

@@ -15,6 +15,7 @@
 #include <string>
 #include <unordered_map>
 #include <utility>
+#include <variant>
 
 #include "leveldb/db.h"
 #include "memory_resource.hpp"
@@ -51,12 +52,21 @@ enum class Tag {
   kTitle = 0,
   kArtist = 1,
   kAlbum = 2,
-  kAlbumTrack = 3,
+  kAlbumArtist = 3,
-  kGenre = 4,
+  kDisc = 4,
-  kDuration = 5,
+  kTrack = 5,
+  kAlbumOrder = 6,
+  kGenres = 7,
 };
 
-auto TagToString(Tag t) -> std::string;
+using TagValue = std::variant<std::monostate,
+                              std::pmr::string,
+                              uint32_t,
+                              cpp::span<const std::pmr::string>>;
+
+auto tagName(Tag) -> std::string;
+auto tagHash(const TagValue&) -> uint64_t;
+auto tagToString(const TagValue&) -> std::string;
 
 /*
  * Owning container for tag-related track metadata that was extracted from a
@@ -65,29 +75,43 @@ auto TagToString(Tag t) -> std::string;
 class TrackTags {
  public:
   TrackTags()
-      : encoding_(Container::kUnsupported), tags_(&memory::kSpiRamResource) {}
+      : encoding_(Container::kUnsupported), genres_(&memory::kSpiRamResource) {}
 
   TrackTags(const TrackTags& other) = delete;
   TrackTags& operator=(TrackTags& other) = delete;
 
   bool operator==(const TrackTags&) const = default;
 
+  auto get(Tag) const -> TagValue;
+  auto set(Tag, std::string_view) -> void;
+
+  auto allPresent() const -> std::vector<Tag>;
+
   auto encoding() const -> Container { return encoding_; };
   auto encoding(Container e) -> void { encoding_ = e; };
 
-  std::optional<int> channels;
+  auto title() const -> const std::optional<std::pmr::string>&;
-  std::optional<int> sample_rate;
+  auto title(std::string_view) -> void;
-  std::optional<int> bits_per_sample;
+
+  auto artist() const -> const std::optional<std::pmr::string>&;
+  auto artist(std::string_view) -> void;
 
-  std::optional<int> duration;
+  auto album() const -> const std::optional<std::pmr::string>&;
+  auto album(std::string_view) -> void;
 
-  auto set(const Tag& key, const std::pmr::string& val) -> void;
+  auto albumArtist() const -> const std::optional<std::pmr::string>&;
-  auto at(const Tag& key) const -> std::optional<std::pmr::string>;
+  auto albumArtist(std::string_view) -> void;
-  auto operator[](const Tag& key) const -> std::optional<std::pmr::string>;
 
-  auto tags() const -> const std::pmr::unordered_map<Tag, std::pmr::string>& {
+  auto disc() const -> const std::optional<uint8_t>&;
-    return tags_;
+  auto disc(const std::string_view) -> void;
-  }
+
+  auto track() const -> const std::optional<uint16_t>&;
+  auto track(const std::string_view) -> void;
+
+  auto albumOrder() const -> uint32_t;
+
+  auto genres() const -> cpp::span<const std::pmr::string>;
+  auto genres(const std::string_view) -> void;
 
   /*
    * Returns a hash of the 'identifying' tags of this track. That is, a hash
@@ -99,7 +123,14 @@ class TrackTags {
 
  private:
   Container encoding_;
-  std::pmr::unordered_map<Tag, std::pmr::string> tags_;
+
+  std::optional<std::pmr::string> title_;
+  std::optional<std::pmr::string> artist_;
+  std::optional<std::pmr::string> album_;
+  std::optional<std::pmr::string> album_artist_;
+  std::optional<uint8_t> disc_;
+  std::optional<uint16_t> track_;
+  std::pmr::vector<std::pmr::string> genres_;
 };
 
 /*

src/database/index.cpp

@@ -5,30 +5,39 @@
  */
 
 #include "index.hpp"
+#include <sys/_stdint.h>
 
 #include <cstdint>
+#include <iomanip>
+#include <iostream>
 #include <sstream>
+#include <string>
 #include <variant>
+#include <vector>
 
 #include "collation.hpp"
+#include "cppbor.h"
 #include "esp_log.h"
 #include "komihash.h"
 #include "leveldb/write_batch.h"
 
 #include "records.hpp"
+#include "track.hpp"
 
 namespace database {
 
+[[maybe_unused]] static const char* kTag = "index";
+
 const IndexInfo kAlbumsByArtist{
     .id = 1,
     .name = "Albums by Artist",
-    .components = {Tag::kArtist, Tag::kAlbum, Tag::kAlbumTrack},
+    .components = {Tag::kAlbumArtist, Tag::kAlbum, Tag::kAlbumOrder},
 };
 
 const IndexInfo kTracksByGenre{
     .id = 2,
     .name = "Tracks by Genre",
-    .components = {Tag::kGenre, Tag::kTitle},
+    .components = {Tag::kGenres, Tag::kTitle},
 };
 
 const IndexInfo kAllTracks{
@@ -40,71 +49,144 @@ const IndexInfo kAllTracks{
 const IndexInfo kAllAlbums{
     .id = 4,
     .name = "All Albums",
-    .components = {Tag::kAlbum, Tag::kAlbumTrack},
+    .components = {Tag::kAlbum, Tag::kAlbumOrder},
 };
 
-static auto missing_component_text(const Track& track, Tag tag)
+class Indexer {
-    -> std::optional<std::pmr::string> {
+ public:
-  switch (tag) {
+  Indexer(locale::ICollator& collator, const Track& t, const IndexInfo& idx)
-    case Tag::kArtist:
+      : collator_(collator), track_(t), index_(idx) {}
-      return "Unknown Artist";
+
-    case Tag::kAlbum:
+  auto index() -> std::vector<std::pair<IndexKey, std::string>>;
-      return "Unknown Album";
+
-    case Tag::kGenre:
+ private:
-      return "Unknown Genre";
+  auto handleLevel(const IndexKey::Header& header,
-    case Tag::kTitle:
+                   cpp::span<const Tag> components) -> void;
-      return track.TitleOrFilename();
+
-    case Tag::kAlbumTrack:
+  auto handleItem(const IndexKey::Header& header,
-      return "0000";
+                  std::variant<std::pmr::string, uint32_t> item,
-    case Tag::kDuration:
+                  cpp::span<const Tag> components) -> void;
-    default:
+
-      return {};
+  auto missing_value(Tag tag) -> TagValue {
+    switch (tag) {
+      case Tag::kTitle:
+        return track_.TitleOrFilename();
+      case Tag::kArtist:
+        return "Unknown Artist";
+      case Tag::kAlbum:
+        return "Unknown Album";
+      case Tag::kAlbumArtist:
+        return track_.tags().artist().value_or("Unknown Artist");
+        return "Unknown Album";
+      case Tag::kGenres:
+        return std::pmr::vector<std::pmr::string>{};
+      case Tag::kDisc:
+        return 0u;
+      case Tag::kTrack:
+        return 0u;
+      case Tag::kAlbumOrder:
+        return 0u;
+    }
+    return std::monostate{};
   }
+
+  locale::ICollator& collator_;
+  const Track& track_;
+  const IndexInfo index_;
+
+  std::vector<std::pair<IndexKey, std::string>> out_;
+};
+
+auto Indexer::index() -> std::vector<std::pair<IndexKey, std::string>> {
+  out_.clear();
+
+  IndexKey::Header root_header{
+      .id = index_.id,
+      .depth = 0,
+      .components_hash = 0,
+  };
+  handleLevel(root_header, index_.components);
+
+  return out_;
 }
 
-auto Index(locale::ICollator& collator, const IndexInfo& info, const Track& t)
+auto Indexer::handleLevel(const IndexKey::Header& header,
-    -> std::vector<std::pair<IndexKey, std::pmr::string>> {
+                          cpp::span<const Tag> components) -> void {
-  std::vector<std::pair<IndexKey, std::pmr::string>> out;
+  Tag component = components.front();
-  IndexKey key{
+  TagValue value = track_.tags().get(component);
-      .header{
+  if (std::holds_alternative<std::monostate>(value)) {
-          .id = info.id,
+    value = missing_value(component);
-          .depth = 0,
+  }
-          .components_hash = 0,
+
+  std::visit(
+      [&](auto&& arg) {
+        using T = std::decay_t<decltype(arg)>;
+        if constexpr (std::is_same_v<T, std::monostate>) {
+          ESP_LOGW(kTag, "dropping component without value: %s",
+                   tagName(components.front()).c_str());
+        } else if constexpr (std::is_same_v<T, std::pmr::string>) {
+          handleItem(header, arg, components);
+        } else if constexpr (std::is_same_v<T, uint32_t>) {
+          handleItem(header, arg, components);
+        } else if constexpr (std::is_same_v<
+                                 T, cpp::span<const std::pmr::string>>) {
+          for (const auto& i : arg) {
+            handleItem(header, i, components);
+          }
+        }
       },
+      value);
+}
+
+auto Indexer::handleItem(const IndexKey::Header& header,
+                         std::variant<std::pmr::string, uint32_t> item,
+                         cpp::span<const Tag> components) -> void {
+  IndexKey key{
+      .header = header,
       .item = {},
       .track = {},
   };
+  std::string value;
+
+  std::string item_text;
+  std::visit(
+      [&](auto&& arg) {
+        using T = std::decay_t<decltype(arg)>;
+        if constexpr (std::is_same_v<T, std::pmr::string>) {
+          value = {arg.data(), arg.size()};
+          auto xfrm = collator_.Transform(value);
+          key.item = {xfrm.data(), xfrm.size()};
+        } else if constexpr (std::is_same_v<T, uint32_t>) {
+          value = std::to_string(arg);
+          // FIXME: this sucks lol. we should just write the number directly,
+          // LSB-first, but then we need to be able to parse it back properly.
+          std::ostringstream str;
+          str << std::setw(8) << std::setfill('0') << arg;
+          std::string encoded = str.str();
+          key.item = {encoded.data(), encoded.size()};
+        }
+      },
+      item);
 
-  for (std::uint8_t i = 0; i < info.components.size(); i++) {
+  std::optional<IndexKey::Header> next_level;
-    // Fill in the text for this depth.
+  if (components.size() == 1) {
-    auto text = t.tags().at(info.components.at(i));
+    value = track_.TitleOrFilename();
-    std::pmr::string value;
+    key.track = track_.data().id;
-    if (text) {
+  } else {
-      std::pmr::string orig = *text;
+    next_level = ExpandHeader(key.header, key.item);
-      auto xfrm = collator.Transform({orig.data(), orig.size()});
+  }
-      key.item = {xfrm.data(), xfrm.size()};
-      value = *text;
-    } else {
-      key.item = {};
-      value = missing_component_text(t, info.components.at(i)).value_or("");
-    }
-
-    // If this is the last component, then we should also fill in the track id
-    // and title.
-    if (i == info.components.size() - 1) {
-      key.track = t.data().id;
-      value = t.TitleOrFilename();
-    }
 
-    out.push_back(std::make_pair(key, value));
+  out_.emplace_back(key, value);
 
-    // If there are more components after this, then we need to finish by
+  if (next_level) {
-    // narrowing the header with the current title.
+    handleLevel(*next_level, components.subspan(1));
-    if (i < info.components.size() - 1) {
-      key.header = ExpandHeader(key.header, key.item);
-    }
   }
-  return out;
+}
+
+auto Index(locale::ICollator& c, const IndexInfo& i, const Track& t)
+    -> std::vector<std::pair<IndexKey, std::string>> {
+  Indexer indexer{c, t, i};
+  return indexer.index();
 }
 
 auto ExpandHeader(const IndexKey::Header& header,

src/database/records.cpp

@@ -11,6 +11,7 @@
 
 #include <functional>
 #include <iomanip>
+#include <iostream>
 #include <memory_resource>
 #include <sstream>
 #include <string>

src/database/tag_parser.cpp

@@ -21,26 +21,16 @@
 
 namespace database {
 
-const static std::array<std::pair<const char*, Tag>, 5> kVorbisIdToTag = {{
+const static std::array<std::pair<const char*, Tag>, 7> kVorbisIdToTag = {{
     {"TITLE", Tag::kTitle},
     {"ARTIST", Tag::kArtist},
     {"ALBUM", Tag::kAlbum},
-    {"TRACKNUMBER", Tag::kAlbumTrack},
+    {"ALBUMARTIST", Tag::kAlbumArtist},
-    {"GENRE", Tag::kGenre},
+    {"DISCNUMBER", Tag::kDisc},
+    {"TRACKNUMBER", Tag::kTrack},
+    {"GENRE", Tag::kGenres},
 }};
 
-static auto convert_track_number(int number) -> std::pmr::string {
-  std::ostringstream oss;
-  oss << std::setw(4) << std::setfill('0') << number;
-  return std::pmr::string(oss.str(), &memory::kSpiRamResource);
-}
-
-static auto convert_track_number(const std::pmr::string& raw)
-    -> std::pmr::string {
-  uint32_t as_int = std::atoi(raw.c_str());
-  return convert_track_number(as_int);
-}
-
 static auto convert_tag(int tag) -> std::optional<Tag> {
   switch (tag) {
     case Ttitle:
@@ -50,9 +40,9 @@ static auto convert_tag(int tag) -> std::optional<Tag> {
     case Talbum:
       return Tag::kAlbum;
     case Ttrack:
-      return Tag::kAlbumTrack;
+      return Tag::kTrack;
     case Tgenre:
-      return Tag::kGenre;
+      return Tag::kGenres;
     default:
       return {};
   }
@@ -115,8 +105,6 @@ static void tag(Tagctx* ctx,
   if (value.empty()) {
     return;
   }
-  if (*tag == Tag::kAlbumTrack) {
-  }
   aux->tags->set(*tag, value);
 }
 
@@ -161,19 +149,14 @@ auto TagParserImpl::ReadAndParseTags(const std::string& path)
   // There wasn't a track number found in the track's tags. Try to synthesize
   // one from the filename, which will sometimes have a track number at the
   // start.
-  if (!tags->at(Tag::kAlbumTrack)) {
+  if (!tags->track()) {
     auto slash_pos = path.find_last_of("/");
     if (slash_pos != std::string::npos && path.size() - slash_pos > 1) {
       std::string trunc = path.substr(slash_pos + 1);
-      tags->set(Tag::kAlbumTrack, {trunc.data(), trunc.size()});
+      tags->track({trunc.data(), trunc.size()});
     }
   }
 
-  // Normalise track numbers; they're usually treated as strings, but we would
-  // like to sort them lexicographically.
-  tags->set(Tag::kAlbumTrack,
-            convert_track_number(tags->at(Tag::kAlbumTrack).value_or("0")));
-
   {
     std::lock_guard<std::mutex> lock{cache_mutex_};
     cache_.Put({path.data(), path.size(), &memory::kSpiRamResource}, tags);
@@ -241,18 +224,6 @@ auto GenericTagParser::ReadAndParseTags(const std::string& path)
       out->encoding(Container::kUnsupported);
   }
 
-  if (ctx.channels > 0) {
-    out->channels = ctx.channels;
-  }
-  if (ctx.samplerate > 0) {
-    out->sample_rate = ctx.samplerate;
-  }
-  if (ctx.bitrate > 0) {
-    out->bits_per_sample = ctx.bitrate;
-  }
-  if (ctx.duration > 0) {
-    out->duration = ctx.duration;
-  }
   return out;
 }
 

src/database/track.cpp

@@ -6,14 +6,22 @@
 
 #include "track.hpp"
 
-#include <komihash.h>
+#include <iomanip>
-#include <sys/_stdint.h>
+#include <iostream>
+#include <sstream>
+#include <string>
+
+#include "esp_log.h"
+#include "komihash.h"
 
 #include "memory_resource.hpp"
+#include "span.hpp"
 
 namespace database {
 
-auto TagToString(Tag t) -> std::string {
+static constexpr char kGenreDelimiters[] = ",;";
+
+auto tagName(Tag t) -> std::string {
   switch (t) {
     case Tag::kTitle:
       return "title";
@@ -21,42 +29,241 @@ auto TagToString(Tag t) -> std::string {
       return "artist";
     case Tag::kAlbum:
       return "album";
-    case Tag::kAlbumTrack:
+    case Tag::kAlbumArtist:
-      return "album_track";
+      return "album_artist";
-    case Tag::kGenre:
+    case Tag::kDisc:
+      return "disc";
+    case Tag::kTrack:
+      return "track";
+    case Tag::kAlbumOrder:
+      return "album_order";
+    case Tag::kGenres:
       return "genre";
-    case Tag::kDuration:
-      return "duration";
-    default:
-      return "";
   }
+  return "";
+}
+
+auto tagHash(const TagValue& t) -> uint64_t {
+  return std::visit(
+      [&](auto&& arg) {
+        using T = std::decay_t<decltype(arg)>;
+        if constexpr (std::is_same_v<T, std::monostate>) {
+          return static_cast<uint64_t>(0);
+        } else if constexpr (std::is_same_v<T, std::pmr::string>) {
+          return komihash(arg.data(), arg.size(), 0);
+        } else if constexpr (std::is_same_v<T, uint32_t>) {
+          return komihash(&arg, sizeof(arg), 0);
+        } else if constexpr (std::is_same_v<
+                                 T, cpp::span<const std::pmr::string>>) {
+          komihash_stream_t hash;
+          komihash_stream_init(&hash, 0);
+          for (const auto& i : arg) {
+            komihash_stream_update(&hash, i.data(), i.size());
+          }
+          return komihash_stream_final(&hash);
+        }
+      },
+      t);
+  return 0;
 }
 
-auto TrackTags::set(const Tag& key, const std::pmr::string& val) -> void {
+auto tagToString(const TagValue& val) -> std::string {
-  tags_[key] = val;
+  return std::visit(
+      [&](auto&& arg) -> std::string {
+        using T = std::decay_t<decltype(arg)>;
+        if constexpr (std::is_same_v<T, std::monostate>) {
+          return "";
+        } else if constexpr (std::is_same_v<T, std::pmr::string>) {
+          return {arg.data(), arg.size()};
+        } else if constexpr (std::is_same_v<T, uint32_t>) {
+          return std::to_string(arg);
+        } else if constexpr (std::is_same_v<
+                                 T, cpp::span<const std::pmr::string>>) {
+          std::ostringstream builder{};
+          for (const auto& str : arg) {
+            builder << std::string{str.data(), str.size()} << ",";
+          }
+          return builder.str();
+        }
+      },
+      val);
+  return "";
+}
+
+template <typename T>
+auto valueOrMonostate(std::optional<T> t) -> TagValue {
+  if (t) {
+    return *t;
+  }
+  return std::monostate{};
 }
 
-auto TrackTags::at(const Tag& key) const -> std::optional<std::pmr::string> {
+auto TrackTags::get(Tag t) const -> TagValue {
-  if (tags_.contains(key)) {
+  switch (t) {
-    return tags_.at(key);
+    case Tag::kTitle:
+      return valueOrMonostate(title_);
+    case Tag::kArtist:
+      return valueOrMonostate(artist_);
+    case Tag::kAlbum:
+      return valueOrMonostate(album_);
+    case Tag::kAlbumArtist:
+      return valueOrMonostate(album_artist_);
+    case Tag::kDisc:
+      return valueOrMonostate(disc_);
+    case Tag::kTrack:
+      return valueOrMonostate(track_);
+    case Tag::kAlbumOrder:
+      return albumOrder();
+    case Tag::kGenres:
+      return genres_;
   }
-  return {};
+  return std::monostate{};
+}
+
+auto TrackTags::set(Tag t, std::string_view v) -> void {
+  switch (t) {
+    case Tag::kTitle:
+      title(v);
+      break;
+    case Tag::kArtist:
+      artist(v);
+      break;
+    case Tag::kAlbum:
+      album(v);
+      break;
+    case Tag::kAlbumArtist:
+      albumArtist(v);
+      break;
+    case Tag::kDisc:
+      disc(v);
+      break;
+    case Tag::kTrack:
+      track(v);
+      break;
+    case Tag::kAlbumOrder:
+      // This tag is derices from disc and track, and so it can't be set.
+      break;
+    case Tag::kGenres:
+      genres(v);
+      break;
+  }
+}
+
+auto TrackTags::allPresent() const -> std::vector<Tag> {
+  std::vector<Tag> out;
+  auto add_if_present = [&](Tag t, auto opt) {
+    if (opt) {
+      out.push_back(t);
+    }
+  };
+  add_if_present(Tag::kTitle, title_);
+  add_if_present(Tag::kArtist, artist_);
+  add_if_present(Tag::kAlbum, album_);
+  add_if_present(Tag::kAlbumArtist, album_artist_);
+  add_if_present(Tag::kDisc, disc_);
+  add_if_present(Tag::kTrack, track_);
+  add_if_present(Tag::kGenres, !genres_.empty());
+  return out;
+}
+
+auto TrackTags::title() const -> const std::optional<std::pmr::string>& {
+  return title_;
+}
+
+auto TrackTags::title(std::string_view s) -> void {
+  title_ = s;
+}
+
+auto TrackTags::artist() const -> const std::optional<std::pmr::string>& {
+  return artist_;
+}
+
+auto TrackTags::artist(std::string_view s) -> void {
+  artist_ = s;
+}
+
+auto TrackTags::album() const -> const std::optional<std::pmr::string>& {
+  return album_;
+}
+
+auto TrackTags::album(std::string_view s) -> void {
+  album_ = s;
+}
+
+auto TrackTags::albumArtist() const -> const std::optional<std::pmr::string>& {
+  return album_artist_;
 }
 
-auto TrackTags::operator[](const Tag& key) const
+auto TrackTags::albumArtist(std::string_view s) -> void {
-    -> std::optional<std::pmr::string> {
+  album_artist_ = s;
-  return at(key);
 }
 
-/* Helper function to update a komihash stream with a std::pmr::string. */
+auto TrackTags::disc() const -> const std::optional<uint8_t>& {
-auto HashString(komihash_stream_t* stream, const std::pmr::string& str)
+  return disc_;
-    -> void {
+}
-  komihash_stream_update(stream, str.c_str(), str.length());
+
+auto TrackTags::disc(const std::string_view s) -> void {
+  disc_ = std::stoi({s.data(), s.size()});
+}
+
+auto TrackTags::track() const -> const std::optional<uint16_t>& {
+  return track_;
+}
+
+auto TrackTags::track(const std::string_view s) -> void {
+  track_ = std::stoi({s.data(), s.size()});
+}
+
+auto TrackTags::albumOrder() const -> uint32_t {
+  return (disc_.value_or(0) << 16) | track_.value_or(0);
+}
+
+auto TrackTags::genres() const -> cpp::span<const std::pmr::string> {
+  return genres_;
+}
+
+auto TrackTags::genres(const std::string_view s) -> void {
+  genres_.clear();
+  std::string src = {s.data(), s.size()};
+  char* token = std::strtok(src.data(), kGenreDelimiters);
+
+  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()) {
+      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);
+    }
+  }
 }
 
 /*
- * Uses a komihash stream to incrementally hash tags. This lowers the function's
+ * Uses a komihash stream to incrementally hash tags. This lowers the
- * memory footprint a little so that it's safe to call from any stack.
+ * function's memory footprint a little so that it's safe to call from any
+ * stack.
  */
 auto TrackTags::Hash() const -> uint64_t {
   // TODO(jacqueline): this function doesn't work very well for tracks with no
@@ -64,16 +271,23 @@ auto TrackTags::Hash() const -> uint64_t {
   komihash_stream_t stream;
   komihash_stream_init(&stream, 0);
 
-  HashString(&stream, at(Tag::kTitle).value_or(""));
+  auto add = [&](const uint64_t& h) {
-  HashString(&stream, at(Tag::kArtist).value_or(""));
+    komihash_stream_update(&stream, &h, sizeof(h));
-  HashString(&stream, at(Tag::kAlbum).value_or(""));
+  };
-  HashString(&stream, at(Tag::kAlbumTrack).value_or(""));
+
+  add(tagHash(get(Tag::kTitle)));
+  add(tagHash(get(Tag::kArtist)));
+  add(tagHash(get(Tag::kAlbum)));
+  add(tagHash(get(Tag::kAlbumArtist)));
+
+  // TODO: Should we be including this?
+  add(tagHash(get(Tag::kAlbumOrder)));
 
   return komihash_stream_final(&stream);
 }
 
 auto Track::TitleOrFilename() const -> std::pmr::string {
-  auto title = tags().at(Tag::kTitle);
+  auto title = tags().title();
   if (title) {
     return *title;
   }

src/lua/property.cpp

@@ -5,6 +5,7 @@
  */
 
 #include "property.hpp"
+#include <sys/_stdint.h>
 
 #include <memory>
 #include <string>
@@ -160,6 +161,29 @@ Property::Property(const LuaValue& val,
                    std::function<bool(const LuaValue& val)> cb)
     : value_(val), cb_(cb) {}
 
+static auto pushTagValue(lua_State* L, const database::TagValue& val) -> void {
+  std::visit(
+      [&](auto&& arg) {
+        using T = std::decay_t<decltype(arg)>;
+        if constexpr (std::is_same_v<T, std::pmr::string>) {
+          lua_pushlstring(L, arg.data(), arg.size());
+        } else if constexpr (std::is_same_v<
+                                 T, cpp::span<const std::pmr::string>>) {
+          lua_createtable(L, 0, arg.size());
+          for (const auto& i : arg) {
+            lua_pushlstring(L, i.data(), i.size());
+            lua_pushboolean(L, true);
+            lua_rawset(L, -2);
+          }
+        } else if constexpr (std::is_same_v<T, uint32_t>) {
+          lua_pushinteger(L, arg);
+        } else {
+          lua_pushnil(L);
+        }
+      },
+      val);
+}
+
 auto Property::PushValue(lua_State& s) -> int {
   std::visit(
       [&](auto&& arg) {
@@ -177,9 +201,9 @@ auto Property::PushValue(lua_State& s) -> int {
         } else if constexpr (std::is_same_v<T, audio::Track>) {
           lua_newtable(&s);
           int table = lua_gettop(&s);
-          for (const auto& [key, val] : arg.tags->tags()) {
+          for (const auto& tag : arg.tags->allPresent()) {
-            lua_pushstring(&s, database::TagToString(key).c_str());
+            lua_pushstring(&s, database::tagName(tag).c_str());
-            lua_pushstring(&s, val.c_str());
+            pushTagValue(&s, arg.tags->get(tag));
             lua_settable(&s, table);
           }
           if (arg.db_info) {