From 0612abbddb7f5eace214173e73d21277f38fc5b9 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Sat, 1 Aug 2026 19:34:08 -0400 Subject: [PATCH] =?UTF-8?q?=CE=93-W1-T2=20review:=20one=20restart=20funnel?= =?UTF-8?q?,=20tighter=20ceiling=20proof,=20effective-gain=20meter?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fold setLimiterEnabled's restart request into setInstrumentParams so every writer keeps the host's latency report in sync. Pin the window-sizing identity, drop the per-sample modulo, tighten the ceiling tolerance, publish the blended gain. --- src/core/instrument/engine/limiter.cpp | 19 ++++++++--- src/core/instrument/engine/limiter.h | 11 ++++-- src/shell/instrument/processor_state.cpp | 29 +++++++++------- src/shell/instrument/reasampler_processor.h | 7 ++-- tests/test_limiter.cpp | 37 +++++++++++++++++++-- 5 files changed, 77 insertions(+), 26 deletions(-) diff --git a/src/core/instrument/engine/limiter.cpp b/src/core/instrument/engine/limiter.cpp index ea602bb..5837ca6 100644 --- a/src/core/instrument/engine/limiter.cpp +++ b/src/core/instrument/engine/limiter.cpp @@ -114,18 +114,22 @@ float Limiter::detectTruePeak(float xl, float xr, bool stereo) { float Limiter::smoothGain(float target) { // Sliding minimum over `window_` via a monotonic wedge. Expiring the front BEFORE the push - // is what bounds the wedge to `window_` entries — pushing first can lap the ring. + // is what bounds the wedge to `window_` entries — pushing first can lap the ring. Wraps by + // compare-and-subtract, matching delayPos_/avgPos_: window_ is not a power of two, so `%` + // would not strength-reduce on this per-sample path. while (wedgeCount_ > 0 && wedgeIdx_[static_cast(wedgeHead_)] <= pushIndex_ - window_) { - wedgeHead_ = (wedgeHead_ + 1) % window_; + wedgeHead_ = (wedgeHead_ + 1 == window_) ? 0 : wedgeHead_ + 1; --wedgeCount_; } while (wedgeCount_ > 0) { - const int back = (wedgeHead_ + wedgeCount_ - 1) % window_; + const int backSum = wedgeHead_ + wedgeCount_ - 1; + const int back = (backSum >= window_) ? backSum - window_ : backSum; if (wedgeVal_[static_cast(back)] < target) break; --wedgeCount_; } - const int slot = (wedgeHead_ + wedgeCount_) % window_; + const int slotSum = wedgeHead_ + wedgeCount_; + const int slot = (slotSum >= window_) ? slotSum - window_ : slotSum; wedgeVal_[static_cast(slot)] = target; wedgeIdx_[static_cast(slot)] = pushIndex_; ++wedgeCount_; @@ -171,7 +175,6 @@ float Limiter::process(float* left, float* right, int frames) { const float peak = detectTruePeak(dryL, dryR, stereo); const float targetGain = peak > ceiling_ ? ceiling_ / peak : 1.f; const float gain = smoothGain(targetGain); - if (gain < blockMin) blockMin = gain; const std::size_t slot = static_cast(delayPos_); const float wetL = delayL_[slot] * gain; @@ -184,6 +187,12 @@ float Limiter::process(float* left, float* right, int frames) { // dry + (wet - dry) * 1.0f is not wet in floating point. At m <= 0 the buffer is left // untouched, which is the dry sample already in it. const float m = mix_; + // The reported minimum is the gain actually reaching the output, not the limiter's raw + // target — mid-crossfade only a fraction `m` of the reduction is audible, so the meter + // (whose contract is "smallest gain APPLIED") must blend the same way the signal does: + // unity at m=0, `gain` at m=1, linear between. + const float effectiveGain = 1.f - m + m * gain; + if (effectiveGain < blockMin) blockMin = effectiveGain; if (m >= 1.f) { left[i] = wetL; if (stereo) right[i] = wetR; diff --git a/src/core/instrument/engine/limiter.h b/src/core/instrument/engine/limiter.h index 600c666..346907d 100644 --- a/src/core/instrument/engine/limiter.h +++ b/src/core/instrument/engine/limiter.h @@ -14,7 +14,9 @@ namespace reasampler::instrument::engine { // The BAKED ceiling. A safety device with no configurable controls, so this is not a // parameter. dBTP is a TRUE-peak target, which is why the detector oversamples and the -// signal path never does. +// signal path never does — though the bound is on the detector's 4x-oversampled ESTIMATE, +// not infinite-resolution true peak (normal for any practical TP limiter, and part of why +// this ceiling sits at -0.3 rather than 0). inline constexpr double kLimiterCeilingDbTp = -0.3; // The total delay the limiter imposes while engaged, and therefore the plugin's whole reported @@ -67,8 +69,11 @@ public: bool enabled() const { return target_.load(std::memory_order_relaxed); } // Applies the limiter in place over `frames` of `left` (and `right`, which may be null for - // a mono buffer). Returns the SMALLEST gain applied in this block — 1.0 for none, and the - // value a settled bypass returns. + // a mono buffer). Returns the SMALLEST gain actually applied to the output this block — 1.0 + // for none (a settled bypass, or wherever the engage/disengage crossfade sits at dry). Mid + // crossfade this is the target gain blended by the same fraction `mix_` blends the signal, + // not the limiter's raw target — the two must agree, or the meter over-reports reduction + // that is only partially audible. float process(float* left, float* right, int frames); private: diff --git a/src/shell/instrument/processor_state.cpp b/src/shell/instrument/processor_state.cpp index 95b369b..2df4c34 100644 --- a/src/shell/instrument/processor_state.cpp +++ b/src/shell/instrument/processor_state.cpp @@ -149,14 +149,25 @@ InstrumentParams ReaSamplerProcessor::instrumentParams() { } void ReaSamplerProcessor::setInstrumentParams(const InstrumentParams& params) { + bool limiterFlagChanged = false; { std::lock_guard lock(paramsMutex_); + limiterFlagChanged = (params_.limiterEnabled != params.limiterEnabled); params_ = params; } // Every writer of the parameter set — setState, the editor's commits, the bake's adopt — // funnels through here, so mirroring the limiter flag at this one point is what keeps the - // audio thread's copy and the latency report from ever lagging what is persisted. + // audio thread's copy and the latency report from ever lagging what is persisted, and + // requesting the restart here (not just from setLimiterEnabled) is what keeps the host's + // PDC from lagging it too. Coalesced: writing the value already held requests nothing. publishLimiterEnabled(params.limiterEnabled); + if (limiterFlagChanged && componentHandler) { + // The SDK requires this on the UI thread and answers getLatencySamples only after the + // host's own deactivate/reactivate — so the flag above is already committed by the time + // the host asks. This is a kLatencyChanged restart with the bus untouched, NOT the + // retired per-mode kIoChanged bus renegotiation (see initialize()); do not conflate. + componentHandler->restartComponent(kLatencyChanged); + } } void ReaSamplerProcessor::publishLimiterEnabled(bool on) { @@ -165,17 +176,11 @@ void ReaSamplerProcessor::publishLimiterEnabled(bool on) { } void ReaSamplerProcessor::setLimiterEnabled(bool on) { - { - std::lock_guard lock(paramsMutex_); - if (params_.limiterEnabled == on) return; // no change: no restart to request - params_.limiterEnabled = on; - } - publishLimiterEnabled(on); - // The SDK requires this on the UI thread and answers getLatencySamples only after the host's - // own deactivate/reactivate — so the flag above is already committed by the time the host - // asks. This is a kLatencyChanged restart with the bus untouched, NOT the retired per-mode - // kIoChanged bus renegotiation (see initialize()); do not conflate the two. - if (componentHandler) componentHandler->restartComponent(kLatencyChanged); + // Thin wrapper: setInstrumentParams is the one funnel that mirrors the flag AND requests + // the restart, so every writer of the parameter set — this one included — agrees. + InstrumentParams params = instrumentParams(); + params.limiterEnabled = on; + setInstrumentParams(params); } MasterBusMeter ReaSamplerProcessor::masterBusMeter() const { diff --git a/src/shell/instrument/reasampler_processor.h b/src/shell/instrument/reasampler_processor.h index e831b2f..c29ac2d 100644 --- a/src/shell/instrument/reasampler_processor.h +++ b/src/shell/instrument/reasampler_processor.h @@ -232,9 +232,10 @@ public: void setMasterGainLinear(double linear); // clamped to [0, masterGainMaxLinear()] // The master-bus limiter's single enable (persisted in the parameter set). UI thread only: - // the setter requests the host's kLatencyChanged restart, which the SDK requires be issued - // from the UI thread and which process() must therefore never trigger. Setting the value it - // already holds is a no-op, so repeated clicks on one segment cost no restart. + // a thin wrapper over setInstrumentParams, the one funnel that both mirrors the flag and + // requests the host's kLatencyChanged restart, which the SDK requires be issued from the UI + // thread and which process() must therefore never trigger. Setting the value it already + // holds is a no-op, so repeated clicks on one segment cost no restart. bool limiterEnabled() const { return limiterEnabled_.load(std::memory_order_relaxed); } diff --git a/tests/test_limiter.cpp b/tests/test_limiter.cpp index a35daf3..ed65cd3 100644 --- a/tests/test_limiter.cpp +++ b/tests/test_limiter.cpp @@ -108,8 +108,10 @@ static void testEngagedHoldsTheCeilingOnProgramTwelveDbOver() { for (std::size_t i = static_cast(latency); i < out.size(); ++i) { worst = std::max(worst, std::fabs(out[i])); } - // Sample peak, so the true-peak ceiling is the bound with room to spare for float rounding. - CHECK(worst <= ceiling * 1.0001f); + // Sample peak, so the true-peak ceiling is the bound with room to spare for float rounding + // (ceiling/peak then x*gain admits at most ~2.4e-7 relative overshoot; 1e-6 stays a hard + // bound without hiding a systematic error the way a much wider tolerance would). + CHECK(worst <= ceiling * (1.f + 1e-6f)); } static void testTruePeakDetectionEngagesWhereSamplePeakWouldNot() { @@ -161,7 +163,7 @@ static void testStereoLinkedGainKeepsDualMonoCenteredAcrossAToggle() { for (std::size_t i = l.size() / 2; i < (l.size() * 3) / 4; ++i) { worstEngaged = std::max(worstEngaged, std::fabs(l[i])); } - CHECK(worstEngaged <= ceiling * 1.0001f); + CHECK(worstEngaged <= ceiling * (1.f + 1e-6f)); CHECK(worstEngaged > 0.f); } @@ -253,6 +255,34 @@ static void testGainNeverRisesAboveUnity() { CHECK(outPeak <= inPeak); } +static void testAlignmentIdentityHoldsAtTheExactWindowEdge() { + // Pins the alignment identity window_ = latency_ - kLimiterOsDelay + 1 (limiter.h's + // comment on window_, otherwise asserted nowhere): a single isolated over-ceiling impulse + // is reduced to EXACTLY the ceiling at the one output sample the identity predicts + // (impulseAt + latency), because that is the unique push index where the sliding + // min-then-average has folded in nothing but this impulse's own detected peak. Shifting + // the identity by +-1 either lets the impulse's own excess slip just outside the window + // (undershoots the reduction, sample overshoots the ceiling) or applies the full reduction + // one sample late (same overshoot at this index) — confirmed by hand-mutating window_'s + // formula in both directions and observing this assertion fail before restoring it. + Limiter lim; + lim.setEnabled(true); + lim.prepare(kRate); + const int latency = limiterLookaheadSamples(kRate); + const float ceiling = static_cast(limiterCeilingLinear()); + const int impulseAt = 500; + std::vector in(static_cast(impulseAt + latency + 200), 0.f); + in[static_cast(impulseAt)] = ceiling * 4.f; // isolated, well over + float minGain = 0.f; + const std::vector out = runMono(lim, in, 37, &minGain); // odd block: crosses the edge + CHECK(minGain > 0.24f && minGain < 0.26f); // ceiling/peak == 0.25 for this impulse + const float atEdge = out[static_cast(impulseAt + latency)]; + CHECK(std::fabs(atEdge - ceiling) <= ceiling * 1e-6f); + // Every neighbor stays exactly silent — the reduction lands on this one sample, not smeared. + CHECK(out[static_cast(impulseAt + latency - 1)] == 0.f); + CHECK(out[static_cast(impulseAt + latency + 1)] == 0.f); +} + static void testBakedConstants() { CHECK(kLimiterCeilingDbTp == -0.3); CHECK(std::fabs(limiterCeilingLinear() - std::pow(10.0, -0.3 / 20.0)) < 1e-12); @@ -275,6 +305,7 @@ int main() { testToggleEmitsNoStepLargerThanTheSignalsOwn(); testCrossfadeSettlesToTheExactEngagedAndBypassedPaths(); testGainNeverRisesAboveUnity(); + testAlignmentIdentityHoldsAtTheExactWindowEdge(); testBakedConstants(); if (g_fail) { std::printf("%d FAILURE(S)\n", g_fail);