Add modulators checking at soundfont loading time #467
Conversation
- This step 1 adds modulators checking when loading soundfont. - This is done when importing soundfont. - Invalid modulators are removed from zones modulators list.
- Removing modulators check at noteOn time. This is done by introducing fluid_voice_add_mod_local() function.
- Since duplicate modulators were removed at soundfont loading, it worth to avoid wasting cpu cycles at noteon time.
- change name voice_mod_list_limit_count to voice_mod_limit_count. - Fix typos.
-rename count to mod_idx
src/synth/fluid_voice.c
Outdated
| * local version of fluid_voice_add_mod function. Called at noteon time. | ||
| * @param voice, mod, mode, same as for fluid_voice_add_mod() (see above). | ||
| * @param check_limit_count is the modulator number limit to handle with existing | ||
| * identical modulator(for mode FLUID_VOICE_OVERWRITE or FLUID_VOICE_OVERWRITE only). |
There was a problem hiding this comment.
FLUID_VOICE_OVERWRITE or FLUID_VOICE_OVERWRITE ?
There was a problem hiding this comment.
I meant why are you listing FLUID_VOICE_OVERWRITE twice?
src/sfloader/fluid_defsfont.c
Outdated
| * #FLUID_VOICE_ADD to add (offset) the modulator amounts, | ||
| * #FLUID_VOICE_OVERWRITE to replace the modulator, | ||
| */ | ||
| void static |
There was a problem hiding this comment.
Please make sure that static is always at the beginning of the declaration: https://travis-ci.org/FluidSynth/fluidsynth/jobs/458603436#L982
There was a problem hiding this comment.
There are more occurrences, pls see the provided build log.
- clarify comment in fluid_voice_add_mod_local. - move static at the beginning of declaration. - Avoiding compiler complaints "comment within a comment". - fix typos "unsing" to "using".
Yes, actually current implementation fluid_voice_add_mod() has no return code. For now, this PR reproduces same behaviour. However, assuming this PR accepted, maybe future implementation of this function API could return an int (FLUID_OK, FLUID_FAILED) ?. Of course doing this will introduce an API change. |
Yes. But since the current implementation ignores it anyway, I currently consider this nice-to-have. |
- this is done by adding global modulators after local modulators.
…synth into modulator-check
- xxx_limit count replaced by unique identity_limit_count variable. - clarifying comment about this useful limit count variable. - fix typos in comments.
- Merging reducing defsfont.c code
- this was forgotten in commit fcd6ec6.
- inside fluid_zone_mod_import_sfont.
to fluid_zone_check_mod()
|
I would schedule this for 2.1 if you don't mind? Just let me know when you think it's ready. |
I am fine with this PR. I consider this ready.
Ok. For information, i am currently working on adding
|
|
Ok. I'll try to test and merge this within the next 2 or 3 days. |
Please, no need to hurry. |
|
Works well for me. Thanks! |
Actually some basic modulators check are done at noteon time (in fluid_voice_add_mod()). That means that we know if a modulator (modx) is invalid only when a MIDI noteon is received and only for a preset modx belongs to. This is not appropriate.
This PR moves the modulator checking at soundfont loading time. Enhancements are:
A better verbose modulator integrity check, for any soudnfont loaded at appropriate time.
1.1) All modulators are checked (preset zone (local/global), instrument zone (local/global).
1.2.1) Modulators check are enforced to source src1 and src2 (for non-CC and CC sources) (following SF specs (except for CC LSB) ( see comment in fluid_synth_cc_LOCAL()).
Modulators CC sources checking is coherent with the actual behaviour in fluid_synth_cc_LOCAL() in regard of modulation triggering.
1.2.2)Also, identic modulator in the same zone are detected.
1.2.3)Any invalid modulator(sources invalid, or modulator identic) is removed at loading time with a warning message displaying the cause and name of the modulators.
This fix a bug in noteon, in the case of identic modulators in global preset zone.
Assuming 2 identics modulator (m1 and m2) in a preset global zone, the actual noteon doesn't check this case. (the actual code detect identic modulator in all others zones (instrument (local or global), preset(local)) but not preset global).
NoteOn is faster.
2.1)There is no more modulators checks at noteon making this more efficient.
2.2 As there are no identic modulator in the same zone, there is no more identity modulator check (i.e local zone against local zone), (i.e global zone against global zone). This result in a faster code and the bug described in (2) is gone.
Modulators sources checking as been added in API functions fluid_synth_add_default_mod() and fluid_voice_add_mod(). Please note that actually fluid_voice_add_mod() has no return code, but this could be perhaps changed ?