Rockbox

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

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#9665 - pictureflow resize-on-load

Attached to Project: Rockbox
Opened by Andrew Mahone (Unhelpful) - Thursday, 18 December 2008, 11:02 GMT+2
Last edited by Andrew Mahone (Unhelpful) - Saturday, 17 January 2009, 01:37 GMT+2
Task Type Patches
Category Plugins
Status Closed
Assigned To No-one
Player Type All players
Severity Low
Priority Normal
Reported Version Release 3.0
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

This is a first attempt at pictureflow resize-on-load using the new core scaler. It uses the BM_*_SIZE macros to calculate static buffer sizes, so you will need to apply the most recent scaler update patches from  FS#9458  for it to compile. Determining what size to scale to was difficult, and in the end I based it on the PREFERRED_IMG_* defines that pictureflow.c was previously using, since it was already looking for prescaled covers in those sizes. I've replaced the selection of 50 or 100 pixels with a size based on the LCD size, which should be small enough to allow other covers to be seen, even on portrait targets. The bottom edge of all images should also align now, and I replaced some divides with multiply/shift calculations instead.
   pictureflow-scaled-load-200812180447.patch (9.9 KiB) 
 b/apps/plugins/pictureflow.c |  139 +++++++++++--------------------------------
 b/tools/buildzip.pl          |    4 +
 2 files changed, 41 insertions(+), 102 deletions(-)

Closed by  Andrew Mahone (Unhelpful)
Saturday, 17 January 2009, 01:37 GMT+2
Reason for closing:  Accepted
Additional comments about closing:  superceded by work already committed in r19598
Comment by Andrew Mahone (Unhelpful) - Thursday, 18 December 2008, 11:03 GMT+2
Forgot to mention, since resize-on-load relies on loading an on-disk bitmap, the pictureflow empty slide image is now installed as .rockbox/rocks/demos/pictureflow_emptyslide.bmp
Comment by Thomas Martitz (kugel.) - Thursday, 18 December 2008, 13:18 GMT+2
PREFFERED_IMG_WIDTH/_HEIGHT are not applicable for scaling dimensions. I'd just say #define RESIZE_WIDTH/_HEIGHT LCD_HEIGHT/2 like I did it  FS#8335 .
Note that you directly influence the album art search (success rate) with changing PREFERRED_IMG_WIDTH/_HEIGHT as those are used for the size string in cover.<size>.bmp too at the search. I suppose leaving those as they are, and use other symbols for resizing target size.

ALso, it doesn't really make sense to search for covers that already have the target dimensions, does it? :)

Can you explain the LCD_WIDTH*10/32 part? I guess this should be for portrait targets, but 1/3 the screen width is pretty small (that's just 60 on a e200, which has 220 pixel rows). Just use LCD_HEIGHT/2. It's fine, I've been using that in  FS#8335  for a while now too and it looks great.
The key to see other covers is to change center margin (i.e. the space between the center and the side cover) in the settings, not reducing the imagine size. The default value is bad anyway, it's like miles away.
Comment by Andrew Mahone (Unhelpful) - Thursday, 18 December 2008, 13:56 GMT+2
My aim with the *10/32 was to hit the apparent "suggested" size of 100px on 320x240 landscape, but that seems really quite tiny. Ideally, I think that LCD_HEIGHT should be set as the maximum and default scaled size, and perhaps that the zoom % which adjusts the "distance" of covers from the camera should be replaced with an option to select the size of the largest square album art should be loaded as. It would probably not be a bad idea to replace the +/- margin with an actual pixel value, and set the default to something reasonable based on LCD_WIDTH.
As far as PREFERRED_HEIGHT and searching for covers by size, that should probably just go away. PF can't reasonably guess what size AA the user might have, and can't search for every possibility. Removing the size-based search would probably also get back a little bit of the binsize that resize-on-load cost.
Comment by Thomas Martitz (kugel.) - Thursday, 18 December 2008, 14:43 GMT+2
But keep in mind that "Zoom" can zoom without recreating the cache. Resizing needs recreation of the cache. I would mind losing that option.

I'd keep zoom, and add a bool option whether to resize or not upon the next cache creation. I guess there's people who don't want them resized in pf, or have bigger album art where resizing would just make them too small for them.
Comment by Andrew Mahone (Unhelpful) - Friday, 19 December 2008, 02:48 GMT+2
Removed MAX_* and PREFERRED_* entirely, as well as looking for a prescaled AA. Defined a now DISPLAY_SIZE as LCD_HEIGHT/2, and reduced default margin to make other covers more visible. Regarding making zoom change the scaled size, if I were to go that route, I feel that it would be necessary to make the cache rebuild automatically in the background, and working out from the currently centered album, instead of from the head of the list. I have no idea how to do that without making a horrible mess of it, so for right now, zoom stays as-is.
Comment by Andrew Mahone (Unhelpful) - Friday, 19 December 2008, 02:54 GMT+2
Correct buffer size calculation, it was twice as large as needed and too large for plugin buffer.
   pictureflow-scaled-load-200812182052.patch (10.9 KiB) 
 b/apps/plugins/pictureflow.c |  148 ++++++++++---------------------------------
 b/tools/buildzip.pl          |    4 +
 2 files changed, 41 insertions(+), 111 deletions(-)

Loading...