Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category Plugins
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version Version 3.1
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by Andrew Mahone - 2009-01-08
Last edited by Andrew Mahone - 2009-01-17

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

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.

Closed by  Andrew Mahone
2009-01-17 00:39
Reason for closing:  Accepted
Additional comments about closing:  

Committed as r19776

Andrew Mahone commented on 2009-01-10 10:53

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).

Jens Arnold commented on 2009-01-10 12:37

‘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.

Andrew Mahone commented on 2009-01-10 14:00

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

Michael Sevakis commented on 2009-01-10 22:16

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.

Andrew Mahone commented on 2009-01-10 22:38

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?

Andrew Mahone commented on 2009-01-11 17:55

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

Andrew Mahone commented on 2009-01-15 06:05

Resync, as align_buffer→ALIGN_BUFFER broke some things.

Andrew Mahone commented on 2009-01-16 01:27

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...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing