Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category Operating System/Drivers
  • Assigned To No-one
  • Operating System All players
  • Severity Low
  • Priority Very Low
  • Reported Version Daily build (which?)
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by Nils Wallménius - 2007-06-07
Last edited by Nils Wallménius - 2007-06-13

FS#7264 - Build with -Os switch for coldfire targets.

This patch has a few different pieces.

0) Change the configure script to enable the -Os switch for m68k-elf-gcc.

1) It adds wrapper functions for memcpy and memset to some plugins to allow gcc to use those functions when optimizing.

2) Some small changes to playback_control to avoid memcpy here.

3) In playback.c change voicebuf to be unsigned char and adapt callbacks to this and make them accept the size parameter as size_t.

Both these changes were to avoid casting which made gcc throw a warning about dereferencing type-punned pointers.

Size drops about 20kb for rockbox.iriver for h300 builds.
I tested a build on my player and everything works as far as i can see.

Test built for a number of other targets without errors too.

I would like if some playback guru out there could take a look at this to see if my changes seem ok, i’m not very confident with changing playback.c and mp3_playback.c…

I tried to build with -O2 too but it fails on IRAM_FULL for the nsf codec.

   os.diff (12.5 KiB)
Closed by  Nils Wallménius
2007-06-13 15:36
Reason for closing:  Accepted
Nils Wallménius commented on 2007-06-07 11:35

hmmm, forgot to bump plugin api version, that can be done when (if?) this gets committed.

Nils Wallménius commented on 2007-06-10 19:10

new patch,
* change the voice_info struct to have the same types as voicebuf and the size parameters to get rid of some uglyness

* bumped plugin api

Would really like to get someone else to test this, both on coldfire and archos targets
and some plaback.c guru to take a quick look at the changes in there too.

   os2.diff (13.2 KiB)
Nils Wallménius commented on 2007-06-11 14:31

* Moved the wrapper functions to a macro called MEM_FUNCTION_WRAPPERS(api)

in a new file apps/plugins/lib/mem_function_wrappers.h 
and put this in every plugin that needed it.
   os3.diff (16.2 KiB)
Magnus Holmgren commented on 2007-06-11 16:53

Have you tried putting the wrappers in the codec lib?

Nils Wallménius commented on 2007-06-11 20:59

Magnus, Yes I tried that and it worked, but there was one nasty thing. I had to call an
init function to point the api pointer correctly (like in playback_control.c for example).
Doing that worked fine but if I did not do it the linker didn't complain because the plugins
are linked against everything in the plugin lib and the plugins just froze when they were run.

The bad thing is that gcc decides itself which plugins should use the mem* functions and some
arbitrary change might trigger it which then will cause the plugin to freeze, not nice.
A workaround would be to add the init call to every plugin but I thought this was nicer.
I would like to hear suggestions on how to make it better though

Nils Wallménius commented on 2007-06-12 15:26

hmm, I wanted to try another idea, to add the mem* functions themselves to the pluginlib, that did not work however.
I added

../../../firmware/common/memcpy.c
../../../firmware/common/memset.c

to the end of apps/plugins/lib/SOURCES

and everything compiled fine but plugins using any of the two functions froze when started…

as a test I also copied the .c files to the lib dir and changed SOURCES accordingly but with the same result.

Is it worth it to continue trying to make that work or should we go with one of the other ways?

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing