Skip to content

DaisySP integration#80

Merged
pingdynasty merged 9 commits intoRebelTechnology:developfrom
antisvin:feature/daisysp
Feb 10, 2021
Merged

DaisySP integration#80
pingdynasty merged 9 commits intoRebelTechnology:developfrom
antisvin:feature/daisysp

Conversation

@antisvin
Copy link
Copy Markdown
Collaborator

@antisvin antisvin commented Feb 2, 2021

It generally works. Patch header should contain the following:

#include "Patch.h"

#undef min
#undef max
#undef abs
#undef sin
#undef cos
#undef exp
#undef sqrt
#undef pow
#undef log
#undef log10

#include "daisysp.h"

using namespace daisysp;

DaisySP objects can be statically or dynamically allocated, then constructor should call .Init(getSampleRate()) for each of them.

@pingdynasty
Copy link
Copy Markdown
Collaborator

Got any example patches?

@antisvin
Copy link
Copy Markdown
Collaborator Author

antisvin commented Feb 2, 2021

Sure, I'll share a patch or two today.

@antisvin
Copy link
Copy Markdown
Collaborator Author

antisvin commented Feb 3, 2021

Example1, should look vaguely familiar :-) - https://github.com/antisvin/MyPatches/blob/master/DaisySP/KickBoxDPatch.hpp

It would be too demanding for OWL2, so I'll add something simpler

@pingdynasty
Copy link
Copy Markdown
Collaborator

Will need at least one good example which shows clearly how to use it, and which works on all (or most) devices.
Also does the submodule need to point to your fork? What are the differences that can't be pushed upstream?

@antisvin
Copy link
Copy Markdown
Collaborator Author

antisvin commented Feb 5, 2021

Some news:

  1. Resolved some issues with math, now it seems to run about twice as fast than before on F4
  2. Fixing math (and integrating our optimizations) requires overriding their "dsp.h" header which we can't do easily as library is using includes with double quotes. This causes local file to be used with higher priority than anything we can specify as -I or alternatives
  3. They've just released an update that completely changes modules layout, but at least files are group in directories that are easier to navigate. However this required extra updates to makefile for building everything - just solved recently
  4. The fork is required until they merge this or provide some other way for overriding dsp.h. If that won't happen, we can move this fork in your repo or add library source.
  5. Here's a more lightweight (but filthy!) patch here, I use it for testing on Magus

Comment thread compile.mk Outdated
Comment on lines +24 to +33
CPPFLAGS += -idirafter $(DAISYSP)
CPPFLAGS += -idirafter $(DAISYSP)/Control
CPPFLAGS += -idirafter $(DAISYSP)/Drums
CPPFLAGS += -idirafter $(DAISYSP)/Dynamics
CPPFLAGS += -idirafter $(DAISYSP)/Effects
CPPFLAGS += -idirafter $(DAISYSP)/Filters
CPPFLAGS += -idirafter $(DAISYSP)/Noise
CPPFLAGS += -idirafter $(DAISYSP)/PhysicalModeling
CPPFLAGS += -idirafter $(DAISYSP)/Synthesis
CPPFLAGS += -idirafter $(DAISYSP)/Utility
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you use -idirafter here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's due to their excellent taste in naming headers ;-)
electro-smith/DaisySP#137
So including it with -I causes wrong file getting used in basicmath.c and a compilation error due to memset not getting found there.

@antisvin
Copy link
Copy Markdown
Collaborator Author

antisvin commented Feb 9, 2021

Switched to use upstream repo as submodule, no problems with test patch

@pingdynasty pingdynasty merged commit 3172877 into RebelTechnology:develop Feb 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants