Rockbox

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 !

here is the patch with a %ik wps tag.

   discnumber2.patch (7 KiB)

Should this possibly be merged with http://www.rockbox.org/tracker/task/2974 ?

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.

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.

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.

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.

I would also like this patch if it were compatible with the cuurent source code.

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
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
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
worked correctly with Vorbis, MP3 and MPC. I haven’t tested anything else, but I think it will work with some other format
if you want to change the syntax of TagCache config file, you can use “discnum” to deal with it.

   disc.patch (8.1 KiB)

I just tried danhibiki’s patch and it works well on my iRiver H120. Thanks a bunch!

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
If X is greater than 9, display disc number as
If X is smaller than 10, display disc number as B

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?

Thanks

Andy

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.

   discsvn20070214.patch (7.5 KiB)

If you run into trouble compiling the simulator, you just need to re-run ../tools/configure, it did the trick for me.

It seems that the patch is broken again, please don’t use it, I’ll take a look at it soon.

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)?

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.

http://forums.rockbox.org/index.php?topic=8723.0

as per previous comment, the comment tag can be put in the wps to display disc number.

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.

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.

   discsvn20070223.patch (7.9 KiB)

Updated to the current SVN.

   discsvn20070227.patch (8.2 KiB)

Commit of WPS tokenizer obviously broke this. Here’s an update to the current SVN.

   discsvn20070405.patch (9.6 KiB)

WPS code is still changing fairly rapidly. Another update.

   discsvn20070409.patch (9.6 KiB)

synced to current SVN.

   discsvn20070702.patch (10.1 KiB)

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?

Cheers

Andy

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.

same as above with tagviewer integration. (the “Show ID3 Info” thing)

   discsvn20070702_2.patch (13.7 KiB)

In rockbox/apps/gui/wps_parser.c part of the patch, what’s the WPS_TOKEN_METADATA_TRACK_TITLE token about?

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.

I created a new patch with WPS_TOKEN_METADATA_DISC instead of WPS_TOKEN_METADATA_TRACK_TITLE.

   discsvn20070707.patch (13.7 KiB)

@David Gentzel: It’s not ID3 only. The only not-MP3 musicfiles i have are Vorbis and they work too.