Rockbox

Tasklist

FS#9770 - provide a loader-initialized global struct plugin_api

Attached to Project: Rockbox
Opened by Andrew Mahone (Unhelpful) - Thursday, 08 January 2009, 07:09 GMT
Last edited by Andrew Mahone (Unhelpful) - Saturday, 17 January 2009, 00:39 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

Details

This patch provides a global struct plugin_api *rb which is initialized by the loader. This is done by adding a declaration for rb to the PLUGIN_HEADER macro, and a pointer to rb to the plugin header structure, so that the loader can initialize rb before starting the plugin. All (I think) pluginlib functions and plugins are modified to use this global plugin API, including the overlay loader. I have not tested plugins exhaustively, but I have clean firmware buidls for recorderv2 and e200, with all plugins that I tried working on e200, and also a clean build with working plugins for ipod video sim.
This task depends upon

Closed by  Andrew Mahone (Unhelpful)
Saturday, 17 January 2009, 00:39 GMT
Reason for closing:  Accepted
Additional comments about closing:  Committed as r19776
Comment by Andrew Mahone (Unhelpful) - Saturday, 10 January 2009, 10:53 GMT
Resync, and remove lib/resize.h from the patch, since its deletion seems to confuse patch. Also move the api pointer init in the plugin loader up a bit, to make sure it's seen correctly by COP (per Jens Arnold).
Comment by Jens Arnold (amiconn) - Saturday, 10 January 2009, 12:37 GMT
'rb' must not reside in .bss for plugins which use IRAM, because .bss reuses the iram section, so it must not be touched before IRAM_INIT.
This potentially affects all plugins using IRAM on targets where iram usage is enabled for plugins.

I've added an updated patch that puts 'rb' into the data section.
Comment by Andrew Mahone (Unhelpful) - Saturday, 10 January 2009, 14:00 GMT
A slighly different solution. Works exactly the same as original patch on sim, but targets alias rb to the api field of the header, storing pointer to it directly.

pros:
saves 4B on target - not a big deal
this is what i thought "should" be done in the first place

cons:
slightly larger difference in target vs sim than before
Comment by Michael Sevakis (MikeS) - Saturday, 10 January 2009, 22:16 GMT
I'm all for saving bytes, but why bother with a different method on sim vs. target to save four bytes and make it pretty (it really isn't that bad)? Minimizing sim/target disparity would be a more worthy goal imho.
Comment by Andrew Mahone (Unhelpful) - Saturday, 10 January 2009, 22:38 GMT
Fair enough, I mostly did it to start getting some experience with the linker, anyway. We can continue with the declaration change to place it in .data. Any other suggestions? PLUGIN_IRAM_INIT can probably lose the parameter, as many other things have done already. the _WRAPPER macros could be replaced with wrappers in pluginlib, and maybe plugin_iram_init should go into pluginlib?
Comment by Andrew Mahone (Unhelpful) - Sunday, 11 January 2009, 17:55 GMT
Removed api argument from some macros, there should be nothing left that passes it. plugin_iram_init stays in core for now, Michael pointed out some problems with moving it, primarily that we'd need to force plugin rebuilds any time the needed interactions with core to stop audio and free the audio buffer change.

EDIT: removed deleted file apps/plugins/lib/resize.h from patch
Comment by Andrew Mahone (Unhelpful) - Thursday, 15 January 2009, 06:05 GMT
Resync, as align_buffer->ALIGN_BUFFER broke some things.
Comment by Andrew Mahone (Unhelpful) - Friday, 16 January 2009, 01:27 GMT
Fix CACHE_FUNCTION_WRAPPERS, they need to be redefined in plugins in case they were already defined via codec.h (mpegplayer had trouble with this).

Loading...