From 808a3fafc1d89a9b8ec76bbcc5b2514cefa9345d Mon Sep 17 00:00:00 2001
From: Sune Vuorela <sune@vuorela.dk>
Date: Sun, 24 Jun 2012 16:18:31 +0200
Subject: [PATCH 8/8] fix parsing of genre field in id3v2 tags and clean code
 up a bit

the genre field of a id3v2 tag might or might not be a number that matches
entries in a lookup table or alternatively a string. If it is a number,
then it might or might not be in parenthesis.
Handle all of the above and also handle the fact that some people might
enjoy adding numbers that are outside the range of the lookup table
---
 lib/endanalyzers/id3endanalyzer.cpp |   62 +++++++++++++++++++++++++++++++----
 1 file changed, 55 insertions(+), 7 deletions(-)

diff --git a/lib/endanalyzers/id3endanalyzer.cpp b/lib/endanalyzers/id3endanalyzer.cpp
index 677ece0..0db3728 100644
--- a/lib/endanalyzers/id3endanalyzer.cpp
+++ b/lib/endanalyzers/id3endanalyzer.cpp
@@ -81,7 +81,9 @@ replaygain
 VBR detection
 */
 
-static const string genres[148] = {
+#define ID3_NUMBER_OF_GENRES 148
+
+static const string genres[ID3_NUMBER_OF_GENRES] = {
   "Blues",
   "Classic Rock",
   "Country",
@@ -372,6 +374,54 @@ static bool extract_and_trim(const char* buf, int offset, int length, string& s)
     return !s.empty();
 }
 
+/**
+ * Functional helper class to get the right numbers out of a 'genre' string which
+ * might be a number in a index
+ */
+class genre_number_parser {
+  private:
+    bool success;
+    long result;
+    void parse_string( string genre ) {
+        char* endptr;
+        int r = strtol(genre.c_str(),&endptr, 10);
+        if(*endptr == '\0') { //to check if the convertion went more or less ok
+	    if(r >=0 && r < ID3_NUMBER_OF_GENRES ) { //to ensure it is within the range we have
+	        success=true;
+	        result=r;
+	    }
+        }
+    }
+  public:
+    /**
+     * constructor taking the genre string you want parsed as a number
+     */
+    genre_number_parser(string genre) : success(false), result(-1) {
+        if(genre.size()==0) {
+	  //if the string is empty, there is no need to try to parse it
+	    return;
+        }
+        //the string might start and end with parenthesis
+        if(genre[0]=='(' && genre[genre.size()-1]==')') {
+	    parse_string(genre.substr(1,genre.length()-2));
+	    return;
+        }
+        parse_string(genre);
+    }
+    /**
+     * wether or not parsing was successful
+     */
+    operator bool() {
+        return success;
+    }
+    /**
+     * the actual result of the parsing, or -1 if parsing wasn't successful
+     */
+    operator long() {
+        return result;
+    }
+};
+
 signed char
 ID3EndAnalyzer::analyze(Strigi::AnalysisResult& indexable, Strigi::InputStream* in) {
   const int max_padding = 1000;
@@ -512,11 +562,9 @@ ID3EndAnalyzer::analyze(Strigi::AnalysisResult& indexable, Strigi::InputStream*
 		    addStatement(indexable, albumUri, titlePropertyName, value);
 		    found_album = true;
 		} else if (strncmp("TCON", p, 4) == 0) {
-		    // The Genre is stored as (number)
-		    if( value[0] == '(' && value[value.length()-1] == ')' ) {
-			//vHanda: Maybe one should check if all the characters in between are digits
-			int genreIndex = atoi( value.substr( 1, value.length()-1 ).c_str() );
-			indexable.addValue(factory->genreField, genres[ genreIndex ]);
+		    genre_number_parser p(value);
+		    if(p) {
+			indexable.addValue(factory->genreField, genres[ p ]);
 			found_genre = true;
 		    } else {
 			// We must not forget that genre could be a string.
@@ -629,7 +677,7 @@ ID3EndAnalyzer::analyze(Strigi::AnalysisResult& indexable, Strigi::InputStream*
 	    if (!found_track && !buf[125] && buf[126]) {
 		indexable.addValue(factory->trackNumberField, (int)(buf[126]));
 	    }
-	    if (!found_genre && (unsigned char)(buf[127]) < 148)
+	    if (!found_genre && (unsigned char)(buf[127]) < ID3_NUMBER_OF_GENRES)
 		indexable.addValue(factory->genreField, genres[(uint8_t)buf[127]]);
 	}
     }
-- 
1.7.10.4