Merge main into dev - #2852
Conversation
…frame (#2768) Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
#2795) Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…#2782) Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…ver (#2785) Co-authored-by: Luke Curley <kixelated@gmail.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…rs forever (#2804) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…hem (#2803) Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: moq-bot[bot] <186640430+moq-bot[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Codex <noreply@openai.com>
…ditions (#2805) Co-authored-by: Luke Curley <kixelated@gmail.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…2801) Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Luke Curley <kixelated@gmail.com> Co-authored-by: GPT-5 <noreply@openai.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…2844) Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Jakub Perżyło <jakub.perzylo@swmansion.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20798fd9d8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // dial as before failover existed. `new` warns about this once. | ||
| if self.bind.port() != 0 { | ||
| candidates.truncate(1); | ||
| candidates = candidates.with_limit(1); |
There was a problem hiding this comment.
Preserve DNS preference for single-attempt quiche dials
When quiche uses a nonzero --connect-bind port, this limit can select the IPv4 fast-path result before the authoritative all-family lookup finishes. If A resolves more than resolution_delay before AAAA, but the platform would rank a working IPv6 address first and the IPv4 path is broken, the sole IPv4 attempt fails and the later IPv6 result is never tried. Previously this mode waited for the complete resolver result before truncating to its first address; the single-attempt path should likewise wait for that result rather than using the speculative fast lane.
Useful? React with 👍 / 👎.
…unce loop Two defects the merge introduced, found reviewing it. `container::Producer::with_latency` is buffering (packing several samples into one moof+mdat); dev renamed it `with_buffer`. The resolution had substituted `track::Info::with_latency_max`, which is cache retention, so `fetch_media_group_decodes_multiple_cmaf_samples` and its moq-mux twin stopped exercising multi-sample parsing while still passing. main's unsolicited announce loop broke when `#broadcasts` went undefined, which `Publisher.close()` did on session close. dev borrows the origin instead, which outlives the session and has neither that method nor its caller, so the merged loop had no ending: a closed connection left it parked on a shared origin, waking on an unrelated publish to fail against a dead transport. It now races the session's own closed promise, like the solicited loop does with its stream. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`Subscription::latency` was metadata: moq-net put it on the wire and never acted on it, so only `moq_mux::container::Consumer` ever skipped a stale group. A relay hop, a `moq-json` track, a catalog, or a raw `track::Subscriber` got no skipping whatever budget it declared. That matters now because #2852 deliberately reverted #2785 on dev: a takeover boundary becomes subscription demand again, unbounded. `start`/`end` are filters rather than requests for reliability, so the ceiling on a takeover backlog has to come from the latency budget instead. See #2784. Enforcement lives in one place, `TrackState::is_stale`, reached by every read that hands content over: `poll_recv_group`, `poll_next_group`, and the frame-level `poll_read_frame`. Both ends of a subscription get there, since a relay serves downstream through `track::Subscriber` (#2771) and a local consumer reads the same way. Neither end alone suffices: `Latency::merge` takes the most tolerant budget, so what reaches a publisher is only ever as tight as its most patient viewer, and the subscriber applies it again where its own budget is exact. `fetch_group` stays exempt, naming one old group rather than following a live edge. An explicit `start` gets no exemption either, which is what lets a publisher apply the budget without having to tell a requested start from a resume-synthesized one. Drift is measured in presentation time: a group's first frame timestamp against the highest-sequence group above it that has one. Both are stamped once, when a group's first frame is created, so a backlog delivered as a burst still reads as its true age. The anchor must sit strictly above the candidate, or backfill and the tail of a rewound timeline convict the groups that follow them. It is resolved once per poll rather than per candidate: the highest-sequence stamped group in range is the anchor for every group below it, so walking a backlog of N groups off stays linear rather than quadratic. The anchor is capped by what the subscription could actually be served, since a reader that cannot jump past a boundary is not late against what lies beyond it. That means the read cursor's `end_at`, the requested `Subscription::end`, and -- for a spliced segment, which is deliberately left uncapped so its completion stays visible -- the cap its reader imposes from outside. A group parked above a cap is re-checked when the cap rises, since the live edge moved while it waited. `clamp_latency` becomes the single clamp point, applied to the aggregate the publisher sees and to one subscriber's own budget alike; the two agree because `min` distributes over the `max` that combines them. Skips are counted as `stats.stale`, disjoint from `groups`. They accumulate on the cursor and are drained by the `track::Subscriber` that owns the stats scope: a spliced subscriber reads through untagged per-segment cursors, so that wrapper is the only place attribution happens exactly once. Accepted limits, documented rather than fixed: - On pre-Lite05 drafts a receiver can only stamp frames on arrival, so the measure is burst-blind there: thirty seconds of backlog delivered in three reads as three. The publisher's copy is stamped as it produces, so the gate there still holds, but only at aggregate granularity. - A group that has presented nothing is never stale, and neither is one on a track where nothing newer has. With no timestamp there is nothing to measure and the frames may simply not have arrived; the publisher's retention window bounds such a group instead. This is narrower than the draft's Expiration section, which also wants a wall-clock backstop. `moq_mux::container::Consumer::with_latency` and `set_latency` only ever set a local field, so a tolerance never reached the subscription and the transport would now skip the very groups the consumer was told to wait for. They push it through, which also fixes moq-rtmp's 2s default being silently ignored. Tests that assert every group arrives now say so with a budget, which is stronger than the default REAL_TIME promises: the cluster failover test, the resume backfill test, and the moq-mux/moq-audio/moq-cli/moq-video batch pipelines that write a whole broadcast before reading it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Resolves 27 conflicts from the 11 commits `main` gained since moq-dev#2852. The non-mechanical ones: **Catalog broadcast references.** Two independent changes, both kept. main moq-dev#2855 changed the *resolution algorithm* to relative-URL semantics (a non-empty reference replaces the base's last segment, then `.`/`..` apply); dev moq-dev#2630 changed *escape handling* to reject the whole catalog. main already ships both `resolve` (clamping) and `try_resolve` (`Option`), so its `path.rs` is taken wholesale and dev's callers point at `try_resolve`. This shifts what escapes by one segment, which is why several tests move from `../x` to `./x`. The escape policy is two-sided rather than one choice. main's `retain_valid`/`resolve_reference` is publisher-side sanitisation with 9 call sites across moq-mux, moq-hls, and moq-rtc: kept. dev's `EscapingBroadcast` plus `Source::target` returning `Result` is consumer-side rejection: also kept, now layered on `try_resolve`. main's duplicate `InvalidBroadcastReference` variant is dropped for dev's better-documented `EscapingBroadcast`. `Source` grows a `request`/`try_request` pair mirroring `resolve`/`try_resolve`, so the exporters keep skipping one bad rendition while consumers keep reporting the fault. `js/watch` is a pure consumer, so it takes the reject policy alone; main's `filterCatalog` is dropped (it also missed the `text` section). Its `findEscaping` had to move to `Path.tryResolve`, since main's `resolve` clamps and would have silently disabled the check. **`Connecting` removal.** main moq-dev#2856 deletes the mechanism; dev had rewritten the same thing into its poll driver (`SubscriberDriver`, `AnnouncePrefix`, 27 references). main's removal wins, applied on top of dev's driver: the per-prefix producer, the `initial_count`/`initial_remaining` bookkeeping that existed only to release it, and the `client.rs` wait sites all go, keeping dev's `goaway` plumbing. **Fixes carried across a rewrite.** main moq-dev#2862's live-edge cursor fix landed in an async `run_track` that dev deleted, so it is reapplied to dev's poll-based `TrackServe::new`; its regression test now drives that. main moq-dev#2841's WebSocket `SessionInputs` refactor gains dev's `shutdown` field. main moq-dev#2874's target-carrying `DialSources` gains dev's mDNS source, and `run_mdns` moves to `upsert`/`release` (dev's 2-arg `release` was superseded). The hang draft and docs state main's URL resolution semantics *and* dev's reject-the-catalog rule. `just check` and `just test` pass: 3185 tests, 0 failures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Summary
devhad not mergedmainsince9bfb4a480, so it was missing #2748, #2749, #2785, #2801, #2804, #2805, #2809, #2811, #2825, #2827, #2837, #2844 and the releases in between.Merge with a merge commit, not squash. Squashing collapses both parents and makes the next
mainmerge re-diff everything already landed here.The substantive decision is
slice()inrs/moq-net/src/model/resume.rs, where the two branches disagree about what a takeover boundary means.dev's rule is kept, and #2785 is deliberately reverted on this branch. Reasoning below, since that is a mergedmainfix being undone, and it will reachmainat the nextdevmerge.The takeover boundary stays demand (reverting #2785)
main#2785 changedslice()so a takeover boundary never becomes a subscription start: a live-edge subscriber that failed over asked the replacement for the live edge rather than for the boundary. It fixed #2784, where a reconnect replayed 12 seconds of stale media that downstream QoS then dropped.The diagnosis in #2784 was right; the fix is at the wrong layer.
start/endare filters, not requests for reliability. They bound what a subscriber is served, not a guarantee it receives all of it. A takeover boundary entering the demand aggregate is therefore not a request to replay an outage, and the ceiling on how much gets replayed belongs to the latency budget rather than to the range.resumeputs a media policy in the transport.main's rule is incompatible with frame-precise splicing.dev's boundaries carry a frame offset and the read cursor is group-granular, so the offset can only reach the replacement as demand. Undermain's ruletakeover_splices_mid_groupandtakeover_redelivers_an_incomplete_frameboth fail: a track whose current group never rolls (a JSON append log, a catalog with deltas) cannot survive a route change at all.What that leaves exposed, stated plainly rather than argued away:
#2784's own trigger cannot recur on
dev. Its root cause step 1 is origin linger holding the logical broadcast alive across a relay restart;devremoved linger in #2704 and this merge deletes the six linger tests with it, includingtest_linger_reconnect_splices, the test #2784 names as encoding the behavior. Detaching the last source now closes the broadcast synchronously, so the single-relay repro ends the subscription instead of splicing.What remains is narrower and real: a preferred route that stalls while still alive, with a standby advancing behind it, leaves the boundary trailing the standby's live edge by more than failover latency, and the takeover requests that gap.
moq_mux::container::Consumerdrops it for media consumers on its own drift budget, but a relay hop, amoq-jsontrack, or a rawtrack::Subscriberhas no such filter. This branch accepts that exposure. The fix is to enforceSubscription::latencyinmoq-netitself (capped byInfo::latency_max, preferring the moq-lite-05 presentation timestamp and falling back to a creation stamp, applied at both publisher and subscriber), which is a follow-up PR rather than something to bury in a merge. #2784 has a note pointing at it.Tests updated to match, all of which assert demand rather than delivery:
resume::test::takeover_after_produced_segment_keeps_live_edge->..._resumes_at_the_boundary, keeping its coverage that a below-boundary group from the replacement is filtered.takeover_keeps_an_explicit_startandtakeover_after_empty_segment_keeps_live_edgeare unchanged; both hold under either rule.origin_implroute/failover/standby/reconnect assertions back to the boundary.moq-native::broadcast::broadcast_route_migration, fix(net): keep a live-edge subscriber at the live edge across a takeover #2785's end-to-end regression, now expects["b2", "b3"]. The subscriber there has a 10s latency tolerance and B holds the continuation of the same content, so skippingb2was discarding a group the subscriber had asked to wait for.cluster_diamond_goaway_seamless_failoverpasses unchanged, which is the property this preserves: a cluster swap loses no group. It asserts completeness for aREAL_TIMEsubscriber, which is stronger than that budget promises; it holds today because nothing enforces the budget, and it gains an explicit tolerance in the enforcement PR.Other conflict resolutions worth review
rs/moq-net/src/model/resume.rs:main's fix(net): surface a dropped resume producer instead of stalling readers forever #2804 hardening (theSnapshotstruct,closed->Error::Droppedafter drain,Fetching::poll_latch) grafted ontodev'sPositionmodel. The parked-groupwatchclosure moved above the loop, sincedeviterates by index forhand_out's&mut self.rs/moq-net/src/model/group.rs:Consumer::poll_closedauto-merged ontoself.state, which does not exist underdev'sConsumerKindsplit. Reimplemented per-variant; a spliced group reads as closed without registering, since no single abort empties it andis_abortedcan never turn true.rs/moq-net/src/ietf/publisher.rs:Target/Namespaces(new in feat(net): announce namespaces unasked, with a SETUP opt-out #2748) retargeted fromweb_transport_trait::Sessiontodev'stransport::poll::Session, andTarget::closedconverted topoll_closed. The announce loop destructuresNamespacesto split the borrow between the close watch and the route sweep.rs/moq-net/src/lite/test_transport.rs: fix(net): surface a dropped resume producer instead of stalling readers forever #2804'sopen_gateported to the poll interface with akio::Parkto hold the registration across calls.rs/moq-cli: feat(cli): run multiple import/export stages over one connection #2809's multi-stage pipeline meetsdev's--cluster-lan.main'sDirections { publish, consume }replacesdev's two-variantDirectionand is threaded throughspawn_server/spawn_serve/cluster::serve, so the LAN mesh can serve both directions.spawn_moqdials viaClient::connectrather thanpublish/consume, which each attach one direction.rs/moq-native: feat(native): start dialing before the AAAA answer lands #2749's Happy Eyeballs resolution delay moved intoconnect.rsalongside refactor(moq-native)!: name the endpoint roles connect and listen #2750's rename, as--connect-resolution-delaywith--client-resolution-delaykept as a hidden legacy spelling folded inresolved()(an alias would not carry the env var).Fixed while reviewing the merge (403acc7)
Two defects the merge itself introduced, both invisible to the conflict list because they auto-merged and compiled:
container::Producer::with_latencyis buffering, not retention. It packs several samples into one moof+mdat;devrenamed itwith_buffer. The first resolution substitutedtrack::Info::with_latency_max, which is cache retention, sofetch_media_group_decodes_multiple_cmaf_samples(feat(bindings): fetch and decode a retained media group #2827) and itsmoq-muxtwin kept passing while no longer exercising multi-sample parsing at all.main,Publisher.#broadcastsis a session-ownedSignalandPublisher.close()(called fromconnection.ts) set it toundefined, which is exactly the loop's exit condition.devborrows the origin instead (feat(js/net)!: route publish and consume through Origins, share one connection per relay #2705), which outlives the session and has neither that method nor its caller, so the merged loop could not be stopped: a closed connection left it parked on a shared origin, waking on an unrelated publish to fail against a dead transport. It now races the session's own closed promise, like the solicited loop does with its stream. Regression test included, verified to fail without the fix.Public API changes
None beyond what the two branches already carried, except on
moq-native, where #2749's knob lands underdev's new names:connect::Config::resolution_delay/resolved_resolution_delay(rather thanClientConfig::*), flag--connect-resolution-delay, envMOQ_CONNECT_RESOLUTION_DELAY.--client-resolution-delay/MOQ_CLIENT_RESOLUTION_DELAYkeep working, hidden.Test plan
just check: clean, including the wasm32 gate and rustdoc.just test: 3142 Rust tests passed, JS 0 fail across every package, 52 Python passed.cluster_diamond_goaway_seamless_failoverandbroadcast_route_migrationboth pass.the announce loop outlived its session.Cross-package sync
No wire format change, so no draft update.
doc/bin/cli.md,doc/bin/relay/config.md, andrs/moq-cli/README.mdreconciled: the multi-stage docs from #2809 now use--connect/--listen, and the resolution-delay knob documents its new spelling.(written by Opus 5)