From e12db3795aba8643430f51b4c1fb720c1a57f955 Mon Sep 17 00:00:00 2001
From: Stefano D'Angelo <s@dangelo.audio>
Date: Tue, 5 Sep 2023 09:46:09 +0200
Subject: [PATCH] change bp polarity + limit prewarp in bw_svf + cosmetics

---
 TODO             |  3 ---
 include/bw_lp1.h |  3 ++-
 include/bw_mm2.h |  2 +-
 include/bw_svf.h | 22 +++++++++++++++-------
 4 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/TODO b/TODO
index 18248f8..5a2c205 100644
--- a/TODO
+++ b/TODO
@@ -11,17 +11,14 @@ code:
 * mem req -> return value of set sample rate? or sample rate argument of mem req?
 * c++ get coeffs/state? or public? src nIn/OutSamples case (array vs single value), delay read/write, process1? process single?
 * common smoothing policy (as control rate as possible?) - smoothing control?
-* check unititialized warnings (check fxpp_comp in particular vs fxpp_satur)
 * IMPLEMENTATION prepocessor def???
  semi-specific:
 * osc post filter (and one pole init, slew rate, etc.) val from input? set state instead?
 * audio rate optional pulse width/slope inputs?
 * max_delay -> set sample rate? see reverb
  specific:
-* svf bandpass out polarity too confusing (inverted in mm2)?
 * bw_comb: should also modulate feedback?
 * bw_comb: integer target delay values?
-* bw_svf/lp1 automatically limited (max) prewarp frequency to avoid instability?
 * prewarp control in all derived filtering modules
 * src inside distortions? w/ control from outside?
 * peak gain + Q ???
diff --git a/include/bw_lp1.h b/include/bw_lp1.h
index d52ad4e..5cc1388 100644
--- a/include/bw_lp1.h
+++ b/include/bw_lp1.h
@@ -32,6 +32,7 @@
  *    <ul>
  *      <li>Version <strong>1.0.0</strong>:
  *        <ul>
+ *          <li>Limited actual prewarping frequency to prevent instability.</li>
  *          <li>Added <code>bw_lp1_reset_state_multi()</code> and updated C++
  *              API in this regard.</li>
  *          <li>Now <code>bw_lp1_reset_state()</code> returns the initial output
@@ -380,7 +381,7 @@ static inline void bw_lp1_do_update_coeffs(
 	if (prewarp_freq_changed || cutoff_changed) {
 		if (prewarp_freq_changed) {
 			prewarp_freq_cur = bw_one_pole_process1_sticky_rel(&coeffs->smooth_coeffs, &coeffs->smooth_prewarp_freq_state, prewarp_freq);
-			coeffs->t = bw_tanf(bw_minf(coeffs->t_k * prewarp_freq_cur, 1.569796326794897f));
+			coeffs->t = bw_tanf(bw_minf(coeffs->t_k * prewarp_freq_cur, 1.567654734141306f)); // max = 0.499 * fs
 		}
 		if (cutoff_changed) {
 			cutoff_cur = bw_one_pole_process1_sticky_rel(&coeffs->smooth_coeffs, &coeffs->smooth_cutoff_state, coeffs->cutoff);
diff --git a/include/bw_mm2.h b/include/bw_mm2.h
index 09b2800..b7ac032 100644
--- a/include/bw_mm2.h
+++ b/include/bw_mm2.h
@@ -352,7 +352,7 @@ static inline void bw_mm2_set_coeff_lp(bw_mm2_coeffs *BW_RESTRICT coeffs, float
 }
 
 static inline void bw_mm2_set_coeff_bp(bw_mm2_coeffs *BW_RESTRICT coeffs, float value) {
-	bw_gain_set_gain_lin(&coeffs->gain_bp_coeffs, -value);
+	bw_gain_set_gain_lin(&coeffs->gain_bp_coeffs, value);
 }
 
 static inline void bw_mm2_set_coeff_hp(bw_mm2_coeffs *BW_RESTRICT coeffs, float value) {
diff --git a/include/bw_svf.h b/include/bw_svf.h
index 54f8d28..28e8b9a 100644
--- a/include/bw_svf.h
+++ b/include/bw_svf.h
@@ -37,6 +37,9 @@
  *    <ul>
  *      <li>Version <strong>1.0.0</strong>:
  *        <ul>
+ *          <li>Changed model to get positive polarity at the bandpass
+ *              output.</li>
+ *          <li>Limited actual prewarping frequency to prevent instability.</li>
  *          <li>Added <code>bw_svf_reset_state_multi()</code> and updated C++
  *              API in this regard.</li>
  *          <li>Now <code>bw_svf_reset_state()</code> returns the initial output
@@ -364,6 +367,7 @@ struct bw_svf_coeffs {
 	
 	// Coefficients
 	float				t_k;
+	float				prewarp_freq_max;
 
 	float				t;
 	float				kf;
@@ -424,6 +428,7 @@ static inline void bw_svf_set_sample_rate(
 	bw_one_pole_set_sample_rate(&coeffs->smooth_coeffs, sample_rate);
 	bw_one_pole_reset_coeffs(&coeffs->smooth_coeffs);
 	coeffs->t_k = 3.141592653589793f / sample_rate;
+	coeffs->prewarp_freq_max = 0.499f * sample_rate;
 
 #ifdef BW_DEBUG_DEEP
 	coeffs->state = bw_svf_coeffs_state_set_sample_rate;
@@ -448,8 +453,9 @@ static inline void bw_svf_do_update_coeffs(
 				cutoff_cur = bw_one_pole_process1_sticky_rel(&coeffs->smooth_coeffs, &coeffs->smooth_cutoff_state, coeffs->cutoff);
 			if (prewarp_freq_changed) {
 				prewarp_freq_cur = bw_one_pole_process1_sticky_rel(&coeffs->smooth_coeffs, &coeffs->smooth_prewarp_freq_state, prewarp_freq);
-				coeffs->t = bw_tanf(coeffs->t_k * prewarp_freq_cur);
-				coeffs->kf = coeffs->t * bw_rcpf(prewarp_freq_cur);
+				const float f = bw_minf(prewarp_freq_cur, coeffs->prewarp_freq_max);
+				coeffs->t = bw_tanf(coeffs->t_k * f);
+				coeffs->kf = coeffs->t * bw_rcpf(f);
 			}
 			coeffs->kbl = coeffs->kf * cutoff_cur;
 		}
@@ -629,11 +635,11 @@ static inline void bw_svf_process1(
 	BW_ASSERT(y_hp != NULL);
 
 	const float kk = coeffs->kf * state->cutoff_z1;
-	const float lp_xz1 = state->lp_z1 - kk * state->bp_z1;
-	const float bp_xz1 = state->bp_z1 - kk * state->hp_z1;
-	*y_hp = coeffs->hp_x * (x + coeffs->hp_hb * bp_xz1 - lp_xz1);
-	*y_bp = bp_xz1 - coeffs->kbl * *y_hp;
-	*y_lp = lp_xz1 - coeffs->kbl * *y_bp;
+	const float lp_xz1 = state->lp_z1 + kk * state->bp_z1;
+	const float bp_xz1 = state->bp_z1 + kk * state->hp_z1;
+	*y_hp = coeffs->hp_x * (x - coeffs->hp_hb * bp_xz1 - lp_xz1);
+	*y_bp = bp_xz1 + coeffs->kbl * *y_hp;
+	*y_lp = lp_xz1 + coeffs->kbl * *y_bp;
 	state->hp_z1 = *y_hp;
 	state->lp_z1 = *y_lp;
 	state->bp_z1 = *y_bp;
@@ -938,6 +944,8 @@ static inline char bw_svf_coeffs_is_valid(
 	if (coeffs->state >= bw_svf_coeffs_state_set_sample_rate) {
 		if (!bw_is_finite(coeffs->t_k) || coeffs->t_k <= 0.f)
 			return 0;
+		if (!bw_is_finite(coeffs->prewarp_freq_max) || coeffs->prewarp_freq_max <= 0.f)
+			return 0;
 	}
 
 	if (coeffs->state >= bw_svf_coeffs_state_reset_coeffs) {