(fix) AnalyzerEbur128: catch invalid return values inf/-inf #13680
Conversation
ronso0
force-pushed
the
libebur-debug
branch
from
9f57755 to
6306f91
Compare
|
I can reproduce it with a silent track (empty Mixxx recording, ~3 sec) |
|
right, makes sense that you'd need "infinite" gain to get that to a fixed level. This should be caught but libebur though. |
src/analyzer/analyzerebur128.cpp
Outdated
| // This catches inf and -inf that slipped through in libebur for some reason.. | ||
| // TODO Move this to a VERIFY_OR_DEBUG_ASSERT() once the root cause | ||
| // has been identified. |
There was a problem hiding this comment.
| // This catches inf and -inf that slipped through in libebur for some reason.. | |
| // TODO Move this to a VERIFY_OR_DEBUG_ASSERT() once the root cause | |
| // has been identified. | |
| // This catches inf and -inf that slipped through in libebur for some reason. | |
| // TODO: Move this to a VERIFY_OR_DEBUG_ASSERT() once the root cause | |
| // has been identified. |
src/analyzer/analyzerebur128.cpp
Outdated
| // This catches inf and -inf that slipped through in libebur for some reason.. | ||
| // TODO Move this to a VERIFY_OR_DEBUG_ASSERT() once the root cause | ||
| // has been identified. | ||
| util_isinf(averageLufs) != 0 || |
There was a problem hiding this comment.
util_isinf introduces a insymmetry compared the the C89 HUGE_VAL constant.
How about this:
| util_isinf(averageLufs) != 0 || | |
| util_isnormal(averageLufs) || |
We can only really deal with a normal value.
The alternative would be to check for HUGE_VAL instead.
There is by the way the issue that util_isnormal() shall return bool, not int.
There was a problem hiding this comment.
Thanks, I've prepended a commit to fix the return types in fpclassify and use util_isnormal() here
ronso0
force-pushed
the
libebur-debug
branch
from
6306f91 to
8481a3c
Compare
Hmm, the last (code) activity was 3 years ago. |
src/analyzer/analyzerebur128.cpp
Outdated
| if (averageLufs == -HUGE_VAL || | ||
| averageLufs == HUGE_VAL || | ||
| // This catches 0, inf and -inf (that may have slipped through in | ||
| // libebur for some reason. | ||
| // TODO Move this to a VERIFY_OR_DEBUG_ASSERT() once the root cause | ||
| // has been identified, and only do a zero check here. | ||
| util_isnormal(averageLufs)) { |
There was a problem hiding this comment.
Now the check for 0 is gone.
My idea was to just
| if (averageLufs == -HUGE_VAL || | |
| averageLufs == HUGE_VAL || | |
| // This catches 0, inf and -inf (that may have slipped through in | |
| // libebur for some reason. | |
| // TODO Move this to a VERIFY_OR_DEBUG_ASSERT() once the root cause | |
| // has been identified, and only do a zero check here. | |
| util_isnormal(averageLufs)) { | |
| if (!util_isnormal(averageLufs) || averageLufs == 0.0) { | |
| // This catches 0, inf (HUGE_VAL), -inf (-HUGE_VAL) and denormals (treated as zero), | |
| // that may have slipped through in libebur for some reason. |
Returning -HUGE_VAL is the documented invalid value. I am in doubt there will be a "fix" for HUGE_VAL and even then we deal with user data, where Bad things are not "impossible". So I would not add the TODO.
There was a problem hiding this comment.
Zero is covered by std::isnormal
https://en.cppreference.com/w/cpp/numeric/math/isnormal
I'll remove the TODO then.
ronso0
force-pushed
the
libebur-debug
branch
from
8481a3c to
19f699f
Compare
see #13676