Rockbox

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

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#9919 - lib/buffer_alloc support in pictureflow

Attached to Project: Rockbox
Opened by Andrew Mahone (Unhelpful) - Tuesday, 17 February 2009, 01:12 GMT+2
Last edited by Andrew Mahone (Unhelpful) - Wednesday, 04 March 2009, 22:26 GMT+2
Task Type Patches
Category Plugins
Status Closed
Assigned To No-one
Player Type All players
Severity Low
Priority Normal
Reported Version Version 3.1
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Private No

Details

Initial patch converting pictureflow to use buffer_alloc functions rather than rb->bufalloc. Works in sim, but many album covers are not drawn at all.
   pictureflow_buffer_alloc-200902161907.patch (3.4 KiB) 
 b/apps/plugins/pictureflow.c |   52 +++++++++++++++++++++++--------------------
 1 file changed, 28 insertions(+), 24 deletions(-)

Closed by  Andrew Mahone (Unhelpful)
Wednesday, 04 March 2009, 22:26 GMT+2
Reason for closing:  Accepted
Additional comments about closing:  committed as r20203
Comment by Andrew Mahone (Unhelpful) - Tuesday, 17 February 2009, 06:27 GMT+2
Fix a few checks for allocation failure, as buffer_alloc returns 0 for that, not -1. Also, check for valid handle before trying to free it - this apparently works with rb->bufclose, it does not with buf_free.
EDIT: and another replacement, since i failed at diff.
   pictureflow_buffer_alloc-200902171630.patch (4 KiB) 
 b/apps/plugins/pictureflow.c |   62 ++++++++++++++++++++-----------------------
 1 file changed, 30 insertions(+), 32 deletions(-)

Comment by Andrew Mahone (Unhelpful) - Wednesday, 18 February 2009, 07:01 GMT+2
Update data member of bitmap structs after fetching from buffer, free oldest *used* cache entry when loading new slide. need to remove assumption of fixed cache size, as this still eventually breaks when the freed slide doesn't provide enough space for the now one - we should free *until* we can load the new slide.
   pictureflow_buffer_alloc-200902180055.patch (5.7 KiB) 
 b/apps/plugins/pictureflow.c |   75 +++++++++++++++++++++----------------------
 1 file changed, 37 insertions(+), 38 deletions(-)

Comment by Andrew Mahone (Unhelpful) - Thursday, 19 February 2009, 10:20 GMT+2
Replaced slide cache with circular-linked-list-in-an-array, to simplify finding old slides, and clear oldest slide repeatedly until enough space is freed to load new one. Also reduced/optimized memory usage in some other places, dynamically allocating album title text cache from plugin buffer, and reducing the size of some integer values where it was reasonable. Works on my e200, though it can still hang occasionally (i suspect clearing items from LRU can get stuck sometimes, but haven't tracked down where/how yet), and the plugin buffer remaining after the plugin's code and static allocations is only large enough for about 8-9 covers.
   pictureflow_buffer_alloc-200902190409.patch (14.4 KiB) 
 b/apps/plugins/pictureflow.c |  249 +++++++++++++++++++++++++++----------------
 1 file changed, 159 insertions(+), 90 deletions(-)

Comment by Andrew Mahone (Unhelpful) - Sunday, 22 February 2009, 04:45 GMT+2
Redo/cleanup of linked-list implementation, and store a second linked list for the free slides, rather than having to search the array for one. Works well on all of my targets, but requires 1MB plugin buffer to hold even one screenful of cached slides on color targets, until we have a way to steal only a part of the audio buffer.
   pictureflow_buffer_alloc-200902212235.patch (16.3 KiB) 
 b/apps/plugins/pictureflow.c |  297 ++++++++++++++++++++++++++++---------------
 1 file changed, 198 insertions(+), 99 deletions(-)

Comment by Andrew Mahone (Unhelpful) - Thursday, 26 February 2009, 04:39 GMT+2
Removed a 256KiB static buffer, reducing the space used by static allocations and code to about 49KiB on ARM. This now works quite nicely on my beast with number of slides set to 3, higher can still cause some cache fighting, as LRU is not actually the smartest way to manage this cache, at least not if slides are touched every time they are fetched for display.
   pictureflow_buffer_alloc-200902252234.patch (17.2 KiB) 
 b/apps/plugins/pictureflow.c |  305 ++++++++++++++++++++++++++++---------------
 1 file changed, 203 insertions(+), 102 deletions(-)

Comment by Andrew Mahone (Unhelpful) - Friday, 27 February 2009, 07:40 GMT+2
Reduce album index to one long per album, fetch album names from tagcache as needed. album index, and a string buffer the length of the longest album are both allocated from the plugin buffer.
EDIT: fix some minor problems with the first patch uploaded
   pictureflow_buffer_alloc-200902270149.patch (18.8 KiB) 
 b/apps/plugins/pictureflow.c |  347 ++++++++++++++++++++++++++++---------------
 1 file changed, 226 insertions(+), 121 deletions(-)

Comment by Andrew Mahone (Unhelpful) - Wednesday, 04 March 2009, 07:18 GMT+2
Final (I think) version before ready to commit. Album titles and their index table are both allocated from plugin buffer, and a new "stable cache" is implemented, which keeps as many slides loaded near the current center slide as possible. Ties in priority are determined in fixed ways that should prevent any of the "cache fight" flickering seen when using LRU caching. Album names from tagcache on demand is reverted, as it made PF crawl on my e200, but can be revisited if better-performing code for the needed query comes along.
   pictureflow_buffer_alloc-200903040114.patch (29.7 KiB) 
 b/apps/plugins/pictureflow.c |  620 ++++++++++++++++++++++++++-----------------
 1 file changed, 381 insertions(+), 239 deletions(-)

Comment by Andrew Mahone (Unhelpful) - Wednesday, 04 March 2009, 12:35 GMT+2
Update to match renames in  FS#9916 
   pictureflow_buffer_alloc-200903040630.patch (29.8 KiB) 
 b/apps/plugins/pictureflow.c |  622 ++++++++++++++++++++++++++-----------------
 1 file changed, 383 insertions(+), 239 deletions(-)

Loading...