Open ngeiswei opened 5 years ago
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
...
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.
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.
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 releasingnew_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.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.
Multiple messages are fine, great that you are finding the solution!
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?
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 https://github.com/kometbomb/klystrack/issues/265#issuecomment-466628902, or mute the thread https://github.com/notifications/unsubscribe-auth/ABCK6RtEd68a0jzywoyBlgQOsbf_1UjAks5vQPtqgaJpZM4bKN9i .
How to reproduce
What should be expected
The note volume should decrease at the speed set to
REL
regardless of the ADSR phase.