Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe exporter now normalizes audio fallback paths before audio finalization and native audio-plan construction. Tests cover Windows paths, sidecar deduplication, embedded and silent sources, native-to-WebCodecs retry, and offline audio preparation. ChangesAudio fallback routing
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix · Severity of issue fixed: Medium Suggested reviewers: Merge Risk: ⚪ Minimal · up to The updated export paths consistently preserve embedded and sidecar audio routing, with no identified merge-blocking risk. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
1 participant
Pull Request Template
In
src/lib/exporter/modernVideoExporter.ts, compute the browser audio source list from the loadedvideoInfousing the existinggetNativeAudioFallbackPathsnormalization, which includes the local video when it has embedded audio and companion files are present. Pass that normalized list into the existingAudioProcessor.processcall and its source-presence guard, preserving delay metadata, source settings, clips, trims, and speed edits; if the helper is renamed to describe its shared use, update its existing native caller in this same file. This causes the existing production routing inaudioEncoder.tsto recognize embedded audio plus a microphone sidecar and use the established offline mixer instead of the single-sidecar fast path, without changing the routing policy or introducing a helper only used by tests.The reporter hears system and microphone audio in the editor but gets an MP4 without any audio stream, confirmed with ffprobe, across Recordly versions through 1.3.5 on Windows 7 using VxKex-NEXT. They also report Lightning Export failure, but the bundle supplies only an external screenshot URL, not its error text. A second human reports on September 15 that Windows 11 exports retain microphone audio while losing desktop audio. The current clone contains a deterministic source-routing mismatch that can produce that second symptom, although neither reporter supplied a project establishing that this is their exact execution path.
Fixes #792
Description
In
src/lib/exporter/modernVideoExporter.ts, compute the browser audio source list from the loadedvideoInfousing the existinggetNativeAudioFallbackPathsnormalization, which includes the local video when it has embedded audio and companion files are present. Pass that normalized list into the existingAudioProcessor.processcall and its source-presence guard, preserving delay metadata, source settings, clips, trims, and speed edits; if the helper is renamed to describe its shared use, update its existing native caller in this same file. This causes the existing production routing inaudioEncoder.tsto recognize embedded audio plus a microphone sidecar and use the established offline mixer instead of the single-sidecar fast path, without changing the routing policy or introducing a helper only used by tests.Motivation
Covered in the summary above.
Type of Change
Related Issue(s)
Not applicable to this change.
Screenshots / Video
No user-visible surface changes in this PR, so there is nothing to show.
Testing Guide
Regression: embedded desktop audio plus only
C:\recordly\recording.mic.wavin the original fallback list, no edits, default source gains, and browser AAC support available. Public browser export must pass video plus microphone paths into the real audio routing boundary; the processor must mix both instead of demuxing only the microphone.Reuse the existing audio preparation harness with distinguishable embedded and microphone buffers, asserting both reach the mix. Assert the WAV case goes through
renderAndMuxOfflineAudio, notloadAudioFileDemuxer; do not test only a newly extracted helper.Simulate native export failure followed by the existing browser retry with the same metadata and companion list. Assert the retried export retains both sources and does not retry indefinitely.
No embedded audio plus one mic sidecar must retain the direct single-sidecar path. Embedded-only and genuinely silent source videos must retain their current export behavior.
A video already present in the fallback list, including Windows file-URL/path representations, must not be added twice. Existing system+mic companions must retain dedicated-track precedence and avoid double-mixing an embedded copy.
Preserve explicit companion start delays, trim/speed edits, and source mute/volume settings when passing the normalized list. Rerun existing tests covering cancellation and clip mute; no new behavior is needed for these cases.
Validation during implementation:
npm test -- src/lib/exporter/modernVideoExporter.fallback.test.ts src/lib/exporter/audioEncoder.test.ts src/lib/exporter/modernVideoExporter.nativeStaticLayout.test.ts src/lib/exporter/sourceTrackRoutingPolicy.test.ts src/lib/exporter/sourceAudioFallback.test.ts, followed by the repository TypeScript check using installed tooling and focused lint. Planning did not install dependencies or run tests.When a suitable runtime is available, export a short local video with an embedded system tone and a separate microphone tone through the browser route; ffprobe must report an audio stream and playback must contain both tones without doubled gain or timing drift. An audio stream alone is insufficient proof of retaining desktop audio. Verify both buffer and file-backed output using existing export modes. Do not claim Windows 7 or Lightning reproduction without that environment and the missing error details.
Checklist
Not verified: this needs a person on the named hardware or environment.
Thank you for contributing!
Summary by CodeRabbit