Portamento ST mode does not go all the way to zero #7224

Sin-tel · 2023-09-27T19:15:57Z

Bug Description:
When setting portamento time to 0ms in ST mode, there is still noticable portamento on some oscillator and filter types.
For self-oscillating filters it is much longer than 32 samples so I don't think it is related to internal block size.
This also happens on 'modern' oscillator type, but not on wavetable or classic.
I have attached two patches: self osc.fxp and modern.fxp that demonstrate the issue, as well as exported samples and screenshots highlighting the issue in Audacity.
report.zip

Surge XT Version
Version: Surge XT 1.3.nightly.2405dfd
Build: 2023-09-27 @ 15:01:20 on 'fv-az407-912/pipeline' with 'MSVC-19.35.32217.1' using JUCE 7.0.5
System: Windows 64-bit VST3 on AMD Ryzen 5 3550H with Radeon Vega Mobile Gfx
Host: Ableton Live 11 @ 44.1 kHz / Process block size: 32 samples

Reproduction Steps:
Steps to reproduce the behavior:

Init saw patch
Mute osc 1
Unmute noise and add a tiny bit
Set filter 1 to LP12dB and set resonance to 100
Set F1 keytrack to maximum
Set play mode to MONO ST or MONO ST+FP
(see patch in attach)

Expected Behavior:
Instantaneous change - or at least on the order of 32 samples.

Screenshots:

Computer Information:

OS: Windows 10
Host: Ableton Live
Version: Tried both 1.2.3 and latest nightly [NIGHTLY-2023-09-27-2405dfd]

Additional Information:
Related:
#6984
#6986

Andreya-Autumn · 2023-09-27T19:27:22Z

Thanks for reporting! This is suspiciously similar to 6984, I wonder if something has regressed that. Investigation needed.

baconpaul · 2023-09-27T23:19:58Z

Yeah thanks for the excellent report

I wonder where the lag is coming from. Something - at least in modern - must be lagged too aggressively. With this report it should be straight forward to at least diagnose it.

baconpaul · 2023-09-30T20:16:49Z

Ahh I see exactly why this happens with modern. It is a remnant of the original version of the algorithm which is no longer needed. Let me see if I can at least clean up that branch.

baconpaul · 2023-09-30T20:22:12Z

Yeah OK here's the fix for modern.

Need to look at the filters separately. That may be smoothing of the key track.

addresses surge-synthesizer#7224 The original version of Modern used the trailing three points for the derivative, but didnt' recaclulate all three at once. This meant it was a bit unstable under extreme pitch changes so we used a lag/lpf on pitch and dphase. We subsequently changed the algorithm to always calculate the trailing three points but never undid that lag, leading to low porta high shift having an audible sweep in modern, as reported in surge-synthesizer#7224. This change fixes that and simply block smooths pitch shifts.

baconpaul · 2023-09-30T20:42:30Z

Initial investigation on the filter is: it is not the key track being smoothed. the filter gets the cutoff reset immediately.

The filter does have history (it is a filter after all) and I wonder if that's what's actually smoothing the self oscillation.

baconpaul · 2023-09-30T20:48:59Z

OK the filter is getting reset immediately but the algorithm has enough history that there's a small lag. the coefficients reset instantly though.

So to understand the filter behavior gotta wade through this

{
    f->C[0] = _mm_add_ps(f->C[0], f->dC[0]); // F1
    f->C[1] = _mm_add_ps(f->C[1], f->dC[1]); // Q1

    __m128 L = _mm_add_ps(f->R[1], _mm_mul_ps(f->C[0], f->R[0]));
    __m128 H = _mm_sub_ps(_mm_sub_ps(in, L), _mm_mul_ps(f->C[1], f->R[0]));
    __m128 B = _mm_add_ps(f->R[0], _mm_mul_ps(f->C[0], H));

    __m128 L2 = _mm_add_ps(L, _mm_mul_ps(f->C[0], B));
    __m128 H2 = _mm_sub_ps(_mm_sub_ps(in, L2), _mm_mul_ps(f->C[1], B));
    __m128 B2 = _mm_add_ps(B, _mm_mul_ps(f->C[0], H2));

    f->R[0] = _mm_mul_ps(B2, f->R[2]);
    f->R[1] = _mm_mul_ps(L2, f->R[2]);

    f->C[2] = _mm_add_ps(f->C[2], f->dC[2]);
    const __m128 m01 = _mm_set1_ps(0.1f);
    const __m128 m1 = _mm_set1_ps(1.0f);
    f->R[2] = _mm_max_ps(m01, _mm_sub_ps(m1, _mm_mul_ps(f->C[2], _mm_mul_ps(B, B))));

    f->C[3] = _mm_add_ps(f->C[3], f->dC[3]); // Gain
    return _mm_mul_ps(L2, f->C[3]);
}

and turn it into math and understand how to make it have less hysteresis when self oscillating. It seems.

So either i'm wrong about that and there's a lag elsewhere (but I did a print-when-coefs chang and they are instant) or I'm right about that. In either case: not today!

Andreya-Autumn · 2023-09-30T21:24:21Z

That's the parallel intrinsics stuff right? Looks intimidating... Anyway, nice about the modern fix!

addresses #7224 The original version of Modern used the trailing three points for the derivative, but didnt' recaclulate all three at once. This meant it was a bit unstable under extreme pitch changes so we used a lag/lpf on pitch and dphase. We subsequently changed the algorithm to always calculate the trailing three points but never undid that lag, leading to low porta high shift having an audible sweep in modern, as reported in #7224. This change fixes that and simply block smooths pitch shifts.

baconpaul · 2023-09-30T21:54:59Z

yeah its not that hard to read just you need to do it. then figure out the math. putting it here for when i stumble on it next.

My guess is the filter has enough history that it takes its time adjusting state. It is a low pass filter after all. But I can't demonstrate that. What I was able to show is that the inputs to configure the filter snap immediately, and there's no other code lagging in the way. So I think the lag is in the math on that.

baconpaul · 2023-09-30T21:59:06Z

{
    //f->C[0] = _mm_add_ps(f->C[0], f->dC[0]); // F1
    //f->C[1] = _mm_add_ps(f->C[1], f->dC[1]); // Q1
   F1 = C[0]; Q1 = C[1];

    //__m128 L = _mm_add_ps(f->R[1], _mm_mul_ps(f->C[0], f->R[0]));
    L = R[1] + F1 * R[0];

    //__m128 H = _mm_sub_ps(_mm_sub_ps(in, L), _mm_mul_ps(f->C[1], f->R[0]));
    H = in - L - Q1 * R[0];

   // __m128 B = _mm_add_ps(f->R[0], _mm_mul_ps(f->C[0], H));
    B = F[0]+ F1 * H;

    //__m128 L2 = _mm_add_ps(L, _mm_mul_ps(f->C[0], B));
    L2 = L + F1 * B;

    // __m128 H2 = _mm_sub_ps(_mm_sub_ps(in, L2), _mm_mul_ps(f->C[1], B));
    H2 = in - L2 - Q1 * B;

    //__m128 B2 = _mm_add_ps(B, _mm_mul_ps(f->C[0], H2));
    B2 = B + F1 * H2;

    //f->R[0] = _mm_mul_ps(B2, f->R[2]);
    R[0] = B2 * R[2];
    //f->R[1] = _mm_mul_ps(L2, f->R[2]);
    R[1] = L2 * R[2];

    // f->C[2] = _mm_add_ps(f->C[2], f->dC[2]); // C2 exists
   // const __m128 m01 = _mm_set1_ps(0.1f);
   // const __m128 m1 = _mm_set1_ps(1.0f);
   // f->R[2] = _mm_max_ps(m01, _mm_sub_ps(m1, _mm_mul_ps(f->C[2], _mm_mul_ps(B, B))));
   R2 = max(0.1, 1 - C[2] * B * B);

    //f->C[3] = _mm_add_ps(f->C[3], f->dC[3]); // Gain
    //return _mm_mul_ps(L2, f->C[3]);

    return L2 * C[3]
}

baconpaul · 2023-09-30T22:00:18Z

{
    F1 = C[0]; Q1 = C[1];
    L = R[1] + F1 * R[0];
    H = in - L - Q1 * R[0];
    B = F[0]+ F1 * H;
    L2 = L + F1 * B;
    H2 = in - L2 - Q1 * B;
    B2 = B + F1 * H2;
    R[0] = B2 * R[2];
    R[1] = L2 * R[2];
    R2 = max(0.1, 1 - C[2] * B * B);
    return L2 * C[3]; // C[3] is 'gain' in the comments
}

so when that is self oscillating what's the lag under coefficient change? The C[0] etc.. are all set in the coefficient maker.

Not obvious.

Andreya-Autumn · 2023-09-30T22:07:48Z

well ≈512 samples, I guess judging by Sintels report. But indeed, it's not obvious why that should be so. Lemme try if it's different at blocksize 8 just for good measure.

Andreya-Autumn · 2023-09-30T22:17:44Z

Hmmm...

That's with the patch from Sintels report. That looks and sounds pretty near instant to me. Doubt this has to do with block size. Wonder if it's a platform thing. Different platforms interpret those parallel instructions differently right? Would that matter?

baconpaul · 2023-09-30T22:39:37Z

no. there's a slightly different coefficient restatement speed but that's block size nothing else. and the statements are just math so they work same on intel and arm.

that's with the filter patch?

the modern problem was a legit one though and is now fixed in nightly.

Andreya-Autumn · 2023-09-30T22:43:14Z

Ok, that wouldn't be it then.

Yeah that's the filter patch. I tried with a bandpass too and it was also quick. It's late here and I'm not gonna compile a block size 32 Surge to test with, but other than that and windows IDK what would be different between Sintels setup and mine. Weird.

baconpaul · 2023-09-30T22:46:02Z

I used print statements didn’t even scope it

I have an 8 16 and 32 build all set up in my ide so can peek tomorrow when I have my laptop open next

thanks so much for looking. I can’t believe I didn’t!

Andreya-Autumn · 2023-09-30T22:47:20Z

No worries. Catch ya tomorrow.

Sin-tel · 2023-10-01T13:35:01Z

Can confirm on latest nightly modern osc is fixed!
The filter thing seems unlikely to be related to any specific DSP algorithm, since it happens on any filter I tried (at least ones that can self-oscillate, which is most of them, but not notch and allpass).
Weirdly, for the two comb filters, slewing seems to happen when ramping up but not down.

baconpaul · 2023-10-01T13:45:04Z

OK I'll try and reproduce this week. That's useful information. It means something is slewing your key track. But I don't see in the code what that could be and it didn't happen when I tried it.

Glad Modern is fixed! Great find on that. Thanks.

baconpaul · 2023-10-05T11:57:43Z

I have had to revert the modern change since it causes pops under extreme modulation, which it shouldn't do but did.

patches.zip

Demonstrating patches there for further investigation.

Sin-tel added the Bug Report Item submitted using the Bug Report template label Sep 27, 2023

Andreya-Autumn added this to the Surge XT 1.3 milestone Sep 27, 2023

mkruselj added the DSP Issues and feature requests related to sound generation in the synth label Sep 28, 2023

baconpaul mentioned this issue Sep 30, 2023

Modern Oscillator Pitch: Lag -> Lipol #7231

Merged

baconpaul mentioned this issue Oct 5, 2023

Revert "Modern Oscillator Pitch: Lag -> Lipol (#7231)" #7242

Merged

baconpaul modified the milestones: Surge XT 1.3, Surge XT 1.x Nov 4, 2023

This was referenced Mar 5, 2024

Monthly issue metrics report bobobo199733/issues-metrics-tool-repo#5

Closed

Monthly issue metrics report bobobo199733/issues-metrics-tool-repo#6

Closed

This was referenced Mar 5, 2024

Monthly issue metrics report bobobo199733/issues-metrics-tool-repo#7

Closed

Issue metrics report bobobo199733/issues-metrics-tool-repo#8

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Portamento ST mode does not go all the way to zero #7224

Portamento ST mode does not go all the way to zero #7224

Sin-tel commented Sep 27, 2023 •

edited

Loading

Andreya-Autumn commented Sep 27, 2023

baconpaul commented Sep 27, 2023

baconpaul commented Sep 30, 2023

baconpaul commented Sep 30, 2023

baconpaul commented Sep 30, 2023

baconpaul commented Sep 30, 2023 •

edited by mkruselj

Loading

Andreya-Autumn commented Sep 30, 2023

baconpaul commented Sep 30, 2023

baconpaul commented Sep 30, 2023

baconpaul commented Sep 30, 2023 •

edited

Loading

Andreya-Autumn commented Sep 30, 2023

Andreya-Autumn commented Sep 30, 2023

baconpaul commented Sep 30, 2023

Andreya-Autumn commented Sep 30, 2023

baconpaul commented Sep 30, 2023

Andreya-Autumn commented Sep 30, 2023

Sin-tel commented Oct 1, 2023

baconpaul commented Oct 1, 2023

baconpaul commented Oct 5, 2023

Portamento ST mode does not go all the way to zero #7224

Portamento ST mode does not go all the way to zero #7224

Comments

Sin-tel commented Sep 27, 2023 • edited Loading

Andreya-Autumn commented Sep 27, 2023

baconpaul commented Sep 27, 2023

baconpaul commented Sep 30, 2023

baconpaul commented Sep 30, 2023

baconpaul commented Sep 30, 2023

baconpaul commented Sep 30, 2023 • edited by mkruselj Loading

Andreya-Autumn commented Sep 30, 2023

baconpaul commented Sep 30, 2023

baconpaul commented Sep 30, 2023

baconpaul commented Sep 30, 2023 • edited Loading

Andreya-Autumn commented Sep 30, 2023

Andreya-Autumn commented Sep 30, 2023

baconpaul commented Sep 30, 2023

Andreya-Autumn commented Sep 30, 2023

baconpaul commented Sep 30, 2023

Andreya-Autumn commented Sep 30, 2023

Sin-tel commented Oct 1, 2023

baconpaul commented Oct 1, 2023

baconpaul commented Oct 5, 2023

Sin-tel commented Sep 27, 2023 •

edited

Loading

baconpaul commented Sep 30, 2023 •

edited by mkruselj

Loading

baconpaul commented Sep 30, 2023 •

edited

Loading