Bug in adsr?

I believe I have found a bug in the new adsr class. The issue stems from the fact that the release rate is always calculated based on the sustain level:

    void calculateRates (const Parameters& parameters)
    {
        // need to call setSampleRate() first!
        jassert (sr > 0.0);

        attackRate  = (parameters.attack  > 0.0f ? static_cast<float> (1.0f                  / (parameters.attack * sr))  : -1.0f);
        decayRate   = (parameters.decay   > 0.0f ? static_cast<float> ((1.0f - sustainLevel) / (parameters.decay * sr))   : -1.0f);
        releaseRate = (parameters.release > 0.0f ? static_cast<float> (sustainLevel          / (parameters.release * sr)) : -1.0f);
    }

However, if decay is long and sustain is low, noteOff() will be called during the decay phase, and the releaseRate will be wrong, resulting in a much longer release time.

My quick and dirty local fix to noteOff() was to recalculate the releaseRate using the current envelopeVal, in case that’s any use :slight_smile:

    /** Starts the attack phase of the envelope. */
    void noteOn()
    {
        releaseRate = bufferedReleaseRate;

        if      (attackRate > 0.0f)  currentState = State::attack;
        else if (decayRate > 0.0f)   currentState = State::decay;
        else                         currentState = State::sustain;
    }

    /** Starts the release phase of the envelope. */
    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();
        }
    }

    void calculateRates (const Parameters& parameters)
    {
        // need to call setSampleRate() first!
        jassert (sr > 0.0);

        attackRate  = (parameters.attack  > 0.0f ? static_cast<float> (1.0f              / (parameters.attack * sr))  : -1.0f);
        decayRate = (parameters.decay > 0.0f ? static_cast<float> ((1.0f - sustainLevel) / (parameters.decay * sr)) : -1.0f);
        if (currentState == State::release)
        {
            releaseRate = (parameters.release > 0.0f ? static_cast<float> (envelopeVal / (parameters.release * sr)) : -1.0f);
        }
        else
        {            
            releaseRate = (parameters.release > 0.0f ? static_cast<float> (sustainLevel / (parameters.release * sr)) : -1.0f);
            bufferedReleaseRate = releaseRate;
        }
    }

Thanks for reporting, I’ve pushed this (and another ADSR fix) to the develop branch here.

1 Like

Hi, I used that fix, but I also had to reset the releaseRate to its nominal value in the reset function, otherwise it leads to strange behaviours, because the release rate keeps its “patched” value forever…

Thanks, I’ll get this sorted out.

Hi, I had another issue with this class, when attack is null, decay is not working as expected because the envelope value starts from 0.
I used the following fix to set the envelopeVal to 1 when attack time is null :

    void noteOn()
    {
        if      (attackRate > 0.0f)  currentState = State::attack;
	else if (decayRate > 0.0f) { currentState = State::decay; envelopeVal = 1.0f; } // PATCH
        else                         currentState = State::sustain;
    }

Thanks, I’ll get a fix pushed shortly.

Thanks for the fixes!

When can we expect to see these in the master branch? It feels dirty to take the file from the develop branch just for these fixes.

I’ll get the fixes cherry-picked onto the master branch, they should be there shortly.