Rockbox

Tasklist

FS#11292 - PictureFlow doesn't respect "Show album title" setting on file.

Attached to Project: Rockbox
Opened by Alejandro Arellano (Vague Rant) - Tuesday, 18 May 2010, 22:04 GMT
Last edited by Teruaki Kawashima (teru) - Sunday, 10 October 2010, 14:36 GMT
Task Type Bugs
Category Plugins
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

PictureFlow's config reading/writing with regard to having album titles display at top seems to be broken. Setting album title to display at top only lasts until PictureFlow is next started, at which point it defaults to displaying at bottom again. "Hide" and "Bottom" settings both work as expected and are retained across launches. Manually altering the .cfg file does not have the desired effect either; "top" in here is simply ignored and art is once again displayed at bottom.
This task depends upon

Closed by  Teruaki Kawashima (teru)
Sunday, 10 October 2010, 14:36 GMT
Reason for closing:  Accepted
Additional comments about closing:  fix for album tite setting was already commited.
Accept fix for configfile.h.
Comment by Chris Savery (csavery) - Thursday, 20 May 2010, 21:39 GMT
I'm not seeing your behaviour here with Sansa Fuze v2.
The top setting is the default if your lcd is > 100 pixels high, as mine is. What is your lcd height?
I checked the config code and could not see anything that looked suspicious.
Comment by Chris Savery (csavery) - Friday, 21 May 2010, 02:23 GMT
I've verified this now. It only occurs when the default is bottom but you save top. If default is top and you save bottom it works ok.
The default depends on LCD_HEIGHT so device like the Clip+ have this problem.

It's caused by a bug in apps/plugins/lib/configfile.c that occurs when TYPE_ENUM is loaded with an array of value strings.
In this case it loops for j < max instead of j <= max. Since the value of max is taken from the last enum value it shouldn't be incremented in the config structure passed in. I think the loop should be changed.

I have here a patch but I don't know what other plugins may already be dependent on the buggy behaviour and that needs to be checked before this is accepted. Tested against r26185 in UISim for Clip+.
   config-1.patch (0.6 KiB)
Comment by Chris Savery (csavery) - Friday, 21 May 2010, 08:32 GMT
After checking the other plugins for the same issue I found that the general usage of the configdata structure is that TYPE_ENUM needs to have the number of enum values possible rather than the last value indicated. So this bug should be fixed by a patch to PictureFlow rather than a change to configfile.c.

I have add that the docs are incorrect in configfile.h - it says the values in the struct are for min and max values but in fact they are for min and count values. For values min=0, max=2 you need to enter 3 for max not 2.

I'm posting here a patch for pictureflow.c only and suggest the patch above not be used since other plugins will be affcted adversely.


   pictureflow-bug-title.patch (0.6 KiB)
Comment by Chris Savery (csavery) - Friday, 21 May 2010, 08:40 GMT
Here's a simple patch for the doc info in configfile.h
   config-doc.patch (0.5 KiB)

Loading...