Merge #190

190: Feature/phase tools r=jordens a=jordens



Co-authored-by: Robert Jördens <rj@quartiq.de>

dsp/src/pll.rs

@@ -10,9 +10,8 @@ use serde::{Deserialize, Serialize};
 /// stable for any gain (1 <= shift <= 30). It has a single parameter that determines the loop
 /// bandwidth in octave steps. The gain can be changed freely between updates.
 ///
-/// The frequency settling time constant for an (any) frequency jump is `1 << shift` update cycles.
-/// The phase settling time in response to a frequency jump is about twice that. The loop bandwidth
-/// is about `1/(2*pi*(1 << shift))` in units of the sample rate.
+/// The frequency and phase settling time constants for an (any) frequency jump are `1 << shift`
+/// update cycles. The loop bandwidth is about `1/(2*pi*(1 << shift))` in units of the sample rate.
 ///
 /// All math is naturally wrapping 32 bit integer. Phase and frequency are understood modulo that
 /// overflow in the first Nyquist zone. Expressing the IIR equations in other ways (e.g. single
@@ -20,7 +19,8 @@ use serde::{Deserialize, Serialize};
 ///
 /// There are no floating point rounding errors here. But there is integer quantization/truncation
 /// error of the `shift` lowest bits leading to a phase offset for very low gains. Truncation
-/// bias is applied. Rounding is "half up".
+/// bias is applied. Rounding is "half up". The phase truncation error can be removed very
+/// efficiently by dithering.
 ///
 /// This PLL does not unwrap phase slips during lock acquisition. This can and should be
 /// implemented elsewhere by (down) scaling and then unwrapping the input phase and (up) scaling
@@ -89,6 +89,7 @@ mod tests {
                 assert_eq!(f.wrapping_sub(f0).abs() <= 1, true);
             }
             if i > n / 2 {
+                // The remaining error is removed by dithering.
                 assert_eq!(y.wrapping_sub(x).abs() < 1 << 18, true);
             }
         }

dsp/src/unwrap.rs

@@ -1,23 +1,25 @@
 use serde::{Deserialize, Serialize};
 
-/// Get phase wrap from x to y.
+/// Subtract `y - x` with signed overflow.
 ///
-/// Phases are modulo integer overflow.
-///
-/// Args:
-/// * `x`: Old phase sample
-/// * `y`: New phase sample
+/// This is very similar to `i32::overflowing_sub(y, x)` except that the
+/// overflow indicator is not a boolean but the signum of the overflow.
+/// Additionally it's typically faster.
 ///
 /// Returns:
-/// A tuple containg the (wrapped) phase difference and
-/// one times the direction of the wrap.
-pub fn get_wrap(x: i32, y: i32) -> (i32, i8) {
+/// A tuple containg the (wrapped) difference `y - x` and the signum of the
+/// overflow.
+#[inline(always)]
+pub fn overflowing_sub(y: i32, x: i32) -> (i32, i8) {
     let delta = y.wrapping_sub(x);
     let wrap = (delta >= 0) as i8 - (y >= x) as i8;
     (delta, wrap)
 }
 
-/// Phase unwrapper.
+/// Overflow unwrapper.
+///
+/// This is unwrapping as in the phase unwrapping context, not unwrapping as
+/// in the `Result`/`Option` context.
 #[derive(Copy, Clone, Default, Deserialize, Serialize)]
 pub struct Unwrapper {
     // last input
@@ -27,19 +29,18 @@ pub struct Unwrapper {
 }
 
 impl Unwrapper {
-    /// Unwrap a new sample from a phase sequence and update the
-    /// unwrapper state.
+    /// Unwrap a new sample from a sequence and update the unwrapper state.
     ///
     /// Args:
-    /// * `x`: New phase sample
+    /// * `x`: New sample
     ///
     /// Returns:
-    /// A tuple containing the (wrapped) phase difference
-    /// and the signed number of phase wraps corresponding to the new sample.
+    /// A tuple containing the (wrapped) difference `x - x_old` and the signed
+    /// number of wraps accumulated by `x`.
     pub fn update(&mut self, x: i32) -> (i32, i32) {
-        let (dx, v) = get_wrap(self.x, x);
+        let (dx, v) = overflowing_sub(x, self.x);
         self.x = x;
-        self.v = self.v.wrapping_add(v as i32);
+        self.v = self.v.saturating_add(v as i32);
         (dx, self.v)
     }
 }
@@ -51,23 +52,32 @@ mod tests {
     fn mini() {
         for (x0, x1, v) in [
             (0i32, 0i32, 0i8),
-            (1, 1, 0),
-            (-1, -1, 0),
-            (1, -1, 0),
-            (-1, 1, 0),
+            (0, 1, 0),
+            (0, -1, 0),
+            (1, 0, 0),
+            (-1, 0, 0),
             (0, 0x7fff_ffff, 0),
             (-1, 0x7fff_ffff, -1),
+            (-2, 0x7fff_ffff, -1),
+            (-1, -0x8000_0000, 0),
             (0, -0x8000_0000, 0),
             (1, -0x8000_0000, 1),
             (-0x6000_0000, 0x6000_0000, -1),
             (0x6000_0000, -0x6000_0000, 1),
-            (0x6000_0000, -0x6000_0000, 1),
-            (0x6000_0000, -0x6000_0000, 1),
+            (-0x4000_0000, 0x3fff_ffff, 0),
+            (-0x4000_0000, 0x4000_0000, -1),
+            (-0x4000_0000, 0x4000_0001, -1),
+            (0x4000_0000, -0x3fff_ffff, 0),
+            (0x4000_0000, -0x4000_0000, 0),
+            (0x4000_0000, -0x4000_0001, 1),
         ]
         .iter()
         {
-            let (_dx, w) = get_wrap(*x0, *x1);
-            assert_eq!(*v, w, " = get_wrap({:#x}, {:#x})", *x0, *x1);
+            let (dx, w) = overflowing_sub(*x1, *x0);
+            assert_eq!(*v, w, " = overflowing_sub({:#x}, {:#x})", *x0, *x1);
+            let (dx0, w0) = x1.overflowing_sub(*x0);
+            assert_eq!(w0, w != 0);
+            assert_eq!(dx, dx0);
         }
     }
 }