Fix floating point errors #57
Conversation
* swh - dyson_compress_1403, fix NaN * swh - shaper_1187, division with 0
dyson_compress_1403.xml
Outdated
| <name>Release time (s)</name> | ||
| <p>Controls the time taken for the compressor to relax its gain control over the input signal.</p> | ||
| <range min="0" max="1"/> | ||
| <range min="0.0000001" max="1"/> |
There was a problem hiding this comment.
I chose this number to be small enough for the value on the knobs in lmms to round off to 0. We do some odd math for our knob values so this need to be tested. The value is only used in one place though so it may be cleaner to change:
float rgainfilter = 1.0f / (release_time * sample_rate); to something like:
float rgainfilter = 1.0f / f_max(release_time * sample_rate, 1.0f);
| } else if (shape < 0) { | ||
| shape = -1.0f / shape; | ||
| } else if (shapep < 0) { | ||
| shape = -1.0f / shapep; |
There was a problem hiding this comment.
The commit message says division with 0 but that's wrong. shape < 0 never is true so we drop to else for any number that is negative and feed this as exponent in a pow() operation. No fun Not the fun you intended .
|
Sorry, I'm away on business at the moment, and don't have time to review the changes in context. Should this be merged in its current state, or are there still open questions? |
We're changing the |
PEMDAS will allow it, but let's be consistent. ;)
|
@swh I think this new logic is better. [IN] [OUT]
BEFORE: release_time: 0.25 rgainfilter: 176400.000000
AFTER: release_time: 0.25 rgainfilter: 176400.000000
BEFORE: release_time: 0.001250 rgainfilter: 35280000.000000
AFTER: release_time: 0.001250 rgainfilter: 35280000.000000
- BEFORE: release_time: 0 rgainfilter: inf
+ AFTER: release_time: 0 rgainfilter: 441000001536.000000Edit: Updated input from ms (lmms), to secs (swh). Should be good to squash and merge. |
|
Looks good to me. |
3 participants
Cherry-pick from LMMS/lmms@88b940f.
@zonkmachine, a second set of eyes would be nice :)