From 7c5f8217ec1271f214bfae620c82e8e1a9ad6572 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 7 Oct 2026 14:09:45 -0700 Subject: [PATCH 1/3] quest: plan the 2026-10-07 audit findings Seven new quests (five m1, two m2) and amendments to ws-unauthorized and av-clock. Findings already covered by open PRs (#5025, #4927, #4820, #5027) are not requoted. Co-Authored-By: Claude Opus 5.5 --- quest/m1/README.md | 6 +++++ quest/m1/auth/ws-unauthorized.md | 13 +++++++++- quest/m1/av-clock.md | 6 +++++ quest/m1/cmaf-sample-defaults.md | 38 +++++++++++++++++++++++++++++ quest/m1/mkv-lacing.md | 28 ++++++++++++++++++++++ quest/m1/papercuts-js.md | 36 ++++++++++++++++++++++++++++ quest/m1/papercuts-rs.md | 41 ++++++++++++++++++++++++++++++++ quest/m1/tokio-poll-closed.md | 21 ++++++++++++++++ quest/m1/watch-decode-error.md | 28 ++++++++++++++++++++++ quest/m2/README.md | 2 ++ quest/m2/fmp4-edit-lists.md | 32 +++++++++++++++++++++++++ quest/m2/watch-decode-gate.md | 27 +++++++++++++++++++++ 12 files changed, 277 insertions(+), 1 deletion(-) create mode 100644 quest/m1/cmaf-sample-defaults.md create mode 100644 quest/m1/mkv-lacing.md create mode 100644 quest/m1/papercuts-js.md create mode 100644 quest/m1/papercuts-rs.md create mode 100644 quest/m1/tokio-poll-closed.md create mode 100644 quest/m1/watch-decode-error.md create mode 100644 quest/m2/fmp4-edit-lists.md create mode 100644 quest/m2/watch-decode-gate.md diff --git a/quest/m1/README.md b/quest/m1/README.md index 45b2059842..ab4aa8503b 100644 --- a/quest/m1/README.md +++ b/quest/m1/README.md @@ -31,6 +31,7 @@ blocks. The quests that gated m0 lines moved under them. - [One max_age meaning](/quest/m1/cache-max-age.md) - a superseded group goes stale on wall clock since its successor arrived or on media time, whichever is first, in Rust and js/net; fixes the untimed failover stall - [JS untimed model](/quest/m1/js-untimed-model.md) - the untimed model in @moq/net - [CMAF frame timestamp](/quest/m1/cmaf-frame-timestamp.md) - CMAF decoders time samples from the moq-lite frame timestamp, using `tfdt` only within the fragment +- [CMAF sample defaults](/quest/m1/cmaf-sample-defaults.md) - one trun, tfhd, trex resolver and one keyframe rule in the importer, the Rust decoder, and JS - [Shared import clock](/quest/m1/shared-clock.md) - an importer joining a clock already in use offsets its PTS instead of moving it, so captures and imports share one timeline - [FFI shape](/quest/m1/ffi-shape/README.md) - the bindings mirror Rust's layers: net at the root, then media, json, flate, audio, and video namespaces built from the handle below - [Publishing never invents a timestamp](/quest/m1/publish-timestamp.md) - no Rust or binding publish API fills in a timestamp; an untimed payload goes out untimed @@ -150,6 +151,8 @@ blocks. The quests that gated m0 lines moved under them. - [Front deadlines](/quest/m1/front-deadline-index.md) - a front's per-event cost stops growing with its track count: an expiry index and per-track wakes, proven by a churn benchmark - [Listener deadlines](/quest/m1/listener-deadlines.md) - io_uring, HTTP/2, and the internal listener bound slow handshakes and headers, and iroh honors `quic.keep_alive` - [Papercuts](/quest/m1/papercuts.md) - JS refuses to serve a broadcast it did not produce, and a uring test stops sleeping +- [Rust papercuts](/quest/m1/papercuts-rs.md) - HTTP refusals are counted, a `u64::MAX` resume is unbounded, GOING_AWAY falls forward, and GRO stride 0 cannot panic +- [JS papercuts](/quest/m1/papercuts-js.md) - IETF status 0 keeps the subgroup open, a muted rendition change settles, and bad element attributes warn - [Front parking](/quest/m1/origin-front-parks.md) - an unroutable request waits on a front instead of re-asking on every route-table move - [Route wakes](/quest/m1/route-wakes.md) - a route change wakes only the fronts it can move, so pool churn stops scaling with served paths - [Copy-walk bench](/quest/m1/admission-bench.md) - a front's walk over its copies on a route change is benchmarked over tracks and copies @@ -157,6 +160,7 @@ blocks. The quests that gated m0 lines moved under them. - [JS request deadline](/quest/m1/js-request-deadline.md) - each JS request, PUBLISH_NAMESPACE included, has one fatal 10 s timer from create to answer and fails fast without stream credit, so nothing queues or retries - [Rust request credit](/quest/m1/rs-request-credit.md) - a moq-net request fails at once without stream credit instead of waiting - [qmux no-wait opens](/quest/m1/qmux-no-wait.md) - @moq/qmux rejects an over-limit create when waitUntilAvailable is false, like Chrome +- [tokio poll_closed](/quest/m1/tokio-poll-closed.md) - moq-tokio's stream adapter wakes a close-only waiter after the read-ahead cap - [E2EE](/quest/m1/e2ee/README.md) - TypeScript and Rust peers interoperate over encrypted broadcasts no relay can decrypt - [#3056](/quest/m1/3056-watch-video-decoder-captures-the-rewind-generation-at.md) - watch: the video decoder resets on a declared discontinuity - [#933](/quest/m1/933-video-rotation-metadata-not-propagated-from-mobile-camera.md) - the catalog rotation follows the live camera's orientation @@ -168,12 +172,14 @@ blocks. The quests that gated m0 lines moved under them. - [Encoder colour](/quest/m1/color-model.md) - every moq-video encode path signals the colour its output actually has, or refuses instead of mislabelling - [T-STD TS export](/quest/m1/tstd/README.md) - `moq export ts` is a proper remux that passes the T-STD buffer model, starting with a fixed `--delay` - [Open-GOP leading pictures](/quest/m1/open-gop-leading-pictures.md) - a viewer joining at a recovery point drops the leading pictures it cannot decode; continuous viewers keep them +- [Watch decode errors](/quest/m1/watch-decode-error.md) - a WebCodecs error ends the subscription and the element reports it - [Catalog warmup](/quest/m1/catalog-warmup.md) - `warmup` on video and audio renditions, in the catalog and the draft - [Audio warmup](/quest/m1/audio-warmup.md) - a viewer joining an Opus rendition mid-stream never hears the unconverged first 80 ms - [#3021](/quest/m1/3021-moq-gst-anchor-generated-media-timelines-to-wall-clock.md) - moq-gst picks the broadcast wall epoch; a restarted source is a new epoch, not a forward re-anchor - [TS passthrough](/quest/m1/ts-passthrough.md) - `--passthrough` carries the multiplex as whole packets, listed in an `m2ts` catalog section, and writes it back byte-identical (less late drops) on a fixed delay - [FLV export delay](/quest/m1/flv-export-delay.md) - FLV interleaves through the shared fixed-delay release stage - [MKV export delay](/quest/m1/mkv-export-delay.md) - MKV interleaves through the shared fixed-delay release stage +- [MKV lacing](/quest/m1/mkv-lacing.md) - laced MKV blocks import as one timed frame each, refused without DefaultDuration - [Release profile](/quest/m1/release-profile.md) - every release build gets fat LTO, one codegen unit, and stripping from the workspace profile instead of three script exports - [Size report](/quest/m1/size-report.md) - a nightly job reports every shipped artifact's size, native and JS, and alerts when one grows - [JS bundle trims](/quest/m1/js-bundle-trims.md) - no bowser, split pako, and lazy qmux and captions diff --git a/quest/m1/auth/ws-unauthorized.md b/quest/m1/auth/ws-unauthorized.md index a4621a6071..f62671fa02 100644 --- a/quest/m1/auth/ws-unauthorized.md +++ b/quest/m1/auth/ws-unauthorized.md @@ -27,8 +27,19 @@ Check that Rust clients, which treat HTTP 401 as terminal today, treat the session close the same way. The 401 in `doc/bin/relay/auth.md` describes the auth webhook, not the relay's answer; leave it. +Admitting after SETUP also closes two gaps the 2026-10-07 audit found in the +current pre-upgrade admit, which has no role and no SETUP token: a session +whose role its grant cannot serve (a publisher holding a subscribe-only +grant) is upgraded and its broadcasts silently go nowhere, where QUIC refuses +it in `Cluster::scope`; and a moqt-over-WebSocket client that authenticates +only with the SETUP `AUTHORIZATION TOKEN` is treated as anonymous. Neither +grants extra access, so this stays in m1 (decided 2026-10-07). Copy the SETUP +token onto the auth request as the QUIC path does in `request_for`. + Tests: a WebSocket connect with a bad token ends in a JS `SessionError` with -the unauthorized code and the reload loop stops; a Rust client stops too. +the unauthorized code and the reload loop stops; a Rust client stops too. A +publish-role WebSocket session with a subscribe-only grant is refused, and a +SETUP-token-only client is admitted with that token's grant. ## Closes diff --git a/quest/m1/av-clock.md b/quest/m1/av-clock.md index 9b364ddb67..fc85b3842b 100644 --- a/quest/m1/av-clock.md +++ b/quest/m1/av-clock.md @@ -9,6 +9,12 @@ the AudioContext clock while video paces against a wall clock anchored at the earliest arrival, so a ring that re-buffers or skips drifts against video until the next re-anchor. +Acceptance includes the 2026-10-07 audit's skip case: a video-only latency +skip (`js/watch/src/video/decoder.ts`, `sync.reset()` on a video +discontinuity) must not move audio's timeline. Today it re-anchors the shared +`Sync` while the audio ring stays on the old one. Folded in here rather than +fixed separately, since audio driving the clock removes the shared anchor. + ## Plan Moved from m0 to m1 in the 2026-09-30 audit: it waits on the whole jitter diff --git a/quest/m1/cmaf-sample-defaults.md b/quest/m1/cmaf-sample-defaults.md new file mode 100644 index 0000000000..8e2569ce49 --- /dev/null +++ b/quest/m1/cmaf-sample-defaults.md @@ -0,0 +1,38 @@ +# [S] CMAF sample defaults and keyframes resolve one way everywhere + +## Goal + +Every CMAF reader resolves a sample's size, duration, and flags the same way, +and agrees on which samples are keyframes. A fragment that keeps its defaults +only in `trex` decodes in Rust, and the importer opens a new group on every +sync sample the JS decoder would also treat as one. + +## Plan + +Today there are three rules (2026-10-07 audit): + +- `Wire::from_init` in `rs/moq-mux/src/container/fmp4/mod.rs` keeps only the + `trak` and drops `mvex/trex`, so `decode` falls back from `trun` to `tfhd` + only, and missing values become 0. A duration that only `trex` carries is + `MissingSampleDuration`, and a size that only `trex` carries is an empty + sample. The importer forwards `trex` in the catalog init and passes the + fragments through, so its own output can hit this. +- Rust `decode` reads only the `trun` entry flags, ignores the non-sync bit, + and needs `sample_depends_on == 2`. The importer's `is_sync_sample` + (`import.rs`) needs `depends_on == 2` with non-sync clear. JS + (`js/hang/src/container/cmaf/decode.ts`) treats flags 0 or a clear non-sync + bit as a keyframe. + +Decided (2026-10-07): one quest and one resolver. Resolve each of size, +duration, and flags as `trun`, then `tfhd`, then `trex`, in one place shared +by the importer and `Wire::decode`, and mirror it in JS. Use one sync rule in +all three. Add a fixture set that only Rust can generate today (defaults +only in `trex`, flags only in `tfhd`, a sync sample with `depends_on` unset) +and decode it in both languages. + +Public API: none. Wire: none. + +## Related + +- [CMAF frame timestamp](/quest/m1/cmaf-frame-timestamp.md) - touches the same `decode` functions; land either first and rebase +- [Export sync flags](/quest/m2/intra-refresh/export-sync-flags.md) - the export side of the same flags diff --git a/quest/m1/mkv-lacing.md b/quest/m1/mkv-lacing.md new file mode 100644 index 0000000000..cc32fb00c2 --- /dev/null +++ b/quest/m1/mkv-lacing.md @@ -0,0 +1,28 @@ +# [S] MKV import splits laced blocks into frames + +## Goal + +`moq import mkv` publishes each frame of a laced block as its own hang frame +with its own timestamp, so mkvmerge files (which lace audio by default) play. +A laced block whose per-frame duration cannot be determined is refused with +an error that names lacing. + +## Plan + +`rs/moq-mux/src/container/mkv/import.rs` passes `raw_frame_data()` straight +through for both SimpleBlock and BlockGroup. webm-iterable documents that this +includes the lace headers, so a laced block becomes one undecodable frame. +Each audio block is also marked a keyframe and then `cut`, so each block is +its own group. + +Decided (2026-10-07): split Xiph, EBML, and fixed-size lacing, and time the +frames after the first by the track's `DefaultDuration`. Refuse a laced block +on a track without `DefaultDuration` rather than guess from the codec. Check +whether webm-iterable exposes the lacing type and frame splits before +writing a parser. Test with an mkvmerge-style laced Opus or AAC fixture. + +Public API: none. Wire: none. + +## Related + +- [MKV export delay](/quest/m1/mkv-export-delay.md) - the export side of the same container diff --git a/quest/m1/papercuts-js.md b/quest/m1/papercuts-js.md new file mode 100644 index 0000000000..e079ab7a2f --- /dev/null +++ b/quest/m1/papercuts-js.md @@ -0,0 +1,36 @@ +# [S] JavaScript papercuts from the 2026-10-07 audit + +## Goal + +Three small JavaScript fixes from the 2026-10-07 code audit, each with a +regression test that fails without it: + +- `@moq/net` delivers an IETF object with status 0 and the end bit clear as an + empty frame and keeps reading the subgroup, as Rust does. +- A buffered audio rendition change settles while muted. +- An invalid attribute on `` or `` warns and falls + back to the default instead of throwing. + +## Plan + +- `js/net/src/ietf/object.ts` (around line 350) returns `new Frame()` with an + undefined payload for status 0, and `subscriber.ts` breaks out of the + subgroup on that. Rust delivers an empty object and continues + (`rs/moq-net/src/ietf/subscriber.rs`), and the JS file's own TODO agrees. + Only third-party publishers hit this, since Rust peers set the end bit. +- `js/watch/src/audio/decoder.ts` awaits `ring.wait` (around lines 462 and + 562) without racing the effect's abort. Mute freezes the playhead, so the + previous spawn never settles until something flushes the ring. Race it + against the effect. +- `js/watch/src/element.ts` and `js/publish/src/element.ts`: `new URL(bad)` + throws out of `attributeChangedCallback`, and `volume="foo"` parses to + `NaN`, which later throws inside the gain node. Per `js/AGENTS.md`, an + invalid value warns and falls back to the default, as `parseDelay` and + `parseVisible` already do. + +Public API: none. Wire: none. + +## Related + +- [Rust papercuts](/quest/m1/papercuts-rs.md) - the Rust half of the same audit +- [Audio jitter target](/quest/m0/audio-jitter-target/README.md) - owns the audio ring and its backpressure diff --git a/quest/m1/papercuts-rs.md b/quest/m1/papercuts-rs.md new file mode 100644 index 0000000000..1f1506de61 --- /dev/null +++ b/quest/m1/papercuts-rs.md @@ -0,0 +1,41 @@ +# [S] Rust papercuts from the 2026-10-07 audit + +## Goal + +Four small Rust fixes from the 2026-10-07 code audit, each with a regression +test that fails without it: + +- HTTP `/announced` and `/fetch` refusals count in the relay's refusal + metrics, as QUIC, WebSocket, and io_uring refusals already do. +- A replaced route whose newest group is `u64::MAX` keeps delivering that + group from the old copy instead of being capped at group 0. +- A future draft version sends stream-reset `GOING_AWAY` as 0x4, not as an + internal error. +- A `UDP_GRO` cmsg of 0 cannot panic an io_uring worker. + +## Plan + +- `rs/moq-relay/src/web.rs`: both `admit_http` call sites (`serve_announced` + and `serve_fetch`) return the error with `?` and never call + `cluster.refusals.record`. The missing-subscriber 401 in `serve_announced` + is uncounted too. Record each refusal the way `websocket.rs` does. +- `rs/moq-net/src/model/resume.rs` (around line 267): `Position::after_group` + returns `None` for the last group, meaning "no end", and + `unwrap_or_default()` turns that into position 0. Treat `None` as unbounded, + as `let_go` in the same file already does. lite-07 varints can carry + `u64::MAX`, so this is reachable from the wire. +- `rs/moq-net/src/ietf/error.rs`: `has_going_away` lists Draft18 through + Draft22 explicitly. Flip it to `!matches!(older drafts)` like the + request-error helper next to it, so a new draft variant falls forward. +- `rs/moq-uring/src/udp.rs`: `RecvMeta::parse` stores a stride of 0 as + `Some(0)`, and `Packet::segments` then calls `chunks_mut(0)`. Drop a zero + stride in the parser. Linux only emits `UDP_GRO` when `gso_size != 0`, so + this is defensive and lives in m1, not m0's peer-input hardening (decided + 2026-10-07). + +Public API: none. Wire: none (the `GOING_AWAY` change only affects drafts +that do not exist yet). + +## Related + +- [JS papercuts](/quest/m1/papercuts-js.md) - the JavaScript half of the same audit diff --git a/quest/m1/tokio-poll-closed.md b/quest/m1/tokio-poll-closed.md new file mode 100644 index 0000000000..d1dcb300d7 --- /dev/null +++ b/quest/m1/tokio-poll-closed.md @@ -0,0 +1,21 @@ +# [S] moq-tokio's stream adapter wakes a close-only waiter + +## Goal + +`poll_closed` in moq-tokio's stream adapter (used by WebSocket, TCP, and Unix +sockets) always registers the caller's waker before returning `Pending`, so a +task that only waits for the stream to close wakes when the peer sends a FIN, +even after the read-ahead cap is hit. + +## Plan + +`rs/moq-tokio/src/transport.rs` (around line 770) returns `Pending` without +registering `cx` once read-ahead is full. The comment there relies on the +caller's own reads re-polling, which is not part of the trait's contract. The +io_uring receive stream parks on its readable list instead. Nothing in the +repo is a close-only caller today, so the bug is in the contract, not in a +user-visible path. Register the waker, or park it the way io_uring does, and +add a test with mocked time: fill the read-ahead, FIN, and assert that a +`poll_closed`-only waiter wakes. + +Public API: none. Wire: none. diff --git a/quest/m1/watch-decode-error.md b/quest/m1/watch-decode-error.md new file mode 100644 index 0000000000..e99b4e6497 --- /dev/null +++ b/quest/m1/watch-decode-error.md @@ -0,0 +1,28 @@ +# [S] A WebCodecs error stops playback loudly + +## Goal + +When a video or audio decoder in `@moq/watch` errors, the track's subscription +ends and the element reports an error state, instead of video silently +stopping or audio decoding nothing while its subscription stays open. + +## Plan + +Today the video decoder's `error` callback (`js/watch/src/video/decoder.ts`, +around line 404) closes the track effect with a `// TODO bubble up error`, and +`#runActive` keeps the dead track. Audio logs the error, then its decode loop +`break`s while the subscription stays open. + +Decided (2026-10-07): surface the error and stop, rather than resubscribe. A +deterministic decode error would just loop under resubscription, and the repo +rule is to fail loudly. Both decoders end the subscription and set one visible +error signal that the element exposes. Reuse an existing error or status +signal on the element if there is one; otherwise propose the name in the PR. +Update `doc/` wherever the element's states are documented. + +Public API: likely one new or widened error/status signal on `@moq/watch`. +Wire: none. + +## Related + +- [Open-GOP leading pictures](/quest/m1/open-gop-leading-pictures.md) - removes one trigger of this error, not the handling diff --git a/quest/m2/README.md b/quest/m2/README.md index 06af5ba204..df8dccd030 100644 --- a/quest/m2/README.md +++ b/quest/m2/README.md @@ -23,11 +23,13 @@ waiting on an upstream release waits in [m4](/quest/m4/README.md). - [Publishers learn their cap](/quest/m2/rate-grant.md) - the AUTH grant carries the caps and publishers clamp their encoder to them - [One port on the io_uring workers](/quest/m2/uring-demux.md) - `moq-uring`'s workers host the UDP demux, so a ring relay keeps STUN, WebRTC, and SRT on its QUIC port - [Synced data playback](/quest/m2/watch-data-sync.md) - js/watch releases JSON and binary payloads on the media playhead, and a slow data track holds media back +- [Watch decode gate](/quest/m2/watch-decode-gate.md) - video lookahead stays encoded until it is near presentation, like audio - [Stream sessions](/quest/m2/uring-tcp/README.md) - serve WebSocket and HTTP from the io_uring workers, where io_uring pays off most - [Hitless TS legs](/quest/m2/ts-hitless.md) - two `--sync` export legs emit packet-identical TS for ST 2022-7 - [DVB E-AC-3](/quest/m2/ts-eac3.md) - E-AC-3 private data is split per sync frame and buffer-modelled in TS export - [T-STD controls](/quest/m2/tstd-controls.md) - the harness gains an MB-overflow control and an AAC broadcast reference - [MP4 export](/quest/m2/mp4-export.md) - `moq export mp4 --output` records crash-safe fragments, then finishes a regular MP4 with moov at the end +- [fMP4 edit lists](/quest/m2/fmp4-edit-lists.md) - the fMP4 importer applies edit lists to frame timestamps - [Linux decoded frames](/quest/m2/obs-decode-linux.md) - present supported native decoded surfaces with visible CPU fallback - [Windows decoded frames](/quest/m2/obs-decode-windows.md) - present decoded D3D11 surfaces in OBS without CPU readback - [macOS GPU input](/quest/m2/obs-macos.md) - feed the encoder from the OBS compositor without CPU readback diff --git a/quest/m2/fmp4-edit-lists.md b/quest/m2/fmp4-edit-lists.md new file mode 100644 index 0000000000..ac7967ffdc --- /dev/null +++ b/quest/m2/fmp4-edit-lists.md @@ -0,0 +1,32 @@ +# [M] fMP4 import applies edit lists to timestamps + +## Goal + +An fMP4 source with an edit list (AAC priming, a non-zero `media_time`, or an +empty edit) publishes frames at their presentation time, so both players and +every export see the timeline the source meant. + +## Plan + +Today `edts` is forwarded in the catalog init but read by no Rust or JS code, +and `rs/moq-mux/src/container/fmp4/export.rs` strips it on the way out. Both +players present `tfdt` plus the composition offset. ffmpeg's fragmented +output avoids `elst` by default, so this mostly affects GPAC, Bento4, and +Shaka sources (2026-10-07 audit). + +Decided (2026-10-07): the importer applies the edit list to each frame's +timestamp and drops `edts` from the catalog init, so no consumer needs edit +list code. This builds on decoders taking time from the frame timestamp +rather than `tfdt`. Decide in the PR whether priming samples before the edit +are dropped or published with negative-offset handling, and refuse edit +lists with more than one non-empty entry rather than guess. + +Public API: none. Wire: none (catalog init loses `edts`). + +## Required + +- [CMAF frame timestamp](/quest/m1/cmaf-frame-timestamp.md) - decoders time samples from the frame timestamp, which is where the edit lands + +## Related + +- [MP4 export](/quest/m2/mp4-export.md) - its open "edit lists" item is the output side diff --git a/quest/m2/watch-decode-gate.md b/quest/m2/watch-decode-gate.md new file mode 100644 index 0000000000..e5369a1078 --- /dev/null +++ b/quest/m2/watch-decode-gate.md @@ -0,0 +1,27 @@ +# [M] Watch video waits before decode, not after + +## Goal + +`@moq/watch` keeps its video lookahead encoded, as audio already does. Only +frames within the decoder's latency of presentation are decoded, so a long +`buffer` holds encoded chunks instead of one decoded `VideoFrame` per frame. + +## Plan + +Today the video decoder's output callback (`js/watch/src/video/decoder.ts`, +around line 380) awaits `sync.wait` before presenting and closing each frame, +so every buffered frame is held decoded. `Sync.received` caps this at +delay + buffer, so it only bites with a large `buffer`, where it can starve +the hardware decoder's frame pool (2026-10-07 audit). + +Decided (2026-10-07): wait before decode, like the audio `ring.wait`. Feed a +chunk to the decoder only once its presentation time is within the decoder's +measured output latency. Re-check against the watch worker plan, which moves +`Sync` off the main thread. + +Public API: none. Wire: none. + +## Related + +- [Watch worker](/quest/m1/watch-worker.md) - moves decode and `Sync` into a worker +- [A/V clock](/quest/m1/av-clock.md) - changes what `Sync` waits on From bf42eb800573639e6be40e00634cf075bf233491 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 7 Oct 2026 16:48:36 -0700 Subject: [PATCH 2/3] quest: address review on the audit plan Drop tokio-poll-closed (no close-only caller, and the cap makes the test unachievable). Tighten the resume, refusal, GOING_AWAY, CMAF flag, and edit-list wording. Co-Authored-By: Claude Opus 5.5 --- quest/m1/README.md | 1 - quest/m1/auth/ws-unauthorized.md | 5 +++-- quest/m1/cmaf-sample-defaults.md | 21 ++++++++++++--------- quest/m1/papercuts-rs.md | 19 ++++++++++++------- quest/m1/tokio-poll-closed.md | 21 --------------------- quest/m2/fmp4-edit-lists.md | 4 +++- 6 files changed, 30 insertions(+), 41 deletions(-) delete mode 100644 quest/m1/tokio-poll-closed.md diff --git a/quest/m1/README.md b/quest/m1/README.md index ab4aa8503b..5c5056ffa9 100644 --- a/quest/m1/README.md +++ b/quest/m1/README.md @@ -160,7 +160,6 @@ blocks. The quests that gated m0 lines moved under them. - [JS request deadline](/quest/m1/js-request-deadline.md) - each JS request, PUBLISH_NAMESPACE included, has one fatal 10 s timer from create to answer and fails fast without stream credit, so nothing queues or retries - [Rust request credit](/quest/m1/rs-request-credit.md) - a moq-net request fails at once without stream credit instead of waiting - [qmux no-wait opens](/quest/m1/qmux-no-wait.md) - @moq/qmux rejects an over-limit create when waitUntilAvailable is false, like Chrome -- [tokio poll_closed](/quest/m1/tokio-poll-closed.md) - moq-tokio's stream adapter wakes a close-only waiter after the read-ahead cap - [E2EE](/quest/m1/e2ee/README.md) - TypeScript and Rust peers interoperate over encrypted broadcasts no relay can decrypt - [#3056](/quest/m1/3056-watch-video-decoder-captures-the-rewind-generation-at.md) - watch: the video decoder resets on a declared discontinuity - [#933](/quest/m1/933-video-rotation-metadata-not-propagated-from-mobile-camera.md) - the catalog rotation follows the live camera's orientation diff --git a/quest/m1/auth/ws-unauthorized.md b/quest/m1/auth/ws-unauthorized.md index f62671fa02..7ac20c2bed 100644 --- a/quest/m1/auth/ws-unauthorized.md +++ b/quest/m1/auth/ws-unauthorized.md @@ -33,8 +33,9 @@ whose role its grant cannot serve (a publisher holding a subscribe-only grant) is upgraded and its broadcasts silently go nowhere, where QUIC refuses it in `Cluster::scope`; and a moqt-over-WebSocket client that authenticates only with the SETUP `AUTHORIZATION TOKEN` is treated as anonymous. Neither -grants extra access, so this stays in m1 (decided 2026-10-07). Copy the SETUP -token onto the auth request as the QUIC path does in `request_for`. +grants extra access, so this stays in m1 (decided 2026-10-07). Serving +through `Connection::run` already applies `request_for` and `Cluster::scope`, +so both close with no relay-side admit; the tests below pin them. Tests: a WebSocket connect with a bad token ends in a JS `SessionError` with the unauthorized code and the reload loop stops; a Rust client stops too. A diff --git a/quest/m1/cmaf-sample-defaults.md b/quest/m1/cmaf-sample-defaults.md index 8e2569ce49..af694d0bc5 100644 --- a/quest/m1/cmaf-sample-defaults.md +++ b/quest/m1/cmaf-sample-defaults.md @@ -17,18 +17,21 @@ Today there are three rules (2026-10-07 audit): `MissingSampleDuration`, and a size that only `trex` carries is an empty sample. The importer forwards `trex` in the catalog init and passes the fragments through, so its own output can hit this. -- Rust `decode` reads only the `trun` entry flags, ignores the non-sync bit, - and needs `sample_depends_on == 2`. The importer's `is_sync_sample` +- Rust `decode` reads only the `trun` entry flags (`entry.flags.unwrap_or(0)`), + ignoring `first_sample_flags`, the `tfhd` and `trex` default flags, and the + non-sync bit, and needs `sample_depends_on == 2`. The importer's `is_sync_sample` (`import.rs`) needs `depends_on == 2` with non-sync clear. JS - (`js/hang/src/container/cmaf/decode.ts`) treats flags 0 or a clear non-sync - bit as a keyframe. + (`js/hang/src/container/cmaf/decode.ts`) already resolves size, duration, + and flags through `trun` (including `firstSampleFlags`), `tfhd`, and `trex`, + but treats flags 0 or a clear non-sync bit as a keyframe. Decided (2026-10-07): one quest and one resolver. Resolve each of size, -duration, and flags as `trun`, then `tfhd`, then `trex`, in one place shared -by the importer and `Wire::decode`, and mirror it in JS. Use one sync rule in -all three. Add a fixture set that only Rust can generate today (defaults -only in `trex`, flags only in `tfhd`, a sync sample with `depends_on` unset) -and decode it in both languages. +duration, and flags as `trun` (with `first_sample_flags` for the first +sample), then `tfhd`, then `trex`, in one place shared by the importer and +`Wire::decode`, matching JS. Use one sync rule in all three; JS changes only +there. Add fixtures that Rust cannot decode today (defaults only in `trex`, +flags only in `tfhd` or `first_sample_flags`, a sync sample with `depends_on` +unset) and decode them in both languages. Public API: none. Wire: none. diff --git a/quest/m1/papercuts-rs.md b/quest/m1/papercuts-rs.md index 1f1506de61..cdf42675ca 100644 --- a/quest/m1/papercuts-rs.md +++ b/quest/m1/papercuts-rs.md @@ -17,16 +17,21 @@ test that fails without it: - `rs/moq-relay/src/web.rs`: both `admit_http` call sites (`serve_announced` and `serve_fetch`) return the error with `?` and never call - `cluster.refusals.record`. The missing-subscriber 401 in `serve_announced` - is uncounted too. Record each refusal the way `websocket.rs` does. + `cluster.refusals.record`. The missing-subscriber 401 in each handler is + uncounted too. Record each refusal the way `websocket.rs` does. - `rs/moq-net/src/model/resume.rs` (around line 267): `Position::after_group` returns `None` for the last group, meaning "no end", and - `unwrap_or_default()` turns that into position 0. Treat `None` as unbounded, - as `let_go` in the same file already does. lite-07 varints can carry - `u64::MAX`, so this is reachable from the wire. + `unwrap_or_default()` turns that into position 0. `until` is + `Option>`, and an inner `None` (replaced before any group) must + still end at 0; only `after_group`'s overflow is unbounded. Reuse `let_go`'s + `until.map_or(Some(0), |last| last.checked_add(1))` and test both cases. + lite-07 varints can carry `u64::MAX`, so this is reachable from the wire. - `rs/moq-net/src/ietf/error.rs`: `has_going_away` lists Draft18 through - Draft22 explicitly. Flip it to `!matches!(older drafts)` like the - request-error helper next to it, so a new draft variant falls forward. + Draft22 explicitly. Flip it to `!matches!(older drafts)` like + `has_too_far_behind` and `has_malformed_track` next to it, keeping Draft16 + and Draft17 excluded (0x4 is UNKNOWN_OBJECT_STATUS there), so a new draft + variant falls forward. This also changes `from_stream_code`; update its doc, + which says values a later draft may add stay `Unknown`. - `rs/moq-uring/src/udp.rs`: `RecvMeta::parse` stores a stride of 0 as `Some(0)`, and `Packet::segments` then calls `chunks_mut(0)`. Drop a zero stride in the parser. Linux only emits `UDP_GRO` when `gso_size != 0`, so diff --git a/quest/m1/tokio-poll-closed.md b/quest/m1/tokio-poll-closed.md deleted file mode 100644 index d1dcb300d7..0000000000 --- a/quest/m1/tokio-poll-closed.md +++ /dev/null @@ -1,21 +0,0 @@ -# [S] moq-tokio's stream adapter wakes a close-only waiter - -## Goal - -`poll_closed` in moq-tokio's stream adapter (used by WebSocket, TCP, and Unix -sockets) always registers the caller's waker before returning `Pending`, so a -task that only waits for the stream to close wakes when the peer sends a FIN, -even after the read-ahead cap is hit. - -## Plan - -`rs/moq-tokio/src/transport.rs` (around line 770) returns `Pending` without -registering `cx` once read-ahead is full. The comment there relies on the -caller's own reads re-polling, which is not part of the trait's contract. The -io_uring receive stream parks on its readable list instead. Nothing in the -repo is a close-only caller today, so the bug is in the contract, not in a -user-visible path. Register the waker, or park it the way io_uring does, and -add a test with mocked time: fill the read-ahead, FIN, and assert that a -`poll_closed`-only waiter wakes. - -Public API: none. Wire: none. diff --git a/quest/m2/fmp4-edit-lists.md b/quest/m2/fmp4-edit-lists.md index ac7967ffdc..91c62fc7a3 100644 --- a/quest/m2/fmp4-edit-lists.md +++ b/quest/m2/fmp4-edit-lists.md @@ -21,7 +21,9 @@ rather than `tfdt`. Decide in the PR whether priming samples before the edit are dropped or published with negative-offset handling, and refuse edit lists with more than one non-empty entry rather than guess. -Public API: none. Wire: none (catalog init loses `edts`). +Public API: none. Wire: none, but a catalog behavior change: the init +loses `edts`, so a third-party consumer that applied `elst` itself must stop, +or it shifts twice. Note it in the changelog. ## Required From 72b69b7791d76c73dc8b44785de626389a2da702 Mon Sep 17 00:00:00 2001 From: Luke Curley Date: Wed, 7 Oct 2026 17:29:43 -0700 Subject: [PATCH 3/3] quest: wording nits from the audit plan re-review Co-Authored-By: Claude Opus 5.5 --- quest/m1/papercuts-rs.md | 4 ++-- quest/m2/fmp4-edit-lists.md | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/quest/m1/papercuts-rs.md b/quest/m1/papercuts-rs.md index cdf42675ca..159968b5ef 100644 --- a/quest/m1/papercuts-rs.md +++ b/quest/m1/papercuts-rs.md @@ -28,8 +28,8 @@ test that fails without it: lite-07 varints can carry `u64::MAX`, so this is reachable from the wire. - `rs/moq-net/src/ietf/error.rs`: `has_going_away` lists Draft18 through Draft22 explicitly. Flip it to `!matches!(older drafts)` like - `has_too_far_behind` and `has_malformed_track` next to it, keeping Draft16 - and Draft17 excluded (0x4 is UNKNOWN_OBJECT_STATUS there), so a new draft + `has_too_far_behind` and `has_malformed_track` next to it, keeping Draft14 + through Draft17 excluded (0x4 is UNKNOWN_OBJECT_STATUS in 16 and 17), so a new draft variant falls forward. This also changes `from_stream_code`; update its doc, which says values a later draft may add stay `Unknown`. - `rs/moq-uring/src/udp.rs`: `RecvMeta::parse` stores a stride of 0 as diff --git a/quest/m2/fmp4-edit-lists.md b/quest/m2/fmp4-edit-lists.md index 91c62fc7a3..464f93d84c 100644 --- a/quest/m2/fmp4-edit-lists.md +++ b/quest/m2/fmp4-edit-lists.md @@ -21,9 +21,9 @@ rather than `tfdt`. Decide in the PR whether priming samples before the edit are dropped or published with negative-offset handling, and refuse edit lists with more than one non-empty entry rather than guess. -Public API: none. Wire: none, but a catalog behavior change: the init -loses `edts`, so a third-party consumer that applied `elst` itself must stop, -or it shifts twice. Note it in the changelog. +Public API: none. Wire: none, but a behavior change: frame timestamps now +include the edit and the init loses `edts`, so a consumer that offsets AAC +priming on its own must stop, or it shifts twice. Note it in the changelog. ## Required