fix(js): Safari playback/publish support, connection reliability, and a capture-releasing mute#2163
Open
fperex wants to merge 19 commits into
Open
fix(js): Safari playback/publish support, connection reliability, and a capture-releasing mute#2163fperex wants to merge 19 commits into
fperex wants to merge 19 commits into
Conversation
Publish: hardware H.264 encode with worker capture and honest support labels, 48 kHz Opus with cadence-snapped timestamps and capture-rate resampling. Watch: audio resampling to the context rate, timestamp snap, gesture-driven AudioContext resume with a running-gated graph, decoder restart budgets (audio + video) with codec-switch supersede detection and keyframe resync, decode-queue backpressure, unsupported-codec detection with a UI notice, stall/visibility reconnect recovery, and announce-generation republish re-consume. Net: code-aware isStreamAbort logging so real subscribe/publish faults surface while routine stream teardown stays at debug.
Document the getting-started flow (nix develop / just check / just fix), the main-vs-dev branch-targeting rule (base on and target dev unless a change is purely additive), the Cross-Package Sync expectation, code-style rules (no em dashes, document exported symbols, no AI attribution), and the review flow.
Safari cannot use WebTransport on moq-lite-05. Reading `WebTransport.datagrams.readable` tears down the whole session, and `Connection` always reads it once the version carries datagrams, so the session dies and Reload loops on "connection error". The read cannot be guarded, only avoided. Route Safari to the existing WebSocket/qmux transport instead, next to the Firefox force already in connect.ts. qmux implements `datagrams` itself, so Safari never touches the native API. Nothing in hang/publish/watch sends or receives datagrams today, and Firefox has run this path since lite-05 landed. Safari therefore has no `getStats()`, so `sendBandwidth` stays undefined and the encoder skips its bandwidth cap, exactly as Firefox already does. The publish support panel now reports WebTransport as "partial" for Safari to surface the degraded path, and the browser-support docs say so. Also widen `isStreamAbort` to treat a write-after-close as routine teardown: over qmux it surfaces as a generic Streams-API error rather than a coded RESET_STREAM. Chromium and Firefox say the stream is "closed or closing"; Safari throws InvalidStateError.
…op the reconnect storm Extract transport selection into a pure `pickTransport(userAgent, hasWebTransport)` so "Safari never gets WebTransport" is covered by a test rather than by reading one boolean expression. The eleven-user-agent matrix pins the cases that matter: Safari 26 with WebTransport present still returns websocket, and iOS Chrome (CriOS) and iOS Firefox (FxiOS) fall through to the WebKit branch because they report neither "chrome" nor "firefox". Chrome and Firefox are unchanged. Expose the negotiated transport as `Established.transport`, derived from the race winner, and render it in the demo. The Network panel previously hardcoded the word "WebTransport", so a Safari session on WebSocket looked like it was on WebTransport. `connect()` now reads that same `transportOf()` for its own log, so the console line and the field cannot disagree. Fix fullscreen on Safari: `#fullscreen()` compared `document.fullscreenElement` against the shadow-internal `.player`, but the spec retargets that to the shadow host. Chrome and Firefox were rescued by `ShadowRoot.fullscreenElement`, which Safari does not expose, so the video never filled the screen. Add a `:fullscreen` match alongside every existing check rather than replacing them. Remove the Safari stall/visibility reconnect recovery, and `Reload.reconnect()` with it. A hidden `<moq-watch>` stops rendering, so `stalled` is always true on return, and the recovery forced a full reconnect on every tab switch. It existed to recover a wedged WebTransport session, which Safari can no longer have. Neither symbol exists upstream, so this moves reload.ts back toward it. Also report WebTransport as "partial" on Safari in both support panels, since Safari defines it but never uses it.
The message heuristic ran before the coded-fault check, so a WebTransportError carrying a client-actionable code was reclassified as routine teardown whenever the browser's prose happened to mention a closing stream. And `invalid state` matched any message containing that substring, so one of our own ordering bugs throwing "the decoder is in an invalid state" was downgraded to debug and never seen. Decide on the coded reset first, and key Safari's InvalidStateError on `err.name` rather than on prose. Gate the two `probe stream error` logs on isStreamAbort as well. They had been downgraded to an unconditional debug, which hid an auth or protocol fault on the bandwidth side channel on every browser, not just during teardown. Restore the Jan-Ivar attribution and source link for the rAF capture polyfill, which the Worker-capture rewrite dropped even though that code still ships as `rafTrackProcessor`. Restore the upstream note about disconnecting the worklet to save power, and correct the `#expectedNext` comment: the timestamp snap runs on every frame, not only on the resample path.
`pagehide` set `#suspended`, but only a `pageshow` with `persisted === true` cleared it. Safari fires `pagehide` when it freezes a page and can resume that page without ever firing a persisted `pageshow`, so `#suspended` stayed true, `#active` stayed false, and the element's Reload was disabled forever. The connection never came back and never retried. In the demo this reads as "Connected" next to "Offline": the page's own Reload is constructed with `enabled: true` and keeps its session, while the <moq-watch> tile's Reload is gated on `#active` and is permanently disabled. Throughput sits at 0 and the relay sees no further connection attempts. Clear `#suspended` on any `pageshow`, and on the page becoming visible again, so every path that sets it has a counterpart that clears it. Chrome and Firefox never hit this: they do not fire `pagehide` on a tab switch.
Closing a session fails every stream still open on it. qmux words that "Connection closed", optionally carrying the peer's CONNECTION_CLOSE code, and isStreamAbort recognised none of those forms. So tearing a connection down warned on the probe stream, on the WebSocket fallback, for every viewer that stopped watching. Apply the same rule a stream reset gets: routine unless the peer signalled a client-actionable fault code. A close carrying Unauthorized or ProtocolViolation still surfaces at warn; a plain close, a zero code, and an abnormal WebSocket closure do not. Native WebTransport already reported the equivalent as a WebTransportError with no streamErrorCode. Safari's Error.stack omits the message line, which is why this only ever showed up as a bare stack in the console.
`Established.closed` resolves on every session end. qmux always resolves it (`#closedResolve` in its `#close`), whatever the close code, and WebTransport rejects only on an abnormal close. So a dropped session fell out of `#connect`'s try block without ever reaching the catch, and nothing scheduled a retry. The connection then stayed dead forever: `established` kept pointing at the dead session, `status` still read "connected", and no further attempt was made. In the demo this reads as a green "Connected" pill next to an "Offline" broadcast, with throughput at 0, an empty round-trip graph, and no new "connected via WebSocket" line in the console. Safari and Firefox are forced onto the qmux WebSocket by `pickTransport`, so for them this hit on every drop and nothing short of a page reload brought playback back. On WebTransport an abnormal close rejects and did recover; a clean one did not. Treat a resolved `closed` as a disconnect: clear `established`, report "disconnected", and retry on the existing backoff ladder. Only reset that ladder once a session has stayed up for STABLE_CONNECTION_MS, so a relay that accepts us and drops us straight away (bad auth, shedding load) is not hammered once a second forever. Settle the teardown race with a sentinel rather than a falsy check, and pin `effect.cancel` and `effect.abort` per run, since a rerun swaps both before it awaits the spawn.
`#runCatalog` subscribed once and, on any end of its fetch loop, wrote status="offline". The effect tracks only enabled, catalogFormat, name and output.active, none of which change when just the catalog stream is reset, so nothing ever reopened the subscription. A transient reset on a live connection stranded the tile offline for good. Reopen it on a bounded backoff, but only when the loop ended with a stream reset (the subscription was killed under us by a slow-consumer drop, a relay bounce, or a publisher handover). A clean end means the publisher stopped, and a coded fault fails identically every attempt, so both still report offline immediately rather than stalling the badge behind a retry ladder. Every bail-out now reports offline as well: a pending retry leaves the status at "loading", so a rerun that finds nothing left to subscribe to has to settle it or the tile spins forever. The net layer already drops a closed track from its cache, so resubscribing the same consumer works; guard the closed-broadcast case, since subscribing to one throws.
The overlay printed `err.stack || err.message`. JavaScriptCore and SpiderMonkey build `stack` as a bare frame list with no leading "Name: message" line, so on Safari and Firefox the message was dropped entirely and a qmux teardown surfaced as an anonymous stack with no error code in it. Prepend "Name: message" when the stack does not already carry it. Keying on the stack contents rather than on a user-agent check keeps V8, which does include the header, from printing it twice.
…eline Muting used to gate audio capture: it stopped the microphone track, closed the AudioContext + worklet + AudioEncoder, closed the audio/data track producer, and cascaded the catalog to drop its audio section. On the watcher that meant a full audio-subtree rebuild (AudioContext + worklet + ring buffer) plus head-of-line contention on the shared WebSocket, which froze video. Mute is now a soft mute: audio capture stays live and the encoder ramps its gain to zero, so a muted broadcast keeps publishing its audio track as silence and the catalog never flaps. #audioEnabled is constant-true (nothing is captured until a source is selected, so an idle element still never touches the mic), and state.muted feeds the encoder's existing #runGain via broadcast.audio.muted. Also scope the four #runSource source-proxy effects to the passed child effect (effect.run/effect.set) instead of registering on the root this.signals, so each source switch tears its proxies down in order rather than leaking a root-scoped writer onto broadcast.video/audio.source.
The tile's active-state effect both wrote watch.muted and read the catalog (for the speaker badge), so every catalog frame re-ran it and re-asserted muted=false on the active tile, reverting the user's speaker-mute a moment after each click. Split it into two effects: the active styling + muted policy now depends on `active` alone, and the badge keeps its own catalog-reading effect.
Audio.Decoder is built once and its #discontinuity outlives the subscription, but the container Consumer's rewind counter is per-consumer and a fresh subscription restarts at zero. After any rewind bumped the decoder's count to >=1, the next resubscribe delivered 0, which looked like a rewind and reset the Sync reference shared with video. Re-anchor #discontinuity = 0 when the subscription is rebuilt. Add a consumer test pinning that a fresh/resubscribed Consumer starts at zero.
…ransport rule Remove the unused `safariVersion` export from @moq/hang (zero consumers repo-wide; detectSafariVersion stays, still used by the worker-capture gate). This is a breaking export removal, hence the dev base. The Safari-detection rule is duplicated in @moq/hang's detectSafari and @moq/net's pickTransport and cannot share code (@moq/hang depends on @moq/net, and pickTransport is internal). Add cross-reference comments on both stating they must agree, and widen the shared UA corpus in hacks.test.ts and transport.test.ts (iOS Firefox, an uppercase UA, and a legacy Android stock-browser UA) so a drift between them fails a test in each package. Also add the missing `test` script to js/hang/package.json so hang's tests actually run under `just js test` (they were silently skipped, which would have hidden the drift guard).
Add min-width: 0 to the buffer-bar canvas so it can shrink below its intrinsic width inside the flex row instead of overflowing it.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @fperex, your pull request is larger than the review limit of 150000 diff characters
…lace Muting previously kept the microphone capturing and only zeroed the encoder gain, so the OS recording indicator stayed on while "muted". Now muting stops audio capture per source: a microphone track is stopped outright (releasing the OS device and its indicator), while screen/file audio detaches from the encoder, since stopping a share track would kill the share and re-prompt. The encoder pipeline survives the mute: a silent ConstantSourceNode keeps the capture worklet emitting true silence on the same AudioContext, so the catalog never drops or reshapes the audio track and watchers keep their audio subtree instead of rebuilding it. Selecting a source while muted never touches the microphone at all; unmuting re-acquires it. Verified with a headless-Chrome fake-device run: zero getUserMedia calls while muted, the mic track ends on every mute, the catalog stays shape-identical across mute cycles, and a single AudioContext spans the whole cycle.
Cleanups from a full quality audit of the fork diff (duplication, dead machinery, comment conventions, API hygiene). No behavior changes beyond console log levels: - Finish the isFirefox migration in watch/support (the publish twin already uses Util.Hacks.isFirefox); drop the local UA sniff. - Replace the fork-added #closing flag in lite/connection with the isStreamAbort classification every other run-loop catch already uses. - Remove the audio encoder's #codecMime computed signal and the manual format field-compare; Signal.set's deep-equality dedupe already covers both, and the ordering hazard the computed worked around disappears. - Stop exporting configSuperseded from the @moq/watch video barrel; it is a test helper, and the test imports it directly. - Drop a try/catch around a spec-infallible track.stop() and a stale-timer guard in the catalog retry arm that defends against an impossible race. - Compress comments that restated the same invariant in multiple places (mute policy, bitrate-churn capture, isStreamAbort rationale, generation bump) and rewrite history-tense comments to describe the current code. - Document the new public members (announcedGenerations, bytesEncoded, AudioBuffer.wait's resolve-on-abort contract); align README/doc pages with the unsupported-codec indicator and mute semantics. - Use the branded Time.Milli constructor over a bare cast and match the file's import-extension style in video/polyfill.
fperex
changed the title
There was a problem hiding this comment.
Sorry @fperex, your pull request is larger than the review limit of 150000 diff characters
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.
Batch of browser fixes developed on the fperex fork, focused on making Safari a first-class client without touching Chrome/Firefox behavior (Safari-only changes are gated on UA predicates), plus connection-reliability fixes that apply everywhere.
Safari transport
WebTransport.datagrams.readablehard-kills the Safari session with no catchable error, so the QUIC race is never entered (pickTransportin@moq/net).@moq/publish/support,@moq/watch/support) reportwebtransport: "partial"on Safari/Firefox to surface the degraded path.Connection reliability
Connection.Reloadnow reconnects when an established session closes, not just when a connect attempt fails.closedresolves (never rejects) on qmux, so a dropped session previously left a deadestablishedand a permanent "connected" status, with Safari/Firefox never recovering from any drop.pagehideno longer latches the connection off permanently; the connection closes eagerly on pagehide/freeze (so the relay unannounces immediately) and resumes on pageshow/visibility.announcedGenerationstracks a per-path announce generation so a watcher re-consumes a same-name republish even when the unannounce+announce coalesce.isStreamAbort): routine lifecycle resets (unsubscribe, handover, session close) log at debug; client-actionable faults (auth, not-found, protocol, unroutable) still warn, on every run-loop catch across net/hang/watch.Publish: audio privacy mute
mutednow stops audio capture while the encoder pipeline stays up and publishes true silence via a keep-alive graph. The catalog never drops or reshapes the audio track, so watchers keep their audio subtree instead of rebuilding it (no video freeze on mute).track.stop(), so the OS recording indicator turns off); screen/file audio is detached from the encoder instead, since stopping a share track would kill the share and force a re-prompt.Publish: capture/encode
safariWorkerCapture, WebKit 18+).ended(all browsers); Safari-gated recovery for a backgrounded tab leaving the camera stuck muted.Watch: decode robustness
DataError, and a decode-queue cap.Public API changes (js/*)
Breaking (why this targets
dev):@moq/netConnection.Established.transport— new required readonly member on a public interface. Additive for consumers; a compile break for external implementers of the interface.Additive:
@moq/net:isStreamAbort(root export),Connection.Transporttype,Connection.Reload.announcedGenerations.@moq/hang:Util.Hacks.isSafari,Util.Hacks.safariWorkerCapture(plus puredetect*cores primarily consumed internally/by tests).@moq/publish:Video.Encoder.bytesEncoded.@moq/watch: optionalannouncedGenerationsin theBroadcastconstructor options bag,Video.Source.output.unsupported,AudioBuffer.wait(timestamp, signal?).Behavior changes on existing surface: Safari/Firefox transport routing, reconnect-on-close, mic-releasing
muted, 48 kHz Opus catalog rate,isSupported()matrices (details above).Known upstream issue
Testing this PR on Safari/Firefox (which it routes onto the WebSocket/qmux fallback) surfaces
Unhandled rejection: TypeError: ReadableStreamDefaultController is not in a state where chunk can be enqueuedwhen a peer reloads while connected. This is a known@moq/qmux <= 0.3.1bug, already fixed upstream in moq-dev/web-transport (release pending); it is not introduced by this PR and the pin can be bumped when the release ships.PR Link: moq-dev/web-transport#285
Test plan
just js checkandjust js testgreen (544 tests across 8 packages; new unit tests for transport picking, error classification, cadence/resampler math, video catalog capture, config supersession, timestamp snapping, container consumer).getUserMediacalls; every mute ends the mic track; the audio catalog stays present and shape-identical across mute cycles; exactly one AudioContext across the full cycle (pipeline survives mute).