FS#1126 - id3 freeform genre & composer; cleaner code

Attached to Project: Rockbox
Opened by Thomas Paul Diffenbach (tpdiffenbach) - Wednesday, 19 March 2003, 08:26 GMT
Last edited by Björn Stenberg (zagor) - Wednesday, 04 June 2003, 15:10 GMT
Task Type Patches
Category ID3 / meta data
Status Closed
Assigned To No-one
Operating System
Severity Low
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Votes 0
Private No


Added freefrorm genre and composer to supported id3v2 tags.

Removed repeated code and hardcoded id3 tags from the
setid3v2title function to an array of structures. This
allows new id3 tags to be added by simply adding an
entry to an array, without modifying the code as such.
(For those keeping score, this is Meyer's "Open-Closed"
principle.) Of course, wpd-displayu.c must also be
modified for teh tag to be recognized by the wps system.

All tags are treated as strings, rather than as ints.
Converting string to ints means the ints must be
re-converted to strings for display. This adds code and
runtime costs for both conversions. Keeping the tag a
string uses some memory, but the memory is statically
allocated anyway. Where the original code performed the
conversions to int (genre, year, and tracknum), these
conversions are still performed for backwards
compatibility (via post-processing functions).

String tags also allow for cases where a number won't
do, e.g., YEAR: "circa 1765", "1790/1977"
(composed/performed), "28 Feb 1969"; TRACK: "1/12", "1
of 12"; GENRE: "Freeform genre name".

The structure used to identify tags and the mp3entry
member which points to the tag is:

struct id3TagResolver {
const char* tag;
int tag_length ;
size_t offset ;
tagPostProcessFunc ppFunc ;
} ;

tag is the id3v2 tag, tag_length is its length (as
strlen would return), offset is the offset of a char*
in struct mp3entry that will point to the tag data,
ppFunc is an optional (pointer to) function that will
post-process the recovered tag. Note that all members
can be resolved at compile time, so the code space and
runtime needed for calling strlen is no longer needed.

To recover id3 tags, the array tagList is iterated for
each id3 header, and if found, the char* at offset is
pointed to a copy of the tag in struct
mp3entry.id3v2buf. Multiple tags may reference the same
pointer in mp3entry; following the original id3 code,
only the first found is retained.

To add a new tag, add a char* to mp3entry, and an entry
to tagList as in the example below:

struct id3TagResolver tagList[] = {
{ "TPE1", sizeof( "TPE1" ) - 1, offsetof( struct
mp3entry, artist ), NULL },
{ "TP1", sizeof( "TP1" ) - 1, offsetof( struct
mp3entry, artist ), NULL },

Note that both of the above tags set mp3entry.artist.
If the id3 data contains both a "TPE1" and a "TP1" tag,
the first found will be used.

The post-processing function pointer is called if it is
not NULL; it can re-arrange the string saved in
mp3Entry, increasing the string's size to the extent of
the remaining buffer space, or truncating it entirely.
It must only return the offset within
mp3entry.id3v2buff of the first free byte. See the
function ppGenre for an example.

This task depends upon

Closed by  Björn Stenberg (zagor)
Wednesday, 04 June 2003, 15:10 GMT
Reason for closing:  Accepted
Comment by Linus Nielsen Feltzing (linusnielsen) - Wednesday, 19 March 2003, 08:54 GMT

Sounds pretty cool. It would be even cooler if you attached
the patch.
Comment by Björn Stenberg (zagor) - Thursday, 24 April 2003, 13:19 GMT

Ouch, the patch is evil, it replaces the entire files. Could you make a new
one with "cvs diff -ub" to ignore whitespace changes?
Comment by Thomas Paul Diffenbach (tpdiffenbach) - Thursday, 22 May 2003, 09:25 GMT

Corrected diffs, diffed from the 2003-05-22 cvs, submitted.
Comment by Thomas Paul Diffenbach (tpdiffenbach) - Thursday, 22 May 2003, 09:29 GMT

New diffs added, diffed from cvs 2003-05-22.