BUG REPORT: ADSR noteOn

General JUCE discussion
1

I came across some wired behaviour were I wanted to overwrite the ADSR Parameters and start over. I did this by just calling setParameters and noteOn.

But if the first set of parameters doesn’t have an attack phase and the second does, the envelopeVal stays at the sustain/decay level, since it is not being set to zero in noteOn.

I ended up just calling reset first, but for me this bug was not only hard to find, but also hard to spot that there was something not going as planed. I think it would be a good addition to make the ADSR class more fool proof.

    void noteOn() noexcept
    {
        if (attackRate > 0.0f)
        {
            envelopeVal = 0.0f;
            state = State::attack;
        }
        else if (decayRate > 0.0f)
        {
            envelopeVal = 1.0f;
            state = State::decay;
        }
        else
        {
            envelopeVal = parameters.sustain;
            state = State::sustain;
        }
    }

(line 5 is the addition I’m proposing)

2

Thank you for reporting.

3

I don’t have a preference either way - but I think this may change the behavior of existing apps in an undesirable way.

Say you have a single voice in the release phase and you trigger a note on – previously it will re-enter attack from the current value, which is desirable as you keep a continuous signal.

Now it will snap back to 0 and create a click, and re-enter a full attack stage regardless of the current state of then envelope?

I would argue that it’s actually that these envelope don’t support changing their parameters during playback, and if the parameters change – then calling the reset is a requirement before the next note on.

Feature or Bug 0.o

3 Likes
4

This is actually a good point, haven‘t thought of that!

With that in mind I think it is actually better to leave it as is. You could argue that this is also not the way it is supposed to work but you can say the same about changing the parameters half way through.

5
3 Likes