Rockbox

Tasklist

FS#7670 - Read rating value from MP3 Id3 tag for display in WPS

Attached to Project: Rockbox
Opened by Nick Waterton (Nick_W) - Tuesday, 28 August 2007, 22:39 GMT
Task Type Patches
Category ID3 / meta data
Status Unconfirmed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Votes 1
Private No

Details

This Patch changes the way that ratings work. It's pretty simple, it reads the rating info from an MP3 files ID3 POPM tag, and converts that to a Rockbox rating value (0 to 10).

You can then use patch  FS#6301  - new tags for WPS and tagcache patch 4 (4_rating.patch) to display the rating on the WPS (tag is %rr). Some Themes use this tag already so I'm not sure if it is in SVN already.

The patch is written for MediaMonkey (alpha 3), but it should work with other version of MediaMonkey, and other programs that put ratings in the ID3 tag (not iTunes).

Tested on iPod nano.

One thing I'm not sure of is what value to return for bufferpos - should this return an int ponting to the next id3 tag position, or is it OK to return it as is? it dosen't seem to make any difference either way.

Comments are welcome.
This task depends upon

Comment by Nick Waterton (Nick_W) - Tuesday, 28 August 2007, 22:41 GMT
OK here is the patch.
Comment by Robert Kukla (roolku) - Wednesday, 29 August 2007, 14:21 GMT
A few comments just from memory and reading the patch:

- if you want this to be included in svn consider the other audio formats and standards for storing the rating in their metadata
- 4_rating.patch is in svn, so you will need to think about how your patch will co-exist. At the moment a value in the ID3 will have priority over run-time data from rockbox; you will still be able to change it, the change will be committed to the runtime db and used in the database browser, but the next time you play the song, the wps will display the old value from the ID3 while the runtime db still has the changed value - seems very inconsitent and confusing to me (not sure what the best solution is - use a different tag or clarify priority of the two mechanisms)
- the sequence of the "if" statements should be optimised by using "else"
- bufferpos should point to the position after the tag data, but I can't remember if it is actually used
Comment by Nick Waterton (Nick_W) - Friday, 31 August 2007, 13:38 GMT
Thanks for the comments.

I have looked at other formats to see how "rating" tags are stored, and there isn't really a defined or consistent method. A specific tag equivalent to POPM is not available in AAC, WMA, FLAC, OGG or MPC. In AAC a tag "rtng" is used, but only in m4p files (not m4a), and iTunes does not store rating in the files, but in it's own database. The tag TXXX is sometimes used to store ratings (keyword RATING), but this is a user defined tag, and it's use is somewhat random. Sometimes a tag "rating" is used, but again it's use is not standardized.

Also, the values used in various schemes are not defined (except for ID3 POPM tag). Some use the 0 to 100 value (as iTunes does), others use the ID3 format (0 to 255). As ID3 is clearly defined, I decided to start with that. I could add others if present, but the lack of standards makes it difficult to make it consistent. should a TXXX tag override a POPM tag or vice versa? how should a rtng (or rating) tag be measured 0-100 or 0-255?

It is probably worth looking into for popular database organizers and formats - but best to get started with the "low hanging fruit". Comments on this topic are welcome.

How the rating in the file should co-exist with the rating on the player is also worthy of some discussion. The way it works now is a little inconsistent, however if no POPM tag exists (ie not rated) the existing scheme works. If the tag does exist, then it overrides the real time rating (if "gather real time data" is turned on). It's a little inconsistent if the tag exists, but has a value of zero (0) as this technically means unrated - but then why would you have a tag that says the tag has not been assigned? still it is possible, and maybe I should assign NULL in that case. Maybe I should add a configuration menu item "use real time rating" or "use track rating" so you can select which rating scheme to use. Again, comments welcome.

Another case not handled too well is that it is possible to have more than one POPM tag (with different email addresses - ie different raters). In that case, the last POPM tag encountered is used, but really there should be some way to say specifically which persons (or usually programs) rating you want to select. Also the earlier ID3V2 tag POP is not handled at all, although it should be easy to add, I'm just not sure if I should add support for older depreciated formats.

I guess I should optimize the code a little. Easy enough to do.

Still not sure what to do with bufferpos, perhaps I should just return the offset to the next ID3 tag to be consistent.

Again, thanks for the comments, I'll probably put some time in over the long weekend, and see if I can produce an updated version.
Comment by Nick Waterton (Nick_W) - Tuesday, 04 September 2007, 14:11 GMT
Optimized code as suggested.

Added support for OGG, FLAC, WMA and ASF file types (in addition to MP3).

Tested on iPod nano.

Let me know if anyone has other suggestions.
Will add other file types when I have time.

patch is for r14606 (tested and works with no other patches required). 4th Sept 2007.
Comment by Nick Waterton (Nick_W) - Wednesday, 05 September 2007, 19:21 GMT
Added MP4 (M4a, AAC etc) MPC and APEv2 support.

I don't think game sound formats (ADX, SPC, SID) format will be supported as they have no built in rating mechanism. WAV format could be added as I have the tag for ratings, but as rockbox has no RIFF decoder, I would have to implement a full RIFF decoder to do this (which may be beyond the scope of this patch). AIFF also does not seem to have a decoder for it.

Still need some control features to be added (ie what tag takes priority), but I could use some help testing on different file formats (it's very hard to get clean copies of some formats eg wavepack files).
Comment by Nick Waterton (Nick_W) - Friday, 07 September 2007, 12:19 GMT
A significant re-write,

A new menu entry has been added to General Settings, Database called "Display Rating from File". If this is set to "yes" (the default), then the rating is taken from the file metadata tags. If it is set to "no" then the rating is taken from the run time data tagcache. When the rating is read from the file, you can view the rating in the context menu (now says "View Rating (file)) but you can't change it. If the rating is read from tagcache, you can still change the rating on the context menu as before.

A new tag has been added to the WPS screen (%rs) which can also be used as a conditional (%?rs) this will display the source of the rating (file or tagcache) example %?rs<File|Tag>, and could be used in conjunction with %rr to display different graphics depending on the source of the rating.

File type supported are currently:
MP3, OGG, FLAC, WMA, ASF, MP4 (AAC, M4a etc.), MPC and APE (v2).

This should work on all platforms (even those without tagcache support) I would appreciate any testing that could be done on other platforms, and various file formats/rating systems.
Comment by Nick Waterton (Nick_W) - Saturday, 06 October 2007, 18:41 GMT
Resync to r15005
Comment by Enno (onkel_enno) - Thursday, 01 November 2007, 11:18 GMT
Wouldn't it make more sense to read the ratings from the files and save them into the tagcache WHILE adding files to the tagcache?
I would really like to be able to read the ratings from the files, but if I can't use them in the Database and can't change them, it seem to me a kind of useless to only see them in the WPS.

Is that possible?

Thx
Comment by Tim Crist (ts-x) - Saturday, 22 December 2007, 01:17 GMT
I'm getting these errors when attempting to patch the '2007-12-21 20:48' build...

apps/onplay.c
Hunk #1 FAILED at 966
Hunk #2 FAILED at 987
firmware/export/id3.h
Hunk #1 FAILED at 206

By the way, great work on this patch. I've been hoping for this functionality since I started using Rockbox several years ago. Thanks for all of your effort!

Loading...