Release section of envelope issues

[YurkoFlisk]

This problem may be similar to #3086, but more general I think.
When the release section of envelope is entered (whether it's for volume or not) and AMT knob is somewhere between 0 and 1, there's a click even with nonzero release time, which is well noticeable if AMT is around 0.5 and sustain value is at max. Zip with 2 example presets is attached: one for volume envelope, second for cutoff envelope (with volume envelope set to AMT 1 so that it doesn't cause problems).

The issue seems to be in EnvelopeAndLfoParameters.cpp and is the way m_rEnv is computed in void EnvelopeAndLfoParameters::updateSampleVars. In particular, void EnvelopeAndLfoParameters::fillLevel function suggests that this array is used for calculating envelope levels during release period, and it's values are multiplied by the envelope value from which the release began (m_pahdEnv[_release_begin] or m_sustainLevel, depending on release beginning time). So, for a continuous transition, at the first release frame this value should be multiplied by 1 (or very close to that), so m_rEnv[0] should be 1, whereas, by the way it's filled in updateSampleVars function, m_rEnv[0] is m_amount. This also explains why there's no problem when AMT is 1.
What's interesting is that if m_rEnv is calculated starting from 1, release period should always start smoothly, so even if m_amount is 0 we will have release from actual value. In particular, since for the volume section default release time is already nonzero, there won't be click at the end by default, so this should resolve #3086 as well.

So, it seems like change here from
const float rfI = ( 1.0f / m_rFrames ) * m_amount;
to
const float rfI = ( 1.0f / m_rFrames );
should fix the problems. I'll try to setup compilation process and test whether this will fix the problem and not introduce other ones.

ClickExamples.zip

[YurkoFlisk]

I tried the changes (sorry for long time) (this branch) and, with them applied, in attached project (in zip) three 3OSC presets sound correctly, whereas without them there are clicks due to envelope discontinuity.
test.zip

However, when amount is zero, due to the logic of EnvelopeAndLfoParameters::updateSampleVars either the envelope is not used or release is 1-sample wide, so in this case (including default 3OSC preset) there is still a click and this means that the change doesn't make envelope 'enabled' by default (which could be the problem because this was the temporal fix for #3086 and was then reverted).

So, this doesn't resolve #3086, but seems to resolve the general problem in calculating release section of envelope addressed in this issue (#4672).

[zonkmachine]

Hi! Thanks for looking into this! I may look into your finds over the holidays, just don't hope too much... :)

[zonkmachine]

Holidays are now over and I'm back testing this.

I tried the changes (sorry for long time) (this branch)

I can confirm that this looks like a fix for the issue with transients in the envelope with amount set to ~0.5 . Needs more testing but it looks promising.

@@ -490,7 +490,7 @@ void EnvelopeAndLfoParameters::updateSampleVars()

-	const float rfI = ( 1.0f / m_rFrames ) * m_amount;
+	const float rfI = 1.0f / m_rFrames;

So, this doesn't resolve #3086,

No. This was fixed in #6908 with three separate issues solved and this here is a fourth.

@michaelgregorius You may want to have a look at this.

[michaelgregorius]

@zonkmachine, @YurkoFlisk, for me the change removes the click.

I am not sure though if the ADSRs are implemented correctly in the first place. I'm under the impression that the release stage always takes the same time to reach silence, regardless of whether it starts at full sustain, i.e. 1, or at a lower level, e.g. 0.2. This is not how others plugins (and I guess real ADSRs) behave.

The current implementation is also rather resource intensive as it might create long buffers for something that should be computable. On top of that it also does memory allocation for these buffers (which I assume happen in the audio thread).

It would be really great if the ADSRs would be implemented as self-contained classes which compute the next value from a minimal state. They could also implement linear and exponential decay. However, doing so would likely be the ultimate backwards compatibility headache.

[Rossmaxx]

Close since #6908 got merged?

[zonkmachine]

Close since #6908 got merged?

That's a separate issue. I'm looking into this one.

[FyiurAmron]

@zonkmachine hard to say if this if relevant, but I stumbled upon a variation of this - tl;dr 3osc creates clicks on keyoff due to abrupt termination of the wave - even with envs. edit: yup, I forgotten to turn all the ASDR knobs to the values needed for reasonable behaviour in current version. Seems related to this but a solution of just setting env AMT to 1.0 and release to 0.1 did the trick.

[zonkmachine]

@zonkmachine ... 3osc with lopass filter from the filter tab (+reso in my case, but read on) creates clicks on keyoff due to abrupt termination of the wave - even with envs. Disabling lopass filter eliminates the clicks. Reso makes this more pronounced, but even with 0 reso this is still audible. Seems like a bug. Is this a bug different to any of those two, or is it a new one? Is a repro case needed for this?

It would be really helpful with a project that demonstrates what you're seeing.

[FyiurAmron]

@zonkmachine OK, I went to the bottom of this. Not really a distinct bug, but a result of the current UI/UX decisions mixed with some quirks probably reported here plus me being LMMS newb. In my case, the envelope didn't have the full 1.0 amount, and after noticing that I still missed that the release was 0. With no lopass, it wasn't audible on the sawwave, but with lopass it got exposed, see below:
(no filtering)

(with lopass)

Nothing to fix additionally, but definitely something to keep in mind if anyone still gets the clicks. TBH I'd set 1.0 env and > 0 release (and other reasonable env settings) as the defaults, since it's really easy to miss this, especially with no visual clue which point on the ADSR env is which (i.e. with more than one point in the same place, you might mistake one for the other etc.) - I guess that's the reason why some toolings use colours and/or letters on the env graph,

e.g.

Anyway, many thanks for the work on this, kudos!

[FyiurAmron]

FWIW, my problem basically mirrored stuff from #6121 , #5527 , and some other ones, so it seems it's not opaque only to me :)

[zonkmachine]

Here is the tones/envelopes of a test run with single notes and just decay. I'm sweeping the envelope from 0 to close to 1.0 (max) and the notes are of different length. You can see that the one liner applied in the lower graph is adding the envelope full on after key off.

1 - master
2 - master + fix

Here is a close up of the key release of the two test runs above and a third one, with the envelope off added. You can see how the release frames that is default when there is no envelope engaged is not part of the equation when the envelope is engaged. With just a hint of envelope on you will essentially get less decay than with the envelope off.

1 - master
2 - master + fix
3 - master envelope off

• The fix presented here works by adding proper release to the note. It's probably not the way you want this to work but it definitely sound nicer.
• The default release frames with no envelope is not present with just a hint of decay and this is what makes the noise.
• The 'Amount' knob of the volume envelope pans between all on/off action and envelope control.

I know of no other synth that works this way but I haven't tried FruityLoops. I can see no case where any setting other than full on/off of the volume envelope is really useful.