Rockbox

I don’t know the database at all but for all our current targets “long” is as large as “int” so you’ll have to use “long long” or an explicit type like int64_t

I think you rather want to search for short, and change that to int/long,

I have changed the shorts and some ints in apps/tag* is there somewhere else I should be looking?

I had a misunderstanding of how the simulator works. Now that I am clear on that. I put the code back to normal and started over. I then changed what I thought
would have been the most obvious line:
tagcache.c
170: short idx_id; to an int.

After compiling and running it will build the database (I erased the database files) when it commits it seems to add only to _4 and _idx. When I try to view
the database it looks like it is blank. Any ideas?

Try to change tag_length to int too, other than that, I have no real idea. Slasheri is the man to ask that.

Err, wait, you’ll need to change static const char *tagfile_entry_ec = “ss”; too. This is for endianess correction. The s is for short. You’ll want to use “l” for int/long/int32_t.

That’s the ticket.. I would not have seen that. I knew you could find it. :) I am rebuilding the db now to 32768+ to see how it goes. If that works then I will be adding the (your?) sort tag patch and testing that.. if that is all good then I will run all this joy on my real device… which I sadly just converted back to mp3s for the moment so I could use the oemOS until I could work out my rockbox woes.

MP3 isn’t too bad on an ipod, one of the best in terms of battery life time (it uses the 2nd core).

Hi Brian,

Please could you attach a patch of your database changes (and only those) at some point?

Shortest. Patch. Ever.
Here is a patch to up the number of files allowed in the db from 32k to more than you would ever want. This does not include anything to enable LBA48 in any targets or add a 4096 sector size like my hard drive needs. I mention this because the odds are if you need this patch you have a drive over 138GB in size.

   manyfile.patch (0.8 KiB)

And this works? You use “ll”, but still short and long in the struct.

Yes, seems to be fine. I have tested it in the simulator and on my device.
The only short in that struct is “short tag_length” which I believe has to do with the tags within any single file.
If that is correct then 32768 should be plenty enough.

Yea, but I image “sl” would be correct then.

riiiiight. I will change and test and send a new patch.

I put it to sl, rebuilt the database from scratch and its back to not displaying anything at all in the tag database. ‘ll’ seems to be the ticket as to how that relates to the short/int/char in the struct is a little beyond me. I have updated the patch though. It seems I should have put “int idx_id” not “long idx_id” the long makes the db look empty just like “sl” does. If anyone has some comments or something else you want me to test on this I will be glad to.

   manyfile_v2.patch (0.8 KiB)

Right, i was clearly not thinking right before, 32 bit should be Enough for Anyone ™ and yes int
is better than long as long is 64 bits on amd64 linux so it would mess up the sim for some of us.

About this ec thing thoug, a struct member is aligned to it’s size so there will be 2 bytes of padding
after the short now that the following member is an int so using “sl” will not work as then two bytes from
the int will be byteswapped with the padding bytes.
Changing the other short to int and using “ll” or placing the int before the short and using “ls” should work
in this respect, not sure if any other part of the database depends on member order or something like that though.

Ah right, due to alignment, the short will be the size of an int anyway (at least on most/some archs).

I toyed with a bunch of different combinations.. sl, ls (reversing the struct) etc… It looks like the only one that will work on a 64bit simulator and the iPod is the one in the v2 patch. int and ll.

I’d be interested in the impact, particularly with “Load to RAM” enabled. Given that the struct is now 12 bytes instead of 8, each tagcache entry (in each of the *.idx files) has added 4 bytes at least. You’re basically the first person I heard of experiencing this limit.

I do like to be an over achiever… I do have my db loaded into ram. I have 38057 oggs on my player at the moment. I also have the sort patch applied. Every Ogg has
Artist/Track/Album/Albumartist/albumsort/title/date filled out at the minimum. With all that going on the ramcache is right around 6MB according to the debug info and so far everything
seems quite snappy. My player is a 5.5G that was an 80gb so it has 64mb of ram. One other thing I noticed is my “Disk Info” looks find but the Disk Info under debug does not seem happy with the
LBA48. I might see if I can find a fix for that.

How big is the ram cache without this patch here?

Well… I tried to test that in the simulator… the database still seems to be around 6MB but when I try to enable ram cache on the simulator and reboot it just keeps seg faulting… which is a whole nother bag of worms I guess. I tried to build it on the sim and copy it to the ipod and it works fine until I turn on ramcache and then it wants to rebuild the db.. So I guess I would have rebuild it on the device in order to get an accurate reading. But let me make some speculation… The whole 38k gets built into the database*.tcd files with out and issue. It was only the reading of the later parts of the db that caused problems. I would assume the ramcache just puts the whole db in memory. That being said and that the db seems to be about 6MB either way.. there may not be much change. I guess if we assume 4 bytes for a library of 38k that would be about 148KB maybe times two on the high end for the index and all that. Since some players really need their ram and have no need for this number of files.. I think it would make sense for this to be a compile time option if it were to go into a release. I’ll get exact numbers maybe tonight. I just don’t feel like putting my pod out of commission right now to rebuild. Anyone else running Linux on AMD64 getting a sim crash with ramdb turned on?

Also of course.. anyone who did not need 38k files would have no where near the extra memory use.

Hm, I don’t think the sim is overly useful for this.

oh, and PS: I don’t think the 38k files properly get index’d.

On that point I guess the wise thing to do it take about 10k files on the device and index them with and without the patch. That way we don’t have to worry about what is happening to anything over 32768 in either scenario.

Well, you don’t need to delete files. You can also put some database_ignore files around.

Oh don’t worry that was the plan. :)

FWIW, the sim normally does work ok for database building, at least on a 32-bit system…

Good :)

No matter of the outcome of the RAM usage, I think the limit should at least be raised for high mem and tagcache-as-pc-app.

I used the sim to build my current DB. I think what is messing me up is I was trying to be lazy and build it completely offline with the files stored on my computer and then copy it to the device (the device is rsync’d from those files). But my working one I actually just pointed the sim to the mounted ipod.

If you’re building the database this way, it would be interesting if you compiled tagcache as standalone and use this then to build the db.

Yeah.. not real sure I know how to do that. Don’t see any obvious way to do that and I am not so much of a programmer.

make in tools/database should build it.

Oh right that.. I have played with that before. It’s pretty gimpy. heh

Ok here we go:
8713 Files

Pre-Patch:
1554060/1587296 Bytes
Post-Patch:
1667164/1700400 Bytes

Also remember I have the sort patch applied and all tracks were tagged with:
Artist/Track Number/Track Total/Album/Album Artist/Album Sort/Title/Date/Disc Number on multi disc sets.

So I guess it grew by 110KB or about 13 Bytes per song

IMHO, the database size increase is irrelevant. We need to fix this bug, and if this patch works, it should be committed. My only worry is that it may fail to handle existing database files. Is there a way to handle this incompatibility?

The database needs a version number, and the data should be treated as non-existant if out-of-date. I don’t know if we have such a field at the moment, but if not, this would be a good chance to introduce it.

Yes, it has a version number in tagcache.h. Just increase it and existing tagcache will be invalid.

Let’s see how irrelevant this increase really is :)

Great!

I’d suggest someone checks that that 64-bit systems are still happy with the sim (and database tool), then this should go in.

Let me suggest something.

#if MEMORY_SIZE ⇐ 2

33k files [or, if possible change to unsigned,

#else

2/4M files

#endif

I don’t really want to restrict (and I think on low mem they’re just as restricted too due to the ram usage increase, but probably less if we leave things unchanged for them) database user which depend on voice more, or something like that.

It works fine on the 64bit sim. The version number of the tag cache can be upped and I think I rebuild of the tagcache is just gonna have to happen. I expect we do have some targets that every little bit counts… we could def try to help them out by making this a choice. Of course also the smaller the target the less this would effect them anyway.

targets with less than 8 megs of ram don’t have ramcache at all so this would
only “waste” a couple kb’s of diskpace, i vote for consistency across targets
(this also helps in case people want to generate the db externally)

Hm, yea, you’re probably right. Let’s keep it consistent.

BTW: The clip has the database and 2MB of RAM.