Rockbox

Here is the patch that works against current versions of apps/playlist_viewer.c

   playlist_viewer_0.6.patch (0.6 KiB)

This is an update to the above and perhaps an improvement. I added an item to the playlist context menu for “Tag Data”. So if you are viewing a playlist and hit “Menu” then “Track Display”, you have 3 options: “Track Name Only”,”Full Path”, and “Tag Data”. The latter extracts the Data from tags to put in the display instead of using filenames. As with the FS6084 patch, you must first
* enable TagCache (the database)
* configure it to load to RAM
* wait for it to finish loading

As with FS6084 and per related discussions, this can be slow and a bit of a resource hog. That said, it seems to work well enough for me as my tracks do have good tag data. Navigration within a large playlist (scrolling) is rather slow. Again, I expected this based on other discussion related to FS6084 and feature requests in Flyspray. Also, I only provided an English translation for the setting item. I’m not really a C coder but I can occassionally fix small things. This patch is mostly useful to IPOD users who still like to use the built-in firmware. With the recent patches to fix the power consumption issues on IPOD, there are really very few reasons (in my opinion) to use the built-in firmware. It’s fairly trivial to use something like EasyTag to rename all the files on the IPOD if you decide to use rockbox exclusively and pretty much negating the usefulness of this patch.

   playlist_viewer_0.7.patch (2.2 KiB)

Hi,

i tried both versions, none of them works unfortunately. First (0.6) compiles fine but doesn’t do anything on my 3rd Gen. 0.7 patches (or maybe it doesn’t patch completely) but doesn’t compile. I guess something is wrong with the lang part of the file (sorry I’m no C coder).

On the .6 patch, what did you do on the IPOD? It’s not automatic. You have to enable an option. specifically the context menu in the playlist viewer you would select “Track Display→Full Path” with that version of the patch and it will replace the filename contents with the ID3 details. You also need to have dircache set up and loading to RAM enable before the view is altered.

On the 0.7 patch, I did mess it up. This 0.8 should be fixed. I grabbed a fresh copy from svn trunk and applied only this patch and compiled. Thanks for trying this out and letting me know that I had it messed up!

sharn90@cctuwsehlaptop:~/source/rockbox$ svn co svn://svn.rockbox.org/rockbox/trunk rockbox/apps
simulator/common/sound.h

(big snip)

A rockbox/apps/uisimulator/battery.c
Checked out revision 16287.
sharn90@cctuwsehlaptop:~/source/rockbox/apps$ patch -p0 < ../../playlist_viewer_0.8.patch
patching file apps/settings_list.c
patching file apps/playlist_viewer.c
patching file apps/lang/english.lang
sharn90@cctuwsehlaptop:~/source/rockbox/apps$

I then compiled and built it without error. I went ahead a built it for 3G and also tested in a 3G uisimulator.

   playlist_viewer_0.8.patch (2.3 KiB)

Great job on this patch, it works. I think it’s quite useful not only for the iPod but for other targets as well.

BTW, you were right on the lag, it is some lag there but it isn’t very annoying. IMO, the only thing that prevents this patch from committing is the lag.

Once again, good job.

I don’t notice the lag so much with smaller playlists, but if I do shuffle every track in the IPOD (2500+ in my case) scrolling through the list is laggy, but certainly still usable. I’m sure there’s a better way to do it, but I don’t really have handle on how the tag database works so I’m not sure if it’s possible to optimize it for this.

For whatever reason, the uisimulator doesn’t show the tag data correctly in the playlist view, but it’s fine on the actual ipod.

We’ll see if anybody else interested in it. I posted about it in a thread for tdtooke’s underground build http://forums.rockbox.org/index.php?topic=12742.0 . I just took his set of patches and applied them to current svn. then I took the patch from 6084 and I figured out where it had gone wrong. Once I got it working like it used to, I decided to see if I could add a third playlist view option and leave the original two options intact.

Are you planning to FIX the problems that got the last task closed, because opening a new task just to resync a problematic patch is unacceptable.

Well this is probably an inherently sub-optimal way of doing it. But without actually trying it and testing it out as I have, I had no way of understanding/knowing that. And some users still find this useful/interesting. And perhaps someone else will look at this and figure out a smarter solution. There’s no comment in the original entry as to why or even if it was in fact rejected (as opposed to just abandoned).

Note that I did make substantive changes to the way the original 6408 patch operated, actually adding an appropriate settings entry rather than merely overriding the existing “Full path” display option.

I am currently looking at FS7652 which currently reads EXTM3U formatted playlists. If we added the ability to write dynamic playlists with id3 data in EXTM3U format, it may be a better long term solution than this patch. When you create or update the playlist, the files would be accessed to extract the tags but they would not be consulted (from the cache) each time you viewed the playlist. Also, it may be desirable to have the control file hold the id3 data (optionally?) though this would mean a format change for that. I haven’t had much luck with either approach thus far, because I’m not much of a C coder but at least I’m trying.

I do see several requests both in FS and forums to use ID3 tags for the playlist viewer display. This patch does accomplish that though doing it in the best possible way requires a pretty deep understanding of the guts of how playlists are managed within rockbox that I don’t fully have a handle on yet. But if I can make it better, I will. And if this scratches someone else’s itch and they can do it quicker/better/smarter than that would be fine too.

This tracker isn’t for “Some users”. It’s for “patches that are being developed for the intent of inclusion.” By reposting it, I believed you were intending to fix the problems that got it closed before. If you are not, I’m closing it again. If it needs a better solution, then start a new patch making use of the better solution, or work on an existing patch like the EXTM3U one , which is what I intended to suggest as more suitable anyway.

But please, in the future, if a patch is closed for multiple reasons, don’t restart it unless you intend to address all of the reasons.

PS, the original was closed with the comment: “Additional comments about closing: and probably would be rejected anyway if it makes the viewer slow”, and that slowness is what I refer to as needing resolved.