Problems with stereo playback at specific speeds

[h4yn0nnym0u5e]

There appear to be issues with playing back stereo files at specific rates - I've tested 2.5x and 2.25x, but suspect it can be generalised to rates of the form N+1/(2^m) (N and m are integers).

There may be more than one issue here. I'm highly suspicious of, for example, ResamplingReader.h:359
_interpolationPoints[channel][3].y = getSourceBufferValue(_bufferPosition1-(i*_numChannels)+1+channel);
which looks suspiciously as if it's going to read data from the wrong channel - I did a quick fix of
_interpolationPoints[channel][3].y = getSourceBufferValue(_bufferPosition1-(i*_numChannels)+_numChannels+channel);
and the output was better, but not entirely fixed.

I think the main issue could be due to differences shuffling the interpolation values around - I've added a few lines to fill in the x member with the buffer position used to generate the y value, and got some weird results where values looked to be older than expected (I'd expect, for a stereo file, to get x values of X, X-2, X-4 and X-6 - but I don't).

Having said all that, I don't fully understand the code and there's a lot of repeats of "shuffle up the interpolation points and refill" with subtle differences, so I'd rather you have a look and do the thing properly, if possible! If I do it, I'm liable to leave a few cases untouched, or even more broken than before...

[newdigate]

I'll have a quick look if I can find some time. yes sorry the interpolation code is horrific, I've been wanting to sort it out for a while, theres a few things wrong... the earlier versions of this library had a better approach for interpolation, there was an array containing the last 4 samples (per channel) which were sampled on a whole number...

[h4yn0nnym0u5e]

Hmmm ... I thought that was pretty much the current approach?! It just seems that the "last 4" are not managing to be the actual last 4, but have gaps in. Something to do with how you do the calculations with _remainder and _playbackRate, I think, and turn it into an array-filling loop.

On the value getting, I did a fix putting in a protected function

int16_t _getSourceBufferValue(int bufpos, int sampleOffset, int channel) { return getSourceBufferValue(bufpos + sampleOffset*_numChannels + channel); }

and then using that everywhere getSourceBufferValue() was previously used. Should be OK, I think C++ inlines it.

[h4yn0nnym0u5e]

By the way, thanks for (possibly!) taking time to have a look - much appreciated.

And I forgot to say, the overflow protection code in fastinterpolate() needs to be enabled - I got some overflows with a 100% amplitude 330Hz sine wave played back at ... 2.5x. That does seem to be a problem value!

[newdigate]

https://github.com/newdigate/teensy-variable-playback/tree/fix/rate_two_and_a_half

does that fix the issue?

Edit: diff is here

[h4yn0nnym0u5e]

Nope, don't think so. Caveat, I haven't checked out the repo, just made what I believe is the equivalent change by editing line 358 to if (false && !_useDualPlaybackHead) {

We now have some gaps in the samples, and some duplicated!

The negative x values are to show the associated y values are from result, not the indexed call to getSourceBufferValue() that should be disabled. Hence they're <minus><the sample number in the file>, left channel sample numbers being even and right channel being odd. I was getting things like -23, -25, -27, -33 before - it looked as if the for-loop is executing only once, and grabbing the last sample, rather than N times.

Also, I put numberOfSamplesToUpdate in as a class member so it would print, and it only ever comes out as 2; I would expect alternating 2 and 3, if the playback rate is 2.5x.

[h4yn0nnym0u5e]

I think I just spotted something ... every other time through, abs_remainder is zero, so only one sample is shuffled in to the interpolation filter. But at 2.5x we always need at least two new samples!

[newdigate]

Ah! yes its all coming back now... For interpolation, I used to store the X and Y values of the previous 4 samples, then I changed it to just the Y values... and the X values were hardcoded to 0,1,2,3. I think this issue was introduced then. Give me some time, I'll fix it.

Thank you very much for your help and time!

[h4yn0nnym0u5e]

No worries, always a pleasure to help make something good even better! And thank you for giving of your valuable bandwidth....

[newdigate]

@h4yn0nnym0u5e can you try this?

I think the bad logic was if remaider was 0 then it was assuming there was no wholenumber jump.

Sorry my brain is a bit broken. I'm hopeful this is an improvement....

Cheers.

[h4yn0nnym0u5e]

Better, in some small way, but still not right, I'm afraid.

Lines like _interpolationPoints[channel][3].y = getSourceBufferValue(_bufferPosition1+(i*_numChannels)-1+channel); are still in there, and wrong - the -1 switches to the other channel in a stereo file! Try something like
_interpolationPoints[channel][3].y = getSourceBufferValue(_bufferPosition1+((i-1)*_numChannels)+channel);

A good test case for this is a stereo file with radically different audio in the two channels - I crafted one with an up-sweep on one, and a reverse of that on the other.

The other issue is this:

for (int i=numberOfSamplesToUpdate; i > 0; i--) {
    _interpolationPoints[channel][0].y = _interpolationPoints[channel][1].y;
    _interpolationPoints[channel][1].y = _interpolationPoints[channel][2].y;
    _interpolationPoints[channel][2].y = _interpolationPoints[channel][3].y;
    _interpolationPoints[channel][3].y = result;
    if (_numInterpolationPoints < 4) _numInterpolationPoints++;
}

This puts in numberOfSamplesToUpdate copies of the same sample, which is clearly incorrect. It also seems to be slightly the wrong sample - I end up with samples N-4, N-3, N, N in the _interpolationPoints[] array when this code fires.

As an experiment I changed if (abs_remainder > 0.0) { to if (true || abs_remainder > 0.0) { and everything works much better!

[newdigate]

Hey @h4yn0nnym0u5e. Thanks for the testing/feedback/investigation.

I've had another attempt to improve the interpolation (there is still a tiny error of the first 4 samples will be zero), but I think it works slightly better, now on this branch - If it works for you then I will try to fix the first 4 samples....

[h4yn0nnym0u5e]

Not working for me, still :(

I've been having a go myself, as your changes were helping me understand what is intended (I think...) - you might want to have a crack at this branch.

I think I have interpolation working, and also repeat or pingpong looping with and without cross-fades. I've only tested on-screen using a single file (250ms stereo tone sweep, left rising and right falling), at ±2.5x. I'm hoping to try a wider range of settings tomorrow, using an oscilloscope.

Is it intentional that there seems to be no way to set linear interpolation, just none or quadratic?

[newdigate]

I was able to reproduce the issue before, but I'm no longer able to reproduce now, using the fix/interpolation2 branch. The issue with the 2.5 x rate (and 1.5x, etc....) was that when the remainder was 0.0 (i,e. every second sample...), it was assuming there was no jump over a whole number. This is fine when the rate is < 1.0, but when rate = 2.5, it was not detecting the jump and not updating the interpolation buffer. So a better mechanism fixed that issue, storing the last sample index at which the interpolation buffer was updated (per channel).

I tried processing a stereo sweep @ +2.5x with the left channel reversed.... Inspecting the output in audacity looks perfect.

[newdigate]

Is it intentional that there seems to be no way to set linear interpolation, just none or quadratic?

I think linear interpolation should be working, I just haven't enabled or tested it for years. It is there for reference and if somebody has a need to reduce the potentially heavy/broken quad interpolation

[newdigate]

looking at it..... linear interpolation has the same bug!!! doh! I don't think it'll work well with 1.5, 2.5, 1.25, very well...

[newdigate]

ops, sorry correction on line 504 ResamplingReader, should be:

        for(int i=0;i<8;i++) {
            _numInterpolationPoints[i] = 0;
            _lastInterpolationPosition[i] = _header_offset/_numChannels;
        }

it was...

        for(int i=0;i++;i<8)
            _numInterpolationPoints[i] = 0;

....which makes no sense

[newdigate]

I have reverted all the changes locally, so I just have fix/interpolation2 branch, re-ran my 2.5 x tests and all seems fine to me... Unfortunately I'm running these tests on x86 cpu, not on a teensy - so the only thing I can say is that the algorithm now works on x86 for 2.5 x playback rate. I thinks its highly unlikely, but it could be a processor architecture issue.

I might move the dual playback head to a different branch because It makes the interpolation code pretty messy...

[h4yn0nnym0u5e]

I've been having another play, and I think my confusion arises from leaving useDualPlaybackHead enabled - this breaks your branch pretty severely, though I know it wasn't on your radar right now.

Observations:

• getSourceBufferValue(_bufferPosition1-(i*_numChannels)+1+channel) causes the channels to be swapped - you don't need the +1 as far as I can tell, seems to be in pretty much all the lines where the interpolation buffer is being filled
• pingpong loops aren't working
• as noted, useDualPlaybackHead isn't working

On my branch I think I got all loop and dual playback working, though I completely re-wrote your crossfade start / stop logic because I simply couldn't fathom it, and it gets really weird if you were doing a crossfade on a pingpong loop, as the bufferPosition indexes are going in the opposite direction during the crossfade. I do need to do more testing, but would welcome any comments you might have.

[newdigate]

useDualPlaybackHead

Its a bit of an experimental feature. The concept is easy, but it starts getting a bit tricky when doing the interpolation. Maybe one-day when Im inspired I will reimplement it. For the meantime, its going on a experimental branch.... and I'll remove it from the master.... It should be compatible with all features...

getSourceBufferValue(_bufferPosition1-(i*_numChannels)+1+channel) causes the channels to be swapped - you don't need the +1 as far as I can tell, seems to be in pretty much all the lines where the interpolation buffer is being filled

goitcha. I had noticed the left and right channels were swapped but didnt know why. Thanks.

• pingpong loops aren't working

dual playback heads dont need to switch when doing a ping pong, no fade is required when the loop end is hit or the begging is hit, so maybe disable dualplayback head when ping ponging....

[h4yn0nnym0u5e]

I'm finding pingpong not working at all, dual heads or not.

I'm not 100% sure if cross-fading is unnecessary for pingpong: obviously the sample level is the same but the rate of change could reverse rapidly, which may give an audible click similar to a level change. For people tuning their loop points via a front panel control it might be worth leaving in / implementing.

Very weird, I've just noticed my code puts in a glitch, in the left channel only, just after a pingpong reverse! (Without crossfade; with, it either doesn't happen or the crossfade has masked it). Most odd, have to fix that.

[h4yn0nnym0u5e]

Found the glitch - you've got it too... In ResamplingReader.h, line 173, we have
if (readNextValue(index[channel], channel)) {

If the read fails (because the loop is ended), then the else clause is actioned to sort out the buffer position, but the readNextValue() isn't re-tried, and execution proceeds with the other channel (which succeeds because the buffer position is now OK). This means a stale sample is left at the end of the Channel 0 values for this update(), downstream of where the issue really occurred.

[newdigate]

ah! makes sense now. well spotted. thank you Johnathan!