RustAudio / dasp

The fundamentals for Digital Audio Signal Processing. Formerly `sample`.
Other
883 stars 64 forks source link

Overflow when adding two unsigned integers #167

Open PanieriLorenzo opened 2 years ago

PanieriLorenzo commented 2 years ago

This issue is related to #73 but occurs when using add_amp on unsigned integers, for which the result would be the maximum value (or minimum value) that unsigned integer can store, i.e. 1.0 when converted to float representation. In these cases the function will overflow internally, as the unsigned integers are converted to signed integers of the same size during the operation.

Despite the fact that the discussion in #73 arrived at the conclusion that it's intended for some types to not be able to store 1.0, in this case I'd argue it is still a problem, because in theory unsigned types should be able to store 1.0, but in certain cases they can't (unexpectedly). Moreover, a possible solution in this case seems quite simple to implement and not particularly controversial, simply use a larger signed integer conversion internally when dealing with unsigned integer addition, e.g. for u8, use i16 internally.

PanieriLorenzo commented 2 years ago

I'd like to add that maybe instead of changing how numbers are represented, a more general solution that would help alleviate #73 as well, would be to saturate signals rather than overflowing. This is more consistent to how most dsp things handle overflows. In other words, hard-clip rather than panic.