Skip to content

Commit

Permalink
FIR: clipping: Fix FIR filters. Add clipping indicator
Browse files Browse the repository at this point in the history 
When adding the 'FIR filter normalisation' for v1.2, it looks like
I messed up, and made things worse. The primary 'visible' results
from this were:
 - it was very easy to clip the input audio levels, which then gave a
   nasty output
 - the morse decoders were struggling a lot more detecting tones.

The core issue was some mismash of inherited code that meant we ended
up with two notions of 'SAMPLE_RATE', and the wrong one (an index,
not a value) ended up being used for the FIR evaluation, thus
breaking it. Fix that by ensuring we now only have one idea of
SAMPLE_RATE.

Whilst there, it seemed obvious that we already measured the input
volume level with a peak meter, so why not use that to try and give
a clear indication when we were clipping the input. Now when a clipped
input is detected (too loud!), we show a '!' in the input volume slot
for 0.5s to make it clear the input is too high.

Ultimately:
 - FIR filters are now better, so clipping reduced
 - CW decoders seem back to detecting tones better
 - Visual clip indicator should help identify why clipping is happenning.

Mark this as release v1.2, as this is a reasonaly important 'fix' on the
quality front.

Signed-off-by: Graham Whaley <[email protected]>
  • Loading branch information
@grahamwhaley
grahamwhaley committed Mar 15, 2021
1 parent 8e7d37a commit 772d159
Show file tree
Hide file tree
Showing 8 changed files with 87 additions and 169 deletions.
39 changes: 32 additions & 7 deletions DSPham.ino
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ AudioRecordQueue Q_in_L; // We only need one channel - we are working with mono
AudioPlayQueue Q_out_R;
//AudioPlayQueue Q_out_L;

AudioAnalyzePeak input_peak_detector, output_peak_detector;
AudioAnalyzePeak input_peak_detector, output_peak_detector, postfir_peak_detector;

//RMS detection seems to work OK for auto output level matching, but
// then we don't get 'peak-o-meter' to show input levels...
Expand Down Expand Up @@ -76,9 +76,11 @@ AudioConnection patchCord4(firfilter, 0, Q_in_L, 0);
AudioConnection patchCord6(Q_out_R, 0, peak_amp, 0);
AudioConnection patchCord7(peak_amp, 0, i2s_out, 0);
AudioConnection patchCord8(peak_amp, 0, i2s_out, 1);
//Wire up the peak detectors
AudioConnection patchCord9(input_mixer, 0, input_peak_detector, 0);
AudioConnection patchCord10(peak_amp, 0, output_peak_detector, 0);
AudioConnection patchCord11(Q_out_R, 0, toneDetect, 0); //Should we do these after the peak amp?
AudioConnection patchCord10(firfilter, 0, postfir_peak_detector, 0);
AudioConnection patchCord11(peak_amp, 0, output_peak_detector, 0);
AudioConnection patchCord12(Q_out_R, 0, toneDetect, 0); //Should we do these after the peak amp?
AudioConnection patchCord13(Q_out_R, 0, noteFreq, 0); //Should we do these after the peak amp?

//The FFT might be expensive, and we may not be using it - we should probably put an 'amp switch' before it and
Expand Down Expand Up @@ -110,11 +112,15 @@ float32_t FIR_int_state [n_int_states];
// How much gain to apply to try and match the output 'volume' to the original
// input volume.
float32_t peak_gain = 1.0;
float32_t input_peak = 1.0, output_peak = 1.0; //Default start as the same
float32_t input_peak = 1.0, output_peak = 1.0, postfir_peak = 1.0; //Default start as the same
float32_t input_peak_acc = 0;
bool input_peak_clipped = false;

float32_t peak_ratio;
unsigned long peak_ticktime;
#define PEAK_MS_UPDATE 100 //Update 10 times a second
unsigned long peak_clipped_timer;
#define PEAK_MS_CLIPPED_CLEAR 500 //Hold the input clipped signal on the screen for a bit...

unsigned long display_update_deadline = 0;
#define DISPLAY_UPDATE_MS 250
Expand All @@ -124,7 +130,7 @@ unsigned long tone_update_deadline = 0;
#define TONE_UPDATE_MS 250

void setup() {
#ifdef DEBUG
#if DEBUG
Serial.begin(115200);
Serial.println("Starting");
#endif
Expand Down Expand Up @@ -362,13 +368,32 @@ void loop() {
input_peak = input_peak_detector.read();
// Take a rolling average of the input peak for the 'peak' display.
input_peak_acc = (input_peak_acc * 0.9) + input_peak;

//Did we clip the input?
if (input_peak >= 1.0 ) {
//Set the clip flag, and set the timout to clear that flag
input_peak_clipped = true;
peak_clipped_timer = ms + PEAK_MS_CLIPPED_CLEAR;
} else {
//We didn't clip, but have we timed out in order to clear the flag?
if (ms >= peak_clipped_timer)
input_peak_clipped = false;
}
}

if( output_peak_detector.available() )
output_peak = output_peak_detector.read();

if( postfir_peak_detector.available() )
postfir_peak = postfir_peak_detector.read();

//Enable if you need - but we evaluate often, so this generates a lot of output.
// You might want to increase the evaluation timeout if you are debugging, and re-enable this print.
//if (DEBUG) Serial.printf("Peaks in/fir/out: %f:%f:%f\n", input_peak, postfir_peak, output_peak);

// We only compare and calculate against the output peak if we are
// in output tracking mode.
if( (agc_mode == AGC_MODE_TRACK) && (nr_mode != NR_MODE_COMPLETE_BYPASS) ) {
if( output_peak_detector.available() )
output_peak = output_peak_detector.read();

// How different are the input and output peaks?
// Hmm, would it be better to use the RMS here, rather than the peak...
Expand Down
27 changes: 16 additions & 11 deletions dynamicFilters.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -275,41 +275,42 @@ void bandstop(short h[], const int &N, const int &WINDOW, const double &fc1, con

//---------------------------------------------------------------
void audioFilter(short h[], const int &N, const int &TYPE, const int &WINDOW, const double &fc1, const double &fc2) {

switch (TYPE) {
case ID_LOWPASS:
#ifdef DEBUG
#if DEBUG
Serial.print("LowPass Freq:");
Serial.println(fc1);
#endif
lowpass(h, N, WINDOW, fc1/44117);
lowpass(h, N, WINDOW, fc1/AUDIO_SAMPLE_RATE_EXACT);
break;
case ID_HIGHPASS:
#ifdef DEBUG
#if DEBUG
Serial.print("HiPass Freq:");
Serial.println(fc1);
#endif
highpass(h, N, WINDOW, fc1/44117);
highpass(h, N, WINDOW, fc1/AUDIO_SAMPLE_RATE_EXACT);
break;
case ID_BANDPASS:
#ifdef DEBUG
#if DEBUG
Serial.print("BandPass LFreq:");
Serial.print(fc1);
Serial.print(" HFreq:");
Serial.println(fc2);
#endif
bandpass(h, N, WINDOW, fc1/44117, fc2/44117);
bandpass(h, N, WINDOW, fc1/AUDIO_SAMPLE_RATE_EXACT, fc2/AUDIO_SAMPLE_RATE_EXACT);
break;
case ID_BANDSTOP:
#ifdef DEBUG
#if DEBUG
Serial.print("Bandstop LFreq:");
Serial.print(fc1);
Serial.print(" HFreq:");
Serial.println(fc2);
#endif
bandstop(h, N, WINDOW, fc1/44117, fc2/44117);
bandstop(h, N, WINDOW, fc1/AUDIO_SAMPLE_RATE_EXACT, fc2/AUDIO_SAMPLE_RATE_EXACT);
break;
default:
#ifdef DEBUG
#if DEBUG
Serial.println("Unknown");
#endif
break;
Expand All @@ -329,16 +330,20 @@ float32_t getFilterGain(int16_t *coeffs, int ncoeffs, float32_t frequency, float
float32_t gain = 0.0;
float32_t f = frequency / samplerate;

if (DEBUG) Serial.printf("getFilterGain(ncoeff: %d, f:%f, rate:%f\n", ncoeffs, frequency, samplerate);

for(int i=0; i<ncoeffs; i++ ) {
float32_t c = cos(2.0 * M_PI * f * i);
float32_t s = sin(2.0 * M_PI * f * i);

cgain += ((float32_t)coeffs[i]/32768.0) * c;
sgain += ((float32_t)coeffs[i]/32768.0) * s;
cgain += (((float32_t)coeffs[i])/32768.0) * c;
sgain += (((float32_t)coeffs[i])/32768.0) * s;
}

gain = (cgain*cgain) + (sgain*sgain);

if (DEBUG) Serial.printf(" Gain: %fhz, %f sample: %f gain\n", frequency, samplerate, sqrt(gain));

//FIXME - we probably should limit the gain returned somewhere - possibly here.
return (sqrt(gain));
}
Expand Down
26 changes: 0 additions & 26 deletions global.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,32 +24,6 @@ float32_t NR_alpha = 0.95; // default 0.99 --> range 0.98 - 0.9999; 0.95 acts mu
float32_t DMAMEM NR_last_iFFT_result [NR_FFT_L / 2];
float32_t DMAMEM NR_Gts[NR_FFT_L / 2][2]; // time smoothed gain factors (current and last) for each of the 128 bins

uint8_t SAMPLE_RATE = SAMPLE_RATE_44K;
uint8_t LAST_SAMPLE_RATE = SAMPLE_RATE_44K;

const SR_Descriptor SR [18] =
{ // x_factor, x_offset and f1 to f4 are NOT USED ANYMORE !!!
// SR_n , rate, text, f1, f2, f3, f4, x_factor = pixels per f1 kHz in spectrum display
{ SAMPLE_RATE_8K, 8000, " 8k", " 1", " 2", " 3", " 4", 64.0, 11}, // not OK
{ SAMPLE_RATE_11K, 11025, " 11k", " 1", " 2", " 3", " 4", 43.1, 17}, // not OK
{ SAMPLE_RATE_16K, 16000, " 16k", " 4", " 4", " 8", "12", 64.0, 1}, // OK
{ SAMPLE_RATE_22K, 22050, " 22k", " 5", " 5", "10", "15", 58.05, 6}, // OK
{ SAMPLE_RATE_32K, 32000, " 32k", " 5", " 5", "10", "15", 40.0, 24}, // OK, one more indicator?
{ SAMPLE_RATE_44K, 44100, " 44k", "10", "10", "20", "30", 58.05, 6}, // OK
{ SAMPLE_RATE_48K, 48000, " 48k", "10", "10", "20", "30", 53.33, 11}, // OK
{ SAMPLE_RATE_50K, 50223, " 50k", "10", "10", "20", "30", 53.33, 11}, // NOT OK
{ SAMPLE_RATE_88K, 88200, " 88k", "20", "20", "40", "60", 58.05, 6}, // OK
{ SAMPLE_RATE_96K, 96000, " 96k", "20", "20", "40", "60", 53.33, 12}, // OK
{ SAMPLE_RATE_100K, 100000, "100k", "20", "20", "40", "60", 53.33, 12}, // NOT OK
{ SAMPLE_RATE_101K, 100466, "101k", "20", "20", "40", "60", 53.33, 12}, // NOT OK
{ SAMPLE_RATE_176K, 176400, "176k", "40", "40", "80", "120", 58.05, 6}, // OK
{ SAMPLE_RATE_192K, 192000, "192k", "40", "40", "80", "120", 53.33, 12}, // not OK
{ SAMPLE_RATE_234K, 234375, "234k", "40", "40", "80", "120", 53.33, 12}, // NOT OK
{ SAMPLE_RATE_256K, 256000, "256k", "40", "40", "80", "120", 53.33, 12}, // NOT OK
{ SAMPLE_RATE_281K, 281000, "281k", "40", "40", "80", "120", 53.33, 12}, // NOT OK
{ SAMPLE_RATE_353K, 352800, "353k", "40", "40", "80", "120", 53.33, 12} // NOT OK
};

int nr_mode = NR_MODE_SPECTRAL;

// Is the menu active, or should we 'display' our status and decoder output etc.
Expand Down
46 changes: 6 additions & 40 deletions global.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
#include <arm_math.h>

#define VERSION_MAJOR 1
#define VERSION_MINOR 1
#define VERSION_MINOR 2

#define VERSION_UINT16 ((uint16_t)((VERSION_MAJOR<<8)|VERSION_MINOR))

Expand All @@ -16,9 +16,13 @@
#define VERSION_STRING STR(VERSION_MAJOR) "." STR(VERSION_MINOR)

//#define DEBUG 1 //Enable to get serial port output
#define DEBUG 0 //No debug output

#define BUFFER_SIZE 128

//this is the raw hardware rate, before decimation
#define SAMPLE_RATE ((float32_t)AUDIO_SAMPLE_RATE_EXACT)

// Decimate down to 11kHz - see if that helps the NR systems perform, as they will then not be trying
// to operate on the 5-20kHz data, which we never listen to anyway!
#define DF 4
Expand Down Expand Up @@ -60,45 +64,6 @@ extern float32_t NR_alpha;
extern float32_t DMAMEM NR_last_iFFT_result [NR_FFT_L / 2];
extern float32_t DMAMEM NR_Gts[NR_FFT_L / 2][2]; // time smoothed gain factors (current and last) for each of the 128 bins

#define SAMPLE_RATE_MIN 6
#define SAMPLE_RATE_8K 0
#define SAMPLE_RATE_11K 1
#define SAMPLE_RATE_16K 2
#define SAMPLE_RATE_22K 3
#define SAMPLE_RATE_32K 4
#define SAMPLE_RATE_44K 5
#define SAMPLE_RATE_48K 6
#define SAMPLE_RATE_50K 7
#define SAMPLE_RATE_88K 8
#define SAMPLE_RATE_96K 9
#define SAMPLE_RATE_100K 10
#define SAMPLE_RATE_101K 11
#define SAMPLE_RATE_176K 12
#define SAMPLE_RATE_192K 13
#define SAMPLE_RATE_234K 14
#define SAMPLE_RATE_256K 15
#define SAMPLE_RATE_281K 16 // ??
#define SAMPLE_RATE_353K 17 // does not work !
#define SAMPLE_RATE_MAX 15

extern uint8_t SAMPLE_RATE;
extern uint8_t LAST_SAMPLE_RATE;

typedef struct SR_Descriptor
{
const uint8_t SR_n;
const uint32_t rate;
const char* const text;
const char* const f1;
const char* const f2;
const char* const f3;
const char* const f4;
const float32_t x_factor;
const uint8_t x_offset;
} SR_Desc;

extern const SR_Descriptor SR [18];

// global decoder stuff
#define DECODER_OFF 0
#define DECODER_MORSE 1
Expand Down Expand Up @@ -146,6 +111,7 @@ extern float32_t agc_sg5k_decay;

// From the main loop
extern float32_t input_peak_acc;
extern bool input_peak_clipped;

//Display our output (or, is the menu active...)
extern bool display;
Expand Down
4 changes: 4 additions & 0 deletions lcd.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,10 @@ CPU is updated in the main loop.
if (invol == 0 ) buf[0] = ' ';
else buf[0] = invol;

//And then, if we clipped on input, overwrite with a *splat* char to try and make
// it visually obvious.
if (input_peak_clipped) buf[0] = '!';

if (nr_mode != NR_MODE_COMPLETE_BYPASS ) {
//Filter mode
switch (current_filter_mode) {
Expand Down
28 changes: 19 additions & 9 deletions menu.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,28 +125,38 @@ void updateFilter() {
int ncoeff = filterList[current_filter_mode].coeff;
float32_t centref = (filterList[current_filter_mode].freqLow + filterList[current_filter_mode].freqHigh) / 2.0;
const float32_t maxgain = 2.0;
float32_t gain;
float32_t gain, multiplier;

audioFilter(fir_active1,
ncoeff,
filterList[current_filter_mode].filterType,
filterList[current_filter_mode].window,
filterList[current_filter_mode].freqLow,
filterList[current_filter_mode].freqHigh );
filterList[current_filter_mode].freqHigh);

gain = getFilterGain(fir_active1, ncoeff, centref, SAMPLE_RATE/(uint32_t)DF);
//Don't forget - the FIR filters happen before decimation, so are at the full sample rate...
gain = getFilterGain(fir_active1, ncoeff, centref, SAMPLE_RATE);

//For CW filters at least, we get a tiny gain - requiring a multiplication factor of
// ~167 in some cases for instance - and this seems to make the FIR filter then just generate
// hiss and noise, even when fed no signal.
// Thus, try limiting the max 'gain' to something 'sensible'. Well, OK, I'd like it so we never
if (DEBUG) Serial.printf("FIR default gain measured as %f\n", gain);

// Try limiting the max 'gain' to something 'sensible'. Well, OK, I'd like it so we never
// need to apply any gain to the FIR coefficients in the first place, but that is not what we seem
// to get from the FIR dynamic calculators.
if (1.0/gain > maxgain) gain = 1.0/maxgain;
if (1.0/gain > maxgain){
if (DEBUG) Serial.println("Clipping FIR gain");
multiplier = maxgain;
} else {
multiplier = 1.0/gain;
}

//Scale the multiplier down to 90%, so we avoid any risk of clipping
multiplier *= 0.9;

if (DEBUG) Serial.printf("FIR multiplier set to %f\n", multiplier);

//And scale it so we try not to be at 100% for a pure signal, to try and avoid
// any potential clipping (unlikely it is that we will ever end up in that situation).
normaliseCoeffs(fir_active1, ncoeff, 0.9/gain);
normaliseCoeffs(fir_active1, ncoeff, multiplier);

firfilter.begin(fir_active1, ncoeff);

Expand Down
Loading

0 comments on commit 772d159

Please sign in to comment.