-
Notifications
You must be signed in to change notification settings - Fork 259
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
Modulation Depth Range #1355
Comments
When implementing this feature, we should consider introducing a new general controller flag, like Reading through the RPN, it suggests that it should work together with CC1 Modulation Depth controller. This does have a default modulator in place. Potentially this existing default modulator could be modified, by adding a second source for the new fluidsynth/src/synth/fluid_synth.c Lines 455 to 469 in 66adce6
On the other hand, changing that existing modulator may cause incompatibilities when existing implementations try to override that modulator. Probably adding a new default modulator would be safer. What would be the maximum default amount in cents? The RPN suggests a minimum of 600. That sounds like a reasonable proposal. Implementations are free to override the modulator, if they need a bigger amount.
This would also be a nice solution, possibly an even more generic one. However, I would restrict this to that single default modulator for CC1, since the RPN suggests they work hand in hand. fluidsynth/src/synth/fluid_synth.c Lines 371 to 379 in 66adce6
|
That's a bad idea IMO. Some soundfonts ( for example the one attached to #1059 ) like to disable the stock vib lfo and use mod lfo for vibrato instead. Adding a new modulator would result in a messed up vibrato since the new modulator using mod wheel sensitivity would not get disabled. And changing the current modulator would break things too since modulators disabling it won't be considered "identical". The proposed solution of an internal depth multiplier will respond to all of the modulators. Or an even simpler solution: Apply the multiplier to the CC itself! For example if the depth is 100 cents, it's twice the default, so multiplier is too. And then if CC1 is set to 127, it's effectively interpreted as 254, leading to increased depth on all modulators, no matter what destination they use. I don't know if fluid is able to handle CC values above 127, but if it is, then that would be the perfect solution IMO, because:
|
Alright, to sum it up: Solution requirementsThe solution must:
Proposed solution v2Here's my solution which meets all the requirements:
This meets all of our requirements:
Should be relatively easy to implement (forget that comment about multiplying the controller value, this will work too and won't require promoting cc table to int). |
Is your feature request related to a problem?
This RPN is not implemented in fluidsynth.
This document, section 3.4.4
Expected behavior
this MIDI file should have more and more modulation range depth with each note played.
Describe the solution you'd like
Since there's no
Modulation Sensitivity
(or anything similar) source in the sf2 modulator source enum, we have to give it special treatment.My proposed solution is to use a modulation multiplier.
The sf2 spec assumes 50 cents depth by default, so when the RPN gets called, calculate a modulation multiplier by dividing the target depth range by 50. Then when running the DSP chain, simply multiply
vibLfoToPitch
ormodLfoToPitch
(or both) by that amount.That way, if the soundfont creator wanted a more subtle vibrato effect on a given instrument (25 cents for example) and the MIDI composer would want to double the modulation depth (by setting it via RPN to 100 cents, which is the double of MIDI spec default), the final depth would be 50 cents, so double the initial depth, instead of suddenly jumping from 25 cents to 100.
Describe alternatives you've considered
Another solution could be finding all modulators that use mod wheel as source and vibLfoToPitch as destination and overriding the amount with the set range, but I think it could be quite destructive because of the problem mentioned above.
Additional context
My implementation of the solution 1
The text was updated successfully, but these errors were encountered: