Handle disabling of Velocity to Filter Cutoff modulator in a better manner about fluidsynth

[derselbst]

Original comment author: berarma

I think the current implementation is just bad. Spec. 2.01 says 2 times that it's a negative unipolar switch, but the text has it wrong when it says that it "has the effect of turning off velocity-to-filter for velocity numbers less than 64". The current FS implements the later behaviour.

Like Christian, I don't understand how this switch behaviour could be useful, also, spec. 2.04 fixes this by completely removing this source. It doesn't even mention it as a change in the spec, so it seems to me like an error in 2.01.

Josh, I don't understand why you say this: "But as FluidSynth is now, it requires that this
modulator also includes the switch as the secondary source.". Really?

Original comment by: elementgreen

[derselbst]

Original comment author: berarma

I must correct myself, FS doesn't implement the default modulator as I said above, it just cancels any modulator defined in this way.

Christian's original problem might be that FS wasn't matching modulators correctly because spec 2.01 was misinterpreted and now it's superseded by corrected version 2.04 too.

I think we should stick to the 2.04 spec that seems correct and maybe gather information about bad soundfonts if we're going to implement a workaround for them.

Original comment by: elementgreen

[derselbst]

Original comment author: berarma

I'm using FS with this patch that follows 2.04 specs and it sounds good with the soundfonts I've tried it, no big differences though. I'd like to try Christian's soundfont but he's not hosting it anymore.

I think we should apply this patch and figure out which soundfonts have problems and why. If they are related to this modulator cancelation we could fix it when loading the soundfont by checking for these bad modulator variants.

Original comment by: elementgreen

[derselbst]

Original comment author: berarma

I've been able to download the test soundfont by Christian and I've looked a bit more into this.

Soundfont creators can delete default modulators from their soundfont editor, but, actually, it will create a matching modulator that cancels the default one. The problem is that since the definition for the default FC modulator has changed from 2.01 to 2.04, modulators won't match.

The default FC modulator defined in 2.01 is strange, it would be interesting to know if is implemented by any soundfont engine, maybe not. Following 2.04 specs seems more sane but it's important to be able to match the 2.01 modulator so it can be removed for soundfonts following that spec.

Original comment by: elementgreen

[derselbst]

Original comment author: Mr_Bumpy

For those who have been unable to download my test SoundFont, here is the updated link: http://www.schristiancollins.com/temp/GeneralUser%20GS%20FluidSynth%20v1.43%20RC1.sf2.bz2

Original comment by: elementgreen

[derselbst]

Original comment author: Mr_Bumpy

Berarma, FYI, the default FC modulator is implemented on the Creative Audigy that I have, so that's one SoundFont engine that supports it.

Regarding my original message to fluid-dev, my fix was rather a hack to just disable the default velocity->filter cutoff modulator altogether, since FluidSynth ignores the deletion of said modulator in my patches. So, the first step would be to get FluidSynth to notice when the default vel->fc modulator is deleted at the instrument level and act accordingly.

Original comment by: elementgreen

[derselbst]

Original comment author: berarma

Thanks Christian. Everything seems clear now. I've committed a fix that applies the correct default modulator depending on sound font version, thus, cancellation is done rightly.

I've done some tests with 2.01 sound fonts, but I haven't tried any 2.04 sound font. Christian's test sound fonts and explanations have helped a lot since I don't have a SF capable soundcard, looking forward to more of this collaborations. Tests, comments and corrections welcome.

Josh, unless something bad happens this might close the ticket. I've touched one external declaration in sfont.h, so this could break binary compatibility in some cases. This is almost inevitable when there's so many internals exposed.

Original comment by: elementgreen

[derselbst]

berama: Great that you are addressing this issue. I had for some reason thought it was resolved. I think breaking backwards compatibility in this case, isn't really worth it though. Perhaps a better way to handle this would be to detect either form of a Velocity to FC canceling modulator in fluid_voice_add_mod(), with or without the secondary switch source and set the corresponding default modulator value to 0. How does that sound?

Original comment by: elementgreen

[derselbst]

Original comment author: berarma

Josh: That would address the cancellation issue but it wouldn't set up the correct default modulator for the sound font. It's not easy to solve in another way since the sound font loader and the synthesis process are highly coupled, but we can hear other suggestions.

The biggest problem I had is knowing which bank or bank version the sample came from. I had a a similar problem when trying to implement the polyphonic key pressure effect, because some things aren't well connected and defined by FS structures. Finally I gave up because few keyboards implement it although MIDI files could use it too.

Original comment by: elementgreen

[derselbst]

berama: I'm not quite seeing what the problem is, that you mention. I think the 2.04 functionality should be adhered to always for FluidSynth. I don't believe any SoundFont author would expect the 2.01 behavior, since its a rather abrupt transition of the filter effect based on velocity. So the default should be the velocity to FC as described be 2.04 and either form of disabling it should be honored. I also think FluidSynth should have a command line option to turn off this default modulator altogether, or even better, allow for any default modulator to be added or removed (Swami has this functionality). Does that make sense? Perhaps I'm still overlooking something.

Original comment by: elementgreen

[derselbst]

Original comment author: berarma

Josh: I thought the same but as it seems that this is the expected behavior for 2.01 sound fonts, and there's a lot of them, we should follow it if we want to sound as the original. Maybe I'm being a bit purist but I guess sound font designers would expect their sound fonts to work the same on every synthesizer engine that claims to be 2.01 compatible.

I agree we could overlook this change for FS1.x and just implement the 2.04 default modulator, then we should guess the cancellation modulator for 2.01. Sound fonts with the default 2.01 modulator will sound different but we could live with it. If we are to maintain API compatibility this might be the way to go.

I'm ready to do it unless someone comes up with another solution.

Original comment by: elementgreen

[derselbst]

berama: That is just it though, I don't think anyone would want the 2.01 functionality, since its rather broken. I seem to remember some note about despite the fact that the 2.01 defined it with a second switch source, most implementations ignored that. The 2.04 behavior is definitely better, but even then, I'm not even sure why this is a default modulator.

Original comment by: elementgreen

[derselbst]

Original comment author: Mr_Bumpy

I have updated GeneralUser GS and the corresponding Rosegarding instrument file. You can find the new files on my GeneralUser GS page.

Secondly, I agree that the SoundFont 2.01 default modulator definition should not be adhered to for the reasons that others have brought up (it's broken, etc.). I don't believe any SoundFonts out there will be made worse as a result. I believe sticking to the 2.04 specification and allowing the modulator to be disabled or changed is the best way, so I'm agreeing with Josh here.

If you have as much as one SoundFont developer upset over the above mentioned interpretation, I would be very surprised.

Original comment by: elementgreen

[derselbst]

Original comment author: berarma

JFTR. I would like to stick to specifications and existing compliant implementations as closely as possible. Although this one might not be very important except for some exceptional case, it's very easy to fix. The only reason to avoid fixing it in a compliant way that I cant think of is that it would change the API for the stable branch, so I'm keeping this fix in the 2.0 branch.

Original comment by: elementgreen

[derselbst]

@mawe42 I would like to get rid of this hack:
https://github.com/FluidSynth/fluidsynth/blob/master/src/synth/fluid_mod.c#L327-L342

What is your opinion of handling velocity 2 attenuation modulator? Should it be default according to 2.04 spec? Should we have a setting to let the user decide whether it should be default? Or should we completely remove it from the code and the user would still have the option to manually add such a modulator to any instrument?

Edit: Here's a possible solution (not my preferred one): b1ce120

[mawe42]

You mean velocity to filter cutoff, correct? Well, musically I think it makes no sense to have this as a default modulator. Especially not the 2.01 version of it. And I've read the old mailing lists posts, the discussion in this ticket and the commit logs, but I'm a little confused.

If I understand it right, then elements last decision was to implement the 2.04 vel2fc modulator, add a setting to turn it off, and detect if a soundfont tries to disable either the 2.01 or 2.04 version and react accordingly. That sounds like a good plan to me.

[mawe42]

And for the 2.01 disabling detection, allow both types: the one according to 2.01 spec with negative polarity for source2, and the one mentioned in the code and used in the "Kingston Piano" with positive polarity. So don't check source2 polarity at all.

Then we've got another hack, but I think this one can't be avoided. And it is still much nicer than the current way of doing things...

[derselbst]

Oh yes vel2fc (copy'n paste :) ).

Well, musically I think it makes no sense to have this as a default modulator.

Same opinion. We have been pretty successful although ignoring it. And if there is an instrument where it would really make sense, the user could still manually add a modulator for it.

and detect if a soundfont tries to disable either the 2.01 or 2.04 version and react accordingly... Then we've got another hack

I was actually not intending to fight a hack with yet another hack. I could imagine that there might be cases causing unwanted behaviour.

[mawe42]

So you would rather have no vel2fc modulator by default and add a setting to enable 2.04 behaviour on request? That sounds fine by me. Then FS doesn't behave according to spec, but with the current hack it hasn't for a long time anyway. And nobody seems to have noticed or cared :-)

[derselbst]

Not quite sure regarding the setting yet. The advantage of having no such setting would be that any vel2fc modulator hack in any soundfont could be safely removed. It would be absolutely clear to the user that if any filtercutoff is needed, one would have to manually add a custom modulator (and I dont think there would be many instruments requiring that). Introducing a setting however would mean that the user must be aware of whether this must be turned on or off and this again could vary from file to file... not the clean solution I was looking for.

[derselbst]

In fact the amount of this modulator really matters and heavily depends among different instruments. E.g. listening to a standard piano with the default 2.04 vel2fc mod I cannot tell the difference whether the mod is on or off. I have to at least triple the amount to hear any effect.

So I really think this should be manually specified on a per instrument basis by the user. Having this as def mod seems to be pointless to me. But I'm a programmer, no musician, so...

[mawe42]

The advantage of having no such setting would be that any vel2fc modulator hack in any soundfont could be safely removed.

But only if those soundfonts are only used by FluidSynth. There are many other engines out there, some of which probably implement the default vel2fc modulator. So if I would create a new SoundFont, I would always add the vel2fc override (unless I want this effect, of course).

[derselbst]

Main engines like SynthFont, VirtualMidiSynth and TiMidity dont support it either, see a test done by Christian Collins. And some further info here.

[mawe42]

Ah, very interesting! Well, that seems to settle it, doesn't it? Remove vel2fc default modulator completely but allow for custom modulators to be added. But what Christian Collins says about FluidSynth in the compatibility chart is not quite correct:

FluidSynth is right, in my opinion, to leave a default curve out of their implementation, while allowing custom modulators to function normally.

Only the not-quite-to-spec default modulator with a positive unipolar switch on the secondary source is ignored. The negative unipolar switch as specified in 2.01 is still applied. And "allowing custom modulators to function normally" is only true if the custom modulator doesn't use the same setup as the default one but with a different amount.

And that's the thing that is still bugging me: if they decided a few years ago that ignoring the 2.01 vel2fc is the way to go, why didn't they just remove the default modulator but decided on this hacky (and incomplete, it seems) override in fluid_mod_get_value?

[mawe42]

Only the not-quite-to-spec default modulator with a positive unipolar switch on the secondary source is ignored. The negative unipolar switch as specified in 2.01 is still applied.

Ignore this please, I was confused. There is no vel2fc with negative unipolar switch as secondary source set up as default modulator in FluidSynth. So there's no need to ignore that as well.