From 63215ccf1677ac2ea41cd6f64025d55f6f4918a0 Mon Sep 17 00:00:00 2001
From: jacqueline <me@jacqueline.id.au>
Date: Wed, 6 Sep 2023 12:38:40 +1000
Subject: [PATCH] Fix missing track number issues

---
 src/database/index.cpp      |  6 +++-
 src/database/records.cpp    |  9 +++---
 src/database/tag_parser.cpp | 57 ++++++++++++++++++++++++++-----------
 src/database/track.cpp      |  1 +
 4 files changed, 51 insertions(+), 22 deletions(-)

diff --git a/src/database/index.cpp b/src/database/index.cpp
index 6ec88622..9d77e530 100644
--- a/src/database/index.cpp
+++ b/src/database/index.cpp
@@ -5,10 +5,13 @@
  */
 
 #include "index.hpp"
-#include <stdint.h>
+
+#include <cstdint>
 #include <variant>
+
 #include "komihash.h"
 #include "leveldb/write_batch.h"
+
 #include "records.hpp"
 
 namespace database {
@@ -49,6 +52,7 @@ static auto missing_component_text(const Track& track, Tag tag)
     case Tag::kTitle:
       return track.TitleOrFilename();
     case Tag::kAlbumTrack:
+      return "0000";
     case Tag::kDuration:
     default:
       return {};
diff --git a/src/database/records.cpp b/src/database/records.cpp
index 72608eb0..0d28ca28 100644
--- a/src/database/records.cpp
+++ b/src/database/records.cpp
@@ -309,7 +309,8 @@ auto EncodeIndexKey(const IndexKey& key) -> OwningSlice {
   // Construct the footer.
   out << kFieldSeparator;
   if (key.track) {
-    out << TrackIdToBytes(*key.track).data;
+    auto encoded = TrackIdToBytes(*key.track);
+    out << encoded.data;
   }
   return OwningSlice(out.str());
 }
@@ -394,9 +395,9 @@ auto ParseIndexKey(const leveldb::Slice& slice) -> std::optional<IndexKey> {
 
   buffer = {};
   in.get(buffer);
-  if (buffer.str().size() > 1) {
-    std::string raw_id = buffer.str().substr(1);
-    result.track = BytesToTrackId(raw_id);
+  std::string id_str = buffer.str();
+  if (id_str.size() > 1) {
+    result.track = BytesToTrackId(id_str.substr(1));
   }
 
   return result;
diff --git a/src/database/tag_parser.cpp b/src/database/tag_parser.cpp
index 03e92b95..a4aaf0f5 100644
--- a/src/database/tag_parser.cpp
+++ b/src/database/tag_parser.cpp
@@ -17,7 +17,26 @@
 
 namespace database {
 
-auto convert_tag(int tag) -> std::optional<Tag> {
+const static std::array<std::pair<const char*, Tag>, 5> kVorbisIdToTag = {{
+    {"TITLE", Tag::kTitle},
+    {"ARTIST", Tag::kArtist},
+    {"ALBUM", Tag::kAlbum},
+    {"TRACKNUMBER", Tag::kAlbumTrack},
+    {"GENRE", Tag::kGenre},
+}};
+
+static auto convert_track_number(int number) -> std::string {
+  std::ostringstream oss;
+  oss << std::setw(4) << std::setfill('0') << number;
+  return oss.str();
+}
+
+static auto convert_track_number(const std::string& raw) -> std::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:
       return Tag::kTitle;
@@ -92,10 +111,6 @@ static void tag(Tagctx* ctx,
     return;
   }
   if (*tag == Tag::kAlbumTrack) {
-    uint32_t as_int = std::atoi(v);
-    std::ostringstream oss;
-    oss << std::setw(4) << std::setfill('0') << as_int;
-    value = oss.str();
   }
   aux->tags->set(*tag, value);
 }
@@ -137,6 +152,21 @@ auto TagParserImpl::ReadAndParseTags(const std::string& path, TrackTags* out)
     return false;
   }
 
+  // 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 (!out->at(Tag::kAlbumTrack)) {
+    auto slash_pos = path.find_last_of("/");
+    if (slash_pos != std::string::npos && path.size() - slash_pos > 1) {
+      out->set(Tag::kAlbumTrack, path.substr(slash_pos + 1));
+    }
+  }
+
+  // Normalise track numbers; they're usually treated as strings, but we would
+  // like to sort them lexicographically.
+  out->set(Tag::kAlbumTrack,
+           convert_track_number(out->at(Tag::kAlbumTrack).value_or("0")));
+
   {
     std::lock_guard<std::mutex> lock{cache_mutex_};
     cache_.Put(path, *out);
@@ -226,18 +256,11 @@ auto OpusTagParser::ReadAndParseTags(const std::string& path, TrackTags* out)
   }
 
   out->encoding(Container::kOpus);
-  const char* tag = NULL;
-  tag = opus_tags_query(tags, "TITLE", 0);
-  if (tag != NULL) {
-    out->set(Tag::kTitle, tag);
-  }
-  tag = opus_tags_query(tags, "ARTIST", 0);
-  if (tag != NULL) {
-    out->set(Tag::kArtist, tag);
-  }
-  tag = opus_tags_query(tags, "ALBUM", 0);
-  if (tag != NULL) {
-    out->set(Tag::kAlbum, tag);
+  for (const auto& pair : kVorbisIdToTag) {
+    const char* tag = opus_tags_query(tags, pair.first, 0);
+    if (tag != NULL) {
+      out->set(pair.second, tag);
+    }
   }
 
   op_free(f);
diff --git a/src/database/track.cpp b/src/database/track.cpp
index dc33701d..fa65261e 100644
--- a/src/database/track.cpp
+++ b/src/database/track.cpp
@@ -45,6 +45,7 @@ auto TrackTags::Hash() const -> uint64_t {
   HashString(&stream, at(Tag::kTitle).value_or(""));
   HashString(&stream, at(Tag::kArtist).value_or(""));
   HashString(&stream, at(Tag::kAlbum).value_or(""));
+  HashString(&stream, at(Tag::kAlbumTrack).value_or(""));
 
   return komihash_stream_final(&stream);
 }