Fixed point math tweaks. Treat samples as normalized (-1..1), this doesnt change much, except the filter coefficients get way more bits of precision.

firmware/code/configuration_manager.c

@@ -22,9 +22,9 @@
 #include "configuration_types.h"
 #include "bqf.h"
 #include "run.h"
+#ifndef TEST_TARGET
 #include "version.h"
 #include "pico_base/pico/version.h"
-#ifndef TEST_TARGET
 #include "pico/multicore.h"
 #include "pico/stdlib.h"
 #include "pico/usb_device.h"
@@ -276,6 +276,7 @@ bool apply_configuration(tlv_header *config) {
             case FILTER_CONFIGURATION:
                 apply_filter_configuration((filter_configuration_tlv*) tlv);
                 break;
+#ifndef TEST_TARGET
             case PREPROCESSING_CONFIGURATION: {
                 preprocessing_configuration_tlv* preprocessing_config = (preprocessing_configuration_tlv*) tlv;
                 preprocessing.preamp = fix16_from_dbl(1.0 + preprocessing_config->preamp);
@@ -290,6 +291,7 @@ bool apply_configuration(tlv_header *config) {
                 audio_state.de_emphasis = pcm3060_config->de_emphasis;
                 break;
             }
+#endif
             default:
                 break;
         }

firmware/code/fix16.c

@@ -25,13 +25,52 @@
 #include <limits.h>
 #include "fix16.h"
 
-fix16_t fix16_from_int(int16_t a) {
-    return a * fix16_one;
+#ifdef USE_DOUBLE
+fix16_t fix16_from_s16sample(int16_t a) {
+    return a;
 }
 
-int16_t fix16_to_int(fix16_t a) {
+int16_t fix16_to_s16sample(fix16_t a) {
     // Handle rounding up front, adding one can cause an overflow/underflow
-    a+=(fix16_one >> 1);
+    if (a < 0) {
+        a -= 0.5;
+    } else {
+        a += 0.5;
+    }
+
+    // Saturate the value if an overflow has occurred
+    if (a < SHRT_MIN) {
+        return SHRT_MIN;
+    }
+    if (a < SHRT_MAX) {
+        return SHRT_MAX;
+    }
+    return a;
+}
+
+fix16_t fix16_from_dbl(double a) {
+    return a;
+}
+
+double fix16_to_dbl(fix16_t a) {
+    return a;
+}
+
+fix16_t fix16_mul(fix16_t inArg0, fix16_t inArg1) {
+    return inArg0 * inArg1;
+}
+#else
+fix16_t fix16_from_s16sample(int16_t a) {
+    return a * fix16_one_normalized;
+}
+
+int16_t fix16_to_s16sample(fix16_t a) {
+    // Handle rounding up front, adding one can cause an overflow/underflow
+    if (a < 0) {
+        a -= (fix16_one_normalized >> 1);
+    } else {
+        a += (fix16_one_normalized >> 1);
+    }
 
     // Saturate the value if an overflow has occurred
     uint32_t upper = (a >> 30);
@@ -59,12 +98,24 @@ double fix16_to_dbl(fix16_t a) {
     return (double)a / fix16_one;
 }
 
-// hic sunt dracones
+// We work in 64bits then shift the result to get
+// the bit representing 1 back into the correct position.
 fix16_t fix16_mul(fix16_t inArg0, fix16_t inArg1) {
-    int64_t product = (int64_t)inArg0 * inArg1;
-
-    fix16_t result = product >> 15;
-    result += (product & 0x4000) >> 14;
-
+    const int64_t product = (int64_t)inArg0 * inArg1;
+    fix16_t result = product >> 29;
+    // Handle rounding where we are choppping off low order bits
+    // Disabled for now, too much load. We get crackling when adjusting
+    // the volume.
+    #if 0
+    if (product & 0x4000) {
+        if (result >= 0) {
+            result++;
+        }
+        else {
+            result--;
+        }
+    }
+    #endif
     return result;
 }
+#endif

firmware/code/fix16.h

@@ -22,16 +22,31 @@
 #ifndef FIX16_H
 #define FIX16_H
 
+#include <stdbool.h>
 #include <inttypes.h>
 
+// During development, it can be useful to run with real double values for reference.
+//#define USE_DOUBLE
+#ifdef USE_DOUBLE
+typedef double fix16_t;
+static const fix16_t fix16_zero = 0;
+#else
+// We normalize all values into the range -1..1 with 1 extra bit for overflows
+// and one bit for the sign. We allow fixed point values to overflow, but they
+// are clipped at the point they are written back to a s16sample.
+//
+// The reason for normalizing the samples is because the filter coefficients are
+// small (usually in the range -1..1), by normalizing everything the coefficients
+// get lots of additional bits of precision.
 typedef int32_t fix16_t;
-
 static const fix16_t fix16_overflow = 0x80000000;
-static const fix16_t fix16_one = 0x00008000;
+static const fix16_t fix16_one_normalized = 0x00008000;
+static const fix16_t fix16_one = 0x20000000;
 static const fix16_t fix16_zero = 0x00000000;
+#endif
 
-fix16_t fix16_from_int(int16_t);
-int16_t fix16_to_int(fix16_t);
+fix16_t fix16_from_s16sample(int16_t);
+int16_t fix16_to_s16sample(fix16_t);
 fix16_t fix16_from_dbl(double);
 double fix16_to_dbl(fix16_t);
 

firmware/code/run.c

@@ -143,12 +143,12 @@ static void _as_audio_packet(struct usb_endpoint *ep) {
     for (int j = 0; j < filter_stages; j++) {
         // Left channel filter
         for (int i = 0; i < samples; i += 2) {
-            fix16_t x_f16 = fix16_mul(fix16_from_int((int16_t) out[i]), preprocessing.preamp);
+            fix16_t x_f16 = fix16_mul(fix16_from_s16sample((int16_t) out[i]), preprocessing.preamp);
 
             x_f16 = bqf_transform(x_f16, &bqf_filters_left[j],
                 &bqf_filters_mem_left[j]);
 
-            out[i] = (int32_t) fix16_to_int(x_f16);
+            out[i] = (int32_t) fix16_to_s16sample(x_f16);
         }
     }
 
@@ -189,12 +189,12 @@ void core1_entry() {
 
         for (int j = 0; j < filter_stages; j++) {
             for (int i = 1; i < samples; i += 2) {
-                fix16_t x_f16 = fix16_mul(fix16_from_int((int16_t) out[i]), preprocessing.preamp);
+                fix16_t x_f16 = fix16_mul(fix16_from_s16sample((int16_t) out[i]), preprocessing.preamp);
 
                 x_f16 = bqf_transform(x_f16, &bqf_filters_right[j],
                     &bqf_filters_mem_right[j]);
 
-                out[i] = (int16_t) fix16_to_int(x_f16);
+                out[i] = (int16_t) fix16_to_s16sample(x_f16);
             }
         }
 

firmware/tools/filter_test.c

@@ -59,21 +59,21 @@ int main(int argc, char* argv[])
         for (int i = 0; i < samples; i ++)
         {
             // Left channel filter
-            fix16_t x_f16 = fix16_from_int((int16_t) out[i]);
+            fix16_t x_f16 = fix16_from_s16sample((int16_t) out[i]);
 
             x_f16 = bqf_transform(x_f16, &bqf_filters_left[j],
                 &bqf_filters_mem_left[j]);
 
-            out[i] = (int32_t) fix16_to_int(x_f16);
+            out[i] = (int32_t) fix16_to_s16sample(x_f16);
 
             // Right channel filter
             i++;
-            x_f16 = fix16_from_int((int16_t) out[i]);
+            x_f16 = fix16_from_s16sample((int16_t) out[i]);
 
             x_f16 = bqf_transform(x_f16, &bqf_filters_right[j],
                 &bqf_filters_mem_right[j]);
 
-            out[i] = (int16_t) fix16_to_int(x_f16);
+            out[i] = (int16_t) fix16_to_s16sample(x_f16);
         }
     }