Preserve's period detection: probes are placed by position, and a sustain loop is the span analysed

src/core/instrument/CLAUDE.md

@@ -300,7 +300,12 @@ anything for a trigger shape.
   is DERIVED from the audio, so it is cache and not state: nothing persists it, and it takes no
   rung of the payload ladder. **Answering "none" is a first-class result** — noise, polyphony,
   percussion and a source whose period changes mid-sample all return it, and the shifter's
-  fixed-window geometry is the documented fallback.
+  fixed-window geometry is the documented fallback. **Detection analyses the SUSTAIN LOOP when
+  the capture carries one long enough to host the full search band** (`periodAnalysisSpan`),
+  otherwise the whole source: the loop is what a Gate voice asymptotically plays, and a phrase
+  whose head is pitched differently from its sustain would otherwise disagree its way to none.
+  A shorter loop analyses the whole source rather than a narrowed band — a narrower span may
+  never buy itself a higher lowest-findable fundamental.
 - `time_stretch` — the TIME half beside `pitch_shift`'s PITCH half, header-only: `StretchCursor`, the per-output-frame source-feed schedule (a fractional cursor carrying its rate debt, loop-wrapped), plus the rate bounds and their clamp. Rate 1.0 is exactly one source frame per output frame with no residue, which is what makes the unity Preserve read bit-identical to the pre-stretch engine. The bounds are **measured**, not arbitrary — see the header.
 - `velocity_curve` — THE monotone spline, shared by every consumer: the three velocity transfer curves and the three spline EGs. `VelocityCurve` is evaluated as ONE OR MORE Fritsch–Carlson monotone cubic Hermite splines joined at its HARD points — a hard knot is a sub-curve boundary for tangent purposes (exactly what the point array's own ends already are), so the two adjacent segments meet at their natural angle instead of a shared derivative and the no-overshoot guarantee holds PER SEGMENT rather than globally. Points are smooth by default; the ceiling is `kMaxCurvePoints` = 128, a MUSICAL bound (long rhythmic phrases, ~two points per articulation event) and not a performance one — **do not lower it**. `eval(velocity)` is the COLD reader, called once per note-on or once per drawn pixel column; `SplineCursor` is the RT one, an indexed segment search plus one Hermite evaluation with the segment and its tangents cached across samples. Both share the same `segmentTangents`/`hermiteAt` free functions, so there is one spline and not two. It carries its own y `CurveDomain`: UNIPOLAR [0,1] is the amp's GAIN, defaulting to `flat()` (y=1, every velocity→unity — a deliberate non-back-compat replacement of the old fixed `velocity/127` path, Daniel-approved); BIPOLAR [−1,1] is the signed modulation shape for pitch and filter, defaulting to `zero()` so velocity modulates neither until a curve is drawn. A bipolar curve does not imply the absence of a depth beside it: the filter keeps its `velAmount` knob and the two compose multiplicatively (`velAmount × curve.eval(v)`, `play_params.h`), while the pitch curve's throw is the fixed `kVelocityPitchRangeSemitones`.
 - `master_gain` — pure dB↔linear taper math (FB1): normalized [0,1] ↔ dB ↔ linear for the post-mixer master gain control (−∞…+24 dB, norm 0 = true silence, unity ≈ 0.714). Shared by the editor knob and the processor multiply so the needle, persisted value, and audio multiply cannot drift.

src/core/instrument/engine/period_detect.cpp

@@ -120,30 +120,34 @@ double blockRms(const std::vector<AudioSample>& pcm, std::size_t from, std::size
 
 }  // namespace
 
-PeriodEstimate detectPeriod(const std::vector<AudioSample>& pcm, int sampleRate) {
-    if (sampleRate <= 0 || pcm.empty()) return {};
+PeriodEstimate detectPeriod(const std::vector<AudioSample>& pcm, int sampleRate,
+                            std::size_t spanFrom, std::size_t spanCount) {
+    if (sampleRate <= 0 || spanCount == 0) return {};
+    if (spanFrom > pcm.size() || spanCount > pcm.size() - spanFrom) return {};
     const double rate = static_cast<double>(sampleRate);
     std::size_t lagHi = static_cast<std::size_t>(rate / kPeriodDetectMinHz);
     const std::size_t lagLo = static_cast<std::size_t>(rate / kPeriodDetectMaxHz);
     if (lagLo < 2) return {};  // a rate so low the whole search band collapses
 
     // One probe block is W + lagHi frames with W == lagHi (YIN's usual sizing: the analysis
-    // window must cover the longest lag being tested). A short sample shortens the search
+    // window must cover the longest lag being tested). A short span shortens the search
     // rather than refusing outright — a 200 ms one-shot still has a period worth finding.
-    if (pcm.size() < 2 * lagHi) lagHi = pcm.size() / 2;
+    if (spanCount < 2 * lagHi) lagHi = spanCount / 2;
     if (lagHi <= lagLo + 2) return {};
     const std::size_t block = 2 * lagHi;
-    const std::size_t probes =
-        std::min<std::size_t>(kPeriodDetectProbes, std::max<std::size_t>(1, pcm.size() / block));
+    // Probe POSITIONS, not disjoint blocks — see kPeriodDetectProbes in the header for why
+    // lagHi is the separation that makes two overlapping probes independent evidence.
+    const std::size_t room = spanCount - block;
+    const std::size_t probes = std::min<std::size_t>(kPeriodDetectProbes, 1 + room / lagHi);
     // Room to spare after the last probe's block is spread between them, so the probes sample
-    // the whole sample rather than only its opening.
-    const std::size_t stride = probes > 1 ? (pcm.size() - block) / (probes - 1) : 0;
+    // the whole span rather than only its opening.
+    const std::size_t stride = probes > 1 ? room / (probes - 1) : 0;
 
     std::vector<double> periods;
     std::vector<double> confidences;
     for (std::size_t p = 0; p < probes; ++p) {
-        const std::size_t from = p * stride;
-        if (from + block > pcm.size()) break;
+        const std::size_t from = spanFrom + p * stride;
+        if (from + block > spanFrom + spanCount) break;
         if (blockRms(pcm, from, block) < kSilenceRms) continue;
 
         const std::vector<double> small = decimate(pcm, from, block);
@@ -173,6 +177,22 @@ PeriodEstimate detectPeriod(const std::vector<AudioSample>& pcm, int sampleRate)
 
     if (periods.empty()) return {};
 
+    // ONE surviving probe: the span could not host a second probe position, so there is no
+    // second estimate for the majority rule below to rule on — it would be deciding on an
+    // empty comparison. The accept rests on pickPeriod's absolute threshold, which is a real
+    // test and not an absence of one: the block genuinely repeats at this lag across its whole
+    // analysis window. Refusing instead would deny every short one-shot a period, and a period
+    // that turns out wrong costs a mis-centred correlation search at the splice, not an
+    // unrefined one (pitch_shift.cpp's splice searches +/- maxLag around whichever jump it is
+    // handed). Do not "unify" this back into the majority test — at size 1 that test accepts
+    // unconditionally, which is the same behaviour with none of the reasoning.
+    if (periods.size() == 1) {
+        PeriodEstimate lone;
+        lone.frames = periods[0];
+        lone.confidence = confidences[0];
+        return lone;
+    }
+
     std::vector<double> sorted = periods;
     std::sort(sorted.begin(), sorted.end());
     const double median = sorted[sorted.size() / 2];
@@ -192,6 +212,7 @@ PeriodEstimate detectPeriod(const std::vector<AudioSample>& pcm, int sampleRate)
     // first half is one period and second half another gives two probes each way, and taking
     // either as "the" period would misalign every splice in the other half. Refusing is the
     // right answer there — the fixed-window fallback is what a source with no ONE period gets.
+    // Reached only with two or more probes; the lone-probe case returned above.
     if (agree * 2 <= periods.size()) return {};
 
     PeriodEstimate est;
@@ -200,4 +221,24 @@ PeriodEstimate detectPeriod(const std::vector<AudioSample>& pcm, int sampleRate)
     return est;
 }
 
+PeriodEstimate detectPeriod(const std::vector<AudioSample>& pcm, int sampleRate) {
+    return detectPeriod(pcm, sampleRate, 0, pcm.size());
+}
+
+AnalysisSpan periodAnalysisSpan(std::size_t frameCount, std::int64_t loopStart,
+                                std::int64_t loopEnd, bool hasLoop, int sampleRate) {
+    const AnalysisSpan whole{0, frameCount};
+    if (!hasLoop || sampleRate <= 0) return whole;
+    if (loopStart < 0 || loopEnd <= loopStart) return whole;
+    if (static_cast<std::uint64_t>(loopEnd) > frameCount) return whole;
+
+    const std::size_t length = static_cast<std::size_t>(loopEnd - loopStart);
+    // One full probe block. Below it detectPeriod shortens lagHi to fit, which raises the
+    // lowest findable fundamental — the one thing the narrower span may never cost.
+    const std::size_t minimum =
+        2 * static_cast<std::size_t>(static_cast<double>(sampleRate) / kPeriodDetectMinHz);
+    if (length < minimum) return whole;
+    return AnalysisSpan{static_cast<std::size_t>(loopStart), length};
+}
+
 }  // namespace reasampler::instrument::engine

src/core/instrument/engine/period_detect.h

@@ -6,6 +6,7 @@
 // translation unit on the render path can name detectPeriod. A sampler's source is fixed and
 // fully known at load, which is the whole reason a detector is affordable here at all.
 
+#include <cstddef>
 #include <cstdint>
 #include <vector>
 
@@ -19,9 +20,12 @@ using audio::AudioSample;
 // authored and never persisted — this is a cache, not state.
 struct PeriodEstimate {
     double frames = 0.0;      // 0 = no single period (inharmonic, polyphonic, percussive, noise)
-    // 1 - the accepted dissimilarity, [0,1]; 0 when frames == 0. Diagnostic only today: the
-    // accept decision is `valid()` alone, and the loader takes `.frames` without reading this —
-    // do not assume it is load-bearing without checking who reads it.
+    // 1 - the accepted dissimilarity, [0,1]; 0 when frames == 0. Diagnostic: the accept decision
+    // is `valid()` alone and the loader takes `.frames` without reading this — its consumers are
+    // the tests and the measurement harness. It is deliberately NOT a second accept gate: every
+    // accepted probe already cleared kPeriodDetectThreshold, so confidence > 0.88 holds by
+    // construction and any gate below that is a no-op while any gate above it is a tuned number
+    // with nothing to derive it from.
     double confidence = 0.0;
 
     bool valid() const { return frames > 0.0; }
@@ -43,15 +47,51 @@ inline constexpr double kPeriodDetectThreshold = 0.12;
 // How many blocks across the sample are estimated independently, and how far apart two of them
 // may land and still be called the same period. Agreement is what separates a genuinely
 // periodic source from one whose opening happens to look periodic.
+//
+// Probes are placed by POSITION and may overlap: what the rule needs is estimates from
+// different places in the source, and two blocks a full longest-lag apart already differ by a
+// whole cycle of the lowest frequency in the band, so neither can be a trivially shifted copy
+// of the other at any period searched. Requiring DISJOINT blocks instead left every source
+// under ~4x the longest lag with a single probe and so with no agreement to check at all.
+// One probe survives as an irreducible case below `block + longest lag` frames and is accepted
+// on the absolute threshold alone — see detectPeriod's contract.
 inline constexpr int kPeriodDetectProbes = 4;
 inline constexpr double kPeriodDetectAgreeTolerance = 0.02;  // 2% of the median
 
-// Estimates `pcm`'s fundamental period at `sampleRate`. Cost is bounded by the constants above,
-// not by the sample length: at most kPeriodDetectProbes blocks of ~2 x the longest searched lag
-// are analysed however long the source is. Allocates; never call from process().
+// Estimates the fundamental period of `pcm[from, from+count)` at `sampleRate`. Cost is bounded
+// by the constants above, not by the span length: at most kPeriodDetectProbes blocks of ~2 x
+// the longest searched lag are analysed however long the span is. Allocates; never call from
+// process(). An out-of-range span estimates nothing and returns none.
 //
 // Returns an invalid estimate (frames == 0) for silence, noise, and anything whose probes
 // disagree — the caller's documented fallback is the fixed-window splice geometry.
+//
+// Two probes or more must reach a STRICT MAJORITY agreement. A lone probe — which only happens
+// on a span too short to host a second probe position — is accepted on the absolute threshold
+// alone, because there is no second estimate for a majority rule to rule on and refusing would
+// deny every short one-shot a period.
+PeriodEstimate detectPeriod(const std::vector<AudioSample>& pcm, int sampleRate,
+                            std::size_t from, std::size_t count);
+
+// The whole source.
 PeriodEstimate detectPeriod(const std::vector<AudioSample>& pcm, int sampleRate);
 
+// The frames detection should analyse for a capture that carries a sustain loop, and the reason
+// the answer is not simply "all of them": under Gate the loop region is asymptotically ALL the
+// splicer plays, so a phrase whose head is pitched differently from its sustain would otherwise
+// disagree its way to none over the whole source. `[loopStart, loopEnd)` is used only when it
+// is at least `2 * (sampleRate / kPeriodDetectMinHz)` frames — the span below which detectPeriod
+// starts shortening its own search band — so choosing the narrower span never costs search-band
+// width and so can never lose a low fundamental that the whole source would have found.
+// Anything else (no loop, an out-of-range span, a short one) yields the whole source.
+//
+// The read path's loop-validity authority is loop_span's resolveLoop; the bounds check here is
+// on a cache input, not a second validity rule, and it refuses rather than repairs the same way.
+struct AnalysisSpan {
+    std::size_t from = 0;
+    std::size_t count = 0;
+};
+AnalysisSpan periodAnalysisSpan(std::size_t frameCount, std::int64_t loopStart,
+                                std::int64_t loopEnd, bool hasLoop, int sampleRate);
+
 }  // namespace reasampler::instrument::engine

src/core/instrument/map/sample_map.cpp

@@ -329,9 +329,14 @@ SampleData buildSampleData(const ResolvedCapture& resolved, DecodedPcm decoded)
     data.play = resolvePlay(resolved.play, data.sampleRate);
     // The one place Preserve's source period is computed: the load, off the audio thread.
     // Channel 0 only — a stereo pair's two channels share a fundamental, and the splice
-    // schedule is linked across them anyway.
+    // schedule is linked across them anyway. The span is the sustain loop where one is long
+    // enough (periodAnalysisSpan owns that rule) — every input to it commits through a full
+    // reload, so the cache is re-derived whenever the span it was chosen from moves.
+    const instrument::engine::AnalysisSpan span = instrument::engine::periodAnalysisSpan(
+        data.frames.size(), data.loop.start, data.loop.end, data.loop.hasLoop, data.sampleRate);
     data.sourcePeriodFrames =
-        instrument::engine::detectPeriod(data.frames, data.sampleRate).frames;
+        instrument::engine::detectPeriod(data.frames, data.sampleRate, span.from, span.count)
+            .frames;
     return data;
 }