sdroege / ebur128

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

Bugfix: True-peak broken when first samples are high-valued #41

Closed rawler closed 3 years ago

rawler commented 3 years ago

When the first samples are high-valued (a cosine-curve is a perfect example), the true-peak smoothing filter will incorporate some noise for the first samples. The solution is to not emit samples before the filter is properly saturated.

rawler commented 3 years ago

As a side-note, for some odd reason, this yielded a small speedup (3% for interleaved audio, 10% for planar) on my machine. It was not intended, and I would not count on it being a stable win, but at least performance did not regress.

sdroege commented 3 years ago

I assume this doesn't apply to the original C implementation because it actually resamples and then looks for the peak in the resampler output?

Thanks for finding this :)

rawler commented 3 years ago

I assume this doesn't apply to the original C implementation because it actually resamples and then looks for the peak in the resampler output?

Good question. Looking through the C-impl, I think it should suffer equally. The problem is (AFAIU, I'm no expert) called "transients", and in this case, it's only really a "problem" if the measured signal starts with a high sample-value (such as a cosine-curve). In that case, the interpolation will consider that a very "high energy movement" from 0 (the start-value of the filter buffer), and it is likely to become a peak higher than any other in the subsequent signal. In practice, I think is rarely a problem, since most "real" signals does not start with a max-sample.

The reason I discovered this, is since I'm working on enabling "chunked" analysis, where an input-stream can be divided in chunks and analyzed in parallel. I stumbled over the problem by accident on a synthetic signal, since every "chunk" is a new potential place where this can happen.

For the "chunking" to work however, I've also discovered I need to do similar priming on the loudness-filter. If we want to generally retain the behavior of the C-implementation, we could drop this PR (maybe merge the "owned" Samples-commit, since it seems to bring a small performance improvement), and simply add the required priming of this filter to the general "priming"-behavior? I'll work on such a PR today.

sdroege commented 3 years ago

One way around these discontinuities at the beginning would be to apply some windowing function at the beginning (and only there) to smoothen it. That of course also affects the loudness though.

I'm not sure what the right approach here is (maybe you can also check if windowing works for your use case?) but having the user worry about this seems a bit suboptimal.

If it also affects the C library, would you mind reporting it there too? At this point I don't think we should focus too much anymore on the behaviour between both being exactly the same as long as we know why they behave different and ideally also document that. That the initial port and the C library are equivalent was already proven, it wouldn't be useful to prevent future improvements over the C library just because of that.

sdroege commented 3 years ago

Also note https://github.com/sdroege/ebur128/pull/39 where I remove the C tests. I didn't get to write the corresponding tests for the Rust code yet though. I'd like to have tests for the individual components to ensure they continue behaving the same and to keep the same test coverage.

rawler commented 3 years ago

Have not had time to check the C-library if this is the case there too, sorry.

My case would most likely not be solved by a windowing-function, at least not completely. It would however be solved by #42, so I'm closing this now.

sdroege commented 3 years ago

ok :)