diff --git a/static/js/main.js b/static/js/main.js index 4009ce6c..80e6f331 100644 --- a/static/js/main.js +++ b/static/js/main.js @@ -1,5 +1,5 @@ import { - playBtn, loopBtn, multitrack, totalDuration, loopEnabled, loopStart, loopEnd, + playBtn, loopBtn, totalDuration, loopEnabled, loopStart, loopEnd, setLoopStart, setLoopEnd, selectedStems, saveSelectedStems, stemSelectionReady, currentJobId, vocalSplitMode, vocalSplitModeReady, setVocalSplitMode, setAutoSectionsResetFn, @@ -10,7 +10,7 @@ import { wireJobForm, showError } from "./job.js"; import { initSearch } from "./search.js"; import { wireTransportButtons } from "./transport.js"; import { wireBeatGridUi } from "./beatgridUi.js"; -import { togglePlayPause, updateLoopRegionVisual, toggleMetronome } from "./transport.js"; +import { togglePlayPause, updateLoopRegionVisual, toggleMetronome, transport, setPlayheadTime } from "./transport.js"; import { wireStemListControls, wireMixerToolbar } from "./mixer.js"; import { initCatalog, collectDiagnostics } from "./catalog.js"; import { initNotifications, notifyFailure, dismissFailuresByJobId } from "./notifications.js"; @@ -486,10 +486,14 @@ function wireFooterControls() { const scrub = document.getElementById("footer-scrub"); if (scrub) { function seekToX(clientX) { - if (!multitrack || !totalDuration) return; + // setPlayheadTime, not multitrack.setTime: on the default chunked engine + // the multitrack is silent and this whole bar did nothing. It also moves + // the playhead marker, footer times and presence playhead, which the old + // call never did (#515). + if (!totalDuration) return; const rect = scrub.getBoundingClientRect(); const frac = Math.max(0, Math.min(1, (clientX - rect.left) / rect.width)); - multitrack.setTime(frac * totalDuration); + setPlayheadTime(frac * totalDuration); } let _scrubbing = false; scrub.addEventListener("mousedown", (e) => { @@ -645,32 +649,36 @@ function wireAppShellControls() { // ─── Keyboard shortcuts ─── document.addEventListener("keydown", (e) => { - if (!multitrack) return; - if (e.target instanceof HTMLInputElement) return; + if (!transport()) return; + // Textareas were not excluded, so Space in the log viewer started playback + // instead of scrolling. + if (e.target instanceof HTMLInputElement || e.target instanceof HTMLTextAreaElement) return; if (e.code === "Space") { e.preventDefault(); togglePlayPause(); } else if (e.code === "BracketLeft") { e.preventDefault(); - multitrack.setTime(Math.max(0, multitrack.getCurrentTime() - 5)); + // setPlayheadTime clamps to [0, totalDuration] itself, so the Math.max / + // Math.min the multitrack version needed are gone with it. + setPlayheadTime(transport().getCurrentTime() - 5); } else if (e.code === "BracketRight") { e.preventDefault(); - multitrack.setTime( - Math.min(multitrack.getDuration(), multitrack.getCurrentTime() + 5), - ); + setPlayheadTime(transport().getCurrentTime() + 5); } else if (e.code === "KeyL") { e.preventDefault(); loopBtn.click(); } else if (e.code === "KeyK") { e.preventDefault(); toggleMetronome(); - } else if (e.code === "KeyI" && loopEnabled && multitrack) { + } else if (e.code === "KeyI" && loopEnabled) { e.preventDefault(); - setLoopStart(Math.min(multitrack.getCurrentTime(), loopEnd - 0.5)); + // multitrack.getCurrentTime() is pinned at 0 on the engine path, so "set + // loop in at playhead" always wrote 0 regardless of where the playhead was. + setLoopStart(Math.min(transport().getCurrentTime(), loopEnd - 0.5)); updateLoopRegionVisual(); - } else if (e.code === "KeyO" && loopEnabled && multitrack) { + } else if (e.code === "KeyO" && loopEnabled) { e.preventDefault(); - setLoopEnd(Math.max(multitrack.getCurrentTime(), loopStart + 0.5)); + setLoopEnd(Math.max(transport().getCurrentTime(), loopStart + 0.5)); updateLoopRegionVisual(); } }); diff --git a/static/js/transport.js b/static/js/transport.js index 7b6e92fc..826456e8 100644 --- a/static/js/transport.js +++ b/static/js/transport.js @@ -70,7 +70,18 @@ function timeFromClientX(clientX) { return frac * totalDuration; } -function setPlayheadTime(sec) { +/// The clock that actually owns playback. +/// +/// engineMode() defaults to "chunked", where audioEngine drives audio and the +/// multitrack is mounted with url: null for visuals only -- so operating on +/// `multitrack` directly moves nothing and reads 0. Everything in this module +/// already went through `audioEngine ?? multitrack`; exporting it stops other +/// modules re-deriving it and drifting (#515). +export function transport() { + return audioEngine ?? multitrack; +} + +export function setPlayheadTime(sec) { const tx = audioEngine ?? multitrack; if (!tx || !totalDuration) return; const next = Math.max(0, Math.min(totalDuration, sec)); diff --git a/tests/js/transport-clock.test.mjs b/tests/js/transport-clock.test.mjs new file mode 100644 index 00000000..61bf6f06 --- /dev/null +++ b/tests/js/transport-clock.test.mjs @@ -0,0 +1,72 @@ +// main.js drove `multitrack` directly while audioEngine owns the clock (#515). +// +// engineMode() defaults to "chunked", where the multitrack is mounted with +// url: null for visuals only. So the footer scrub bar did nothing at all, and +// "set loop in at playhead" always wrote 0 because multitrack.getCurrentTime() +// is pinned there. +// +// A structural check rather than a DOM one: the bug was not a wrong value from +// a function, it was reaching for the wrong object. Anything that reintroduces +// a bare multitrack playback call in main.js fails here. + +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import { dirname, join } from 'node:path'; + +const root = join(dirname(fileURLToPath(import.meta.url)), '..', '..'); +const mainSrc = readFileSync(join(root, 'static/js/main.js'), 'utf8'); +const transportSrc = readFileSync(join(root, 'static/js/transport.js'), 'utf8'); + +let passed = 0; +let failed = 0; + +function check(name, condition, detail = '') { + if (condition) { + passed++; + console.log(`PASS ${name}`); + } else { + failed++; + console.log(`FAIL ${name}${detail ? ` -- ${detail}` : ''}`); + } +} + +// Strip comments so the explanatory ones below don't count as usage. +const code = mainSrc.replace(/\/\/.*$/gm, '').replace(/\/\*[\s\S]*?\*\//g, ''); + +for (const call of ['multitrack.setTime', 'multitrack.getCurrentTime', 'multitrack.getDuration']) { + check( + `main.js does not call ${call}`, + !code.includes(call), + 'the multitrack is silent on the default chunked engine', + ); +} + +check( + 'transport.js exports the accessor', + /export function transport\(\)/.test(transportSrc), +); + +check( + 'the accessor prefers the audio engine', + /return audioEngine \?\? multitrack;/.test(transportSrc), +); + +check( + 'transport.js exports setPlayheadTime', + /export function setPlayheadTime\(/.test(transportSrc), +); + +check( + 'main.js imports both rather than re-deriving them', + /import \{[^}]*\btransport\b[^}]*\} from "\.\/transport\.js"/.test(mainSrc) && + /import \{[^}]*\bsetPlayheadTime\b[^}]*\} from "\.\/transport\.js"/.test(mainSrc), +); + +check( + 'the keyboard guard excludes textareas', + code.includes('HTMLTextAreaElement'), + 'Space in the log viewer used to start playback instead of scrolling', +); + +console.log(`\n${passed}/${passed + failed} checks passed`); +process.exit(failed === 0 ? 0 : 1);