On Wed, 9 Feb 2022 01:34:19 +0100 Ichthyostega <p...@ichthyostega.de> wrote:
PS: last weekend I was struggling to chase down some problem within the Reverb effect, which seems to produce spurious differences in the test suite.

Am 09.02.22 um 12:22 schrieb Will Godfrey:
I would think that is actually the most likely place for errors to accumulate. It massively loops, as well as calling our old friend analogfilter :(


Hi Yoshimi-devs,

while investigating (and in the end solving) the "1st note problem",
I've noticed some remaining differences on the test case I've distilled.

As you might recall, Will hinted this problem shows up when loading "Ooo"
after "Coir Stabs". After patching in my "numerics dump changeset", it quickly
became clear that these intermittent differences emanate from the Reverb effect.


Now my first thought was to put that off as "rounding" problems.
However, what irritated me, was the fact that, while the absolute differences
initially are quite small, the relative differences were well above factor 100
too large to be explained by floating point rounding errors.

For example:  2.83512e-22  vs  2.83606e-22

As we all know: floating point numbers aren't just some mysterious wonder box
to produce "random" differences. Rather, they work by well defined rules, and
within certain limits, results can be expected to be totally reproducible
(at least on the same compiler / machine).

Thus, even while this is a digression of kinds, I decided to have a closer
look, hoping I might find a reason for those random fluctuations. Well --
in the end I didn't find "one" reason, rather several inconsistencies,
mostly revolving around the implementation of Reverb::cleanup().

And after addressing those problems, now Reverb calculations are reproducible!



While not serious in any sense, these hickups are interesting non the less...


(1)
First thing I noticed were re-calculations of AnalogFilter coefficients.
These happened with a random irregular pattern, but say each 3rd compute(buffer)
cycle. The passed-in frequency fluctuated at the last place, and thus the
slightly changed filter coefficients amplified those changes. As it turned out,
all these recalculations were completely spurious. They were triggered here....

if (hpf)
{
    float fr = hpffr.getValue();
    hpffr.advanceValue(synth->sent_buffersize);
    if (fr != hpffr.getValue())
    {
        hpf->interpolatenextbuffer();
        hpf->setfreq(hpffr.getValue());
    }
    hpf->filterout(inputbuf.get());
}

Can you spot the problem? :-D


In fact we were constantly interpolating between the *same* values, but

getOldValue() * (1.0f - factor()) + getNewValue() * factor();

...just happens to move around by some ULP up and down during the interpolation
cycle. Thus one problem was that the existing code for the interpolation just
did not stop, rather repeatedly computed

interpolationPos += samples - interpolationLength;

...and thus never reached the end condition. And the other problem is the
naive equality comparison of floating point numbers. In this case, this is
easy to fix by introducing a static "epsilon" (in the general case such an
epsilon should be relative, and doing that right can be complicated. But here
we just know that the compared values are frequencies, and we certainly do not
care for frequency differences below 0.001 Hz as far as interpolation is 
concerned.


(2)
Then there were several further problems related to resetting the state of the
Effect. As you know, the Effect::cleanup() is typically called when changing
effect parameters. But it is also invoked from SynthEngine::shutup() -- and the
TestInvoker calls that prior to each test note.

One interesting problem was that the Reverb::cleanup() did not clean the
output buffer, while the reverb calculation in fact draws its feedback from
that very buffer, thus leaking reverberant sound from before the cleanup().

Moreover, the current read positions in the 16 + 8 feedback lines within Reverb
weren't reset either, and so the generated sound depends on state from before
launching the test. All of this is trivial to fix of course.


(3)
But then there was yet another problem very well hidden, namely within the
InterpolatedValue for the Pan-Gain. It wasn't even related to Reverb at all.
The Effect() ctor sets the initial value for pangain to 0.5, while it should
be cos(45°), since all effects use (and always used) the "normal" panning law.
And moreover, the interpolation position within InterpolatedValue is likewise
not affected by Effect::cleanup().

Interestingly enough, this problem did rarely show up, if at all, since the
typical setup scripts for test cases finish within less than one compute cycle.
However, now we've introduced background building of PADsynth wavetables, and we
call "set Apply Force" (with a blocking wait). So this introduces some +-1 buff
cycle fluctuation on average, and thus now we saw 2 or 3 different output values
repeatedly, depending on how much buffer cycles the script execution takes.

As a fix I decided to add a function InterpolatedValue::pushToTarget()
which has the effect to fast-forward to the next target value and thus disable
any ongoing interpolation. I arranged to call that from all Effect::cleanup()
derivatives. And I also fixed the init value of the pangain....

Cheers,
Hermann



_______________________________________________
Yoshimi-devel mailing list
Yoshimi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/yoshimi-devel

Reply via email to