mirror of
https://github.com/got-feedBack/feedBack-desktop.git
synced 2026-08-14 04:31:20 +00:00
fix(audio): stop signal-chain duplication on renderer re-evaluation (#71)
Testers on 0.3.0-alpha.1 reported the signal chain duplicating (every VST/NAM/IR exactly twice) with blown-out gain after leaving the Audio menu, plus VST edit windows closing and the Edit button going dead. Root cause: the native JUCE chain lives in the Electron main process and survives renderer reloads and screen.js re-evaluations (host re-hydration after a backend restart), but init() unconditionally restored the localStorage-saved chain by APPENDING — aeRestoreSavedChain never clears. The #50 review added a clear-before-restore in the amp-sims toggle handler only; the identical hazard at init() remained. Since the saved chain mirrors the live chain, every init re-run produced an exact 2x duplicate (two amp stages in series = the blown-out gain). Fixes: - init(): probe getChainState() first and skip ALL auto-load (default preset and saved-chain restore) when the engine already has a live chain. Also covers splitscreen pop-out windows re-running init. - saveChainStateFromChain(): never persist a Rig-Builder-owned chain (identified by its _rb_unit_impulse / RB Final Leveler plumbing stages). Rig Builder reloads its default tone off-screen on its own schedule, so it is routinely the ambient live chain; snapshotting it made the saved chain resurrect Rig Builder's tone on restore — the exact processor set in the tester screenshot. - aeRestoreSavedChain(): drop Rig Builder plumbing stages from legacy polluted saves and rewrite the cleaned list (self-healing). - _aeOpenEditor(): a false return means the baked-in slot id went stale (chain rebuilt while the list was on screen); refresh the chain list instead of silently doing nothing. - Install-once guard (hookState) on the arrangement:changed/song:ready reapply listeners — they stacked one pair per re-evaluation, running N racing clear+load sequences per song load after a re-eval. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
be71e7a13a
commit
e22981405c
+65
-4
@@ -965,12 +965,28 @@ window.__feedBackDesktopAudioHooks = window.__feedBackDesktopAudioHooks || {};
|
|||||||
const _ampSimsEnabled = await aeUseAmpSims();
|
const _ampSimsEnabled = await aeUseAmpSims();
|
||||||
ampSimsCheckbox.checked = _ampSimsEnabled;
|
ampSimsCheckbox.checked = _ampSimsEnabled;
|
||||||
|
|
||||||
|
// Auto-load must only ever seed an EMPTY engine. The native chain lives
|
||||||
|
// in the Electron main process and survives renderer reloads, screen.js
|
||||||
|
// re-evaluations (host re-hydration after a backend restart), and
|
||||||
|
// splitscreen pop-out windows — while init() runs once per evaluation.
|
||||||
|
// The saved chain in localStorage mirrors the live chain, so restoring
|
||||||
|
// it on top of the surviving chain exactly duplicates every stage (two
|
||||||
|
// amp stages in series = the tester "chain duplicated after leaving the
|
||||||
|
// Audio menu" / "gain blown out" reports).
|
||||||
|
let _chainAlreadyLive = false;
|
||||||
|
try {
|
||||||
|
const _existing = await api.getChainState();
|
||||||
|
_chainAlreadyLive = Array.isArray(_existing) && _existing.length > 0;
|
||||||
|
} catch (_) { /* probe failed — treat as empty (cold-start behavior) */ }
|
||||||
|
|
||||||
// Try the default preset first; only restore the saved chain if no default preset is
|
// Try the default preset first; only restore the saved chain if no default preset is
|
||||||
// configured or the preset load fails (corrupted blob, missing VST, etc.). This avoids
|
// configured or the preset load fails (corrupted blob, missing VST, etc.). This avoids
|
||||||
// redundant native load/unload when the preset immediately replaces the chain, while
|
// redundant native load/unload when the preset immediately replaces the chain, while
|
||||||
// ensuring a valid chain is always available as a fallback.
|
// ensuring a valid chain is always available as a fallback.
|
||||||
let _defaultLoaded = false;
|
let _defaultLoaded = false;
|
||||||
if (_ampSimsEnabled) {
|
if (_chainAlreadyLive) {
|
||||||
|
console.info('[audio-engine] Engine already has a live chain — skipping auto-load (re-evaluation guard).');
|
||||||
|
} else if (_ampSimsEnabled) {
|
||||||
try {
|
try {
|
||||||
_defaultLoaded = await loadDefaultPreset('app-init');
|
_defaultLoaded = await loadDefaultPreset('app-init');
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
@@ -979,7 +995,7 @@ window.__feedBackDesktopAudioHooks = window.__feedBackDesktopAudioHooks || {};
|
|||||||
} else {
|
} else {
|
||||||
console.info('[audio-engine] Amp sims opt-out — skipping saved tone-chain restore (own-rig monitoring).');
|
console.info('[audio-engine] Amp sims opt-out — skipping saved tone-chain restore (own-rig monitoring).');
|
||||||
}
|
}
|
||||||
if (_ampSimsEnabled && !_defaultLoaded) {
|
if (!_chainAlreadyLive && _ampSimsEnabled && !_defaultLoaded) {
|
||||||
await aeRestoreSavedChain();
|
await aeRestoreSavedChain();
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1015,6 +1031,20 @@ window.__feedBackDesktopAudioHooks = window.__feedBackDesktopAudioHooks || {};
|
|||||||
console.warn('[audio-engine] Corrupted slopsmith-signal-chain; starting empty:', e);
|
console.warn('[audio-engine] Corrupted slopsmith-signal-chain; starting empty:', e);
|
||||||
savedChain = [];
|
savedChain = [];
|
||||||
}
|
}
|
||||||
|
// Drop Rig Builder plumbing stages from legacy saves. Before
|
||||||
|
// saveChainStateFromChain learned to skip Rig-Builder-owned chains, a
|
||||||
|
// user chain action taken while Rig Builder's default tone was live
|
||||||
|
// persisted its wrap stages (unit-impulse trim IR, Final Leveler) into
|
||||||
|
// the saved chain; restoring those as plain processors resurrects a
|
||||||
|
// tone this panel never built. Rewrite the cleaned list back so the
|
||||||
|
// save self-heals.
|
||||||
|
const _cleaned = savedChain.filter((item) => !isRigBuilderChainStage(item));
|
||||||
|
if (_cleaned.length !== savedChain.length) {
|
||||||
|
console.info('[audio-engine] Dropped', savedChain.length - _cleaned.length,
|
||||||
|
'Rig Builder plumbing stage(s) from the saved chain.');
|
||||||
|
try { localStorage.setItem('slopsmith-signal-chain', JSON.stringify(_cleaned)); } catch (_) {}
|
||||||
|
savedChain = _cleaned;
|
||||||
|
}
|
||||||
for (const item of savedChain) {
|
for (const item of savedChain) {
|
||||||
try {
|
try {
|
||||||
if (item.type === 'VST' && item.path) {
|
if (item.type === 'VST' && item.path) {
|
||||||
@@ -1031,7 +1061,24 @@ window.__feedBackDesktopAudioHooks = window.__feedBackDesktopAudioHooks || {};
|
|||||||
if (savedChain.length > 0) await refreshChain();
|
if (savedChain.length > 0) await refreshChain();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Rig Builder plumbing markers. Its default-tone/preview wraps carry a
|
||||||
|
// 1-sample unit-impulse trim IR and an auto-appended "RB Final Leveler"
|
||||||
|
// stage — a chain containing either was built by the Rig Builder plugin,
|
||||||
|
// not by this panel.
|
||||||
|
const RB_PLUMBING_MARKER = /_rb_unit_impulse|RB Final Leveler/i;
|
||||||
|
function isRigBuilderChainStage(stage) {
|
||||||
|
return RB_PLUMBING_MARKER.test(String(stage?.path || ''))
|
||||||
|
|| RB_PLUMBING_MARKER.test(String(stage?.name || ''));
|
||||||
|
}
|
||||||
|
|
||||||
function saveChainStateFromChain(chain) {
|
function saveChainStateFromChain(chain) {
|
||||||
|
// Never persist a Rig-Builder-owned chain. Rig Builder reloads its
|
||||||
|
// default tone into the engine on its own schedule (song stop, screen
|
||||||
|
// leave), so it is routinely the ambient live chain while the user is
|
||||||
|
// in this panel; snapshotting it here would make the saved chain
|
||||||
|
// resurrect Rig Builder's tone on the next restore. Keep the last
|
||||||
|
// panel-built chain instead.
|
||||||
|
if (Array.isArray(chain) && chain.some(isRigBuilderChainStage)) return;
|
||||||
const typeMap = { 0: 'VST', 1: 'NAM', 2: 'IR' };
|
const typeMap = { 0: 'VST', 1: 'NAM', 2: 'IR' };
|
||||||
const items = chain.filter(s => s.type === 0 || s.type === 1 || s.type === 2).map(s => ({
|
const items = chain.filter(s => s.type === 0 || s.type === 1 || s.type === 2).map(s => ({
|
||||||
type: typeMap[s.type] || 'VST',
|
type: typeMap[s.type] || 'VST',
|
||||||
@@ -1287,7 +1334,16 @@ window.__feedBackDesktopAudioHooks = window.__feedBackDesktopAudioHooks || {};
|
|||||||
};
|
};
|
||||||
|
|
||||||
window._aeOpenEditor = async (slotId) => {
|
window._aeOpenEditor = async (slotId) => {
|
||||||
await api.openPluginEditor(slotId);
|
const ok = await api.openPluginEditor(slotId);
|
||||||
|
// A false return means the slot id went stale — the chain was rebuilt
|
||||||
|
// (song stop/preset load clears + reloads, assigning new ids) while
|
||||||
|
// this list stayed on screen with the old ids baked into its buttons.
|
||||||
|
// Re-render so the buttons pick up the live ids instead of silently
|
||||||
|
// doing nothing ("Edit no longer opens the VST window").
|
||||||
|
if (ok === false) {
|
||||||
|
console.warn('[audio-engine] openPluginEditor rejected slot', slotId, '— refreshing chain (stale slot id?)');
|
||||||
|
await refreshChain();
|
||||||
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
// ── VST Browser ───────────────────────────────────────────────────────────
|
// ── VST Browser ───────────────────────────────────────────────────────────
|
||||||
@@ -4143,7 +4199,12 @@ window.__feedBackDesktopAudioHooks = window.__feedBackDesktopAudioHooks || {};
|
|||||||
renderToneAutomationSettings();
|
renderToneAutomationSettings();
|
||||||
}).catch(e => console.error('[audio-engine] init error:', e));
|
}).catch(e => console.error('[audio-engine] init error:', e));
|
||||||
|
|
||||||
if (window.slopsmith?.on) {
|
// Install-once across re-evaluations (mirrors installToneSetupLifecycleHooks):
|
||||||
|
// without the guard each re-evaluation stacks another listener pair, so one
|
||||||
|
// song:ready would run N racing tone-mapping applies (each a clear+load
|
||||||
|
// that also closes every open VST editor window).
|
||||||
|
if (window.slopsmith?.on && !hookState.toneReapplyHooksInstalled) {
|
||||||
|
hookState.toneReapplyHooksInstalled = true;
|
||||||
let _reapplyDebounceTimer = null;
|
let _reapplyDebounceTimer = null;
|
||||||
let _reapplyFollowupTimer = null;
|
let _reapplyFollowupTimer = null;
|
||||||
const scheduleReapply = () => {
|
const scheduleReapply = () => {
|
||||||
|
|||||||
@@ -0,0 +1,115 @@
|
|||||||
|
// Guards against the alpha-tester chain-duplication bug: the audio_engine
|
||||||
|
// panel must (1) never persist a Rig-Builder-owned live chain into
|
||||||
|
// localStorage, (2) drop Rig Builder plumbing stages from legacy saves on
|
||||||
|
// restore, and (3) never auto-load (default preset / saved-chain restore) on
|
||||||
|
// top of an engine that already has a live chain — the native chain survives
|
||||||
|
// renderer re-evaluations, so an unconditional restore appended an exact
|
||||||
|
// duplicate of every stage (two amp stages in series = "gain blown out").
|
||||||
|
|
||||||
|
const test = require('node:test');
|
||||||
|
const assert = require('node:assert/strict');
|
||||||
|
const path = require('node:path');
|
||||||
|
const fs = require('node:fs');
|
||||||
|
const vm = require('node:vm');
|
||||||
|
|
||||||
|
const ROOT = path.join(__dirname, '..');
|
||||||
|
const SCREEN_JS = fs.readFileSync(path.join(ROOT, 'src', 'renderer', 'screen.js'), 'utf8');
|
||||||
|
|
||||||
|
// Brace-balanced extraction of `function NAME(...) { ... }`.
|
||||||
|
function extractFunction(src, name) {
|
||||||
|
const sig = `function ${name}(`;
|
||||||
|
const start = src.indexOf(sig);
|
||||||
|
assert.ok(start !== -1, `function '${name}' not found`);
|
||||||
|
const openBrace = src.indexOf('{', start);
|
||||||
|
let depth = 1;
|
||||||
|
let i = openBrace + 1;
|
||||||
|
while (i < src.length && depth > 0) {
|
||||||
|
if (src[i] === '{') depth++;
|
||||||
|
else if (src[i] === '}') depth--;
|
||||||
|
i++;
|
||||||
|
}
|
||||||
|
assert.ok(depth === 0, `unbalanced braces in '${name}'`);
|
||||||
|
return src.slice(start, i);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Extract the marker const + both helpers into one sandbox.
|
||||||
|
function setupSandbox() {
|
||||||
|
const markerStart = SCREEN_JS.indexOf('const RB_PLUMBING_MARKER');
|
||||||
|
assert.ok(markerStart !== -1, 'RB_PLUMBING_MARKER not found');
|
||||||
|
const markerDecl = SCREEN_JS.slice(markerStart, SCREEN_JS.indexOf(';', markerStart) + 1);
|
||||||
|
const code = [
|
||||||
|
markerDecl,
|
||||||
|
extractFunction(SCREEN_JS, 'isRigBuilderChainStage'),
|
||||||
|
extractFunction(SCREEN_JS, 'saveChainStateFromChain'),
|
||||||
|
].join('\n');
|
||||||
|
const stored = new Map();
|
||||||
|
const sandbox = {
|
||||||
|
localStorage: {
|
||||||
|
getItem: (k) => (stored.has(k) ? stored.get(k) : null),
|
||||||
|
setItem: (k, v) => { stored.set(k, String(v)); },
|
||||||
|
},
|
||||||
|
};
|
||||||
|
vm.runInNewContext(code, sandbox, { filename: 'chain-persistence.js' });
|
||||||
|
return { sandbox, stored };
|
||||||
|
}
|
||||||
|
|
||||||
|
const RB_CHAIN = [
|
||||||
|
{ type: 0, path: '/vst/SamplegSBTCL.vst3', name: 'SamplegSBTCL' },
|
||||||
|
{ type: 2, path: '/irs/_rb_unit_impulse.wav', name: '_rb_unit_impulse' },
|
||||||
|
{ type: 0, path: '/vst/RB Final Leveler.vst3', name: 'RB Final Leveler' },
|
||||||
|
];
|
||||||
|
|
||||||
|
test('saveChainStateFromChain skips a Rig-Builder-owned live chain', () => {
|
||||||
|
const { sandbox, stored } = setupSandbox();
|
||||||
|
stored.set('slopsmith-signal-chain', '[{"type":"NAM","path":"/nam/amp.nam","name":"amp"}]');
|
||||||
|
sandbox.saveChainStateFromChain([...RB_CHAIN, { type: 1, path: '/nam/vox.nam', name: 'VOX' }]);
|
||||||
|
assert.equal(
|
||||||
|
stored.get('slopsmith-signal-chain'),
|
||||||
|
'[{"type":"NAM","path":"/nam/amp.nam","name":"amp"}]',
|
||||||
|
'the previously saved panel-built chain must be preserved',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('saveChainStateFromChain persists a panel-built chain normally', () => {
|
||||||
|
const { sandbox, stored } = setupSandbox();
|
||||||
|
sandbox.saveChainStateFromChain([
|
||||||
|
{ type: 0, path: '/vst/MyAmp.vst3', name: 'MyAmp' },
|
||||||
|
{ type: 1, path: '/nam/vox.nam', name: 'VOX' },
|
||||||
|
{ type: 7, path: '/x', name: 'not-a-chain-type' },
|
||||||
|
]);
|
||||||
|
assert.deepEqual(JSON.parse(stored.get('slopsmith-signal-chain')), [
|
||||||
|
{ type: 'VST', path: '/vst/MyAmp.vst3', name: 'MyAmp' },
|
||||||
|
{ type: 'NAM', path: '/nam/vox.nam', name: 'VOX' },
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('isRigBuilderChainStage matches plumbing by path or name only', () => {
|
||||||
|
const { sandbox } = setupSandbox();
|
||||||
|
assert.equal(sandbox.isRigBuilderChainStage({ path: '/irs/_rb_unit_impulse.wav' }), true);
|
||||||
|
assert.equal(sandbox.isRigBuilderChainStage({ name: 'RB Final Leveler' }), true);
|
||||||
|
assert.equal(sandbox.isRigBuilderChainStage({ path: '/vst/MyAmp.vst3', name: 'MyAmp' }), false);
|
||||||
|
assert.equal(sandbox.isRigBuilderChainStage(null), false);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('init auto-load is gated on an empty engine chain (re-evaluation guard)', () => {
|
||||||
|
// Structural assertions: the guard exists and both auto-load paths honor it.
|
||||||
|
assert.equal(SCREEN_JS.includes('_chainAlreadyLive'), true);
|
||||||
|
assert.equal(
|
||||||
|
SCREEN_JS.includes('if (!_chainAlreadyLive && _ampSimsEnabled && !_defaultLoaded)'),
|
||||||
|
true,
|
||||||
|
'saved-chain restore must be gated on the live-chain probe',
|
||||||
|
);
|
||||||
|
assert.equal(
|
||||||
|
SCREEN_JS.includes('if (_chainAlreadyLive) {'),
|
||||||
|
true,
|
||||||
|
'default-preset auto-load must be gated on the live-chain probe',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('restore drops Rig Builder plumbing stages from legacy saves', () => {
|
||||||
|
// aeRestoreSavedChain is async and coupled to the api bridge; assert the
|
||||||
|
// sanitize step is present and rewrites the cleaned save.
|
||||||
|
const fn = extractFunction(SCREEN_JS, 'aeRestoreSavedChain');
|
||||||
|
assert.equal(fn.includes('isRigBuilderChainStage'), true);
|
||||||
|
assert.equal(fn.includes("localStorage.setItem('slopsmith-signal-chain', JSON.stringify(_cleaned))"), true);
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user