diff --git a/src/core/instrument/engine/filter/CLAUDE.md b/src/core/instrument/engine/filter/CLAUDE.md index ecec0f7..47b8835 100644 --- a/src/core/instrument/engine/filter/CLAUDE.md +++ b/src/core/instrument/engine/filter/CLAUDE.md @@ -3,7 +3,11 @@ ## Scope The pure 2-pole resonant low/high-pass a sounding voice runs. No REAPER, no VST3, no -allocation, no I/O. Four files, one responsibility each: +allocation, no I/O. Everything here lives in `reasampler::instrument::engine::filter`, +nested per the directory-mirrors-namespace convention — this keeps `FilterMode` and +friends out of `reasampler::instrument::engine` proper, where `zone_params.h` lives, since +this module has no call site yet to force the collision into the open at compile time. +Four files, one responsibility each: - `filter_params` — the control domain: `FilterMode`, normalized [0,1] knob position → cutoff Hz and Q, and the exact inverses. @@ -43,9 +47,10 @@ is 200 Hz and 2/3 is 2 kHz) and takes no sample rate. The persisted value is the normalized knob position, so a rate-derived endpoint would make one preset sound different at 44.1k and 96k. The Nyquist clamp (`kFilterNyquistFraction`, 0.48) is a property of the bilinear transform — `tan(pi*fc/sr)` diverges at Nyquist — so it lives in `biquadCoeffs` -where the rate is already a parameter. 20 kHz is under 0.48·sr at every supported rate, so -the clamp never eats live knob travel; the source's hardcoded 23 kHz endpoint did exactly -that at 44.1k. +where the rate is already a parameter. 20 kHz is under 0.48·sr at 44.1k and above, so the +clamp never eats live knob travel there; the source's hardcoded 23 kHz endpoint did exactly +that at 44.1k. Below 44.1k (e.g. 32k, 22.05k) the clamp still handles the math correctly — +it just legitimately eats the top of the knob travel at those rates. `biquadCoeffs` with a non-positive sample rate returns pass-through coefficients. It does **not** fall back to 44100 — that would breach the standing no-hardcoded-sample-rates diff --git a/src/core/instrument/engine/filter/filter_coeffs.cpp b/src/core/instrument/engine/filter/filter_coeffs.cpp index 2cd973d..8baa29c 100644 --- a/src/core/instrument/engine/filter/filter_coeffs.cpp +++ b/src/core/instrument/engine/filter/filter_coeffs.cpp @@ -2,7 +2,7 @@ #include -namespace reasampler::instrument::engine { +namespace reasampler::instrument::engine::filter { namespace { // M_PI is not standard C++ and is absent on MSVC without _USE_MATH_DEFINES. @@ -40,4 +40,4 @@ BiquadCoeffs biquadCoeffs(FilterMode mode, float cutoffHz, float q, double sampl return c; } -} // namespace reasampler::instrument::engine +} // namespace reasampler::instrument::engine::filter diff --git a/src/core/instrument/engine/filter/filter_coeffs.h b/src/core/instrument/engine/filter/filter_coeffs.h index 67699e2..c4d5a66 100644 --- a/src/core/instrument/engine/filter/filter_coeffs.h +++ b/src/core/instrument/engine/filter/filter_coeffs.h @@ -7,7 +7,7 @@ #include "core/instrument/engine/filter/filter_params.h" -namespace reasampler::instrument::engine { +namespace reasampler::instrument::engine::filter { // Already normalized by a0. The denominator is 1 + a1*z^-1 + a2*z^-2, so the difference // equation SUBTRACTS the a terms: y = b0*x + b1*x1 + b2*x2 - a1*y1 - a2*y2. @@ -28,4 +28,4 @@ inline constexpr double kFilterNyquistFraction = 0.48; // no-hardcoded-sample-rates ruling means we refuse to invent a rate rather than assume 44.1k. BiquadCoeffs biquadCoeffs(FilterMode mode, float cutoffHz, float q, double sampleRate); -} // namespace reasampler::instrument::engine +} // namespace reasampler::instrument::engine::filter diff --git a/src/core/instrument/engine/filter/filter_params.cpp b/src/core/instrument/engine/filter/filter_params.cpp index 7ac5bbf..ead4ca0 100644 --- a/src/core/instrument/engine/filter/filter_params.cpp +++ b/src/core/instrument/engine/filter/filter_params.cpp @@ -2,7 +2,7 @@ #include -namespace reasampler::instrument::engine { +namespace reasampler::instrument::engine::filter { namespace { double clamp01(double v) { return v < 0.0 ? 0.0 : (v > 1.0 ? 1.0 : v); } @@ -56,4 +56,4 @@ float filterNormFromQ(float q) { return static_cast(clamp01((-k.b + std::sqrt(d)) / (2.0 * k.c))); } -} // namespace reasampler::instrument::engine +} // namespace reasampler::instrument::engine::filter diff --git a/src/core/instrument/engine/filter/filter_params.h b/src/core/instrument/engine/filter/filter_params.h index 9b642c5..f5faf91 100644 --- a/src/core/instrument/engine/filter/filter_params.h +++ b/src/core/instrument/engine/filter/filter_params.h @@ -5,7 +5,7 @@ #pragma once -namespace reasampler::instrument::engine { +namespace reasampler::instrument::engine::filter { enum class FilterMode { LowPass, HighPass }; @@ -13,7 +13,8 @@ enum class FilterMode { LowPass, HighPass }; // norm 2/3 is 2 kHz. NOT derived from the sample rate — a rate-dependent endpoint would make // one saved preset sound different at 44.1k and 96k, and at 44.1k the top of the travel would // be dead against the Nyquist clamp (the ported firmware's 23 kHz endpoint had exactly that -// defect). 20 kHz sits under 0.48*sr at every rate we support, so the whole knob stays live. +// defect). 20 kHz sits under 0.48*sr at 44.1 kHz and above; below that (e.g. 32 kHz, 22.05 kHz) +// the clamp still handles it correctly, it just eats the top of the knob travel at those rates. inline constexpr float kFilterCutoffMinHz = 20.0f; inline constexpr float kFilterCutoffMaxHz = 20000.0f; @@ -36,4 +37,4 @@ float filterQFromNorm(float norm); // Exact inverse of filterQFromNorm; out-of-range Q clamps to 0 or 1. float filterNormFromQ(float q); -} // namespace reasampler::instrument::engine +} // namespace reasampler::instrument::engine::filter diff --git a/src/core/instrument/engine/filter/filter_saturate.h b/src/core/instrument/engine/filter/filter_saturate.h index 1a20da2..a6fd8b8 100644 --- a/src/core/instrument/engine/filter/filter_saturate.h +++ b/src/core/instrument/engine/filter/filter_saturate.h @@ -4,7 +4,7 @@ #pragma once -namespace reasampler::instrument::engine { +namespace reasampler::instrument::engine::filter { // Rational tanh approximation inside +/-threshold, continued past it with a gentle 0.1 slope // anchored at the threshold value so the curve stays continuous rather than hard-clipping. @@ -24,4 +24,4 @@ inline float tanhSaturate(float x, float threshold, float a, float b) { // path produces, not for audio-level signals — do not reuse it as a general waveshaper. inline float feedbackSaturate(float x) { return tanhSaturate(x, 2.0f, 27.0f, 9.0f); } -} // namespace reasampler::instrument::engine +} // namespace reasampler::instrument::engine::filter diff --git a/src/core/instrument/engine/filter/voice_filter.cpp b/src/core/instrument/engine/filter/voice_filter.cpp index d926973..d868e48 100644 --- a/src/core/instrument/engine/filter/voice_filter.cpp +++ b/src/core/instrument/engine/filter/voice_filter.cpp @@ -1,6 +1,6 @@ #include "core/instrument/engine/filter/voice_filter.h" -namespace reasampler::instrument::engine { +namespace reasampler::instrument::engine::filter { void VoiceFilter::prepare(const FilterSettings& settings, double sampleRate) { mode_ = settings.mode; @@ -26,4 +26,4 @@ bool VoiceFilter::isSilent() const { return true; } -} // namespace reasampler::instrument::engine +} // namespace reasampler::instrument::engine::filter diff --git a/src/core/instrument/engine/filter/voice_filter.h b/src/core/instrument/engine/filter/voice_filter.h index 5e200ef..a1aa307 100644 --- a/src/core/instrument/engine/filter/voice_filter.h +++ b/src/core/instrument/engine/filter/voice_filter.h @@ -11,7 +11,7 @@ #include "core/instrument/engine/filter/filter_params.h" #include "core/instrument/engine/filter/filter_saturate.h" -namespace reasampler::instrument::engine { +namespace reasampler::instrument::engine::filter { // Normalized control positions, as the editor moves them and the persisted state carries them. struct FilterSettings { @@ -23,6 +23,13 @@ struct FilterSettings { // Share of the last output fed back into the high-pass input at full resonance. Driven by the // raw control position rather than by Q: Q reaches 10, and scaling the feedback by it would // push the loop gain past unity at the top of the range. +// +// The HP/LP resonance asymmetry this produces is a known ear call reserved for Daniel, not a +// bug: measured peak/passband at res=1.0, fc=1kHz/sr=48k is LP 9.98 (flat at every input level) +// vs HP 7.44 (input 0.001-0.1), 7.59 (0.3), 8.52 (1.0) — HP resonance is level-dependent because +// feedbackSaturate's threshold (+/-2.0) is an absolute level, not a fraction of the signal. +// Retuning this constant alone cannot make the two modes track, since it does not touch that +// level-dependence. inline constexpr float kHighPassFeedbackShare = 0.24f; // Below this the recursion has decayed past -600 dB. Flushing keeps the history out of the @@ -56,7 +63,8 @@ public: // The high-pass numerator collapses toward zero as cutoff falls, taking the resonance // with it; feeding a saturated share of the last output back into the input restores - // the character the coefficients alone stop producing down there. + // the character the coefficients alone stop producing down there. The 0.9f pre-scale is + // carried from the source firmware, uncalibrated here — no derivation is known for it. const float in = (mode_ == FilterMode::HighPass) ? x - fbAmount_ * feedbackSaturate(s.fb * 0.9f) : x; @@ -69,15 +77,25 @@ public: s.y2 = s.y1; s.y1 = y; - // Snap the WHOLE state once the recursion as a whole has decayed past -600 dB. - // Zeroing individual samples instead does not work: a resonator swings through zero - // twice a cycle, so a per-sample flush injects a step in phase with the resonance, - // which the resonance then amplifies — the filter limit-cycles at the floor forever - // rather than going quiet. Testing y1 AND y2 tests the envelope, not one sample. + // Snap the RECURSIVE half of the state once it has decayed past -600 dB. Only y1/y2 + // are flushed (and only they are tested) — x1/x2 is an FIR tail that shifts out within + // two samples on its own, and a high-pass has an exact DC null (b1 == -2*b0 bit-exactly), + // so under a constant/DC-biased input y decays to zero while x1/x2 sit at the input + // level; clearing x1/x2 too would discard that history and the next sample would + // recompute a full-amplitude step from b0*in alone, re-ringing forever (a click train). + // Zeroing individual samples instead of the pair does not work either: a resonator + // swings through zero twice a cycle, so a per-sample flush injects a step in phase with + // the resonance, which the resonance then amplifies — the filter limit-cycles at the + // floor forever rather than going quiet. Testing y1 AND y2 tests the envelope, not one + // sample. if (s.y1 > -kFilterDenormalFloor && s.y1 < kFilterDenormalFloor && s.y2 > -kFilterDenormalFloor && s.y2 < kFilterDenormalFloor) { - s = State{}; + s.y1 = 0.0f; + s.y2 = 0.0f; } + // Stored unconditionally even in LP mode, where nothing reads it: the mode branch above + // already exists, but gating this one store on it buys nothing a dead-store-eliminating + // compiler doesn't already do for free, at the cost of a second branch on the mode. s.fb = s.y1; return y; } @@ -110,4 +128,4 @@ private: static_assert(!std::is_polymorphic_v, "no vtable on the per-sample path"); static_assert(std::is_trivially_copyable_v, "state is plain values, never owned"); -} // namespace reasampler::instrument::engine +} // namespace reasampler::instrument::engine::filter diff --git a/tests/test_filter.cpp b/tests/test_filter.cpp index 22e06c7..d5c9c18 100644 --- a/tests/test_filter.cpp +++ b/tests/test_filter.cpp @@ -13,7 +13,7 @@ #include #include -using namespace reasampler::instrument::engine; +using namespace reasampler::instrument::engine::filter; static int g_fail = 0; #define CHECK(cond) do { if(!(cond)) { \ @@ -41,9 +41,6 @@ static void testCutoffMapsThreeDecadesLogarithmically() { CHECK_NEAR(filterCutoffHzFromNorm(1.0f / 6.0f), 20.0 * std::sqrt(10.0), 1e-3); CHECK_NEAR(filterCutoffHzFromNorm(0.5f), 20.0 * std::sqrt(1000.0), 1e-2); - // A linear sweep would put the midpoint at 10010 Hz; the log sweep is nowhere near it. - CHECK(filterCutoffHzFromNorm(0.5f) < 1000.0f); - CHECK_NEAR(filterCutoffHzFromNorm(-1.0f), 20.0, 1e-3); CHECK_NEAR(filterCutoffHzFromNorm(2.0f), 20000.0, 1e-2); } @@ -351,6 +348,33 @@ static void testStateFlushesToZeroWithoutStallingInDenormals() { } } +// A high-pass has an exact DC null (b1 == -2*b0 bit-exactly), so under sustained DC the +// recursive y decays to zero while x1/x2 sit pinned at the DC level -- the case the zero-input +// test above cannot see, since there x1/x2 are zero anyway. A flush that clears x1/x2 along +// with y1/y2 discards that pinned history; the next sample then recomputes a full-amplitude +// step from b0*in alone, which re-rings and repeats forever (a click train). This must fail +// against a flush that also clears x1/x2. +static void testHighPassSustainedDCDoesNotReRing() { + const double sr = 48000.0; + VoiceFilter f; + f.prepare({FilterMode::HighPass, filterNormFromCutoffHz(1000.0f), 0.0f}, sr); + f.reset(); + + const int settle = 1000; + float worstAfterSettle = 0.0f; + for (int i = 0; i < 20000; ++i) { + const float y = f.process(0, 1.0f); + if (i >= settle) { + const float a = std::fabs(y); + if (a > worstAfterSettle) worstAfterSettle = a; + } + } + // A correct flush leaves the settled output pinned near zero. The click train this + // regresses against recurs every ~4760 samples at a magnitude around 0.6 -- nowhere near + // this tolerance. + CHECK(worstAfterSettle < 1e-3f); +} + // --------------------------------------------------------------------------- // Impulse / step sanity and saturation // --------------------------------------------------------------------------- @@ -380,11 +404,23 @@ static void testLowpassStepSettlesToUnity() { for (int i = 0; i < 48000; ++i) y = f.process(0, 1.0f); CHECK_NEAR(y, 1.0, 1e-3); // DC passes a lowpass at unity + // A DC step through a highpass should settle to (and STAY AT) zero. Sampling only the + // final value is not enough to prove that: a resonator swings through zero twice a cycle, + // so a single late sample can land near zero while the envelope is still ringing well + // above it elsewhere in the same run -- track the worst case over the settled region. VoiceFilter hp; hp.prepare({FilterMode::HighPass, filterNormFromCutoffHz(1000.0f), 0.0f}, sr); hp.reset(); - for (int i = 0; i < 48000; ++i) y = hp.process(0, 1.0f); - CHECK_NEAR(y, 0.0, 1e-3); // and is fully rejected by a highpass + const int settle = 200; + float worstAfterSettle = 0.0f; + for (int i = 0; i < 48000; ++i) { + y = hp.process(0, 1.0f); + if (i >= settle) { + const float a = std::fabs(y); + if (a > worstAfterSettle) worstAfterSettle = a; + } + } + CHECK(worstAfterSettle < 1e-3f); // fully rejected by a highpass, not just at one instant } static void testResetClearsHistoryButPrepareKeepsIt() { @@ -415,13 +451,14 @@ static void testChannelStateIsIndependent() { CHECK(frame[0] != frame[1]); } -static void testFeedbackSaturationIsContinuousAndBounded() { +static void testFeedbackSaturationIsContinuousWithGentleLinearTail() { CHECK_NEAR(feedbackSaturate(0.0f), 0.0, 1e-9); // Odd symmetry. CHECK_NEAR(feedbackSaturate(1.5f), -feedbackSaturate(-1.5f), 1e-6); // Continuous across the threshold at +/-2. CHECK_NEAR(feedbackSaturate(2.0f - 1e-4f), feedbackSaturate(2.0f + 1e-4f), 1e-4); - // Compresses hard: a 100x input does not give a 100x output. + // Past the threshold the curve continues on a 0.1 slope rather than hard-clipping -- it is + // NOT bounded, so this pins the linear continuation's shallow slope, not a ceiling. CHECK(std::fabs(feedbackSaturate(100.0f)) < 12.0f); CHECK(feedbackSaturate(100.0f) > feedbackSaturate(50.0f)); } @@ -438,11 +475,12 @@ int main() { testMeasuredResponsePeaksAtCutoffInBothModes(); testFullRangeCutoffSweepAtAudioRateStaysBounded(); testStateFlushesToZeroWithoutStallingInDenormals(); + testHighPassSustainedDCDoesNotReRing(); testImpulseResponseMatchesDifferenceEquation(); testLowpassStepSettlesToUnity(); testResetClearsHistoryButPrepareKeepsIt(); testChannelStateIsIndependent(); - testFeedbackSaturationIsContinuousAndBounded(); + testFeedbackSaturationIsContinuousWithGentleLinearTail(); if (g_fail == 0) std::printf("filter_tests: all passed\n"); else std::printf("filter_tests: %d FAILED\n", g_fail);