-
Notifications
You must be signed in to change notification settings - Fork 0
Mark I OPS recalibration — plan + divisor A/B harness (markI-ops-dac findings 1–3) #15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,142 @@ | ||
| // MarkIDivisorABTests.swift | ||
| // M2DX-Core — INVESTIGATION harness for the Mark I forward-modulation divisor | ||
| // recalibration (audit markI-ops-dac finding 2; see docs/mark-i-recalibration-plan.md). | ||
| // | ||
| // This is NOT a pass/fail test. It renders the two patches the Mark I divisor was | ||
| // originally calibrated against — ROM1A "BASS 1" (alg 15) and factory "E.PIANO 1" | ||
| // (alg 4, has feedback) — through the full production SynthEngine at several Mark I | ||
| // forward-mod divisors plus the Modern engine as the brightness reference, then: | ||
| // • prints raw peak + sustain spectral centroid (the brightness number), and | ||
| // • writes a PEAK-NORMALIZED (−3 dBFS) WAV per variant to /tmp/m2dx-marki-ab/ so an | ||
| // A/B listen compares TIMBRE, not loudness (loudness is a separate finding-1 axis, | ||
| // reported via the raw peak). | ||
| // | ||
| // Run: `M2DX_MARKI_AB=1 swift test --filter renderMarkIDivisorAB` (clean-room: no GPL Dexed). | ||
|
|
||
| import Testing | ||
| import Foundation | ||
| @testable import M2DXCore | ||
|
|
||
| @Suite("Mark I divisor A/B (investigation — writes WAVs, no assertions)") | ||
| struct MarkIDivisorABTests { | ||
|
|
||
| private let sampleRate: Float = 48000 | ||
| private let total = 144000 // 3.0 s at 48 kHz | ||
|
|
||
| /// Render a preset through the full SynthEngine at a chosen engine + Mark I divisor. | ||
| private func render(_ preset: DX7Preset, engine: FMEngine, divisor: Double?, | ||
| note: UInt8, vel: Int) -> [Float] { | ||
| let synth = SynthEngine() | ||
| synth.setSampleRate(sampleRate) | ||
| synth.setMasterVolume(1.0) | ||
| synth.setFMEngine(engine) | ||
| if let d = divisor { synth.setMarkIModDivisor(d) } | ||
| synth.loadDX7Preset(preset) | ||
|
|
||
| let blk = 512 | ||
| var wL = [Float](repeating: 0, count: blk), wR = [Float](repeating: 0, count: blk) | ||
| wL.withUnsafeMutableBufferPointer { lp in wR.withUnsafeMutableBufferPointer { rp in | ||
| synth.render(into: lp.baseAddress!, bufferR: rp.baseAddress!, frameCount: blk) | ||
| }} | ||
| synth.sendMIDI(MIDIEvent(kind: .noteOn, data1: note, data2: UInt32(vel) << 9)) | ||
|
|
||
| var out = [Float](repeating: 0, count: total) | ||
| var rr = [Float](repeating: 0, count: total) | ||
| out.withUnsafeMutableBufferPointer { op in rr.withUnsafeMutableBufferPointer { rp in | ||
| var off = 0 | ||
| while off < total { | ||
| let c = min(blk, total - off) | ||
| synth.render(into: op.baseAddress! + off, bufferR: rp.baseAddress! + off, frameCount: c) | ||
| off += c | ||
| } | ||
| }} | ||
| return out | ||
| } | ||
|
|
||
| /// Spectral centroid (Hz) over `range` via a coarse 40 Hz-grid DFT (lower = darker). | ||
| private func centroid(_ samples: [Float], over range: Range<Int>) -> Double { | ||
| let sr = Double(sampleRate) | ||
| let n = range.count | ||
| var numer = 0.0, denom = 0.0, f = 40.0 | ||
| while f < 10000.0 { | ||
| var re = 0.0, im = 0.0 | ||
| let w = 2.0 * Double.pi * f / sr | ||
| for i in range { | ||
| let s = Double(samples[i]); let p = w * Double(i - range.lowerBound) | ||
| re += s * cos(p); im -= s * sin(p) | ||
| } | ||
| let mag2 = (re * re + im * im) / Double(n * n) | ||
| numer += f * mag2; denom += mag2; f += 40.0 | ||
| } | ||
| return denom > 0 ? numer / denom : 0 | ||
| } | ||
|
|
||
| /// Minimal 16-bit mono PCM WAV writer (avoids a PresetLabKit test-target dependency). | ||
| private func writeWAV(_ samples: [Float], sampleRate: Int, to url: URL) throws { | ||
| var data = Data() | ||
| func le32(_ v: UInt32) { var x = v.littleEndian; withUnsafeBytes(of: &x) { data.append(contentsOf: $0) } } | ||
| func le16(_ v: UInt16) { var x = v.littleEndian; withUnsafeBytes(of: &x) { data.append(contentsOf: $0) } } | ||
| let dataSize = samples.count * 2 | ||
| data.append(contentsOf: Array("RIFF".utf8)); le32(UInt32(36 + dataSize)) | ||
| data.append(contentsOf: Array("WAVE".utf8)) | ||
| data.append(contentsOf: Array("fmt ".utf8)); le32(16); le16(1); le16(1) | ||
| le32(UInt32(sampleRate)); le32(UInt32(sampleRate * 2)); le16(2); le16(16) | ||
| data.append(contentsOf: Array("data".utf8)); le32(UInt32(dataSize)) | ||
| for s in samples { | ||
| let c = max(-1.0, min(1.0, s)) | ||
| le16(UInt16(bitPattern: Int16(c * 32767.0))) | ||
| } | ||
| try data.write(to: url) | ||
| } | ||
|
|
||
| @Test("Render Mark I divisor A/B WAVs (BASS 1 + E.PIANO 1)") | ||
| func renderMarkIDivisorAB() throws { | ||
| // Investigation harness: it mutates the process-global `markIModScaleQ12` (via | ||
| // setMarkIModDivisor), which races the Mark I parity suite under Swift Testing's parallel | ||
| // runner. Gate it behind an env var so a normal `swift test` is a no-op; render with: | ||
| // M2DX_MARKI_AB=1 swift test --filter renderMarkIDivisorAB | ||
| guard ProcessInfo.processInfo.environment["M2DX_MARKI_AB"] != nil else { | ||
| print("MarkIDivisorABTests skipped — set M2DX_MARKI_AB=1 to render the A/B WAVs") | ||
| return | ||
| } | ||
| defer { SynthEngine().setMarkIModDivisor(8.0) } // restore the global default (÷8) | ||
| let dir = URL(fileURLWithPath: "/tmp/m2dx-marki-ab") | ||
| try? FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) | ||
|
|
||
| let bass1 = DX7SysExParser.parse(data: bankFromVoice(rom1aBass1Packed), bankName: "ROM1A")!.presets[0] | ||
| let epiano1 = DX7FactoryPresets.all[11] // ROM1A E.PIANO 1 (alg 4, feedback) | ||
| let patches: [(name: String, preset: DX7Preset)] = [("bass1", bass1), ("epiano1", epiano1)] | ||
|
|
||
| // Modern = the brightness reference; Mark I at four divisors bracketing the | ||
| // audit-faithful ÷4 … the bit-parity ÷8. | ||
| let variants: [(name: String, engine: FMEngine, divisor: Double?)] = [ | ||
| ("modern", .modern, nil), | ||
| ("mki_div4", .markI, 4.0), | ||
| ("mki_div5", .markI, 5.0), | ||
| ("mki_div6", .markI, 6.0), | ||
| ("mki_div8", .markI, 8.0), | ||
| ] | ||
|
|
||
| let note: UInt8 = 60, vel = 100 | ||
| let sustain = 19200..<43200 // 0.4 .. 0.9 s, same window as MarkICalibrationTests | ||
| let targetPeak: Float = 0.707 // −3 dBFS after normalization | ||
|
|
||
| print("── Mark I divisor A/B (note 60, vel 100, peak-normalized to −3 dBFS) ──") | ||
| for (pname, preset) in patches { | ||
| for v in variants { | ||
| let raw = render(preset, engine: v.engine, divisor: v.divisor, note: note, vel: vel) | ||
| let peak = raw.map { abs($0) }.max() ?? 0 | ||
| let cent = centroid(raw, over: sustain) | ||
| let g = peak > 1e-9 ? targetPeak / peak : 1 | ||
| let norm = raw.map { $0 * g } | ||
| let url = dir.appendingPathComponent("\(pname)_\(v.name).wav") | ||
| try writeWAV(norm, sampleRate: Int(sampleRate), to: url) | ||
| let q12 = v.divisor != nil ? " Q12=\(Int((4096.0 / v.divisor!).rounded()))" : "" | ||
| let pp = pname.padding(toLength: 8, withPad: " ", startingAt: 0) | ||
| let vv = v.name.padding(toLength: 9, withPad: " ", startingAt: 0) | ||
| print(" \(pp) \(vv) centroid=\(Int(cent.rounded())) Hz rawPeak=\(String(format: "%.4f", peak))\(q12) → \(url.path)") | ||
| } | ||
| } | ||
| print("── A/B WAVs in \(dir.path) — listen Modern vs ÷4/÷5/÷6/÷8 ──") | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,124 @@ | ||
| # Mark I OPS recalibration — plan & A/B (markI-ops-dac findings 1–3) | ||
|
|
||
| Status: **DECIDED 2026-06-30 — finding 2 closed (÷5 maintained); findings 1 & 3 deferred.** This doc frames the Mark I forward-modulation | ||
| ("darkness") recalibration raised by the DX7 fidelity audit, with A/B render data, so the | ||
| final divisor can be chosen by ear against a real DX7 / Dexed reference. No code that ships | ||
| is changed yet. **Clean-room:** the project deliberately does not consult Dexed's GPL | ||
| `EngineMkI.cpp`; nothing here does either — the reference is your own ears + the audit's | ||
| reasoning + public DX7 OPS docs. | ||
|
|
||
| Parent: `dx7-fidelity-audit-2026-06-30.md` (`markI-ops-dac` dimension, findings 1–3). | ||
| A/B harness: `Tests/M2DXCoreTests/MarkIDivisorABTests.swift` → `M2DX_MARKI_AB=1 swift test --filter renderMarkIDivisorAB` (env-gated: it mutates the process-global divisor, so it is a no-op in a normal `swift test`). | ||
|
|
||
| ## Decision (2026-06-30) | ||
|
|
||
| **Finding 2 (forward-mod divisor) — CLOSED, ÷5 maintained.** An A/B by ear (BASS 1 + E.PIANO 1, | ||
| peak-normalized) found ÷4 / ÷5 / ÷6 / ÷8 **nearly indistinguishable**, matching the data (BASS 1 | ||
| spans only ~12%, E.PIANO 1 is divisor-insensitive at sustain). The divisor's "darkness" is a | ||
| low-impact calibration, so the shipping default stays **÷5** — changing it is not worth a | ||
| recalibration of the default engine. `MarkICalibrationTests.darkerSustain` is weakened from the | ||
| (pre-KLS-fix, over-bright-Modern) `÷0.6` threshold to assert only the surviving true property | ||
| **`Mark I < Modern`**; `MarkIDivisorABTests` is kept as the living A/B record. | ||
|
|
||
| **Findings 1 (4× / 12 dB hot output) & 3 (4× feedback) — DEFERRED.** These are the genuinely | ||
| audible Mark I issues (the A/B normalized loudness away, so they were not evaluated): the 12 dB | ||
| engine-switch level jump / clip-and-Maximizer reliance, and over-hot feedback. They are separate, | ||
| more structural, and tracked in the audit (`markI-ops-dac` findings 1 & 3) for a future, measured pass. | ||
|
|
||
| ## Why this is open | ||
|
|
||
| The audit found three **shared Swift+C, parity-invisible** issues in the Mark I (OPS) path, | ||
| all about *calibration*, not structure (the OPS pipeline is bit-exact to the `dx7refmki` twin): | ||
|
|
||
| 1. **(finding 1, HIGH)** Mark I carrier output is `result << 13` ≈ 2²⁶ — **4× (12 dB) hotter** | ||
| than Modern's 2²⁴ through the same `/2²⁸` normalization. | ||
| 2. **(finding 2, HIGH)** The forward-mod divisor (`markIModScaleQ12`, default ÷8 = parity; | ||
| app default ÷5) makes Mark I **darker than a real DX7** — the audit argues the faithful | ||
| value is **÷4** (mod index = Modern ≈ 2π, since DEXED level-matches its two engines). | ||
| 3. **(finding 3, MEDIUM)** Feedback re-injects the 4×-hot signal through the same shift as | ||
| Modern → **4× too strong** for every feedback algorithm except the +2-compensated 4/6/32. | ||
|
|
||
| These three are **entangled**: the divisor (2) sets brightness, but the `<<13` output scale (1) | ||
| sets loudness *and* the feedback strength (3). Changing the divisor alone leaves the 12 dB | ||
| loudness step and the hot feedback. So this is a **coordinated recalibration**, not a one-liner. | ||
|
|
||
| It also entangles with the **KLS fidelity fix (v1.16.0)**: that fix corrected an inverted/under- | ||
| scaled keyboard level scaling that had been over-driving a BASS 1 modulator, so Modern BASS 1 is | ||
| now **correctly darker** (sustain centroid 685 → 180 Hz). The old `MarkICalibrationTests` | ||
| `darkerSustain` assertion (`Mark I < Modern × 0.6`) was tuned to the *old over-bright* Modern and | ||
| no longer holds; it is currently `withKnownIssue`, to be resolved **together with** this decision. | ||
|
|
||
| ## Current state (code) | ||
|
|
||
| | Knob | Where | Value | | ||
| |---|---|---| | ||
| | `markIModScaleQ12` | `DX7MarkI.swift:68` | **512 = ÷8** (bit-parity default; matches `dx7refmki`) | | ||
| | app `markIDivisor` | `M2DXAudioEngine.swift` (`setMarkIModDivisor`) | **÷5** default, range **2–16**, Pro "Mark I Depth" slider | | ||
| | carrier output | `DX7MarkI.swift` `mkiSin` | `result << 13` (≈2²⁶, 4× Modern) — *unchanged so far* | | ||
| | feedback | `DX7MarkI.swift` / `DX7Voice` | unscaled `(y0+y)>>(fbShift+1)`, +2 only on alg 4/6/32 — *unchanged* | | ||
| | default engine | `M2DXAudioEngine.swift:143` | **`.markI`** (every user hears this) | | ||
|
|
||
| ## A/B render data | ||
|
|
||
| `note 60, vel 100, peak-normalized to −3 dBFS so you compare TIMBRE not loudness.` Sustain-window | ||
| spectral centroid (lower = darker); raw peak is the un-normalized loudness (finding-1 axis). | ||
|
|
||
| | patch | Modern | ÷4 | ÷5 (app) | ÷6 | ÷8 (parity) | | ||
| |---|---|---|---|---|---| | ||
| | **BASS 1** (alg 15) centroid | 180 Hz | **160** | 152 | 148 | **142** | | ||
| | BASS 1 raw peak | 0.0871 | 0.0848 | 0.0824 | 0.0802 | 0.0773 | | ||
| | **E.PIANO 1** (alg 4) centroid | 280 Hz | 280 | 280 | 280 | 280 | | ||
| | E.PIANO 1 raw peak | 0.0993 | 0.0957 | 0.0957 | 0.0957 | 0.0957 | | ||
|
|
||
| WAVs: `/tmp/m2dx-marki-ab/{bass1,epiano1}_{modern,mki_div4,mki_div5,mki_div6,mki_div8}.wav`. | ||
|
|
||
| **What the data says** | ||
| - The divisor's brightness effect is **patch- and time-dependent.** On BASS 1 (modulation sustains) | ||
| it spans 142→160 Hz across ÷8→÷4 — a real but **compressed ~12%** range. On E.PIANO 1 the | ||
| modulators have decayed by the sustain window, so the divisor barely moves the *sustain* centroid | ||
| (its effect is in the **attack** transient instead) — **judge E.PIANO by ear over the whole note.** | ||
| - Even at the audit-"faithful" ÷4, BASS 1 Mark I (160 Hz) stays **darker than Modern** (180 Hz). | ||
| So ÷4 does **not** make Mark I identical to Modern — the OPS log-sin/exp quantization is inherently | ||
| a touch darker. The audit's "mod index = Modern" is approximate, not exact, in practice. | ||
| - The raw-peak column shows the divisor barely changes loudness (0.085→0.077). The 4×-hot **12 dB** | ||
| step (finding 1) is the `<<13` output scale vs Modern, a **separate** axis the A/B normalizes away. | ||
|
|
||
| ## Decisions to make | ||
|
|
||
| 1. **Forward-mod divisor** (the brightness knob): **÷4** (audit-faithful, brightest, closest to | ||
| Modern) · **÷5** (today's app default) · **÷6** · keep **÷8** (parity, darkest). The audible | ||
| spread is modest; pick by ear from the WAVs / on device. | ||
| 2. **Finding 1 (4× hot output):** fix `mkiSin` `<<13 → <<11` (→2²⁴, level-matched to Modern) and | ||
| re-derive the divisor accordingly? This removes the 12 dB engine-switch jump and clipping/Maximizer | ||
| reliance, but **changes Mark I loudness** and must be done *with* the divisor (the A/B already | ||
| isolates timbre, so the chosen ÷ stays valid after a pure output re-scale). | ||
| 3. **Finding 3 (4× hot feedback):** scale the feedback term like Modern and drop the alg-4/6/32 +2 | ||
| band-aid? Best done after (1) so one shift law serves both engines. | ||
| 4. **`darkerSustain` test:** with corrected KLS + any of the above, "Mark I markedly darker than | ||
| Modern (÷0.6)" is the wrong assertion. Options: (a) re-tune to the chosen ÷'s measured ratio, | ||
| (b) weaken to "Mark I ≤ Modern" (still true at every ÷ for BASS 1), or (c) retire it and keep the | ||
| A/B harness as the calibration record. Decide once ÷ is fixed. | ||
|
|
||
| ## Recommendation | ||
|
|
||
| - **Divisor:** **÷5 maintained** (see the Decision at the top) — the A/B found ÷4…÷8 nearly | ||
| indistinguishable by ear, so the brightness knob is a low-impact calibration not worth changing | ||
| the shipping default. The bracketing data + WAVs remain for any future re-evaluation. | ||
| - **Do the coordinated fix, not just the divisor:** also `<<13→<<11` (finding 1) + feedback scaling | ||
| (finding 3), so loudness, brightness, and feedback are all faithful and the engine-switch level jump | ||
| goes away. Treat as one reviewed, RT-careful change with a fresh on-device A/B. | ||
| - **Test:** weaken `darkerSustain` to "Mark I ≤ Modern" (b) and keep `MarkIDivisorABTests` as the | ||
| living calibration record. | ||
|
|
||
| ## Implementation plan (once ÷ + scope are chosen) | ||
|
|
||
| 1. `DX7MarkI.swift`: set `markIModScaleQ12` default to the chosen value (÷4 → 1024); if doing finding 1, | ||
| change `mkiSin` `<<13 → <<11` and re-derive the divisor; mirror both in `dx7refmki.c`. Keep the app's | ||
| `setMarkIModDivisor` range/UI (recenter on the new default). | ||
| 2. Finding 3: scale the feedback term (`DX7MarkI.swift` `computeFb*MkI`) consistently; remove the | ||
| alg-4/6/32 +2 special-case if (1) makes it unnecessary. | ||
| 3. Tests: re-baseline `MarkICalibrationTests` (`darkerSustain` per the chosen option; `headroom` peak | ||
| threshold if loudness changed); the `dx7refmki` parity tests must stay green (Swift == C twin). | ||
| 4. App: bump `M2DXAudioEngine` default divisor + UI range; new M2DX-Core release + pin bump; on-device A/B. | ||
|
|
||
| Until then: v1.16.0 ships the KLS fix with Mark I unchanged (÷5), `darkerSustain` `withKnownIssue`. |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this A/B harness,
DX7FactoryPresets.allis built as[initVoice] + customPresets, andkeysBatchstarts withE.PIANO 1, so index11selectsTINE BELLrather thanE.PIANO 1. That means the generated WAVs and the calibration data documented as E.PIANO 1 are for the wrong patch, which can lead the divisor decision to be based on an unrelated bell preset; select the named preset or the correct index instead.Useful? React with 👍 / 👎.