feat: MSE-based Media Player for Browser Compatibility #797
Conversation
WalkthroughThis change introduces MediaSource Extensions (MSE) support to the hang video publishing and playback system. It adds a new MoqClient class for MOQ stream connections, a MoqMsePlayer web component for MSE-based playback and a watch-mse demo page. On the publishing side, new PublishFormat variants (Fmp4 and Fmp4Mse) and an ImportMode enum enable frame-by-frame or complete-segment fMP4 exports. The Fmp4 importer is extended to support both modes, managing per-track init segments and assembling segments for MSE compatibility. Build config and tooling are updated to include the new demo and publish flows. Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![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 (5)
js/hang-demo/src/mse/moq-mse-player.ts (2)
585-593: Consider warning before queue reaches critical threshold.The queue size check at 1000 items is good for preventing memory exhaustion, but clearing the entire queue loses all buffered data. Consider logging warnings at lower thresholds (e.g., 500) to help diagnose backpressure issues before they become critical.
1096-1113: Hardcoded frame duration assumptions may cause issues with non-standard content.The TREX patching assumes 30fps video and 1024-sample AAC frames. While these are common defaults, content with different frame rates (24fps, 60fps) or audio codecs (Opus uses 960 samples) may have timing issues.
Consider extracting frame rate from the catalog or stsd box if available, or at minimum document this limitation in the code comments.
js/hang-demo/src/mse/moq-client.ts (1)
63-90: Consider closing the catalog track subscription on timeout.When the catalog fetch times out (line 77-78), the
catalogTracksubscription remains open. While it may be garbage collected eventually, explicitly closing it would be cleaner and prevent potential resource leaks.🔎 Proposed fix
private async fetchCatalog(): Promise<Catalog.Root | null> { if (!this.broadcast) return null; console.log("[MoqClient] Fetching catalog.json..."); const catalogTrack = this.broadcast.subscribe("catalog.json", 100); try { // Wait for catalog with timeout const frame = await Promise.race([ catalogTrack.readFrame(), new Promise<null>((resolve) => setTimeout(() => resolve(null), 5000)) ]); if (!frame) { console.warn("[MoqClient] Catalog fetch timed out"); + catalogTrack.close?.(); return null; } // Use the catalog decode function from @moq/hang const catalog = Catalog.decode(frame); console.log("[MoqClient] Received catalog:", catalog); return catalog; } catch (error) { console.warn("[MoqClient] Error fetching catalog:", error); + catalogTrack.close?.(); return null; } }rs/hang-cli/src/publish.rs (1)
91-97: Consider extracting the stdin reading logic to reduce duplication.The
Fmp4arm duplicates the stdin reading pattern from theDecoderarm. This could be extracted into a helper, but given the method name difference (decode_streamvsdecode), the current approach is acceptable.🔎 Optional: Shared helper approach
If both
DecoderandFmp4implemented a common trait withis_initialized()anddecode()methods, the stdin reading logic could be unified. However, this may be over-engineering for just two variants.Also applies to: 113-119
rs/hang/src/import/fmp4.rs (1)
650-692: Consider extracting shared keyframe detection logic.The keyframe detection logic (flags parsing, audio sync on video keyframe, 10-second fallback) is duplicated between
extract()andextract_segment(). Extracting this into a helper would improve maintainability.🔎 Example helper signature
fn is_keyframe( &mut self, track_id: u32, flags: u32, handler: &[u8; 4], timestamp: Timestamp, moov: &Moov, ) -> bool { // Shared keyframe detection logic }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
js/hang-demo/src/index.htmljs/hang-demo/src/mse/moq-client.tsjs/hang-demo/src/mse/moq-mse-player.tsjs/hang-demo/src/watch-mse.htmljs/hang-demo/src/watch-mse.tsjs/hang-demo/vite.config.tsjustfilers/hang-cli/src/publish.rsrs/hang/src/import/fmp4.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
In Rust crates, tests should be integrated within source files using inline test modules
Files:
rs/hang-cli/src/publish.rsrs/hang/src/import/fmp4.rs
🧠 Learnings (2)
📚 Learning: 2025-12-10T04:00:14.871Z
Learnt from: CR
Repo: moq-dev/moq PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-10T04:00:14.871Z
Learning: Core protocol implementation in the `moq` layer must be generic and not contain media-specific logic; CDN/relay does not know anything about media
Applied to files:
js/hang-demo/src/mse/moq-client.ts
📚 Learning: 2025-12-20T13:22:03.497Z
Learnt from: Frando
Repo: moq-dev/moq PR: 794
File: justfile:150-156
Timestamp: 2025-12-20T13:22:03.497Z
Learning: In moq-relay, WebTransport connections automatically prepend the URL path (e.g., `/anon`) as a prefix to all broadcasts. However, raw QUIC and iroh connections have no HTTP layer, so there's no way to send a path. To match the expected broadcast path for the web UI, the prefix must be manually added to the broadcast name (e.g., `anon/{{name}}`) when publishing via iroh or raw QUIC.
Applied to files:
js/hang-demo/src/mse/moq-client.ts
🧬 Code graph analysis (5)
rs/hang-cli/src/publish.rs (3)
rs/hang/src/import/fmp4.rs (2)
with_mode(109-124)new(97-99)rs/hang/src/import/hls.rs (3)
new(37-39)new(104-110)new(115-132)rs/hang/src/import/decoder.rs (1)
new(65-73)
js/hang-demo/src/watch-mse.ts (1)
js/lite/src/ietf/publisher.ts (1)
path(54-74)
rs/hang/src/import/fmp4.rs (2)
rs/hang-cli/src/publish.rs (1)
new(38-74)rs/hang/src/import/decoder.rs (1)
new(65-73)
js/hang-demo/src/mse/moq-mse-player.ts (1)
js/hang-demo/src/mse/moq-client.ts (1)
MoqClient(14-181)
js/hang-demo/src/mse/moq-client.ts (4)
js/lite/src/util/error.ts (1)
error(2-4)js/lite/src/connection/established.ts (1)
Established(6-14)rs/moq-native/src/server.rs (1)
resolve(444-459)js/hang/src/frame.ts (1)
group(147-192)
🪛 ast-grep (0.40.3)
js/hang-demo/src/mse/moq-mse-player.ts
[warning] 949-961: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: debugEl.innerHTML = <div>Latency: ${latency.toFixed(3)}s</div> <div>Buffer: ${bufferedStart.toFixed(2)} - ${bufferedEnd.toFixed(2)}s (${(bufferedEnd - bufferedStart).toFixed(2)}s)</div> <div>Ranges: V=${vRanges} A=${aRanges}</div> <div>Rate: ${this.video.playbackRate.toFixed(2)}x</div> <div>Res: ${this.video.videoWidth}x${this.video.videoHeight}</div> <div>Time: ${this.video.currentTime.toFixed(2)}s</div> <div>State: ${this.connectionState}</div> <div>Queue: V=${vQ} A=${aQ}</div> <div>Init: V=${this.videoInitReceived ? "\u2713" : "..."} A=${this.audioInitReceived ? "\u2713" : "..."}</div> <div>Ready: ${this.video.readyState} | Net: ${this.video.networkState}</div> <div>Dropped: ${dropped} / ${total}</div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 949-961: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: debugEl.innerHTML = <div>Latency: ${latency.toFixed(3)}s</div> <div>Buffer: ${bufferedStart.toFixed(2)} - ${bufferedEnd.toFixed(2)}s (${(bufferedEnd - bufferedStart).toFixed(2)}s)</div> <div>Ranges: V=${vRanges} A=${aRanges}</div> <div>Rate: ${this.video.playbackRate.toFixed(2)}x</div> <div>Res: ${this.video.videoWidth}x${this.video.videoHeight}</div> <div>Time: ${this.video.currentTime.toFixed(2)}s</div> <div>State: ${this.connectionState}</div> <div>Queue: V=${vQ} A=${aQ}</div> <div>Init: V=${this.videoInitReceived ? "\u2713" : "..."} A=${this.audioInitReceived ? "\u2713" : "..."}</div> <div>Ready: ${this.video.readyState} | Net: ${this.video.networkState}</div> <div>Dropped: ${dropped} / ${total}</div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Check
🔇 Additional comments (26)
js/hang-demo/vite.config.ts (1)
14-14: LGTM!The new
watch-msebuild input follows the established pattern for multi-page app configuration and correctly references the newwatch-mse.htmlpage.js/hang-demo/src/index.html (1)
40-40: LGTM!The new navigation link follows the existing pattern and provides clear labeling for the MSE player demo.
js/hang-demo/src/watch-mse.ts (1)
1-12: LGTM!The module correctly imports the MSE player component and provides a simple mechanism to override the broadcast path via URL query parameter. The null check on line 6 properly guards the attribute setting.
js/hang-demo/src/watch-mse.html (1)
1-105: LGTM!Well-structured demo page with clear documentation on usage, live sync strategies, and a helpful comparison table between WebCodecs and MSE approaches. The page correctly uses the Vite environment variable placeholder for the relay URL.
js/hang-demo/src/mse/moq-mse-player.ts (8)
15-41: LGTM!Well-organized state management with appropriate defaults for video/audio timescales. The processing timestamps for stuck detection (lines 25-26) are a good defensive pattern.
43-122: LGTM!Good Shadow DOM encapsulation with appropriate video element configuration. The
muted=truesetting correctly enables autoplay while bypassing browser policies. The stalled event handler's recovery logic (seeking forward 0.1s) is a reasonable approach.
304-343: LGTM!Good separation of video and audio SourceBuffers with proper error event listeners. The sequence mode is appropriate for live streaming, and the catalog deduplication check prevents re-processing.
377-401: LGTM!Correct QUIC VarInt parsing according to the specification. The segment type detection using MP4 box fourccs (
ftyp,moof,styp) is standard and appropriate.
605-665: LGTM!The queue processing logic is well-designed with stuck detection (2-second timeout), proper async waiting for buffer updates, and appropriate error handling for
QuotaExceededError. The performance timing for slow appends aids debugging.
728-752: LGTM!The sync offset calculation logic is correct, and the documentation clearly explains why the offset is computed but not applied in sequence mode (both tracks start at 0). This provides useful diagnostic information while deferring actual sync fixes to the server/encoder side.
949-962: Static analysis false positive: No XSS risk here.The static analysis tool flagged
innerHTMLusage, but all interpolated values are internally controlled numbers (fromtoFixed()) and internal state strings. No user input is incorporated, so there's no XSS vulnerability. The current approach is acceptable for a debug overlay.
1174-1191: LGTM!The tfdt parsing correctly handles both 32-bit and 64-bit baseMediaDecodeTime formats. The comment on line 1183 appropriately notes potential precision loss for very large 64-bit values when using JavaScript numbers.
js/hang-demo/src/mse/moq-client.ts (4)
4-22: LGTM!Clean interface design with optional callbacks for flexible integration. The nullable connection/broadcast state with
isRunningflag provides good lifecycle control.
24-61: LGTM!The connect flow is well-structured with proper error handling. The fallback to default track names ("video0", "audio1") when no catalog is available provides reasonable backwards compatibility.
137-166: LGTM!The nested loop structure correctly handles MOQ group/frame hierarchy. The
isRunningchecks in both loops ensure clean termination during disconnect, and error suppression when not running prevents noise during intentional shutdown.
168-180: LGTM!Clean disconnect implementation with correct ordering: setting
isRunning = falsefirst to signal loops to exit, then closing resources and nulling references.justfile (1)
149-165: LGTM!The new
pub-msetarget follows the established pattern from the existingpubtarget with the appropriate format switch tofmp4-mse. The comments clearly document the purpose.rs/hang-cli/src/publish.rs (3)
3-16: LGTM!The imports and
PublishFormatenum are properly extended with descriptive doc comments that clarify the use cases for each format variant.
25-29: LGTM!The new
Fmp4variant inPublishDecodercorrectly enables direct use of theFmp4importer with theSegmentsmode, bypassing theDecoderwrapper which defaults toFramesmode.
52-56: LGTM!The
Fmp4Msecase correctly instantiatesFmp4withImportMode::Segmentsfor MSE-compatible segment output.rs/hang/src/import/fmp4.rs (6)
9-28: LGTM!The
ImportModeenum is well-designed with clear documentation explaining the trade-offs between the two modes. The#[default]onFramesmaintains backward compatibility.
73-87: LGTM!The new fields for MSE segment mode are appropriately scoped and documented. The memory overhead is negligible in
Framesmode since the buffers remain empty.
89-124: LGTM!The constructor pattern with
new()delegating towith_mode()is clean and maintains backward compatibility.
136-176: LGTM!The decode method cleanly handles both modes with appropriate byte capture for segment assembly. The mode-based dispatch to
extract()vsextract_segment()is well-structured.
249-287: LGTM!The per-track init segment generation is correctly implemented. Each track gets its own
ftyp + moovcombination, which is required for MSE's separateSourceBufferapproach.
225-231: Both audio and video tracks correctly have priority 1 in MOQ's priority scheme. Equal track priorities are appropriate for live streaming without requiring mode-specific handling or additional documentation.
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
rs/hang/src/import/fmp4.rs (1)
860-1131: LGTM! Test module addresses past review requirements.The test module correctly fulfills the requirement from the past review to add unit tests with
#[cfg(test)]annotation. The tests effectively validate:
- Per-track moof/mdat extraction isolates track data
- Data offsets are correctly recalculated for per-track layout
- Moof contains exactly one traf for the target track
Optional: Consider adding test coverage for edge cases such as:
- Multiple truns per traf
- Tracks using default sample sizes from tfhd/trex
- Empty truns or trafs
As per coding guidelines, tests are appropriately placed in an inline test module within the source file.
js/hang-demo/src/mse/moq-mse-player.ts (1)
770-794:applySyncOffsetis misleading—it calculates but doesn't apply the offset.The method name suggests it applies a synchronization offset, but the implementation only calculates and logs the offset without actually applying it (lines 788-792). The comment explains this is intentional for sequence mode, but the name is confusing.
🔎 Suggested refactor
Consider renaming to better reflect its purpose:
-private applySyncOffset() { +private calculateAndLogSyncOffset() { if (this.firstVideoTimestamp === null || this.firstAudioTimestamp === null) return; if (this.syncApplied) return; if (!this.audioSourceBuffer) return; // Convert both to seconds using their respective timescales const videoStartSec = this.firstVideoTimestamp / this.videoTimescale; const audioStartSec = this.firstAudioTimestamp / this.audioTimescale; // Calculate offset: we want audio to start when video starts const offset = videoStartSec - audioStartSec; console.log( `[Sync] Video starts at ${videoStartSec.toFixed(3)}s (${this.firstVideoTimestamp}/${this.videoTimescale})`, ); console.log( `[Sync] Audio starts at ${audioStartSec.toFixed(3)}s (${this.firstAudioTimestamp}/${this.audioTimescale})`, ); console.log(`[Sync] Calculated offset: ${offset.toFixed(3)}s (NOT applying - sequence mode starts both at 0)`); // In sequence mode, both tracks start at 0 regardless of original media timestamps. // Applying an offset would break sync by delaying one track. // The real fix for audio/video timestamp mismatch is on the server/encoder side. this.syncApplied = true; }And update the call site on line 722:
- this.applySyncOffset(); + this.calculateAndLogSyncOffset();
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
js/hang-demo/src/mse/moq-client.tsjs/hang-demo/src/mse/moq-mse-player.tsrs/hang/src/import/fmp4.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
In Rust crates, tests should be integrated within source files using inline test modules
Files:
rs/hang/src/import/fmp4.rs
🧠 Learnings (1)
📚 Learning: 2025-12-10T04:00:14.871Z
Learnt from: CR
Repo: moq-dev/moq PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-10T04:00:14.871Z
Learning: Core protocol implementation in the `moq` layer must be generic and not contain media-specific logic; CDN/relay does not know anything about media
Applied to files:
js/hang-demo/src/mse/moq-client.ts
🧬 Code graph analysis (2)
js/hang-demo/src/mse/moq-mse-player.ts (1)
js/hang-demo/src/mse/moq-client.ts (1)
MoqClient(14-175)
js/hang-demo/src/mse/moq-client.ts (3)
js/lite/src/util/error.ts (1)
error(2-4)js/lite/src/connection/established.ts (1)
Established(6-14)js/hang/src/frame.ts (1)
group(147-192)
🪛 ast-grep (0.40.3)
js/hang-demo/src/mse/moq-mse-player.ts
[warning] 1008-1020: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: debugEl.innerHTML = <div>Latency: ${latency.toFixed(3)}s</div> <div>Buffer: ${bufferedStart.toFixed(2)} - ${bufferedEnd.toFixed(2)}s (${(bufferedEnd - bufferedStart).toFixed(2)}s)</div> <div>Ranges: V=${vRanges} A=${aRanges}</div> <div>Rate: ${this.video.playbackRate.toFixed(2)}x</div> <div>Res: ${this.video.videoWidth}x${this.video.videoHeight}</div> <div>Time: ${this.video.currentTime.toFixed(2)}s</div> <div>State: ${this.connectionState}</div> <div>Queue: V=${vQ} A=${aQ}</div> <div>Init: V=${this.videoInitReceived ? "\u2713" : "..."} A=${this.audioInitReceived ? "\u2713" : "..."}</div> <div>Ready: ${this.video.readyState} | Net: ${this.video.networkState}</div> <div>Dropped: ${dropped} / ${total}</div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 1008-1020: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: debugEl.innerHTML = <div>Latency: ${latency.toFixed(3)}s</div> <div>Buffer: ${bufferedStart.toFixed(2)} - ${bufferedEnd.toFixed(2)}s (${(bufferedEnd - bufferedStart).toFixed(2)}s)</div> <div>Ranges: V=${vRanges} A=${aRanges}</div> <div>Rate: ${this.video.playbackRate.toFixed(2)}x</div> <div>Res: ${this.video.videoWidth}x${this.video.videoHeight}</div> <div>Time: ${this.video.currentTime.toFixed(2)}s</div> <div>State: ${this.connectionState}</div> <div>Queue: V=${vQ} A=${aQ}</div> <div>Init: V=${this.videoInitReceived ? "\u2713" : "..."} A=${this.audioInitReceived ? "\u2713" : "..."}</div> <div>Ready: ${this.video.readyState} | Net: ${this.video.networkState}</div> <div>Dropped: ${dropped} / ${total}</div>
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Check
🔇 Additional comments (13)
rs/hang/src/import/fmp4.rs (5)
9-28: LGTM! Well-designed mode enum with clear documentation.The
ImportModeenum cleanly separates the two import strategies (WebCodecs frames vs MSE segments) with appropriate documentation and sensible defaults.
97-124: LGTM! Constructor pattern maintains backward compatibility.The delegation from
new()towith_mode()with a default ensures existing code continues to work while allowing new code to specify the import mode.
249-287: LGTM! Correct per-track init segment generation for MSE.This implementation correctly creates isolated init segments for each track (ftyp + single-track moov), which is required for MSE with separate SourceBuffers. The debug logging is helpful for troubleshooting.
711-717: LGTM! Per-track segment assembly correctly implemented.The
build_per_track_segmentmethod properly creates isolated moof/mdat pairs for each track:
- The moof contains only the current track's
traf(line 828-831)- The mdat contains only the current track's sample data (lines 762-795)
- The trun data_offsets are correctly recalculated for the per-track layout (lines 797-825)
This resolves the concern from the past review about sending complete moof/mdat to all tracks—the implementation now correctly isolates data per track for MSE compatibility with separate SourceBuffers.
Also applies to: 742-843
225-225: Verify that identical audio/video priority is intentional for both modes.Both audio and video tracks are assigned priority 1 (lines 214, 225, 231), regardless of the import mode. While this makes sense for MSE mode where tracks need tight synchronization, please confirm this doesn't negatively impact the existing Frames/WebCodecs mode if the priorities were previously different.
Also applies to: 231-231
js/hang-demo/src/mse/moq-mse-player.ts (8)
285-364: Good use of catalog-driven SourceBuffer initialization.The catalog handling correctly extracts codecs, validates support, creates separate SourceBuffers in sequence mode, and sets up appropriate event listeners. The
catalogReceivedflag prevents duplicate processing.
519-641: Data handling pipeline is well-structured.The
handleDatamethod correctly:
- Strips QUIC VarInt timestamps
- Detects and handles init segments (with duplicate prevention)
- Splits bundled init+media segments
- Captures timestamps for A/V sync calculation
- Drops segments before init is received (prevents MSE errors)
- Implements queue size limits (1000 segments) as a safety valve
The logic flow is appropriate for MSE segment handling in live streaming scenarios.
643-703: Robust queue processing with stuck-loop detection and QuotaExceededError recovery.The video queue processing correctly:
- Guards against missing SourceBuffer/catalog/init
- Detects stuck processing loops (2s timeout) and forces reset
- Waits for SourceBuffer to finish updating before appending
- Handles QuotaExceededError by triggering aggressive cleanup and retrying
- Drops problematic segments that cause other errors
- Uses isVideoProcessing flag to prevent concurrent processing
The performance measurement for slow appends (>50ms) is helpful for debugging.
796-858: Buffer append logic is correct, with helpful diagnostics.The
appendToBuffermethod properly:
- Validates SourceBuffer and MediaSource state
- Checks for updating state (redundant with caller checks, but good defense)
- Detects and logs segment types
- Handles TypeScript compatibility with BufferSource cast
- Uses Promise + event listener pattern for updateend
- Logs buffer ranges after append for debugging
The diagnostic logging (lines 848-851) is verbose but valuable during development.
860-886: Buffer cleanup removes one range at a time (by design).The cleanup logic removes old buffer ranges incrementally, exiting after each
remove()call (lines 876, 882) becauseSourceBuffer.remove()is asynchronous and triggers an update. Subsequent cleanups are handled by periodic calls fromcheckLiveEdge()(line 986, every 1s).This gradual approach is appropriate for live streaming and avoids overwhelming the buffer with multiple simultaneous remove operations.
888-990: Well-designed live-edge tracking with HLS.js-style catch-up logic.The implementation correctly:
- Detects and logs buffer fragmentation
- Handles paused state with autoplay recovery (respecting
userPaused)- Implements hard seeking for large latency (>5s) or out-of-sync conditions
- Uses playback rate adjustments (1.1x) for smooth catch-up
- Handles ahead-of-buffer scenarios by pausing and seeking back
- Updates debug overlay and triggers periodic buffer cleanup
The target latency of 2.0s (line 941) aligns with the PR's goal of ~2s latency for MSE-based playback.
992-1025: Debug overlay innerHTML usage is safe (internal data only).Static analysis flags
innerHTMLas a potential XSS risk, but all interpolated values are internal numbers, booleans, or controlled strings from the player state—no user input or external data. The debug overlay is also opt-in (requiresdebugattribute).While using
textContentor element creation would be more idiomatic, the current approach is safe for this debug-only feature.
1027-1316: MP4 parsing and patching utilities are well-implemented.The MP4 box parsing and init segment patching logic correctly:
- Traverses MP4 box hierarchy (moov → trak → mdia, mvex → trex)
- Extracts track IDs, types, and timescales
- Patches missing default durations in trex boxes (assumes 30fps for video, 1024 samples for audio)
- Stores timescales for A/V sync calculations
- Implements debug inspection with rate limiting to avoid log spam
The parsing handles both version 0 and version 1 boxes appropriately (e.g., 32-bit vs 64-bit timestamps in tfdt).
| private async fetchCatalog(): Promise<Catalog.Root | null> { | ||
| if (!this.broadcast) return null; | ||
|
|
||
| console.log("[MoqClient] Fetching catalog.json..."); | ||
| const catalogTrack = this.broadcast.subscribe("catalog.json", 100); | ||
|
|
||
| try { | ||
| // Wait for catalog with timeout | ||
| const frame = await Promise.race([ | ||
| catalogTrack.readFrame(), | ||
| new Promise<null>((resolve) => setTimeout(() => resolve(null), 5000)), | ||
| ]); | ||
|
|
||
| if (!frame) { | ||
| console.warn("[MoqClient] Catalog fetch timed out"); | ||
| return null; | ||
| } | ||
|
|
||
| // Use the catalog decode function from @moq/hang | ||
| const catalog = Catalog.decode(frame); | ||
| console.log("[MoqClient] Received catalog:", catalog); | ||
|
|
||
| return catalog; | ||
| } catch (error) { | ||
| console.warn("[MoqClient] Error fetching catalog:", error); | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
Catalog track resource may leak on timeout.
The catalogTrack subscription created on line 67 is never explicitly closed if the timeout occurs (line 73) or if an error is thrown (line 87). While the track may be closed when this.broadcast is closed later, it's better to explicitly clean up to avoid potential resource accumulation during reconnects.
🔎 Proposed fix
private async fetchCatalog(): Promise<Catalog.Root | null> {
if (!this.broadcast) return null;
console.log("[MoqClient] Fetching catalog.json...");
const catalogTrack = this.broadcast.subscribe("catalog.json", 100);
try {
// Wait for catalog with timeout
const frame = await Promise.race([
catalogTrack.readFrame(),
new Promise<null>((resolve) => setTimeout(() => resolve(null), 5000)),
]);
if (!frame) {
console.warn("[MoqClient] Catalog fetch timed out");
+ catalogTrack.close?.();
return null;
}
// Use the catalog decode function from @moq/hang
const catalog = Catalog.decode(frame);
console.log("[MoqClient] Received catalog:", catalog);
return catalog;
} catch (error) {
console.warn("[MoqClient] Error fetching catalog:", error);
return null;
+ } finally {
+ catalogTrack.close?.();
}
}🤖 Prompt for AI Agents
In js/hang-demo/src/mse/moq-client.ts around lines 63 to 90, the catalogTrack
subscription created on line 67 is not cleaned up on the timeout path or when an
exception occurs; ensure you always close the subscription: wrap the await
Promise.race(...) and subsequent decode in a try/finally (or add explicit
cleanup in each return path) and call the track's close/unsubscribe method
(e.g., catalogTrack.close() or catalogTrack.unsubscribe() depending on the API)
in the finally block so the subscription is released on success, timeout, or
error.
| private async subscribeToTracks(videoTrackName: string | null, audioTrackName: string | null): Promise<void> { | ||
| if (!this.broadcast) return; | ||
|
|
||
| const promises: Promise<void>[] = []; | ||
|
|
||
| if (videoTrackName) { | ||
| const videoTrack = this.broadcast.subscribe(videoTrackName, 2); | ||
| console.log("[MoqClient] Subscribed to video track:", videoTrackName); | ||
| promises.push(this.processTrack(videoTrack, "video")); | ||
| } | ||
|
|
||
| if (audioTrackName) { | ||
| const audioTrack = this.broadcast.subscribe(audioTrackName, 2); | ||
| console.log("[MoqClient] Subscribed to audio track:", audioTrackName); | ||
| promises.push(this.processTrack(audioTrack, "audio")); | ||
| } | ||
|
|
||
| await Promise.all(promises); | ||
| } |
There was a problem hiding this comment.
Early rejection in subscribeToTracks could leave one track processing.
If processTrack for one track throws an error before the other starts processing, Promise.all will reject immediately, but the other processTrack call (if already started) will continue running in the background since errors are caught internally at line 154. While isRunning will eventually stop the loop when disconnect() is called, there's a window where one track could continue processing after subscribeToTracks has rejected.
This is a minor edge case but could cause unexpected behavior during error recovery.
🔎 Proposed fix
Consider using Promise.allSettled to ensure both tracks are properly tracked even if one fails:
- await Promise.all(promises);
+ const results = await Promise.allSettled(promises);
+ const failures = results.filter(r => r.status === 'rejected');
+ if (failures.length > 0) {
+ throw new Error(`Track processing failed: ${failures.map(f => (f as PromiseRejectedResult).reason).join(', ')}`);
+ }📝 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.
| private async subscribeToTracks(videoTrackName: string | null, audioTrackName: string | null): Promise<void> { | |
| if (!this.broadcast) return; | |
| const promises: Promise<void>[] = []; | |
| if (videoTrackName) { | |
| const videoTrack = this.broadcast.subscribe(videoTrackName, 2); | |
| console.log("[MoqClient] Subscribed to video track:", videoTrackName); | |
| promises.push(this.processTrack(videoTrack, "video")); | |
| } | |
| if (audioTrackName) { | |
| const audioTrack = this.broadcast.subscribe(audioTrackName, 2); | |
| console.log("[MoqClient] Subscribed to audio track:", audioTrackName); | |
| promises.push(this.processTrack(audioTrack, "audio")); | |
| } | |
| await Promise.all(promises); | |
| } | |
| private async subscribeToTracks(videoTrackName: string | null, audioTrackName: string | null): Promise<void> { | |
| if (!this.broadcast) return; | |
| const promises: Promise<void>[] = []; | |
| if (videoTrackName) { | |
| const videoTrack = this.broadcast.subscribe(videoTrackName, 2); | |
| console.log("[MoqClient] Subscribed to video track:", videoTrackName); | |
| promises.push(this.processTrack(videoTrack, "video")); | |
| } | |
| if (audioTrackName) { | |
| const audioTrack = this.broadcast.subscribe(audioTrackName, 2); | |
| console.log("[MoqClient] Subscribed to audio track:", audioTrackName); | |
| promises.push(this.processTrack(audioTrack, "audio")); | |
| } | |
| const results = await Promise.allSettled(promises); | |
| const failures = results.filter(r => r.status === 'rejected'); | |
| if (failures.length > 0) { | |
| throw new Error(`Track processing failed: ${failures.map(f => (f as PromiseRejectedResult).reason).join(', ')}`); | |
| } | |
| } |
🤖 Prompt for AI Agents
In js/hang-demo/src/mse/moq-client.ts around lines 114 to 132, subscribeToTracks
uses Promise.all which can reject early and leave the other processTrack
running; change to use Promise.allSettled so both track processing promises are
awaited to completion, then examine the settled results to log or rethrow errors
as appropriate (e.g., if any promise rejected, surface a combined error or
rethrow the first failure) to ensure both tracks are tracked and cleaned up
before subscribeToTracks returns.
| private async processTrack(track: Moq.Track, trackType: "video" | "audio"): Promise<void> { | ||
| console.log(`[MoqClient] Processing ${trackType} track:`, track.name); | ||
|
|
||
| try { | ||
| while (this.isRunning) { | ||
| const group = await track.nextGroup(); | ||
| if (!group) { | ||
| console.log(`[MoqClient] ${trackType} track ended`); | ||
| break; | ||
| } | ||
|
|
||
| while (this.isRunning) { | ||
| const frame = await group.readFrame(); | ||
| if (!frame) break; | ||
|
|
||
| // For segment mode, the frame IS the complete fMP4 segment | ||
| // No timestamp stripping needed | ||
| this.config.onData?.(frame, trackType); | ||
| } | ||
| } | ||
| } catch (error) { | ||
| if (this.isRunning) { | ||
| console.error(`[MoqClient] Error processing ${trackType} track:`, error); | ||
| this.config.onError?.(error as Error); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Potential race condition when setting isRunning = false during active processing.
In processTrack, the while (this.isRunning) checks on lines 138 and 145 could create a race where:
- Line 138 checks
isRunning(true) and enters outer loop disconnect()is called, settingisRunning = false- Line 139 awaits
nextGroup()which could take time - Line 145 checks
isRunningbut line 146 still awaitsreadFrame()
While the error handling at line 155 checks isRunning before logging, the loop could still process one more frame after disconnect is called. This is a minor timing issue but could cause confusion during debugging.
Consider checking isRunning immediately after each await:
🔎 Proposed fix
private async processTrack(track: Moq.Track, trackType: "video" | "audio"): Promise<void> {
console.log(`[MoqClient] Processing ${trackType} track:`, track.name);
try {
while (this.isRunning) {
const group = await track.nextGroup();
+ if (!this.isRunning) break;
if (!group) {
console.log(`[MoqClient] ${trackType} track ended`);
break;
}
while (this.isRunning) {
const frame = await group.readFrame();
+ if (!this.isRunning) break;
if (!frame) break;
// For segment mode, the frame IS the complete fMP4 segment
// No timestamp stripping needed
this.config.onData?.(frame, trackType);
}
}
} catch (error) {
if (this.isRunning) {
console.error(`[MoqClient] Error processing ${trackType} track:`, error);
this.config.onError?.(error as Error);
}
}
}📝 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.
| private async processTrack(track: Moq.Track, trackType: "video" | "audio"): Promise<void> { | |
| console.log(`[MoqClient] Processing ${trackType} track:`, track.name); | |
| try { | |
| while (this.isRunning) { | |
| const group = await track.nextGroup(); | |
| if (!group) { | |
| console.log(`[MoqClient] ${trackType} track ended`); | |
| break; | |
| } | |
| while (this.isRunning) { | |
| const frame = await group.readFrame(); | |
| if (!frame) break; | |
| // For segment mode, the frame IS the complete fMP4 segment | |
| // No timestamp stripping needed | |
| this.config.onData?.(frame, trackType); | |
| } | |
| } | |
| } catch (error) { | |
| if (this.isRunning) { | |
| console.error(`[MoqClient] Error processing ${trackType} track:`, error); | |
| this.config.onError?.(error as Error); | |
| } | |
| } | |
| } | |
| private async processTrack(track: Moq.Track, trackType: "video" | "audio"): Promise<void> { | |
| console.log(`[MoqClient] Processing ${trackType} track:`, track.name); | |
| try { | |
| while (this.isRunning) { | |
| const group = await track.nextGroup(); | |
| if (!this.isRunning) break; | |
| if (!group) { | |
| console.log(`[MoqClient] ${trackType} track ended`); | |
| break; | |
| } | |
| while (this.isRunning) { | |
| const frame = await group.readFrame(); | |
| if (!this.isRunning) break; | |
| if (!frame) break; | |
| // For segment mode, the frame IS the complete fMP4 segment | |
| // No timestamp stripping needed | |
| this.config.onData?.(frame, trackType); | |
| } | |
| } | |
| } catch (error) { | |
| if (this.isRunning) { | |
| console.error(`[MoqClient] Error processing ${trackType} track:`, error); | |
| this.config.onError?.(error as Error); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In js/hang-demo/src/mse/moq-client.ts around lines 134 to 160, processTrack can
continue processing a group/frame after disconnect() flips isRunning to false
because awaits aren't re-checked; after each await (after nextGroup() and after
readFrame()) immediately test this.isRunning and break/return if false so no
further frames are processed, and ensure the catch logging still guards on
isRunning to avoid spurious error reports.
| this.video.addEventListener("stalled", () => { | ||
| console.log("[MoqMsePlayer] Video stalled, attempting to recover..."); | ||
| // Try to seek to current position to unstick | ||
| if (this.videoSourceBuffer && this.videoSourceBuffer.buffered.length > 0) { | ||
| const bufferedEnd = this.videoSourceBuffer.buffered.end(0); | ||
| if (bufferedEnd > this.video.currentTime + 0.5) { | ||
| console.log("[MoqMsePlayer] Buffer ahead, seeking to resume"); | ||
| this.video.currentTime = this.video.currentTime + 0.1; | ||
| } | ||
| } | ||
| }); |
There was a problem hiding this comment.
Stalled recovery logic assumes single buffer range.
The stalled event handler checks buffered.length > 0 but then uses buffered.end(0), which only checks the first buffer range. If the buffer is fragmented (multiple ranges), this may not find the correct buffered end position to determine if seeking forward would help.
🔎 Proposed fix
this.video.addEventListener("stalled", () => {
console.log("[MoqMsePlayer] Video stalled, attempting to recover...");
// Try to seek to current position to unstick
if (this.videoSourceBuffer && this.videoSourceBuffer.buffered.length > 0) {
- const bufferedEnd = this.videoSourceBuffer.buffered.end(0);
+ const bufferedEnd = this.videoSourceBuffer.buffered.end(
+ this.videoSourceBuffer.buffered.length - 1
+ );
if (bufferedEnd > this.video.currentTime + 0.5) {
console.log("[MoqMsePlayer] Buffer ahead, seeking to resume");
this.video.currentTime = this.video.currentTime + 0.1;
}
}
});📝 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.
| this.video.addEventListener("stalled", () => { | |
| console.log("[MoqMsePlayer] Video stalled, attempting to recover..."); | |
| // Try to seek to current position to unstick | |
| if (this.videoSourceBuffer && this.videoSourceBuffer.buffered.length > 0) { | |
| const bufferedEnd = this.videoSourceBuffer.buffered.end(0); | |
| if (bufferedEnd > this.video.currentTime + 0.5) { | |
| console.log("[MoqMsePlayer] Buffer ahead, seeking to resume"); | |
| this.video.currentTime = this.video.currentTime + 0.1; | |
| } | |
| } | |
| }); | |
| this.video.addEventListener("stalled", () => { | |
| console.log("[MoqMsePlayer] Video stalled, attempting to recover..."); | |
| // Try to seek to current position to unstick | |
| if (this.videoSourceBuffer && this.videoSourceBuffer.buffered.length > 0) { | |
| const bufferedEnd = this.videoSourceBuffer.buffered.end( | |
| this.videoSourceBuffer.buffered.length - 1 | |
| ); | |
| if (bufferedEnd > this.video.currentTime + 0.5) { | |
| console.log("[MoqMsePlayer] Buffer ahead, seeking to resume"); | |
| this.video.currentTime = this.video.currentTime + 0.1; | |
| } | |
| } | |
| }); |
🤖 Prompt for AI Agents
In js/hang-demo/src/mse/moq-mse-player.ts around lines 98 to 108, the stalled
event handler assumes a single buffered range by calling buffered.end(0); update
the logic to handle multiple TimeRanges: inspect all buffered ranges to find
either the range that contains currentTime (use its end) or fall back to the
last buffered range end (buffered.end(buffered.length - 1)); then compare that
end against video.currentTime and only perform the small seek when the selected
buffered end is sufficiently ahead. Ensure you check buffered.length and handle
the case where no range contains currentTime.
| private async initMediaSource(): Promise<void> { | ||
| return new Promise((resolve) => { | ||
| this.mediaSource = new MediaSource(); | ||
| this.video.src = URL.createObjectURL(this.mediaSource); | ||
|
|
||
| this.mediaSource.addEventListener("sourceopen", () => { | ||
| console.log("[MoqMsePlayer] MediaSource opened, waiting for catalog..."); | ||
| resolve(); | ||
| }); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Missing timeout in initMediaSource could cause connection to hang indefinitely.
If the MediaSource fails to open or the sourceopen event never fires, the promise returned by initMediaSource will never resolve, causing connect() to hang at line 183. This could happen if there's a browser compatibility issue or if the MediaSource API is not available.
🔎 Proposed fix
private async initMediaSource(): Promise<void> {
return new Promise((resolve, reject) => {
this.mediaSource = new MediaSource();
this.video.src = URL.createObjectURL(this.mediaSource);
+ const timeout = setTimeout(() => {
+ reject(new Error("MediaSource failed to open within 10 seconds"));
+ }, 10000);
+
this.mediaSource.addEventListener("sourceopen", () => {
+ clearTimeout(timeout);
console.log("[MoqMsePlayer] MediaSource opened, waiting for catalog...");
resolve();
});
+
+ this.mediaSource.addEventListener("error", (e) => {
+ clearTimeout(timeout);
+ reject(new Error(`MediaSource error: ${e}`));
+ });
});
}🤖 Prompt for AI Agents
In js/hang-demo/src/mse/moq-mse-player.ts around lines 273 to 283,
initMediaSource currently returns a Promise that only resolves on the
"sourceopen" event and can hang forever; change it to fail fast by adding a
timeout (e.g., 5s) and reject the promise if "sourceopen" doesn't fire, also
handle the case where MediaSource is unavailable by rejecting immediately;
ensure you clear the timeout and remove the "sourceopen" listener when the event
fires or when timing out, and revoke the created object URL and null out
this.mediaSource on failure to avoid leaks.
| /// Extract complete fMP4 segment (moof + mdat) as a single frame. | ||
| /// | ||
| /// Used when [`ImportMode::Segments`] is selected for MSE compatibility. | ||
| fn extract_segment(&mut self, mdat: Mdat, _header_size: usize) -> anyhow::Result<()> { |
There was a problem hiding this comment.
Use actual header_size instead of hardcoded 8 for mdat header.
The header_size parameter is passed to extract_segment (line 618) but ignored in favor of a hardcoded -8 at line 780. The extract() method correctly uses the actual header_size at line 532:
offset = base_offset + data_offset - self.moof_size - header_size;While most mdat boxes have an 8-byte header (4 bytes size + 4 bytes 'mdat'), the MP4 format supports extended size boxes (16-byte header) for large files. This inconsistency will cause incorrect offset calculations for extended-size mdat boxes.
🔎 Proposed fix
-fn extract_segment(&mut self, mdat: Mdat, _header_size: usize) -> anyhow::Result<()> {
+fn extract_segment(&mut self, mdat: Mdat, header_size: usize) -> anyhow::Result<()> {And at line 780:
- offset = base_offset + data_offset_usize - self.moof_size - 8;
+ offset = base_offset + data_offset_usize - self.moof_size - header_size;Also update the test helper at line 1073:
- offset = base_offset + data_offset_usize - moof_size - 8;
+ offset = base_offset + data_offset_usize - moof_size - header_size;And pass header_size as a parameter to the test helper function.
Also applies to: 780-780
🤖 Prompt for AI Agents
In rs/hang/src/import/fmp4.rs around lines 618 and 780 (and test helper at
~1073): extract_segment receives header_size but the code uses a hardcoded 8
when computing offsets (line 780); replace that hardcoded 8 with the passed
header_size so offset math uses the actual mdat header length (supporting
16-byte extended-size boxes), update any related calculations accordingly, and
modify the test helper at ~1073 to accept and propagate header_size from the
caller so tests exercise both 8- and 16-byte headers.
|
I'm sorry, this is waaaay too vibe coded. It's not even a library. Let me know if you want to take another stab at it and I can provide some guidance. |
2 participants
feat: MSE-based Media Player for Browser Compatibility
Summary
This PR adds a new MediaSource Extensions (MSE) based player as an alternative to the existing WebCodecs-based
<hang-watch>component. MSE provides broader browser compatibility and native<video>element support.New Features
1. MSE Player Component (
<moq-mse-player>)A new web component that uses MSE for media playback:
Key capabilities:
<video>element with browser controls2. Server-side Segment Mode (
fmp4-mse)New import mode in
hang::import::Fmp4that sends complete fMP4 segments instead of individual frames:Changes to
rs/hang/src/import/fmp4.rs:ImportMode::Segmentsvariantextract_segment()sends completemoof+mdatboxes3. Audio-Video Synchronization Fix
Fixed timing issues where audio would be delayed 2-7 seconds behind video:
Architecture
Comparison: WebCodecs vs MSE
<hang-watch>)<moq-mse-player>)<canvas><video>just pub bbbjust pub-mse bbbFiles Changed
Rust (Server)
rs/hang/src/import/fmp4.rsImportModeenum withFramesandSegmentsvariantsextract_segment()for MSE-compatible segment extractioncreate_per_track_init_segments()for separate audio/video initrs/hang-cli/src/publish.rsFmp4Mseformat variantTypeScript (Client)
js/hang-demo/src/mse/moq-mse-player.ts[NEW]js/hang-demo/src/mse/moq-client.ts[NEW]@moq/litejs/hang-demo/src/watch-mse.html[NEW]Build/Config
justfilepub-mserecipe for MSE publishingUsage
Pending Work
Important
The MSE client is currently implemented only in
hang-demo. Integration into the@moq/hangpackage is pending.TODO:
@moq/hangMSE ClientThe following work remains to make MSE a first-class citizen in the hang library:
MoqMsePlayercomponent from@moq/hang<hang-watch-mse>wrapper component<hang-watch>Testing
Breaking Changes
None. This is an additive feature.
Related Issues: N/A
Depends On: N/A