feat(editor): Shift Audio — slide the recording in time (chart fixed, non-destructive) - #236
Conversation
Christian's ask: be able to shift the audio track(s). The editor only ever let you offset the CHART against a fixed recording; nothing moved the audio itself. This adds a non-destructive audio placement shift — the inverse of the chart offset — for when a recording starts late, has leading silence, or was swapped via Replace Audio and no longer lines up with the chart you already built. - New `S.audioShift` (seconds): at playhead chart-time T the audio plays buffer-time (T - audioShift). The samples are never stretched; only the buffer read position moves (`_audioBufferStartPure` handles the pre-audio delay and the past-the-end no-source case — the transport clock/cursor/guide are untouched, so only the audio slides). The waveform and onset strip render shifted to match; onset snap / Suggest / Sync read a shifted-onset accessor (`_ensureOnsetsShifted`, zero-alloc when unshifted) so they follow the audio. - One value for the whole audio group, so stems (when they land in the editor) will move together — per Christian's spec. - Undoable via `AudioShiftCmd` (song-scoped); a **Shift Audio…** button next to Replace Audio prompts for the shift; reset to 0 on song load, and the value is wired onto the save/load path (`data.audio_shift`) so a persisted shift is honored on load. `tests/audio_shift.test.mjs` (9: the buffer-start boundary pure + the command round-trip); `onset_snap.test.js` updated for the shifted-onset accessor. 129 JS suites green, lint 0-err (3 pre-existing warnings). routes.py untouched. Verified live on AC/DC — Back In Black: Shift Audio… → +0.5s → the waveform slid right (a flat lead-in appeared) while the notes/grid stayed put, status "Audio shifted +500ms — recording moved, chart unchanged"; undo restored. Follow-up: persisting audio_shift INTO the built pack (a feedpak-spec/routes field + the final-app consumption) is deferred — the value already round-trips on the wire and is honored on load; the pack write is a focused next step. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EBQCHCNA81E9tHmSDHSe2Q
|
Warning Review limit reached
Next review available in: 15 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds an undoable audio-placement shift that moves playback, waveform, onset analysis, snapping, tempo tools, and persistence while keeping the chart fixed. A toolbar prompt and session save/load support are included. ChangesAudio shift
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
actor Editor
participant Prompt
participant AudioShiftCmd
participant Playback
participant Renderer
Editor->>Prompt: enter audio shift
Prompt->>AudioShiftCmd: apply shift
AudioShiftCmd->>Playback: restart at chart cursor
AudioShiftCmd->>Renderer: redraw shifted audio
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/waveform.js (1)
98-113: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMinor duplication: shift/visible-range computation repeated.
The
sh/xLo/xHicomputation here mirrors the one indrawWaveform(Lines 36-39). Could be factored into a small shared helper (e.g._visibleAudioXRange(w, dur, sh)), though the duplication is small and low-risk as-is.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/waveform.js` around lines 98 - 113, Factor the duplicated shifted visible-range calculation from drawWaveform and this onset-rendering block into a shared helper such as _visibleAudioXRange(w, dur, sh). Update both call sites to reuse the helper’s xLo and xHi values while preserving the existing LABEL_W, timeToX, and rounding behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/waveform.js`:
- Around line 98-113: Factor the duplicated shifted visible-range calculation
from drawWaveform and this onset-rendering block into a shared helper such as
_visibleAudioXRange(w, dur, sh). Update both call sites to reuse the helper’s
xLo and xHi values while preserving the existing LABEL_W, timeToX, and rounding
behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 925ed6a1-f728-46c6-8390-a8e380a19512
📒 Files selected for processing (13)
CHANGELOG.mdscreen.htmlsrc/audio.jssrc/create.jssrc/file-ops.jssrc/input.jssrc/loop.jssrc/main.jssrc/state.jssrc/sync-tempo.jssrc/waveform.jstests/audio_shift.test.mjstests/onset_snap.test.js
# Conflicts: # CHANGELOG.md
The editor only ever let you offset the chart against a fixed recording — nothing moved the audio itself. This adds a non-destructive audio placement shift: slide the recording earlier/later while the chart/grid/notes stay put. It's the fix for a recording that starts late, has leading silence, or was swapped via Replace Audio and no longer lines up with the chart you already built — move the audio, not every note.
How it works
S.audioShift(seconds): at playhead chart-timeTthe audio plays buffer-timeT − audioShift. The samples are never stretched — only the buffer read position moves._audioBufferStartPurehandles the pre-audio delay (chart ahead of where the audio begins) and the past-the-end no-source case; the transport clock / cursor / guide-metronome are untouched, so only the audio slides._ensureOnsetsShifted, which returns the raw cached array untouched when there's no shift — zero allocation on hot paths).AudioShiftCmd, song-scoped). A Shift Audio… button next to Replace Audio prompts for the shift; reset to 0 on load; the value round-trips on the save/load wire (data.audio_shift) and is honored on load.Divergence note
This is a rigid shift, not a warp — fully consistent with the editor's "audio is immutable, the grid fits the audio" rule (the opposite of Ableton's destructive warp). The samples are never re-timed; playback just reads from a shifted offset.
Tests / gates
tests/audio_shift.test.mjs(9: the buffer-start boundary pure + theAudioShiftCmd/editorSetAudioShiftround-trip);onset_snap.test.jsupdated for the shifted-onset accessor. 129 JS suites green, lint 0 errors (3 pre-existing warnings).routes.pyuntouched.Verified live
On Test Artist — Test Track: Shift Audio… → +0.5s → the waveform slid right (a flat lead-in appeared) while the notes/grid stayed put; status "Audio shifted +500ms — recording moved, chart unchanged"; undo restored. No page errors.
Follow-up (scoped, not in this PR)
Persisting
audio_shiftinto the built pack (a feedpak-spec/routes.pyfield + the final-app's consumption of it) is deferred — the value already round-trips on the wire and is honored on load; the pack write is a focused next step needing the format-field decision.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes