From 5140b76f08eb500633234704de30b27d2c1489f2 Mon Sep 17 00:00:00 2001
From: Martchus <martchus@gmx.net>
Date: Wed, 13 Feb 2019 18:06:02 +0100
Subject: [PATCH] Treat 255 as empty ID3v1 genre index

---
 CMakeLists.txt     |  2 +-
 id3/id3genres.cpp  |  5 +++++
 id3/id3genres.h    | 20 ++++++++++++++++++++
 tagvalue.cpp       | 25 +++++++++++++++++--------
 tests/tagvalue.cpp |  2 ++
 5 files changed, 45 insertions(+), 9 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index f0879b1..fce4e21 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -170,7 +170,7 @@ set(META_APP_AUTHOR "Martchus")
 set(META_APP_URL "https://github.com/${META_APP_AUTHOR}/${META_PROJECT_NAME}")
 set(META_APP_DESCRIPTION "C++ library for reading and writing MP4 (iTunes), ID3, Vorbis, Opus, FLAC and Matroska tags")
 set(META_VERSION_MAJOR 8)
-set(META_VERSION_MINOR 1)
+set(META_VERSION_MINOR 2)
 set(META_VERSION_PATCH 0)
 set(META_PUBLIC_SHARED_LIB_DEPENDS c++utilities)
 set(META_PUBLIC_STATIC_LIB_DEPENDS c++utilities_static)
diff --git a/id3/id3genres.cpp b/id3/id3genres.cpp
index 52e25fe..24dda00 100644
--- a/id3/id3genres.cpp
+++ b/id3/id3genres.cpp
@@ -11,6 +11,7 @@ namespace TagParser {
 
 /*!
  * \brief Returns all known genre names.
+ * \todo Fix const-correctness in v9.
  */
 const char **Id3Genres::genreNames()
 {
@@ -37,9 +38,13 @@ const char **Id3Genres::genreNames()
 
 /*!
  * \brief Returns the numerical denotation of the specified \a genre or -1 if \a genre is unknown.
+ * \remarks If \a string is empty, the non-standard Id3Genres::emptyGenreIndex() is returned.
  */
 int Id3Genres::indexFromString(const string &genre)
 {
+    if (genre.empty()) {
+        return emptyGenreIndex();
+    }
     const char *const *ptr = genreNames();
     for (int index = 0; index < genreCount(); ++ptr, ++index) {
         if (genre == *ptr) {
diff --git a/id3/id3genres.h b/id3/id3genres.h
index 22f1e95..b1595c3 100644
--- a/id3/id3genres.h
+++ b/id3/id3genres.h
@@ -14,6 +14,8 @@ public:
     static inline const char *stringFromIndex(int index);
     static int indexFromString(const std::string &genre);
     static constexpr int genreCount();
+    static constexpr int emptyGenreIndex();
+    static constexpr bool isEmptyGenre(int index);
     static constexpr bool isIndexSupported(int index);
 
 private:
@@ -36,6 +38,24 @@ constexpr int Id3Genres::genreCount()
     return 192;
 }
 
+/*!
+ * \brief Returns the preferred genre index to indicate that no genre is set at all.
+ * \remarks Apparently some files use 255 to indicate the genre information is missing although this
+ *          is not explicitely specified on [ID3.org](http://id3.org/ID3v1).
+ */
+constexpr int Id3Genres::emptyGenreIndex()
+{
+    return 255;
+}
+
+/*!
+ * \brief Returns whether the genre \a index indicates the genre field is not set at all.
+ */
+constexpr bool Id3Genres::isEmptyGenre(int index)
+{
+    return index == emptyGenreIndex();
+}
+
 /*!
  * \brief Returns an indication whether the specified numerical denotation is
  *        supported by this class.
diff --git a/tagvalue.cpp b/tagvalue.cpp
index db3cb03..781db93 100644
--- a/tagvalue.cpp
+++ b/tagvalue.cpp
@@ -212,7 +212,7 @@ int32 TagValue::toInteger() const
 int TagValue::toStandardGenreIndex() const
 {
     if (isEmpty()) {
-        return 0;
+        return Id3Genres::emptyGenreIndex();
     }
     int index = 0;
     switch (m_type) {
@@ -470,13 +470,17 @@ void TagValue::toString(string &result, TagTextEncoding encoding) const
     case TagDataType::PositionInSet:
         result = toPositionInSet().toString();
         break;
-    case TagDataType::StandardGenreIndex:
-        if (const char *genreName = Id3Genres::stringFromIndex(toInteger())) {
+    case TagDataType::StandardGenreIndex: {
+        const auto genreIndex = toInteger();
+        if (Id3Genres::isEmptyGenre(genreIndex)) {
+            result.clear();
+        } else if (const char *genreName = Id3Genres::stringFromIndex(genreIndex)) {
             result.assign(genreName);
-            break;
         } else {
             throw ConversionException("No string representation for the assigned standard genre index available.");
         }
+        break;
+    }
     case TagDataType::TimeSpan:
         result = toTimeSpan().toString();
         break;
@@ -547,12 +551,17 @@ void TagValue::toWString(std::u16string &result, TagTextEncoding encoding) const
     case TagDataType::PositionInSet:
         regularStrRes = toPositionInSet().toString();
         break;
-    case TagDataType::StandardGenreIndex:
-        if (const char *const genreName = Id3Genres::stringFromIndex(toInteger())) {
+    case TagDataType::StandardGenreIndex: {
+        const auto genreIndex = toInteger();
+        if (Id3Genres::isEmptyGenre(genreIndex)) {
+            regularStrRes.clear();
+        } else if (const char *genreName = Id3Genres::stringFromIndex(genreIndex)) {
             regularStrRes.assign(genreName);
-            break;
+        } else {
+            throw ConversionException("No string representation for the assigned standard genre index available.");
         }
-        throw ConversionException("No string representation for the assigned standard genre index available.");
+        break;
+    }
     case TagDataType::TimeSpan:
         regularStrRes = toTimeSpan().toString();
         break;
diff --git a/tests/tagvalue.cpp b/tests/tagvalue.cpp
index 2e9d47a..3093ad6 100644
--- a/tests/tagvalue.cpp
+++ b/tests/tagvalue.cpp
@@ -84,6 +84,8 @@ void TagValueTests::testInteger()
     CPPUNIT_ASSERT_EQUAL("42"s, integer.toString());
     integer.assignInteger(2);
     CPPUNIT_ASSERT_EQUAL("Country"s, string(Id3Genres::stringFromIndex(integer.toStandardGenreIndex())));
+    integer.assignInteger(255);
+    CPPUNIT_ASSERT_EQUAL(string(), string(Id3Genres::stringFromIndex(integer.toStandardGenreIndex())));
 
     // negative number
     integer.assignInteger(-25);