Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category Plugins
  • Assigned To No-one
  • Operating System SW-codec
  • Severity Low
  • Priority Very Low
  • Reported Version Version 3.2
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by wincent - 2009-05-25
Last edited by petur - 2009-07-03

FS#10244 - Pure Data for Rockbox plugin - patch round 2

Patch pdbox-3-dbestfit-rockboxed.diff: dbestfit got rockboxed. printf-s were excluded using #ifdef DEBUG and #ifndef ROCKBOX if appropriate. Some additions to the signedness of variables were made. Most of changes were responses to the compiler warnings when compiling for the H300 DAP.

Patch pdbox-3-dbestfit-integrated.diff: Integration of dbestfit library both into the building process and into the pdbox plug-in.

Closed by  petur
2009-07-03 22:18
Reason for closing:  Accepted

m_memory.c from the PDa source tree compiles and works with simple tests.

Corrected some misconceptions in the pdbox.make file to let pdbox recompile upon a make reconf.

comitted.

Ported the functions in the s_print.c file, basically functions around printf.
Included this file into the SOURCES file.

I just took a short look at this patch.
- Is it correct that the functions in this patch do nothing on target ?
- In the target case, you do things like : “fmt=fmt;” is this to supress warnings ? if so, better use “(void)fmt;”

Your assumption that the functions in s_print.c are void on the target is almost correct.
Functions postatom and postfloat are still fully present on target.
I replaced the “fmt=fmt”. Shall I post the new diff?

if this should get commited, yes please post a new patch.

THIS PATH MAKES THE PREVIOUS ONE OBSOLETE!
IF YOU APPLIED THE PREVIOUS ONE – REVERSE IT!

(I hope capital letters are appropriate here.)

The same patch, where 1,$ s/fmt\ =\ fmt/(void)\ fmt/g was applied.

There where a few instances of the same x=x thing still left in the code.
I changed them, and commited it.

I proceeded the start sequence further, the patches made are attached.

Jesus that’s a lot of patches, Hope this works.

Dose this work on the iPod 5Gen?

petur commented on 2009-06-20 21:21

Wincent, is there a specific reason why you split the patch into one file for each modified file?

Actually, it was only to separate changes and so to ease the reviewing.
If it does not achieve it’s aim, I won’t do that again.

petur commented on 2009-06-21 00:20

We only split into multiple patches if the changes are not or less related, so that separate features or bugfixes can be committed….. So, no need to separate these ;)

I have compiled the startup sequence, and I don’t get a pdbox.rock on my 5th Gen. Is this supposed to be a plugin or am I just looking stupid?

@Gman: PDBox is still work in progress.I do not recommend to try to do something with it yet.

PDBox compiles now, all missing functions were replaced. I am doing audio output subsystem now.

ATTENTION: This patch contains the patches I posted previously and so renders them obsolete!

I just tried to compile this patch but the plugin is too big for the Plugin Buffer. :-)

A first look shows the really really big cosinus table. Is this really needed ? Maybe you could use a smaller table and interpolate inbetween.
Also cos_table.h misses the protection against multiple inclusions, and cos_table.c seems to include the wrong header.
And filters.h includes <math.h> Is this available in Rockbox ?

The rest of the changes look fine, from my first look at it.

About splitting changes into different patches:
This time it would have been good to make one with only the changes to suppress compiler warning, and another with the real changes needed.

This is quite strange, as I battled this problem specifically. As I found out, the cosine table was indeed included three times. This was the cause of relocating this table into the file cos_table.c . For now, everything compiles smoothly with a cross-compiler for H300. It also runs within a simulator.

As for math.h , it gets included for constant values (like M_PI) only; while rockbox does not include libm, the compiler includes it’s built-ins when working with floating-point values.

As for splitting changes, I am afraid that I am far ahead again, so for the next patch I will have to split it between suppressions for compiler warnings, floating point additions and sound additions. I hope they are separatable, because many changes might be made in the same file. Do you know by chance a tool that might help here?

I just looked at cos_table.c . It includes the only header it needs.

As requested I am splitting the big patch:

The attached file contains additional functions Rockbox does not have yet.

Additional information: It seems that pdbox.make produces wrong dependencies. Therefore, if you have compiled pdbox before, execute “make clean” or remove the pdbox object files before compiling anew after applying a patch or updating from SVN.

A big patch anew. As previous patches were not committed, this patch should be applied
without all the previous ones that were posted here!

Current status: a pd file gets loaded and a structure gets created from the contents
of the file (or many files, if another file has to be loaded into the first one).
Furthermore, the system begins to generate data. There is no sound output yet.

Warning: The patch still has debugging printfs!

Warning2: If you compiled pdbox before, remove it’s object files (with “make clean” or explicitely)
to recompile with new sources (something wrong in the pdbox.make file).

petur commented on 2009-07-01 20:17

Remarks (editing this entry as I go over the patch):
- remove the #defines that wrap the rb→ calls, and use apps/plugins/lib/wrappers.h

wrappers.h included, redundant #defines removed. Shall I make a ne diff and repost it?

New diff, suggestion incorporated.

petur commented on 2009-07-02 22:01

great, I’ll have a look at it Friday evening and commit it

petur commented on 2009-07-03 22:17

OK, committed, I’ll close this FR so you can start another one.

I also fixed some svn keyword issues and wrong line-endings. Please check that too next time you make a patch.

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing