Skip to content
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

Audio Limiter Effect Introduces Clipping Instead of Preventing It #36631

Closed
Tracked by #76797
jitspoe opened this issue Feb 28, 2020 · 19 comments
Closed
Tracked by #76797

Audio Limiter Effect Introduces Clipping Instead of Preventing It #36631

jitspoe opened this issue Feb 28, 2020 · 19 comments

Comments

@jitspoe
Copy link
Contributor

jitspoe commented Feb 28, 2020

Godot version: 3.2, 3.1

OS/device including version: Windows 8.1

Issue description: Audio Limiter effect does not reduce volume to avoid clipping. Instead, it increases volume, and introduces clipping where there was none.

Example of sound after going through Limiter:
image
Note how the waves spike up after exceeding certain values.

Sine wave example.
Before:
image
After:
image

Steps to reproduce: Add Limiter effect to master bus with default settings. Play a sound that exceeds the soft clip threshold.

Minimal reproduction project:
test_audio_limiter.zip
WARNING: TURN YOUR VOLUME DOWN BEFORE PLAYING!

@Calinou
Copy link
Member

Calinou commented Feb 28, 2020

cc @reduz, as he implemented the various audio effects.

@akien-mga akien-mga added this to the 4.0 milestone Feb 28, 2020
@jitspoe
Copy link
Contributor Author

jitspoe commented Feb 29, 2020

Looking at the code, it seems like it's not fully implemented. There's a property, soft_clip_ratio, which is not used. Also, the logic seems to be operating on a purely per-sample basis. I've never implement a limiter before, but it seems like they typically have some sort of attack and decay when sound crosses some threshold, so it should scale the volume down for a bit, then back up again over time using a curve, which would require some sort of persistent data to track that volume change.

@Calinou
Copy link
Member

Calinou commented Feb 29, 2020

I've never implement a limiter before, but it seems like they typically have some sort of attack and decay when sound crosses some threshold,

Isn't that the job of a compressor audio effect? (AudioEffectCompressor)

@jitspoe
Copy link
Contributor Author

jitspoe commented Mar 1, 2020

Compressors and limiters are very similar. To be honest, I've never fully understood the difference between them.

@Calinou
Copy link
Member

Calinou commented Apr 12, 2020

@jitspoe This page explains the difference between a compressor and limiter.

@freemanfromgodotengine
Copy link

I can confirm this bug. In my experiments parts of the amplitude selected by Celling Db and Treshold Db are being distorted, so basically limiter does not work at all in my Godot 3.2.1.

Like @jitspoe nicely presented, limiter clearly does not attenuate the high amplitude part of the sound wave. Actually it sounds like it does contrary and distorts those fragments even more by increasing their level.

@morphiser
Copy link

It's questionable what the limiter in godot is trying to achieve. Usually a limiter uses a little bit of lookahead to calculate the signal energy and then regulates the gain accordingly (with an adjustable attack/release time). The current implementation rather seems to aim at a soft clipping function. Clipping does not have any time dependencies but generally leads to some signal distortion (introducing harmonics and aliasing), which may or may not be desired.
An actual limiter is usually used on the master bus to prevent digital clipping of the mix.

@Calinou
Copy link
Member

Calinou commented Aug 25, 2020

reduz implemented all the audio effects but he's currently busy working on the Vulkan renderer for 4.0. That said, it might be possible for a contributor to chip in and contribute a pull request 🙂

The source file to modify for this is servers/audio/effects/audio_effect_limiter.cpp.

Make sure to open the pull request against the master branch (we can cherry-pick it to the 3.2 branch later).

@Tony-Goat
Copy link
Contributor

I implemented a temporary solution for this to work for now. I'm no audio engineer, so my solution may fall apart under certain circumstances.

@morphiser
Copy link

morphiser commented Aug 29, 2020

@Tony-Goat I couldn't really reproduce your result with this patch. Which limiter parameters did you use for this screenshot?

@Tony-Goat
Copy link
Contributor

@Tony-Goat I couldn't really reproduce your result with this patch. Which limiter parameters did you use for this screenshot?

The results I posted are from the default settings. I used the test project provided here.

@akien-mga
Copy link
Member

Is this still reproducible in 4.0 RC 3 or later?

@freemanfromgodotengine
Copy link

Is this still reproducible in 4.0 RC 3 or later?

Yes. Problem remains in 4.0-stable released yesterday.

@akien-mga akien-mga modified the milestones: 4.1, 4.2 Jun 23, 2023
@wareya
Copy link
Contributor

wareya commented Nov 3, 2023

Limiters should never have any kind of soft clipping or hard clipping configuration. If they have it, they're not limiters, they're waveshapers.

Proper limiters are very similar to peak compressors with an infinite compression ratio, but they have a different type of attack curve. Compressors have a type of attack curve that allows part of the audio signal to temporarily be louder than the limit. Limiters have an attack curve that does lookahead to prevent this from happening, or just zero attack if lookahead is too hard to implement. (Compressors also usually use RMS instead of peak amplitude, and limiters always use peak amplitude, but there isn't a significant conceptual difference between the two.)

Here's an example:

image

Allowing the compressor effect to go down to zero attack would allow people to use it as a limiter. However, "good" limiters have lookahead, adding a small amount of latency (usually 1ms or 2ms) to allow for an attack curve without causing the initial transient spike to go above the limit. This is necessary because otherwise the upswing part of loud signals gets clipped as the limiter's compression value goes down every sample to meet the new peak.

Robust attack curves for limiters are basically blurs applied to the envelope. If you implement attack this way, you also need to implement "sustain", or else the envelope won't fully envelope the signal during transients. You can implement sustain by basically doing max(sample[t], sample[t-1], sample [t-2], ...). For long attack/sustain values, this is expensive unless you use a bucketing circular buffer, where every N samples gets bucketed into its own max bucket and you only recalculate the buckets that have had a sample changed.

If you don't want to deal with the complexity of a lookahead/sustain system, you can "over-limit" signals when they cross the limiting threshold, by setting the compression value to like, 0.9 * limit_amplitude / actual_amplitude instead of limit_amplitude / actual_amplitude, when the actual amplitude is greater than the limit amplitude.

@wareya
Copy link
Contributor

wareya commented Nov 4, 2023

I just implemented a correct limiter in gdscript and uploaded it under a public domain license (CC0) here:

https://github.com/wareya/LimiterTest

Main.gd contains a limiter that implements release, sustain, and attack. The core logic of it is around 50 lines long (90 lines total including declarations and initialization etc). As examples of why it has to be 50 lines long, MainNoAttack.gd and MainSimple.gd implement limiters that lack sustain and/or attack; they produce distorted output.

In case I never get around to it, I give permission to anyone to port this to C++ (or do absolutely anything with it in general; it's CC0).

Here's what the output sounds like for three sine waves added together, faded out, and multiplied by 4.0 before being limited:

(make sure to unmute)

2023-11-03_20-54-27.mp4

@YuriSizov YuriSizov modified the milestones: 4.2, 4.x Nov 15, 2023
@lander-vr
Copy link
Contributor

lander-vr commented Feb 27, 2024

I tried to implement the GDScript example limiter @wareya made in C++ (through GDExtension currently). It limits, however there is an issue where it seems to miss the first few samples that are above the ceiling threshold, and it doesn't limit those.

I admittedly don't fully grasp how those buckets and memory arrays work, as well as why the box_blur variables are multiplied by 32767.0, to then later at output be divided again. @wareya (or someone else who knows) if you have some time, it would be great if you could give some further explanations on these, if you wouldn't mind taking a peek at this.

Either way thanks for sharing that GDScript example project, I'm learning a bunch!

This screenshot shows a constant very quiet white-noise, and a single loud sine beep played over top. Notice those first loud samples of the since beep (which just go through the limiter completely unlimited), and the actually limited samples that come after.
image

However, "good" limiters have lookahead, adding a small amount of latency (usually 1ms or 2ms) to allow for an attack curve without causing the initial transient spike to go above the limit.

It seems like this is the issue I'm experiencing, but I'm not sure what the best way would be to go about adding that latency.

@wareya
Copy link
Contributor

wareya commented Feb 28, 2024

I'll try to take a look at some point within the next couple days.

The bucketing system is a fast way to calculate the max() of a large series of numbers without having to recalculate the whole thing each time. Instead, it only has to recalculate a small fraction of it.

The 32767.0 stuff is part of a way to implement a moving-window box blur with only two additions/subtractions per timestep instead of having to recalculate the whole thing from scratch every timestep. Doing this with floats is numerically unstable, so it has to do them with integers instead. Multiplying by 32767 allows it to convert to the equivalent of signed 16-bit PCM instead of everything getting rounded to -1, 0, or 1. (Float audio data is normally in the range -1.0 to 1.0.)

@lander-vr
Copy link
Contributor

lander-vr commented Feb 28, 2024

Thanks for the insights, that actually helps a lot with my understanding of it.

I had a typo the "update affected bucket" section which meant I was taking the wrong element from the amp_memory array to store into the amp_min_buckets.

Context of what that typo caused:

  • Only the amp values of the first couple of samples in an audio frame were being stored, which meant that unless the limiting already took place during those first samples of the current AudioFrame, it'd store a non-limiting amp value in that bucket. This caused the very first sine-beep to be limited properly, but any other weren't. This also meant limiting was delayed based on at what point in the AudioFrame the sine-signal started playing, creating inconsistent delays in how fast the limiter would take action (essentially only until the next AudioFrame started)

Having fixed that, I get these results on a normalized sine beep. Limited with a ceiling of: 0dB, -6dB, -12dB, -18dB, and -24dB, giving us the expected halving each time.
image

Zooming into a limited signal, you can see the waveshape is preserved:
image

It seems to work as it should now, I've tested several sound files, boosting the sound way beyond the ceiling and it never goes over the ceiling. I will attempt to make an implementation of this into the engine.

I don't know if it should act as a replacement of the current limiter though, or if it should be implemented as a new hard limiter. Though the current limiter in the engine doesn't work, it was implemented as a soft limiter.

My suggestion would be that we replace the current "limiter" effect in the engine, because boosting a signal when the user is expecting it to be limited can be dangerous hearing-damage wise. A working "soft limiter" effect can then be implemented as a separate effect.

Edit:
An issue is that this limiter works quite differently compared to the one that was originally implemented, it doesn't make use of the same inputs (ceiling dB, attack, sustain, release VS ceiling dB, threshold dB, soft clip dB, soft clip ratio). I'm guessing this will make backwards compatibility tricky to do if it replaces the original limiter, however, given that the original limiter didn't work in the first place I'm also not sure how big of an issue that is, or if it can be worked around somehow.

@akien-mga
Copy link
Member

Should be fixed by the new AudioEffectHardLimiter added in #89088.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants