Rockbox.org home
release
dev builds
extras
themes manual
wiki
device status forums
mailing lists
IRC bugs
patches
dev guide



Rockbox mail archive

Subject: Re: ID3V2 genre
From: TP Diffenbach (rockbox_at_diffenbach.org)
Date: 2003-06-22


Yes, you're correct.

I'm responsible for this code; when I simplified adding tags, in order to be
backward compatible with the existing ID3 code, I copied the test for genre-list
genres from the existing Rockbox code:

- else if (!entry->genre &&
- !strncmp(header, "TCON", 4)) {
- char* ptr = buffer + bufferpos;
- bytesread = read(fd, ptr, framelen);
- if(bytesread < 0 || bytesread < framelen)
- return;
-
- if (ptr[1] == '(' && ptr[2] != '(')
- entry->genre = atoi(ptr+2);
- bufferpos += bytesread + 1;

I had assumed the existing code was working (although I'm aware that neither it or
my copy fully conforms to the ID3 specification, namely that /all/ encoded genres
should be decoded, not just the first), and as my code was there to be backward
compatible, didn't think it needed changing. My mistake.

We should be testing the first byte, not the second, for an open parenthesis.

Conforming to the specification that all encoded genres be decoded, along with any
interstitial text, however, would require a more compilcated parser; to my mind,
if the tag writer wants to include more than one genre or intersitial freeform
text, it makes more sense to make the entire tag freeform text.

--Tom

Quoting Magnus Holmgren <lear_at_algonet.se>:

> Hi,
>
> Regarding the function parsegenre() in id3.c, shouldn't the "if"
> statement begin like this:
>
> if( tag[ 0 ] == '(' && tag[ 1 ] != '(' ) {
> entry->genre = atoi( tag + 1 );
>
> That's at least required for it to work on the tags I have (written by
> Lame 3.90)...
>
> --
> Magnus Holmgren
>
>
>

-- 
Archos FM has a Rockbox!



Page was last modified "Jan 10 2012" The Rockbox Crew
aaa