Polyphonic MIDI synth example

Dewb commented 11 years ago

I plan to take a stab at this sometime. I'm thinking a basic extension of the StandaloneTonicDemo sample with RtMidi and code for a simple voice allocator added.

ndonald2 commented 11 years ago

Sounds cool!

If it makes sense to abstract the voice manager into its own BufferFiller subclass that could be pretty useful. Unfortunately there's not currently a way to copy a synth since we can't walk the generator graph, but you could probably template it and do it with a synth subclass.

Dewb commented 11 years ago

I went ahead and whipped up the simplest possible strategy that occurred to me: https://github.com/Dewb/Tonic/tree/midi/Demo/Standalone/PolyMIDIDemo

Comments on how to make this more idiomatic and useful are definitely welcome!

Issues:

It will eventually crash in ControlGenerator::_tick() inside of ControlMidiToFreq()::computeOutput(), probably because I'm doing something dumb. Part of my motivation here was to start understanding Tonic better from a user perspective, so this is a good learning opportunity.
When actively playing notes, CPU usage is around 4% for a simple 8-voice synth. When I stop playing, though, usage goes up to 13%! Any thoughts? Might be the fault of the RtMIDI or Windows event loop.
The Max poly~ object is smart enough to not even run DSP for subpatches that are not playing notes. Is that currently possible in Tonic? Would it work to add and remove sub-synths from the mixer as voices go active and inactive?
Is there a way to get a signal when an ADSR finishes its release cycle? This would be necessary for properly disabling DSP for a voice, but it would also be useful so I could prefer voices that were not still releasing over voices that were.

ndonald2 commented 11 years ago

Sweet!

A few comments

It will eventually crash in ControlGenerator::_tick() inside of ControlMidiToFreq()::computeOutput()

I believe this can be attributed to the way you are using ControlMidiToFreq in VoiceData. This line is not thread safe:

pVoice->frequency.input(ControlValue(note));

Currently, hot-swapping inputs to any ControlGenerator or Generator is not supported due to the fact that we don't have a good concurrency model for the synthesis graphs yet. If the voice is in the middle of processing its output and the existing input to ControlMidiToFreq is on the stack when you swap it out for a new one, it will crash.

To get this to work, you should hold on to the ControlValue that feeds ControlMidiToFreq and just update its value when the note changes.

We want to ideally avoid taking locks as much as possible in the buffer callback -- you may have noticed that generator-level mutexes were removed. They were eating up way to much CPU - ideally there would be only one mutex per SynthesisContext, and all generators in the graph have access to it, but I haven't figured that out yet. If you have any thoughts I'd love to chat sometime.

When actively playing notes, CPU usage is around 4% for a simple 8-voice synth. When I stop playing, though, usage goes up to 13%

Hmm... I've seen this before in other C++ synth implementations I've done, and it's usually due to subnormal numbers in the sample data somewhere. However, I'm not really sure where they'd be coming from - typically this will come up for floats that are repeatedly multiplied by something less than 1.

Have you tried running it through a profiler to see where the extra cycles are being used?

The Max poly~ object is smart enough to not even run DSP for subpatches that are not playing notes. Is that currently possible in Tonic? Would it work to add and remove sub-synths from the mixer as voices go active and inactive?

That sounds like it might work. Currently, disabling a particular input or portion of the graph isn't really supported because all generators operate under the assumption that they are being ticked at the audio or control rate. I have plans to add a "reset" method to the Generator base classes so that it will work to disable them while not active and reset the state when they become active again, but that's not in place yet. A lot of these things will depend on the ability to efficiently keep track of the inputs of each object in the graph, which we haven't solved yet (member variables don't provide a generic pattern, and keeping inputs in stdlib containers is way too slow for efficient processing - I'd like to pick your brain on that too).

Is there a way to get a signal when an ADSR finishes its release cycle?

Not yet - I have an idea on how to create multiple outputs from a single object, but I haven't gotten around to working on it yet.

morganpackard commented 11 years ago

My comments inline

Sent from my iPhone

On Jun 7, 2013, at 6:26 PM, "Nick D." notifications@github.com wrote:

Sweet!

A few comments

It will eventually crash in ControlGenerator::_tick() inside of ControlMidiToFreq()::computeOutput()

I believe this can be attributed to the way you are using ControlMidiToFreqin VoiceData. This line is not thread safe:

pVoice->frequency.input(ControlValue(note));

Currently, hot-swapping inputs to any ControlGenerator or Generator is not supported due to the fact that we don't have a good concurrency model for the synthesis graphs yet. If the voice is in the middle of processing its output and the existing input to ControlMidiToFreq is on the stack when you swap it out for a new one, it will crash.

To get this to work, you should hold on to the ControlValue that feeds ControlMidiToFreq and just update its value when the note changes.

We want to ideally avoid taking locks as much as possible in the buffer callback -- you may have noticed that generator-level mutexes were removed. They were eating up way to much CPU - ideally there would be only one mutex per SynthesisContext, and all generators in the graph have access to it, but I haven't figured that out yet. If you have any thoughts I'd love to chat sometime.

When actively playing notes, CPU usage is around 4% for a simple 8-voice synth. When I stop playing, though, usage goes up to 13%

Hmm... I've seen this before in other C++ synth implementations I've done, and it's usually due to subnormal numbers in the sample data somewhere. However, I'm not really sure where they'd be coming from - typically this will come up for floats that are repeatedly multiplied by something less than 1.

Have you tried running it through a profiler to see where the extra cycles are being used?

The Max poly~ object is smart enough to not even run DSP for subpatches that are not playing notes. Is that currently possible in Tonic? Would it work to add and remove sub-synths from the mixer as voices go active and inactive?

That sounds like it might work. Currently, disabling a particular input or portion of the graph isn't really supported because all generators operate under the assumption that they are being ticked at the audio or control rate. I have plans to add a "reset" method to the Generator base classes so that it will work to disable them while not active and reset the state when they become active again, but that's not in place yet. A lot of these things will depend on the ability to efficiently keep track of the inputs of each object in the graph, which we haven't solved yet (member variables don't provide a generic pattern, and keeping inputs in stdlib containers is way too slow for efficient processing - I'd like to pick your brain on that too).

Currently, controlgenerators rely on the tick of the audio generators to advance. If each synth has its own metro, and we stop calling tick on silent synths, the metros will get out of sync. I

Is there a way to get a signal when an ADSR finishes its release cycle?

Not yet - I have an idea on how to create multiple outputs from a single object, but I haven't gotten around to working on it yet.

The way I'd do it is add a "getFinishedTrigger" method to adsr, which returns a controlgenerator (controltrigger maybe?) you can query for "has changed".

— Reply to this email directly or view it on GitHubhttps://github.com/TonicAudio/Tonic/issues/178#issuecomment-19136286 .

Dewb commented 11 years ago

Aha, yeah, getting rid of the input frobbing fixed it, thanks! I switched to named parameters rather than holding on to the ControlGenerators, something I was meaning to do anyway.

Dewb commented 11 years ago

Built OSX so I could use the Xcode profiler. On my MBP, the CPU usage is 12% when playing notes and 85% when idle!

I tried setting the flush denormal to zero flag. At first I thought it helped a small amount, but after a few runs the numbers didn't look statistically different than the runs without the flag.

Here's a snapshot of the profiler after running the program and doing nothing: (Flush flag was off.)

screen shot 2013-06-07 at 11 10 32 pm

Here's a snapshot after playing a few notes and then stopping for a very long time (which seems to produce higher CPU usage than if you never played any notes.) Looks like a lot of time is spent in Compressor_::computeSynthesisBlock(). (Flush flag was on this run.)

screen shot 2013-06-07 at 11 27 45 pm

ndonald2 commented 11 years ago

Here's a snapshot after playing a few notes and then stopping for a very long time (which seems to produce higher CPU usage than if you never played any notes.) Looks like a lot of time is spent in Compressor_::computeSynthesisBlock().

That sounds an awful lot to me like denormals are the culprit. The compressor's amplitude envelope release phase is a one-pole recursive filter tending toward zero, so it fits exactly the scenario I described. If no note has ever been played, the amplitude envelope is exactly zero. After playing a note, the envelope will go positive and tend toward zero until it drops to denormal range.

Not sure why the flush flag didn't work for you -- did you use _MM_SET_FLUSH_ZERO_MODE(_MM_FLUSH_ZERO_ON)? If so, be aware that it needs to be done on the audio thread, so I don't know of a good place to do it once and only once, independent of platform.

Just as a sanity check, to verify it's a denormal issue, try running the same test, but in FilterUtils.h make the following change to onePoleLPFTick

  inline void onePoleLPFTick( TonicFloat input, TonicFloat & output, TonicFloat coef){
    output = ((1.0f-coef) * input) + (coef * output);
    if (output < std::numeric_limits<float>::min()){
      output = 0;
    }
  }

ndonald2 commented 11 years ago

Obviously that is an inefficient solution but it should verify that it's denormals that are the issue.

Dewb commented 11 years ago

Turning off limiting inside each of the Synth voices and the outer Synth seems to have helped some; now the CPU percentages are around 6% (active) and 30% (idle.)

Dewb commented 11 years ago

To turn the flag on I was using:

#include <fenv.h>
fesetenv(FE_DFL_DISABLE_SSE_DENORMS_ENV);

I'll try your suggestion in onePoleFPLTick().

Dewb commented 11 years ago

Turned all the limiting back on and tried your solution; not much change, back to 12%/85%.

ndonald2 commented 11 years ago

Hmm I will try to do some profiling and see if I can figure it out. I can't imagine what else it would be besides denormals, especially since I've had this exact problem before.

ndonald2 commented 11 years ago

Ah I did forget there are a ton of other places that might have denormal issues - recursive filters, delays with feedback, etc - so a way to flush them globally is pretty important.

ndonald2 commented 11 years ago

I think I got it! Add this to the start of BufferFiller::fillBufferOfFloats():

      // flush denormals on this thread

#ifdef _MM_SET_FLUSH_ZERO_MODE
      _MM_SET_FLUSH_ZERO_MODE(_MM_FLUSH_ZERO_ON);
#endif

For my test setup this cut my CPU usage from early-stage idle ~5%, late stage idle ~10-15% down to consistent ~2-3% idle.

Dewb commented 11 years ago

Aha, I got it too. Apparently, to use fesetenv on OSX there's a pragma you have to set first. You ran your test with _MM_SET_FLUSH_ZERO_MODE on Windows, right?

#include <fenv.h>
#pragma STDC FENV_ACCESS ON
...

int renderCallback( void *outputBuffer, void *inputBuffer, unsigned int nBufferFrames,
        double streamTime, RtAudioStreamStatus status, void *userData )
{
    static bool flagSet = false;
    if (!flagSet) {
        fesetenv( FE_DFL_DISABLE_SSE_DENORMS_ENV );
        flagSet = true;
    }

    synth.fillBufferOfFloats((float*)outputBuffer, nBufferFrames, nChannels);
    return 0;
}

ndonald2 commented 11 years ago

Nope, I was testing on OSX. The _MM_SET_FLUSH_ZERO_MODE macro is part of the xmm intrinsics header <xmmintrin.h> so in theory it should be available on any platform where SSE is supported. Repeated calls don't seem to incur any noticeable overhead so putting it in the bufferfiller fill function seems appropriate, that way it (ideally) will work for anyone using Tonic anywhere - not just the standalone demo, as you have it here.

I'm not sure what is more widely compatible between fesetenv() and the intrinsics macro - do you know? I don't mind either way, but I definitely think it should live internally to Tonic, and BufferFiller seems like a good place.

Dewb commented 11 years ago

Yeah, it should definitely be inside Tonic. std::fesetenv has been added to the C++ library as of C++11. Will the _MM macro work on iOS? No SSE on ARM, right?

ndonald2 commented 11 years ago

I'm not 100% sure, but that's why I wrapped it in the #ifdef. If it's not there, it won't be compiled.

I want to say the ARM FPU already rounds denormals as the default, since the systems with which it's used are typically less concerned with precision and more with efficiency. This thread seems to sort of verify that:

http://stackoverflow.com/questions/7346521/subnormal-ieee-754-floating-point-numbers-support-on-ios-arm-devices-iphone-4

For now I'm going to push the macro-based fix to the development branch. Tomorrow I will do some on-device iOS profiling to see if it's an issue there.

Yet another great bug find. Thanks!

ndonald2 commented 11 years ago

Yep looks like it won't compile on-device for iOS with the xmmintrin header. I'll make the denormal flushing conditional to the build environment.

ndonald2 commented 11 years ago

Pushed to development. I'm not sure of any other non-SSE platforms that will need denormal rounding enabled, so for now it's defined like this:

#ifdef __SSE__
  #include <xmmintrin.h>
  #define  TONIC_ENABLE_DENORMAL_ROUNDING() _MM_SET_FLUSH_ZERO_MODE(_MM_FLUSH_ZERO_ON) 
#else
  #define  TONIC_ENABLE_DENORMAL_ROUNDING()
#endif

Dewb commented 11 years ago

MSVS apparently doesn't define __SSE__, so this returns denormals to the Windows build.

ndonald2 commented 11 years ago

Hmm... let me boot to windows and check it out.

On Sat, Jun 8, 2013 at 1:37 PM, Michael Dewberry notifications@github.comwrote:

MSVS apparently doesn't define SSE, so this returns denormals to the Windows build.

— Reply to this email directly or view it on GitHubhttps://github.com/TonicAudio/Tonic/issues/178#issuecomment-19152430 .

Nick Donaldson ndonald2@gmail.com

Dewb commented 11 years ago

This works:

#if defined(__SSE__) || defined(_WIN32)
  #include <xmmintrin.h>
  #define  TONIC_ENABLE_DENORMAL_ROUNDING() _MM_SET_FLUSH_ZERO_MODE(_MM_FLUSH_ZERO_ON) 
#else
  #define  TONIC_ENABLE_DENORMAL_ROUNDING()
#endif

ndonald2 commented 11 years ago

Cool, I'll add that.

On Sat, Jun 8, 2013 at 1:39 PM, Michael Dewberry notifications@github.comwrote:

This works:

if defined(SSE) || defined(_WIN32)

include

define TONIC_ENABLE_DENORMAL_ROUNDING() _MM_SET_FLUSH_ZERO_MODE(_MM_FLUSH_ZERO_ON)

else

define TONIC_ENABLE_DENORMAL_ROUNDING()

endif

— Reply to this email directly or view it on GitHubhttps://github.com/TonicAudio/Tonic/issues/178#issuecomment-19152455 .

Nick Donaldson ndonald2@gmail.com

Dewb commented 11 years ago

Should the macro definition go in TonicCore.h? Even if it's never called anywhere but BufferFiller, that's where all the other platform-specific code is.

ndonald2 commented 11 years ago

Good call.

On Sat, Jun 8, 2013 at 1:45 PM, Michael Dewberry notifications@github.comwrote:

Should the macro definition go in TonicCore.h? Even if it's never called anywhere but BufferFiller, that's where all the other platform-specific code is.

— Reply to this email directly or view it on GitHubhttps://github.com/TonicAudio/Tonic/issues/178#issuecomment-19152550 .

Nick Donaldson ndonald2@gmail.com

morganpackard commented 11 years ago

You guys are amazing.

On Sat, Jun 8, 2013 at 1:45 PM, Nick D. notifications@github.com wrote:

Good call.

On Sat, Jun 8, 2013 at 1:45 PM, Michael Dewberry notifications@github.comwrote:

Should the macro definition go in TonicCore.h? Even if it's never called anywhere but BufferFiller, that's where all the other platform-specific code is.

— Reply to this email directly or view it on GitHub< https://github.com/TonicAudio/Tonic/issues/178#issuecomment-19152550> .

Nick Donaldson ndonald2@gmail.com

—

Reply to this email directly or view it on GitHubhttps://github.com/TonicAudio/Tonic/issues/178#issuecomment-19152556 .

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

Dewb commented 11 years ago

FYI, I think my next step here is to put this together with JUCE and make a VST demo.

kphillisjr commented 4 years ago

I saw this issue, and was wondering what the current status is. I did not see a lot of changes on the tree.

kallaballa commented 4 years ago

I'm currently working on reviving the project and it probably will take a while before things get in motion. Please check out https://github.com/TonicAudio/Tonic/issues/282

kallaballa commented 4 years ago

oh i forgot... there is an example for a polyphonic synth, but it isn't a minimal one. https://github.com/kallaballa/MidiPatch

Dewb commented 4 years ago

This didn't get PR'd to master at the time because the demos folder was being reorganized and the approach to building cross-platform was changing. It would be fairly straightforward to update and PR this branch for PC/Mac (iOS is likely more complicated.)

kallaballa commented 4 years ago

Alright. I'll look into it. But first i gotta get around to making a release.

TonicAudio / Tonic

Polyphonic MIDI synth example #178

if defined(SSE) || defined(_WIN32)

include

define TONIC_ENABLE_DENORMAL_ROUNDING() _MM_SET_FLUSH_ZERO_MODE(_MM_FLUSH_ZERO_ON)

else

define TONIC_ENABLE_DENORMAL_ROUNDING()

endif