Rockbox

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

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#11955 - Corrupted metadata as of r29350

Attached to Project: Rockbox
Opened by Luca Leonardo Scorcia (LucaLeonardoScorcia) - Monday, 21 February 2011, 01:04 GMT+2
Last edited by Andree Buschmann (Buschel) - Monday, 21 February 2011, 22:36 GMT+2
Task Type Bugs
Category ID3 / meta data
Status Closed
Assigned To Andree Buschmann (Buschel)
Player Type Sansa AMSv2
Severity High
Priority High
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

Hello. I want to report that metadata display is broken on r29350 on my FuzeV2. It shows some random buffer, sometimes text, sometimes GUIDs, sometimes 'boxes'. The problem is not present in r29348.
Rebuilding the database fails with data abort at 30800530 address 0x646c6f47. I checked the map file and 30800530 is inside strlen.
This task depends upon

Closed by  Andree Buschmann (Buschel)
Monday, 21 February 2011, 22:36 GMT+2
Reason for closing:  Fixed
Additional comments about closing:  Submitted with r29372
Comment by Michael Sparmann (TheSeven) - Monday, 21 February 2011, 01:26 GMT+2
Quick fix. I've never dealt with the metadata code before, and don't fully understand how exactly it was broken, but this seems to be the straight forward way to do what was intended to be done at that line, and it seems to work for me.
   r29349-fix.patch (0.6 KiB) 
 id3tags.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comment by Michael Sparmann (TheSeven) - Monday, 21 February 2011, 09:29 GMT+2
This one should also handle the case where the tag gets removed completely. Thanks jhMikeS for pointing out.
   r29349-fix-v2.patch (0.6 KiB) 
 id3tags.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comment by Andree Buschmann (Buschel) - Monday, 21 February 2011, 11:52 GMT+2
Seems like I missed a few facts :/

I reviewed the parser this morning, but I will need some additional time to check the handling when replaygain data is contained in the tag.

If I do not have a proper solution this evening I will reject my change to ensure svn Trunk is working fine. In this case I will re-open  FS#11920  and close this one.

Sorry for the inconvenience...
Comment by Magnus Holmgren (learman) - Monday, 21 February 2011, 19:28 GMT+2
The simple fix, I think, is to only do the buffer adjustment if the expression "(!tr->binary && !tr->ppFunc)" is true (works in a quick test at least). That way, a ppFunc can/should do its own adjustments (the replaygain parser does that), while still allowing unicode_munge to read all strings (and the replaygain parser needs that).
Comment by Andree Buschmann (Buschel) - Monday, 21 February 2011, 22:27 GMT+2
It was a bad idea to perform trimming after calling ppFunc. The different parsers which are called via ppFunc have different internal handling and need different trimming. The attached patch does four changes to the broken svn:
1) revert r29349 -> fixes the error in svn
2) implement trimming inside parsegenre() -> saves buffer as intended
3) implement trimming inside parseuser() in case of "ALBUM ARTIST" -> saves buffer as intended
4) correct trimming inside parseuser in case of !"ALBUM ART" and !CONFIG_CODEC == SWCODEC -> error in svn
   r29349-fix-v03.patch (2.5 KiB) 
 apps/metadata/id3tags.c |   21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

Loading...