Repository navigation
quest: plan follow-ups from the Oct 17 merge run #5884
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
d9a8572
quest: plan follow-ups from the Oct 17 merge run
kixelated 0fc5ab8
quest: widen request cleanup to Rust, make the OBS qlog option standa…
kixelated 4404c0f
quest: plan the IETF dead-route label, and answer review
kixelated 653b775
Merge remote-tracking branch 'origin/main' into quest/follow-ups-oct17
kixelated 3ac9fdc
quest: plan moq-net announced follow from #5700
kixelated e0ed59a
quest: settle the partial-group and hole-budget decisions, plan the a…
kixelated 116729b
quest: drop the anchor quest, cover Matroska crop and units, note JS-…
kixelated 4030bec
quest: settle the blind-fallback decision for announced follow
kixelated e1a005c
Merge remote-tracking branch 'origin/main' into quest/follow-ups-oct17
kixelated fde35af
quest: widen error cleanups to Rust's transport-failure code
kixelated 6ca6d93
quest: test local readers on partial groups, align the aspect precede…
kixelated 445cbdf
quest: settle the SPS-first aspect precedence
kixelated c899eb8
quest: hand path interest to lazy discovery, fail the qlog option on …
kixelated 2880292
quest: pin the epoch-pinned follow and HEVC aspect regressions
kixelated File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| # [S] moq-ffi reports why a first dial fails | ||
|
|
||
| ## Goal | ||
|
|
||
| While a moq-ffi client's first dial keeps failing and retrying, the caller can | ||
| read why, so the OBS dock names the reason instead of showing only | ||
| "Connecting…". moq-c already does this: `moq_session_connect` returns a | ||
| session before the first connect, and `moq_session_failure` names the | ||
| retrying dial. | ||
|
|
||
| Today the native `connect` in `rs/moq-ffi/src/session.rs` awaits | ||
| `connection.established()` before it returns a `MoqSession`, so | ||
| `MoqSession::failure()` (#5723) is unreachable until the first success. | ||
|
|
||
| ## Plan | ||
|
|
||
| Decided with the maintainer on 2026-10-17: the goal as drafted, in m1 | ||
| because OBS needs it. | ||
|
|
||
| The API shape is the maintainer's call; bring options with a recommendation. | ||
| Candidates: a failure accessor on `MoqClient` readable while `connect()` is | ||
| pending, returning the session before the first connect as moq-c does, or | ||
| failing the first dial fast with its error. The first is additive and keeps | ||
| `connect()` surfacing auth refusals. Follow the moq-ffi row of the | ||
| Cross-Package Sync table, ending with the dock in `cpp/obs`. | ||
|
|
||
| Test: a dial to a listener that hangs up reports its reason before any | ||
| session connects, and the report clears once one does. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| # [XS] No rustls panic on an http dial | ||
|
|
||
| ## Goal | ||
|
|
||
| An `http://` dial through moq-tokio's QUIC backend never panics for want of a | ||
| rustls crypto provider: it installs one before fetching the certificate | ||
| fingerprint. moq-ffi and moq-c reach this path. | ||
|
|
||
| Today the fetch of `/certificate.sha256` (`reqwest::get` in | ||
| `rs/moq-tokio/src/quic/backend.rs`) panics inside reqwest when the build | ||
| unifies reqwest's `rustls` feature and no process default is installed, for | ||
| example `cargo test -p moq-tokio -p moq-ffi -p moq-c --all-features`. | ||
|
|
||
| ## Plan | ||
|
|
||
| Decided with the maintainer on 2026-10-17: the goal as drafted, from the | ||
| follow-ups of [#5723](https://github.com/moq-dev/moq/pull/5723). | ||
|
|
||
| `moq_tokio::crypto::install_default()` installs the provider the crate | ||
| features select, and returns an error that can be ignored when one is already | ||
| installed. Add a regression test that dials `http://` under the unified | ||
| features and fails without the fix; moq-c's | ||
| `session_failure_names_a_failing_dial` dials `ws://` only to stay clear of | ||
| this. Make sure a CI lane builds that feature combination. |
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| # [S] Timed-out requests clean up on drafts 14-16 | ||
|
|
||
| ## Goal | ||
|
|
||
| When a request on moq-transport drafts 14-16 times out after it reached the | ||
| peer, Rust and JS both send the legacy cleanup message before the local | ||
| reset: PUBLISH_NAMESPACE_DONE for an advertisement, and UNSUBSCRIBE_NAMESPACE | ||
| for a namespace subscription on drafts 14-15. Unanswered advertisements stay | ||
| terminal, as #5741 decided; only the cleanup changes. | ||
|
|
||
| On drafts 14-16 a request rides a virtual control-stream stream whose reset | ||
| never reaches the peer, so today a timed-out PUBLISH_NAMESPACE is never | ||
| withdrawn and the peer can hold the namespace until the session closes: | ||
|
|
||
| - JS: `Deadline` (`js/net/src/util/deadline.ts`) aborts every stream it | ||
| recorded when the deadline fires, before the caller can write the cleanup. | ||
| - Rust: when `read_response` in `rs/moq-net/src/ietf/publisher.rs` gives up | ||
| on an advertisement, the request is dropped without PUBLISH_NAMESPACE_DONE. | ||
|
|
||
| ## Plan | ||
|
|
||
| Decided with the maintainer on 2026-10-17: | ||
|
|
||
| - The goal comes from the review of | ||
| [#5741](https://github.com/moq-dev/moq/pull/5741), which put the JS fix in | ||
| `Deadline`'s timeout path rather than in each caller. | ||
| - Widened to Rust and JS in one quest, rather than a separate Rust quest, | ||
| since both have the same gap on the same drafts. | ||
|
|
||
| The JS legacy SUBSCRIBE path opens its virtual stream outside `Deadline`, and | ||
| its timeout cleanup already sends UNSUBSCRIBE (`#cancelSubscribe`); keep that | ||
| covered by a test while moving the mechanism. Check every other timeout on | ||
| these drafts, in both languages, for the same gap. Draft 17 and later keep | ||
| the stream reset alone. | ||
|
|
||
| Test with mocked time on drafts 14, 15 and 16, in Rust and JS: a timed-out | ||
| PUBLISH_NAMESPACE whose message was sent is followed by PUBLISH_NAMESPACE_DONE | ||
| on the control stream, and the same for UNSUBSCRIBE_NAMESPACE on drafts | ||
| 14-15. Wire: no new messages. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| # [M] moq-net follows announced instances | ||
|
|
||
| ## Goal | ||
|
|
||
| moq-net offers the Rust equivalent of JS `request(path, { announced: true })` | ||
| (`js/net/src/origin.ts`): one request that follows each publisher instance | ||
| serving a path, unless an epoch is pinned. moq-ffi's | ||
| `MoqAnnouncedBroadcast::available()` (`rs/moq-ffi/src/origin.rs`) and moq-c's | ||
| `moq_origin_announced_broadcast` (`rs/moq-c/src/origin.rs`) use it and drop | ||
| their hand-rolled follow loops. | ||
|
|
||
| ## Plan | ||
|
|
||
| Decided with the maintainer on 2026-10-17: the goal as drafted, from | ||
| decision 5 of [#5700](https://github.com/moq-dev/moq/pull/5700), which kept | ||
| the loops per binding until moq-net owns them. Mirror the JS name. | ||
|
|
||
| The sink follow helper waits on this quest so it does not add a third loop; | ||
| the OBS source keeps calling `available()`, whose API does not change. | ||
|
|
||
| The loops #5700 added drain `origin::Consumer::follow`, resolve the path with | ||
| `routed_broadcast`, and settle three cases moq-net should own once: | ||
|
|
||
| - An `End` forgets the last instance, so the same instance returning after | ||
| a gap is handed out again. | ||
| - The last instance is held weakly, so a handle the caller let go of never | ||
| keeps its instance from retiring. | ||
| - A change that arrives while a resolution is in flight, such as an end and | ||
| a start for the instance it resolved, resolves afresh; and a resolution | ||
| that runs ahead of its held `Restart` is not handed out twice. | ||
|
|
||
| Two JS behaviors have no Rust counterpart to mirror. JS falls back to blind | ||
| resolution while a `consume` session lacks discovery; moq-net has no blind | ||
| requests. Decided with the maintainer on 2026-10-17: no blind fallback. The | ||
| mode waits on an announced route as `routed_broadcast` does, and blind | ||
| requests stay out of scope, so it differs from JS when discovery is | ||
| unavailable. JS also watches the path so a `consume` session asks its peer about | ||
| it. A moq-net session asks for its whole granted scope at setup today | ||
| (`interest_prefixes` in `rs/moq-net/src/lite/subscriber.rs`), so this quest | ||
| registers no interest. [Rust lazy discovery](/quest/m1/lazy-discovery/rust.md) | ||
| narrows that to watched prefixes, lands after this quest, and owns making | ||
| this mode watch its path, as JS does. | ||
|
|
||
| JS returns a handle whose current broadcast changes per instance | ||
| (`Request.active`); Rust's `request_broadcast` takes only an optional epoch | ||
| today. Propose the Rust shape in the PR. Keep #5700's binding tests passing | ||
| unchanged as the regression, and add moq-net tests for the three cases plus | ||
| a pinned epoch: an announced request that pins epoch A refuses a replacement | ||
| announced with epoch B instead of following it, mirroring JS `an | ||
| epoch-pinned announced request refuses another instance` | ||
| (`js/net/src/origin-request.test.ts`). | ||
| Public API: a new moq-net request mode or handle, and simpler binding | ||
| internals with no binding API change. Wire: none. | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| # [S] A partial group is never cached as whole | ||
|
|
||
| ## Goal | ||
|
|
||
| A subscription group whose max-delay budget expires before the publisher | ||
| finishes it fails with `Old` instead of ending normally, for every reader: | ||
| a local reader at the group's tail and a publisher serving it alike, on every | ||
| hop, in Rust and JS. A serving publisher resets the stream with `Old`, so a | ||
| relay that receives it never caches the prefix as the complete group. | ||
|
|
||
| Today a reader that has taken every frame the group holds treats expiry as | ||
| losing nothing and ends the group cleanly: `expired_truncates` in | ||
| `rs/moq-net/src/model/group.rs` and `#expire` in `js/net/src/group.ts`. The | ||
| lite publisher already resets with `Old` when a group expires before its | ||
| stream opens or while a write is blocked, but when it is parked at the tail | ||
| waiting for the next frame, `poll_next_frame` returns `Ok(None)` and | ||
| `GroupServe::poll_serve` (`rs/moq-net/src/lite/publisher.rs`) finishes the | ||
| stream. The next hop then stores the truncated group as finished. | ||
|
|
||
| ## Plan | ||
|
|
||
| Decided with the maintainer on 2026-10-17: the goal as drafted, from the | ||
| follow-up to [#5680](https://github.com/moq-dev/moq/pull/5680), which made an | ||
| expiring FETCH reset with `OLD` for the same reason. | ||
|
|
||
| Decided with the maintainer on 2026-10-17: every reader, local or served, | ||
| gets `Old` when an unfinished group expires. Rejected: resetting only on the | ||
| serving path, which leaves a local reader unable to tell a truncated group | ||
| from a whole one, and a separate expired-at-tail state, which adds API for | ||
| the same outcome. The clean end was chosen for a local reader stalled at a | ||
| group's tail; it goes, along with `expired_truncates`' distinction. Readers | ||
| already treat `Old` like a latency skip. | ||
|
|
||
| Add regression tests that fail without the fix: | ||
|
|
||
| - A Rust model test in `rs/moq-net/src/model/group.rs` and its JS twin for | ||
| `js/net/src/group.ts`: a local reader that has taken every frame of an | ||
| unfinished group receives `Old` when the group expires. | ||
| - An upstream group stalls past the budget after its last frame, and the | ||
| downstream hop holds no finished group for it. | ||
|
|
||
| Wire: no new codes; lite already resets with `Old`. |
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| # [S] Sequence holes get a time budget | ||
|
|
||
| ## Goal | ||
|
|
||
| A container consumer waiting on a missing group sequence gives up on it once | ||
| a budget expires on max(wall clock, pts), then reports the gap and what | ||
|
coderabbitai[bot] marked this conversation as resolved.
Resolved
|
||
| follows. A break marker behind a hole on a paused track is delivered without | ||
| waiting for media to resume or the track to finish. Rust | ||
| (`rs/moq-mux/src/container/consumer.rs`) and JS | ||
| (`js/hang/src/container/consumer.ts`) agree. | ||
|
|
||
| ## Plan | ||
|
|
||
| Decided with the maintainer on 2026-10-17: the goal as drafted, from the | ||
| review of [#5815](https://github.com/moq-dev/moq/pull/5815). The budget covers | ||
| every kind of hole, not only one before a data marker, and runs on | ||
| max(wall, pts) so it fires on a paused track. | ||
|
|
||
| Today a marker carries no timestamp, so a hole before it on an open, paused | ||
| track waits indefinitely. A buffered higher sequence alone proves nothing, | ||
| since a late group is not a lost one; the budget is what decides. | ||
|
|
||
| The two anchors, matching the timers the TS line already uses: | ||
|
|
||
| - PTS: the existing age measure, how far the newest content | ||
| (`max_timestamp`) runs past where the next stamped group begins | ||
| (`next_start`), in `rs/moq-mux/src/container/consumer.rs`, and its | ||
| counterpart in `js/hang/src/container/consumer.ts`. A bare marker has no | ||
| timestamp, so this side never fires behind one. | ||
| - Wall: the `moq_clock` instant the reader first found itself blocked on | ||
| the hole, the way the TS export anchors a frame's arrival at the instant | ||
| its source was last found empty (`Track::empty` in | ||
| `rs/moq-mux/src/container/ts/export.rs`, judged by `jitter::Buffer` in | ||
| `rs/moq-mux/src/jitter.rs`). A timer wakes the reader at the anchor plus | ||
| the budget, with no new group needed. | ||
|
|
||
| The hole is lost once either side reaches the budget. Decided with the | ||
| maintainer on 2026-10-17: the budget is the subscription's max delay, which | ||
| already drives latency skips, so both sides share one number. With the | ||
| default of zero, a hole counts as lost as soon as the reader blocks on it. | ||
|
|
||
| Test with mocked time in both languages: read group N, lose N+1, receive a | ||
| closed marker as N+2 on a paused track; after the budget the reader reports a | ||
| gap, then the end. It fails without the fix. | ||
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.