958473 - Improve the OscillatorNode, when using BLITs

Status: RESOLVED WONTFIX

(Core :: Web Audio, defect, P3)

Description

[Paul Adenot (:padenot)]

From bug 952860:

(In reply to :Karl Tomlinson (back Jan 28) from comment #4)
> Comment on attachment 8357168 [details] [diff] [review]
> Use a DC blocker on the square and triangle oscillators. r=
> 
> (In reply to Paul Adenot (:padenot) from comment #3)
> > The paper from which I implemented this clearly stated that leaky integrator
> > / dc blockers would be needed for this technique to work.
> 
> Yes, I understood leaky integrators were necessary for floating point
> rounding
> and at frequency changes.  Perhaps they are also enough smooth out the
> integration discretization error.  The paper suggests varying the leak
> coefficient when the frequency changes.  Perhaps that's an option for future
> improvement.
> 
> > > But we should have some leaky integration regardless.
> > > I guess 0.999 would be time constant of about 20-25 ms at 44.1 kHz.
> > > i.e. a high pass filter at about 40-50 Hz.
> > 
> > I now remember why I chose 0.995: its cut frequency is lower than 20Hz,
> > which is kind of the limit under which it does not really matter anymore
> > (considering audio equipment and ears).
> 
> I guess my time constant to filter cut-off conversion must be wrong.  0.999
> should give a lower cut-off and less distortion, so keep that if it works
> well
> enough.  The choice now is a bit arbitrary and we can tune later when other
> issues are addressed.
> 
> > Anyways, leaky integrating should work, but I switch to merely using `sLeak`
> > in this patch, and we still get a bad DC offset.
> 
> Are you are suggesting that leaky integration is different from using
> DCBlocker, or just that the parameter is different?  I would have expected
> 0.995 to kill offsets more rapidly, so I don't understand.
> 
> -      mSquare += BipolarBLIT();
> +      mSquare = 0.999 * mSquare + BipolarBLIT();
> 
> -      mSaw += UnipolarBLIT() - dcoffset;
> +      mSaw = 0.999 * mSaw + UnipolarBLIT() - dcoffset;
> 
> should be equivalent to this patch, and seems to be in my testing.  You can
> go
> ahead and assume my r+ if you are happy to land a patch like that, with a
> coefficient of sLeaky or closer to 1, whatever you think best.  (I won't be
> around next week.)
> 
> > I think the problem we have
> > is that we don't do anything when changing frequency, whereas I think we
> > should be having new integration initial condition for the new frequency /
> > phase combination. I'm also pretty sure my maths for the initial integration
> > condition at t = 0 were wrong.
> 
> One problem is that the integration sample of the BLIT() should be in the
> middle of a phase increment at the current frequency.  Currently the BLIT is
> performed at t, to integrate from t - 1/(2*samplerate) to
> t + 1/(2*samplerate), assuming the frequency is constant over that period.
> The phase increment is applied to increment the phase from t to
> t + 1/samplerate assuming the frequency is constant over that period.
> When the frequency changes, one of these assumptions is incorrect.
> I think either the calculation of the BLIT or the phase increment should be
> offset by 1/(2*samplerate).
> 
> This also helps consider the initial conditions, which should be for the
> point 
> 1/(2*samplerate) before the BLIT sample.
> 
> The triangle is more complex because the double integration means that DC
> offsets add up before they are smoothed by the leaking.  The mSquare
> integration may need to be leakier for the triangle than for the plain
> square.
> 
> I also expect some jumps on frequency changes due to harmonic count changes
> changing the positions of the bumps.  Resetting initial conditions may halve
> this error if done well but it is tricky to set the initial conditions well
> near the step in the function.  Perhaps blending in the harmonic count
> changes
> may be an option.
> 
> > In the meantime, we can probably take a patch like this.
> 
> Yes.  I'm marking this version r- because the leaky integrator is much
> more concise, and easier to see the equation, than using DCBlocker.

and then:

(In reply to :Karl Tomlinson (back Jan 28) from comment #5)
> Created attachment 8358245 [details] [diff] [review]
> blit integration experiments
> 
> I'm just putting up this patch to record what I experimented with.
> These things at least we should do at some point:
> 
> * Leaky integrate mSquare for the triangle wave.
> 
> * Remove C6 because mSquare has no DC offset.
> 
> * Move the mSignalPeriod adjustment on mTriangle into the integrand so it is
>   used while it is based on the current frequency.
> 
> These were other experiments that I'm not sure about or not sure I have right
> yet:
> 
> * Integrate UniBLIT at half sample interval offset.
> 
> * Get another harmonic for BPBLIT so that frequencies > nyquist/2
>   generate output.
> 
> * Integrate at BPBLIT at twice the sample rate.

Comment 1

[Paul Adenot (:padenot)]

We don't use BLIT anymore, marking WONTFIX.