Bug in generic interpolators?

I’ve been doing some sub sampling with Lagrange and Linear interpolators, and everything works great, EXCEPT when I go from a speed >1.0 to a speed < 1.0 - just to clarify, I mean starting at say 1.0001 for 512 samples, and then moving to 0.9999 for another 512 samples, for example. There’s an audible glitch and I believe it’s to do with the subsample position or something. Moving upwards/downwards while above or below 1.0 has no issues, so it’s definitely just an issue swapping over.

Looking in to it, it appears to be related to this section of code.

Is there a workaround anyone knows of for doing this, or is there a potential fix?

1 Like

There are some very recent updates to that class, so if you’re not using the very latest develop branch, please update and check that the problem is still present.

I was on JUCE 6, but I’ve tested tested with the latest develop of JUCE 7 and am still having the same issue.

Doing a quick skim over the changes and the original, it looks like when swapping between > 1.0 and < 1.0, you may need to swap pos to the inverse alpha or something

Here’s a test example which demonstrates the issue:
NewProject.zip (7.4 KB)

1 Like

In the example above, just swap out the interp tests in the process block, randomPositiveFloat(), randomNegativeFloat() and 1.0 all work without issues.

Swapping to randomFloat() which generates a random float between 1.0001 - 0.9998 causes audio glitches consistently.

1 Like

Hello! The interpolation code is not mine, but indeed I can see the problem there with the behaviour of pos being different whether ratio > 1 or not, with discontinuity at the transition.

Could you tell me if replacing:

if (speedRatio < 1.0)
{
    for (int i = numOutputSamplesToProduce; --i >= 0;)
    {
        if (pos >= 1.0)
        {
            pushSample();
            pos -= 1.0;
        }

        *output = process (*output, InterpolatorTraits::valueAtOffset (lastInputSamples, (float) pos, indexBuffer));
        ++output;

        pos += speedRatio;
    }
}
else
{
    for (int i = numOutputSamplesToProduce; --i >= 0;)
    {
        while (pos < speedRatio)
        {
            pushSample();
            pos += 1.0;
        }

        pos -= speedRatio;

        *output = process (*output, InterpolatorTraits::valueAtOffset (lastInputSamples, jmax (0.0f, 1.0f - (float) pos), indexBuffer));
        ++output;
    }
}

with:

for (int i = numOutputSamplesToProduce; --i >= 0;)
{
    pos += speedRatio;
    while (pos >= 1.0)
    {
        pushSample();
        pos -= 1.0;
    }

    *output = process(*output, InterpolatorTraits::valueAtOffset(lastInputSamples, (float)pos, indexBuffer));
    ++output;
}

solves the artefacts issues and doesn’t bring any additional problem?

I looked at something similar, and had a solution almost identical, and while it does resolve the base issue, I believe it means the interpolation is no longer working properly. I only briefly looked over it, but the second value of InterpolatorTraits::valueAtOffset is the one that seems to be the important one for the different interpolations, and for whatever reason it’s using the inv alpha of pos in some cases, with the normal in others.

Before posting my message, I implemented my fix and tested it with your example project, to ensure that the artefacts are no longer there, and with various projects of mine and specifically one allowing to read an audio file at various speeds with various interpolation algorithms. My fix didn’t break the interpolation algorithm, since the speed up/down was still working properly and sounding the same than with the previous version of the code. So I would say my fix is right and works, unless you prove me wrong with some additional testing :wink:

I just created a second method which uses your version and duplicated the sine wave buffer and inversed the phase of it.

Processing both, synchronously (with different interpolator objects) and the same values, they should phase each other out.

For values > 1.0 they phase out, as expected since it’s the same code. For < 1.0 they do not phase using negative numbers (this is even with a constant value, not a changing value), so I would say this is not resolved with that code, especially since for slower speed ratios, this is where the interpolation really kicks in and the quality of the algorithm plays part.

I can provide the code if you wish to troubleshoot it further if you wish though.

Edit: I also went a bit further in and checked, the valueAtOffset (pos) section is used in the different interpolators in a specific way, for example here, so having the correct inverse etc appears to be important

Why?

Especially now that we know that the original algorithm had at least one issue? And I saw a very ugly juce::jmax(0, 1-pos) there that was quite questionnable honestly…

On review, you’re right - my apologies. I was misinterpretting the pos section for the one in JUCE 6 which uses the indexBuffer of the interpolation sample!

Unless someone else can find a bug or confirm it’s not working as intended, this looks like a fix then :slight_smile:

1 Like

Great to hear! Going to test this more extensively asap (this is very timely, I’m currently working on a BBD model which uses extensively live resampling), and then I’ll submit the fix to the JUCE team :wink:

@reuk @t0m

So replacing this:

if (speedRatio < 1.0)
{
    for (int i = numOutputSamplesToProduce; --i >= 0;)
    {
        if (pos >= 1.0)
        {
            pushSample();
            pos -= 1.0;
        }

        *output = process (*output, InterpolatorTraits::valueAtOffset (lastInputSamples, (float) pos, indexBuffer));
        ++output;

        pos += speedRatio;
    }
}
else
{
    for (int i = numOutputSamplesToProduce; --i >= 0;)
    {
        while (pos < speedRatio)
        {
            pushSample();
            pos += 1.0;
        }

        pos -= speedRatio;

        *output = process (*output, InterpolatorTraits::valueAtOffset (lastInputSamples, jmax (0.0f, 1.0f - (float) pos), indexBuffer));
        ++output;
    }
}

with this:

for (int i = numOutputSamplesToProduce; --i >= 0;)
{
    while (pos >= 1.0)
    {
        pushSample();
        pos -= 1.0;
    }

    *output = process (*output, InterpolatorTraits::valueAtOffset(lastInputSamples, (float)pos, indexBuffer));
    ++output;

    pos += speedRatio;
}

seems to do the job for me. Does one know why there is such conditional processing originally?

1 Like

Thank you for reporting the issue and for @IvanC for suggesting basically the right solution. A fix is now out on develop.

2 Likes