Repository navigation
Technical review of the plugin, with its recommended fixes and sound-design measurements - #8
Merged
Conversation
added 4 commits
November 3, 2026 15:11
…d a technical review Applies the recommended items of the review in review/2026-11-03-code-review.md: - The echo's delay line started at 5 s and glided towards the knob for seconds, settling up to 3 ms short of it: it now jumps to the knob on the first tick and after a NaN recovery. The three reference renders with echo change, the seven without are byte-identical; SHA256SUMS regenerated. New test echoStartsAtKnobTime. - processBlockBypassed delays the input by the declared latency, so hosts at rates other than 48 kHz keep the bypassed audio aligned. New ui_test checks. - ParameterRegistry stores its parameters in a fixed array and runs the handlers in an explicit order, the one libc++'s unordered_map happened to give, instead of depending on the standard library's hash-map iteration. - DaisySP WhiteNoise multiplies as uint32_t: the signed product overflowed (undefined behaviour) and stopped ASan/UBSan at the first sample. Same bits. Recorded in PATCHES.txt. - prepareToPlay clamps a sample rate of 0 to 48 kHz. - CI builds the AU and validates it with auval and pluginval. - JUCE is fetched by commit hash, not by tag. The review folder holds the report, the experiments it cites and the test logs before and after the fixes.
20 s of the Standalone's output, taken through BlackHole at 48 kHz, compared with the engine rendering the same restored state: levels, partials, echo period and stereo correlation match, with no clipping or dropouts. The scripts that render the state and compute the comparison sit with the other experiments.
Automatic measurements on the engine and on the hosted VST3: preset levels and birth times, the MIDI pitch map with and without reverb, repeatability of a loaded state, mono compatibility, ten-minute runs at the extremes, extreme settings and input, and the same presets at 44.1, 48 and 96 kHz. The two findings worth acting on: five presets settle on a different partial depending on when the host delivers the state, and the reverb in the loop detunes MIDI notes below 48 by up to 46 cents at 35 % mix. The measurement programs sit with the other experiments.
…nes low notes A host hands the plugin its state after prepareToPlay, so the parameters glided from the defaults to the project's values. The drone is chaotic: the path taken decided which resonance it settled on, and five presets landed on a different partial, up to an octave away, depending on when the state arrived. The next block after setStateInformation now takes the parameters at once, as the first block after Init does; presets chosen in the window still glide. New ui_test checks that a restored state jumps while a knob move still glides. The manual's MIDI tab and known limits give the measured detuning: dry, notes 18 to 84 stay within 17 cents; at 35 % Reverb Mix, notes below 48 read up to 46 cents off.
The report and the measurements stay; the 600 KB of logs they cited were evidence for the review session, not something to carry in the history.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What changed
A technical review of the plugin (DSP, real-time, VST3/AU, build), the fixes it recommended, and the measurements a sound designer would want next. The report and the experiments it cites are in
review/; the build and test logs were kept out of the repository.Fixes from the review (
349127c)echoStartsAtKnobTime.processBlockBypasseddelays the input by the declared latency, so hosts at rates other than 48 kHz keep the bypassed audio aligned. Newui_testchecks.ParameterRegistrykeeps its parameters in a fixed array and runs the handlers in an explicit order, the one libc++'sunordered_maphappened to give, instead of depending on the standard library's iteration order.WhiteNoiseoverflowed a signedint32_ton every sample, which stopped ASan/UBSan at the first sample. It now multiplies asuint32_t: same bits. Recorded inthird_party/DaisySP/PATCHES.txt.prepareToPlayclamps a placeholder rate of 0 to 48 kHz.Drosera_AU, installs it for the runner and runsauvalandpluginvalon it. Passed on its first run.Measurements and the fix they led to (
ec6e150,0237f49,bac7cf6)review/2026-11-03-sound-design-measurements.md: preset levels and birth times, the MIDI pitch map with and without reverb, repeatability of a loaded state, mono compatibility, ten-minute runs at the extremes, extreme settings, and the same presets at 44.1/48/96 kHz.prepareToPlay, so the parameters glided from the defaults and five presets settled on a different partial (up to an octave away) depending on when the state arrived. The block aftersetStateInformationnow takes the parameters at once; presets chosen in the window still glide. Newui_testcheck.Reference renders
Rule 2 of
CLAUDE.md: the echo fix changes the three reference renders with echo (p05, p06, p10) throughout, because the old one-pole never reached the knob; the seven without echo are byte-identical, which also shows the registry change alters nothing.SHA256SUMSis regenerated.tests/fixtures/state-baseline.binis untouched.Verification
voice_allocator_test,render_test(472 checks),ui_test,plugin_host_testwith the baseline fixture,check_reference.sh: all pass on every commit.349127c(before that change UBSan halted on the noise generator). Not rerun for the later commits, which add a flag, documents and tests.pluginvalandauval(CI does), Intel build.For the reviewer