Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Bugs
  • Category Plugins
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version Daily build (which?)
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by Vague Rant - 2010-05-18
Last edited by teru - 2010-10-10

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

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.

Closed by  teru
2010-10-10 14:36
Reason for closing:  Accepted
Additional comments about closing:   Warning: Undefined array key "typography" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 371 Warning: Undefined array key "camelcase" in /home/rockbox/flyspray/plugins/dokuwiki/inc/parserutils.php on line 407

fix for album tite setting was already commited.
Accept fix for configfile.h.

csavery commented on 2010-05-20 21:39

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.

csavery commented on 2010-05-21 02:23

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)
csavery commented on 2010-05-21 08:32

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)
csavery commented on 2010-05-21 08:40

Here’s a simple patch for the doc info in configfile.h

   config-doc.patch (0.5 KiB)

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing