search

FluidSynth / fluidsynth

Software synthesizer based on the SoundFont 2 specifications
https://www.fluidsynth.org
GNU Lesser General Public License v2.1
1.88k stars 260 forks source link

strange audio distortion when using filter envelope after merging #1345 #1417

Open mrbumpy409 opened 1 week ago

mrbumpy409 commented 1 week ago

FluidSynth version

2.4 (and master)

Describe the bug

Since #1345 was merged, there is now a strange audio distortion when the filter is modulated by the modulation envelope. I created a demo MIDI file and audio renderings that should hopefully make this quite evident: filter envelope noise.zip

Inside the zip file, you will find the following files:

It is unfortunate that such a substantial change was made to FluidSynth's behavior so soon before a major new release. From my perspective, FluidSynth's filter behavior was far better than any other SoundFont synth I had tested, and now it's broken. I'm really upset that I didn't have the time to test this until now, otherwise I would have been able to say something sooner.

ReinholdH commented 1 week ago

I can confirm the distortion in the sample and agree to all your points. More to #1415.

derselbst commented 1 week ago

It is unfortunate that such a substantial change was made to FluidSynth's behavior so soon before a major new release

Let me clarify a few things:

Yet, I appreciate your input Chris and will look into that when I find the time.

klerg commented 1 week ago

I must admit the Roland Sound Canvas sample has the most clicks and cracks. And the distortion is more pronounced at around 7 seconds or so in FluidSynth 2.4 sample. My hope is that it can be fixed or at least reduced

derselbst commented 4 days ago

I started investigating this. The good thing is, that we now got several different use-cases and test data, thanks for that.

I incorporated the test data into the buildsystem's unit tests. The data is available in the following directory, on the iir_tests branch currently: https://github.com/FluidSynth/fluidsynth/tree/iir-tests/test/manual/iir_filter

The tests can be run during build by executing make check_manual. This will render the tunes into build/test/manual/iir_filter/, so one can at least manually verify by listening whether everything works as expected when changing the filter implementation.

Additionally, in order to get a better understanding of the filter's behavior, I created an interactive Bode plot by using Matlab. The script is also available in this directory.

It let's you adjust fc and Q with sliders. One can observe that the filter's phase is quite sensitive whenever any of those two parameters change.

The clicks heard in all three issues (#1415, #1424, and this one) are much more subtle and decent, compared to the artifacts I attempted to fix by #1345. I believe the distortions here (and clicks in the other two issues) are the result of the filter's phase changing rapidly, i.e. causing the frequency-dependent delay inside the filter to change. Therefore I agree with Chris' comment that the fc (and ideally Q as well) need to be smoothed out.

In the next days I'll attempt to implement this smoothing by using a linear interpolation of the fc. We'll see how this works.

Currently, I still believe that #1345 itself was correct. I don't see how the linear interpolation of the filter coefficients itself can preserve any state of the filter at all - the coefficients do not change in a linear way, but rather in a sinusoidal way. So yes, while there are cases where this linear interpolation of the coefficients apparently yields the desired effect (as heard in the example files posted by Chris here), it can also break stuff in a much more louder way as heard in #1345.

spessasus commented 4 days ago

Looks like:

Therefore, I think #1345 should be reverted and AWE implementation changed. There's probably a good reason for the coefficient interpolation, given that it stayed for 13 (17?) years without any issues. Reverting and fixing AWE would fix this bug (along with the one i reported) and nervous filter would be fine too.

And then there's no need to interpolate the Fc or Q in ModEnv, which is not good according to Christian Collins:

If NRPN control of filter cutoff is causing pops, you might want to incorporate "smoothing" logic to limit the speed at which a NRPN or modulator can increase/decrease the cutoff, though I would advise against also applying this smoothing logic to modulation from the modulation envelope, as this may mess with the sound of presets.

derselbst commented 4 days ago

Why are you so focused on the new AWE NRPN feature? The NRPNs just modulate the fc in a way, that could be achieved with any custom modulator and CC as well. It was just that particular MIDI file, modulating the fc in that particular way to cause the observed artifacts.

AWE implementation changed. [...] Reverting and fixing AWE would fix this bug

So, what exactly makes you believe that the AWE implementation needs to be changed / reverted / fixed? Can you pls. be a bit more specific?

There's probably a good reason for the coefficient interpolation, given that it stayed for 13 (17?) years without any issues

Preservation of tradition is not a legitimate reason. Pls. argue on a technical level. I have tried to explain that the clicks are related to the filter's changing phase whenever fc and Q are changed. For the filter it doesn't matter through which mechanism these parameters are changed.

And then there's no need to interpolate the Fc or Q in ModEnv

I'm planning to introduce a smoothing, that is agnostic of what has caused the parameter changes. Just like the previous coefficient smoothing also was agnostic of its cause. If all that effort doesn't yield the desired effect, reverting #1345 will obviously be a last resort.

spessasus commented 4 days ago

So, what exactly makes you believe that the AWE implementation needs to be changed / reverted / fixed? Can you pls. be a bit more specific?

Sorry for not being specific, I simply meant to smooth out AWE params instead of messing with the filter, for example by setting a target value when the NRPN is received and changing fc to it slowly enough for no clicks. I'm not against removing this feature (i meant reverting the PR, not AWE!), I just want to keep the filter as it was before, because it just worked and it worked perfectly. Now it does not.

I hope this clears things up for you : )

derselbst commented 3 days ago

I have applied the previous linear smoothing logic to the fc only in f123fa127830dcfca7d06866e6d414bde7a9c173. With that clicks of #1417 and #1424 can no longer be heard. #1415 still has a potential for clicks, which is likely due to the fact that it also changes the filter's Q, for which I'm currently not accounting at all. But I think I'm on the right track. I'll try to get one or two more tests incorporated esp. with the one observed in #1345. When it's ready, I'll provide the renderings for comparison so you can have a listen as well.

I just want to keep the filter as it was before, because it just worked and it worked perfectly.

I'm glad that it worked for you. I've been messing around with the filter for the past seven years using custom modulators, and I can tell you that it wasn't "perfect" and probably will never be. Yet, implementing the smoothing on NRPN-level only is not a meaningful solution.

mrbumpy409 commented 2 days ago

I can confirm that the current iir-tests branch eliminates the distortion that was introduced after the merge of #1345.