TonicAudio / Tonic

Easy and efficient audio synthesis in C++
The Unlicense
523 stars 63 forks source link

outsource DSP functions #253

Open andik opened 9 years ago

andik commented 9 years ago

Hey guys,

as I look into the various (control-)generators of Tonic, I see this #ifdef __APPLE__ ... #else ... #endif stuff all around, for example in the ADSR. I personally think this makes the code hard to comprehend and hard to maintain/adapt. I propose therefore a TonicDSP.h Headerfile which is used to abstract the DSP stuff away as much as possible...

for example (from ADSR)

namespace Tonic { 
  namespace dsp {
    inline void ramp(float start, float increment, float* buffer, int numSamples) {
      #ifdef USE_APPLE_ACCELERATE
        // starting point
        start += increment;
        // vector calculation
        vDSP_vramp(&start, &increment, buffer, 1, numSamples);
        // end point
        start += increment*(numSamples-1);
      #else
        for (int i=0; i<numSamples; i++){
          start += increment;
          *buffer++ = start;
        }
      #endif
    }
  }
}

another option would be macros:

#ifdef USE_APPLE_ACCELERATE
  #define TONIC_DSP_RAMP(start, increment, buffer, numSamples) \
    vDSP_vramp(&start, &increment, buffer, 1, numSamples);
#else
  #define TONIC_DSP_RAMP(start, increment, buffer, numSamples) \
    for (int i=0; i<numSamples; i++){                          \
      start += increment;                                      \
      *buffer++ = start;                                       \
    }
#endif

I prefer the first.

one could create that header, mark it as DSP Port Header and add the DSP Functions whenever one finds one. This way nearly no immediate efford is required. And it would also help to port Tonic to new platforms...

andik commented 9 years ago

If you like, I'd create the file and start to port the stuff. Is mostly a matter of grepping for vDSP and outsource this all. BUT I do not know the Apple DSP functions, thus you'd have to look over my source before merging (which you will do anyway). Or you give me some tipps (using const float references as parameters maybe?)

morganpackard commented 9 years ago

This sounds like a good idea to me. @ndonald2, what do you think? I'd rather not introduce another layer of function lookup overhead (even though I know the effect will be fairly negligible). I know macros won't introduce any function overhead. I suspect that any functions defined in a dsp header would be inlined, but I don't know for sure.

ndonald2 commented 9 years ago

Yes, this is a great idea. I had started down this road awhile back but never saw it through. Another good option might be to have multiple .cpp implementation files for the core DSP header that each use platform macros to compile the whole thing or nothing.

i.e.

// TonicDSP_apple.cpp
#ifdef USE_APPLE_ACCELERATE
#include "TonicDSP.h"
...
// All the implementations for the DSP functions
...
#endif

and so on.

andik commented 9 years ago

@morganpackard I'm not a native english speaker so I'm not shure if I understood you correctly. Did you mean that the compiler is free to not inline functions marked with inline? I heard that this is possible. But in my working place we inline a lot to have high performance and at least the DSP forwarding functions are so small they definitively get inlined. I'm not shure for the naive (for-loop) implementations. But there are so much calls, it doesn't matter ^^ I'd especcially go for inline functions because they are easier to use by the end-user and they raise better errors. Using macros was my first Idea, but I'd prefer not to use them.

@ndonald2 this is a good idea, except that we'd loose inlining, which may be not problem for the naive (for-loop) implementation of the functions.

So maybe we could use something like

// TonicDSP.h
#ifdef USE_APPLE_ACCELERATE
#include "TonicDSP_Apple.h"
...
// other Systems
...
#else
#include "TonicDSP_Naive.h"
#endif
morganpackard commented 9 years ago

Supercollider uses something like this, that has been factored out into a standalone library. We could take a look at that, but it could mean breaking the "no dependencies" rule. Also could have license implications.

On Wed, Jan 28, 2015 at 3:55 PM, Nick Donaldson notifications@github.com wrote:

Yes, this is a great idea. I had started down this road awhile back but never saw it through. Another good option might be to have multiple .cpp implementation files for the core DSP header that each use platform macros to compile the whole thing or nothing.

i.e.

// TonicDSP_apple.cpp

ifdef USE_APPLE_ACCELERATE

include "TonicDSP.h"

...// All the implementations for the DSP functions ...

endif

and so on.

Reply to this email directly or view it on GitHub https://github.com/TonicAudio/Tonic/issues/253#issuecomment-71913896.

Morgan Packard cell: (720) 891-0122 twitter: @morganpackard

morganpackard commented 9 years ago

@andik I meant that (as I understand it) the compiler may inline functions even if they aren't marked inline. I'd vote for header-only. The function call will have a tiny impact on performance, but I'd rather eliminate that tiny impact if there's no strong reason not to.

On Wed, Jan 28, 2015 at 4:39 PM, andik notifications@github.com wrote:

@morganpackard https://github.com/morganpackard I'm not a native english speaker so I'm not shure if I understood you correctly. Did you mean that the compiler is free to not inline functions marked with inline? I heard that this is possible. But in my working place we inline a lot to have high performance and at least the DSP forwarding functions are so small they definitively get inlined. I'm not shure for the naive (for-loop) implementations. But there are so much calls, it doesn't matter ^^ I'd especcially go for inline functions because they are easier to use by the end-user and they raise better errors. Using macros was my first Idea, but I'd prefer not to use them.

@ndonald2 https://github.com/ndonald2 this is a good idea, except that we'd loose inlining, which may be not problem for the naive (for-loop) implementation of the functions.

So maybe we could use something like

// TonicDSP.h

ifdef USE_APPLE_ACCELERATE

include "TonicDSP_Apple.h"

...// other Systems ...

else

include "TonicDSP_Naive.h"

endif

Reply to this email directly or view it on GitHub https://github.com/TonicAudio/Tonic/issues/253#issuecomment-71921706.

Morgan Packard cell: (720) 891-0122 twitter: @morganpackard

ndonald2 commented 9 years ago

@morganpackard I was wrong in ever worrying about the overhead, it's negligible. Certain wizard-like engineers at a particular audio company that I have connections with have shared certain information about whether or not inlining such functions is important. It's definitely not. Turns out the compiler tends to inline such tiny functions anyway when optimizing.