Simpler power/envelope follower #384
Conversation
|
Ok I would like to suggest a bit more
|
src/sfizz/Voice.cpp
Outdated
| smoothedChannelEnvelopes[i] = | ||
| channelEnvelopeFilters[i].tickLowpass(std::abs(input[s])); | ||
| const float meanPower = sfz::meanSquared(input); | ||
| meanChannelPowers[i] = meanChannelPowers[i] * (1 - factor) + meanPower * factor; |
There was a problem hiding this comment.
The analysis signal is not windowed at the input.
In former experience, RMS value of raw input can be very imprecise.
There was a problem hiding this comment.
Windowed as in https://en.wikipedia.org/wiki/Hann_function ?
There was a problem hiding this comment.
We need some indication or the power btw, it does not need to be too precise.
There was a problem hiding this comment.
Yes, I would once find the measurement to be very noisy, (this was another project out of sfizz)
and the curve next to unusable.
Imo, at the first look, I'm not feeling convinced by this method.
It's tied to the samples per block. If the signal is too low frequency, and the block size is short, the method seems likely to fail.
There was a problem hiding this comment.
It's supposed to be somewhat independent on the samples per block, the factor changes depending on the expected block size and the actual one. I will double check since I did this at 2 in the morning.
I don't see how windowing would help though, it will bias the average? The noisiness can be smoothed away, the issue being that attacks and releases are not tracked quickly enough. Apparently people also take the max of 2 parallel filters for such cases, with different responses, to track attacks fast and releases less so.
There was a problem hiding this comment.
It's supposed to be somewhat independent on the samples per block, the factor changes depending on the expected block size and the actual one. I will double check since I did this at 2 in the morning.
Disregard, as it stands you're right it's not independent since I had a no-op multiplication in the normalizations
Good point.
Won't this be expensive if we do it for each voice? |
|
This implements the same thing, except I had a useless normalization by the maximum block size. I also put the combined filters for release and attacks. The test files are organ ranks (cuz I had them right here); "Plein jeux" is a relatively high-frequency mixture, "Bourdon 16" is a very low frequency quasi-pure sine wave, and "Doublette 2" is a very high frequency rank. It is not block-size independent indeed. I'll try to tweak it further, in the end I'd just like a minimum bias compared to sample-by-sample. |
…Track attack and release separately
|
I added a small correction to the coefficient calculation based on the assumption that:
The results are there. It does not meaningfully improve or change the behavior of the envelope follower. I also tried to go deeper in the Taylor expansions for the block version but also there it does not seem to impact the algorithm positively. I suspect most of the bias stems from assumption 1, which appears a bit unavoidable if we want simpler per-block version for the follower. I'm tempted to label this "good enough for the purpose", what do you think? I wonder also if #353 could just disable the envelope follower in the voices if the voice stealing algorithm does not use it? |
jpcima
force-pushed
the
simpler-envelope-follower
branch
from
73a9384 to
298befa
Compare
|
It's made independent of block size. It's replaced with a fixed block size which is set in config. |
|
The memory cost is a bit higher but I'm OK with this solution. Note that it will still be indirectly dependent on sample rate, but as I said, I don't think it's a big issue given the current use we make of this. Thanks for the improvements 🙂 |
This one is much cheaper and should do the job well enough for what we use it for.