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

One-shot voices stick and fill up the polyphony #63

Closed
jpcima opened this issue Feb 15, 2020 · 6 comments
Closed

One-shot voices stick and fill up the polyphony #63

jpcima opened this issue Feb 15, 2020 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@jpcima
Copy link
Collaborator

jpcima commented Feb 15, 2020

reported by @alcomposer : when you play 64 notes of a one-shot generator, the polyphony is filled and you can't play anymore unless resetting the plugin.

Should the voices be cleared once the envelope has reached the end?
also can we permit new notes to steal a slot more aggressively, in case the polyphony is filled?

<group>
lovel=0
hivel=127

<region>
trigger=release
sample=*noise
loop_mode=one_shot
ampeg_attack=1
ampeg_decay=1
ampeg_release=0
ampeg_sustain=0
@alcomposer
Copy link
Collaborator

trigger=release is what stops the voice from being deactivated. Otherwise the example works fine without that line.

@paulfd paulfd added the bug Something isn't working label Feb 15, 2020
@paulfd paulfd self-assigned this Feb 15, 2020
@paulfd
Copy link
Member

paulfd commented Feb 15, 2020

OK, so we can assume that in this case (trigger=release) the envelope should basically run its course without sustaining at all, would this be right?

If so, the envelope needs a freeRunning parameter in the envelope and get into release state as soon as it hits the sustain state. As a note for me (I may have time to do this in the WE) or if someone wants to try, this should be as simple as having a boolean check to switch the status to Release here and here, along with proper setup depending on the Region trigger value and setting the freeRunning value there.

However, this is a good opportunity to refactor that and the prepareEGEnvelope function above which are ugly and will not scale to flex EGs easily. The solution would be to have it more like this function in the filter pool: give the EG a reference to the Region, the MidiState, as well as the initial delay and velocity, and let it compute its parameters itself.

Short term EGs should be distributed like filters anyway and have a stable reference to MidiState, but for now the parameter is needed in the call.

@paulfd
Copy link
Member

paulfd commented Feb 15, 2020

Note that as soon as the envelope gets into release properly, this line should reset the voice.

All of this may break tests or require some additional tests 🙂

paulfd added a commit to paulfd/sfizz that referenced this issue Feb 17, 2020
- The envelope does not sustain if the region has `trigger=release`
- Change the envelope setup to pass the region rather than
individual envelope parameters
paulfd added a commit to paulfd/sfizz that referenced this issue Feb 18, 2020
- The envelope does not sustain if the region has `trigger=release`
- Change the envelope setup to pass the region rather than
individual envelope parameters
@paulfd paulfd added this to the 0.3.0 milestone Feb 18, 2020
@paulfd
Copy link
Member

paulfd commented Feb 18, 2020

It seems to work now. If you confirm I'll merge the PR and close this issue. Thanks!

@jpcima
Copy link
Collaborator Author

jpcima commented Feb 18, 2020

Yes the problem is gone. However, I have a link error on one of the benchmarks.

/usr/bin/ld : CMakeFiles/bm_envelopes.dir/BM_envelopes.cpp.o : dans la fonction « sfz::ADSREnvelope<float>::reset(sfz::Region const&, sfz::MidiState const&, int, unsigned char, float) » :
/home/jpc/documents/projects/sfizz/benchmarks/../src/sfizz/ADSREnvelope.cpp:21 : référence indéfinie vers « sfz::MidiState::getCCArray() const »

paulfd added a commit that referenced this issue Feb 19, 2020
- The envelope does not sustain if the region has `trigger=release`
- Change the envelope setup to pass the region rather than
individual envelope parameters
@paulfd
Copy link
Member

paulfd commented Feb 19, 2020

Corrected! Thanks.

@paulfd paulfd closed this as completed Feb 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants