Skip to content
This repository
Browse code

[taglib] taglib doesn't resolve genre numbers where just a number is …

…given, so add workaround. fixes #13663
  • Loading branch information...
commit cbb08fb35907c215ae4e78c5e6ae9e198d1c1aa3 1 parent 2ac9b12
authored December 04, 2012

Showing 1 changed file with 20 additions and 3 deletions. Show diff stats Hide diff stats

  1. 23  xbmc/music/tags/TagLoaderTagLib.cpp
23  xbmc/music/tags/TagLoaderTagLib.cpp
@@ -27,6 +27,7 @@
27 27
 #include <taglib/id3v2tag.h>
28 28
 #include <taglib/apetag.h>
29 29
 #include <taglib/xiphcomment.h>
  30
+#include <taglib/id3v1genres.h>
30 31
 
31 32
 #include <taglib/textidentificationframe.h>
32 33
 #include <taglib/uniquefileidentifierframe.h>
@@ -645,8 +646,24 @@ void CTagLoaderTagLib::SetAlbumArtist(CMusicInfoTag &tag, const vector<string> &
645 646
 
646 647
 void CTagLoaderTagLib::SetGenre(CMusicInfoTag &tag, const vector<string> &values)
647 648
 {
648  
-  if (values.size() == 1)
649  
-    tag.SetGenre(values[0]);
  649
+  /*
  650
+   TagLib doesn't resolve ID3v1 genre numbers in the case were only
  651
+   a number is specified, thus this workaround.
  652
+   */
  653
+  vector<string> genres;
  654
+  for (vector<string>::const_iterator i = values.begin(); i != values.end(); ++i)
  655
+  {
  656
+    string genre = *i;
  657
+    if (StringUtils::IsNaturalNumber(genre))
  658
+    {
  659
+      int number = strtol(i->c_str(), NULL, 10);
  660
+      if (number >= 0 && number < 256)
  661
+        genre = ID3v1::genre(number).to8Bit(true);
  662
+    }
  663
+    genres.push_back(genre);
  664
+  }
  665
+  if (genres.size() == 1)
  666
+    tag.SetGenre(genres[0]);
650 667
   else
651  
-    tag.SetGenre(values);
  668
+    tag.SetGenre(genres);
652 669
 }

11 notes on commit cbb08fb

Skixbmc

You have introduced a new bug, i tag my mp3's for example with dance\90 with mp3tag.....
Now i have a lot of new genres :-)
I would suggest that you add an boolean flag so that when the tag is ID3v2 that is not looking up for a number.

Reg.
Fred

jmarshallnz
Owner

Why tag using dance/90 if you don't want Avantgarde dance?

Skixbmc

Playlist, i can create a playlist with dance and only the 90's.

TCON
The 'Content type', which previously was stored as a one byte numeric value only, is now a numeric string. You may use one or several of the types as ID3v1.1 did or, since the category list would be impossible to maintain with accurate and up to date categories, define your own.

References to the ID3v1 genres can be made by, as first byte, enter "(" followed by a number from the genres list (appendix A) and ended with a ")" character. This is optionally followed by a refinement, e.g. "(21)" or "(4)Eurodisco". Several references can be made in the same frame, e.g. "(51)(39)". If the refinement should begin with a "(" character it should be replaced with "((", e.g. "((I can figure out any genre)" or "(55)((I think...)". The following new content types is defined in ID3v2 and is implemented in the same way as the numerig content types, e.g. "(RX").

Reg.
Fred

jmarshallnz
Owner

The only fix possible here would be to ignore numbers if there's also a text entry. A patch to do so would be welcome. Basically run a loop checking for numeric only, and if it's set, do the conversion.

Skixbmc

Is it not a better id to create a seperate void for id3v1 and for id3v2?

jmarshallnz
Owner

No - some tagging apps specify the genre using a numeric value in id3v2.

Skixbmc

But that is not according to the specifications of id3.org. So you think we should add support for apps that do not meet the standards?

jmarshallnz
Owner

Yes - lots of apps ignore standards whether we like it or not, and particularly for anything pre id3v2.4 you get all sorts of weird and wonderful combinations. Very few people use Genre to specify the decade, after all!

Skixbmc

Then I will suggest to change the line to:
if (values.size() == 1 && StringUtils::IsNaturalNumber(genre))

jmarshallnz
Owner

That's not a solution. Indeed, my solution above won't work either. See updateGenre() in:

https://github.com/taglib/taglib/blob/master/taglib/mpeg/id3v2/id3v2framefactory.cpp

You can easily get a combination of numbers and strings out of taglib. e.g. the standard method of tagging a genre with a subgenre
"(90) Funky" will result in "90" and "Funky" being returned in the genre list.

If (and it's a big if) we assume that some app doesn't intentionally specify a string separated list of id3 genre numbers, or genre/subgenre pairs then we could convert the number only if it's the first item in the stringlist. Anything else would require an upstream change.

Skixbmc

Clear, i will look into it and come back when i've think that i have a solution and you agree with it :-)

Please sign in to comment.
Something went wrong with that request. Please try again.