FS#9919 - lib/buffer_alloc support in pictureflow

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


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.
Closed by  Andrew Mahone (Unhelpful)
Wednesday, 04 March 2009, 21:26 GMT
Reason for closing:  Accepted
Additional comments about closing:  committed as r20203
Comment by Andrew Mahone (Unhelpful) - Tuesday, 17 February 2009, 05:27 GMT
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.
Comment by Andrew Mahone (Unhelpful) - Wednesday, 18 February 2009, 06:01 GMT
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.
Comment by Andrew Mahone (Unhelpful) - Thursday, 19 February 2009, 09:20 GMT
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.
Comment by Andrew Mahone (Unhelpful) - Sunday, 22 February 2009, 03:45 GMT
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.
Comment by Andrew Mahone (Unhelpful) - Thursday, 26 February 2009, 03:39 GMT
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.
Comment by Andrew Mahone (Unhelpful) - Friday, 27 February 2009, 06:40 GMT
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
Comment by Andrew Mahone (Unhelpful) - Wednesday, 04 March 2009, 06:18 GMT
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.
Comment by Andrew Mahone (Unhelpful) - Wednesday, 04 March 2009, 11:35 GMT
Update to match renames in  FS#9916