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 Balin - 2009-05-25
Last edited by Peter D'Hoye - 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  Peter D'Hoye
2009-07-03 22:18
Reason for closing:  Accepted
Wincent Balin commented on 2009-05-27 11:10

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

Wincent Balin commented on 2009-06-01 20:31

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

Dominik Wenger commented on 2009-06-02 20:24

comitted.

Wincent Balin commented on 2009-06-07 14:37

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

Dominik Wenger commented on 2009-06-07 15:00

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;”

Wincent Balin commented on 2009-06-07 15:32

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?

Dominik Wenger commented on 2009-06-07 15:39

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

Wincent Balin commented on 2009-06-07 15:55

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.

Dominik Wenger commented on 2009-06-07 19:15

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

Wincent Balin commented on 2009-06-13 16:27

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

Gman commented on 2009-06-16 08:39

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

Gman commented on 2009-06-16 20:17

Dose this work on the iPod 5Gen?

Peter D'Hoye 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?

Wincent Balin commented on 2009-06-20 22:27

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.

Peter D'Hoye 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 ;)

Gman commented on 2009-06-21 02:36

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?

Wincent Balin commented on 2009-06-25 23:28

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

Wincent Balin commented on 2009-06-25 23:30

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

Wincent Balin commented on 2009-06-25 23:31

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

Dominik Wenger commented on 2009-06-27 20:59

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.

Wincent Balin commented on 2009-06-28 22:30

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?

Wincent Balin commented on 2009-06-28 22:32

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

Wincent Balin commented on 2009-06-29 22:00

As requested I am splitting the big patch:

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

Wincent Balin commented on 2009-06-29 22:09

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.

Wincent Balin commented on 2009-07-01 09:46

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

Peter D'Hoye 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

Wincent Balin commented on 2009-07-02 03:20

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

Wincent Balin commented on 2009-07-02 15:52

New diff, suggestion incorporated.

Peter D'Hoye commented on 2009-07-02 22:01

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

Peter D'Hoye 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