FS#11808 - playlist handling changes
Attached to Project:
Rockbox
Opened by Jonathan Gordon (jdgordon) - Monday, 13 December 2010, 11:05 GMT
Last edited by Jonathan Gordon (jdgordon) - Wednesday, 20 July 2011, 14:14 GMT
Opened by Jonathan Gordon (jdgordon) - Monday, 13 December 2010, 11:05 GMT
Last edited by Jonathan Gordon (jdgordon) - Wednesday, 20 July 2011, 14:14 GMT
|
Detailschanges discussed on the maling list:
1) main menu > playlists loads the playlist catalog 2) running a playlist opens it like a browser 3)... other stuff i cant remember |
This task depends upon
Closed by Jonathan Gordon (jdgordon)
Wednesday, 20 July 2011, 14:14 GMT
Reason for closing: Accepted
Additional comments about closing: in r30176 \o/
Wednesday, 20 July 2011, 14:14 GMT
Reason for closing: Accepted
Additional comments about closing: in r30176 \o/
ok so whats changed?
1) main menu > playlists loads the playlist catalog
2) running a playlist opens it like a browser
3) playlist viewer settings menu has moved to "main menu > settings > general > playlists"
4) playlist viewer context menu has changed so it has "playlist, playlist catalog, remove, move" items (the first two are the same as file/db browser, need to change the icons but that isnt so easy)
5) fix a very UI-crap buig where you could endlessly go in a playlist catalog loop.
to be closer to the db/file browsers it should auto-exit to WPS when a new playlist is started, but for now this is good enough (big code changes for that)
When Hotkey is pressed on WPS I get "no files" message box.
long press Select > Playlist Catalog > Add To Playlist
Shows the playlists in the the root of the player ("/"), instead of the ones in "/playlists" dir
long press Select > Playlist Catalog > Add To New Playlist
The default filename has way to many forward slashes, e.g. "/playlists/////////////////////".
* fix typo: onlplay_show_playlist_cat_menu => onplay_show_playlist_cat_menu.
* make onplay_show_playlist_menu() actually work.
* add "save current playlist" item to context menu of playlist viewer.
bugs and comments.
* bookmark isn't resumed when starting a playlist from playlist viewer (when configured to do so of course).
* changing use of GO_TO_PLAYLIST_VIEWER might not be good idea as it is used in %cs (current screen) tag and thus it might break backward compatibility of skin. i don't know if that part in %cs is actually used though.
it causes bug that William reported, WPS Hotkey displays playlists in the playlist catalog directory.
* it should go to WPS after starting playback from playlist viewer.
it looks strange that the list is shuffled in place after starting playback when Shuffle setting is turned on.
iirc, there was an idea to not view playlist in file browser. is it rejected?
It might be nice if there is playlist-related settings in context menu of root menu but i'm not sure.
How about to have setting to set default behaviour, view or start the playlist when it is selected?
* bookmark isn't resumed when starting a playlist from playlist viewer (when configured to do so of course).
how do bookmarks handle .m3u playlists in svn?
* changing use of GO_TO_PLAYLIST_VIEWER might not be good idea as it is used in %cs (current screen) tag and thus it might break backward compatibility of skin. i don't know if that part in %cs is actually used though.
it causes bug that William reported, WPS Hotkey displays playlists in the playlist catalog directory.
Yeah, what does it do in svn? we don't want this to break anything, merely fix gaping holes.
* it should go to WPS after starting playback from playlist viewer.
yes, it should be fixed for this
* it looks strange that the list is shuffled in place after starting playback when Shuffle setting is turned on.
That is what should happen. When you start playback from the viewer it reloads the playlist viewer to show the new playlist which should be shuffled.. this is correct.
* iirc, there was an idea to not view playlist in file browser. is it rejected?
There was a bit of sentiment against this idea but I think it makes sense. This is what started this discussion anyway. m3u files should be considered like a folder in the file browser. This will mean 1 extra button click for a few people but that is worth it.
Please no more settings.
Thanks for syncing and those fixes/.
> how do bookmarks handle .m3u playlists in svn?
if Load Last Bookmark is set to yes, resume the first entry of the bookmark when the playlist is selected.
it should work similar when selected an audio file in the file browser: if there is a bookmark and Load Last Bookmark is set to yes, resume the bookmark and so on.
> * changing use of GO_TO_PLAYLIST_VIEWER might not be good idea as it is used in %cs (current screen) tag and thus it might break backward compatibility of skin. i don't know if that part in %cs is actually used though.
> it causes bug that William reported, WPS Hotkey displays playlists in the playlist catalog directory.
> Yeah, what does it do in svn? we don't want this to break anything, merely fix gaping holes.
it display the plalist currently being played as the name implies. i'm not entirely sure why you changed this to display plalist catalog.
> This will mean 1 extra button click for a few people but that is worth it.
>
> Please no more settings.
I donot understand why you often mention button click, but well, I feel it is more than 1 extra button. it needs to load file first (and doesn't go to WPS immediately after starting playback) i found it slightly annoying.
i against to commit this unless
- bookmark and shuffle setting works correctly.
- go to WPS after starting playback.
# or the action is configurable.
have a read through of the mailing list discussion.the extra button click is the only downside for some people (the file needs to be loaded anyway), but if it opens as the default action then it opens up a heap of other uses which just cant be done right now.
* changing the action to view playlist doesn't add new functionality which can't be done right now.
at most, it saves some clicks for people who wants to view playlist often, from my point of view.
* personally, I think it is better to separate the patch to two. one is adding Playlst and Playlist Catalog to playlist viewer context menu, the other is changing the default action when playlist is selected.
* as for loading, difference is loading happens before the click to start playback. below is what happens when i want to start a playlist.
in svn: select playlist > load playlist(spin up disk) > playback starts
with the change: select playlist > load playlist(spin up disk) > select track > load playlist(disk is already spinning, so there won't be notable delay) > playback starts
it might be nothing to speek of...sorry for an useless comment.
testers badly wanted :)
edit: try again if the playlist viewer doesnt work from the main menu... reuploading patch
I think this is close to commitable, just need to work out what the issue with bookmarking is and any other issues we want to fix
save all playlists in the catalog (except dirplay), rename /root.m3u8 to /Playlists/all.m3u8.
not sure how to handle dirplay saves. currently if you dirplay /Music/foo/bar it ends up as /Music/foo/bar.m3u8 with absolute paths in the file so should it be /Playlists/foo - bar.m3u8 or something else? or leave it where it is?
Would it be possible to make playlist catalog more functional *without* removing existing functionality in the process?
Maybe it would be better to first fix up the Playlist Catalog so it picked up playlists from anywhere first? (Kind of a database browser for playlist files...) Then come back to this?
This change will do nothing for people who dont care about the catalog already. you are of course free to put playlists anywhere you like and view them from the filebrowser. What this does do is make it easier for people who want to use the catalog.
I mentioned using the db to index .m3u8 files in IRC and was shot down pretty convincingly but perhaps a lua script could be added to move all playlists into there if you wanted to do that.
edit: Oh, St. says that the above patch may not be what I expected it to be. if the "Playlists" menu item doesnt drop you in the filebrowser in the catalog (or say "no files") then dont worry too much about testing that version
1) I very much like the "treat playlists as subdirectories" vibe
2) I think it should it go the WPS when I start playback from a playlist, rather than the file browser.
- It would be nice if saved playlists were fully treated as subdirs / fully integrated into the browser as a list of links - Follow Playlists could work in side playlists then!
- There was a discussion about having both a "go into" (RIGHT) and a "go into and play" (SELECT) option where keymaps allow. Any conclusion on this?
1) :)
2) that is fixed (should have been in the above patch
3) not sure how that would work... and this patch is getting big enough already, maybe later
4) no conclusion, maybe later :)
edit: wtf? this patch which is a new diff from origin/master seems to be the same... it defintly works here and was uploaded from the correct place 6bcf3118512f5cb25528e0d3f323d4ad is the md5sum of the patch i tihnk im uploading!