feat(js/publish): publish files by demuxing them, not by capturing a MediaStreamTrack - #2541
feat(js/publish): publish files by demuxing them, not by capturing a MediaStreamTrack#2541kixelated wants to merge 3 commits into
Conversation
…track Publishing a file is a transcode, not a capture. Routing it through a <video> element and scraping tracks back out via captureStream is what caused the Safari gaps, the lost timestamps, and the compositor coupling. Demux and decode the file directly with mediabunny instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @kixelated, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
Warning Review limit reached
Next review available in: 4 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: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughAdds mediabunny-based file decoding with synchronized, looping sample streams for image, video, and audio sources. Audio now uses a shared PCM abstraction supporting worklet and decoded samples, gain control, sample-rate conversion, and updated AAC/Opus framing. Video sources now support either stream tracks or frame streams, with corresponding capture, encoding, and preview updates. New source guards, timeline behavior, resampling tests, and type tests are included. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches✨ Simplify code
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.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
js/publish/src/audio/resampler.test.ts (1)
9-16: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the no-op
/ 1in the timestamp calculation.Harmless, but
Math.round(((offset / rate) * 1_000_000) / 1)divides by 1 for no effect.🧹 Proposed cleanup
- timestamp: micro(Math.round(((offset / rate) * 1_000_000) / 1)), + timestamp: micro(Math.round((offset / rate) * 1_000_000)),🤖 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 `@js/publish/src/audio/resampler.test.ts` around lines 9 - 16, Remove the redundant “/ 1” operation from the timestamp calculation in tone, preserving the existing Math.round conversion and resulting timestamp 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 `@js/publish/src/audio/resampler.test.ts`:
- Around line 9-16: Remove the redundant “/ 1” operation from the timestamp
calculation in tone, preserving the existing Math.round conversion and resulting
timestamp behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8f8ee6df-d65c-4cc0-bc8c-4b03437f0ba9
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
js/publish/package.jsonjs/publish/src/audio/encoder.tsjs/publish/src/audio/pcm.tsjs/publish/src/audio/resampler.test.tsjs/publish/src/audio/resampler.tsjs/publish/src/audio/types.test.tsjs/publish/src/audio/types.tsjs/publish/src/element.tsjs/publish/src/preview.tsjs/publish/src/source/camera.tsjs/publish/src/source/file.tsjs/publish/src/source/timeline.test.tsjs/publish/src/source/timeline.tsjs/publish/src/support/index.tsjs/publish/src/video/capture.tsjs/publish/src/video/encoder.tsjs/publish/src/video/processor.tsjs/publish/src/video/types.test.tsjs/publish/src/video/types.ts
💤 Files with no reviewable changes (1)
- js/publish/src/support/index.ts
…uity Framer derives every output timestamp by counting samples forward from its first input, ignoring later ones. A file whose audio is shorter than its video leaves a gap at each loop wrap, which was silently swallowed: the encoded stream slid earlier by the gap and stayed there, compounding on every pass until audio and video were visibly apart. Detect an input timestamp that leaves the running timeline and start a new segment at it, dropping the partial frame that belonged to the old one. Also install the WebCodecs polyfill before probing decodability, since canDecode() reports false rather than throwing when AudioDecoder is missing, which would drop the audio track before the polyfill could cover it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@js/publish/src/audio/framer.test.ts`:
- Around line 89-103: Update the test around framer.push and the “keeps counting
through jitter below the threshold” case so the nudged timestamp completes and
flushes the pending 960-sample frame before asserting. Add enough input samples
or an explicit flush using the existing framer API, then assert the emitted
frame timestamp remains 1,000,000 microseconds, ensuring the test fails if
jitter handling resets the timeline.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d219afb2-5f37-443a-893c-be24cad2360c
📒 Files selected for processing (4)
js/publish/src/audio/framer.test.tsjs/publish/src/audio/framer.tsjs/publish/src/audio/resampler.test.tsjs/publish/src/source/file.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- js/publish/src/audio/resampler.test.ts
- js/publish/src/source/file.ts
The nudged push added 128 samples to an empty 960-sample frame buffer, so it emitted nothing and the assertion loop ran zero times. The test passed whether or not the discontinuity check worked at all. Push a full frame so one is emitted, and assert on it directly. It now fails if the threshold is tightened enough to re-anchor on jitter. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 participant
Closes #2532. Closes #2374 (same work, older duplicate; #2374 proposed MediaBunny, #2532 restated the problem in more detail).
Summary
js/publish/src/source/file.tspublished a local file by playing it in a<video>element and turning that back into a live capture track. Publishing a file is a transcode, not a capture, and forcing it through a capture pipe is what caused the problems. It now demuxes and decodes the file directly with mediabunny, drivingVideoDecoder/AudioDecoderand pacing the output against a wall clock.Picked mediabunny over mp4box.js/libav.js (the alternatives floated in #2532): zero dependencies, tree-shakable, reads MP4/MOV/WebM/MKV/MP3/WAV/Ogg/ADTS/FLAC/MPEG-TS, and it already wraps WebCodecs decoding with a seek/iterate API, which is exactly the demux + decode + loop shape needed here.
What this fixes, per #2532:
captureStreamorcreateMediaElementSourceany more, so the video-only fallback and the "this browser can't capture audio from files" error are both gone.performance.now()epoch, the same one live capture uses, instead of the canvas track's frozentimestamp: 0.<video>element, so a window that isn't composited no longer freezes the content (see the caveat below).Looping restarts the sinks with a duration offset. Audio and video share one
Timelineso they restart together and stay in sync rather than drifting apart by the difference in their track durations. Images keep working through the same frame-stream shape (a pacedVideoFrameper tick off anImageBitmap), so they are no longer compositor-bound either.Rotation metadata is honored:
toVideoFrame()drops it, so a rotated track draws through anOffscreenCanvasto bake it into the pixels. Sideways phone footage would otherwise publish sideways.Root causes found while verifying in the browser
Two of these only reproduce with this change, and both are fixed here rather than papered over:
EncodingError: Input audio buffer is incompatible with codec parameters. I had assumed WebCodecs resamples mismatched input, which the comment injs/hang/src/util/opus.ts("Chrome hides this by ignoring the configured rate") suggests. That is true of the decode side only: Chrome's Opus encoder rejects 44.1kHz input at a 48kHz config outright. Opus cannot carry 44.1kHz at all and 44.1kHz is the most common file rate, so conversion is mandatory. Addedaudio/resampler.ts, which carries interpolation state across chunks so seams aren't audible; resampling each chunk independently would tick once per chunk. It sits in the encode loop, so it rebuilds with the config instead of being pinned to the one-shot stream.drawImage: The VideoFrame has been closedflooding the console.Preview.Renderercopied the frame througheffect.proxy, so it held a frame one microtask behindCapture, which closes a frame when it replaces it. A camera hides this because its 33ms gap lets the microtask queue drain; bursty file delivery fires it constantly. The renderer now holds the signal and reads the current frame, which is never a closed one. Pre-existing race, exposed by this change.Muting a file used to silence it permanently. The audio encoder gated its whole source setup on
enabled, which for a stream source cancelled the one-shotReadableStream; unmuting then read a dead stream. A sample source now stays running while muted (there is no device to release, and stalling it would put audio behind video on unmute), and#runConfiggates the catalog onenabledexplicitly, mirroring what the video encoder already does.Public API changes
Additive only, all in
@moq/publish(not adev-branch package), no wire or catalog format change:Video.Sourcewidens fromStreamTracktoStreamTrack | FrameSource; newVideo.FrameSourceandVideo.isStreamTrack().Audio.Sourcewidens to include a newAudio.SampleSource; newAudio.isSampleSource()andAudio.sourceKind().Audio.normalizeSource's parameter narrows fromSourcetoStreamTrack | SourceConfig. Not a break: that union is exactly whatSourcewas before this change, so every existing call still typechecks.Audio.Resampleris new but internal to the package (not re-exported fromaudio/index.ts); same foraudio/pcm.tsandsource/timeline.ts.Behavior change: a
<video>preview element can't show a decoded-file source (srcObjectonly takes aMediaStream). It now hides itself and warns to use a<canvas>, which renders both.demo/webalready uses a<canvas>.Caveat: background-tab timer throttling
Pacing uses
setTimeouton the main thread, which browsers clamp to ~1s for a hidden page. Measured in a hidden tab: output stays correct (monotonic timestamps, exact 41.67ms cadence, no dropped or duplicated frames, clean loop wrap) but delivery batches into ~1s bursts. That is strictly better than the old canvas path, which froze on the same throttle and published duplicate frames, but it is not the fully unthrottled result. Moving the decode + pacing loop into a worker (as #2532 sketches) is the follow-up; I left it out rather than guess at whether worker timers are throttled the same way.Follow-up fixes from adversarial review
Framerderives every output timestamp by counting samples forward from its first input and ignores later ones. A file whose audio is shorter than its video leaves a gap at each loop wrap, so the encoded stream slid earlier by the gap and stayed there, compounding on every pass. Fixed inFramer(the layer that owns timestamp derivation) rather than in the file source, so it also covers a stalled mic. Regression tests cover a bare gap, the composed resampler→framer path with a 2s wrap gap, and that sub-millisecond jitter does not re-anchor; all three fail with the resync removed.canDecode()returnsfalserather than throwing whenAudioDecoderis missing, so an engine the polyfill could have covered would have its audio track dropped first. The polyfill now runs before the probe. Note this does not extend AAC coverage: the bundled variant is@libav.js/variant-opus-af(Opus only, with a standingTODO build with AAC supportinjs/hang/src/util/libav.ts), so widening that is a separate change to@moq/hang's bundle.Re-verified in the browser after both fixes: 50.17 audio frames/s and exactly 24.0 video fps across two loop wraps with 0 console errors, and a purpose-built 5s-video/3s-audio file publishes across three wraps at 30.4 audio frames/s (the expected 60% duty cycle) with 0 errors.
Test plan
Unit (
bun test js/publish, 48 pass): newresampler.test.ts(rate conversion up and down, exact linear-ramp reproduction, chunked output identical to unchunked, channel preservation, short-chunk handling),timeline.test.ts(loop offsets, monotonic wrap, unusable durations don't loop), plustypes.test.tsfor both source-shape discriminators.just js checkandjust js fixclean.Browser (Chrome, via
just devagainst the local relay, publishing to a real<moq-watch>subscriber). Test media generated with ffmpeg: 5s 640x480@24 H.264 + 44.1kHz stereo AAC, an audio-only 44.1kHz.m4a, and a PNG.FrameSource, frameRate 24 detected from the container (not 30)opus@ 48000 from 44.1kHz source, resampledNot verified: Safari and Firefox (no driver in this environment), so the Safari audio fix is verified by construction rather than by running it. Rotated-file handling is likewise unexercised.
Cross-Package Sync: no rows apply. No wire, catalog,
moq-ffi, or CLI surface changes;doc/documentssource="file"as a source but nothing about the removedcaptureStreampath.(written by Opus 5)