Rockbox

Ok, next version. This should be ready to commit. I've tested it deeply on my e200. I've added an exception for smaller screens (height < 100) like c200's, though I think pictureflow should be more optimized for small screens in general.


Overall improvement concering the tracklist:
*full screen tracklist if both album titles and fps are hidden
*move the tracklist if album titles and/or fps are shown, including reducing the number of visible items

Just fixed a few typos in comments, and added one. No functional change.

Here's the file.

No fuctional change. Made an enum for show_album title for easier-to-read-code, some changes to the comments.

Sync and implement smooth_resize. Problem: It doesn't resize.

Here's the patch.

Ok, I got resize to work. Doing this, I also introduced a setting to let the user decide if covers should be resized (default is yes). Like before, on color targets, smooth resize is used (on other simple resize).

Secondly, I changed a bit how PREFERRED_IMG_HEIGHT/_WIDTH are calculated. It's now dependent on the screen size instead of a fixed size (e.g. a Gigabeat F/X defaults to 150 height and width). Also, this values will be used for resizing.
NOTE: Resizing will not keep the aspect ratio. If this behavior isn't wanted, please comment on that.

Thirdly, I fixed (or at least worked around) the wrong order of tracks in the tracks list ( FS#8425 ). It's maybe a bit dirty, but I noticed that the tagcache search for the track titles returned the results in the wrong order. My fix is to "invert" the track index when getting the names from the index.

This patch also moves show_fps into the config struct.

This patch also fixes  FS#8347  by forcing the track list to reset upon selecting the album.

The only tracker entry which doesn't get fixed is FS#8927 :) ( FS#8326  doesn't seem to happen anymore)


If there are no objections anymore I'm done with this patch, it should be ready for commit. I hope I didn't change too much at once. If I did I could consider splitting it up a bit.

Hold on, after some talk with Slasheri, the track search isn't reliable at all (in terms of ordering). So, my fix is as worse as the original implemntation.

Slashery said the inverted order I get comes most likely from the filesystem which returns the files in some order. I have named my tracks to have the track number leading, so they are if they are ordered by name they are also ordered by track number.

So, this simply removes the attempt to order the tracklist correctly.

More needs to be done to give it an order. Until then the tracklist is more or less random.

I'm not even sure if the tracklist was intended to be ordered.

Sync, also fix a bug introduced r17781 (1 track missing in the tack list).

Sync, also fix a bug introduced r17781 (1 track missing in the tack list).

Ok, I've noticed the covers were a bit too small on a screen like the one of the h300.

I changed PREFERRED_IMG_WIDTH and _HEIGHT to LCD_HEIGHT / 2 now. This should be fine with the committed smooth scaling. Also, it only uses this when resizing is enabled.

this patch still doesnt work for me.... even after removing the cache folder

It'd be very nice if people could give some feedback on this patch. jdgordon seems to have problems using this patch, while it works just fine for me (e200 sim+target, h300 sim).

e200

- If the track list is taller than the screen the last song title gets cut in half.
- You can zoom way too far in so that the album titles and FPS are mixed with the art. User's choice, I suppose but maybe worth mentionning.

Aside from that it seems fine to me. Required a delete of the rocks/pictureflow folder.

Marc, thanks a lot for your feedback. It's nice to hear that it generally works for you, since I just can't reproduce jdgordons problems.

Regarding your issues:
- I can't confirm that. I've tried some albums with 15+ songs, so that I need to scroll in the track list. The last song was fine in all cases.
- Yea, probably a users choice. I don't see a need to restrict the zoom function in anyway.
- Correct, the pictureflow folder needs to be deleted, because pictureflow uses a rather uncommon way of saving & loading config. Literally all changes to the config struct make existing config files break pictureflow.

ok, i said id have another look and i did...
the only thing i've changed is so it looks for cover.100x100.bmp if the preffered size isnt found, 88x88 isnt exactly a "standard" size, the cache is made but it still says 0 albums...

jdgordon, the problem is that create_albumart_cache() calls create_bmp() and both functions change and use config.avg_album_width in different ways if resize is enabled. v10 should fix this. now the plugin runs fine on my iPodColor :-)
kugel., thanks for this patch, i like the album titles on top ;-)

Nice. Seems to work well!

JdGordon, I've tried to reproduce your problem. I've got something:
You said it only found a few covers, the debug you showed me said it only found 10 out of much more albums. http://pastebin.ca/1091783

My tests with having a cover.120x120.bmp and a cover.100x100.bmp in each of my 7 test music folders were successful. It found every cover.
I also ran a test with having a cover.bmp in a few of the folders, which also have been successful.

I did my tests with e200 and h300 sim.

that was the point of the pastebin... showing a few or showing 10 is the same when I have hundreds on the disk it should have found....

jdgordon, is your problem caused by this patch or does it occur in the unpatched plugin too?

I think we should split this patch to separate the fixes introduced in v5 ( FS#8347 ) and v7 (1 track missing in the tack list) from the improvements. The fixes are ready to commit IMHO.

In pf_album_title_v10.fixes.diff I replaced the reset_track_list() call by a simple "start_index_track_list = 0;", which does the job too.

Should we open a new bug report just for the fixes (or reopen  FS#8347 ) to get them committed ASAP?

Forgot to mention:  FS#8347  is closed (Reason: "Fixed: about to ci a fix") but the bug is still alive in r18359 :-(

Well, I'd like any version to be committed since I have no problems with the resizing.

But I don't have a problem with committing a version without resizing (and without the PREFFERED_IMG stuff rewritten). I just would want to use the config struct from v10, so that a later commit of resizing would break the config again (which in turns means the pf folder needs to be deleted).

As Marc_Guay mentioned there are still some open issues with the tracklist on some screensize/fontsize combinations, e.g. on iPodColor with standard font and a tracklist that contains 13 tracks (see attached screenshot).

This is fixed in this patch.

Sure about this? The above screenshot is from r18359 with pf_album_title_v10.diff applied.

Well, it should be fixed since v2, look at the notes there. Either something reindroduced it, or I...don't know.

I just know, I made the tracklist move up when the album title is at the bottom.

New Version:
- fixed (and slightly changed) calculation of tracklist position
- moved the calculation to reset_track_list(), so it's done only once and not in every show_track_list() loop
- revert my replacing of reset_track_list() in PICTUREFLOW_SELECT_ALBUM handling (cause recalculation may also be needed when returning from the settings menu)

Tested on iPodColor and all screen sizes (in sim) with different fontsize/settings combinations.

(Fixes from v5 and v7 are included.)