SNEX weirdness or me ?
-
@Christoph-Hart Oh yes it was defined below, along with other things...
Anyway I changed this code quite a bit, and at the same time I am training myself to use the
sdouble
type. I am still struggling but I can get the class to work it seems, so I'll come back to report -
@ustk you only have to use
sdouble
for values that come from the outside and need to be smoothed, for internal filter coefficients or state variables that hold the last sample it's counterproductive. -
@Christoph-Hart Ok so here's the problem I have
As you can see, in the second span element,sigma
updates, but not the other variablesI know how to run this as separate channels within the block processing just by using a span of last value, but I wanted to implement a filter as class
I am doing it already successfully for another DSP, but here I can't get it to work
What is the silly thing I am doing right?Also:
- Would it be possible to get the
static const
to show up in the table? - How come the non
static
variables are initialised to0.
if they are not set a second time inprepare
? (I'm referring tofs
,T
,twoOverT
...)
template <int NV> struct VA_LPF_1st_v2 { SNEX_NODE(VA_LPF_1st_v2); enum Parameters { Freq = 0 }; class MyFilter { public: double processFilter(double s) { sigma1 = s - z1; s_alpha = sigma1 * a0; y = s_alpha + z1; z1 = s_alpha + y; return y; } private: double sigma1 = 0.0; double s_alpha = 0.0; double y = 0.0; double z1 = 0.0; }; //! ================================================== Prepare & Reset ================================================ void updateCoefficients() { if(fs > 0.0) { const double wa = twoOverT * Math.tan((TWO_PI * cutoff * T) / 2.0); a0 = (wa * T) / 2.0; } } void prepare(PrepareSpecs ps) { if(fs != ps.sampleRate) { fs = ps.sampleRate; T = 1.0 / fs; twoOverT = 2.0 * fs; a0 = 0.0; updateCoefficients(); } } void reset() { } //! ============================================== Process the signal here ============================================== // Process the signal as block here template <typename ProcessDataType> void process(ProcessDataType& data) { const int numChan = Math.min(data.getNumChannels(), MAX_NUM_CHANNELS); for(int i = 0; i < numChan; i++) { idx = i; for(auto& s: data[i]) { s = (float)vaLpfFirst[idx].processFilter((double)s); } } } // Process the signal as frame here template <int C> void processFrame(span<float, C>& data) { } //! ===================================================== Parameters =================================================== // Set the parameters here template <int P> void setParameter(double v) { if (P == Parameters::Freq) { cutoff = Math.range(v, 20.0, 20000.0); updateCoefficients(); } } void handleHiseEvent(HiseEvent& e) { } void setExternalData(const ExternalData& d, int index) { } private: static const int MAX_NUM_CHANNELS = 2; static const double TWO_PI = 2.0 * Math.PI; span<MyFilter, MAX_NUM_CHANNELS> vaLpfFirst; using IndexType = index::clamped<MAX_NUM_CHANNELS, false>; IndexType idx; double fs = 48000.0; double T = 1.0 / 48000.0; double twoOverT = 2.0 * 48000.0; double cutoff = 1000.0; double a0 = 0.0; };
- Would it be possible to get the
-
- Your
TwoOverT
is in factTwoTimesT
. This blows up the filter immediately :) - You're still using an outer class variable in
processFilter
(in this casea0
). The error is that SNEX compiles and doesn't complain, not that it doesn't work. I'll try to isolate this issue in the playground and make a test for it.
- Your
-
So I could dumb it down to a minimal test case which is indeed failing:
struct Outer { struct Inner { int processInner() const { // this should not compile, // the variable resolver must detect that we're // in the inner class and not the Outer return a0; } int value = 0; }; Inner data1; int a0 = 12; int processOuter() { return data1.processInner(); } }; Outer outer; int main(int input) { return outer.processOuter(); }
So the parser does not detect that it's an illegal operation and just creates a pointer with a 4 byte offset (as it would for the valid member access). This then works with the first instance because the memory address of data1 is the same as the Outer object itself, but it will not work with other memory addresses (this is why your filter failed at the second channel).
Now the fix shouldn't be too difficult but it's been quite some time that I crawled around in the internals of the SNEX compiler...
-
@Christoph-Hart said in SNEX weirdness or me ?:
- Your
TwoOverT
is in factTwoTimesT
. This blows up the filter immediately :)
Nope, T = 1/fs, TwoOverT = 2 / (1 / fs) => simplifies in 2*fs
- You're still using an outer class variable in
processFilter
(in this casea0
).
I am trying to keep common/shared things together and separate what it needs to be. Now I could place
a0
as class member but would it be better or not, this I don't know. :smiling_face_with_halo:I was thinking about a shared memory issue or something related, but as for the technicality of it
In the case of such simple DSP like filter, harmonics, etc... Would using
struct
overclass
change anything in the context of SNEX? - Your
-
Nope, T = 1/fs, TwoOverT = 2 / (1 / fs) => simplifies in 2*fs
One formula (yours) is blowing the filter into NaNs immediately and one formula (mine) makes the filter work ok(ish) - it still blows up if the cutoff frequency goes near Nyquist.
I am trying to keep common/shared things together and separate what it needs to be.
Yes that's a good approach, all you need to do to make it work is to add another argument to the
processFilter()
function where you pass ina0
from the outer class. It's just not legal C++ to access the outer member from the inner class. -
@Christoph-Hart said in SNEX weirdness or me ?:
One formula (yours) is blowing the filter into NaNs immediately and one formula (mine) makes the filter work ok(ish) - it still blows up if the cutoff frequency goes near Nyquist.
Yeah I got that blowing which is part of this algorithm (Pirkle/Zavalishin) so it is mandatory to OS at a minimum of x2, and only then I could stabilise it
It's just not legal C++ to access the outer member from the inner class.
Oh thanks! That is quite an important hint!
-
@ustk should be fixed - the compiler will scream at you now…
-
@Christoph-Hart So in the end I was doing something silly that didn't need a fix if I had followed the conventional approach of C++
-
@ustk yes, but it's the job of the compiler to tell you that you're doing something stupid and the actual effect it had on your class was totally unpredictable, so it's definitely better now than before :)
-
@Christoph-Hart Hmmm... So now it complains on the index type in the process template
private: using IndexType = index::clamped<MAX_NUM_CHANNELS, false>; IndexType idx;
But it is impossible to stick it as member parameter or declare it as constant, am I right?
So what would be the procedure then?
Sorry for the C++ crash course I still need... -
@Christoph-Hart both process and idx are in the same template so this shouldn't be an issue if I am not mistaken
-
@ustk Boing boing
-
@Christoph-Hart Even doing this throws the error:
It seems
index
is considered to be outside -
@ustk ah yeah actually I caught a similar error with the index classes in my test suite when I implemented the fix but it seems that the test suite didn‘t cover index usage within a class.
Back to the SNEX compiler internals I guess…
-
Alright, please try again, now it should not mess with the index templates anymore.
-
@Christoph-Hart Thanks! Building now...
-
@Christoph-Hart Working smoothly! Thanks a lot for the fix!