Rockbox

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

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#11678 - Make the playlist catalog use the normal file browser instead of a custom browser with crappy limits

Attached to Project: Rockbox
Opened by Jonathan Gordon (jdgordon) - Thursday, 14 October 2010, 15:29 GMT+2
Last edited by Teruaki Kawashima (teru) - Wednesday, 15 December 2010, 14:13 GMT+2
Task Type Bugs
Category Playlists
Status Closed
Assigned To No-one
Player Type All players
Severity Low
Priority Normal
Reported Version Release 3.6
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

inspisred by the fact that the playlist cataloge has a file limit http://forums.rockbox.org/index.php?topic=25972.0

This patch makes the playlist catalog use the normal file browser code to select and display a m3u[8].
The changes to tree.c are a bit hacky.. Ideally the context should be changed, but i think this is a good start. Also dirbrowse() really could use better return value handling.
   playlist_catalog.diff (12.3 KiB) 
 b/apps/filetree.c         |    2 
 b/apps/playlist_catalog.c |  223 +++-------------------------------------------
 b/apps/playlist_catalog.h |    2 
 b/apps/settings.h         |    2 
 b/apps/tree.c             |   26 ++++-
 b/apps/tree.h             |    1 
 6 files changed, 45 insertions(+), 211 deletions(-)

This task depends upon

Closed by  Teruaki Kawashima (teru)
Wednesday, 15 December 2010, 14:13 GMT+2
Reason for closing:  Out of Date
Additional comments about closing:  Superseded by  FS#11777 .
Comment by Teruaki Kawashima (teru) - Thursday, 14 October 2010, 17:20 GMT+2
i tried this, and it doesn't work at all in Main menu -> Playlists -> View Catalog.
in Main menu -> Playlists -> View Catalog:
- it also displayes bookmarks.
- if you simply press left, it just go to above directory instead of returning to Playlist menu.
- it doesn't say "No Playlists" when the playlist directory is empty.
- select playlist -> contexet menu -> add to playlist doesn't work.
- most_recent_playlist is no longer used.
- undefined reference to printf :).
i fixed these except most_recent_playlist thing.
   playlist_catalog.2.diff (12.4 KiB) 
 apps/playlist_catalog.h |    2 
 apps/tree.c             |   35 +++++--
 apps/tree.h             |    1 
 apps/settings.h         |    2 
 apps/filetree.c         |    2 
 apps/playlist_catalog.c |  220 +++---------------------------------------------
 6 files changed, 47 insertions(+), 215 deletions(-)

Comment by Jonathan Gordon (jdgordon) - Friday, 15 October 2010, 01:23 GMT+2
Thanks. Thinking about it more this morning, this patch is really nothing more than proof-of-concept. It really needs a bit of a dirbrowse()/tree.c rework to do properly.

the most_recent_playlist thing is an interesting one. The quick fix is getting a it from the browsers sort function and only use it when in this catalog browser. Or the proper way makes dirbrowse() *far* more complicated by passing in a sort callback, an action handler callback, and some way to get a better return value than the GO_TO_* values (we really want to know if the user canceled the screen, or exited because they loaded a file)

Or alternativly, we could ignore the sorting problem and just add another playlist catalog menu item, so along side "add to new playlist" and "add to playlist" we can have "add to: <last playlist name>" which would solve that issue and be the same amount of key presses
Comment by Teruaki Kawashima (teru) - Friday, 15 October 2010, 13:19 GMT+2
as for the most_recent_playlist thing, another option is selecting the last playlist by default when enter the playlist catalog similer to Fonts,WPS, etc. setting menus.
   playlist_catalog.3.diff (13.5 KiB) 
 apps/playlist_catalog.h |    3 
 apps/tree.c             |   48 +++++++---
 apps/tree.h             |    1 
 apps/settings.h         |    2 
 apps/filetree.c         |    2 
 apps/playlist_catalog.c |  226 ++++--------------------------------------------
 6 files changed, 65 insertions(+), 217 deletions(-)

Comment by Jonathan Gordon (jdgordon) - Saturday, 16 October 2010, 11:51 GMT+2
thats a much better idea than a new menu option :p
I'd almost say commit it but I tihnk it needs to wait till the release happens :/
Comment by Teruaki Kawashima (teru) - Saturday, 27 November 2010, 13:30 GMT+2
i made some improvements to rockbox_browse()  FS#11777 . how do you think?

Loading...