I’ve noticed recently that I’m getting clicks and pops when modulating the pulse width of the VCO from the VCV Fundamental collection with a sine wave. The more I increase the speed of the sine wave modulating the pulse width, the more pops and clicks I get. I tried recreating this issue with other oscillators and have encountered the issue on oscillators from Bog Audio, Slime Child, troika from Instruo and EvenVCO from Befaco. This issue does not seem to be present in the following oscillators: ONA from NANO, Fundamental & Basic VCO from Squinky Labs, SurgeOSC from Surge for Rack and Bleak from Vult.
I tried testing the issue in Rack 1.1.6 and it seems that it was present in that version, but I just didn’t notice it, shockingly.
I believe I stumbled upon a thread that brought this issue up about a year ago, but I’m unable to find it now.
Am I the only one running in to this issue? Does anyone know the cause of the pops and clicks?
That’s it!! I’m not super familiar with Github, but it looks like Andrew closed the issued? Doesn’t seem to be fixed though… Any idea why your oscillator doesn’t produce the clicks when modulating the pulse width but many others do?
The Github ticket correctly identifies the cause: VCV VCO’s MinBLEP implementation will sometimes insert a second discontinuity immediately after the first, which results in a pop. Many developers–myself included–have used that implementation as reference, and have inadvertently duplicated the bug, which is why you’re seeing it in other modules too.
The fix is pretty simple: only insert discontinuities for the single sample where the pre-antialiased value has changed (e.g. the exact sample where the value jumps from -1 → +1 or +1 → -1).
Sample SIMD implementation:
// ...
// Calculate waveform
value = rack::simd::ifelse(_phase < _width, 1.0f, -1.0f);
// Check for discontinuity
int change_mask = rack::simd::movemask(value != _last);
_last = value;
if (change_mask != 0) {
// Insert discontinuity where phase crosses 0
rack::simd::float_4 zero_cross = (delta_phase - _phase) / delta_phase;
int zero_mask = change_mask & rack::simd::movemask((0.0f < zero_cross) & (zero_cross <= 1.0f));
if (zero_mask) {
for (int i = 0; i < channels; i++) {
if (zero_mask & (1 << i)) {
rack::simd::float_4 mask = rack::simd::movemaskInverse<rack::simd::float_4>(1 << i);
float p = zero_cross[i] - 1.0f;
rack::simd::float_4 x = mask & static_cast<rack::simd::float_4>(2.0f);
_minblep_generator.insertDiscontinuity(p, x);
}
}
}
// Insert discontinuity where phase crosses pulse width
rack::simd::float_4 pulse_cross = (_width - (_phase - delta_phase)) / delta_phase;
int pulse_mask = change_mask & rack::simd::movemask((0.0f < pulse_cross) & (pulse_cross <= 1.0f));
pulse_mask = pulse_mask & ~zero_mask; // Don't double-insert! This is probably overkill but it's cheap
if (pulse_mask) {
for (int i = 0; i < channels; i++) {
if (pulse_mask & (1 << i)) {
rack::simd::float_4 mask = rack::simd::movemaskInverse<rack::simd::float_4>(1 << i);
float p = pulse_cross[i] - 1.0f;
rack::simd::float_4 x = mask & static_cast<rack::simd::float_4>(-2.0f);
_minblep_generator.insertDiscontinuity(p, x);
}
}
}
}
// Add MinBLEP result
value += _minblep_generator.process();
// ...
Oh, that’s pretty cool! I’m sure mine do that same thing. Like you say, probably a lot do. Why don’t you submit a PR? Seems like unless a bug in a fundamental is catastrophic they never get fixed. (also, if I had a git diff I could see what you did here - compared to the original code
Given that PRs aren’t accepted, and there’s already an outstanding Github issue for this work, I won’t put together a PR, but I will leave a comment on the issue with some info.
That sample implementation is based on Substation’s VCO, not VCV’s, so a diff is less helpful; still, here it is:
minblep_diff.txt (3.4 KB)
THANK YOU SO MUCH!!! I’ve been using Github to submit issues to developers and I also emailed @slimechildaudio, so I really appreciate you looking into this. @Squinky, the only oscillator of yours that I’ve been able to reproduce this on is Substitute.
@slimechildaudio is the Sample SIMD implementation and attached .txt file something I can use to rectify the issue in VCV on your oscillator and others?
Is the best way to address this to Andrew and other developers to link them to this thread on Github issues I’ve already opened (as you did on Bogaudio’s)?
Thanks! This is huge. Once I noticed the problem it started driving me crazy.
I don’t maintain substitute any longer, @robert.kock did all the work to get Squinky Labs in 2.0. I would be very surprised if Substitute is the only Squinky Labs VCO that does this. I would think Basic VCO and perhaps EV3 would also have this problem.
Anyway, afaik the (ancient) bug on github is/was the correct way to report it. VCV has a gigantic backog of work that needs doing, so it’s no surprise (to me) that bugs like this sit around forever.
I’m just going to be a bit bothersome and open a new issue with the linked solution. I’ll also link it to the developers I’ve already opened issues with on Github. I feel like this is a pretty big issue. Once you notice it, it’s impossible to modulate the pulse width of affected modules and not go crazy!
