logicomacorp / WaveSabre

Official WaveSabre repository
MIT License
247 stars 34 forks source link

Proposed fixes for the biquad filter and Leveller device #69

Open armak opened 3 years ago

armak commented 3 years ago

Having done some investigations, I've identified three different issues related to the Leveller EQ/filter device. I'm including all of these in the same issue since in some ways they are interrelated and something that I think would be good to try to fix in the same go. The issues in order of importance are:

1. Biquad filter imprecision at low cutoff frequencies

The current biquad filter implementation (based on the classic RBJ digital filters[1]) suffers from imprecision when the cutoff frequency becomes very low, specifically lower than about 200 Hz, but it's especially bad below 100 Hz. In this picture the cutoff of the Leveller low cut filter is to 20 Hz, and Q is set to neutral 0.70710678... (sqrt(2)/2), yet the filter peaks at about +4dB. Just moving the cutoff up to above 200 Hz gradually improves the filter behavior and it ceases to exhibit this peaking behavior (ignore the peaking at the high end of the spectrum, this is a separate issue that I will talk about later).

biquad q problem

Also worth noting that even though the cutoff is set to 20 Hz, the resonance isn't actually at 20 Hz, but rather closer to 50 Hz or just below it. This appears to be an additional issue produced by the imprecision. We can also try to compensate this peaking by reducing the Q below the neutral 0.7071... value, but we have to assign the Q very close to zero in this case to eliminate the problem. It also doesn't fully resolve the problem, because the filter still starts cutting way too high and is -3dB at around 40 Hz rather than the intended 20, whereas at 20 Hz were down almost -13dB:

filter problem 2

The issue seems to stem from the lookup tables used for the trigonometric functions. Simply increasing the size of the table from the current 512 entries appears to eliminate the problem completely. 1024 entries is a lot better but still exhibits audible peaking, 2048 entries only has a slight amount of error, and 4096 is near perfect.

The magenta/lila colored graph is the current version, and the pink/salmon colored is with increased LUT size (4096 samples). Notice that the filter is now correctly -3dB down at 20 Hz.

leveller q problem

There might be another less of a "brute force" way of fixing this, but increasing the LUT size seems effective anyway. Only performance consideration from it might be cache efficiency, but this remains to be tested.

2. Using neutral Q by default

In the current version the Leveller device and the helper functions that map scalar values to Q and vice versa treat 1.0 as the neutral value: all Leveller Qs are initialized to this, and it's the center 50% setting in all of the Q knobs. However, as with most filter and indeed the RBJ biquad as well, the neutral Q is actually the aforementioned square root of two divided by two, or 0.70710678... .

I would suggest both initializing all Q properties by default to this value, and re-scaling the Q mapping functions so that the middle Q would also map to the same value (might be worth re-thinking the entire function as well). This would eliminate the remaining peaking behavior the low and high cut filters currently have by default, and make it much easier to find the exact neutral Q setting on the knobs.

You can see the previous image for an example of using the neutral Q in Leveller, with the aforementioned biquad problem also addressed.

3. Cutoff frequency mapping ranges

This is less crucial than the other ones in my opinion, but I still wonder whether the range of a cutoff parameter could or should be expanded. The current ParamToFrequency and FrequencyToParam helper functions implicitly define the range as 20 Hz to 20 kHz, which certainly is a sensible range, but filters still start cutting frequencies already before those cutoffs are reached. Sometimes you might want to extend e.g. the cutoff of a highpass filter to below 20 Hz, so that no potentially audible frequencies would be cut. Same applies to the top end.

Of course most digital filters have the issue that they start cramping near the nyquist, so we can't extend the upper range all the way there, but just below it. Not sure how much of a margin is necessary in practice before numerical problems start to appear, but a 22 kHz cutoff seems to still be fine at a 44.1 kHz sampling rate.

Finally, below is a comparison of graphs between the current Leveller device at its initial settings, compared to a possible "Leveller 2" with all of the proposed changes above, at default settings as well.

leveller full changes

[1] https://www.musicdsp.org/en/latest/Filters/197-rbj-audio-eq-cookbook.html

yupferris commented 3 years ago

While not directly related to the issues above, another thing we've discussed previously is that the current param <-> freq mapping is too coarse at low frequencies. Admittedly, the currently-used curve is something I just threw together as a "nonlinear mapping" and is basically x^2 mapped to the desired cutoff range. It seems the desired curve is typically logarithmic (eg exp(ln(20) + x * (ln(20000) - ln(20))), proposed here). I graphed the two to compare:

bilde

Clearly, the logarithmic curve offers significantly better resolution in the lowend (as well as other crucial parts of the spectrum), just like we want. Further, the curve is the same regardless of the base used, which is convenient for us, as we already have an implementation of exp2 in Helpers. This implies that we'd likely need to add log2 as well, but this may not be the case - if we can get away with only ever doing the forward mapping, then because the inputs to log2 are only ever constants, constant folding removes the calls altogether.

Thus, I propose the new mapping be exp2(log2(low) + x * (log2(high) - log2(low))) where low=10 (judging from the -3dB point in your graph above) and high=22000, with no inverse; if an inverse is desired, it should be (log2(y) - log2(low)) * (1 / (log2(high) - log2(low))) and the log2 shim, if required, is then trivial to write using fyl2x.

Admittedly, though, I have yet to actually try such a mapping (nor the extended range) in an actual plugin; these are just some notes as I happened to think about it. It should be tested, preferably against another plug like Pro-Q 3.

EDIT: Just to make the logarithmic mapping a bit clearer, I generated a log-linear plot as well:

bilde

Even though I've been a bit lazy with the labels, I still find it more intuitive to imagine scaling the parameter linearly and using this plot to determine what the frequencies would be. I'm guessing this would feel a lot better. In fact, even just comparing against a screenshot of Pro-Q 2:

bilde

Just to pick a point of reference, in both pictures, 1kHz appears to be mapped to ~0.6, which is a good sign.

yupferris commented 3 years ago

Also, to clarify: the peaking behavior of the LPF with default settings currently, is that only due to the current “neutral” Q mapping, and/or is it also related to the trig tables like the HPF?