GNOME Bugzilla – Bug 575380
[jamendo] Better genre loading
Last modified: 2012-03-28 22:31:59 UTC
In the current version of the plugin, the genre for the track is taken from the album genre in the database. But there are several inconsistencies in the jamendo database: Sometimes, only the album has a genre, and not the tracks (this is OK) Sometimes, only tracks have a genre, but not the album Sometimes, neither have a genre Sometimes, both album and tracks have a genre (and sometimes they are different) Sometimes, the genre is 0, which is probably some kind of default for certain mp3 tagging software (officially, 0 is Blues, but I've yet to find a single Blues track on Jamendo. I think that these tracks should have an unknown genre (maybe it's blues, but very probably not). As a part of rewriting the jamendo parser, I implemented the following genre resolution scheme: * 0 is the same as undefined * If both genres agree, display a single genre * If track or album genre is undefined, use the other one * If both genres are specified and different, display "Album genre / Track genre" Let me know what you think.
This sounds reasonable to me except for the last point. In this case, I'd arbitrarily pick one of the 2 defined genres.
(In reply to comment #1) > This sounds reasonable to me except for the last point. In this case, I'd > arbitrarily pick one of the 2 defined genres. You mean toss a coin and implement either one, or the other way, or pick one at runtime? I'd probably use the album genre, up until now no-one has complained about genres from the jamendo plugin (while track genres seem to be generally more specific, there are some weird cases like Reggae / Hard Rock where the song doesn't sound much like rock to me). Maybe turn this into a preference, with Album genre being the default?
I haven't looked at the genre data much, but it seems logical that we'd use the track genre if available, and the album genre otherwise. This is just not important enough to add a preference for.
Created attachment 130707 [details] [review] Better genre loading Ok, here's a patch to change how genres are loaded. I'm not too happy with it, it looks somewhat verbose (two try except blocks to handle missing tags and a bunch of conditions to handle the different cases). If both genres are present, the track genre is used, being (probably) more accurate for a track. Maybe the condition should therefore be simplified? Something like if trackgenre!=-1: maingenre = trackgenre else: maingenre = albumgenre This patch is based on top of the new parser from attachement 130696 (should this bug depend on Bug 424423?)
If you use 0 as the 'no genre specified' value, then you can just do 'maingenre = trackgenre or albumgenre'. I'd suggest doing the int conversion in resolve_genre too, to keep the actual parser code simpler.
The try except blocks aren't there because of the int conversion, but because of the fact that these two elements are often missing (the only ones we care about that are sometimes missing), and to catch the KeyError (I probably should have commented on that in the code, or make the exception explicit). Moving the conversion to int from the parser into the resolver method won't make it much simpler. It's either catching an exception, or explicitly testing for the presence of the attribute. Using 0 as "no genre specified" has occured to me too, but I thought it would be overoptimizing. I also thought to simply replace the zeroth element of the genre list with _('Undefined') instead of "Blues". I'm not sure about 'maingenre = trackgenre or albumgenre' because sometimes they are both nonzero, but different and to detect that case I still have to have several conditions, or am I missing something?
Oh, right. In that case, you should use dict.get('id3genre', 0) rather than dict['id3genre'], which will return 0 rather than throwing an exception. 'trackgenre or albumgenre' will yield trackgenre if it's nonzero, otherwise albumgenre. It's not a bitwise 'or' operation.
The jamendo plugin no longer exists.