FS#4961 - Adds Disc Number ID Tag for Tagcache (Feature Request FS#4957)

Attached to Project: Rockbox
Opened by Charles Voelger (rotundo) - Thursday, 30 March 2006, 17:32 GMT
Last edited by Dan Everton (safetydan) - Friday, 03 August 2007, 10:13 GMT
Task Type Patches
Category ID3 / meta data
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 1
Private No


This adds disc numbers to tag cache.
This task depends upon

Closed by  Dan Everton (safetydan)
Friday, 03 August 2007, 10:13 GMT
Reason for closing:  Accepted
Additional comments about closing:  Patch committed to SVN. Thanks.
Comment by Marc Cramdal (Bono) - Saturday, 01 April 2006, 19:13 GMT
I think this patch would be perfect if you added the wps entry for printing the disknumber. I think %ik would be nice (k like disK)

Thanks a lot for your patch !
Comment by Charles Voelger (rotundo) - Tuesday, 04 April 2006, 19:44 GMT
here is the patch with a %ik wps tag.
Comment by Ruben Petersen (Luckz) - Tuesday, 25 April 2006, 13:50 GMT
Should this possibly be merged with ?
Comment by Charles Voelger (rotundo) - Tuesday, 25 April 2006, 19:12 GMT
well as they are two different features, I would say no. Although I wonder if this every works with current cvs, I will have to look at it when I can.
Comment by Ruben Petersen (Luckz) - Wednesday, 26 April 2006, 05:09 GMT
The discnumber tag is just a tag. So are disc title, album artist and so on. One could even just add generic support for other tags, no? %ic_'taggoeshere' or so.
Comment by Charles Voelger (rotundo) - Wednesday, 26 April 2006, 16:06 GMT
Well from what I saw while adding the discnumber stuff, all the tags have to have individual support in code. There is not a method where it reads them all and they can be dynamically used. Each one needs to be added to the code.
Comment by Markus Berndt (monnemer) - Wednesday, 16 August 2006, 19:38 GMT
I would find this patch extremely useful, since I have a lot of sets of discs. However, this patch does not work anymore with the latest CVS version. Is there a newer version of the patch that is compatible with what's in CVS? If not, I will make one an post it.
Comment by Phillip Michael (moufsnafa) - Friday, 20 October 2006, 17:22 GMT
I would also like this patch if it were compatible with the cuurent source code.
Comment by danhibiki (danhibiki) - Friday, 24 November 2006, 16:36 GMT
I've made a patch based on this and it seems to work well here... If anyone is interested in trying it, feel free to do so.
I've tested on my 4GB Nano and it worked fine, but I haven't tested on anything else (besides the simulator) and I don't guarantee it will work or that it will not cause any troubles.
The default behaviour is to show the tracknumber with discnumber in front of it, but when there is no discnumber tag (or when it is a string instead of a number), it shows one more 0... But at least it is now correctly sorted =)
It worked correctly with Vorbis, MP3 and MPC. I haven't tested anything else, but I think it will work with some other format too.
And if you want to change the syntax of TagCache config file, you can use "discnum" to deal with it.
Comment by Markus Berndt (monnemer) - Wednesday, 29 November 2006, 05:37 GMT
I just tried danhibiki's patch and it works well on my iRiver H120. Thanks a bunch!
Comment by Chris (decayed.cell) - Sunday, 28 January 2007, 01:33 GMT
Working(?) with 5.5G 30GB Video iPod - displays zero hmmm... maybe the logic should be changed to something like this:

Lets assume that the disc number tag is AB, where A is the first digit and B is the second digit

1. Read Disc number from ID3 tag, store in variable X
2. If X is greater than 9, display disc number as AB
3. If X is smaller than 10, display disc number as B
Comment by Andy Hawkins (adhawkins) - Wednesday, 14 February 2007, 10:35 GMT
I might be missing something here, but reading through the patch doesn't appear to include anything for sorting the playlist based on the disc number.

Is this automatic, or has this been missed?


Comment by danhibiki (danhibiki) - Wednesday, 14 February 2007, 18:15 GMT
Ok, just updated it to the current SVN.

Andy, you probably missed the change done in tagnavi.config, there you can see that it's now sorted by disc number first. The change in other files are to make rockbox understand the disc number tag.

I'm having trouble to compile the simulator with this patch, some problem with speex it seems (probably I will have the same error without the patch...), but it compiled fine for iPod Nano.
Comment by danhibiki (danhibiki) - Wednesday, 14 February 2007, 19:02 GMT
If you run into trouble compiling the simulator, you just need to re-run ../tools/configure, it did the trick for me.
Comment by danhibiki (danhibiki) - Thursday, 15 February 2007, 04:05 GMT
It seems that the patch is broken again, please don't use it, I'll take a look at it soon.
Comment by Andy Hawkins (adhawkins) - Thursday, 15 February 2007, 10:18 GMT
Can't come soon enough for me danhibiki :)

On an unrelated note, what is required for this patch to be accepted into the 'real' code (and hence provided in the daily builds)?
Comment by Alan F (alsaf) - Thursday, 15 February 2007, 15:13 GMT
I don't know if I'm talking about the wrong thing here but you can get disc numbers as a filter in tag navigation. The details are in the below rockbox forum topic.
Comment by Alan F (alsaf) - Thursday, 15 February 2007, 15:15 GMT
as per previous comment, the comment tag can be put in the wps to display disc number.
Comment by Andy Hawkins (adhawkins) - Thursday, 15 February 2007, 16:07 GMT
Yes, but there's already a disk number tag. And just displaying it in the WPS screen isn't enough it it? It needs to be used to sort items in a playlist.
Comment by danhibiki (danhibiki) - Friday, 23 February 2007, 19:18 GMT
Ok, it took a while because I was without access to a PC for a few days, but now it seems to be working fine with the latest SVN.

Andy, I have no idea about what it is required to this patch to be accepted, but I may join #rockbox someday and ask some developer with access to SVN. But for now you need to build your own Rockbox.
Comment by danhibiki (danhibiki) - Wednesday, 28 February 2007, 02:20 GMT
Updated to the current SVN.
Comment by David Gentzel (gentzel) - Thursday, 05 April 2007, 04:23 GMT
Commit of WPS tokenizer obviously broke this. Here's an update to the current SVN.
Comment by David Gentzel (gentzel) - Monday, 09 April 2007, 05:16 GMT
WPS code is still changing fairly rapidly. Another update.
Comment by Christoph Reiter (lazka) - Monday, 02 July 2007, 15:52 GMT
synced to current SVN.
Comment by Andy Hawkins (adhawkins) - Monday, 02 July 2007, 16:12 GMT
What chance of this getting added in to SVN? It's been around for well over a year now, and it's a major pain in the current operation.

Also, does it support getting disc numbers from VORBIS comments?


Comment by David Gentzel (gentzel) - Monday, 02 July 2007, 17:06 GMT
I'm certainly in favor of it getting into SVN. I doubt there's anything unpleasant enough about the code keeping it out, so the reason is probably either lack of general interest (or lack of someone lobbying hard enough) or the fact that it's presently ID3 only.
Comment by Christoph Reiter (lazka) - Monday, 02 July 2007, 17:42 GMT
same as above with tagviewer integration. (the "Show ID3 Info" thing)
Comment by Dan Everton (safetydan) - Saturday, 07 July 2007, 08:18 GMT
In rockbox/apps/gui/wps_parser.c part of the patch, what's the WPS_TOKEN_METADATA_TRACK_TITLE token about?
Comment by danhibiki (danhibiki) - Saturday, 07 July 2007, 18:33 GMT
I think it's a mistake, if you take a look at the first patch from David Gentzel it uses the correct token, but in the second one there is this weird WPS_TOKEN_METADATA_TRACK_TITLE.

Comment by Christoph Reiter (lazka) - Sunday, 08 July 2007, 17:15 GMT
@David Gentzel: It's not ID3 only. The only not-MP3 musicfiles i have are Vorbis and they work too.