From: TP Diffenbach <rockbox_at_diffenbach.org>
Date: Sun, 22 Jun 2003 17:12:55 -0400

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!

Received on 2003-06-22