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

voice: use 2x regular release for fast release #2061

Conversation

sapphire-arches
Copy link
Collaborator

@sapphire-arches sapphire-arches commented May 31, 2024

This is much more conservative than the original logic, avoiding note-off clicks for soft culls.

@sapphire-arches sapphire-arches changed the title softcull: use semi-exponential release for fast release envelopes softcull: use accelerated release for fast release envelopes May 31, 2024
Copy link
Contributor

github-actions bot commented May 31, 2024

Test Results

36 tests  ±0   36 ✅ ±0   0s ⏱️ ±0s
 7 suites ±0    0 💤 ±0 
 7 files   ±0    0 ❌ ±0 

Results for commit 217aa7a. ± Comparison against base commit d5ab4ee.

♻️ This comment has been updated with latest results.

@nikodemus
Copy link
Contributor

i gotta be honest i have no idea why this works, but it helps with the House 15 song from chewabledrapery.

I'm not sure if this goes against project policy, but I at least would appreciate linking to the song in question. (Maybe also mentioning the reason/uncertainty of this in a comment, so one doesn't need to trawl commit messages to figure out why this is the way it is?)

@sapphire-arches
Copy link
Collaborator Author

sapphire-arches commented May 31, 2024

i gotta be honest i have no idea why this works, but it helps with the House 15 song from chewabledrapery.

I'm not sure if this goes against project policy, but I at least would appreciate linking to the song in question. (Maybe also mentioning the reason/uncertainty of this in a comment, so one doesn't need to trawl commit messages to figure out why this is the way it is?)

yeah, i asked chewabledrapery to file a bug and it looks like they have, which has the song -- this PR is currently underbaked since i just wanted to get something for them to test today. I'll figure out why this works and retarget it to community before marking it ready for review

@sapphire-arches sapphire-arches changed the base branch from release/1.1 to community June 1, 2024 02:18
@sapphire-arches sapphire-arches added the cherry-pick Commit to cherry pick to release branch label Jun 1, 2024
@sapphire-arches sapphire-arches linked an issue Jun 1, 2024 that may be closed by this pull request
@sapphire-arches sapphire-arches changed the title softcull: use accelerated release for fast release envelopes voice: use 2x regular release for fast release Jun 1, 2024
@sapphire-arches sapphire-arches marked this pull request as ready for review June 1, 2024 02:20
@sapphire-arches
Copy link
Collaborator Author

Cleaned up the commit message and thought the logic through correctly, should be better now.

@m-m-adams
Copy link
Collaborator

I believe this is actually making fast release at least twice as fast as normal release

A release of 0 has an increment of 8192 iirc. A fast release from polyphony limits (mono/auto/legato) is at an increment of 4096, which is doubled by this PR. A soft cull is at an increment of 65536, and so won't be affected

Good to merge though since it does seem like a good change

@sapphire-arches sapphire-arches added this pull request to the merge queue Jun 2, 2024
Merged via the queue into SynthstromAudible:community with commit 6a8f0c3 Jun 2, 2024
6 checks passed
tastycode pushed a commit to tastycode/DelugeFirmware that referenced this pull request Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-pick Commit to cherry pick to release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cull clicks/glitches in 1.1 RC1
3 participants