Rockbox

This is the bug/patch tracker for Rockbox. Click here for more information.

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#12474 - [PATCH] Database: Support for multiple search roots.

Attached to Project: Rockbox
Opened by Thomas Martitz (kugel.) - Wednesday, 21 December 2011, 20:52 GMT+2
Task Type Patches
Category Database
Status New
Assigned To Thomas Martitz (kugel.)
Player Type All players
Severity Low
Priority Normal
Reported Version Release 3.9
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Private No

Details

The setting works similar to the autoresume dirs: Directories are seperated
by colon, e.g. "/Music:/Podcasts". Default is "/sdcard" on android, "/" on
all other targets.

A maximum of 8 dirs can be selected, the setting cannot be longer than 80 chars.

Note: There's one problem. If you specifiy "/Music:/" (or any folder and one if its parents afterwards), then /Music is scanned twice. It's tricky to fix, and I'm not sure if it's worth it. This does not happen for "/:/Music" as it can be detected more easily in this order (and this is already implemented).
   0001-Database-Support-for-multiple-search-roots.p... (11.5 KiB) 
 b/apps/lang/english.lang     |   14 ++++++
 b/apps/menus/settings_menu.c |   23 ++++++++++
 b/apps/misc.c                |   31 ++++++++++++++
 b/apps/misc.h                |    1 
 b/apps/settings.h            |    1 
 b/apps/settings_list.c       |    8 +++
 b/apps/tagcache.c            |   92 ++++++++++++++++++++++++++++++++-----------
 b/apps/tagcache.h            |    2 
 8 files changed, 147 insertions(+), 25 deletions(-)

This task depends upon

Comment by Alexander Levin (fml2) - Thursday, 22 December 2011, 09:43 GMT+2
I do not use the databse but I think it's a very nice feature and should be committed.
Comment by Jonathan Gordon (jdgordon) - Wednesday, 28 December 2011, 02:19 GMT+2
merge my ui patch, not really tested and diff has extra crud.... laptop battery going to run out before i can finish today
   changes.diff (46.1 KiB) 
 b/apps/SOURCES                                |    1 
 b/apps/gui/bitmap/list.c                      |   30 +
 b/apps/gui/tagcache_config.c                  |  392 ++++++++++++++++
 b/apps/lang/english.lang                      |   14 
 b/apps/menus/settings_menu.c                  |   22 
 b/apps/misc.c                                 |   31 +
 b/apps/misc.h                                 |    1 
 b/apps/plugins/random_folder_advance_config.c |  602 +++++++++++++-------------
 b/apps/tagcache.c                             |  173 ++++++-
 b/apps/tagcache.h                             |    2 
 10 files changed, 938 insertions(+), 330 deletions(-)

Comment by Jonathan Gordon (jdgordon) - Wednesday, 28 December 2011, 07:14 GMT+2
better patch, my end is done, looks like your tagcache.c changes need a bit of cleaning up.

Last thing thats needed on the ui side is to fix skin lists to do indenting (not trivial so too hard for my current extremely overtired state)
   changes.diff (23.1 KiB) 
 b/apps/SOURCES               |    1 
 b/apps/gui/bitmap/list.c     |   30 +++
 b/apps/gui/tagcache_config.c |  392 +++++++++++++++++++++++++++++++++++++++++++
 b/apps/lang/english.lang     |   14 +
 b/apps/menus/settings_menu.c |   22 ++
 b/apps/root_menu.c           |    6 
 b/apps/tagcache.c            |  111 +++++++++---
 b/apps/tagcache.h            |    5 
 8 files changed, 552 insertions(+), 29 deletions(-)

Comment by Thomas Martitz (kugel.) - Wednesday, 28 December 2011, 23:44 GMT+2
Thanks for porting the UI!

Which of my tagcache.c changes are you refering to? You seem to have added some which need cleaning up (the if (ignore) closedir()..., and #if 0 stuff).

A few other remarks:
a) IMO the search roots shouldn't be a separate file. It's a setting after all.
b) Shouldn't your new fancy UI be more generic and not a tagcache specific file (going by the file name)?
c) fast_readline looks wrong buggy. The callback is not called for every line, but for the first one of each read (the size of which depends on the buffer passed to it).

I haven't actually tested yet, just looked at the diff. Will try out later
Comment by Jonathan Gordon (jdgordon) - Thursday, 29 December 2011, 00:12 GMT+2
a) if you're talking about tagcache_config.c then no, it should stay seperate, 1) it makes it easier to becomee generic later, and 2) its 400 lines which isnt relelvant to anything else
b) deal with that later if anything else wants to use it
c) ?


ah, maybe i didnt merge our ptches cleanly, I'll go through it again if you say those changes arent yours.
Comment by Thomas Martitz (kugel.) - Thursday, 29 December 2011, 00:20 GMT+2
a) I meant the TAGCACHE_SEARCHROOTS file where the dirs are saved to with.
b) should be easy enough to do now. The tagcache specific part can move to tagcache.c. Isn't that only one function anyway, the rest being completely generic? My fear is that nothing is going to want to use it because it's quite hidden behind a file name that suggests being quite tagcache-specific (devs just don't notice).
c): The fast_readline() function looks bogus to me, as if the callback isn't actually called for each line of the file. It's one call to the callback per read(). One read() can get multiple lines at once, though.
Comment by Jonathan Gordon (jdgordon) - Thursday, 29 December 2011, 00:23 GMT+2
a) ah, well that gives more flexibility than using global_settings (and wastes less ram doing so), the disk needs to be spinning when the db is built anyway so no penalty there.
b) meh, nothing will use it because nothing else needs to really. if someone wants to split it later they can
c) I honestly have no idea what you're saying, if fast_readling() is broken its not new in this patch.
Comment by Thomas Martitz (kugel.) - Thursday, 29 December 2011, 08:13 GMT+2
a) I don't see how it gives more flexibility. But I guess your code is already done so it assumes one item per line as you often use readline().
b) I'm sure autoresume wants it
c) yes, not new in this patch. It just looks wrong to me unrelated of this patch.
Comment by Jonathan Gordon (jdgordon) - Thursday, 29 December 2011, 08:44 GMT+2
fast_readline is correct.. the read() call will continue to fail once it hits the end of the file but the loop wont exit until if ( (p = strchr(buf, '\n')) != NULL) fails also.
Comment by Thomas Martitz (kugel.) - Wednesday, 18 July 2012, 10:47 GMT+2
Here's an update. It takes jdgordon's work, but removes the extra configuration file and shows the root folder ("/") so that the entire filesystem is selectable.
   0001-Configurable-database-scan-directories.patch (28.7 KiB) 
 b/apps/SOURCES               |    1 
 b/apps/gui/bitmap/list.c     |   30 ++
 b/apps/gui/tagcache_config.c |  454 +++++++++++++++++++++++++++++++++++++++++++
 b/apps/lang/english.lang     |   14 +
 b/apps/menus/settings_menu.c |   22 +-
 b/apps/misc.c                |   31 ++
 b/apps/misc.h                |    1 
 b/apps/root_menu.c           |    4 
 b/apps/settings.h            |    1 
 b/apps/settings_list.c       |    8 
 b/apps/tagcache.c            |   95 ++++++--
 b/apps/tagcache.h            |    3 
 12 files changed, 636 insertions(+), 28 deletions(-)

Loading...