Rockbox

This is the bug/patch tracker for Rockbox. Click here for more information.

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

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

Attached to Project: Rockbox
Opened by Alejandro Arellano (Vague Rant) - Wednesday, 19 May 2010, 00:04 GMT+1
Last edited by Teruaki Kawashima (teru) - Sunday, 10 October 2010, 16:36 GMT+1
Task Type Bugs
Category Plugins
Status Closed
Assigned To No-one
Player Type All players
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
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, 16:36 GMT+1
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, 23:39 GMT+1
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, 04:23 GMT+1
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) 
 src/apps/plugins/lib/configfile.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comment by Chris Savery (csavery) - Friday, 21 May 2010, 10:32 GMT+1
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) 
 src/apps/plugins/pictureflow/pictureflow.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comment by Chris Savery (csavery) - Friday, 21 May 2010, 10:40 GMT+1
Here's a simple patch for the doc info in configfile.h
   config-doc.patch (0.5 KiB) 
 src/apps/plugins/lib/configfile.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Loading...