-
Notifications
You must be signed in to change notification settings - Fork 275
feat: add audio peak limiting with configurable settings #751
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
Conversation
src/source/amplify.rs
Outdated
} | ||
|
||
/// Internal function that converts decibels to linear | ||
pub(super) fn to_linear(decibels: f32) -> f32 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we move these methods to mod.rs
or some utility module?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have src/math
it could fit there. Feel free to move it and rename it to decibels_to_linear()
. But its also fine where it is right? this is always amplifying, sometimes a number sometimes a source.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved and renamed them to src/math.rs
in b1497f8. It doesn't feel right to me that we were adding a utility function to_db
to a module that didn't use it. I also made them public, because they are useful as general-purpose audio functions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
they are useful as general-purpose audio functions
I don't follow exactly. We have different amplify methods on source for the different scales. How does converting a single float to another float help audio? Do you mean they are useful as parts for custom sources?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a user of this library, I frequently run into the need to convert one into the other and display it to the end user. For example: Deezer, Qobuz and Spotify all send linear values and I want to print their dB equivalents on screen. Easy peasy when Rodio has them public.
I love the settings example showing some analysis! It shows a different application of rodio then playback with effects. Could we augment that example with 2 of the examples from #750? I would say the "compressor_wav" and "compressor_wav_alternate.rs". We could rename those to "compressor_basic" and "compressor_changing". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation is spot on, its a nice balance between explaining audio and being to the point.
I fear we need to talk about our nemesis spans again. Specifically what happens when the channel count changes. Do we need to switch from limit::Mono
to Limit::Stereo
then?
The rest is minor stuff: language preference (you can ignore if you disagree), a test that's not needed, comments that are superfluous etc.
Thank you for your review.
I did not add anything like |
Provides a new LimitSettings API and limiter filter to prevent audio peaks from exceeding a threshold. Supports soft-knee limiting, per-channel detection, configurable attack/release, and efficient processing for mono, stereo, and multi-channel audio. Includes comprehensive tests and usage examples.
…dule - Add LimitSettings presets for common use cases (dynamic_content, broadcast, mastering, gaming, live_performance) - Expand and clarify LimitSettings documentation and dBFS reference - Move dB/linear conversion functions and tests from amplify.rs to new math.rs module - Refactor limiter implementation for clearer internal structure and documentation - Add new example (limit_wav) and integration tests for limiter behavior - Clean up and modernize limit_settings.rs example - Update module imports to use new math API
e47684a
to
e61dce9
Compare
Then lets not do that 👍 |
ready to merge now right? |
👍 I will squash merge now. |
I may have some confusion regarding terminology, how such limiter is different to a compressor? Compressor also has attack/release time parameters, with perhaps only difference that below-the-knee-threshold samples are also affected. |
A limiter is also a compressor, a particular type of it. Any compressor has a threshold (amplitude) beyond which it starts reducing dynamic range with a certain ratio. For a limiter, that ratio is very high or infinite. |
Provides a new LimitSettings API and limiter filter to prevent audio peaks from exceeding a threshold. Supports soft-knee limiting, per-channel detection, configurable attack/release, and efficient processing for mono, stereo, and multi-channel audio. Includes comprehensive tests and usage examples.