feat(music-studio): render/bounce a song to a downloadable WAV (Tone.Offline)#1650
Conversation
Music Studio could play live and export MIDI/.taosong but produced no audio file. Add an offline Tone.Offline render that reuses the live engine's own scheduling (scheduleTrackNotes, buildSynthInstrument, volumeToDb, now exported from audio-engine.ts) so a bounce sounds like playback instead of a re-implementation of it. Sampled (smplr) instruments fetch samples from a CDN and drive playback off the real wall clock, neither of which works inside an OfflineAudioContext, so those tracks render with their closest synth substitute and the UI surfaces which tracks were swapped. Adds an "Export WAV" action next to the existing MIDI/JSON export with a rendering state and error/notice display.
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Warning Review limit reached
Next review available in: 44 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds WAV export capability to Music Studio: an offline renderer ( ChangesWAV Export Feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant MusicStudioApp
participant WavExport
participant AudioEngine
participant ToneOffline
User->>MusicStudioApp: Click "Download .wav"
MusicStudioApp->>WavExport: exportSongWavFile(song)
WavExport->>WavExport: computeSongDurationSeconds(song)
WavExport->>AudioEngine: scheduleTrackNotes(track, instrument)
WavExport->>ToneOffline: Tone.Offline(callback, duration)
ToneOffline-->>WavExport: AudioBuffer
WavExport->>WavExport: audioBufferToWavBlob(buffer)
WavExport-->>MusicStudioApp: RenderResult (blob, substitutedTracks)
MusicStudioApp-->>User: Trigger file download / show notice or error
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Docs-Reviewed: wav-export.ts and wav-export.test.ts are new source files inside the existing Music Studio app (desktop/src/apps/musicstudio/), not a new desktop app. They add offline WAV bounce to the existing app and register no new app in the app registry, so the apps structural rule does not require a README change.
|
|
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| export async function renderSongToWav(song: Song): Promise<RenderResult> { | ||
| const tracks = audibleTracks(song); | ||
| const duration = computeSongDurationSeconds(song); | ||
| if (duration <= RELEASE_TAIL_SECONDS) { |
There was a problem hiding this comment.
WARNING: The "no notes to export" guard uses duration <= RELEASE_TAIL_SECONDS, which silently renders 2 seconds of silence for a song whose notes have durationTicks: 0 or start at startTick: 0 with zero length (those notes don't advance lastEndTicks). Users would get a silent file with no error, rather than the intended rejection.
Consider tracking the total note count (or any note with durationTicks > 0) alongside lastEndTicks and rejecting when no such note exists.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| transport.start(0); | ||
| }, duration, 2, sampleRate); | ||
|
|
||
| return { blob: audioBufferToWavBlob(buffer.get()!), substitutedTracks }; |
There was a problem hiding this comment.
WARNING: buffer.get()! uses a non-null assertion. If Tone.Offline ever resolves with no rendered buffer (e.g. context failure), the resulting TypeError will surface as an opaque crash in the UI rather than the friendly error path in handleExportWav. Add an explicit null check and throw a descriptive Error.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| for (const track of tracks) { | ||
| const channel = new Tone.Channel({ volume: volumeToDb(track.volume), pan: track.pan }).toDestination(); | ||
| const renderInstrumentId = offlineInstrumentId(track.instrument); | ||
| if (renderInstrumentId !== track.instrument) substitutedTracks.push(track.name); |
There was a problem hiding this comment.
SUGGESTION: The substitution notice reports track.name, so two tracks that happen to share a name will be reported as one. Using track.id (or ${track.name} (${track.id})) makes the message unambiguous.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| const absoluteTicks = clip.startBar * BEATS_PER_BAR * TICKS_PER_BEAT + note.startTick; | ||
| const position = ticksToTransportPosition(absoluteTicks); | ||
| const eventId = transport.schedule((time) => { | ||
| const bpm = transport.bpm.value; |
There was a problem hiding this comment.
WARNING: scheduleTrackNotes reads note.durationTicks and note.startTick without validating they're non-negative. A negative durationTicks would compute a negative durationSeconds, which triggerAttackRelease will reject or, worse, accept silently in some engines. A negative startTick would schedule the note before its clip's bar. Add an assertion or clamp at the model layer so the offline renderer can't propagate garbage.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (4 files)
Fix these issues in Kilo Cloud Previous Review Summary (commit 1de58f7)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 1de58f7)Status: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Files Reviewed (4 files)
Reviewed by minimax-m3 · Input: 22.2K · Output: 832 · Cached: 73.5K |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
desktop/src/apps/musicstudio/wav-export.test.ts (1)
210-253: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winRound-trip test doesn't exercise the negative/clamped PCM encoding branches.
The
audioBufferToWavBlobencoder uses an asymmetric scale (sample < 0 ? sample * 0x8000 : sample * 0x7fff) and clamps values to[-1, 1]. The test data includes a negative extreme (right[1] = -1) and boundary values (left[3] = 1), but assertions only round-tripfirstLeft,firstRight(both 0) andsecondLeft(0.5) — the negative-scale branch and clamping logic are never independently verified.Consider adding an assertion for the negative sample (e.g.,
secondRight) and/or a value outside[-1, 1]to confirm clamping works as intended.✅ Suggested additional assertions
const secondLeft = view.getInt16(48, true) / 0x7fff; expect(secondLeft).toBeCloseTo(0.5, 3); + // right[1] = -1 exercises the negative-scale (0x8000) branch. + const secondRight = view.getInt16(50, true) / 0x8000; + expect(secondRight).toBeCloseTo(-1, 3); }); });🤖 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 `@desktop/src/apps/musicstudio/wav-export.test.ts` around lines 210 - 253, The `audioBufferToWavBlob` round-trip test currently misses coverage for the negative PCM scaling and clamping paths. Update the `audioBufferToWavBlob` test in `wav-export.test.ts` to assert the encoded value for a negative sample such as the second right-channel frame, and add a sample outside [-1, 1] to verify clamping behavior. Keep the existing header checks and extend the frame-decoding assertions so both the asymmetric negative encoding branch and clamp logic are exercised.
🤖 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.
Inline comments:
In `@desktop/src/apps/musicstudio/wav-export.ts`:
- Around line 84-104: The export guard in renderSongToWav currently checks
computeSongDurationSeconds(song) for all tracks, but only audibleTracks(song)
are actually scheduled, so muted/filtered-out songs can render a silent WAV
without error. Update renderSongToWav to base the “no notes to export” check on
the audible subset (for example by deriving duration from tracks or explicitly
verifying any audible track has schedulable notes before Tone.Offline runs), and
keep the existing scheduling logic in scheduleTrackNotes/buildSynthInstrument
unchanged.
- Around line 156-165: The object URL in exportSongWavFile is being revoked too
early, which can race the browser download kickoff. Update exportSongWavFile so
the URL created with URL.createObjectURL is only cleaned up after the download
has had time to start, and keep the anchor click/download flow intact while
delaying URL.revokeObjectURL until after that delay.
---
Nitpick comments:
In `@desktop/src/apps/musicstudio/wav-export.test.ts`:
- Around line 210-253: The `audioBufferToWavBlob` round-trip test currently
misses coverage for the negative PCM scaling and clamping paths. Update the
`audioBufferToWavBlob` test in `wav-export.test.ts` to assert the encoded value
for a negative sample such as the second right-channel frame, and add a sample
outside [-1, 1] to verify clamping behavior. Keep the existing header checks and
extend the frame-decoding assertions so both the asymmetric negative encoding
branch and clamp logic are exercised.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 21213471-646a-430c-ba56-899ef1ca3cd0
📒 Files selected for processing (4)
desktop/src/apps/MusicStudioApp.tsxdesktop/src/apps/musicstudio/audio-engine.tsdesktop/src/apps/musicstudio/wav-export.test.tsdesktop/src/apps/musicstudio/wav-export.ts
| export async function exportSongWavFile(song: Song): Promise<RenderResult> { | ||
| const result = await renderSongToWav(song); | ||
| const url = URL.createObjectURL(result.blob); | ||
| const a = document.createElement("a"); | ||
| a.href = url; | ||
| a.download = `${slugify(song.name)}.wav`; | ||
| a.click(); | ||
| URL.revokeObjectURL(url); | ||
| return result; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== File context =="
git ls-files 'desktop/src/apps/musicstudio/wav-export.ts'
echo
sed -n '130,190p' desktop/src/apps/musicstudio/wav-export.ts
echo
echo "== Search for similar blob-download patterns =="
rg -n "createObjectURL|revokeObjectURL|download =" desktop/src -g '!**/node_modules/**' || trueRepository: jaylfc/taOS
Length of output: 5242
🌐 Web query:
MDN URL.revokeObjectURL download anchor click asynchronous download timing
💡 Result:
When using URL.createObjectURL() for a file download via an anchor element's click() method, there is no standardized, browser-agnostic event to signal when a download has completed, making the timing of URL.revokeObjectURL() challenging [1][2]. If you call URL.revokeObjectURL() immediately after anchor.click(), the download may fail in some browsers or environments because the browser might not have finished fetching the data from the Blob object before the URL is invalidated [3][4][5]. Because click() triggers the download process asynchronously, the browser needs the Blob URL to remain valid until the internal download operation has at least begun and initialized its stream [4][2]. To handle this safely, common practices include: 1. Using a setTimeout() delay: A common workaround is to revoke the URL after a short delay (e.g., setTimeout(() => URL.revokeObjectURL(url), 1000)), which allows the browser sufficient time to begin the download process before the URL is revoked [1][4][2][5]. 2. Relying on document lifecycle: Browsers automatically release object URLs when the document is unloaded [6][2]. If memory constraints are not immediate or critical for your application, you may choose not to revoke the URL explicitly, letting the browser handle cleanup automatically [6][2]. 3. For browser extensions: If you are working within a browser extension, you can use specialized APIs like downloads.onChanged to listen for the specific completion of the download and revoke the URL reliably once the operation is finished [7]. There is no "perfect" synchronous way to know when an anchor-based download completes, so using a small delay is the most widely used approach for web pages [1][2]. Avoid revoking the URL immediately in the same synchronous execution block as the click event [4][5].
Citations:
- 1: https://stackoverflow.com/questions/79920765/at-what-specific-point-should-url-revokeobjecturl-be-called
- 2: https://stackoverflow.com/questions/37240551/how-can-i-revoke-an-object-url-only-after-its-downloaded
- 3: https://issues.chromium.org/issues/41380177
- 4: https://stackoverflow.com/questions/54156588/revokeobjecturl-not-work-in-mozilla-firefox-and-microsoft-edge
- 5: https://bugzilla.mozilla.org/show_bug.cgi?id=2005952
- 6: https://developer.mozilla.org/en-US/docs/Web/URI/Reference/Schemes/blob
- 7: https://discourse.mozilla.org/t/how-to-revoke-an-object-url/65479
Delay object URL cleanup until after the download starts
Revoking the blob URL in the same tick as a.click() can race the browser’s download start and leave the export empty or failed in some environments.
Proposed fix
const url = URL.createObjectURL(result.blob);
const a = document.createElement("a");
a.href = url;
a.download = `${slugify(song.name)}.wav`;
a.click();
- URL.revokeObjectURL(url);
+ setTimeout(() => URL.revokeObjectURL(url), 0);
return result;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function exportSongWavFile(song: Song): Promise<RenderResult> { | |
| const result = await renderSongToWav(song); | |
| const url = URL.createObjectURL(result.blob); | |
| const a = document.createElement("a"); | |
| a.href = url; | |
| a.download = `${slugify(song.name)}.wav`; | |
| a.click(); | |
| URL.revokeObjectURL(url); | |
| return result; | |
| } | |
| export async function exportSongWavFile(song: Song): Promise<RenderResult> { | |
| const result = await renderSongToWav(song); | |
| const url = URL.createObjectURL(result.blob); | |
| const a = document.createElement("a"); | |
| a.href = url; | |
| a.download = `${slugify(song.name)}.wav`; | |
| a.click(); | |
| setTimeout(() => URL.revokeObjectURL(url), 0); | |
| return result; | |
| } |
🤖 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 `@desktop/src/apps/musicstudio/wav-export.ts` around lines 156 - 165, The
object URL in exportSongWavFile is being revoked too early, which can race the
browser download kickoff. Update exportSongWavFile so the URL created with
URL.createObjectURL is only cleaned up after the download has had time to start,
and keep the anchor click/download flow intact while delaying
URL.revokeObjectURL until after that delay.
- Reject a render when no audible track has notes (all-muted or empty) instead of inferring emptiness from the duration, so an all-muted song no longer bounces 2s of silence. - Replace the ToneAudioBuffer.get() non-null assertion with an explicit check that routes a missing render buffer to the friendly error path. - Clamp note start/duration ticks to non-negative in scheduleTrackNotes so a malformed note can't produce a negative transport position or duration in either the live engine or the offline bounce. Docs-Reviewed: source-only changes inside the existing Music Studio app; no app added or removed.
1 participant
Summary
Music Studio could play a song live and export MIDI +
.taosongJSON, but had no way to produce an actual audio file. This adds an offline render to a downloadable WAV.desktop/src/apps/musicstudio/wav-export.ts:renderSongToWav(song)renders viaTone.Offline, encodes the result as 16-bit PCM WAV (RIFF/WAVE/fmt/data header, interleaved samples, no new dependency), andexportSongWavFile(song)triggers a browser download of<songname>.wav.desktop/src/apps/musicstudio/audio-engine.ts: extractedscheduleTrackNotesfrom a privateAudioEnginemethod into a top-level exported function, and exportedbuildSynthInstrumentandInstrumentHandle.Offline render approach
Tone.Offlineswaps Tone's global context for anOfflineAudioContextfor the duration of its callback, so calls toTone.getTransport(),new Tone.Channel(...), etc. inside that callback resolve to the offline context automatically. That means the bounce reuses the exact same scheduling code the live engine uses (scheduleTrackNotes,buildSynthInstrument,volumeToDb) rather than a parallel re-implementation, so a rendered WAV matches live playback.Duration is computed from the last note's end tick across every track (regardless of mute/solo, matching how long the transport actually runs) plus a 2s release tail so synth envelopes aren't cut off.
Only audible tracks are scheduled into the render: if any track is soloed, only soloed tracks; otherwise every unmuted track. Muted/non-soloed tracks are skipped entirely rather than scheduled-and-silenced, which also means they don't spend render time instantiating instruments.
smplr-in-offline fallback
Sampled (smplr) instruments fetch sample audio from a CDN and drive playback off the real wall clock -- neither works inside an
OfflineAudioContext. Tracks using a sampled instrument (Rhodes Mk I, Sub 808) are rendered with their closest Tone.js synth substitute instead (sampled-piano->synth-keys,sampled-bass->synth-bass). The render result reports which tracks were substituted, and the UI surfaces a one-line notice rather than silently changing the sound.WAV encoding
Standard 16-bit PCM WAV: 44-byte RIFF/WAVE/fmt/data header, samples clamped to [-1, 1] and interleaved per channel, no external encoder library.
UI
Added a "Download .wav" action next to the existing MIDI/JSON export buttons in the Export view, with a rendering/progress state (spinner + disabled button while the offline render runs) and an
aria-livestatus region for the substitution notice or a render error.Test plan
cd desktop && npx vitest run src/apps/musicstudio-- 56 tests passing (newwav-export.test.tscovers duration calculation, Tone.Offline invocation/duration, solo/mute scheduling filters, sampled-instrument fallback + substitution reporting, and WAV header byte correctness against a synthetic buffer)npx vitest run src/apps/MusicStudioApp.test.tsx-- 8 tests passingnpm run build(tsc -b && vite build) -- 0 TypeScript errorsSummary by CodeRabbit
New Features
Bug Fixes