RFC: Sweep and Phasor subsample interpolation fixes

[elgiano]

@nufets

I’d second your point about overwhelming users with new variants of UGens in suggestion C. However, what about leaving interpolation in Phasor and Sweep as it is and instead creating new variants (don’t know - PhasorN, SweepN or so?) that have interpolation removed. That would at least eliminate the risk of breaking existing user code, wouldn’t it?

Sorry I had missed your comment! I have two concerns:

• interpolation in Sweep and Phasor is calculated wrong: we would need to break user code anyways if we wanted to fix it. We could argue that user code is already broken since interpolation adds the wrong offset in a way that is hard to mitigate.
• having the default UGens doing interpolation (even if fixed) would make them behave unexpectedly with Impulse, which is arguably the most basic use case.

So I think it would be preferable to have default ugens without interpolation and add new UGens that do it. There is always the option to keep the old UGens as PhasorOld or PhasorGlitch, or to just leave them be and make PhasorN and PhasorL, but I think this is for a subsequent discussion if we decide to go for adding UGens.

[elgiano]

Now that I’m thinking about a ramp going [1, 0, 0.1] I thought of another problem: if a ramp like that crosses zero between samples, for example [0.6, 0.9, 0.2] where the zero was between y[0] and y[-1], it would never be recognized as a trigger.

{PulseCount.ar(Phasor.ar(rate: 0.3))}.loadToFloatArray(0.001, action:_.postln)
// phasor: [0.0, 0.3, 0.6, 0.9, 0.2, 0.5, 0.8, 0.1, 0.4, 0.7, 1.19e-17, 0.3, ...]
// count:  [0.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, ...]

So one would have to convert ramp to trigger manually (and also calculate and add offset manually). But at this point we would want correct trig behavior and no interpolation.

I’m starting to think we can’t support unipolar ramps as triggers at all. So, linear interpolation would only be useful for bipolar signals, that cross zero after having been negative, bringing back James’ formula. But I still think linear interpolation should be made controllably optional somehow.

[dietcv]

hey, i think if we want to try to make option B available, then an unmodulated, unipolar, linear ramp between 0 and 1 is definetly the most common use case for a continuous signal.
With all my research i have done, it seems to me that using unipolar ramps as a source of time is not really common in SC, but it definetly is in other creative coding environments.
Deriving triggers, slopes etc. from a ramp signal seems also not to be very common.
Maybe thats because we have Demand Ugens, which are implementing on the server what we are used to use with the Pattern library in the language. It seems to me that the same thinking about events, which we find in the language is transferred to event scheduling on the server with Demand Ugens, where i think server side sequencing could be thought differently with all the benefits you get from sub-sample accuracy, when using unipolar ramp signals.
For me it would be more beneficial if there was a guide which explains what you can do with ramp signals then having a Ugen with does everything behind the scenes.
I think that im alone with this point of view, but thats okay.

[jamshark70]

@elgiano

Now that I’m thinking about a ramp going [1, 0, 0.1] I thought of another problem: if a ramp like that crosses zero between samples, for example [0.6, 0.9, 0.2] where the zero was between y[0] and y[-1], it would never be recognized as a trigger.

Well… to get into that scenario (a subsample local minimum)… even in a “simple” unipolar ramp case, e.g., [0.0, 0.125, 0.25, 0.375, 0.5, 0.625, 0.75, 0.875, 0.0, 0.125, 0.25...], the real trigger point is not exactly on the 0.0 sample.

E.g., let’s lay out 4x this ramp, in 32 samples. Then use FFT to interpolate up to 1024 samples, band-limited.

(
var n = 32, n2 = 1024;
var halfN = n div: 2, halfN2 = n2 div: 2;

~original = Pn(Pseries(0.0, 1/8, 8), inf).asStream.nextN(32);

~fft = ~original.as(Signal).fft(Signal.newClear(n), Signal.fftCosTable(n));

~fft2 = Complex(
	Signal.newClear(n2)
	.overWrite(~fft.real[0 .. halfN], 0)
	.overWrite(~fft.real[halfN ..], n2 - halfN),
	Signal.newClear(n2)
	.overWrite(~fft.imag[0 .. halfN], 0)
	.overWrite(~fft.imag[halfN ..], n2 - halfN)
);

// n2/n corrects for FFT normalization
~upsampled = ~fft2.real.ifft(~fft2.imag, Signal.fftCosTable(n2)).real * (n2/n);

~plot = [
	~original.as(Array).resamp1(n2),  // actually this isn't exactly right, but, eh...
	~upsampled.as(Array)
].lace(n2*2).plot(numChannels: 2);
)

The wiggly line is the true signal represented by the 8-sample phasor.

The 0 sample, then, is on the down slope, and the upward zero crossing is itself a subsample offset:

(
~zeroxIndex = block { |break|
	~upsampled.doAdjacentPairs { |a, b, i|
		if(a < 0 and: { b > 0 }) {
			break.(i)
		}
	}
};

[~zeroxIndex, ~zeroxIndex * (~original.size / ~upsampled.size)]
)

-> [20, 0.625]  // 5/8, so, subsample offset should be 3/8 of a sample

So if we really want to be completely theoretically correct about subsample offsets, then we should sinc-oversample the trigger input to Phasor and Sweep… which… that would be CPU-heavy in a way that I’m pretty sure nobody wants.

We could get the “doesn’t touch 0” case by shifting and downsampling the upsampled signal:

~shiftedOrig = ~upsampled.as(Array)[~zeroxIndex + 5, ~zeroxIndex + 5 + (~upsampled.size div: ~original.size) ..];

~shiftedOrig.plot;

… which exposes the curved nature of the bandlimited signal (and uses that to encode the phase shift). Yep, this is what a subsample phase shift of a “linear” ramp looks like – jump to 20:30 in this video – Xiph.Org Video Presentations: Digital Show & Tell (truly excellent video btw – it dispels many, many misunderstandings about sampled signals).

Looking at these trigger inputs as a series of linear segments is already an approximation. It’s a useful approximation when using a phasor as a trigger, but it was never correct. If the trigger input has a more “interesting” shape, then the linear approximation becomes less useful.

I’m starting to think we can’t support unipolar ramps as triggers at all.

That’s one reason why I brought up Max/MSP’s [sah~] – if the trigger is a unipolar ramp (as [phasor~] is), then it requires a threshold > 0. It doesn’t support 0 as both the threshold and the ramp’s lower bound.

hjh

[elgiano]

@dietcv

For me it would be more beneficial if there was a guide which explains what you can do with ramp signals then having a Ugen with does everything behind the scenes.
I think that im alone with this point of view, but thats okay.

I totally agree! I don’t think Sweep and Phasor should do interpolation behind the scenes. A user knows what signal a Phasor is given as an input, and could choose explicitly if the UGen should interpolate or not. So I would go for options C or A. Additionally, using an unipolar phasor as a subsample-accurate trigger requires pre-processing (and you’ve posted quite some formulas for it, on the github issue). We could work towards an HelpFile with some examples.

That said, it seems like the only viable case for interpolation is if the input is a bipolar signal. Under this condition, the formulas @jamshark70 and @mike proposed would be viable, as they wouldn’t interpolate anything that doesn’t go negative.
But I also think it would be complicated to explain this behavior:
“this UGen does interpolation only if its input transitions from negative to positive, or if it transition from 0 to positive and the sample before 0 was negative”
compared to
“this UGen does linear interpolation, this other does not”
But maybe there are clearer ways to express it?

@jamshark70

So if we really want to be completely theoretically correct about subsample offsets, then we should sinc-oversample the trigger input to Phasor and Sweep… which… that would be CPU-heavy in a way that I’m pretty sure nobody wants.

thanks @jamshark70 for the insights, it all makes sense, and I agree we wouldn’t want such basic UGens to upsample their trigger input

[nufets]

Thanks for the clarification! So, I was under the (wrong) impression that option C meant to leave Phasor and Sweep as they are, just under different names. All good. I’m just a lurker but somehow interested in the topic. Yet I don’t really know how the outcome would affect my work or how I would even gain from it. Just curious

[nathan]

I confess I am having trouble understanding the status of this conversation, so forgive me if this is a non-issue or has already been resolved, but I have a comment as a stakeholder.

I frequently write Hasher.ar(Sweep.ar(Impulse.ar(60))) to get looped white noise, which relies on the fact that I’m getting the exact same sequence of floating-point sample values out of Sweep at each trigger. It’s fine if those floating-point values change between versions of SC, since that just re-randomizes the white noise, but I need them to be identical for each impulse. If this assumption stops working, it will break numerous SynthDefs I’ve written over the years.

[jamshark70]

@nathan

It’s fine if those floating-point values change between versions of SC, since that just re-randomizes the white noise, but I need them to be identical for each impulse.

Not to worry –

Option A would add an input to let you specify whether you want subsample interpolation or not (you’d set it to no interpolation).

Option B would treat positive-or-zero signals as impulses, and also not interpolate in that case.

Option C would provide both subsample-interpolating and non-interpolating Sweep units, with (I believe) the default Sweep being the non-interpolating version that you want (with SweepL as an alternative).

There is no move to remove non-interpolating Sweep/Phasor.

hjh

[aleksandarkoruga]

There’s a technique used in old shader code where a similar problem occurred with pixels. The solution was to add a half to the pixel hence it was called “half-pixel offset”, for pixel center correction. It’s basically adding a half (0.5) to the coordinates to have the correct interpolation. Correct me if i’m wrong, but I think this solution might be a simpler fix?

[dietcv]

The sub-sample offset is dependent on the rate of the continious signal (e.g. the higher the rate the more it has advanced within one sample).

[dietcv]

This might be a bit offtopic (so i have collapsed the code). If i understand you correctly there is a related technique for anti-aliasing of a ramp signal (it works even better then the native Saw Ugen).

The simple idea for bandlimiting in this case is instead of trying to synthesize an ideal phasor, to synthesize a function that already has the single-sample slope requirement embedded into it. This function should be independent from the sample points and is used to place the slope precisely centered on the ideal phasors transition point rather than the sampling grid.

To do that you calculate the values of the ideal phasor one-half a sample frame before and after it has a transition and the subsample location of the ideal transition relative to the sampling grid to get the ramp between these two corner points and replace the naive phasor when it is in its “wrapping frame” with a “replacement phasor” taken from these calculations.

But this should not be confused with what should happen when a signal resets the ramp signal.

phasorAntiAliased

(
var phasorAntiAliased = { |freq|

	var slope, phase;
	var loopSubSample, detectTrans;
	var phaseBeforeTrans, phaseAfterTrans;
	var replacePhaseBeforeTrans, replacePhaseAfterTrans;

	slope = freq * SampleDur.ir;
	phase = Phasor.ar(DC.ar(0), slope);

	loopSubSample = phase.wrap(-0.5, 0.5) + (slope * 0.5) / slope;
	detectTrans = (loopSubSample >= 0) * (loopSubSample < 1);

	phaseBeforeTrans = (phase - (slope * loopSubSample)).wrap(0, 1);
	phaseAfterTrans = (phase - (slope * (loopSubSample - 1))).wrap(0, 1);

	replacePhaseBeforeTrans = Select.ar(detectTrans, [
		phase,
		phaseBeforeTrans
	]);

	replacePhaseAfterTrans = Select.ar(detectTrans, [
		phase,
		phaseAfterTrans
	]);

	LinXFade2.ar(
		replacePhaseBeforeTrans,
		replacePhaseAfterTrans,
		loopSubSample.clip(0, 1) * 2 - 1
	);

};

{
	var sig;
	sig = phasorAntiAliased.(\freq.kr(4017));
	//sig = Saw.ar(\freq.kr(4017));
	sig = LeakDC.ar(sig);
	sig!2 * 0.1;
}.play
)

s.freqscope;

[dietcv]

@elgiano

We could work towards an HelpFile with some examples.

Thats a cool idea Some of the examples i have shown on github for deriving triggers at zero crossings (non-positive to positive transition), when a signal changes slope etc. can also be implemented with native HPZ1 instead of manually calculating the delta using Delay1.
I would actually love to have a Ugen which just calculates the delta (current - previous). All these Ugens like Slope, HPZ1 etc. calculate the delta and multiply it with some additional stuff.
In the case of Slope I would even say that its incorrect to multiply by the sampling rate, because then you get frequency in Hz instead of a normalized slope.

[jamshark70]

@dietcv

I would actually love to have a Ugen which just calculates the delta (current - previous).

I think that would be y - Delay1.ar(y) – I’m not sure it would be worth adding a C++ UGen only for this.

hjh

[dietcv]

yeah, its just y - Delay1.ar(y), i was just pointing out that the ugens we normally use for this (Slope, HPZ1, etc.) are additionally doing something else. Drastically speaking we could get rid of all those and just have one Delta ugen and additionally multiply by 0.5 (like HPZ1 does) or by the sampling rate (like Slope does).

Edit: or like delta.abs > threshold like Changed does.