testhost: prove the actual chunk-based state path via raw VST2 dispatcher

Add Vst2RawChunk, a raw effGetChunk/effSetChunk bypass around JUCE's
VSTPluginInstance::usesChunks() gate, plus --loadBankChunk/--saveBankChunk/
--loadPatchChunk/--savePatchChunk CLI flags and a new test_chunk_roundtrip
regression test asserting the saved file is genuinely zlib/XML (0x78 header,
>1000 bytes), not a per-parameter fallback.

Building and using this bypass disproved an earlier documented finding: the
compiled VST2 wrapper does set effFlagsProgramChunks and usesChunks() is
genuinely true, so the previous "chunk-advertisement gap" theory in TODO.md
and tests/README.md was wrong. Corrected in both places. The real cause of
old bundled .fxp sample files having no effect remains open and is now
narrowed to a JaySynth-side XML/schema parsing question, not a host/plugin
capability mismatch.

Also adds a PluginHost-method-to-VST2-wiring-to-test coverage table in
tests/README.md, and documents the remaining "exercised but not asserted"
gaps (channel/param counts, DSP correctness) in TODO.md.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011dhtwRLARk4eiPngcQykLJ
This commit is contained in:
2026-07-27 21:22:24 +02:00
co-authored by Claude Sonnet 5
parent 2a0ea726f3
commit 8194bda9c8
7 changed files with 363 additions and 58 deletions
+76 -28
View File
@@ -63,40 +63,88 @@ restructuring. Sets a continuous parameter (index 0 = `SYNTH_PARAM_VOLUME`)
to a distinctive value, saves, reloads in a fresh process, and checks the
value reads back within a small tolerance (not exact equality — the
slider/internal-value scaling curve, `toParam`/`toSlider`, isn't bit-exact
across a round trip). Uses `testhost`'s own save→load cycle as the fixture,
not an externally-provided `.fxp`/`.fxb` — see the chunk-format note below
for why.
across a round trip). Goes through JUCE's generic `AudioProcessor::get/
setStateInformation`/`get/setCurrentProgramStateInformation` - see the
corrected-finding note below for what that path actually does.
**`bank_chunk_roundtrip`** — same round-trip, but via `effGetChunk`/
`effSetChunk` called *directly* on the raw `AEffect*` (`Vst2RawChunk.h`/
`--saveBankChunk`/`--loadBankChunk`), bypassing JUCE's host-side wrapper
entirely. Also positively confirms the real chunk path was used (not just
that a parameter round-trips): checks the saved file starts with a zlib
stream header (`0x78`) and is far larger than a raw 140-parameter float
array could be (140×4 = 560 bytes) - a plain per-parameter fallback
wouldn't look anything like this.
**`stability_sweep`** — loads every bundled `.fxp` under `extras/sounds/`
and renders a held note through each, checking for crashes/hangs/NaN. A
broad regression net, not a targeted test for any one finding.
broad regression net, not a targeted test for any one finding. See the
corrected-finding note below for why this only checks "doesn't crash," not
"changes the patch."
## Two things discovered while building this suite that are worth knowing
## `PluginHost` method coverage table
**The VCF coefficient-buffer bug above** (already fixed, see TODO.md).
Every method on `PluginHost` (`src/PluginHost.h`), what it actually wires
to on the VST2 side for a hosted plugin, and which test(s) exercise it.
"Exercised" means the call happens during that test; "asserted" means the
test actually checks something about its result, not just that it ran
without crashing.
**JaySynth's compiled VST2 wrapper never advertises chunk support to the
host**, so loading the bundled sample `.fxp` files through `testhost`'s
generic `--patch` (i.e. through `AudioProcessor::setCurrentProgramState-
Information`, exactly what any real VST2 host uses) has **zero effect**
confirmed by diffing `--printParams` output with and without `--patch`.
Those sample files are legitimately in VST2's opaque-chunk ("FPCh") format
(verified via hex dump: correct `CcnK`/`FPCh`/`Jsy1` header), but JUCE
3.1.1's plugin-side VST wrapper (`sdk/juce/JUCE-3.1.1/modules/
juce_audio_plugin_client/VST/juce_VST_Wrapper.cpp`, compiled into
`JaySynth.so`) never sets the VST2 `effFlagsProgramChunks` flag on the
`AEffect` struct, so JUCE's own host-side `VSTPluginInstance::usesChunks()`
sees false and `setChunkData()` silently no-ops — for both loading *and*
saving. This is **not** one of the tracked `TODO.md` findings and this
suite doesn't attempt to fix it (it would mean patching JUCE's own VST
wrapper, a materially bigger and riskier change). It's why
`stability_sweep` only checks "doesn't crash," not "changes the patch," and
why the round-trip tests use `testhost`'s own save output as the fixture
rather than a real sample file — `testhost`'s save path has the same
`usesChunks()`-is-false behaviour, so save and load are at least
consistent with *each other*, even though neither talks to the external
`.fxp` files' actual chunk data. Tracked as a discovered issue in
`tools/testhost/TODO.md`'s "Investigate" section.
| `PluginHost` method | VST2 wiring | Covering test(s) |
|---|---|---|
| `load()` | `effOpen` (via `createPluginInstance`), then inside `prepareToPlay`: `effSetSampleRate`, `effSetBlockSize`, `effMainsChanged(1)`, `effStartProcess` | Every test - exercised, not specifically asserted beyond "didn't return false" |
| `releaseResources()` | `effMainsChanged(0)`/`effStopProcess` (`VSTPluginInstance::releaseResources`) | Every test calls it before exit (`main.cpp`) - exercised only, no assertion |
| `getNumInputChannels()` / `getNumOutputChannels()` | Reads cached `AEffect::numInputs`/`numOutputs`, populated at `effOpen` | Every test (printed in the `"loaded ... (N in/N out...)"` banner) - exercised, not asserted against an expected value for any specific plugin |
| `getNumParameters()` | Reads cached `AEffect::numParams` | Every test (same load banner; also implicit in every `--printParams` use) - exercised, not asserted |
| `getParameter(index)` | `AEffect::getParameter` callback | `patch_save_load_roundtrip`, `bank_save_load_roundtrip`, `bank_chunk_roundtrip`, `patch_chunk_roundtrip` (all via `--printParams`) - asserted (value checked within tolerance) |
| `setParameter(index, value)` | `AEffect::setParameter` callback | Same four round-trip tests (via `--param`) - asserted indirectly (the value it sets is what gets read back) |
| `process()` - audio only | `processReplacing`/`processDoubleReplacing` | Every test - asserted only for NaN/Inf absence, not correctness of the DSP output itself |
| `process()` - MIDI note on/off | `effProcessEvents` (`VstMidiEvent`) | `oversized_blocksize`, `nrpn_out_of_range`, `nrpn_in_range`, `stability_sweep` (all use `--note`) - exercised (audio is produced), not asserted beyond non-silence/no-NaN |
| `process()` - MIDI CC/NRPN | `effProcessEvents` (`VstMidiEvent`, controller sub-type) | `nrpn_out_of_range`, `nrpn_in_range` - exercised and the specific regression (bounds check) is asserted via clean completion |
| `getCurrentProgramStateInformation()` | `saveToFXBFile(isFXB=false)``effGetChunk(index=1)` (confirmed chunk-based, see below) | `patch_save_load_roundtrip` - asserted |
| `setCurrentProgramStateInformation()` | `loadFromFXBFile``effSetChunk(index=1)` | `patch_save_load_roundtrip` - asserted; also used by `stability_sweep` via `--patch`, but **not asserted there** - see the corrected-finding note, those particular files don't actually change anything |
| `getStateInformation()` | `saveToFXBFile(isFXB=true)``effGetChunk(index=0)` | `bank_save_load_roundtrip` - asserted |
| `setStateInformation()` | `loadFromFXBFile``effSetChunk(index=0)` | `bank_save_load_roundtrip` - asserted |
| `getInstance()` | n/a (plain accessor, returns the raw `AudioPluginInstance*`) | Used internally by every `Vst2RawChunk`-based test to reach the raw `AEffect*` |
| *(bypass, not on `PluginHost`)* `Vst2RawChunk::getChunk`/`setChunk` | `effGetChunk`/`effSetChunk` called directly on the raw `AEffect*`, bypassing `usesChunks()` and `AudioProcessor` entirely | `bank_chunk_roundtrip`, `patch_chunk_roundtrip` - asserted, plus the zlib-header/size check that positively confirms the real chunk path ran |
**Known gaps**: `load()`/`releaseResources()`/`getNumInputChannels()`/
`getNumOutputChannels()`/`getNumParameters()` are exercised by literally
every test run but never *asserted* against a specific expected value for
a specific plugin (e.g. nothing currently pins "JaySynth must report
exactly 0 in / 2 out / 140 parameters" - a change to any of those would go
unnoticed by this suite unless it also broke something else). Same for
`process()`'s actual DSP correctness - the suite checks "ran without
crashing/NaN," never "the audio sounds right." Both are reasonable
follow-ups noted in `tools/testhost/TODO.md`.
## Corrected finding: it's not a chunk-advertisement gap after all
An earlier version of this suite (and `tools/testhost/TODO.md`) claimed
that JaySynth's compiled VST2 wrapper never sets `effFlagsProgramChunks`,
so JUCE's host-side `usesChunks()` would see false and silently skip the
chunk mechanism - offered as the reason bundled sample `.fxp` files have no
effect when loaded via `--patch`. **That diagnosis was wrong.** It's
disproved directly: `--saveBank`/`--savePatch` (the exact same generic
`AudioProcessor::getStateInformation` call) produce real `FBCh`/`FPCh`-
magic files containing a genuine zlib-compressed XML blob (confirmed via
hex dump), not a per-parameter fallback - meaning `usesChunks()` is
genuinely `true`. To be certain, `Vst2RawChunk` was added specifically to
rule out any remaining JUCE-side gating: it calls `effGetChunk`/
`effSetChunk` straight on the raw `AEffect*`, and feeding a bundled sample
file's inner chunk bytes through it *this way* still has zero effect.
`test_chunk_roundtrip` above proves this exact mechanism round-trips
correctly for files this build produces.
So the real, still-open question is narrower: **why do these specific
(10+ year old) external sample files fail to apply**, given the mechanism
that's supposed to load them demonstrably works? Most likely a compression
or XML-schema shape from an older JaySynth/JUCE combination that
`JaySynthAudioProcessor::setCurrentProgramStateInformation`/
`getXmlFromBinary` no longer parses - not investigated further here (would
mean decompressing and diffing one of these files' XML against what this
build produces). Not one of the tracked repo-root `TODO.md` findings.
Tracked in `tools/testhost/TODO.md`'s "Investigate" section.
## Debugging notes: how the VCF bug was actually found