Release is abrupt during attack phase

[ngeiswei]

How to reproduce

1. Create a new instrument with ADSR

ATK:3F
SUS:1F
DEC:00
REL:3F

2. Press some key down to play a note
3. In the middle of the attack phase release the note. The note volume decreases abruptly.
4. Press again some key down, this time release only after a while once it has reached the sustain phase. The note volume decreases slowly as expected.

What should be expected

The note volume should decrease at the speed set to REL regardless of the ADSR phase.

[ngeiswei]

For what I can gather from klystron/src/snd/cyd.c, adsr->envelope seems to be evolving as expected. Maybe the problem is in cyd->lookup_table...

[ngeiswei]

I think I know! There is a discontinuity between the formula used to turn the envelope into amplifier factor during attack (see cyd_env_output of klystron/src/snd/cyd.c)

		if (adsr->envelope_state == ATTACK)
			return ((Sint64)input * ((Sint32)adsr->envelope / 0x10000) / 256) * (Sint32)(adsr->volume) / MAX_VOLUME;

and the formula used during release

		else
			return ((Sint64)input * (cyd->lookup_table[(adsr->envelope / (65536*256 / LUT_SIZE) ) & (LUT_SIZE - 1)]) / 65536) * (Sint32)(adsr->volume) / MAX_VOLUME;

When release occur right in the middle of the attack the amplifier factor suddenly jumps to another value!

I don't know what could be the proper fix, and I might not see the big picture well enough to find it.

[ngeiswei]

It seems to me the simplest trick to solve that would be to re-ajust adsr->envelope right at release time so that the amplifier factor resulting from lookup_table would be close to the last amplifier factor calculated by the attack formula.

[ngeiswei]

Given the way lookup_table is defined, the translation formula should be something like

new_env = sqrt(old_env) * alpha

where

• old_env is the value of adsr->envelope right after releasing
• new_env should be the new value assigned to adsr->envelope at transition (only need to do it once, at the start of the release)
• alpha is some constant factor to determine.

[ngeiswei]

If I'm correct the correct translation formula is

new_env = sqrt(old_env * 65536 * 256)

now time to try!

P.S: sorry for the multiplicity of posts. I prefer to update my progress frequently to not duplicate effort, in case someone else is looking at this too.

[kometbomb]

Multiple messages are fine, great that you are finding the solution!

[ngeiswei]

It works! :-) :-) :-)

I still need a bit more time to create a proper PR but here comes the question, what to do about backward compatibility?

[kometbomb]

Perhaps this can be postponed for 1.8 as it's clearly a middle sized improvement. People can use older versions for compability. la 23. helmikuuta 2019 klo 10.27 Nil Geisweiller <notifications@github.com> kirjoitti:

…

It works! :-) :-) :-) I still need a bit more time to create a proper PR but here comes the question, what to do about backward compatibility? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#265 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABCK6RtEd68a0jzywoyBlgQOsbf_1UjAks5vQPtqgaJpZM4bKN9i> .