ADSR sustain and release stage not working properly

#1

Dear all,

I am trying to implement an audio unit synth. I am having a weird behaviour with the built-in ADSR of JUCE. Essentially, when I am setting the sustain parameter to 0 the release stage is not happening, and also when I am setting the sustain below 0.1 for example, the release time is longer than the settings. I am not sure what I am doing wrong here…
Thanks a lot!

This my code of the synth voice:

/*
  ==============================================================================

    SynthVoice.h
    Created: 7 Apr 2019 8:02:12pm
    Author:  Giuliano Anzani

  ==============================================================================
*/

#pragma once
#include "../JuceLibraryCode/JuceHeader.h"
#include "SynthSound.h"
#include "maximilian.h"

class SynthVoice : public SynthesiserVoice
{
public:
     
     bool canPlaySound (SynthesiserSound* sound) override
     {
         return dynamic_cast<SynthSound*>(sound) != nullptr;
     }

    //========================================
    
    void setADSRSampleRate(double sampleRate)
    {
        adsr.setSampleRate(sampleRate);
    }

    void getEnvelopeParam (float* attack, float* decay, float* sustain, float* release)
    {
        // envelope
        adsrParams.attack = *attack;
        adsrParams.decay = *decay;
        adsrParams.sustain = *sustain;
        adsrParams.release = *release;
    }
    
    void getMasterParam (float *mGain)
    {
        masterGain.setTargetValue(double (*mGain));
    }
    
    void setMasterSmoothing (double samplerate)
    {
        masterGain.reset(samplerate, 0.1);
    }
      
    //========================================
    
    void startNote (int midiNoteNumber, float velocity, SynthesiserSound* sound, int currentPitchWheelPosition) override
    {
        // start the envelope
        adsr.noteOn();
        level = velocity;
        frequency = MidiMessage::getMidiNoteInHertz(midiNoteNumber);
    }
    
    //========================================
    
        void stopNote (float velocity, bool allowTailOff) override
        {
            if(allowTailOff)
            {
                adsr.noteOff();
            }
            else
            {
                clearCurrentNote();
            }
        }
    
    //========================================
    // Audio callback
    void renderNextBlock (AudioBuffer<float> &outputBuffer, int startSample, int numSamples) override
    {
        adsr.setParameters(adsrParams);
        
        if(adsr.isActive() != 0) {
            for (int sample = 0; sample < numSamples; ++sample)
            {
//                adsr.setParameters(adsrParams);
                
                // sin test
                double theWave = osc1.sinewave(frequency) * 0.5;
                
                for (int channel = 0; channel < outputBuffer.getNumChannels(); ++channel)
                {
                    outputBuffer.addSample(channel, startSample, theWave * masterGain.getNextValue() * adsr.getNextSample());
                }
                ++startSample;
            }
        }
        else
        {
            clearCurrentNote();
        }
            
        
        
    }
    
    //========================================

private:
    double level;
    double frequency;
    SmoothedValue<double, ValueSmoothingTypes::Linear> masterGain;
    
    maxiOsc osc1; // from maximilian lib
    ADSR adsr; // JUCE envelope
    ADSR::Parameters adsrParams;
};

EDIT (I forgot a piece :slight_smile:)
here is the part of the code in the processor in which I am updating the samplerate for the ADSR:

void SynthFrameworkAudioProcessor::processBlock (AudioBuffer<float>& buffer, MidiBuffer& midiMessages)
{
    ScopedNoDenormals noDenormals;
    for (int i = 0; i < mySynth.getNumVoices(); i++)
    {
        if ((myVoice = dynamic_cast<SynthVoice*>(mySynth.getVoice(i))))
        {
            // set samplerate ADSR
            myVoice->setADSRSampleRate(lastSampleRate);
            // set parameters
            myVoice->getEnvelopeParam(
                                      // envelope
                                      tree.getRawParameterValue(ATTACK_ID),
                                      tree.getRawParameterValue(DECAY_ID),
                                      tree.getRawParameterValue(SUSTAIN_ID),
                                      tree.getRawParameterValue(RELEASE_ID)
                                      );
            myVoice->getMasterParam(tree.getRawParameterValue(MASTER_ID));
        }
    }
    
    
    buffer.clear();
    // audio callback from SynthVoice.h
    mySynth.renderNextBlock(buffer, midiMessages, 0, buffer.getNumSamples());
    
}
#2

If you have a sustain level of 0 then it’s expected that you won’t have a release stage as the release is the time it takes for the signal to return to 0, which it is already at.

Are you using the latest tip of the develop branch? There have been a few changes to the ADSR class that may be related to the other release time issue you’re seeing. I’ve put together a quick test using the develop branch and I’m seeing the expected release time for sustain levels < 0.1s.

#3

Thanks a lot for the answer!

I was thinking in the case of a percussive sound, in which, the sustain stage is not necessary ( I believe). After checking the source code of the JUCE ADSR I think that this extra check for the releaseRate (releaseRate > 0.0f)) in the noteOff function is not necessary, but I didn’t try yet, could be a solution?.

void noteOff()
    {
        if (currentState != State::idle)
        {
            if (releaseRate > 0.0f)
            {
                if (currentState != State::sustain)
                    releaseRate = static_cast<float> (envelopeVal / (currentParameters.release * sr));

                currentState = State::release;
            }
            else
            {
                reset();
            }
        }
    }

What I am thinking is that when you release the note you just need to take the last value of envelope and start the release stage.

Regarding this issue, I realised that I was updating the parameter of the envelope in the audio callback. Moving it in the noteOn function solved the problem:

void startNote (int midiNoteNumber, float velocity, SynthesiserSound* sound, int currentPitchWheelPosition) override
    {
        // update the parameter of the ADSR and...
        adsr.setParameters(adsrParams);
        // ...start the envelope
        adsr.noteOn();

        level = velocity;
        frequency = MidiMessage::getMidiNoteInHertz(midiNoteNumber);
        gendy1.setFrequency(frequency);
    }
#4

I opened a Pull Request here implementing a fix for this issue.

My solution was to perform the calculation of releaseRate within the noteOff() function, using envelopeVal to set the decay time. Thus all notes will take the same time to decay after noteOff().

#5

This is not true for most synths.
The release should start decreasing the amplitude from where it is at the time of the note off, doesn’t matter if you were in the attack, decay, or sustain stage.
As @giuaaa said, in a percussive sound envelope you usually find that the sustain is zero, and the attack and decay values are the same, so the sound is identical no matter how long one holds the key.
I came here because I had the same issue, just my two cents.

Edit: Oh, happy to see it was fixed in 5.4.7, thanx!