sdroege / ebur128

Implementation of the EBU R128 loudness standard
MIT License
93 stars 15 forks source link

Get rid of unsafe code in Histogram handling #28

Closed sdroege closed 4 years ago

sdroege commented 4 years ago

And replace it by using once_cell and passing around a static reference to the histogram metadata in various places.

While this decreases performance of the benchmark that tests addition of 1M values into the histogram by about 12% it has no measurable impact on any of the other benchmarks, and most importantly has no measurable impact when used as part of the whole EbuR128 benchmarks.

Once f64::powf() becomes a const function we can get rid of this entirely and have it all static.

jneem commented 4 years ago

Would you be opposed to just generating the arrays with some one-off script and checking in the generated rust code?

sdroege commented 4 years ago

That's the other option I'm currently looking at actually :) I'm a bit worried that the results might be slightly different depending on the architecture, but only one way to find out I guess.

Generally it makes me a bit unhappy that f64::powf() is not a const function, and these arrays should really be static, immutable arrays. So generating them via a script is probably the closest to getting to that, and it's not like they're ever going to change.

sdroege commented 4 years ago

Closing this here, replaced by https://github.com/sdroege/ebur128/pull/29 . It's less bad than expected.

The numbers here (together with the commit message of 40d23b84bcef0c7f9d59959765df08a0df25ec4f) can be useful for future reference though.