sdroege / ebur128

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

Bufferless True Peak-analysis #36

Closed rawler closed 3 years ago

rawler commented 3 years ago

This is the second of the two TruePeak analysis optimizations. The key optimization here, is avoiding extra memory-copying by not keeping input and output from the upsamling. Every new input-frame is fed immediately to the interpolator, generating 2 or 4 new frames which are immediately checked for new max before being discarded.

The net gain according to my benchmark:

true_peak: 48kHz 2ch f32/Rust /Interleaved: 545.50 -> 395.99 (-27.4%) /Planar: 556.07 -> 424.97 (-23.6%)

true_peak: 48kHz 2ch i16/Rust /Interleaved: 550.95 -> 436.63 (-20.7%) /Planar: 579.58 -> 476.53 (-17.8%)

As a nice bonus, it also cleans up a lot of code from the previous step of optimization, causing a significant net reduction of code.

8 files changed, 462 insertions(+), 363 deletions(-)

sdroege commented 3 years ago

Thanks, this seems great. I'll take a proper look this weekend :)

sdroege commented 3 years ago

Is it ready for review now? I saw you fixed up/improved various things in the meantime :)

rawler commented 3 years ago

Fair point. :) I'll look into it.

Den lör 7 nov. 2020 kl 15:56 skrev Sebastian Dröge <notifications@github.com

:

@sdroege commented on this pull request.

In src/interp.rs https://github.com/sdroege/ebur128/pull/36#discussion_r519185506:

  • let imp: Box = match (taps, factor, channels) {
  • (49, 2, 1) => Box::new(specialized::Interp2F::<[f32; 1]>::new()),
  • (49, 2, 2) => Box::new(specialized::Interp2F::<[f32; 2]>::new()),
  • (49, 2, 4) => Box::new(specialized::Interp2F::<[f32; 4]>::new()),
  • (49, 2, 6) => Box::new(specialized::Interp2F::<[f32; 6]>::new()),
  • (49, 2, 8) => Box::new(specialized::Interp2F::<[f32; 8]>::new()),
  • (49, 4, 1) => Box::new(specialized::Interp4F::<[f32; 1]>::new()),
  • (49, 4, 2) => Box::new(specialized::Interp4F::<[f32; 2]>::new()),
  • (49, 4, 4) => Box::new(specialized::Interp4F::<[f32; 4]>::new()),
  • (49, 4, 6) => Box::new(specialized::Interp4F::<[f32; 6]>::new()),
  • (49, 4, 8) => Box::new(specialized::Interp4F::<[f32; 8]>::new()),
  • (taps, factor, channels) => Box::new(generic::Interp::new(taps, factor, channels)),
  • };
  • Self(imp)
  • pub fn new(_taps: usize, _factor: usize, _channels: u32) -> Self {
  • unimplemented!()

This suggests that this commit should be squashed with another one :) This alone doesn't seem runnable as-is.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/sdroege/ebur128/pull/36#pullrequestreview-525658383, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAADLXCEWE7724P6UJQBMDTSOVNZBANCNFSM4TKQV5ZQ .

rawler commented 3 years ago

I'd say it's ready for review. I'm still looking for ways to improve performance further, but this is good to merge as-is (I'll look into the squashing-topic though)

rawler commented 3 years ago

You're welcome. Thanks for all the other code I did not have to write :) I think all the feedback is addressed now. Please have a look again.

sdroege commented 3 years ago

You forgot to update benches/interp.rs for push() ->interpolate(). I've updated that now, seems all good to me otherwise and I'll merge once the CI is also happy :)