Rockbox

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

Quick links: Bugs · Patches · Rockbox frontpage

Tasklist

FS#9916 - bufalloc-like memory allocator for use with plugin buffer

Attached to Project: Rockbox
Opened by Andrew Mahone (Unhelpful) - Monday, 16 February 2009, 21:28 GMT+2
Last edited by Andrew Mahone (Unhelpful) - Wednesday, 04 March 2009, 22:11 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

This patch adds a set of dynamic allocation routines to the pluginlib, with an API similar to rb->bufalloc.

cons:
not malloc-compatible

pros:
can compact
easy replacement for plugins using bufalloc
can be initialized to allocate from plugin buffer, or any other chunk of contiguous memory
   pluginlib_buffer_alloc-200902161522.patch (6.8 KiB) 
 b/apps/plugins/lib/SOURCES        |    1 
 b/apps/plugins/lib/buffer_alloc.c |  193 ++++++++++++++++++++++++++++++++++++++
 b/apps/plugins/lib/buffer_alloc.h |   40 +++++++
 3 files changed, 234 insertions(+)

This task depends upon

View Dependency Graph

This task blocks these from closing
 FS#9919 - lib/buffer_alloc support in pictureflow 
Closed by  Andrew Mahone (Unhelpful)
Wednesday, 04 March 2009, 22:11 GMT+2
Reason for closing:  Accepted
Additional comments about closing:  commited as r20202
Comment by Maurus Cuelenaere (mcuelenaere) - Monday, 16 February 2009, 22:54 GMT+2
You seem to have forgotten your attachment?
Comment by Michael Sevakis (MikeS) - Monday, 16 February 2009, 23:36 GMT+2
Yeah, has your desire to work on this turned into a feature request? Should I change the task type? :p
Comment by Michael Sevakis (MikeS) - Tuesday, 17 February 2009, 01:27 GMT+2
This indeed could be very useful for mpegplayer but would need to be dual-core safe since libmpeg2 will call it repeatedly. Perhaps an automatic buffer alignment switch instead of nastying up the code with calls? Perhaps a more thread/core-hardened extension layer could exist in the lib if an app needs that?
Comment by Andrew Mahone (Unhelpful) - Tuesday, 17 February 2009, 18:06 GMT+2
whitespace fixes and docs
   pluginlib_buffer_alloc-200902171205.patch (11.7 KiB) 
 b/apps/plugins/lib/SOURCES        |    1 
 b/apps/plugins/lib/buffer_alloc.c |  277 ++++++++++++++++++++++++++++++++++++++
 b/apps/plugins/lib/buffer_alloc.h |   40 +++++
 3 files changed, 318 insertions(+)

Comment by Andrew Mahone (Unhelpful) - Wednesday, 18 February 2009, 04:08 GMT+2
Track compact/sparse status of buffer (may be falsely marked sparse under some use patterns, should never be falsely marked compact), and base retries during allocation on this status, rather than always trying to compact once. Also fix a bug where compaction after allocating a handle, but before buffer allocation, could cause the handle table to be "shrunk" below the allocated handle. This could have resulted in a handle being issued twice, if the compaction caused the alloc to succeed on retry.
   pluginlib_buffer_alloc-200902172203.patch (11.9 KiB) 
 b/apps/plugins/lib/SOURCES        |    1 
 b/apps/plugins/lib/buffer_alloc.c |  284 ++++++++++++++++++++++++++++++++++++++
 b/apps/plugins/lib/buffer_alloc.h |   41 +++++
 3 files changed, 326 insertions(+)

Comment by Andrew Mahone (Unhelpful) - Wednesday, 18 February 2009, 06:57 GMT+2
Correct update of handle table entries during compaction, update first_free_block on completing compaction, both caused buffer corruption.
   pluginlib_buffer_alloc-200902180051.patch (12 KiB) 
 b/apps/plugins/lib/SOURCES        |    1 
 b/apps/plugins/lib/buffer_alloc.c |  285 ++++++++++++++++++++++++++++++++++++++
 b/apps/plugins/lib/buffer_alloc.h |   41 +++++
 3 files changed, 327 insertions(+)

Comment by Dave Chapman (linuxstb) - Wednesday, 04 March 2009, 11:14 GMT+2
What's the purpose of the global_context struct, and wrapping the API functions with macros that pass that into buffer_alloc.c ?

Couldn't that struct either be declared local to buffer_alloc.c (and hence remove the need for those macros and the extra parameter), or openly declared in the plugin using buffer_alloc.c ? Either of those would be cleaner IMO (unless I'm missing a reason for it to be the way it is - I've only looked at the patch briefly).
Comment by Andrew Mahone (Unhelpful) - Wednesday, 04 March 2009, 12:34 GMT+2
Per Dave's suggestions, the context is now always defined and passed explicitly by the caller. The _ctx function suffix is also removed, and the buf_ prefix is now buflib, and the files buflib.c/h to avoid confusion with core files or functions. Header comments and include guards have also been added.
   pluginlib_buffer_alloc-200903040630.patch (13.7 KiB) 
 b/apps/plugins/lib/SOURCES  |    1 
 b/apps/plugins/lib/buflib.c |  308 ++++++++++++++++++++++++++++++++++++++++++++
 b/apps/plugins/lib/buflib.h |   58 ++++++++
 3 files changed, 367 insertions(+)

Loading...