Files
reasampler/docs/product/audit-notes/q-w0-t3-env-constants.md
T

16 KiB
Raw Blame History

Q-W0 Track 3 — env-coupled-constant domain-modeling audit

Static analysis, 2026-07-28, branch pq-w0-audit. Scope: any value stored in an environment-coupled domain — frames, sample rate, DPI, pixels, tick cadence — that should be stored rate-free / device-free and resolved at the point of use (PLAN.md §Q-W0 env-coupled bullet; docs/product/code-organization.md §2c.3; the load-bearing sample_map seconds invariant). Findings are domain-modeling calls, not "rescale by rate" patches. The judgment bar applied: a finding requires (a) an env-coupled stored domain AND (b) an environment that can actually change under it. Frame counts computed transiently from seconds at the use site are correct and are not reported.

Waves referenced for disposition: Q-W1..Q-W6 open bank_panel.cpp, main.cpp, actions.cpp, persist.cpp, and relocate the clean pure libs — no downstream wave opens reasampler_processor.cpp / sampler_core.h / reasampler_editor.cpp for logic change, so fix-now findings in those files must be remediated in Q-W0 itself.


Findings

T3-01 — master-gain ramp step is a frame-domain constant anchored to 48 kHz

  • Location: src/vst/reasampler_processor.cpp:46-51 (kGainRampRate = 1.0f / 960.0f, kGainRampSnap), applied per-sample at :1069-1085 (stereo) and :1114-1126 (mono).
  • Stored vs. correct domain: stored as a per-sample linear step960 is literally 20 ms × 48 000 Hz, and the comment says so ("960 samples @ 48 kHz ≈ 20 ms"). The intended quantity is a wall-clock ramp time (~20 ms); the correct model is a seconds/ms constant with the per-sample step derived from sampleRate_ at setupProcessing — exactly the pattern the same file already uses two paragraphs away for kPreserveWindowMs (:618-623, :733-735).
  • What breaks when the environment shifts: the ramp's wall-clock length halves at 96 kHz (~10 ms) and quarters at 192 kHz (~5 ms); at 44.1 kHz it stretches to ~21.8 ms. The FB1 "no zipper" contract degrades silently as the host rate rises. Not persisted, so no on-disk breakage — but it is a live hardcoded-rate assumption in src/, which Daniel's standing ruling forbids.
  • Severity: Med.
  • Disposition: fix-now, remediated in Q-W0. Rationale: trivial, isolated, behavior-identical at 48 kHz; no downstream wave opens this file, so deferring means keeping a named violation of the no-hardcoded-rate ruling through the whole reorg. Store kGainRampSeconds = 0.020, derive the step from the live rate where sampleRate_ is set.

T3-02 — takeover-declick decay is a per-frame coefficient (documented deliberate)

  • Location: src/vst/sampler_core.h:409-410 (kDeclickDecay = 0.95, kDeclickFloor = 1e-4), applied per output frame in sampler_core.cpp:514-519, :643-645.
  • Stored vs. correct domain: a per-output-frame exponential coefficient; the implied wall-clock decay-to-floor is ~4.2 ms at 44.1 kHz and ~1.9 ms at 96 kHz. The strict domain-model form would be a time constant in seconds resolved to a coefficient at engine build.
  • What breaks when the environment shifts: the takeover-blend residue fades ~2× faster at 96 kHz. Audibly negligible for a declick micro-ramp — and the in-code comment (sampler_core.h:397-401) already documents this as a deliberate per-frame DSP micro-ramp, "not a stored wall-clock quantity", with the 44.196 kHz variance stated and accepted.
  • Severity: Low.
  • Disposition: document-and-defer. Rationale: the coupling is already an explicit, written, bounded design decision in the code; converting it buys no audible improvement. Triage should ratify the in-code note as the record.

T3-03 — Trigger-fade UI throw ceiling hardcodes 2 s × 44 100 as 88200.0 frames

  • Location: src/vst/reasampler_editor.cpp:395 (constexpr double kFadeMaxFrames = 88200.0), used by controlValue/the commit path to normalize the Trigger fade-in/out knobs.
  • Stored vs. correct domain: the fade storage domain (int64 SOURCE frames, persisted in the zones payload) is settled and correct — a source-timeline fact, invariant under project- rate change (PLAN.md §S15). The UI ceiling, however, encodes a wall-clock intent ("2-second max fade throw") as a frame count at an assumed 44.1 kHz source. 88200 is a rate-derived literal in src/, brushing the no-hardcoded-rate ruling even though it never touches disk.
  • What breaks when the environment shifts: the environment here is the source file's rate: a 96 kHz capture's maximum fade throw is ~0.92 s; a 22.05 kHz file gets 4 s. The knob's full-scale meaning silently varies per loaded sample.
  • Severity: Low (UI-only, not persisted, comment flags it as a "build-time residual — one place to retune").
  • Disposition: fix-now, remediated in Q-W0. Rationale: small and contained — replace with kFadeMaxSeconds = 2.0 resolved against the loaded source's rate at the two normalize sites (the editor already threads frameCount + rate through the pack/unpack path, envelope_edit.cpp:148); storage domain unchanged. If triage prefers zero UI-feel change, the fallback is document-and-defer with the comment amended to name the 44.1 k assumption.

T3-04 — drop-hint banner duration stored in sync-timer ticks

  • Location: src/vst/reasampler_editor.cpp:2920-2923 (dropHintTicks_ = 6), decayed in onSyncTimer (:244-245); field at reasampler_editor.h:457.
  • Stored vs. correct domain: a wall-clock intent ("a few seconds of banner") stored as a count of kSyncTimerIntervalMs ticks (6 × 500 ms). Correct model: a duration in ms, ticks derived — or a GetTickCount-style deadline like the bank panel's tooltip already uses.
  • What breaks when the environment shifts: retuning the sync cadence (a plausible perf tweak — the 500 ms value is itself a tuning constant) silently changes the banner duration. The comment does state the coupling.
  • Severity: Low.
  • Disposition: document-and-defer. Rationale: cosmetic, self-documenting at the single site, and the cadence and hint decay live three lines apart; a fix is fine to fold in opportunistically if the file is ever opened, but does not justify a Q-W0 edit on its own.

T3-05 — systemic: no DPI/content-scale support in either UI surface

  • Location: systemic. VST3 editor: reasampler_editor.cpp:150-153 (ViewRect(0,0,840,620) default and the size floor at :816), all editor_geometry / knob_deck / curve_popup / envelope_overlay px constants (e.g. the 8 px node min-separation, the 28×28 curve button), cached font sizes in draw_kit. Extension side: the LICE-drawn bank_panel dock and its geometry modules. No implementation of VST3's IPlugViewContentScaleSupport anywhere in src/vst/ (grep: zero hits for content-scale/DPI), no scale factor threaded through the pure geometry modules.
  • Stored vs. correct domain: every layout constant is a physical device pixel that silently assumes ~96 DPI. Correct model: logical units × one scale factor resolved at draw time (the pure geometry modules take widths/heights as parameters already, so a scale factor threads through cleanly — the constants are centralized, which is the good news).
  • What breaks when the environment shifts: on a 150200 % Windows display the editor and dock render physically small (or get bitmap-stretched by the host, blurring text); hit targets like the 8 px min node separation shrink below comfortable pointer accuracy. Usability, not correctness — nothing mis-plays and nothing persisted is wrong.
  • Severity: Med (usability on modern displays; Windows-only product makes high-DPI common).
  • Disposition: document-and-defer. Rationale: a proper UI-scaling pass is a feature wave of its own (scale plumbing through ~15 geometry modules + font cache + both shells), far outside Q-W0's remediation budget; deferral should be recorded as a named future phase, and Q-W1's relocation of the geometry modules should keep the constants centralized so the eventual scale factor lands in one place.

T3-06 — legacy v3 zone-payload lift divides by the current project rate

  • Location: src/vst/sample_map.cpp:601-605 (v3 lift inside readZonesPayload), format note at sample_map.h:439-456, :510-513.
  • Stored vs. correct domain: the v3 blobs (Daniel's beta projects) stored wall-clock times as frames — the prior incident itself. The lift converts frames → seconds by dividing by the live projectRate threaded in at read time. That is exact only if the project rate today equals the rate in effect when the S15/S16 editor wrote the frames; the write-era rate was never recorded, so a project whose rate changed since lifts skewed times (old/new ratio, e.g. ~8.8 % for 44.1→48 k).
  • What breaks when the environment shifts: already broken by construction for rate-changed-since-write projects; a one-time lift residue, after which v5+ re-saves in seconds and the skew is frozen in, silently.
  • Severity: Low (legacy-only, beta-project blobs, envelope-time magnitudes; unrecoverable in principle — the missing datum was never written).
  • Disposition: document-and-defer. Rationale: no better conversion exists; this is the documented residue of the incident that motivated the seconds invariant. Worth one sentence in the code-quality-audit report so the skew is a recorded known, not a mystery bug later.

T3-07 — SOLA correlation-segment cap of 512 frames (deliberate CPU bound; cross-ref T1)

  • Location: src/vst/pitch_shift.cpp:72 (corrFrames_ = max(1, min(dLow_ - 1, 512)); rationale comment at :64-67).
  • Stored vs. correct domain: borderline by design. The quantity being bounded is work per splice (multiply-accumulates), which is genuinely frame-domain — a CPU bound should be in frames. The side effect is that the correlation segment's wall-clock span halves at 96 kHz (512 frames ≈ 11.6 ms at 44.1 k, ≈ 5.3 ms at 96 k), raising the lowest frequency the alignment search can lock onto at high rates. All other shifter geometry correctly derives from kPreserveWindowMs resolved at the live rate.
  • What breaks when the environment shifts: alignment quality for low-frequency content degrades somewhat at high host rates; no correctness or persistence impact.
  • Severity: Low.
  • Disposition: document-and-defer, and hand to the T1 DSP audit for the quality call. Rationale: the frame domain is arguably correct for a compute bound; whether 512 is the right number is an algorithm-quality question (T1's territory), not a domain-modeling one.

Surfaces checked clean

  • sample_map v5+ persistence (the reference implementation): AHDSR + pitch-env times as SECONDS doubles; no rate constant anywhere in the read/write paths (kLegacyV3NominalRate deliberately does not exist); legacy v3 lift takes the rate as a parameter. Clean.
  • ComponentState envelope v6v11 fields: channel mode, assign generation, preview velocity, voice count/mode/trigger, masterGainLinear (dimensionless linear), explicit flag, SampleRefs (paths + root/loop/channels intrinsics), instanceGuid — all rate-free or file-fact domains. Clean.
  • Trigger fadeInFrames/fadeOutFrames/startPoint/SampleLoop.start/end persisted as int64 SOURCE frames: deliberate, settled source-timeline facts (PLAN.md §S15; bank_model.h:66-72 documents the loop rationale) — frames of the file are invariant under project-rate change; the file's own rate is stored alongside and resolved at decode. Correct domain, not a finding.
  • trigger_seam: frames↔fraction with startFrame threaded both directions; the overlay's fraction domain is expressly rate-invariant. Clean.
  • kPreserveWindowMs (50 ms): resolved to frames against the live host rate at both call sites (reasampler_processor.cpp:618-623, :733-735) — the correct pattern, cited here as the model T3-01 should copy.
  • pitch_shift internal geometry: ring length, fade, lag band, delay band all derived from the rate-resolved window_; ratio-scaled live fade length. Clean (T3-07 cap noted above).
  • Tail system: TailSetting.manualMs persisted in ms; kMaxTailSeconds/kMaxTailMs wall-clock; trim threshold in dB with the linear ratio derived (render_settings.h:59-86); the realtime decay scan resolves frames against the file's own authoritative rate (capture_realtime.cpp:384-408). Clean.
  • wav_trim: frame counts are parsed file facts and transient truncate plans. Clean.
  • bank_model persisted metadata: source bounds in seconds + PPQ (both stored, each for its consumer); loudness in dB; sampleRate/channelCount are recorded facts about the file, not assumptions; capture tempo + meter stamped at capture time deliberately so the bars.beats read-out is stable under later project meter changes (card_meta). Clean.
  • Ext-state wires (banks JSON, view-mode model, owned manifest, assignment_request, rsusage_*): no frame-domain values; the rate field in the assign wire is a recorded fact. Clean.
  • Envelope schematic (envelope_overlay/envelope_edit): param-domain px↔seconds scale derived from the live rect (gatePxPerSecond), sample-length-free; editor time-slider ceiling is kEnvTimeMaxSeconds = 2.0 (seconds). Clean (pixel constants themselves fall under the systemic T3-05).
  • Timers: bank_panel tooltip delay uses GetTickCount() ms against kTooltipDelayMs = 500 (wall-clock — the pattern T3-04 should copy); editor sync timer is a 500 ms SetTimer interval (ms, not ticks); the new-content detector is an event diff per tick with no wall-clock meaning encoded in tick counts; retireIdleDrain is idleness-driven, not time-driven. Clean.
  • peaks / waveform_view / master_gain / velocity_curve / keyboard_strip: bins and columns derived from rects at use; dB↔linear taper and curve math dimensionless; key rects from the passed strip rect. Clean.

Summary

ID Location Stored domain Severity Disposition
T3-01 reasampler_processor.cpp:46-51 gain-ramp step per-sample step (20 ms @ 48 k baked in) Med Fix-now (Q-W0) — store seconds, derive step from sampleRate_
T3-02 sampler_core.h:409-410 declick decay per-frame coefficient Low Document-and-defer — deliberate, already documented in-code
T3-03 reasampler_editor.cpp:395 fade throw ceiling 88200 source frames (2 s @ 44.1 k) Low Fix-now (Q-W0) — seconds ceiling resolved vs. source rate at use
T3-04 reasampler_editor.cpp:2923 drop-hint duration sync-timer ticks Low Document-and-defer — cosmetic, coupling stated in-code
T3-05 systemic (both UI surfaces) physical px, ~96 DPI assumed; no content-scale Med Document-and-defer — a UI-scaling phase of its own; keep geometry constants centralized through Q-W1
T3-06 sample_map.cpp:601-605 v3 legacy lift frames ÷ current project rate Low Document-and-defer — unrecoverable legacy residue; record as known skew
T3-07 pitch_shift.cpp:72 correlation cap 512 frames (CPU bound) Low Document-and-defer — frame domain arguably correct for a compute bound; hand to T1 for the quality call

Two fix-now findings (T3-01, T3-03), both assigned to Q-W0 itself — no downstream wave opens those files for logic change. Five deferrals, each with a recorded rationale. The persistence surfaces — the highest-stakes case — are clean: every wall-clock quantity written to disk since the S12 remediation is in seconds or ms, and every frame-domain persisted value is a source-file fact whose rate travels with it.