SGTL5000 and filter_biquad biquad coefficients are inconsistent/incompatible

grahamwhaley commented 3 years ago

Description

The biquad coefficients generated from the (somewhat undocumented) control_sgtl5000.cpp:calcBiquad() function differ from those generated and required by the filter_biquad.cpp functions, and thus are not directly useable.

The difference is a x*-1 where x==[a0|a1] - that is, the sign of the Ax coefficients is inverted between the two.

That fundamental difference I think comes from comparing the SGTL5000 spreadsheet against the Robert Bristow-Johnson / Audio-EQ-cookbook text.

Steps To Reproduce Problem

I am constructing a system where the software AudioFilterBiquad is used to process and view (on a small tft) the effects of the biquad filters, but the actual audio processing is handled in the hardware sgtl5000 PEQ. I noticed that the coefficients generated from the sgtl5000 calcBiquad, when passed the correct quantization unit for the software biquad, did not work. The filtering works if I call the AudioFilterBiquad calculation functions directly (such as setLowpass()). I added some debug code to the AudioFilterBiquad in the form of a getCoefficents function to allow comparison of the two co-efficents.

This is the addition to allow viewing/extraction of the internal private coefficients:

void AudioFilterBiquad::getCoefficients(uint32_t stage, int *coefficients)
{
  if (stage >= 4) return;
  int32_t *dest = definition + (stage << 3);
  __disable_irq();
  *coefficients++ = *dest++;
  *coefficients++ = *dest++;
  *coefficients++ = *dest++;
  *coefficients++ = *dest++;
  *coefficients++ = *dest++;
  __enable_irq();
}

This is the debug code in my Sketch:

  if(1) { //debug!
    int calc_coeffs_sg5k[5];
    int calc_coeffs_sw[5];
    int swcoeffs[5];
    int swcoeffs2[5];

    calcBiquad(FILTER_LOPASS, 1000, 0.0, 0.707, 524288, AUDIO_SAMPLE_RATE_EXACT, calc_coeffs_sg5k);
    calcBiquad(FILTER_LOPASS, 1000, 0.0, 0.707, 2147483648, AUDIO_SAMPLE_RATE_EXACT, calc_coeffs_sw);
    mybiquad_vis.setLowpass(0, 1000, 0.707);
    mybiquad_vis.getCoefficients(0, swcoeffs);

    mybiquad_vis.setCoefficients(0, calc_coeffs_sw);
    mybiquad_vis.getCoefficients(0, swcoeffs2);

    for( int i=0; i<5; i++ ) {
      Serial.printf("%d: 5k: %d\n", i, calc_coeffs_sg5k[i]);
      Serial.printf("%d: csw: %d\n", i, calc_coeffs_sw[i]);
      Serial.printf("%d: sw: %d\n", i, swcoeffs[i]);
      Serial.printf("%d: sw2: %d\n---\n", i, swcoeffs2[i]);
    }
  }

And here is the resulting serial log output:

0: 5k: 1207
0: csw: 4943449
0: sw: 4943437
0: sw2: 4943449
---
1: 5k: 2414
1: csw: 9886898
1: sw: 9886875
1: sw2: 9886898
---
2: 5k: 1207
2: csw: 4943449
2: sw: 4943437
2: sw2: 4943449
---
3: 5k: 471616
3: csw: 1931738368
3: sw: 1931738443
3: sw2: -1931738368
---
4: 5k: -214298
4: csw: -877770367
4: sw: -877770369
4: sw2: 877770367

The minor numerical differences are I suspect as the SGTL5000 calculations are done as float, but the filter_biquad calculations are done as double. The real obvious difference being the signed-ness of the 4'th and 5'th coefficients - those being a1 and a2.

And, adding code such as this snippet to my sketch makes the code work as expected - the visual representation via the software biquad matches the audio produced via the sgtl5000 hardware biquads.


    //Calculate coeffs for filter_biquad use case
    calcBiquad(FILTER_LOPASS, 1000, 0.0, 0.707, 2147483648, AUDIO_SAMPLE_RATE_EXACT, coeffs);

    //Correct a1 and a2 coeffs for sign inversion 'bug'
    coeffs[3] *= -1;
    coeffs[4] *= -1;
    biquad_vis.setCoefficients(0, coeffs);

Looking at the code, there is indeed a -1 difference between the two (sgtl5000 and filter_biquad) code sets. The primary item can be seen in the filter_biquad code, where we have:

    *dest++ = *coefficients++;
    *dest++ = *coefficients++;
    *dest++ = *coefficients++;
    *dest++ = *coefficients++ * -1;
    *dest++ = *coefficients++ * -1;

I think the easiest overall clean fix would be to remove those * -1 from those two coefficient saves, and modify the internal coefficient calculations to match. Alternatively, one could:

modify the sgtl5000 code to match the filter_biquad code.
heavily document the current difference and requirement.

Hardware & Software

Teensy 4.0 with audio shield Teensyduino v1.53

Arduino Sketch

The full sketch currently relies on having a 128x160tft installed - the snippets above should provide enough information to reproduce - shout if you need me to distill them into a single ino sketch to show the output on the serial port. It is obviously difficult to show the result of the biquads themselves, although not impossible for the software biquad by passing the white noise generator through it and then through the fft and plotting it (which is effectively what I'm doing on my screen).

grahamwhaley commented 3 years ago

@robsoles - iirc, was the sgtl5000 coeff code from you?

robsoles commented 3 years ago

Greetings Graham.

Calc_Biquad was mine. If it does not include a link to where I STOLE the calc from then it has been deviated away from what I submitted.

It has been approaching 3 years since I looked at it, I remember the flipped coefficients; by memory when I first had Paul pull it into his repository the function to load the coefficients [*not mine] was suited to the flipped sign on the coefficients but at some point later someone else [Actually, I think it was Paul himself] contributed the other methods and changed the function that loaded the coefficients [either that routine was previously flipping those two and was changed not to or it was changed to start flipping them] and I just barely fixed it for the employer I was working for at the time before my life got pretty much completely derailed where I was incapacitated for nearly a year and a half. I just got my github account restored last month, because I am employed coding again [well, this one makes me 3D model, circuit design, plus all related testing and programming and debugging, but I hardly hate him for that, unfortunately he is not really into Audio... lol] so it is hardly like I will not help, especially because I do not mind being made responsible for my actions.

I replied to another related github email a few minutes ago, probably about as [un]informative as this; provide me a challenge, perhaps I will meet it ;)

Best, Rob.

On Tue, Dec 29, 2020 at 3:30 AM Graham Whaley notifications@github.com wrote:

@robsoles https://github.com/robsoles - iirc, was the sgtl5000 coeff code from you?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/PaulStoffregen/Audio/issues/372#issuecomment-751775578, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSHQUWGCZFCRVQKQ72CX3LSXCXCDANCNFSM4VMG3JZQ .

robsoles commented 3 years ago

Re: Apparently float is not a pseudonym for double in your environment...

When building for an acoustic guitar preamp using MK20FX simile of the MK20DX256VLH7 I made it a pseudonym taking advantage of the processor's inbuilt FPU but while I was writing I figured that between having a DX and 'quantization error' they may as well be float instead of double at that point - in what may as well be considered the granularity of quantization you only have so many decimal places before digits that were 'rubbished' by quantization except for in the steps of the quantization; using 'double' there is some tiny improvement in calc prior to quantization and then poof, your result gets relegated to the nearest step the quantization can manage. Hardly a real problem here.

When I deployed on the MK20FX we did not get the improvement we wanted until I rewrote the filter routine itself in ASM to use pure floating point in the actual application of the filters; I re-wrote the entire thing to ditch quantization and finally we could soft pluck each string on the guitar, with all of our intended filters engaged, and not detect blasted artifacts on the blasted G and B strings - lol, buggers were bloody shocking until I stored the coeffs and did the biquads fully floating point

to make it as efficient as possible I had different functions to call pending how many filters to apply to this channel sort of thing; One that converted int16_t buffer to a floating point buffer [*no rescale, data is OK without dividing by ~32767.5) while processing first biquad, second just processes 'mid' biquads without having to convert the buffer, last biquad converting routine convert/copies floating point buffer to int16_t buffer; alternative routine to apply a single biquad to a single channel and to do all other audio processing, applying 21 filters across 3 channels mixing down to 1 and just using 120Mhz for main clock the audio library was only eschewing 27% of the processor's time at worst - before taking care of that biquad operation so bloody carefully I was riding 70% at best.

Unfortunately my life derailed within a week of this or I would have a copy of the ASM and all the other fixes I had to do to the libraries involved to get 'sweet' execution on the FX chip; in my current relationship with the people I was doing it for they have stolen the last of my work for them and cut me out of their dealings. Honestly the only real chagrin to me is that I do not have a copy of what I had to do to everything to make it all work so well.

I would link you to somewhere they advertised the preamp, in their video the blasted thing has all my graphics and behaves exactly as I last saw it but they tell me they got someone else to re-write it from scratch, anyway, I would link you [*should be on facebook, the electronics engineer I was working with posted it and I commented something like 'gee, looks familiar' to-wit I got a call explaining about the re-write...] but you have to promise not to behave toward them as if you are on my side or anything like that - call me odd, but if I choose to tackle them then how is something I have not chosen yet and I would prefer to not even be mentioned to them again till I choose such.

Rob.

On Tue, Dec 29, 2020 at 12:55 PM Robert Thompson robsoles.g@gmail.com wrote:

Greetings Graham.

Calc_Biquad was mine. If it does not include a link to where I STOLE the calc from then it has been deviated away from what I submitted.

It has been approaching 3 years since I looked at it, I remember the flipped coefficients; by memory when I first had Paul pull it into his repository the function to load the coefficients [*not mine] was suited to the flipped sign on the coefficients but at some point later someone else [Actually, I think it was Paul himself] contributed the other methods and changed the function that loaded the coefficients [either that routine was previously flipping those two and was changed not to or it was changed to start flipping them] and I just barely fixed it for the employer I was working for at the time before my life got pretty much completely derailed where I was incapacitated for nearly a year and a half. I just got my github account restored last month, because I am employed coding again [well, this one makes me 3D model, circuit design, plus all related testing and programming and debugging, but I hardly hate him for that, unfortunately he is not really into Audio... lol] so it is hardly like I will not help, especially because I do not mind being made responsible for my actions.

I replied to another related github email a few minutes ago, probably about as [un]informative as this; provide me a challenge, perhaps I will meet it ;)

Best, Rob.

On Tue, Dec 29, 2020 at 3:30 AM Graham Whaley notifications@github.com wrote:

@robsoles https://github.com/robsoles - iirc, was the sgtl5000 coeff code from you?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/PaulStoffregen/Audio/issues/372#issuecomment-751775578, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSHQUWGCZFCRVQKQ72CX3LSXCXCDANCNFSM4VMG3JZQ .

PaulStoffregen / Audio