"Add To Playlist" patch

[hardeep]

Hi all,

I had some time to look into this patch and have done a massive rewrite.  Please try out the new patch and see if there any problems.  Updated patch can be found at http://www.rockbox.org/tracker/task/5179.  Tracker comments duplicated here:

I've done a massive rewrite of this patch to fix most of the lingering issues. Changes include:

1. All playlists must now be in one directory. The default directory is /Playlists but this can be changed by creating the file /.rockbox/playlist_catalog_config and adding the line "dir:/my/playlist/path name". All related add to/remove from playlist catalog options have been removed from the gui.

2. Only the most recently modified playlist is remembered and placed at the top of the list. All other playlists are ordered based on the file tree configuration.

3. Removed most of the duplicated code when inserting directories (required modifications to playlist code as well)

4. All onplay functionality is available when "View playlists" is selected.

One change I didn't make was to move to the gui_synclist_* functions when browsing the playlist. This is mainly because the menu code is working reasonably well. However, this may still be changed. Aside from this, there's still a little cleanup and documentation that needs to be done. Once that's complete, this patch should be ready for inclusion into the builds (post 3.0 of course).

NOTE: When applying the patch you no longer need to manually copy the playlist_catalag.* files, they are now a part of the patch itself.

-Hardeep

[Assimalyst]

Wow, sounds great Hardeep. Thanks. 

I'll test it as soon as I a get chance.

[lenny]

Wow hardeep! Greatly appreciated.

Everything works splendidly. I know Linus said he didn't think that the directory should be configurable, and to some extent I agree, but if it is going to be configurable I configured it via /.rockbox/playlist_catalog_config and I think that's a simple and unobtrusive way to do it. The only place I saw room for improvement was that in the "Playlist Catalog" menu the "View" option is not available when in the directories, only when you are in the WPS.

Thanks for your time and skills. You're awesome.

[Assimalyst]

Been giving the new patch a test, seems to work excellently.

The only place I saw room for improvement was that in the "Playlist Catalog" menu the "View" option is not available when in the directories, only when you are in the WPS.

As regards the 'view' option I quite like the functionality as is. I don't think it needs to be accessible from directory view because you can always pretty quickly navigate directly to the /playlists folder. Also it doesn't require an extra down navigation to 'Add to Playlist' when you're potentially adding numerous files to the catalog form directory view.

just my 0.0000000000000002c 

Great work, roll on 3.0 release so we can get this in CVS!! 

[dunno]

Hardeep, thanks for implementing this feature, much aprreciated. I eagerly await it's inclusion after the feature freeze.

[Assimalyst]

Just a thought. Might be a good idea to add some way of accessing these playlists through TagCache file view. 

[hardeep]

The only place I saw room for improvement was that in the "Playlist Catalog" menu the "View" option is not available when in the directories, only when you are in the WPS.

Just a thought. Might be a good idea to add some way of accessing these playlists through TagCache file view. 

In general, the idea behind the tree/id3 view context menu is to only show options applicable to the selected file which is why I didn't add the "View" option there.  You can always view the playlists from Menu->Playlist Options->Playlist catalog.

Thanks for your feedback.  Please let me know if you find any bugs.

-Hardeep

[hardeep]

Updated patch posted on tracker to sync with latest CVS.  I've also cleaned up and documented the code.  The one visible change is that the config file is now called /.rockbox/playlist_catalog.config to be consistent with the other config files.  Barring any problems/complaints, this should be the final patch.

-Hardeep

[dunno]

I have been fiddling with the feature and I'm not sure that I am doing it right, these are the issues that I'm having when adding a single track to the catalog file,

1). When I play that playlist-catalog with a single track it gives the following errors,
i) playlist buffer full
ii) error in accessing playlist file
iii) wps opens and shows no file

It adds and plays full albums/folders that I add to playlist catalogs OK.


p.s.

if I restart the player after it displays the 'playlist buffer full' error it works fine with single tracks, my playlist buffer is set to 1000 which should be more than enough, I dont have other playlists whilst testing this feature.

[hardeep]

I have been fiddling with the feature and I'm not sure that I am doing it right, these are the issues that I'm having when adding a single track to the catalog file,

1). When I play that playlist-catalog with a single track it gives the following errors,
i) playlist buffer full
ii) error in accessing playlist file
iii) wps opens and shows no file

I can reproduce this problem and I think it may be related to the directory cache.  Do you have that feature enabled?  If so, does the problem still happen when you disable it?

-Hardeep

[hardeep]

I have been fiddling with the feature and I'm not sure that I am doing it right, these are the issues that I'm having when adding a single track to the catalog file,

1). When I play that playlist-catalog with a single track it gives the following errors,
i) playlist buffer full
ii) error in accessing playlist file
iii) wps opens and shows no file

I can reproduce this problem and I think it may be related to the directory cache.  Do you have that feature enabled?  If so, does the problem still happen when you disable it?

I believe I've fixed the problem in the latest CVS build.  It was unrelated to this patch.

-Hardeep

[Assimalyst]

Just wanted to express my appreciation for this excellent patch. Been using it frequently and have yet to encounter any bugs.

Thanks 

[dunno]

Hardeep, hmm not sure this is a bug

whilst playing something from a catalog and in wps,

i), press joystick in for wps context menu
ii), select 'playlist catalog' > 'view' menu, navigate to a catalog to view its contents.

when you 'select' the catalog to'view' it immediately starts to play the contents of that catalog, shouldn't this be more like a browser feature, i.e. one can browse the contents of the various catalogs without it automatically playing the contents ?.

perhaps i'm not understanding the view concept ?.....

p.s.
the previous bug is gone, no more errors.

Thanks for you time and seffort.

[Assimalyst]

Not sure how feasable it would be but how about scanning the playlist before adding a file to it to avoid duplications.

I'm getting a bit old and can never remember if i've added a track or not.   

I know they can always be reasonably easily removed, but i would certainly find this feature useful, provided it does not have too great a detriment on performance.

Thanks

[Llorean]

I dunno, I have playlists that I intentionally put duplicates in. I like to think of it as weighting for shuffle. The more I want a song to show up, the more I put it in a large playlist, so that I don't only hear it once per cycle of a 1000 entry list.