I’ve also run into this problem, and thanks to everybody who has written here. However, I think you still need to call Voice::clearCurrentNote() from within Voice::StopNote() if the voice is stolen/not allowed to trail. This is laid out in the documentation, and with an emphatic comment in the source code. Unless I misunderstand, one of these overflow buffer solutions, or something similar, still seems like the way to go!
EDIT:
I think if you have to implement a buffer solution (but followed the basic tutorial), something else is wrong, and the synth voice system should probably be handling these fades for you. In my particular case, I found out that my problem was that I had different panning of sound on different midi channels (probably other characteristics will cause this, too). I had to override the Synthesiser::findVoiceToSteal method by editing one line where it’s creating a list of voice candidates to steal from:
if (voice->canPlaySound(soundToPlay) && voice->isPlayingChannel(midiChannel))
And that fixed my issues with clicks. It’s probably assumed that you’re using sounds for any significant difference in a sound. I may refactor to that later.
@sudara this is basically the solution I used in my current project… except instead of setting my voice’s ADSR release to a small value for killQuick and then back again afterwards, I just created two ADSRs within my SynthesizerVoice: the standard ADSR that’s triggered by MIDI, and a second one with a very short attack/decay, a sustain of 1.0 and a very short release.
Make sure your quickFade ADSR is basically always on, and then if you call stopNote() with allowTailOff = false, simply call noteOff() on it.
This is what my custom override of SynthesiserVoice::renderNextBlock() looks like:
void SynthesiserVoice::renderNextBlock(AudioBuffer<float> outputAudio, const int inputChan, const int startSample, const int numSamples)
{
if(! (sustainPedalDown || sostenutoPedalDown))
{
if(! keyIsDown)
stopNote(1.0f, false);
}
if(adsr.isActive()) // use the adsr envelope to determine if the voice is on or not...
{
AudioBuffer<float> subBuffer(outputAudio.getArrayOfWritePointers(), 2, startSample, numSamples);
callYourProcessingFunctionHere();
if(adsrIsOn) //...but only apply the envelope if the ADSR on/off user toggle is ON. Here adsrIsOn is a bool member of SynthesiserVoice
adsr.applyEnvelopeToBuffer(subBuffer, startSample, numSamples);
if(isFading) // here isFading is a bool member of SynthesiserVoice that tracks whether a quick fadeout is underway. should usually be false. set to true in your call of stopNote() if allowTrailOff is false
{
if(! quickRelease.isActive()) { quickRelease.noteOn(); quickRelease.noteOff(); } // idk ??
quickRelease.applyEnvelopeToBuffer(subBuffer, startSample, numSamples);
adsr.reset();
isFading = false;
clearCurrentNote();
}
}
else
{
clearCurrentNote();
if(! isFading)
{
quickRelease.noteOn();
}
}
};
for reference, here’s what my SynthesiserVoice::stopNote() looks like:
void SynthesiserVoice::stopNote(const float velocity, const bool allowTailOff)
{
if (allowTailOff)
{
adsr.noteOff(); // this is the voice's PRIMARY adsr
isFading = false;
}
else
{
if(! quickRelease.isActive()) { quickRelease.noteOn(); }
isFading = true;
quickRelease.noteOff(); // this is the adsr used for a quick fade out
}
};
I’ve been working on my synthesizer implementation some more, and I thought this might be helpful to some, so here’s the way I’m handling “quick cutoffs”:
each instance of SynthesiserVoice has two member ADSR objects: one named adsr and one named quickRelease
isFading is a boolean member of SynthVoice that is true if a quick fadeout is currently in progress, and false otherwise
noteTurnedOff is a boolean member of SynthVoice that is true if the last note event was a note off (whether quick-release or not), and should be false if the last note event was a note on
and here’s the relevant parts of my renderNextBlock:
void SynthesiserVoice::renderNextBlock(const int numSamples, AudioBuffer<float>& outputBuffer)
{
// don't want to just use the ADSR to tell if the voice is currently active, bc in my plugin the user can toggle the ADSR envelope on/off...
bool voiceIsOnRightNow;
if(!isFading && parent->adsrIsOn) // adsrIsOn is a bool stored in the top-level Synthesiser
voiceIsOnRightNow = adsr.isActive();
else
voiceIsOnRightNow = isFading ? quickRelease.isActive() : !noteTurnedOff;
if(voiceIsOnRightNow)
{
// do fancy stuff here...
if(parent->adsrIsOn) // only apply the envelope if the ADSR on/off user toggle is ON
adsr.applyEnvelopeToBuffer(subBuffer, 0, numSamples);
if(isFading) // quick fade out for stopNote() w/ allowTailOff = false:
quickRelease.applyEnvelopeToBuffer(subBuffer, 0, numSamples);
}
else
clearCurrentNote();
};
Hi, thanks for all these info, I’m trying to use your method to re-write the synth class, but when I set the value for my quickRelease, it seems like it always took a long time to finish the release, any idea why it’s like this? Or where should I set the ADSR parameter value for my quickRelease?
It’s only one bug along the way, when I DBG the quickRelease release behavior, it seems like 6 seconds long, not 0.006f second, it’s really weird, the adsr release is normal.
Please forgive me for reopening an old wound, however every single synth developer using the JUCE platform must deal with the problem of note thief cracking! I am actually quite surprised a solution is not already built into the Synthesizer class to handle this problem.
Anyways Daniel for your solution suggestion you write;
when stopNote(...) is called with allowTailOff == false:
render** the next 10-50ms of your voice to overlap and apply a fade-out
set overlapIndex to 0"
But am I wrong in assuming “Note Stealing” is literally interrupting “RenderNextBlock” regardless of where in the process it is?
If my assumption is correct, your suggestion seems to solve the problem only if samples are added to the output buffer one at a time, instead of like my synth (using local audio buffer for various first unison level processing, and then stereo voice level processing) at the very end in an entire “numsamples” buffer size block. In other words what seems to happen in my synth, is that when as voice is stolen, I am loosing how ever far off samples out of “numSamples” in RenderNextBlock. Furthermore i don’t even know where my RenderNextBlock was interrupted.
It seems to me a solution should be built into the note stealing procedure, that allows RenderNextBlock to finish first.
I was thinking of a solution that allows “RenderNextBlock” to finish it’s current process, essentially meaning wait for the voice to become free. Firstly by setting a bool flag in SynthVoice, and secondly at the end of “RenderNextBlock” use a ramp on the buffer if flag is set.
However I don’t know how to override “FindFreeVoice”.
SynthesiserVoice* Synthesiser::findFreeVoice (SynthesiserSound* soundToPlay,
int midiChannel, int midiNoteNumber,
const bool stealIfNoneAvailable) const
{
const ScopedLock sl (lock);
for (auto* voice : voices)
if ((! voice->isVoiceActive()) && voice->canPlaySound (soundToPlay))
return voice;
// Thought of modifying this section
if (stealIfNoneAvailable)
{
auto* voice findVoiceToSteal (soundToPlay, midiChannel, midiNoteNumber);
dynamic_cast<SynthVoice*> (voice)->voiceToBeStolen = true;
while voice->isVoiceActive ();
return voice;
}
return nullptr;
}
I think it’s okay that every developer has to deal with this issue simply because there is no standard solution:
You can pre-render the tail of the stolen voice to a tails buffer, as suggested above, but this breaks the regular way how rendering is done. Also there can be no modulation of the tail unless known at the point in time when the voice is stolen and pre-rendered. On the other hand, you need not add the complexity of additional voices to render the tail.
You can add extra voices with a “quick release” state. This will result in a (modest) processing overhead and I am concerned that there might be phase cancellation when the same note is repeated and the short release of the previous note and the attack of the new one overlap. But at least the rendering is done “as usual” and you need not guess in advance how the voice and its modulation will develop during the quick release. One apparent drawback is that voice stealing strategies / play modes become harder to define.
You can treat voice stealing as a legato transition from the original note to the new note.
But then you have to make sure that no aspect of the transition results in pop/click noise.
For example, in an MPE synth, the new note can have a different initial MPE Timbre compared to the current timbre of the stolen note and you have to make sure that the timbre change will not cause audible artifacts.
One disadvantage of the legato approach is that repetitions of the same note may blend together to such an extent that the start of the second note cannot be perceived at all.
Each of these solutions has its Pros and Cons. In my current project, I have chosen the legato approach. For the moment, I am quite happy with the results except for the case when the same note is repeated. Moreover the legato approach will keep the original loudness even if you have a patch with a very slow attack. So an alternative should be available if this behavior is not intended.
Fast release is fine when it’s the same note played repeatedly. But for a stolen note I think it’s useless at all. Assuming the most extreme case that would be a monophonic synth, as soon as playback starts it will truncate the previous note, even if that note is in fastRelease state.
So a solution could be to check at the beginning of startNote if it is a stolen note. If we call clearCurrentNote() only when the note has finished, it means that if an active voice starts playing, it’s because it was stolen,
if (isVoiceActive()) {
}
so here several things could be done, the recommendation of a previous post to maintain the phase or make a legato could work.
Another option would be for each voice to have two independent generators, and for it to switch to another when the voice is active, while the previous one could end with fastRelease.
I am very keen to try out letting “RenderNextBlock” finish it’s process by setting a flag so instead of just adding my local SynthVoice buffer to outputbuffer, I do it with a ramp ending in zero volume.
So for example with a buffer size of 1024 and a sample rate of 48000, at most “FindFreeVoice” would have to wait 21.333 ms.
However as I wrote I don’t know how to, if even possible I can overwrite “FindFreeNote” in the Synthesizer base class.
in my synth renderNextBlock never ends, I just ignore it when the voice is not playing, and I don’t turn off the voice until the envelope is finished. fastRelease is just one stage in the envelope. I establish the time from the frequency of the note, n / freqNoteHz, where n is the number of cycles that it will last, no more than 4 is enough maybe 2.
if (!this->isVoiceActive()) return;
// process wave
if (adsr.isDone())
{
this->clearCurrentNote();
}
I think it is not necessary to deal with the synthesiser class, although there are so many possibilities depending on the implementation of each one that it is impossible to establish something common
However as I understand it, the voice stealing process interrupts RenderNextBlock regardless of where it may be at. And if that is true I want to try and wait for my RenderNextBlock to reach it’s current iteration, meaning reach end of code where I finally add my local buffer to the outputBuffer.
I don’t think it interrupts renderBlock, in fact it is being called continuously without needing to play the note.
void Synthesizer::renderVoices (AudioBuffer<float>& buffer, int startSample, int numSamples)
{
for (auto* voice : voices)
voice->renderNextBlock(buffer, startSample, numSamples);
}
what steals the note is to play a new one using the same voice, but it is doing it with your code.
So if you want to wait for a release to finish, you could ignore the note played in startNote while isVoiceActive(), and then play it manually when the previous one is finished, but this seems a bit convoluted.
I suppose that the ideal would be for there to be no limit to the number of object voices, but a limit to the number of notes playing simultaneously (not counting the fastRelease), and instead of stealing notes, they should be released. Perhaps this could be done in noteOn by keeping a list of voices that are added when played, and are released by calling stopNote with allowTailOff set to false, to perform the quick release.
Ok thanks for that information. I’m used to assembly with “interrupts” that are quite literal.
So if I understand you correctly, once “FindFreeVoice” finds voice to be stolen, “startNote” for that voice will wait for the outputBuffer to receive samples in “RenderNextBlock” or?
I’m just trying to figure out how exactly it works, so I try and come up with a solution that works for my synth.
It’s easy to over think this one. Here’s some thoughts about doing it using fastRelease.
You only need ~3ms to lower the amplitude to avoid clicks. So, when a voice is chosen to play but it’s already playing, save the note to play in your voice class and set the voice state into a ‘playing out’ state. Then just iterate a multiplier from 1 to 0 over 3ms multiplying the amplitude of the voice. This could happen over multiple render block calls.
When either the multiplier hits 0 or the voice envelope ends, use the earlier saved note to set up for a new playing note.
At worst, you’ll have a maximum 3ms when a note is stolen. It won’t be noticeable in terms of delay.
But you will still have the potential of existing playing notes being dropped out. This is unavoidable so the next strategy is to select the best voice to steal.
Suggested priorities for selection:
Lowest amplitude voice in the envelope release state.
If no voices are releasing then lowest amplitude voice in the envelope attack state.
If no voices are releasing or attacking then the oldest playing voice with the lowest amplitude, probably given by decay/sustain state and maybe factors like note velocity etc.
That should pretty much get you most of the way there regarding reasonable handling of voice stealing with no clicks and pops.
Ok so I tried the following as an experiment, that is allowing current “iteration” of "RenderNextBlock to “finish” and it works quite well. Drawback is if you have a large buffer there could then be a slight noticable delay for new note to start. I test my synth at 1024/48000 so maximum wait is “only” 21.333 ms.
In synthVoice;
private:
int voiceToBeStolen = 0;
bool voiceProperlyFinished = true;
In stopNote;
void stopNote (float velocity, bool allowTailOff)
{
ignoreUnused (velocity);
// Voice Stealing
if (allowTailOff == false)
{
for (int tg = 0; tg < maxModules; tg++)
if (synthParams.tgPortamentoReleaseModeCKV[tg] == 3)
synthParams.lastCyclesPerSecond[tg] = cyclesPerSecond;
// Set "flag" to however many of my eight tone generators are currently
// active. Because stopNote is called multiple times when loading DAW
// with synth on track, and/or preset for my synth below "flag" was not
// sufficient (see my startNote)
voiceToBeStolen = tgsOnVoice;
clearCurrentNote ();
}
// Normal Release
else
{
for (int tg = 0; tg < tgsOnVoice; tg++)
{
// Start release stage of my AHDSR1R2 unless already in progress
// Note I have an option to bypass Sustain stage
if (adsrVoice[tg].getStage () < 4) adsrVoice[tg].release ();
if (synthParams.tgPortamentoReleaseModeCKV[tg] == 3)
synthParams.lastCyclesPerSecond[tg] = cyclesPerSecond;
}
}
}
In startNote;
void startNote (int midiNoteNumber, float velocity, SynthesiserSound* sound,
int currentPitchWheelPosition)
{
if (voiceProperlyFinished == false)
{
while (voiceToBeStolen); // Wait for current render iteration to finish
adsrRunning = false;
}
else
voiceToBeStolen = 0;
voiceProperlyFinished = true;
...
...
// Various initialization, start AHDSR1R2 for all tone generators, and finally;
adsrRunning = true;
}
In RenderNextBlock
void renderNextBlock (AudioBuffer<float>& outputBuffer, int startSample,
int numSamples) override
{
if (!adsrRunning) return;
...
for (int tgs = 0; tgs < tgsOnVoice; tgs++)
{
// Here I use a local audio buffer for 8 stereo tone generators
// First I gather up to 16 unison samples, doing any needed modulation
// and effects, then mix that down to a stereo pair, doing any further
// needed modulation, effects, morphing between one or more tone
// generators, and then filtering
// Finally I add local buffer to output buffer
if (voiceToBeStolen)
{ // Here you should already have applied your envelope
// Add local tone generator voice buffers to global output buffer
// with a ramp
int channel = tg * 2;
outputBuffer.addFromWithRamp (channel,
startSample, tgVoiceBuffer[tg].getReadPointer (0),
numSamples, 1.0f, 0.0f);
channel++;
outputBuffer.addFromWithRamp (channel,
startSample, tgVoiceBuffer[tg].getReadPointer (1),
numSamples, 1.0f, 0.0f);
voiceToBeStolen--;
}
else
{
// Add to output buffer normally
}
}
Yes I was indeed overthinking this in my above post, as I now realize I can just do the following, which is allow current cycle of RenderNextBlock to finish and quick release buffer. No flags, no conditionals other places, just a ADSR quick release and let RenderNextBlock handle clearCurrentNote.
void stopNote (float velocity, bool allowTailOff)
{
ignoreUnused (velocity);
// Voice Stealing
if (allowTailOff == false)
{
// Set quick release all my 8 envelopes,
// which will be applied during RenderNextBlock
for (int tg = 0; tg < maxModules; tg++)
adsrVoice[tg].quickRelease ();
}
// Normal Release
else
{
for (int tgs = 0; tgs < tgsOnVoice; tgs++)
{
int tg = tgOnArrayVoice[tgs]; // Sorted array of tone generators
// Start release stage unless already in progress
if (adsrVoice[tg].getStage () < 4) adsrVoice[tg].release ();
}
}
}
