Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Kicker's release stage #7226

Merged

Conversation

michaelgregorius
Copy link
Contributor

When applying its release stage Kicker did not take the frames before the release into account but instead always applied the release to the full buffer. This potentially lead to a jump in the attenuation values instead of a clean linear decay.

Fixes #7225.

When applying its release stage Kicker did not take the frames before the release into account but instead always applied the release to the full buffer. This potentially lead to a jump in the attenuation values instead of a clean linear decay.

See LMMS#7225 for more details.
Copy link
Member

@zonkmachine zonkmachine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested by exporting kick tracks at various sample rates and different buffer sizes and the issue is fixed. The code looks fine but perhaps someone more knowledgeable should review it.

@michaelgregorius
Copy link
Contributor Author

[...] The code looks fine but perhaps someone more knowledgeable should review it.

@zonkmachine, can you propose someone?

@Veratil
Copy link
Contributor

Veratil commented Apr 27, 2024

@LostRobotMusic is our resident audio algorithm expert

@LostRobotMusic
Copy link
Contributor

LostRobotMusic commented Apr 27, 2024

Normally I'd complain about the use of linear interpolation, but to my understanding this is for implementing the release in LMMS's default instrument envelope which is strictly linear currently (unfortunately).

LMMS's release code has always been wonky but this code appears to be correct. I haven't tested the PR directly, but it looks like zonkmachine already has.

@michaelgregorius
Copy link
Contributor Author

Normally I'd complain about the use of linear interpolation, but to my understanding this is for implementing the release in LMMS's default instrument envelope which is strictly linear currently (unfortunately).

Thanks for the additional review @LostRobotMusic! This merely fixes the linear release of the Kicker instrument which was always intended to be linear but which had a bug (see #7225).

Adding exponential envelopes would be a whole different task.

@michaelgregorius michaelgregorius merged commit 8636381 into LMMS:master Apr 27, 2024
9 checks passed
@michaelgregorius michaelgregorius deleted the 7225-FixKickersReleaseStage branch April 27, 2024 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Kicker - Glitch on key off
4 participants