Rockbox

Tasklist

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

Attached to Project: Rockbox
Opened by Wincent Balin (wincent) - Monday, 25 May 2009, 21:20 GMT
Last edited by Peter D'Hoye (petur) - Friday, 03 July 2009, 22:18 GMT
Task Type Patches
Category Plugins
Status Closed
Assigned To No-one
Operating System SW-codec
Severity Low
Priority Normal
Reported Version Version 3.2
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

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.
This task depends upon

Closed by  Peter D'Hoye (petur)
Friday, 03 July 2009, 22:18 GMT
Reason for closing:  Accepted
Comment by Wincent Balin (wincent) - Wednesday, 27 May 2009, 11:10 GMT
m_memory.c from the PDa source tree compiles and works with simple tests.
Comment by Wincent Balin (wincent) - Monday, 01 June 2009, 20:31 GMT
Corrected some misconceptions in the pdbox.make file to let pdbox recompile upon a make reconf.
Comment by Dominik Wenger (Domonoky) - Tuesday, 02 June 2009, 20:24 GMT
comitted.
Comment by Wincent Balin (wincent) - Sunday, 07 June 2009, 14:37 GMT
Ported the functions in the s_print.c file, basically functions around printf.
Included this file into the SOURCES file.
Comment by Dominik Wenger (Domonoky) - Sunday, 07 June 2009, 15:00 GMT
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;"
Comment by Wincent Balin (wincent) - Sunday, 07 June 2009, 15:32 GMT
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?
Comment by Dominik Wenger (Domonoky) - Sunday, 07 June 2009, 15:39 GMT
if this should get commited, yes please post a new patch.
Comment by Wincent Balin (wincent) - Sunday, 07 June 2009, 15:55 GMT
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.
Comment by Dominik Wenger (Domonoky) - Sunday, 07 June 2009, 19:15 GMT
There where a few instances of the same x=x thing still left in the code.
I changed them, and commited it.
Comment by Wincent Balin (wincent) - Saturday, 13 June 2009, 16:27 GMT
I proceeded the start sequence further, the patches made are attached.
Comment by Wincent Balin (wincent) - Saturday, 13 June 2009, 16:31 GMT
... to this comment.
Comment by Gman (Thecoolgman) - Tuesday, 16 June 2009, 08:39 GMT
Jesus that's a lot of patches, Hope this works.
Comment by Gman (Thecoolgman) - Tuesday, 16 June 2009, 20:17 GMT
Dose this work on the iPod 5Gen?
Comment by Peter D'Hoye (petur) - Saturday, 20 June 2009, 21:21 GMT
Wincent, is there a specific reason why you split the patch into one file for each modified file?
Comment by Wincent Balin (wincent) - Saturday, 20 June 2009, 22:27 GMT
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.
Comment by Peter D'Hoye (petur) - Sunday, 21 June 2009, 00:20 GMT
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 ;)
Comment by Gman (Thecoolgman) - Sunday, 21 June 2009, 02:36 GMT
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?
Comment by Wincent Balin (wincent) - Thursday, 25 June 2009, 23:28 GMT
@Gman: PDBox is still work in progress.I do not recommend to try to do something with it yet.
Comment by Wincent Balin (wincent) - Thursday, 25 June 2009, 23:30 GMT
PDBox compiles now, all missing functions were replaced. I am doing audio output subsystem now.
Comment by Wincent Balin (wincent) - Thursday, 25 June 2009, 23:31 GMT
ATTENTION: This patch contains the patches I posted previously and so renders them obsolete!
Comment by Dominik Wenger (Domonoky) - Saturday, 27 June 2009, 20:59 GMT
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.
Comment by Wincent Balin (wincent) - Sunday, 28 June 2009, 22:30 GMT
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?
Comment by Wincent Balin (wincent) - Sunday, 28 June 2009, 22:32 GMT
I just looked at cos_table.c . It includes the only header it needs.
Comment by Wincent Balin (wincent) - Monday, 29 June 2009, 22:00 GMT
As requested I am splitting the big patch:

The attached file contains additional functions Rockbox does not have yet.
Comment by Wincent Balin (wincent) - Monday, 29 June 2009, 22:09 GMT
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.
Comment by Wincent Balin (wincent) - Wednesday, 01 July 2009, 09:46 GMT
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).
Comment by Peter D'Hoye (petur) - Wednesday, 01 July 2009, 20:17 GMT
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
Comment by Wincent Balin (wincent) - Thursday, 02 July 2009, 03:20 GMT
wrappers.h included, redundant #defines removed. Shall I make a ne diff and repost it?
Comment by Wincent Balin (wincent) - Thursday, 02 July 2009, 15:52 GMT
New diff, suggestion incorporated.
Comment by Peter D'Hoye (petur) - Thursday, 02 July 2009, 22:01 GMT
great, I'll have a look at it Friday evening and commit it
Comment by Peter D'Hoye (petur) - Friday, 03 July 2009, 22:17 GMT
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...