James McCartney <asynth@xxxxxx> wrote:
>This is not correct. The phase modulation should not be accumulated
>into the phase.
Im not sure why you say im storing phase modulation _into_ the phase. In each frame rate_acum is computed completely independent from phaseOffset. Note the phase var is not incrementally defined, but in each loop its defined to be rate_acum + ZXP(phaseOffset), and is this value which is used to compute if a new spike should be sent or not. Only rate_acum is incrementally defined. rate_acum is indeed the phase computed as if there were no phaseOffset at all.
The problem is that when phaseOffset is k-rate, the phaseOffset is computed incrementally from a piece of linear slope, but in a-rate to follow this route one should add each loop ZXP(phaseOffset) - prev_phaseOffset, where prev_phaseOffset is the value of ZXP(phaseOffset) in the previous frame, which is not efficient.
The problem i see is of naming; Indeed im using mPhase as it was a mRate_acum, since im storing in unit->mPhase the value of rate_acum. But probably is not worth to add a new var in the Impulse struct just to have a consistent naming for a particular rate computation? Probably that should be commented better?
>If phase modulation is audio rate, I don't see a reason to
>store mPhaseOffset. The phase offset is updated every sample, so
>storing it should be unecessary. All you need to store is phase.
I store mPhaseOffset at the end of the function to be used in the first frame of the next block to compute the phase. Consistenly each frame (no matter if its at the first, or the other frames) phase is always rate_acum + phaseOffset.
> void Impulse_next_aa(Impulse *unit, int inNumSamples)
> {
> float *out = ZOUT(0);
> float *freq = ZIN(0);
> float *phaseOffset = ZIN(1);
>
> float freqmul = unit->mFreqMul;
> double phase = unit->mPhase;
> double rate_acum = unit->mPhase;
> // double phaseSlope = CALCSLOPE(phaseOffset, prev_phaseOffset);
> phase += unit->mPhaseOffset;
>
> LOOP(inNumSamples,
> float z;
>
> if (phase >= 1.f) {
> rate_acum -= 1.f;
> z = 1.f;
> } else if (phase < 0.f) {
> rate_acum += 1.f;
> z = 1.f;
> }
> else {
> z = 0.f;
> }
> rate_acum += (ZXP(freq) * freqmul);
> phase = rate_acum + ZXP(phaseOffset);
> ZXP(out) = z;
> );
>
> unit->mPhase = rate_acum;
> unit->mPhaseOffset = phase - rate_acum;
> }