Γ-W1-T2 review: one restart funnel, tighter ceiling proof, effective-gain meter

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

@@ -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<std::size_t>(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<std::size_t>(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<std::size_t>(slot)] = target;
     wedgeIdx_[static_cast<std::size_t>(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<std::size_t>(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;

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:

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<std::mutex> 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<std::mutex> 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 {

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);
     }

tests/test_limiter.cpp

@@ -108,8 +108,10 @@ static void testEngagedHoldsTheCeilingOnProgramTwelveDbOver() {
     for (std::size_t i = static_cast<std::size_t>(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<float>(limiterCeilingLinear());
+    const int impulseAt = 500;
+    std::vector<float> in(static_cast<std::size_t>(impulseAt + latency + 200), 0.f);
+    in[static_cast<std::size_t>(impulseAt)] = ceiling * 4.f;  // isolated, well over
+    float minGain = 0.f;
+    const std::vector<float> 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<std::size_t>(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<std::size_t>(impulseAt + latency - 1)] == 0.f);
+    CHECK(out[static_cast<std::size_t>(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);