Fix ADSR envelope:

[iBundin]

a note could be released before sustain stage of the envelope.

[sletz]

Thanks, I'll check it ASAP.

[orlarey]

We need to check because this is a rather different behavior. Perhaps a new function is needed instead...

[iBundin]

We need to check because this is a rather different behavior. Perhaps a new function is needed instead...

In my opinion, current ADSR implementation is incorrect, because rDelta does not depend on the envelope output level which could be different from sustain, thus release time is always different depending on attack, decay, and the moment gate becomes 0.

[sletz]

From @orlarey:

Concerning the ADSR, the current behavior is the intended one: the release part starts as soon as the gate signal ends.

Here is a link to experiment with:

https://faustide.grame.fr/?autorun=0&voices=0&name=adsr&inline=aW1wb3J0KCJzdGRmYXVzdC5saWIiKTsKCnByb2Nlc3MgPSBiYS5wdWxzZW4oYmEuc2VjMnNhbXAoMSowLjE3KSwgYmEuc2VjMnNhbXAoMSkpIDogQCgxMDAwMCkgCiAgICAgICAgPDogXywgZW4uYWRzcigwLjEsIDAuMSwgMC43LCAwLjUpOw%3D%3D

Three cases are attached. But I understand that other behaviors could be needed...

=========== From @iBundin

In examples above adsr behavior is fine. But if you set up big decay level and low attack (for percussive-like sounds) release will work incorrectly:

Gate = 0 during sustain => correct behavior:

Gate = 0 during attack stage => incorrect:

It causes a very long release tail. In other cases release could be much shorter than expected.

=========== From @orlarey:

The ADSR of the library is designed to use constant slopes for the attack, the decay, and the release. Therefore the behavior in your examples is the expected one, i.e. coherent with the fact of using constant slopes.

Your ADSR uses a variable slope for the release. This induces a different behavior when the gate is shorter than the AD part. That can make sense, but replacing the ADSR function with yours would potentially break the users' code that relies on the actual behavior. This is why I suggested an addition to the library, but not a replacement.

[orlarey]

@iBundin, I did some tests. As expected the performances of the two adsr are the same. Moreover, as noted by Oleg, your version performs exactly as the old one. It has a constant release time instead of a constant release slope. This implies a more predictable behavior with very short gates.  Therefore it could also make sense to just replace the current version as you initially suggested. What do you think @sletz ?

[sletz]

Its probably OK to change the ADSR release behaviour, but @orlarey you can ask on Faust mail lists to be sure?

[orlarey]

Merged