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
/** 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;
}
}
ed95
March 11, 2019, 10:44am
3
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…
ed95
April 8, 2019, 9:25am
5
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;
}
ed95
May 7, 2019, 9:26am
7
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.
ed95
May 30, 2019, 10:54am
9
I’ll get the fixes cherry-picked onto the master branch, they should be there shortly.