Rockbox

This is the bug/patch tracker for Rockbox. Click here for more information.

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#12266 - Genre tags not properly detected when database built

Attached to Project: Rockbox
Opened by James Zimmerman (Toxikator) - Thursday, 08 September 2011, 22:44 GMT+2
Last edited by Andree Buschmann (Buschel) - Saturday, 24 September 2011, 17:51 GMT+2
Task Type Bugs
Category Database
Status Closed
Assigned To No-one
Player Type iPod 5G
Severity Low
Priority Normal
Reported Version Release 3.9
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

Full details can be found here: http://forums.rockbox.org/index.php/topic,28810.msg184501.html

Short version:

Certain tracks (sometimes a few but not ALL from an album, other times all the tracks on an album) show up as <untagged> in my genre list. I made a list of the tracks in question; every time I rebuild the database, it is the same 10 or so albums that show up in this list. iBUT, in many cases, one album inside an artist's subfolder on the drive will show up correctly even though another is missed. Also, one album (Imogen Heap's "Ellipse", not that it matters) shows up filed under a genre called "p", though I think it's a unicode symbol resembling a lowercase p because it sorts alphabetically after "Soundtrack/Videogame"in my genre list.

It can't be the value in the genre field as these values are not unique to the error (e.g. Imogen Heap's Ellipse is supposed to show up as "Pop/Rock", but other albums show up as "Pop/Rock" just fine so it can't be that value causing the bug). No other fields (Artist, Album, Track title) seem to be affected. There doesn't seem to be any common denominator between the affected tracks and normal tracks in terms of what tags are filled in or what values are in them.

Also, all of the MP3s in question are tagged with id3v2.4, and I've applied/removed the tags as id3v1 just to make sure that some bit of junk data wouldn't be present.

The tracks all show up fine when read in Foobar2k OR in MP3Tag, so I believe the issue isn't with the tracks themselves but with how Rockbox sees them. This bug is present in both 9.3 AND 9.3.1, but did not affect me before I updated; however, the last version I used before these was a patched version of 3.3. The bug is present with both my custom tagnavi AND with the Rockbox standard tagnavi.

HW: iPod Video 5.5G (64MB version) with a 240GB HDD added in, if that matters.

In the linked thread, you can download one of the problem tracks if that helps.
This task depends upon

Closed by  Andree Buschmann (Buschel)
Saturday, 24 September 2011, 17:51 GMT+2
Reason for closing:  Fixed
Additional comments about closing:  Fixed with r30550.
Comment by Andree Buschmann (Buschel) - Friday, 09 September 2011, 08:15 GMT+2
I checked your file with a hex-editor. The tag contains 2 gerne fields: the first contains "Pop/Rock", the second contains a non-zero terminated special character. I have attached a screenshot to show this ('TCON' is the start of a gerne field).

The attached patch just keeps the first gerne that has been found and drops any further gerne. Of course this could just drop the "good" gerne, in case the second one is the valid one... I am not sure how other taggers handle such cases. Maybe there is some logic which drops tags that are most likely unsufficient (e.g. not zero-terminated, non-number but only 1 char long, ...).

Imho the best would be to re-tag those files and remove the 2nd, unwanted, tag.
   Image1.jpg (129.5 KiB)
   id3_v01.patch (2.4 KiB) 
 apps/metadata/id3tags.c |   50 ++++++++++++++++++++++++++++--------------------
 1 file changed, 30 insertions(+), 20 deletions(-)

Comment by James Zimmerman (Toxikator) - Friday, 09 September 2011, 19:58 GMT+2
I'm not coder, Buschel, but it's my suspicion that the foobar source code would tell you how IT handles it, at least. I'd do it myself if I knew the first thing about reading source code. What's strange is that RB didn't always have this problem, so evidently it used to read only the first tag but is only now starting to read the most recent.

Retagging I've already tried, inasmuch as I've deleted the genre tag, applied the changes, and then put it back. So what I'll do is completely strip the tags from the MP3, ALL of them, and then rebuild them from scratch. My assumption here is Foobar is modifying the first tag but is unaware of the second one; hopefully when I strip all tags, even the fields it doesn't recognize will be gone. If not, I guess I'll have to completely rebuild the header (which foobar can also do, and I'm surprised I didn't try).
Comment by Andree Buschmann (Buschel) - Friday, 09 September 2011, 20:16 GMT+2
> What's strange is that RB didn't always have this problem

It is not that strange. There has been a lot of work done in this area. It is just that the behaviour changed when handling inconsistent metadata.

> inasmuch as I've deleted the genre tag, applied the changes, and then put it back

When using MP3Tag you will need to fully remove the tag, save the file and re-create the tags again. I had similar issues with embedded album art which was somehow messed up. In my case rockbox worked fine, but MusicBee did not read the inconsistent metadata. I also needed to remove, save and re-create the tag with MP3Tag.

Comment by Magnus Holmgren (learman) - Saturday, 10 September 2011, 10:10 GMT+2
Another solution, that also addresses a bug in Rockbox. The second TCON is empty (not even a terminating 0), and Rockbox doesn't read that correctly. This patch fixes that, and then also skips completely empty strings. Not sure if empty strings should be skipped, but it seems reasonable to do.
   id3_empty_strings.patch (1.1 KiB) 
 apps/metadata/id3tags.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comment by Andree Buschmann (Buschel) - Saturday, 10 September 2011, 23:36 GMT+2
Magnus, with your fix the file from Jens reads fine. But the embedded album art of one of my files is not loaded anymore.

Link to file: http://www.sendspace.com/file/h0rmi5
Comment by Magnus Holmgren (learman) - Sunday, 11 September 2011, 10:46 GMT+2
Ah, the empty frame fix wasn't correctly done, which was noticed here due to an "empty" COMM frame before the album art... A little confusing with long nested loops like that. :) This version should be better, and includes extra checks to avoid reading past the read/allocated tag buffer when checking for iTunes tags.
   id3_empty_strings_v2.patch (2.3 KiB) 
 ../apps/metadata/id3tags.c |   20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

Comment by Andree Buschmann (Buschel) - Sunday, 11 September 2011, 19:50 GMT+2
Looks good for me.

Loading...