diff --git a/dart/moq_ffi/lib/src/moq.dart b/dart/moq_ffi/lib/src/moq.dart index 1ba1e2385d0..6df66c89d0d 100644 --- a/dart/moq_ffi/lib/src/moq.dart +++ b/dart/moq_ffi/lib/src/moq.dart @@ -4931,9 +4931,8 @@ class FfiConverterMoqAnchor { class AbsoluteMoqAnchor extends MoqAnchor { final int group; - final int frame; - AbsoluteMoqAnchor({required int this.group, required int this.frame}); - AbsoluteMoqAnchor._(int this.group, int this.frame); + AbsoluteMoqAnchor(int this.group); + AbsoluteMoqAnchor._(int this.group); static LiftRetVal read(Uint8List buf) { int new_offset = buf.offsetInBytes; final group_lifted = FfiConverterUInt64.read( @@ -4941,12 +4940,7 @@ class AbsoluteMoqAnchor extends MoqAnchor { ); final group = group_lifted.value; new_offset += group_lifted.bytesRead; - final frame_lifted = FfiConverterUInt64.read( - Uint8List.view(buf.buffer, new_offset), - ); - final frame = frame_lifted.value; - new_offset += frame_lifted.bytesRead; - return LiftRetVal(AbsoluteMoqAnchor._(group, frame), new_offset); + return LiftRetVal(AbsoluteMoqAnchor._(group), new_offset); } @override @@ -4958,9 +4952,7 @@ class AbsoluteMoqAnchor extends MoqAnchor { @override int allocationSize() { - return FfiConverterUInt64.allocationSize(group) + - FfiConverterUInt64.allocationSize(frame) + - 4; + return FfiConverterUInt64.allocationSize(group) + 4; } @override @@ -4971,10 +4963,6 @@ class AbsoluteMoqAnchor extends MoqAnchor { group, Uint8List.view(buf.buffer, new_offset), ); - new_offset += FfiConverterUInt64.write( - frame, - Uint8List.view(buf.buffer, new_offset), - ); return new_offset; } } diff --git a/doc/concept/moq-lite.md b/doc/concept/moq-lite.md index 8bf6311437e..64412e606a7 100644 --- a/doc/concept/moq-lite.md +++ b/doc/concept/moq-lite.md @@ -254,7 +254,7 @@ Each subscription carries the knobs that decide behavior under congestion: | **Priority** (-128..127, default 0) | Higher-priority tracks get bandwidth first. Audio above video, base layer above enhancement. The publisher's track priority breaks ties between subscriptions at one priority. From a native Rust publisher over QUIC or WebTransport (iroh excepted), subscriptions at the same priority and publisher priority split bandwidth evenly by bytes, streams and datagrams alike, however many groups each has queued. A browser publisher and other transports without send groups are not covered. | | **Order** | Which group to send first when several are pending. Newest first for live (the default), oldest first for catch-up. Needs moq-lite 07 or moq-transport; older moq-lite always sends newest first. | | **Max delay** | How long a non-latest group stays worth delivering before it is skipped. Zero means "live edge only", and raising it is also what asks for history. Unset leaves the publisher's retention window alone to bound it. | -| **Ranges** | Which groups to ask for: from a group, a number of groups back from the latest, or the group containing a presentation time, optionally up to an end. The default asks for everything max delay allows. Max delay still caps every range. | +| **Ranges** | Which groups to ask for: from a group, a number of groups back from the latest, or the group containing a presentation time, optionally up to an end. Each group arrives whole, from its first frame. The default asks for everything max delay allows. Max delay still caps every range. | On moq-lite 05 and later, a publisher explicitly reports groups that will not arrive. This lets a finished subscription settle without waiting for missing groups. diff --git a/drafts/draft-lcurley-moq-lite.md b/drafts/draft-lcurley-moq-lite.md index 7dc5acb7547..31d894cb187 100644 --- a/drafts/draft-lcurley-moq-lite.md +++ b/drafts/draft-lcurley-moq-lite.md @@ -211,15 +211,15 @@ The resolved start is therefore always either exactly the requested Position or The asymmetry with Groups is deliberate. A Group is the unit of decodability: dropping leading *Groups* leaves a stream the application can still decode, whereas dropping leading *Frames* often does not, whether because the Group opens with a keyframe the rest depends on or because its compression state lives in the frames that went missing. -Only the subscriber knows whether a partial Group is any use to it, so only the subscriber may ask for one. +A subscription therefore starts at a Group's first frame, and a subscriber asks for a partial Group only to finish one whose earlier frames it already holds, from this publisher or another. Frame indices are only meaningful relative to a Group the subscriber has already begun receiving, so: - A `Frame Start` qualifies the group `Group Start` names, group 0 included. - A subscriber that has seen no such group MUST send `Frame Start` = 0, since it cannot number the frames of a group it has not received. + A subscriber MUST send `Frame Start` = 0 unless it already holds every frame of that group below it. - A `Frame End` is meaningless without a `Group End`; an unbounded subscription (`Group End` = 0) MUST send `Frame End` = 0. -A publisher MUST close the session with a PROTOCOL_VIOLATION if either rule is violated. +A publisher MUST close the session with a PROTOCOL_VIOLATION on a non-zero `Frame End` without a `Group End`. # Flow This section outlines the flow of messages within a moq-lite session. @@ -1271,7 +1271,7 @@ A non-zero value is the absolute group sequence + 1. The index of the first frame to deliver within the `Group Start` group (see [Positions](#positions)). A value of 0 means from the start of that group (default), so the group is delivered whole. Frames before this index are not delivered, even when delivery begins at that exact group; a start resolved at a later group is always delivered from frame 0. -A subscriber that has received no group MUST send 0, since it cannot number the frames of a group it has not seen. +A subscriber MUST send 0 unless it already holds every frame below this index, from this publisher or another, since a Group is the unit of decodability (see [Positions](#positions)). **Frame End**: The last frame to deliver (inclusive) within the end group (see [Positions](#positions)). @@ -1811,6 +1811,7 @@ The `Message Length` describes the payload size on the wire. - Added TRACK_INFO `Refresh` and SUBSCRIBE_UPDATE `Wanted Group`, carrying ratio-based refresh demand through relays. - Added `Compression` to TRACK_INFO: none (0x0) or DEFLATE (0x1), whose Groups are DEFLATE streams per draft-lcurley-moq-flate. FRAME and datagrams carry an `Inflated Size` when it is not none. Earlier versions receive the Track inflated. +- A subscription starts at a Group's first frame: SUBSCRIBE carries a non-zero `Frame Start` only when the subscriber holds every frame below it, compressed Track or not. - Replaced ANNOUNCE_OK Active Count with an ordered, empty SYNCED event (0x4), marking the initial set without retaining routes that end before delivery. diff --git a/go/wrapper/types.go b/go/wrapper/types.go index 96699063136..3c082b6b1cd 100644 --- a/go/wrapper/types.go +++ b/go/wrapper/types.go @@ -62,7 +62,7 @@ type ( Range = ffi.MoqRange // Anchor is where one end of a Range falls: AnchorAbsolute, AnchorRelative, or AnchorTime. Anchor = ffi.MoqAnchor - // AnchorAbsolute is an exact group and frame. + // AnchorAbsolute is a group, delivered from its first frame. AnchorAbsolute = ffi.MoqAnchorAbsolute // AnchorRelative is the group Groups before the latest one, clamped to group 0: 0 is the latest. AnchorRelative = ffi.MoqAnchorRelative diff --git a/kt/moq/src/jvmAndAndroidMain/kotlin/dev/moq/Aliases.kt b/kt/moq/src/jvmAndAndroidMain/kotlin/dev/moq/Aliases.kt index 5a07eb3dd09..9863dc256ca 100644 --- a/kt/moq/src/jvmAndAndroidMain/kotlin/dev/moq/Aliases.kt +++ b/kt/moq/src/jvmAndAndroidMain/kotlin/dev/moq/Aliases.kt @@ -109,7 +109,7 @@ typealias Subscription = uniffi.moq.MoqSubscription typealias Range = uniffi.moq.MoqRange /** Where one end of a [Range] falls: [AnchorAbsolute], [AnchorRelative], or [AnchorTime]. */ typealias Anchor = uniffi.moq.MoqAnchor -/** An exact group and frame. */ +/** A group, delivered from its first frame. */ typealias AnchorAbsolute = uniffi.moq.MoqAnchor.Absolute /** The group `groups` before the latest one, clamped to group 0: 0 is the latest. */ typealias AnchorRelative = uniffi.moq.MoqAnchor.Relative diff --git a/py/moq-rs/tests/test_local.py b/py/moq-rs/tests/test_local.py index d22f95453bc..46e3cf25278 100644 --- a/py/moq-rs/tests/test_local.py +++ b/py/moq-rs/tests/test_local.py @@ -12,7 +12,7 @@ import pytest # Every group from the first, rather than only what the live edge still reaches. -FROM_GROUP_0 = [moq.Range(start=cast(moq.Anchor, moq.Anchor.ABSOLUTE(group=0, frame=0)))] +FROM_GROUP_0 = [moq.Range(start=cast(moq.Anchor, moq.Anchor.ABSOLUTE(group=0)))] async def until_wanted(demand: moq.TrackDemand, wanted: bool = True) -> None: diff --git a/py/moq-rs/tests/test_testing_clock.py b/py/moq-rs/tests/test_testing_clock.py index 8dd4a345221..e5898428c9e 100644 --- a/py/moq-rs/tests/test_testing_clock.py +++ b/py/moq-rs/tests/test_testing_clock.py @@ -26,7 +26,7 @@ async def main(): # The origin's driver runs the timers that judge a group stale. origin = moq.OriginProducer() track = origin.create_broadcast("clock").publish_track("track") - from_group_0 = [moq.Range(start=moq.Anchor.ABSOLUTE(group=0, frame=0))] + from_group_0 = [moq.Range(start=moq.Anchor.ABSOLUTE(group=0))] consumer = track.consume(moq.Subscription(max_delay=timedelta(seconds=1), ranges=from_group_0)) # Group 0 stalls with no frame, so the in-order read waits on it until diff --git a/quest/m1/compress/README.md b/quest/m1/compress/README.md index 6e90dae99ac..63bdb35c992 100644 --- a/quest/m1/compress/README.md +++ b/quest/m1/compress/README.md @@ -52,12 +52,12 @@ Every interview prompt and the selected answer (✅) are in the planning PR. parameter get the inflated copy, with no flag. A sealed compression declaration is preserved, so delivery is refused when plain fallback would remove it. -- **Ranges.** A range starting at frame N of a compressed group is served - from N. The requester only asks from N because it holds 0..N-1, so it - inflates from its own cached prefix. Reading frame N without 0..N-1 is a - non-goal: groups are delta encoded anyway. The requesting model enforces - it: a start on a compressed group it lacks the prefix for widens to frame - 0 before the request leaves, and a server serves exactly what is asked. +- **Ranges.** Compressed tracks are not special: every subscription, new or + resumed, starts at the beginning of a group, and one resuming from another + route may take the group's start from that route's cached prefix + (maintainer, on [moq-dev/moq#6465](https://github.com/moq-dev/moq/pull/6465)). Application + ranges and fetches name whole groups, so a compressed group always + inflates from a prefix its requester holds. - **Specs:** draft-lcurley-moq-flate keeps the codec and becomes the IETF extension; draft-lcurley-moq-lite specifies the lite-07 field and size and references it for the codec. @@ -81,7 +81,6 @@ This README owns the end-to-end proof and the guide once every child lands: ## Required -- [moq-transport mid-group subscriptions widen](/quest/m1/compress/ietf-range-widen.md) - an application range starting mid-group on a compressed track widens to frame 0 at a moq-transport subscriber instead of failing with `Lagged` - [Migrate](/quest/m1/compress/migrate.md) - catalogs, stats, and timelines publish one compressed track; moq-flate, @moq/flate, and every `.z` name are gone ## Related diff --git a/quest/m1/compress/ietf-range-widen.md b/quest/m1/compress/ietf-range-widen.md deleted file mode 100644 index 62b818c0fc0..00000000000 --- a/quest/m1/compress/ietf-range-widen.md +++ /dev/null @@ -1,58 +0,0 @@ -# [M] A moq-transport mid-group subscription to a compressed track widens to frame 0 - -## Goal - -An application range starting mid-group on a compressed track, requested over -moq-transport draft-17+ with the Flate extension negotiated, reads the frames -it asked for instead of failing with `Lagged` against a publisher that serves -exactly the requested Object range. The requesting IETF subscriber widens the -SUBSCRIBE start to Object 0 when it lacks the group's prefix, as lite-07 does -since [moq-dev/moq#6242](https://github.com/moq-dev/moq/pull/6242). The server -never widens and keeps serving exactly what is asked. - -## Plan - -The rule is the [epic](/quest/m1/compress/README.md)'s Ranges rule, decided -with the maintainer on 2026-10-20: the subscriber widens, never the server; -one that holds frames 0..N-1 still asks from N; the reader still starts at the -frame it asked for. Lite-07 applies it through `track::Producer::widen_compressed` -(`rs/moq-net/src/model/track.rs`), which a warm IETF copy can reuse. - -Split from #6242 because a two-hop matrix (lite-07 and moq-transport-17, 18 -and 22 on either hop) found three gaps under the IETF requester that are not -about compression: - -- A cold SUBSCRIBE (`Target::Request` in `rs/moq-net/src/ietf/subscriber.rs`) - learns the track is compressed only from SUBSCRIBE_OK. Widening every cold - mid-group start on a Flate session would make plain tracks pay for the - prefix too. Decide between learning the properties first (TRACK_STATUS where - `TrackStatusOk::describes_track` holds) and widening cold starts only on - Flate sessions. -- Readers get IETF-received groups whole: `track::Subscriber::deliver` skips - the frame window when `group.relayed()`, since Object IDs may not be frame - ordinals. An application on an IETF hop therefore already reads frames below - its start on plain tracks, and widening would hand it more. Filter by - ordinal when the copy's Object IDs are dense, or by Object ID. -- REQUEST_UPDATE carries no range, so a live range change never reaches - upstream; [moq-transport ranges](/quest/m1/subscribe-ranges/ietf.md) owns - that. - -In the matrix, our own IETF publisher served the whole group for a mid-group -Absolute start, so the `Lagged` failure needs a publisher that honors the -exact start; test against the publisher model directly. - -`js/net` needs no mirror: its subscriptions carry group ranges only -(`Subscription.groups`), so a JS SUBSCRIBE always starts at frame 0, and a -compressed group that does not is refused with `TooFarBehind`. - -Regression: extend -`application_ranges_keep_their_filter_after_requesting_the_compressed_prefix` -(`rs/moq-net/tests/integration/compress.rs`) over moq-transport-17, 18 and 22 -hops. - -Public API: none. Wire: none. - -## Related - -- [moq-transport ranges](/quest/m1/subscribe-ranges/ietf.md) - carries a live range change upstream on REQUEST_UPDATE -- [Bounded tails](/quest/m1/subscribe-ranges/bounded-tail.md) - a copy cut at a request's end must accept its tail before a warm prefix can be extended diff --git a/quest/m1/ietf-extension/README.md b/quest/m1/ietf-extension/README.md index cee1283e8db..251d8fc8ff2 100644 --- a/quest/m1/ietf-extension/README.md +++ b/quest/m1/ietf-extension/README.md @@ -112,9 +112,9 @@ them yet. Remaining, in order: edge's TRACK_INFO query becomes TRACK_STATUS upstream, and the pinned MoQTestPublisher inherits NOT_SUPPORTED. Preserve that refusal: widening query-only demand into a SUBSCRIBE is not an authorized fallback. - [Range widening](/quest/m1/compress/ietf-range-widen.md), claimed by - [moq-dev/moq#6465](https://github.com/moq-dev/moq/pull/6465), owns that - separate cold-metadata boundary. Needs a Moxygen revision whose test + No cold request needs that metadata to pick its start, since + [moq-dev/moq#6465](https://github.com/moq-dev/moq/pull/6465) starts every + subscription at a group's first frame. Needs a Moxygen revision whose test publisher answers TRACK_STATUS; then bump the pins, rebase the profile, and add the two-relay cell back to `test/ietf-conformance/run.sh` (it ran in [d96126c65a](https://github.com/moq-dev/moq/commit/d96126c65a52027820884cee7cb9741e79fb674b)). diff --git a/quest/m1/release-audit/README.md b/quest/m1/release-audit/README.md index b4de8e99acd..028f4ec2d02 100644 --- a/quest/m1/release-audit/README.md +++ b/quest/m1/release-audit/README.md @@ -97,7 +97,7 @@ child records whether its behavior is new on main or inherited from release. - [JS subscription ranges](/quest/m1/subscribe-ranges/js.md) - range lists and frame offsets build on the bounds fixed here - [OBS video source](/quest/m2/obs-moq-video/source.md) - the decoder replacement keeps the timing and EOF behavior fixed here -- [Compressed tracks](/quest/m1/compress/README.md) - mid-group widening and application migration have existing owners +- [Compressed tracks](/quest/m1/compress/README.md) - application migration has an existing owner - [Archive consumer](/quest/m1/archive/consumer.md) - current producer and CLI reader still need the new-format roundtrip - [Measured release performance](/quest/m1/release-perf/README.md) - existing paired benchmark regressions and their confirmation runs - [Transport performance audit](/quest/m2/audit-perf.md) - congestion-control coverage remains outstanding diff --git a/quest/m1/resolved-demand.md b/quest/m1/resolved-demand.md index a73902a66a5..0f4b16d90bc 100644 --- a/quest/m1/resolved-demand.md +++ b/quest/m1/resolved-demand.md @@ -71,8 +71,7 @@ with no wire change. because watching for it needs more than this plan carries: a verdict depends on two groups (`drifted` uses G's successor for `reach` and a possibly later presentation edge in `live_above`, and losing either can - free G), a range may end mid-group so an edge needs a frame position, and - a run skipped at the max age bound would no longer be final. Likewise, + free G), and a run skipped at the max age bound would no longer be final. Likewise, a stale drop in flight across an aggregate range change (lite-07, see [Drop set](/quest/m1/drop-set.md)) may settle a reader whose own range would not convict, since SUBSCRIBE_UPDATE has no acknowledgement and the diff --git a/quest/m1/subscribe-ranges/README.md b/quest/m1/subscribe-ranges/README.md index 2312f2827ad..fcd21b4bbc1 100644 --- a/quest/m1/subscribe-ranges/README.md +++ b/quest/m1/subscribe-ranges/README.md @@ -45,7 +45,7 @@ relay bounds a sparse FETCH only if the model can request ranges: an IETF FETCH is served from the model's ranges, capped at the Largest Object. - **Relative ends** (maintainer, 2026-10-11, superseding the 10-07 `Live` - flag and closing #5085). Each range end is `Absolute(group, frame)` or + flag and closing #5085). Each range end is `Absolute(group)` or `Relative(n)`, naming the group n before the latest; either end may be relative, and an omitted end is live. An offset past the track's start clamps to group 0. Before any group exists, relative ends resolve when the @@ -82,9 +82,10 @@ relay bounds a sparse FETCH only if the model can request ranges: its group starts, so a time end settles across a sequence that arrived only as a datagram, and a stream copy of that group arriving later is read even if it starts past the end. -- Ranges are frame-precise (`Position`), not whole groups. The IETF joining - FETCH for a mid-group SUBSCRIBE's uncached prefix stays, because today's - bridge relies on it (maintainer, 09-29). +- Ranges are whole groups: a range always starts at a group's first frame + (maintainer, on [moq-dev/moq#6465](https://github.com/moq-dev/moq/pull/6465)). The IETF joining FETCH for the + live group's uncached prefix stays, because today's bridge relies on it + (maintainer, 09-29). - The relay half of fetch-span moves here; #4558 landed only the no-handler skip. @@ -98,7 +99,6 @@ every sequence delivered or dropped. - [Lite-07 ranges](/quest/m1/subscribe-ranges/lite.md) - the lite-07 wire, Rust publisher and subscriber, and the draft - [moq-transport ranges](/quest/m1/subscribe-ranges/ietf.md) - range FETCH upstream per missing run, and non-blocking FETCH served downstream - [JS ranges](/quest/m1/subscribe-ranges/js.md) - `@moq/net` model and lite-07 parity -- [Bounded tails](/quest/m1/subscribe-ranges/bounded-tail.md) - a copy cut at a request's mid-group end accepts the rest of its group instead of rejecting it as `Duplicate` ## Related diff --git a/quest/m1/subscribe-ranges/bounded-tail.md b/quest/m1/subscribe-ranges/bounded-tail.md deleted file mode 100644 index edd6a9065f1..00000000000 --- a/quest/m1/subscribe-ranges/bounded-tail.md +++ /dev/null @@ -1,42 +0,0 @@ -# [M] A copy that stopped at a request's end bound accepts its tail - -## Goal - -A group received for a request whose end falls mid-group stays open to the -rest of that group. When later demand asks the same sequence past that end, -the tail is accepted instead of rejected as `Duplicate`, so a second -subscriber on the same relay reads the frames it asked for. This holds for -plain and compressed tracks on every version that carries frame bounds. - -## Plan - -Found on [moq-dev/moq#6242](https://github.com/moq-dev/moq/pull/6242) while -answering Codex's -[P1](https://github.com/moq-dev/moq/pull/6242#discussion_r4345457940): through -one relay, a subscriber reads group 5 frames 0..3 and a second then asks for -3..5. The tail never arrives, on a plain track and on a compressed one alike. - -- `GroupRecv` (`rs/moq-net/src/lite/subscriber.rs`) calls `group.finish()` - when a group stream ends cleanly, including one that ended only because the - request's end bound was reached. The truncated copy then looks complete. -- `claim_sequence` and `covering_group` (`rs/moq-net/src/model/track.rs`) - judge a copy only by its first frame, so every later stream for that - sequence is `Duplicate`, whatever frame it was asked from. -- Fix it in the receive path: a stream that stopped at the request's end - bound is not the group's end. Weigh leaving the copy unfinished so a tail - appends from its end (one copy; a compressed group keeps inflating against - its own prefix, which `widen_compressed` relies on when it keeps a request at - N) against letting a bounded copy stop covering a request past its end so - the new stream replaces it (compressed requests would then widen to frame 0 - again). -- A peer must not be able to pass off an early FIN as reaching the bound. - -Test on mock time through a relay: a bounded first subscriber, then a second -asking past its end, on plain and compressed lite-07 tracks, and on -moq-transport draft-20+ if its Absolute end reaches the same path. - -Public API: none expected. Wire: none. - -## Related - -- [Compressed tracks](/quest/m1/compress/README.md) - its Ranges rule keeps a warm request at N, which this makes reachable diff --git a/quest/m1/subscribe-ranges/js.md b/quest/m1/subscribe-ranges/js.md index fd8474e0fc5..546f14c0f76 100644 --- a/quest/m1/subscribe-ranges/js.md +++ b/quest/m1/subscribe-ranges/js.md @@ -33,9 +33,9 @@ IETF FETCH dispatch. Resized from [M] to [L] for it. was never cached, covering exact replay, empty and missing groups, concurrent requests, and cancellation while awaiting a reply. -When ranges add frame bounds, the group model carries a lite group's -`frameStart` instead of refusing a nonzero one as a protocol violation, as -`lite/subscriber.ts` `runGroup` does until then. Preserve the per-reader +Ranges stay whole groups ([moq-dev/moq#6465](https://github.com/moq-dev/moq/pull/6465)), +so the group model keeps refusing a nonzero lite `frameStart` as a protocol +violation, as `lite/subscriber.ts` `runGroup` does. Preserve the per-reader group-bound rule for reliable reads, live-edge selection, subscription updates, and publisher datagrams. diff --git a/quest/m2/net-wrap/e2ee.md b/quest/m2/net-wrap/e2ee.md index 66bd56477c3..b4e87fe946f 100644 --- a/quest/m2/net-wrap/e2ee.md +++ b/quest/m2/net-wrap/e2ee.md @@ -58,12 +58,9 @@ the reverse (decided 2026-10-20, maintainer): without changing moq-net's `track(name)`; propose the encoding in the PR. Record it in `draft-lcurley-moq-e2ee.md`, replacing the Catalogs section's `.z` sibling rule. -- Ranges: moq-net widens a range that starts mid-group on a compressed group - to frame 0 when the requester lacks the prefix, but it sees only the - uncompressed physical track. The wrapper's subscriber applies that rule - itself: it widens such a physical request to frame 0, inflates from - there, and yields only the requested logical range. Test a resumed - mid-group read, not only full round trips. +- Ranges: every moq-net range starts at a group's first frame + ([moq-dev/moq#6465](https://github.com/moq-dev/moq/pull/6465)), so the wrapper reads each physical group whole + and inflates from frame 0. - Frames: AEAD authenticates a whole frame, so the wrapper's frame producer buffers chunks from `write` and seals at `finish`, and its frame consumer yields chunks only after the frame authenticates. diff --git a/rs/moq-archive/src/producer/track.rs b/rs/moq-archive/src/producer/track.rs index efd31c775fe..23ab90a7e65 100644 --- a/rs/moq-archive/src/producer/track.rs +++ b/rs/moq-archive/src/producer/track.rs @@ -163,7 +163,7 @@ impl Track { let config = &shared.config; let range = match self.next { Some(next) => Range { - start: Anchor::Absolute(next), + start: Anchor::Absolute(next.group), end: None, }, None => Range::oldest(), diff --git a/rs/moq-archive/src/reader/tests.rs b/rs/moq-archive/src/reader/tests.rs index 47d73a51092..1f6d72ff79c 100644 --- a/rs/moq-archive/src/reader/tests.rs +++ b/rs/moq-archive/src/reader/tests.rs @@ -194,16 +194,14 @@ async fn open_with(archive: &Archive, config: Config) -> (broadcast::Producer, R (broadcast, reader) } -/// FETCH one group starting at `frame_start`, returning each frame's timestamp and payload. +/// FETCH one group, returning each frame's timestamp and payload. async fn fetch( broadcast: &broadcast::Producer, track: &str, sequence: u64, - frame_start: u64, ) -> std::result::Result, moq_net::Error> { let track = broadcast.consume().track(track)?; - let options = group::Fetch::default().with_frame_start(frame_start); - let mut group = track.fetch_group(sequence, options).await?; + let mut group = track.fetch_group(sequence, None).await?; let mut frames = Vec::new(); while let Some(frame) = group.read_frame().await? { frames.push((frame.timestamp.unwrap().value(), frame.payload)); @@ -211,6 +209,14 @@ async fn fetch( Ok(frames) } +/// Whether reading the whole fetched group from past its `len` frames ends cleanly. +async fn fetch_past_end(broadcast: &broadcast::Producer, track: &str, sequence: u64, len: usize) -> bool { + let track = broadcast.consume().track(track).unwrap(); + let group = track.fetch_group(sequence, None).await.unwrap(); + let mut past = group.with_range(len as u64 + 1..); + past.read_frame().await.unwrap().is_none() +} + fn expected(track: &str, sequence: u64, frames: std::ops::Range) -> Vec<(u64, Bytes)> { frames .map(|i| { @@ -234,22 +240,9 @@ async fn fetch_replays_original_groups() { let (broadcast, _reader) = open(&archive).await; - assert_eq!( - fetch(&broadcast, "video", 0, 0).await.unwrap(), - expected("video", 0, 0..3) - ); - assert_eq!( - fetch(&broadcast, "video", 2, 0).await.unwrap(), - expected("video", 2, 0..2) - ); - assert_eq!( - fetch(&broadcast, "audio", 5, 0).await.unwrap(), - expected("audio", 5, 0..2) - ); - - // A requested frame_start keeps the original frame indices and timestamps. - let tail = fetch(&broadcast, "video", 1, 1).await.unwrap(); - assert_eq!(tail, expected("video", 1, 1..3)); + assert_eq!(fetch(&broadcast, "video", 0).await.unwrap(), expected("video", 0, 0..3)); + assert_eq!(fetch(&broadcast, "video", 2).await.unwrap(), expected("video", 2, 0..2)); + assert_eq!(fetch(&broadcast, "audio", 5).await.unwrap(), expected("audio", 5, 0..2)); let track = broadcast.consume().track("video").unwrap(); let info = track.query().await.unwrap(); @@ -270,18 +263,8 @@ async fn a_group_split_across_records_is_served_whole() { archive.commit("video", &tail, 0).await; let (broadcast, _reader) = open(&archive).await; - assert_eq!( - fetch(&broadcast, "video", 7, 0).await.unwrap(), - expected("video", 7, 0..5) - ); - assert_eq!( - fetch(&broadcast, "video", 7, 4).await.unwrap(), - expected("video", 7, 4..5) - ); - assert_eq!( - fetch(&broadcast, "video", 8, 0).await.unwrap(), - expected("video", 8, 0..5) - ); + assert_eq!(fetch(&broadcast, "video", 7).await.unwrap(), expected("video", 7, 0..5)); + assert_eq!(fetch(&broadcast, "video", 8).await.unwrap(), expected("video", 8, 0..5)); } #[tokio::test] @@ -366,11 +349,7 @@ async fn an_expired_head_is_not_found() { archive.commit("video", &tail, 1).await; let (broadcast, _reader) = open(&archive).await; - not_found(fetch(&broadcast, "video", 7, 0).await); - assert_eq!( - fetch(&broadcast, "video", 7, 3).await.unwrap(), - expected("video", 7, 3..5) - ); + not_found(fetch(&broadcast, "video", 7).await); } #[tokio::test] @@ -384,7 +363,7 @@ async fn requests_download_only_their_object() { for sequence in 0..3 { assert_eq!( - fetch(&broadcast, "audio", sequence, 0).await.unwrap(), + fetch(&broadcast, "audio", sequence).await.unwrap(), expected("audio", sequence, 0..1) ); } @@ -424,18 +403,15 @@ async fn unadvertised_or_unreadable_groups_are_not_found() { let (broadcast, _reader) = open(&archive).await; - not_found(fetch(&broadcast, "audio", 2, 0).await); // between records - not_found(fetch(&broadcast, "audio", 5, 0).await); // past the last record - not_found(fetch(&broadcast, "video", 0, 0).await); // table disagrees with the record - not_found(fetch(&broadcast, "bad", 0, 0).await); // malformed envelope - not_found(fetch(&broadcast, "missing", 0, 0).await); // no object - not_found(fetch(&broadcast, "chat", 0, 0).await); // a track no timeline indexes + not_found(fetch(&broadcast, "audio", 2).await); // between records + not_found(fetch(&broadcast, "audio", 5).await); // past the last record + not_found(fetch(&broadcast, "video", 0).await); // table disagrees with the record + not_found(fetch(&broadcast, "bad", 0).await); // malformed envelope + not_found(fetch(&broadcast, "missing", 0).await); // no object + not_found(fetch(&broadcast, "chat", 0).await); // a track no timeline indexes // Siblings remain usable. - assert_eq!( - fetch(&broadcast, "audio", 4, 0).await.unwrap(), - expected("audio", 4, 0..1) - ); + assert_eq!(fetch(&broadcast, "audio", 4).await.unwrap(), expected("audio", 4, 0..1)); } #[tokio::test] @@ -444,23 +420,20 @@ async fn refresh_follows_new_records_and_pops() { archive.add("video", 0..2, 1).await; let (broadcast, mut reader) = open(&archive).await; - assert!(fetch(&broadcast, "video", 0, 0).await.is_ok()); + assert!(fetch(&broadcast, "video", 0).await.is_ok()); let next = archive.record("video", Position::group(2), Position::group(3), 1).await; // Refreshing before the commit finds nothing new; Not Found is not finality. reader.refresh().await.unwrap(); - not_found(fetch(&broadcast, "video", 2, 0).await); + not_found(fetch(&broadcast, "video", 2).await); archive.commit("video", &next, 1).await; reader.refresh().await.unwrap(); - assert_eq!( - fetch(&broadcast, "video", 2, 0).await.unwrap(), - expected("video", 2, 0..1) - ); + assert_eq!(fetch(&broadcast, "video", 2).await.unwrap(), expected("video", 2, 0..1)); // The popped record's groups are gone, although its object is still stored and was cached. archive.store.inner().take(); - not_found(fetch(&broadcast, "video", 1, 0).await); + not_found(fetch(&broadcast, "video", 1).await); assert_eq!(archive.store.inner().gets(), Vec::::new()); } @@ -472,7 +445,7 @@ async fn a_track_enrolled_after_opening_is_followed() { archive.add("audio", 0..2, 1).await; let (broadcast, mut reader) = open(&archive).await; - not_found(fetch(&broadcast, "video", 0, 0).await); + not_found(fetch(&broadcast, "video", 0).await); archive.add("video", 0..2, 2).await; let timelines = archive.store.timelines().await.unwrap(); @@ -484,10 +457,7 @@ async fn a_track_enrolled_after_opening_is_followed() { ); reader.refresh().await.unwrap(); - assert_eq!( - fetch(&broadcast, "video", 1, 0).await.unwrap(), - expected("video", 1, 0..2) - ); + assert_eq!(fetch(&broadcast, "video", 1).await.unwrap(), expected("video", 1, 0..2)); let timeline = broadcast.consume().track(&timeline("video")).unwrap(); assert!(timeline.subscribe(None).await.is_ok(), "the late timeline replays"); } @@ -523,7 +493,7 @@ async fn a_missing_timeline_segment_recovers_from_the_next_checkpoint() { // Record 2's checkpoint restates record 1, so its group is still served. for sequence in 0..3 { assert_eq!( - fetch(&broadcast, "video", sequence, 0).await.unwrap(), + fetch(&broadcast, "video", sequence).await.unwrap(), expected("video", sequence, 0..1) ); } @@ -539,8 +509,8 @@ async fn a_missing_tail_is_retried_on_the_next_refresh() { archive.store.inner().hide_gets("segments/0000000000000000002"); let (broadcast, mut reader) = open(&archive).await; - assert!(fetch(&broadcast, "video", 1, 0).await.is_ok()); - not_found(fetch(&broadcast, "video", 2, 0).await); + assert!(fetch(&broadcast, "video", 1).await.is_ok()); + not_found(fetch(&broadcast, "video", 2).await); archive.store.inner().heal(); archive.store.inner().take(); @@ -556,10 +526,7 @@ async fn a_missing_tail_is_retried_on_the_next_refresh() { ], "the cursor stays before the missing tail" ); - assert_eq!( - fetch(&broadcast, "video", 2, 0).await.unwrap(), - expected("video", 2, 0..1) - ); + assert_eq!(fetch(&broadcast, "video", 2).await.unwrap(), expected("video", 2, 0..1)); } #[tokio::test] @@ -574,7 +541,7 @@ async fn following_lists_only_new_timeline_keys() { .record("video", Position::group(group), Position::group(group + 1), 1) .await; reader.refresh().await.unwrap(); - not_found(fetch(&broadcast, "video", group, 0).await); + not_found(fetch(&broadcast, "video", group).await); archive.commit("video", &record, 0).await; archive.store.inner().take(); @@ -592,7 +559,7 @@ async fn following_lists_only_new_timeline_keys() { "following record {group} touches no media listing" ); assert_eq!( - fetch(&broadcast, "video", group, 0).await.unwrap(), + fetch(&broadcast, "video", group).await.unwrap(), expected("video", group, 0..1) ); } @@ -622,11 +589,11 @@ async fn an_unordered_listing_replays_in_segment_order() { ); for group in 0..3 { - not_found(fetch(&broadcast, "video", group, 0).await); + not_found(fetch(&broadcast, "video", group).await); } for group in 3..5 { assert_eq!( - fetch(&broadcast, "video", group, 0).await.unwrap(), + fetch(&broadcast, "video", group).await.unwrap(), expected("video", group, 0..1) ); } @@ -651,12 +618,9 @@ async fn a_track_without_usable_info_is_not_found() { let (broadcast, _reader) = open(&archive).await; for track in ["bare", "future"] { - not_found(fetch(&broadcast, track, 0, 0).await); + not_found(fetch(&broadcast, track, 0).await); } - assert_eq!( - fetch(&broadcast, "audio", 0, 0).await.unwrap(), - expected("audio", 0, 0..1) - ); + assert_eq!(fetch(&broadcast, "audio", 0).await.unwrap(), expected("audio", 0, 0..1)); } #[tokio::test] @@ -674,7 +638,7 @@ async fn multi_frame_timeline_groups_decode() { archive.timeline("video", 0, groups).await; let (broadcast, _reader) = open(&archive).await; - assert!(fetch(&broadcast, "video", 1, 0).await.is_ok()); + assert!(fetch(&broadcast, "video", 1).await.is_ok()); } #[tokio::test] @@ -716,11 +680,8 @@ async fn a_record_contradicting_its_timeline_is_not_indexed() { archive.timeline("video", untimed.sequence, groups).await; let (broadcast, _reader) = open(&archive).await; - assert_eq!( - fetch(&broadcast, "video", 0, 0).await.unwrap(), - expected("video", 0, 0..1) - ); - not_found(fetch(&broadcast, "video", 1, 0).await); + assert_eq!(fetch(&broadcast, "video", 0).await.unwrap(), expected("video", 0, 0..1)); + not_found(fetch(&broadcast, "video", 1).await); } #[tokio::test] @@ -745,20 +706,15 @@ async fn evicted_timeline_groups_remain_fetchable() { .await .unwrap(); let serve = tokio::spawn(reader.serve()); - let expected = fetch(&broadcast, &timeline("video"), 0, 0).await.unwrap(); + let expected = fetch(&broadcast, &timeline("video"), 0).await.unwrap(); let now = moq_net::time::Instant::now(); pool.poll(now, &kio::Waiter::noop()); pool.poll(now + Duration::from_secs(1), &kio::Waiter::noop()); - assert_eq!(fetch(&broadcast, &timeline("video"), 0, 0).await.unwrap(), expected); + assert_eq!(fetch(&broadcast, &timeline("video"), 0).await.unwrap(), expected); // Past the end of an evicted complete group is EOF, as it is for a cached one. pool.poll(now + Duration::from_secs(2), &kio::Waiter::noop()); pool.poll(now + Duration::from_secs(3), &kio::Waiter::noop()); - assert!( - fetch(&broadcast, &timeline("video"), 0, expected.len() as u64 + 1) - .await - .unwrap() - .is_empty() - ); + assert!(fetch_past_end(&broadcast, &timeline("video"), 0, expected.len()).await); serve.abort(); let _ = serve.await; } @@ -847,7 +803,7 @@ async fn enrolled_timeline_fetch_survives_finish() { archive.add("audio", 1..2, 1).await; reader.track("audio", &timeline("audio")).await.unwrap(); reader.refresh().await.unwrap(); - let expected = fetch(&broadcast, &timeline("audio"), 0, 0).await.unwrap(); + let expected = fetch(&broadcast, &timeline("audio"), 0).await.unwrap(); reader.finish().unwrap(); let now = moq_net::time::Instant::now(); broadcast.info().pool.poll(now, &kio::Waiter::noop()); @@ -855,19 +811,14 @@ async fn enrolled_timeline_fetch_survives_finish() { .info() .pool .poll(now + Duration::from_secs(31), &kio::Waiter::noop()); - assert_eq!(fetch(&broadcast, &timeline("audio"), 0, 0).await.unwrap(), expected); - let missing = fetch(&broadcast, &timeline("audio"), 2, 0).await; + assert_eq!(fetch(&broadcast, &timeline("audio"), 0).await.unwrap(), expected); + let missing = fetch(&broadcast, &timeline("audio"), 2).await; assert!( matches!(missing, Err(moq_net::Error::NotFound)), "unknown timeline group: {missing:?}" ); // Complete groups retain their known end, so a read past it is EOF. - assert!( - fetch(&broadcast, &timeline("audio"), 0, expected.len() as u64 + 1) - .await - .unwrap() - .is_empty() - ); + assert!(fetch_past_end(&broadcast, &timeline("audio"), 0, expected.len()).await); serving.abort(); let _ = serving.await; } @@ -918,7 +869,7 @@ async fn benchmark_timeline_fetch_fanout() { "benchmark must fetch an evicted group" ); let started = std::time::Instant::now(); - let results = futures::future::join_all((0..readers).map(|_| fetch(&broadcast, &name, 0, 0))).await; + let results = futures::future::join_all((0..readers).map(|_| fetch(&broadcast, &name, 0))).await; elapsed += started.elapsed(); assert!( results diff --git a/rs/moq-c/src/api.rs b/rs/moq-c/src/api.rs index c8d034749c2..048a9b91a02 100644 --- a/rs/moq-c/src/api.rs +++ b/rs/moq-c/src/api.rs @@ -619,7 +619,7 @@ pub struct moq_range { pub end_present: bool, } -/// Where one end of a [moq_range] falls. Zeroing it means group 0, frame 0. +/// Where one end of a [moq_range] falls. Zeroing it means group 0. #[repr(C)] #[derive(Clone, Copy)] pub struct moq_anchor { @@ -628,8 +628,6 @@ pub struct moq_anchor { /// The group, or with `MOQ_ANCHOR_RELATIVE` how many groups before the latest one /// (0 is the latest, and an offset past the first group clamps to it). pub group: u64, - /// The frame within `group`, read only with `MOQ_ANCHOR_ABSOLUTE`. - pub frame: u64, /// The presentation time in microseconds, read only with `MOQ_ANCHOR_TIME`. pub timestamp_us: u64, } @@ -639,7 +637,7 @@ pub struct moq_anchor { #[expect(non_camel_case_types, reason = "C ABI type names match moq.h")] #[derive(Clone, Copy, Debug)] pub enum moq_anchor_kind { - /// A fixed group and frame. + /// A fixed group, delivered from its first frame. MOQ_ANCHOR_ABSOLUTE = 0, /// A count of groups back from the latest one. MOQ_ANCHOR_RELATIVE = 1, @@ -660,12 +658,7 @@ pub enum moq_order { fn parse_anchor(anchor: &moq_anchor) -> Result { Ok(match anchor.kind { - v if v == moq_anchor_kind::MOQ_ANCHOR_ABSOLUTE as u32 => { - moq_net::track::Anchor::Absolute(moq_net::track::Position { - group: anchor.group, - frame: anchor.frame, - }) - } + v if v == moq_anchor_kind::MOQ_ANCHOR_ABSOLUTE as u32 => moq_net::track::Anchor::Absolute(anchor.group), v if v == moq_anchor_kind::MOQ_ANCHOR_RELATIVE as u32 => moq_net::track::Anchor::Relative(anchor.group), v if v == moq_anchor_kind::MOQ_ANCHOR_TIME as u32 => { moq_net::track::Anchor::Time(moq_net::Timestamp::from_micros(anchor.timestamp_us)?) diff --git a/rs/moq-c/src/test.rs b/rs/moq-c/src/test.rs index 32113489cf3..d97b32053b2 100644 --- a/rs/moq-c/src/test.rs +++ b/rs/moq-c/src/test.rs @@ -1805,13 +1805,11 @@ fn raw_track_subscription_options_and_update() { start: moq_anchor { kind: moq_anchor_kind::MOQ_ANCHOR_ABSOLUTE as u32, group: 1, - frame: 0, timestamp_us: 0, }, end: moq_anchor { kind: moq_anchor_kind::MOQ_ANCHOR_ABSOLUTE as u32, group: 2, - frame: 0, timestamp_us: 0, }, end_present: true, @@ -1920,13 +1918,11 @@ fn raw_track_time_anchor() { start: moq_anchor { kind: moq_anchor_kind::MOQ_ANCHOR_TIME as u32, group: 0, - frame: 0, timestamp_us: 30_000, }, end: moq_anchor { kind: moq_anchor_kind::MOQ_ANCHOR_ABSOLUTE as u32, group: 0, - frame: 0, timestamp_us: 0, }, end_present: false, @@ -2021,13 +2017,11 @@ fn raw_track_update_refuses_time_on_untimed() { start: moq_anchor { kind: moq_anchor_kind::MOQ_ANCHOR_TIME as u32, group: 0, - frame: 0, timestamp_us: 0, }, end: moq_anchor { kind: moq_anchor_kind::MOQ_ANCHOR_ABSOLUTE as u32, group: 0, - frame: 0, timestamp_us: 0, }, end_present: false, @@ -2088,7 +2082,6 @@ fn raw_track_relative_update_counts_from_the_track_edge() { let anchor = |kind: moq_anchor_kind, group: u64| moq_anchor { kind: kind as u32, group, - frame: 0, timestamp_us: 0, }; let subscribe = |ranges: &[moq_range]| moq_subscription { @@ -3269,7 +3262,6 @@ fn fetch_group( name: &str, sequence: u64, priority: i8, - frame_start: u64, ) -> mpsc::Receiver), moq_net::Error>> { let consumer = State::lock() .publish @@ -3281,9 +3273,7 @@ fn fetch_group( crate::ffi::RUNTIME.spawn(async move { let res = async { let track = consumer.track(&name)?; - let options = moq_net::group::Fetch::default() - .with_priority(priority) - .with_frame_start(frame_start); + let options = moq_net::group::Fetch::default().with_priority(priority); let mut group = track.fetch_group(sequence, options).await?; let index = group.index(); let frame = group.read_frame().await?.expect("expected a fetched frame"); @@ -3306,7 +3296,7 @@ fn track_dynamic_serves_a_fetch_miss() { let group_cb = Callback::new(); let dynamic = id(unsafe { moq_publish_track_dynamic(track, Some(channel_callback), group_cb.ptr) }); - let fetch = fetch_group(broadcast, "data", 5, 11, 0); + let fetch = fetch_group(broadcast, "data", 5, 11); let request = id(group_cb.recv()); let mut sequence = 0u64; let mut priority = 0i8; @@ -3329,7 +3319,7 @@ fn track_dynamic_serves_a_fetch_miss() { assert_eq!(fetch.recv_timeout(TIMEOUT).unwrap().unwrap(), (0, payload.to_vec())); // A rejected fetch fails as not found. - let fetch = fetch_group(broadcast, "data", 6, 0, 0); + let fetch = fetch_group(broadcast, "data", 6, 0); let request = id(group_cb.recv()); assert_eq!(moq_group_request_abort(request, 9), 0); let err = fetch @@ -3399,41 +3389,6 @@ fn a_group_request_nobody_reads_is_withdrawn() { assert_eq!(moq_origin_close(origin), 0); } -#[test] -fn track_dynamic_serves_a_fetch_from_frame_start() { - let origin = id(moq_origin_create()); - let broadcast = publish_broadcast(origin, b"track-dynamic-frame-start"); - let name = b"data"; - let track = - id(unsafe { moq_publish_track(broadcast, name.as_ptr() as *const c_char, name.len(), std::ptr::null()) }); - - let group_cb = Callback::new(); - let dynamic = id(unsafe { moq_publish_track_dynamic(track, Some(channel_callback), group_cb.ptr) }); - - let fetch = fetch_group(broadcast, "data", 5, 0, 3); - let request = id(group_cb.recv()); - let mut frame_start = 0u64; - assert_eq!(unsafe { moq_group_request_frame_start(request, &mut frame_start) }, 0); - assert_eq!(frame_start, 3); - - let group = id(moq_group_request_accept(request)); - let payload = b"tail"; - assert_eq!( - unsafe { moq_publish_group_frame(group, payload.as_ptr(), payload.len(), 100_000) }, - 0 - ); - assert_eq!(moq_publish_group_finish(group), 0); - let (index, got) = fetch.recv_timeout(TIMEOUT).unwrap().unwrap(); - assert_eq!(index, 3, "the consumer resumes at the requested frame"); - assert_eq!(got, payload); - - assert_eq!(moq_publish_dynamic_cancel(dynamic), 0); - assert_eq!(group_cb.recv_terminal(), 0); - assert_eq!(moq_publish_track_finish(track), 0); - assert_eq!(moq_publish_close(broadcast), 0); - assert_eq!(moq_origin_close(origin), 0); -} - #[test] fn track_request_dynamic_survives_accept() { let origin = id(moq_origin_create()); @@ -3442,7 +3397,7 @@ fn track_request_dynamic_survives_accept() { let _dynamic = id(unsafe { moq_publish_dynamic(broadcast, Some(channel_callback), request_cb.ptr) }); // A fetch of a track the broadcast has not declared requests the track first. - let fetch = fetch_group(broadcast, "archive", 9, 0, 0); + let fetch = fetch_group(broadcast, "archive", 9, 0); let request = id(request_cb.recv()); let group_cb = Callback::new(); let track_dynamic = id(unsafe { moq_track_request_dynamic(request, Some(channel_callback), group_cb.ptr) }); diff --git a/rs/moq-ffi/src/consumer.rs b/rs/moq-ffi/src/consumer.rs index 660d6389b36..ac00e3b5b03 100644 --- a/rs/moq-ffi/src/consumer.rs +++ b/rs/moq-ffi/src/consumer.rs @@ -67,12 +67,10 @@ pub struct MoqRange { /// Where one end of a [`MoqRange`] falls, mirroring [`moq_net::track::Anchor`]. #[derive(Clone, Copy, uniffi::Enum)] pub enum MoqAnchor { - /// This exact group and frame. + /// This group, delivered from its first frame. Absolute { /// The absolute group sequence number. group: u64, - /// The zero-based frame index within the group. - frame: u64, }, /// The group this many before the latest, clamped to group 0: `0` is the latest group. Relative { @@ -101,7 +99,7 @@ impl TryFrom for moq_net::track::Anchor { fn try_from(anchor: MoqAnchor) -> Result { Ok(match anchor { - MoqAnchor::Absolute { group, frame } => Self::Absolute(moq_net::track::Position { group, frame }), + MoqAnchor::Absolute { group } => Self::Absolute(group), MoqAnchor::Relative { groups } => Self::Relative(groups), MoqAnchor::Time { timestamp_us } => { Self::Time(moq_net::Timestamp::from_micros(timestamp_us).map_err(|_| MoqError::TimeOverflow)?) diff --git a/rs/moq-ffi/src/test.rs b/rs/moq-ffi/src/test.rs index 651ad973f87..d09dea9bba0 100644 --- a/rs/moq-ffi/src/test.rs +++ b/rs/moq-ffi/src/test.rs @@ -1108,7 +1108,7 @@ async fn raw_track_update_does_not_wait_for_pending_read() { priority: 10, max_delay_us: Some(25_000), ranges: Some(vec![MoqRange { - start: MoqAnchor::Absolute { group: 0, frame: 0 }, + start: MoqAnchor::Absolute { group: 0 }, end: None, }]), order: None, @@ -4682,7 +4682,7 @@ async fn raw_track_update_during_pending_group_still_reads_datagram() { priority: 10, max_delay_us: Some(25_000), ranges: Some(vec![MoqRange { - start: MoqAnchor::Absolute { group: 0, frame: 0 }, + start: MoqAnchor::Absolute { group: 0 }, end: None, }]), order: None, diff --git a/rs/moq-hls/src/export/rendition.rs b/rs/moq-hls/src/export/rendition.rs index 247ab17c116..0c00839118f 100644 --- a/rs/moq-hls/src/export/rendition.rs +++ b/rs/moq-hls/src/export/rendition.rs @@ -1343,21 +1343,19 @@ impl Drop for Rendition { /// Fetch one group, mapping "no longer (or not yet) servable" to the error that says so /// rather than a bare `None`: a caller abandoning a segment can only explain itself with it. /// -/// Reads from `start` and, when `end` is set, stops before that frame of the group. +/// Fetches the whole group, since a group is the unit of decodability, and reads from `start` +/// and, when `end` is set, stops before that frame of the group. async fn fetch( track: &moq_net::track::Consumer, start: hang::timeline::Position, end: Option, broadcast: &moq_net::broadcast::Consumer, ) -> Result> { - let options = moq_net::group::Fetch::default().with_frame_start(start.frame); - match track.fetch_group(start.group, options).await { - Ok(mut group) => { - if let Some(end) = end { - group = group.with_range(..end); - } - Ok(Ok(group)) - } + match track.fetch_group(start.group, None).await { + Ok(group) => Ok(Ok(match end { + Some(end) => group.with_range(start.frame..end), + None => group.with_range(start.frame..), + })), Err(err) if is_unservable(&err, broadcast) => Ok(Err(err)), Err(err) => Err(err.into()), } diff --git a/rs/moq-net/benches/codec.rs b/rs/moq-net/benches/codec.rs index 651e4c8b636..39f6dad9347 100644 --- a/rs/moq-net/benches/codec.rs +++ b/rs/moq-net/benches/codec.rs @@ -68,45 +68,5 @@ fn bench(c: &mut Criterion) { group.finish(); } -/// An outgoing request reads the counted aggregate and checks only its named group, -/// independent of the audience beside it and the cache's unrelated groups. -fn compressed_request(c: &mut Criterion) { - use moq_net::{broadcast, track}; - use std::hint::black_box; - let mut bench = c.benchmark_group("codec_compressed_request"); - for groups in [1, 64, 1024] { - for readers in [1, 64, 1024] { - for held in [false, true] { - let broadcast = broadcast::Info::default().produce(); - let track = broadcast - .create_track("compressed", track::Info::default().with_compress(true)) - .unwrap(); - for _ in 0..groups { - let mut group = track.append_group().unwrap(); - group.write_frame(None, bytes::Bytes::from_static(b"prefix")).unwrap(); - group.finish().unwrap(); - } - let sequence = if held { groups - 1 } else { groups }; - let subscription = track::Subscription::default().with_ranges([track::Range { - start: track::Anchor::Absolute(track::Position { - group: sequence, - frame: 1, - }), - end: None, - }]); - let _readers: Vec<_> = (0..readers).map(|_| track.subscribe(subscription.clone())).collect(); - bench.bench_function(BenchmarkId::new(format!("held_{held}_{readers}r"), groups), |b| { - b.iter(|| { - let mut subscription = track.subscription().unwrap(); - moq_net::fuzz::widen_compressed(&track, &mut subscription); - black_box(subscription); - }); - }); - } - } - } - bench.finish(); -} - -criterion_group!(benches, bench, compressed_request); +criterion_group!(benches, bench); criterion_main!(benches); diff --git a/rs/moq-net/benches/track.rs b/rs/moq-net/benches/track.rs index a8cffc80d8f..057fff21af1 100644 --- a/rs/moq-net/benches/track.rs +++ b/rs/moq-net/benches/track.rs @@ -1589,7 +1589,7 @@ impl TimeEnd { let subscription = track::Subscription::default() .with_max_delay(AGED_BUDGET) .with_ranges([track::Range { - start: track::Anchor::Absolute(track::Position::group(0)), + start: track::Anchor::Absolute(0), end: Some(track::Anchor::Time(Timestamp::from_secs(u32::MAX.into()).unwrap())), }]); let waiter = kio::Waiter::noop(); diff --git a/rs/moq-net/src/coding/reader.rs b/rs/moq-net/src/coding/reader.rs index 32382d93715..77f60c36862 100644 --- a/rs/moq-net/src/coding/reader.rs +++ b/rs/moq-net/src/coding/reader.rs @@ -413,23 +413,6 @@ impl Reader { Poll::Ready(Ok(bytes)) } - /// Poll to discard `remaining` bytes, counting it down as they arrive. Each byte is - /// released as it is skipped, so a peer cannot make a skipped payload cost credit. - pub(crate) fn poll_skip(&mut self, cx: &mut Context<'_>, remaining: &mut usize) -> Poll> { - self.want = 0; - while *remaining > 0 { - let mut lease = ready!(self.poll_lease(cx, 1))?; - if !lease.has_remaining() { - return Poll::Ready(Err(DecodeError::Short.into())); - } - let skipped = lease.remaining().min(*remaining); - lease.advance(skipped); - lease.release(); - *remaining -= skipped; - } - Poll::Ready(Ok(())) - } - /// Read exactly the given number of bytes from the stream. pub async fn read_exact(&mut self, size: usize) -> Result { std::future::poll_fn(|cx| self.poll_read_exact(cx, size)).await @@ -709,24 +692,6 @@ mod tests { } } - /// A skipped payload is discarded across reads, leaving what follows it, and a skip - /// waiting on more has counted down what it already discarded. - #[test] - fn skip_discards_across_reads() { - let mut reader = queue(&[b"abc", b"defg", b"hi"], crate::lite::Version::Lite05); - let mut cx = Context::from_waker(std::task::Waker::noop()); - - let mut remaining = 6; - assert!(matches!(reader.poll_skip(&mut cx, &mut remaining), Poll::Ready(Ok(())))); - assert_eq!(&reader.read_exact(3).now_or_never().unwrap().unwrap()[..], b"ghi"); - - let mut reader = queue(&[b"ab", b"cd"], crate::lite::Version::Lite05); - let mut remaining = 5; - assert!(reader.poll_skip(&mut cx, &mut remaining).is_pending()); - assert_eq!(remaining, 1); - assert_eq!(leased(&mut reader), 0, "nothing skipped is kept"); - } - /// A message cut across chunks decodes from a copy, waiting for exactly what its /// prefix declared, and consumes only itself. #[test] diff --git a/rs/moq-net/src/fuzz.rs b/rs/moq-net/src/fuzz.rs index 0277bc373b7..8ab9c80d086 100644 --- a/rs/moq-net/src/fuzz.rs +++ b/rs/moq-net/src/fuzz.rs @@ -73,11 +73,6 @@ pub fn object_group(frames: usize, properties: bool) -> (broadcast::Producer, tr (broadcast, track, group) } -/// Normalize a compressed wire request for the private requesting-model benchmark. -pub fn widen_compressed(track: &track::Producer, subscription: &mut track::Subscription) { - track.widen_compressed(subscription); -} - /// Every target, keyed by the name of its `fuzz_targets/.rs` shim. pub const TARGETS: &[(&str, Target)] = &[ ("lite", lite_wire), diff --git a/rs/moq-net/src/ietf/publisher.rs b/rs/moq-net/src/ietf/publisher.rs index f4f5881d55a..1c15b5c5268 100644 --- a/rs/moq-net/src/ietf/publisher.rs +++ b/rs/moq-net/src/ietf/publisher.rs @@ -17212,17 +17212,12 @@ struct ServeRange { } impl ServeRange { - /// Bound `subscription` to this range. + /// Bound `subscription` to the whole groups this range touches: a model subscription + /// starts and ends on a group boundary, and [`GroupSlice`] cuts the objects when serving. fn apply(&self, subscription: &mut Subscription) { *subscription = std::mem::take(subscription) - .with_start(self.start.map(|start| track::Position { - group: start.group, - frame: start.object, - })) - .with_end(self.end.and_then(|end| match end.object { - Some(object) => track::Position::after(end.group, object), - None => track::Position::after_group(end.group), - })); + .with_start(self.start.map(|start| start.group)) + .with_end(self.end.and_then(|end| end.group.checked_add(1))); } } diff --git a/rs/moq-net/src/ietf/subscriber.rs b/rs/moq-net/src/ietf/subscriber.rs index ecff0346fa5..32b487cc290 100644 --- a/rs/moq-net/src/ietf/subscriber.rs +++ b/rs/moq-net/src/ietf/subscriber.rs @@ -579,11 +579,6 @@ struct TrackState { // The joining FETCH's own request id, when one was sent. fetch_id: Option, - // Where the subscription asked to start, when that is partway through a group: a - // resumed subscription picking up after the frames a previous route delivered. The - // stream for that group legitimately starts there, without a head. - resume: Option, - // The data streams read so far, which PUBLISH_DONE's Stream Count is checked against. tail: kio::Producer, } @@ -607,7 +602,6 @@ impl TrackState { timescale: None, fill, fetch_id: None, - resume: None, tail: Default::default(), } } @@ -3055,10 +3049,6 @@ impl Subscriber { state.subscribes.insert( request_id, TrackState { - resume: subscription - .as_ref() - .and_then(|s| s.start()) - .filter(|start| start.frame != 0), // A resumed copy is the session's to abort from here on. producer: target.producer().cloned(), epoch: self.epoch.clone(), @@ -3381,7 +3371,7 @@ impl Subscriber { }; // Pending until the answer is written in below, so a relay serving the copy // never reads the requested floor as where the feed starts. - let request = request.request_start(start.map(|start| start.group)); + let request = request.request_start(start); // Serves cache misses with a group FETCH. Registered before accepting, so a // miss queued meanwhile waits for it rather than failing for want of a handler. let dynamic = request.dynamic(); @@ -3405,7 +3395,7 @@ impl Subscriber { // may still arrive after a higher one's. This resolves the pending start. let _ = match live { true => track.start_at(largest.map_or(0, |largest| largest.group)), - false => track.start_at(start.map(|start| start.group)), + false => track.start_at(start), }; if let Some(staged) = staged { staged.commit(); @@ -4401,15 +4391,13 @@ impl Subscriber { } }; - let (mut track, timescale, fill, resume, mut reading) = { + let (mut track, timescale, fill, mut reading) = { let state = self.state.lock(); let track = state.subscribes.get(&request_id).ok_or(Error::NotFound)?; ( track.producer.clone().ok_or(Error::NotFound)?, track.timescale, track.fill.clone(), - // Only the group the subscription resumes partway through. - track.resume.filter(|resume| resume.group == group.group_id), // Every data stream counts toward PUBLISH_DONE's Stream Count, even one // dropped below, and the subscription's end waits until it is read. Reading::open(&track.tail, Some(group.group_id), self.runtime.now()), @@ -4421,19 +4409,18 @@ impl Subscriber { stream.set_priority(crate::priority::recv_group(priority, group.group_id, order)); // Peek before creating a group: a standalone END_OF_TRACK has no payload - // group, and a fill or resume may already own the prefix. + // group, and a fill may already own the prefix. // // The peek inside blocks until the publisher produces the group's first object, so // race it against the subscription going away the same way the group read below is. // Otherwise dropping the local subscriber cannot end this handler. - let passthrough = !fill.read().outstanding() && resume.is_none(); + let passthrough = !fill.read().outstanding(); let opened = { let mut opening = track.clone(); let mut open = std::pin::pin!(crate::util::sendable(self.open_group( stream, &mut opening, &fill, - resume, &group, &mut reading ))); @@ -4489,7 +4476,7 @@ impl Subscriber { let mut producer = crate::recv::Group::new(producer); let observing = producer.handle().clone(); - let mut ingest = GroupIngest::new(self, &group, timescale, resume.map(|_| start)); + let mut ingest = GroupIngest::new(self, &group, timescale); let res = { kio::wait(|waiter| { if let Poll::Ready(err) = track.poll_closed(waiter) { @@ -4727,7 +4714,6 @@ impl Subscriber { stream: &mut Reader, track: &mut track::Producer, fill: &kio::Producer, - resume: Option, header: &ietf::GroupHeader, reading: &mut Reading, ) -> Result { @@ -4776,21 +4762,9 @@ impl Subscriber { }); } - // Without a fill or resume, a relay preserves the subgroup's first Object ID - // and FIRST_OBJECT claim independently. Plain readers see only frame ordinals. - + // Without a fill, a relay preserves the subgroup's first Object ID and FIRST_OBJECT + // claim independently. Plain readers see only frame ordinals. if !fill.read().outstanding() { - // The group a resumed subscription picks up partway through starts where it - // asked, and a stream with no objects is the end of a group complete there. - // A publisher sending more of it (a pre-draft-20 join asks for all of it) has - // the objects below the start dropped as they arrive. - if let Some(resume) = resume - && first.is_none_or(|first| first.id <= resume.frame) - { - let mut producer = create(track)?; - producer.start_at(resume.frame)?; - return Ok(Opened::Group(producer, first.map_or(resume.frame, |first| first.id))); - } return Ok(Opened::Group(create(track)?, first.map_or(0, |first| first.id))); } @@ -4896,14 +4870,11 @@ struct GroupIngest { timescale: Option, version: Version, prior_object: Option, - start: Option, properties: crate::model::object::Properties, /// The property list `properties` was read from, for a status object, which has no frame. raw: bytes::Bytes, gap: Option, inflated: Option, - /// The object being read is one the group started past, so it is read and dropped. - dropping: bool, phase: IngestPhase, } @@ -4920,30 +4891,21 @@ enum IngestPhase { Status { timestamp: Option }, /// Streaming the object payload. Payload { frame: frame::Producer }, - /// Discarding a dropped object's payload: the bytes of it still to read. - Skip { size: usize }, /// An explicit end-of-group or end-of-track status arrived. Finished(Ended), } impl GroupIngest { - fn new( - subscriber: &Subscriber, - group: &ietf::GroupHeader, - timescale: Option, - start: Option, - ) -> Self { + fn new(subscriber: &Subscriber, group: &ietf::GroupHeader, timescale: Option) -> Self { Self { has_extensions: group.flags.has_extensions, timescale, version: subscriber.version, prior_object: None, - start, properties: Default::default(), raw: bytes::Bytes::new(), gap: None, inflated: None, - dropping: false, phase: IngestPhase::Delta, } } @@ -4971,7 +4933,7 @@ impl Subscriber { return self.recv_group_fetch(stream, slot).await; } - let (subscribe_id, fill, resume, _counted) = { + let (subscribe_id, fill, _counted) = { let state = self.state.lock(); // A draft-20 fill is named by the SUBSCRIBE's request id. A pre-draft-20 joining // FETCH has its own id, which `fetches` maps back to that subscription. @@ -4983,7 +4945,7 @@ impl Subscriber { let counted = joined .is_none() .then(|| Reading::open(&track.tail, None, self.runtime.now())); - (subscribe_id, track.fill.clone(), track.resume, counted) + (subscribe_id, track.fill.clone(), counted) }; // SUBSCRIBE_OK declares the units these object timestamps are in, and this stream can @@ -5031,12 +4993,7 @@ impl Subscriber { // track does not close a group producer, since those lifecycles are independent. let res = { let mut serving = track.clone(); - let mut serve = std::pin::pin!(crate::util::sendable(self.run_fill( - stream, - &mut serving, - timescale, - resume - ))); + let mut serve = std::pin::pin!(crate::util::sendable(self.run_fill(stream, &mut serving, timescale))); kio::wait(|waiter| { if let Poll::Ready(err) = track.poll_closed(waiter) { return Poll::Ready(Err(err)); @@ -5086,11 +5043,10 @@ impl Subscriber { stream: &mut Reader, track: &mut track::Producer, timescale: Option, - resume: Option, ) -> Result { let mut head: Option<(u64, u64, crate::recv::Group)> = None; - match self.run_fill_objects(stream, track, timescale, resume, &mut head).await { + match self.run_fill_objects(stream, track, timescale, &mut head).await { Ok(()) => Ok(match head { Some((sequence, next, producer)) => Fill::Ready { sequence, @@ -5112,17 +5068,12 @@ impl Subscriber { /// absolute IDs. /// /// A joining FETCH and a draft-20 fill are one group, the head the live stream - /// continues. A Prior Object ID Gap must account for every missing ID. Ordinal resume - /// still requires contiguous IDs; refusing its fill leaves the subscription alone. - /// - /// A pre-draft-20 join resuming partway through a group asks for all of it, so the - /// objects below `resume` are dropped: another route already delivered them. + /// continues. A Prior Object ID Gap must account for every missing ID. async fn run_fill_objects( &mut self, stream: &mut Reader, track: &mut track::Producer, timescale: Option, - resume: Option, head: &mut Option<(u64, u64, crate::recv::Group)>, ) -> Result<(), Error> { let mut prior_group = None; @@ -5171,7 +5122,7 @@ impl Subscriber { if id < expected { return Err(Error::ProtocolViolation); } - if id > expected && (resume.is_some() || decoded.gap.is_none_or(|gap| gap < id - expected)) { + if id > expected && decoded.gap.is_none_or(|gap| gap < id - expected) { return Err(Error::Unsupported); } match head.as_ref().map(|(sequence, _, _)| *sequence) { @@ -5183,19 +5134,13 @@ impl Subscriber { _ => {} } - let (sequence, next, producer) = head.as_mut().expect("the head was created above"); + let (_, next, producer) = head.as_mut().expect("the head was created above"); if *next == 0 && let Some(mut wire) = producer.wire() { wire.first_id = id; producer.set_wire(wire)?; } - if *next == 0 - && let Some(resume) = resume.filter(|resume| resume.group == *sequence) - { - producer.start_at(resume.frame)?; - } - let keep = *next >= producer.frame_count() as u64; if self .recv_fetch_payload( stream, @@ -5204,7 +5149,6 @@ impl Subscriber { decoded, raw, timescale, - keep, id, }, ) @@ -5218,8 +5162,7 @@ impl Subscriber { Ok(()) } - /// Read one fetch object's length and payload into `producer`, after its header, or - /// past it unless `keep`. + /// Read one fetch object's length and payload into `producer`, after its header. async fn recv_fetch_payload( &self, stream: &mut Reader, @@ -5230,12 +5173,9 @@ impl Subscriber { decoded, raw, timescale, - keep, id, } = payload; - if keep { - producer.mark_gap(id, decoded.gap)?; - } + producer.mark_gap(id, decoded.gap)?; let size = stream.varint().await?; if size == 0 && matches!(self.version, Version::Draft14 | Version::Draft15) { let status = stream.varint().await?; @@ -5251,9 +5191,7 @@ impl Subscriber { match (status, unframed) { (0, _) => {} (END_OF_GROUP, Some(properties)) => { - if keep { - producer.set_end_object(crate::model::object::Fields { id, properties })?; - } + producer.set_end_object(crate::model::object::Fields { id, properties })?; return Ok(Fetched::EndOfGroup); } // END_OF_TRACK is track-level, so there is nowhere to keep its properties. @@ -5262,11 +5200,6 @@ impl Subscriber { _ => return Err(Error::Unsupported), } } - if !keep { - let mut remaining = usize::try_from(size).map_err(|_| Error::FrameTooLarge)?; - std::future::poll_fn(|cx| stream.poll_skip(cx, &mut remaining)).await?; - return Ok(Fetched::Frame); - } super::object::check_frame(&decoded.properties, timescale)?; let timestamp = object_time(timescale, decoded.timestamp)?; let plain = id == producer.frame_count() as u64 && decoded.properties.is_empty(); @@ -5791,7 +5724,6 @@ impl Subscriber { decoded, raw, timescale, - keep: true, id, }, ) @@ -5843,7 +5775,6 @@ struct FetchPayload { /// The property list `decoded` was read from, for a status object, which has no frame. raw: bytes::Bytes, timescale: Option, - keep: bool, id: u64, } @@ -6028,13 +5959,12 @@ impl GroupIngest { let Some(id_delta) = ready!(reader.poll_varint_maybe(&mut cx))? else { return Poll::Ready(Ok(Ended::Group)); }; - let id = next_object_id(self.prior_object, id_delta, self.start)?; + let id = next_object_id(self.prior_object, id_delta)?; self.prior_object = Some(id); self.properties = Default::default(); self.raw = bytes::Bytes::new(); self.gap = None; self.inflated = None; - self.dropping = self.start.is_some() && id < group.frame_count() as u64; self.phase = match self.has_extensions { true => IngestPhase::ExtSize, false => IngestPhase::Size { timestamp: None }, @@ -6060,11 +5990,6 @@ impl GroupIngest { self.phase = IngestPhase::Status { timestamp: *timestamp }; continue; } - if self.dropping { - let size = usize::try_from(size).map_err(|_| Error::FrameTooLarge)?; - self.phase = IngestPhase::Skip { size }; - continue; - } // `create_object` is the allocation chokepoint: it rejects an oversized // `size` and allocates up front only within the origin's budget, so no // pre-check is needed. @@ -6076,12 +6001,10 @@ impl GroupIngest { IngestPhase::Status { timestamp } => { let status = ready!(reader.poll_varint(&mut cx))?; if status == 0 { - if !self.dropping { - let timestamp = object_time(self.timescale, *timestamp)?; - let header = self.header(group, frame::Info { size: 0, timestamp })?; - let frame = group.create_object(header)?; - group.restore(frame.finish()?); - } + let timestamp = object_time(self.timescale, *timestamp)?; + let header = self.header(group, frame::Info { size: 0, timestamp })?; + let frame = group.create_object(header)?; + group.restore(frame.finish()?); self.phase = IngestPhase::Delta; continue; } @@ -6124,10 +6047,6 @@ impl GroupIngest { } } } - IngestPhase::Skip { size } => { - ready!(reader.poll_skip(&mut cx, size))?; - self.phase = IngestPhase::Delta; - } IngestPhase::Finished(ended) => { let ended = std::mem::replace(ended, Ended::Group); return Poll::Ready(Ok(ended)); @@ -12680,11 +12599,7 @@ struct Join { /// /// A start group we already know is absolute on draft-20 and needs no fill: the /// subscription's own range covers it, which is what our publisher serves from its cache. -fn subscribe_join( - start: Option, - end: Option, - version: Version, -) -> Result { +fn subscribe_join(start: Option, end: Option, version: Version) -> Result { if !Filter::is_draft20(version) { if end.is_some() { return Err(Error::Unsupported); @@ -12708,7 +12623,7 @@ fn subscribe_join( fetch: false, }, // An absolute {0, 0} with no end is defined as unfiltered, so it spells itself. - Some(start) if start == track::Position::group(0) && end.is_none() => Join { + Some(0) if end.is_none() => Join { filter: Filter::Unfiltered, fill: None, fetch: false, @@ -12716,21 +12631,14 @@ fn subscribe_join( Some(start) => Join { filter: Filter::Absolute { start: ietf::Location { - group: start.group, - object: start.frame, + group: start, + object: 0, }, end: end.and_then(|end| { - if end.frame == 0 { - Some(ietf::EndLocation { - group: end.group.checked_sub(1)?, - object: None, - }) - } else { - Some(ietf::EndLocation { - group: end.group, - object: Some(end.frame - 1), - }) - } + Some(ietf::EndLocation { + group: end.checked_sub(1)?, + object: None, + }) }), }, fill: None, @@ -12739,69 +12647,28 @@ fn subscribe_join( }) } -/// Resolve the first absolute ID or a later ID delta without overflowing. -/// -/// Ordinary subgroup relay ingest preserves gaps. A fill or resume supplies `start` -/// and requires contiguous IDs because its public frame ordinals stitch onto another copy. -fn next_object_id(prior: Option, delta: u64, start: Option) -> Result { - let object = match prior { - None => delta, +/// Resolve the first absolute ID or a later ID delta without overflowing. Subgroup relay +/// ingest preserves gaps. +fn next_object_id(prior: Option, delta: u64) -> Result { + match prior { + None => Ok(delta), Some(prior) => prior .checked_add(delta) .and_then(|id| id.checked_add(1)) - .ok_or(Error::Session(SessionError::ProtocolViolation))?, - }; - - let expected = prior.map_or(start.unwrap_or(object), |prior| prior.saturating_add(1)); - if start.is_some() && object != expected { - tracing::warn!( - object, - expected, - "object IDs must start at the group's start and increment by 1" - ); - return Err(Error::Unsupported); + .ok_or(Error::Session(SessionError::ProtocolViolation)), } - - Ok(object) } #[cfg(test)] mod object_id_tests { use super::*; - /// A zero delta throughout is a group numbered from 0 with no gaps, which is the only - /// shape moq-lite can represent. + /// The first delta is the absolute Object ID and each later one counts past the prior. #[test] - fn accepts_sequential_ids_from_zero() { - let mut prior = None; - for expected in 0..4 { - let object = next_object_id(prior, 0, Some(0)).expect("sequential"); - assert_eq!(object, expected); - prior = Some(object); - } - } - - /// The first object's delta is its absolute Object ID, so a non-zero one means the - /// group starts partway through and has a hole at the front. - #[test] - fn rejects_a_group_that_does_not_start_at_zero() { - assert!(matches!(next_object_id(None, 6, Some(0)), Err(Error::Unsupported))); - } - - /// The tail of a stitched group starts where the fill's head stopped, and nowhere else. - #[test] - fn accepts_a_tail_that_starts_where_the_fill_stopped() { - assert_eq!(next_object_id(None, 6, Some(6)).expect("the fill's next object"), 6); - assert_eq!(next_object_id(Some(6), 0, Some(6)).expect("then sequential"), 7); - assert!(matches!(next_object_id(None, 5, Some(6)), Err(Error::Unsupported))); - assert!(matches!(next_object_id(None, 7, Some(6)), Err(Error::Unsupported))); - } - - /// A later delta skips objects, which would renumber every frame after it. - #[test] - fn rejects_a_gap() { - assert!(matches!(next_object_id(Some(0), 1, Some(0)), Err(Error::Unsupported))); - assert!(matches!(next_object_id(Some(3), 9, Some(0)), Err(Error::Unsupported))); + fn resolves_ids_and_gaps() { + assert_eq!(next_object_id(None, 6).unwrap(), 6); + assert_eq!(next_object_id(Some(6), 0).unwrap(), 7); + assert_eq!(next_object_id(Some(6), 2).unwrap(), 9); } /// The running ID is bounded, and the draft makes an overflow a protocol violation @@ -12809,7 +12676,7 @@ mod object_id_tests { #[test] fn rejects_an_overflow() { assert!(matches!( - next_object_id(Some(u64::MAX), 0, Some(0)), + next_object_id(Some(u64::MAX), 0), Err(Error::Session(SessionError::ProtocolViolation)) )); } @@ -12842,12 +12709,7 @@ mod filter_tests { #[test] fn a_past_start_is_absolute() { assert_eq!( - subscribe_join( - Some(track::Position::group(7)), - track::Position::after_group(9), - Version::Draft20, - ) - .unwrap(), + subscribe_join(Some(7), Some(10), Version::Draft20).unwrap(), Join { filter: Filter::Absolute { start: ietf::Location { group: 7, object: 0 }, @@ -12863,7 +12725,7 @@ mod filter_tests { #[test] fn the_whole_track_is_unfiltered() { assert_eq!( - subscribe_join(Some(track::Position::group(0)), None, Version::Draft20).unwrap(), + subscribe_join(Some(0), None, Version::Draft20).unwrap(), Join { filter: Filter::Unfiltered, fill: None, @@ -12887,11 +12749,7 @@ mod filter_tests { #[test] fn older_drafts_always_join_live() { for version in JOINING_DRAFTS { - for start in [ - None, - Some(track::Position::group(7)), - Some(track::Position { group: 7, frame: 1 }), - ] { + for start in [None, Some(7)] { assert_eq!( subscribe_join(start, None, version).unwrap(), Join { @@ -12910,14 +12768,7 @@ mod filter_tests { fn older_drafts_refuse_a_bounded_end() { for version in JOINING_DRAFTS { assert!( - matches!( - subscribe_join( - Some(track::Position::group(7)), - track::Position::after_group(9), - version - ), - Err(Error::Unsupported) - ), + matches!(subscribe_join(Some(7), Some(10), version), Err(Error::Unsupported)), "{version}" ); } @@ -13198,15 +13049,6 @@ mod stitch_tests { self } - /// Resume the subscription partway through a group another route delivered the - /// head of. - fn with_resume(self, resume: track::Position) -> Self { - if let Some(track) = self.subscriber.state.lock().subscribes.get_mut(&REQUEST) { - track.resume = Some(resume); - } - self - } - /// Run the next scripted stream through `recv_group` and the dispatcher's handling of /// its result, asserting that stream alone was refused with STOP_SENDING. async fn dispatch_refused(&self) { @@ -14014,7 +13856,7 @@ mod stitch_tests { let mut run = GroupFetchRun::new(version, objects).await; let fill = h .subscriber - .run_fill(&mut run.stream, &mut h.track, None, None) + .run_fill(&mut run.stream, &mut h.track, None) .await .unwrap(); *h.fill.write().ok().expect("fill open") = fill; @@ -15085,67 +14927,6 @@ mod stitch_tests { assert_eq!(frames.len(), 1, "the group is whatever one producer wrote, not both"); } - /// A resumed subscription that gets its group whole (a pre-draft-20 join can only ask - /// for all of it) keeps only the objects from where it asked: another route already - /// delivered the rest. - #[moq_clock::test] - async fn a_resumed_whole_group_drops_the_delivered_head() { - let h = Harness::new(Fill::Done, vec![tail_stream(SEQUENCE, 0, &[b"0", b"1", b"2"])]).with_resume( - track::Position { - group: SEQUENCE, - frame: 2, - }, - ); - let mut consumer = h.track.subscribe(None); - let mut whole = h.stream().await; - h.subscriber.clone().recv_group(&mut whole).await.expect("the group"); - - let mut group = consumer - .recv_group() - .await - .unwrap() - .expect("the group arrived") - .with_range(0..); - assert_eq!(group.index(), 2, "the group starts where the subscription asked"); - assert_eq!(&group.read_frame().await.unwrap().unwrap().payload[..], b"2"); - assert!(group.read_frame().await.unwrap().is_none()); - } - - /// The same for a joining FETCH's head: the objects below the resume point are - /// dropped, and the live tail continues what is left. - #[moq_clock::test] - async fn a_resumed_fill_drops_the_delivered_head() { - let h = Harness::new( - Fill::Serving(Some(Timescale::MICRO)), - vec![ - fill_stream(SEQUENCE, &[b"0", b"1", b"2"]), - tail_stream(SEQUENCE, 3, &[b"3"]), - ], - ) - .with_resume(track::Position { - group: SEQUENCE, - frame: 2, - }); - let mut consumer = h.track.subscribe(None); - let mut fill = h.stream().await; - let mut tail = h.stream().await; - h.subscriber.clone().recv_fill(&mut fill).await.expect("fill"); - h.subscriber.clone().recv_group(&mut tail).await.expect("tail"); - - let mut group = consumer - .recv_group() - .await - .unwrap() - .expect("the group arrived") - .with_range(0..); - assert_eq!(group.index(), 2, "the group starts where the subscription asked"); - let mut frames = Vec::new(); - while let Some(frame) = group.read_frame().await.unwrap() { - frames.push(frame.payload.to_vec()); - } - assert_eq!(frames, [b"2".to_vec(), b"3".to_vec()]); - } - /// A standalone subgroup tail keeps its identity while plain readers see its payload. #[moq_clock::test] async fn a_tail_without_a_fill_is_preserved() { @@ -15983,13 +15764,13 @@ mod joining_fetch_tests { } impl JoinRun { - async fn start(version: Version, start: Option, ok: Vec, fetch: Vec) -> Self { + async fn start(version: Version, start: Option, ok: Vec, fetch: Vec) -> Self { Self::start_with_control(version, start, [ok, fetch], Control::new(None, false)).await } async fn start_with_control( version: Version, - start: Option, + start: Option, scripts: [Vec; 2], control: Control, ) -> Self { @@ -16404,13 +16185,7 @@ mod joining_fetch_tests { let largest = Some(ietf::Location { group: 9, object: 0 }); for version in JOINING_DRAFTS { for start in [7, 9, 11] { - let run = JoinRun::start( - version, - Some(track::Position::group(start)), - subscribe_ok(version, largest), - fetch_ok(version), - ) - .await; + let run = JoinRun::start(version, Some(start), subscribe_ok(version, largest), fetch_ok(version)).await; let messages = decode_messages(&run.session.log, version); let fetch = messages.iter().find(|(id, _)| *id == ietf::Fetch::ID).expect("FETCH"); @@ -16928,8 +16703,13 @@ mod joining_fetch_tests { ); let dynamic = track.dynamic(); let consumer = track.consume(); - let mut fetch = - std::pin::pin!(consumer.fetch_group(GROUP, group::Fetch::default().with_frame_start(START))); + let mut fetch = std::pin::pin!(consumer.fetch_group( + GROUP, + group::Fetch { + frame_start: START, + ..Default::default() + } + )); assert!(futures::poll!(fetch.as_mut()).is_pending()); let request = dynamic.requested_fill_group().await.expect("no group requested"); @@ -17069,7 +16849,13 @@ mod joining_fetch_tests { ); let dynamic = track.dynamic(); let consumer = track.consume(); - let mut fetch = std::pin::pin!(consumer.fetch_group(GROUP, group::Fetch::default().with_frame_start(START))); + let mut fetch = std::pin::pin!(consumer.fetch_group( + GROUP, + group::Fetch { + frame_start: START, + ..Default::default() + } + )); assert!(futures::poll!(fetch.as_mut()).is_pending()); let request = dynamic.requested_fill_group().await.expect("no group requested"); diff --git a/rs/moq-net/src/lite/ietf.rs b/rs/moq-net/src/lite/ietf.rs index 6c249e67abc..fae719f149a 100644 --- a/rs/moq-net/src/lite/ietf.rs +++ b/rs/moq-net/src/lite/ietf.rs @@ -1265,61 +1265,6 @@ mod hop_tests { } } - /// A subscription capped at a group's last frame stops there: its Group State promises - /// no END_OF_GROUP, so the END_OF_GROUP record past the cap is not sent either. - #[moq_clock::test] - async fn a_capped_stream_carries_no_end_record() { - let upstream = crate::origin::Config::new(crate::Hop::new(2).unwrap()).produce(); - let broadcast = upstream.publish("room", crate::origin::Route::default()).unwrap(); - let mut info = track::Info::timed().with_timescale(Timescale::MILLI); - info.properties = draft22(Vec::new()); - let track = broadcast.create_track("video", info).unwrap(); - let mut group = track.create_group(group::Info { sequence: 0 }).unwrap(); - group - .set_wire(group::Wire { - sparse: false, - first_object: true, - first_id: 0, - extensions: false, - has_end: true, - fetchable: true, - last: None, - }) - .unwrap(); - for (index, payload) in [b"a", b"b"].into_iter().enumerate() { - let fields = crate::model::object::Fields { - id: index as u64, - properties: Properties::default(), - }; - group = write(group, index as u64, Some(fields), payload); - } - group.mark_gap(2, None).unwrap(); - group - .set_end_object(crate::model::object::Fields { - id: 2, - properties: Properties::default(), - }) - .unwrap(); - group.finish().unwrap(); - - let (downstream, _sessions) = test_hop(&upstream, "room", [true, true]).await; - let broadcast = moq_clock::timeout(TIMEOUT, downstream.consume().request_broadcast("room", None)) - .await - .expect("no broadcast") - .unwrap(); - let capped = track::Subscription::default().with_end(track::Position::after(0, 1)); - let mut subscriber = moq_clock::timeout(TIMEOUT, broadcast.track("video").unwrap().subscribe(capped)) - .await - .expect("no subscription") - .unwrap(); - let mut received = recv(&mut subscriber).await; - assert_eq!(&next(&mut received).await.unwrap().payload[..], b"a"); - assert_eq!(&next(&mut received).await.unwrap().payload[..], b"b"); - assert!(next(&mut received).await.is_none()); - assert!(!received.wire().expect("relay state").has_end); - assert!(received.end_object().is_none(), "an END_OF_GROUP past the cap"); - } - /// Subscribe across a negotiated hop to a timed track a draft-22 publisher published /// with `values`. async fn subscribe_across(values: Vec) -> Result<(), crate::Error> { @@ -1521,13 +1466,11 @@ mod hop_tests { .unwrap(); assert_eq!(next(&mut fetched).await.unwrap().timestamp, negotiated.then(|| ts(100))); assert!(next(&mut fetched).await.is_none()); - let mut subscriber = moq_clock::timeout( - TIMEOUT, - copy.subscribe(track::Subscription::default().with_start(track::Position::group(1))), - ) - .await - .unwrap() - .unwrap(); + let mut subscriber = + moq_clock::timeout(TIMEOUT, copy.subscribe(track::Subscription::default().with_start(1))) + .await + .unwrap() + .unwrap(); // A peer may have stamped arrival times. The second group's rewind must not // acquire native moq-lite checks merely because it crossed a relay hop. let mut group = track.create_group(1u64.into()).unwrap(); diff --git a/rs/moq-net/src/lite/publisher.rs b/rs/moq-net/src/lite/publisher.rs index 11d0c817186..42079bf6814 100644 --- a/rs/moq-net/src/lite/publisher.rs +++ b/rs/moq-net/src/lite/publisher.rs @@ -1956,7 +1956,7 @@ impl Request for SubscribeServe { priority: msg.priority, max_delay: serving_max_delay(shared.version, msg.max_delay), order: msg.order, - ..Bounds::from(&msg).positions() + ..Bounds::from(&msg).groups() }; // One subscriber for the whole subscription: the run loop polls its groups and its @@ -2407,7 +2407,7 @@ mod test { let mut producer = track_producer("test"); let mut subscriber = producer.subscribe( track::Subscription::default() - .with_start(track::Position::group(0)) + .with_start(0) .with_max_delay(std::time::Duration::from_secs(60)), ); @@ -3426,25 +3426,17 @@ struct Bounds { } impl Bounds { - /// The requested range as the model's half-open pair of [`track::Position`]s. + /// The whole groups the requested range touches, as the model's half-open range. /// - /// The wire carries the two halves of each bound separately and both ends - /// inclusive; the model carries whole positions with an exclusive end. The - /// [`Subscription`] builders own that conversion, so this goes through them - /// rather than repeating it. - fn positions(&self) -> crate::track::Subscription { + /// A model subscription always starts and ends on a group boundary, so a frame bound + /// widens to its whole group here and [`position_group`] applies it when serving. + fn groups(&self) -> crate::track::Subscription { let mut sub = crate::track::Subscription::default(); if let Some(group) = self.start_group { - sub = sub.with_start(track::Position { - group, - frame: self.start_frame, - }); + sub = sub.with_start(group); } if let Some(group) = self.end_group { - sub = sub.with_end(match self.end_frame { - Some(frame) => track::Position::after(group, frame), - None => track::Position::after_group(group), - }); + sub = sub.with_end(group.checked_add(1)); } sub } @@ -3798,14 +3790,14 @@ impl TrackRun { // Feed the full update into the model subscriber so the producer's // aggregate reflects it (and a relay re-forwards it upstream). // Read first: `update` replaces these preferences. - let requested = self.track.subscription().start().map(|start| start.group); + let requested = self.track.subscription().start(); let floored = requested.is_some_and(|group| group > 0); let bounds = Bounds::from(&upd); let _ = self.track.update(crate::track::Subscription { priority: upd.priority, max_delay: serving_max_delay(self.ctx.version, upd.max_delay), order: upd.order, - ..bounds.positions() + ..bounds.groups() }); // An explicit start moves the read cursor. Lite-06+ encodes an absent // start as no floor, so a subscription that had one above group 0 has to @@ -3943,14 +3935,20 @@ impl TrackRun { && self.ctx.version.has_largest() && let Some(start) = self.track.subscription().start() && let Poll::Ready(Some(largest)) = self.track.poll_live(waiter) - && start > largest + && (track::Position { + group: start, + frame: self + .start_frame + .filter(|(group, _)| *group == start) + .map_or(0, |(_, frame)| frame), + }) > largest { - self.start = Some(start.group); + self.start = Some(start); writer.buffer(&lite::SubscribeResponse::Start(lite::SubscribeStart { - group: start.group, + group: start, largest: Some(largest), }))?; - self.raise_floor(start.group); + self.raise_floor(start); return Poll::Ready(Ok(ControlFlow::Continue(()))); } @@ -4057,6 +4055,18 @@ impl TrackRun { return; } let sequence = datagram.sequence; + // The model bounds whole groups, so the peer's frame bounds are applied here, against + // the Object ID a relayed moq-transport datagram keeps (a public one is frame 0). + let object = datagram.wire.as_ref().and_then(|wire| wire.object_id).unwrap_or(0); + if self + .start_frame + .is_some_and(|(group, frame)| group == sequence && object < frame) + || self + .end_frame + .is_some_and(|(group, frame)| group == sequence && object > frame) + { + return; + } // Metered in the form the wire carries. let (payload, _) = datagram.sent(self.ctx.version.has_compression()); self.track.meter_datagram(payload.len() as u64); @@ -4116,7 +4126,8 @@ impl TrackRun { return Ok(()); }; // A cap below the end bounds what the subscriber is owed, as on its side. - let end = final_sequence(self.track.subscription().end().map_or(fin, |end| end.min(fin))); + let fin = final_sequence(fin); + let end = self.track.subscription().end().map_or(fin, |end| end.min(fin)); self.drop_range(start..end, &StreamError::NotFound, writer) } } @@ -4682,26 +4693,23 @@ mod serve_group_test { use crate::{Timestamp, broadcast}; use futures::FutureExt; - /// The wire's inclusive pair maps to the model's exclusive end. - /// - /// The inverse of the subscriber's `WireBounds`, and the same off-by-one risk: a - /// whole end group becomes the head of the next one, a capped frame becomes the head - /// of the frame above it. + /// The wire's inclusive bounds map to the model's whole groups with an exclusive end: a + /// frame bound widens to its group, which `position_group` cuts when serving. #[test] - fn bounds_convert_to_positions() { + fn bounds_convert_to_whole_groups() { let whole = Bounds { start_group: None, start_frame: 0, end_group: Some(5), end_frame: None, }; - assert_eq!(whole.positions().end(), Some(track::Position::group(6))); + assert_eq!(whole.groups().end(), Some(6)); let capped = Bounds { end_frame: Some(2), ..whole }; - assert_eq!(capped.positions().end(), Some(track::Position { group: 5, frame: 3 })); + assert_eq!(capped.groups().end(), Some(6)); let started = Bounds { start_group: Some(5), @@ -4709,9 +4717,7 @@ mod serve_group_test { end_group: None, end_frame: None, }; - let positions = started.positions(); - assert_eq!(positions.start(), Some(track::Position { group: 5, frame: 3 })); - assert_eq!(positions.end(), None); + assert_eq!((started.groups().start(), started.groups().end()), (Some(5), None)); // A frame bound the peer sent without its group has nothing to count from, so it // cannot reach the model at all. @@ -4721,7 +4727,7 @@ mod serve_group_test { end_group: None, end_frame: Some(7), }; - assert_eq!((orphan.positions().start(), orphan.positions().end()), (None, None)); + assert_eq!((orphan.groups().start(), orphan.groups().end()), (None, None)); } /// A group whose head the publisher no longer holds is skipped, not served short: @@ -5607,6 +5613,70 @@ mod serve_group_test { assert!(order > 1); } + /// The peer's frame bounds cut datagrams by Object ID: a public datagram is frame 0, a + /// relayed moq-transport one keeps its own, and both ends are inclusive. + #[moq_clock::test] + async fn frame_bounds_cut_datagrams_by_object_id() { + for (sequence, object_id, served) in [ + (5, None, false), + (5, Some(1), false), + (5, Some(2), true), + (6, None, true), + (7, Some(3), true), + (7, Some(4), false), + ] { + let mut track = track::Producer::new( + Arc::new(broadcast::Info::default()), + "test", + crate::track::Info::timed(), + ); + let subscriber = track.subscribe(track::Subscription::default().with_max_delay(Duration::from_secs(3600))); + let session = SinkSession::new(Log::default()).with_groups().with_datagrams(1200); + let log = session.log.clone(); + let (mut run, mut writer, _) = lite_run(Version::Lite06, session.clone(), subscriber); + run.ctx.group = crate::transport::poll::Session::send_group(&mut run.ctx.session, 3); + run.update(lite::SubscribeUpdate { + wanted_group: None, + order: Order::Descending, + priority: 0, + max_delay: Duration::from_secs(3600), + start_group: Some(5), + end_group: Some(7), + start_frame: 2, + end_frame: Some(3), + }); + + track + .insert_wire_datagram(crate::model::datagram::Stored { + datagram: crate::model::datagram::Datagram { + sequence, + timestamp: Some(Timestamp::from_millis(sequence).unwrap()), + payload: bytes::Bytes::from_static(b"d"), + }, + wire: object_id.map(|object_id| { + Arc::new(crate::model::datagram::Wire { + object_id: Some(object_id), + priority: None, + end_of_group: false, + status: None, + properties: Default::default(), + }) + }), + deflated: None, + }) + .unwrap(); + { + let mut poll = std::pin::pin!(kio::wait(|waiter| run.poll(&mut writer, waiter))); + assert!(futures::poll!(poll.as_mut()).is_pending()); + } + assert_eq!( + log.datagram_ranks().len(), + usize::from(served), + "group {sequence} object {object_id:?}" + ); + } + } + /// A SUBSCRIBE_UPDATE that flips the group order reranks the next datagram, even with no /// group served in between. #[moq_clock::test] @@ -5916,7 +5986,7 @@ mod serve_group_test { crate::track::Info::timed(), ); let subscription = track::Subscription::default() - .with_start(track::Position::group(5)) + .with_start(5) .with_max_delay(Duration::from_secs(60)); let subscriber = track.subscribe(subscription); let (mut run, mut writer, log) = lite_run(version, SinkSession::new(Log::default()), subscriber); @@ -6290,7 +6360,7 @@ mod serve_group_test { crate::track::Info::timed(), ); let subscription = track::Subscription::default() - .with_start(track::Position::group(5)) + .with_start(5) .with_max_delay(Duration::from_secs(60)); let subscriber = track.subscribe(subscription); let (mut run, mut writer, log) = lite_run(version, SinkSession::new(Log::default()), subscriber); @@ -6336,7 +6406,7 @@ mod serve_group_test { crate::track::Info::timed(), ); let subscription = track::Subscription::default() - .with_start(track::Position::group(5)) + .with_start(5) .with_max_delay(Duration::from_secs(1)); let subscriber = track.subscribe(subscription); let (mut run, mut writer, log) = lite_run(version, SinkSession::new(Log::default()), subscriber); @@ -6385,7 +6455,7 @@ mod serve_group_test { crate::track::Info::timed(), ); let subscription = track::Subscription::default() - .with_start(track::Position::group(0)) + .with_start(0) .with_max_delay(Duration::from_secs(1)); let subscriber = track.subscribe(subscription); let (mut run, mut writer, log) = lite_run(version, SinkSession::new(Log::default()), subscriber); @@ -6433,7 +6503,7 @@ mod serve_group_test { crate::track::Info::timed(), ); let subscription = track::Subscription::default() - .with_start(track::Position::group(0)) + .with_start(0) .with_max_delay(Duration::from_secs(10)); let subscriber = track.subscribe(subscription); let (mut run, mut writer, log) = lite_run(version, SinkSession::new(Log::default()), subscriber); @@ -6477,7 +6547,7 @@ mod serve_group_test { ); let subscriber = track.subscribe( track::Subscription::default() - .with_start(track::Position::group(0)) + .with_start(0) .with_max_delay(Duration::from_secs(60)), ); let (mut run, mut writer, _log) = lite_run(Version::Lite05, SinkSession::new(Log::default()), subscriber); @@ -6518,7 +6588,7 @@ mod serve_group_test { ); let subscriber = track.subscribe( track::Subscription::default() - .with_start(track::Position::group(5)) + .with_start(5) .with_max_delay(Duration::from_secs(60)), ); let (mut run, mut writer, _log) = lite_run(Version::Lite05, SinkSession::new(Log::default()), subscriber); @@ -6586,7 +6656,7 @@ mod serve_group_test { fn new(track: &mut track::Producer, start_group: u64) -> Self { track.request_start(Some(0)).unwrap(); let subscription = track::Subscription::default() - .with_start(track::Position::group(start_group)) + .with_start(start_group) .with_max_delay(Duration::from_secs(30)); let subscriber = track.subscribe(subscription); diff --git a/rs/moq-net/src/lite/subscriber.rs b/rs/moq-net/src/lite/subscriber.rs index 76b8d75cda8..a851010003e 100644 --- a/rs/moq-net/src/lite/subscriber.rs +++ b/rs/moq-net/src/lite/subscriber.rs @@ -2223,7 +2223,7 @@ mod tests { let lowered = Some(Subscription::default().with_range(2..)); // The SUBSCRIBE in force, already answered. - let floor = Position::group(if update { 5 } else { 2 }); + let floor = if update { 5 } else { 2 }; let mut sub = Sub::Active(SubStream { order: crate::track::Order::Descending, stream: Stream::open(&mut crate::driver::Transport::new(h.session.clone()), Version::Lite04) @@ -2690,9 +2690,9 @@ mod tests { stream, id: 0, max_delay: Duration::ZERO, - start: Some(Position::group(2)), + start: Some(2), priority: 0, - requested: Some(Position::group(2)), + requested: Some(2), tail: Default::default(), served: None, end: Some(lite::SubscribeEnd { group: 10 }), @@ -2710,9 +2710,9 @@ mod tests { Some(2..10), "the groups below START are accounted for on arrival" ); - sub.start = Some(Position::group(6)); + sub.start = Some(6); assert_eq!(sub.owed(None), Some(6..10), "a raised floor owes nothing below it"); - sub.start = Some(Position::group(1)); + sub.start = Some(1); assert_eq!( sub.owed(None), Some(1..10), @@ -4399,162 +4399,22 @@ mod tests { } } - /// Mid-group demand: a subscriber resuming at frame 3 of group 5, capped by a later - /// bound in the same group. - fn mid_group_demand() -> Subscription { - Subscription::default() - .with_start(Position { group: 5, frame: 3 }) - .with_end(Position::after(5, 7)) - } - - /// A mid-group resume boundary handed to a peer that predates lite-06 is widened to - /// the whole group rather than refused. - /// - /// The codec rejects a frame bound such a peer cannot carry, so passing the demand - /// through unchanged fails the SUBSCRIBE and hands the track back. The origin then - /// asks the same route again indefinitely, since asking keeps succeeding and the - /// retry budget never trips. - #[moq_clock::test] - async fn frame_bounds_widen_for_an_older_peer() { - let mut h = Harness::new(Version::Lite05); - let mut sub = Sub::None; - - h.serve - .handle_subscription(&mut h.producer, &mut sub, Some(mid_group_demand()), true) - .await - .expect("an older peer must not fail the subscribe"); - - let wire = h.wire(); - let mut wire = wire.as_slice(); - assert_eq!( - crate::coding::decode_buf(&mut wire, Version::Lite05, lite::ControlType::decode).unwrap(), - lite::ControlType::Subscribe - ); - let msg = crate::coding::decode_buf(&mut wire, Version::Lite05, lite::Subscribe::decode).unwrap(); - // The group bounds survive; only the frame offsets are widened away. - assert_eq!((msg.start_group, msg.end_group), (Some(5), Some(5))); - assert_eq!((msg.start_frame, msg.end_frame), (0, None)); - } - - /// A lite-07 SUBSCRIBE keeps a mid-group start: its answer carries the largest position, - /// which says whether what the subscriber holds is current. Lite-06's answer does not, - /// so it asks from the head of the group instead, and the first frame says where the - /// live feed is; its end keeps the frame bound. - #[moq_clock::test] - async fn a_mid_group_start_survives_only_where_the_answer_has_the_largest() { - for (version, start_frame) in [(Version::Lite07, 3), (Version::Lite06, 0)] { - let mut h = Harness::new(version); - let mut sub = Sub::None; - - h.serve - .handle_subscription(&mut h.producer, &mut sub, Some(mid_group_demand()), true) - .await - .unwrap(); - - let wire = h.wire(); - let mut wire = wire.as_slice(); - assert_eq!( - crate::coding::decode_buf(&mut wire, version, lite::ControlType::decode).unwrap(), - lite::ControlType::Subscribe - ); - let msg = crate::coding::decode_buf(&mut wire, version, lite::Subscribe::decode).unwrap(); - assert_eq!((msg.start_frame, msg.end_frame), (start_frame, Some(7)), "{version:?}"); - } - } - - /// A partial prefix, a tail, or a released group cannot seed the inflater. Only - /// a complete live prefix permits requesting the tail without widening its start. - #[moq_clock::test] - async fn compressed_frame_bounds_require_the_entire_cached_prefix() { - for (first, count, aborted, expected) in [ - (0, 0, false, 0), - (0, 2, false, 0), - (0, 3, false, 3), - (1, 3, false, 0), - (0, 3, true, 0), - ] { - let mut h = Harness::new(Version::Lite07); - h.producer = h - ._broadcast - .create_track("compressed", track::Info::timed().with_compress(true)) - .unwrap(); - let mut group = h.producer.create_group(crate::group::Info { sequence: 5 }).unwrap(); - group.start_at(first).unwrap(); - for frame in first..first + count { - group - .write_frame(crate::Timestamp::from_millis(frame).unwrap(), b"prefix".as_ref()) - .unwrap(); - } - if aborted { - group.abort(Error::Cancel).unwrap(); - } else { - group.finish().unwrap(); - } - let mut sub = Sub::None; - h.serve - .handle_subscription(&mut h.producer, &mut sub, Some(mid_group_demand()), true) - .await - .unwrap(); - let wire = h.wire(); - let mut wire = wire.as_slice(); - assert_eq!( - crate::coding::decode_buf(&mut wire, Version::Lite07, lite::ControlType::decode).unwrap(), - lite::ControlType::Subscribe - ); - let msg = crate::coding::decode_buf(&mut wire, Version::Lite07, lite::Subscribe::decode).unwrap(); - assert_eq!( - (msg.start_group, msg.start_frame, msg.end_group, msg.end_frame), - (Some(5), expected, Some(5), Some(7)), - "first={first}, count={count}, aborted={aborted}" - ); - } - } - - /// The model's exclusive end maps back to the wire's inclusive pair. - /// - /// The two disagree deliberately (see [`Position::after_group`]), so this pins the seam: - /// an end at the head of a group means the group below it, served whole, while one - /// mid-group caps the frame below it. Off by one here would silently drop or - /// duplicate a frame at every relay hop. + /// The model's exclusive group end maps back to the wire's inclusive one, and every + /// SUBSCRIBE asks from a group's first frame. Off by one here would silently drop or + /// duplicate a group at every relay hop. #[test] fn wire_bounds_convert_the_exclusive_end() { - // The whole of group 5 is the head of group 6. - let bounds = WireBounds::new(None, Some(Position::group(6))); + let bounds = WireBounds::new(Some(5), Some(6)); + assert_eq!((bounds.start_group, bounds.start_frame), (Some(5), 0)); assert_eq!((bounds.end_group, bounds.end_frame), (Some(5), None)); - // Group 5 through frame 2 is the head of frame 3. - let bounds = WireBounds::new(None, Some(Position { group: 5, frame: 3 })); - assert_eq!((bounds.end_group, bounds.end_frame), (Some(5), Some(2))); - - // Unbounded stays unbounded. let bounds = WireBounds::new(None, None); assert_eq!((bounds.end_group, bounds.end_frame), (None, None)); - - // Starts are inclusive on both sides, so they pass straight through. - let bounds = WireBounds::new(Some(Position { group: 5, frame: 3 }), None); - assert_eq!((bounds.start_group, bounds.start_frame), (Some(5), 3)); - } - - /// The builders produce exactly what the wire conversion expects, so an inclusive - /// bound survives the trip out to a peer unchanged. - #[test] - fn wire_bounds_match_the_builders() { - let whole = Subscription::default().with_end(Position::after_group(5)); - let bounds = WireBounds::new(whole.start(), whole.end()); - assert_eq!((bounds.end_group, bounds.end_frame), (Some(5), None)); - - let capped = Subscription::default().with_end(Position::after(5, 2)); - let bounds = WireBounds::new(capped.start(), capped.end()); - assert_eq!((bounds.end_group, bounds.end_frame), (Some(5), Some(2))); - - let started = Subscription::default().with_start(Position { group: 5, frame: 3 }); - let bounds = WireBounds::new(started.start(), started.end()); - assert_eq!((bounds.start_group, bounds.start_frame), (Some(5), 3)); } /// A subscription that asks for nothing opens nothing. /// - /// `Position::group(0)` is the empty range: nothing sorts below it. The wire cannot + /// An end of group 0 is the empty range: nothing sorts below it. The wire cannot /// say that, and the nearest thing it can say is "through group 0", which would /// deliver the single group the caller excluded. #[moq_clock::test] @@ -4562,7 +4422,7 @@ mod tests { let mut h = Harness::new(Version::Lite06); let mut sub = Sub::None; - let empty = Subscription::default().with_end(Position::group(0)); + let empty = Subscription::default().with_end(0); h.serve .handle_subscription(&mut h.producer, &mut sub, Some(empty), true) .await @@ -4586,7 +4446,7 @@ mod tests { assert!(matches!(sub, Sub::Active(_)), "the first subscriber opens one"); let established = h.wire().len(); - let empty = Subscription::default().with_end(Position::group(0)); + let empty = Subscription::default().with_end(0); h.serve .handle_subscription(&mut h.producer, &mut sub, Some(empty), true) .await @@ -4607,9 +4467,7 @@ mod tests { let mut h = Harness::new(Version::Lite06); let mut sub = Sub::None; - let empty = Subscription::default() - .with_start(Position::group(5)) - .with_end(Position::group(5)); + let empty = Subscription::default().with_start(5).with_end(5); h.serve .handle_subscription(&mut h.producer, &mut sub, Some(empty), true) .await @@ -4633,9 +4491,7 @@ mod tests { assert!(matches!(sub, Sub::Active(_)), "the first subscriber opens one"); let established = h.wire().len(); - let empty = Subscription::default() - .with_start(Position::group(5)) - .with_end(Position::group(5)); + let empty = Subscription::default().with_start(5).with_end(5); h.serve .handle_subscription(&mut h.producer, &mut sub, Some(empty), true) .await @@ -4645,57 +4501,6 @@ mod tests { assert_eq!(h.wire().len(), established, "no SUBSCRIBE_UPDATE inverting the range"); } - /// Widening rounds the end outward at the last group too, keeping the range at least - /// as wide as the request. - /// - /// Rounding inward there would empty a range the caller asked for, and the filter in - /// `handle_subscription` runs before the widening, so nothing downstream would catch - /// it. - #[moq_clock::test] - async fn frame_bounds_widen_outward_at_the_last_group() { - let h = Harness::new(Version::Lite05); - - let mut subscription = Subscription::default() - .with_start(Position { - group: u64::MAX, - frame: 1, - }) - .with_end(Position::after(u64::MAX, 5)); - h.serve.widen_frame_bounds(&mut subscription); - - assert_eq!(subscription.start(), Some(Position::group(u64::MAX))); - // Past the last group there is no position to round up to, and unbounded is the - // wider request. - assert_eq!(subscription.end(), None); - } - - /// The widening covers SUBSCRIBE_UPDATE too: a downstream peer asking for a frame - /// offset must not tear down an older upstream that is already serving. - #[moq_clock::test] - async fn frame_bounds_widen_on_update() { - let mut h = Harness::new(Version::Lite05); - let mut sub = Sub::None; - - h.serve - .handle_subscription(&mut h.producer, &mut sub, Some(Subscription::default()), true) - .await - .unwrap(); - let established = h.wire().len(); - - // A lite-06 subscriber downstream now wants to resume mid-group. - h.serve - .handle_subscription(&mut h.producer, &mut sub, Some(mid_group_demand()), true) - .await - .expect("a downstream frame offset must not tear down an older upstream"); - - // SUBSCRIBE_UPDATE rides the subscribe stream with no control type ahead of it. - let wire = h.wire(); - let mut wire = &wire[established..]; - let msg = crate::coding::decode_buf(&mut wire, Version::Lite05, lite::SubscribeUpdate::decode).unwrap(); - assert_eq!((msg.start_group, msg.start_frame), (Some(5), 0)); - assert_eq!((msg.end_group, msg.end_frame), (Some(5), None)); - } - /// Concurrent joins over the refresh ratio send one SUBSCRIBE_UPDATE upstream, a new /// group past the largest sends the next, and every update restates the latest. #[moq_clock::test] @@ -4758,7 +4563,7 @@ mod tests { let mut h = Harness::new(Version::Lite05); let mut sub = Sub::None; - let demand = |group: u64| Some(Subscription::default().with_start(Position::group(group))); + let demand = |group: u64| Some(Subscription::default().with_start(group)); let applies = |sub: &Sub| matches!(sub, Sub::Active(active) if active.start == active.requested); // Establish from group 3; the peer's START is considered in flight. @@ -4773,11 +4578,7 @@ mod tests { .handle_subscription( &mut h.producer, &mut sub, - Some( - Subscription::default() - .with_start(Position::group(3)) - .with_end(Position::group(9)), - ), + Some(Subscription::default().with_start(3).with_end(9)), true, ) .await @@ -4809,7 +4610,7 @@ mod tests { let mut h = Harness::new(Version::Lite05); let mut sub = Sub::None; - let demand = |group: u64| Some(Subscription::default().with_start(Position::group(group))); + let demand = |group: u64| Some(Subscription::default().with_start(group)); // Establish from group 5: the floor tracks the request until START lands. h.serve @@ -5931,10 +5732,10 @@ mod tests { /// The four wire fields a subscription's half-open range encodes to. /// -/// The inverse of the publisher's `Bounds::positions`: the model carries whole -/// positions with an exclusive end, while the wire splits each bound into a group and a -/// frame and states both ends inclusive. The range must be non-empty, since an inclusive -/// end has nothing to say below the first position it excludes. +/// The model carries whole groups with an exclusive end, while the wire states both ends +/// inclusive. A subscription always starts and ends at a group boundary, so both frame +/// fields stay at their whole-group defaults. The range must be non-empty, since an +/// inclusive end has nothing to say below the first group it excludes. struct WireBounds { start_group: Option, start_frame: u64, @@ -5943,7 +5744,7 @@ struct WireBounds { } impl WireBounds { - fn new(start: Option, end: Option) -> Self { + fn new(start: Option, end: Option) -> Self { // An empty range has no wire encoding: flooring its end below asks for the // position it excludes, or inverts the range. `handle_subscription` drops such a // subscription instead. An absent start is the live edge, so it only makes the @@ -5953,19 +5754,11 @@ impl WireBounds { "an empty range cannot be encoded; it should have been dropped as no demand" ); - let (end_group, end_frame) = match end { - // An exclusive end at the head of a group means the group below it is the - // last one, and it is served whole. - Some(end) if end.frame == 0 => (Some(end.group.saturating_sub(1)), None), - Some(end) => (Some(end.group), Some(end.frame - 1)), - None => (None, None), - }; - Self { - start_group: start.map(|start| start.group), - start_frame: start.map_or(0, |start| start.frame), - end_group, - end_frame, + start_group: start, + start_frame: 0, + end_group: end.map(|end| end.saturating_sub(1)), + end_frame: None, } } } @@ -5978,7 +5771,7 @@ struct SubStream { /// Original SUBSCRIBE params, echoed in every SUBSCRIBE_UPDATE; refreshed as the /// downstream aggregate changes. max_delay: Duration, - start: Option, + start: Option, priority: i8, order: crate::track::Order, /// The start the SUBSCRIBE itself carried, fixed for the stream's life. A @@ -5986,7 +5779,7 @@ struct SubStream { /// so a buffered START only applies while `start` still equals it: while the /// start sits elsewhere the request-tracked floor stands instead, and demand /// returning here makes the declaration valid again. - requested: Option, + requested: Option, /// The groups received for this subscription, shared with its [`TrackEntry`]. tail: kio::Producer, /// The first group the publisher serves (SUBSCRIBE_START), once declared. @@ -6009,7 +5802,7 @@ impl SubStream { let end = self.end.as_ref()?.group; let end = requested_end.map_or(end, |requested| requested.min(end)); let start = match self.served { - Some(served) => self.start.map_or(served, |start| start.group), + Some(served) => self.start.unwrap_or(served), None => end, }; Some(start..end) @@ -6355,41 +6148,6 @@ impl TrackServe { ) } - /// The mid-group start a peer without frame bounds can't be asked for, recorded so - /// the frames below it are dropped when its whole group arrives. - fn widen_frame_bounds(&self, subscription: &mut Subscription) { - // An answer that does not carry the largest position cannot vouch for what a - // subscriber holds, so ask from the head of its group instead: the first frame then - // arrives at once and says where the live feed is. - if !self.subscriber.version.has_largest() { - let start = subscription.start().map(|start| Position::group(start.group)); - *subscription = std::mem::take(subscription).with_start(start); - } - if self.subscriber.version.has_frame_bounds() { - return; - } - - // Round both bounds outward to the enclosing group, so the peer sends at least - // what was asked for and never less. Rounding the end down instead would be able - // to empty a non-empty range, which has no wire encoding at all. - let start = subscription.start().map(|start| Position::group(start.group)); - let end = subscription.end().and_then(|end| match end.frame { - 0 => Some(end), - // Past the last group there is no position to round up to, and unbounded is - // the wider request. - _ => Position::after_group(end.group), - }); - - if (start, end) != (subscription.start(), subscription.end()) { - tracing::debug!( - track = %self.name, - version = ?self.subscriber.version, - "widening frame bounds to whole groups for an older peer" - ); - } - *subscription = std::mem::take(subscription).with_start(start).with_end(end); - } - /// Apply a subscription-demand change: hand back an [`Establish`] to open the /// upstream SUBSCRIBE on the first subscriber, buffer a SUBSCRIBE_UPDATE while /// live (the caller flushes), or cancel outright when the last one leaves. @@ -6414,15 +6172,13 @@ impl TrackServe { // subscribers' own reads narrow again. let start = subscription.start(); subscription = subscription.with_start(start); - producer.widen_compressed(&mut subscription); - self.widen_frame_bounds(&mut subscription); match sub { Sub::None => { // An idle copy is asked from the head of the newest group it cached when // the subscriber has no start of its own: a quiet track (a catalog) sends // that group again, which says the cache is current. if subscription.start().is_none() { - subscription = subscription.with_start(producer.idle_newest().map(Position::group)); + subscription = subscription.with_start(producer.idle_newest()); } // Open an upstream SUBSCRIBE for the first subscriber. Ok(Begin::Establish(self.prepare_establish(producer, subscription))) @@ -6438,8 +6194,8 @@ impl TrackServe { // tail as well as in the track's arrivals, which its start declaration // restarts. let lowered = subscription.start().map(|start| { - let floor = active.start.map(|start| start.group).or(active.served); - start.group..floor.unwrap_or(u64::MAX) + let floor = active.start.or(active.served); + start..floor.unwrap_or(u64::MAX) }); if let Ok(mut tail) = active.tail.write() { tail.set_grace(tail::grace(subscription.budget())); @@ -6458,7 +6214,7 @@ impl TrackServe { // start returns to the demand that produced it (see // `SubStream::requested`). if start_moved { - let _ = producer.start_at(active.start.map(|start| start.group)); + let _ = producer.start_at(active.start); } buffer_update(active, subscription.end())?; } @@ -6497,14 +6253,12 @@ impl TrackServe { fn prepare_establish(&self, producer: &mut track::Producer, subscription: Subscription) -> Establish { let id = self.subscriber.next_id.fetch_add(1, atomic::Ordering::Relaxed); - // Both halves of each bound come from the same position, so a frame can never - // reach the wire without the group it counts from (which the peer would reject). // The floor tracks the requested start until this subscription's own // SUBSCRIBE_START refines it: the peer never serves below the request, a // previous subscription's declaration must not outlive its demand, and // live-edge demand (None) starts with no floor at all. Lite-06+ only resolves // the start with its SUBSCRIBE_START, so until then the floor is just a request. - let floor = subscription.start().map(|start| start.group); + let floor = subscription.start(); let _ = match self.subscriber.version.resolves_start() { true => producer.request_start(floor), false => producer.start_at(floor), @@ -6609,7 +6363,7 @@ fn wire_max_delay(max_delay: Option) -> std::time::Duration } /// Buffer the current SUBSCRIBE parameters with an updated end. The caller flushes. -fn buffer_update(active: &mut SubStream, end: Option) -> Result<(), Error> { +fn buffer_update(active: &mut SubStream, end: Option) -> Result<(), Error> { let bounds = WireBounds::new(active.start, end); active.stream.writer.buffer(&lite::SubscribeUpdate { wanted_group: active.wanted, @@ -7592,7 +7346,7 @@ impl ServeLoop { if let Some(requested) = active.requested && let Ok(mut tail) = active.tail.write() { - tail.account(requested.group..start.group, serve.subscriber.runtime.now()); + tail.account(requested..start.group, serve.subscriber.runtime.now()); } } // The publisher will never send these groups, so they @@ -7630,14 +7384,7 @@ impl ServeLoop { // parks, since a cap can be raised). Those streams can still // be in flight, so wait for the tail before finishing. let subscription = self.serving.subscription(); - let requested_end = - subscription - .as_ref() - .and_then(|sub| sub.end()) - .map(|end| match end.frame { - 0 => end.group, - _ => end.group.saturating_add(1), - }); + let requested_end = subscription.as_ref().and_then(|sub| sub.end()); // The effective max delay is the stopgap grace: the wrong clock // (it bounds presentation-time drift), but it is how long the // subscriber was willing to wait for a late group anyway. diff --git a/rs/moq-net/src/model/group.rs b/rs/moq-net/src/model/group.rs index 38558412137..02cae7143af 100644 --- a/rs/moq-net/src/model/group.rs +++ b/rs/moq-net/src/model/group.rs @@ -1940,13 +1940,6 @@ impl Handle { state.abort.is_none().then_some(state.offset) } - /// Whether this copy still holds every committed frame below `end`, so a compressed - /// tail requested there can inflate against it. Sparse object IDs are not ordinals. - pub(super) fn holds_prefix(&self, end: u64) -> bool { - let state = self.state.read(); - state.abort.is_none() && state.offset == 0 && !state.sparse() && state.committed as u64 >= end - } - /// One past the last frame committed to an unfinished group, when that is past its /// first: where a replacement route resumes. `None` once the group is finished, or /// while it holds nothing a replacement could continue. @@ -2506,12 +2499,6 @@ impl Consumer { self.cursor.state.read().wire().cloned() } - /// Whether the group was relayed from moq-transport, whose Object IDs may not be its frame - /// ordinals. - pub(super) fn relayed(&self) -> bool { - self.cursor.state.read().wire().is_some() - } - /// Read a compressed group's stored DEFLATE slices instead of plain frames, with each /// frame's inflated size beside it ([`frame::Object::inflated`], /// [`frame::Consumer::inflated`]), for egress that carries compression. A no-op on a @@ -3191,15 +3178,14 @@ pub struct Fetch { /// Index of the first frame to fetch within the group. Defaults to 0, the whole group. /// - /// Use this to fill a hole left by a route change: the group's head is already - /// cached locally and only the tail is missing. + /// Only a route change sets it, filling the hole after a head another route already + /// delivered. An application always fetches from a group's first frame, since a group is + /// the unit of decodability, and skips frames locally with [`Consumer::with_range`]. /// - /// There is no matching end: a fetch always runs to the end of the group, and a - /// caller wanting less caps the returned consumer with [`Consumer::with_range`]. Stopping - /// the *fetch* short would put a group in the cache that is indistinguishable from a - /// complete one, so a later fetch of the whole group would resolve from it and come - /// up short. - pub frame_start: u64, + /// There is no matching end: stopping the *fetch* short would put a group in the cache + /// that is indistinguishable from a complete one, so a later fetch of the whole group + /// would resolve from it and come up short. + pub(crate) frame_start: u64, } impl Fetch { @@ -3214,12 +3200,6 @@ impl Fetch { self.max_delay = Some(max_delay); self } - - /// Set the first frame to fetch, returning `self` for chaining. - pub fn with_frame_start(mut self, frame_start: u64) -> Self { - self.frame_start = frame_start; - self - } } #[cfg(test)] diff --git a/rs/moq-net/src/model/origin.rs b/rs/moq-net/src/model/origin.rs index 61171f05ca9..7954ccf79d8 100644 --- a/rs/moq-net/src/model/origin.rs +++ b/rs/moq-net/src/model/origin.rs @@ -11744,13 +11744,13 @@ mod tests { let mut handler = replacement.dynamic(); queued(&standby).await.accept(&replacement); let request = handler.requested_track().await.unwrap(); - assert_eq!(request.subscription().unwrap().start(), Some(track::Position::group(5))); + assert_eq!(request.subscription().unwrap().start(), Some(5)); let track = request.accept(crate::track::Info::timed()); let mut preferences = rig.subscription.subscription(); preferences.priority = 7; rig.subscription.update(preferences).unwrap(); settle(|| track.subscription().is_some_and(|sub| sub.priority == 7)).await; - assert_eq!(track.subscription().unwrap().start(), Some(track::Position::group(5))); + assert_eq!(track.subscription().unwrap().start(), Some(5)); } /// A route updated in place to a new first hop still serves the same broadcast: diff --git a/rs/moq-net/src/model/resume.rs b/rs/moq-net/src/model/resume.rs index c596815b403..435afd3b990 100644 --- a/rs/moq-net/src/model/resume.rs +++ b/rs/moq-net/src/model/resume.rs @@ -113,11 +113,7 @@ impl Producer { Some(copy) if copy.from == preferences.ranges => copy.resolved.clone(), _ => Vec::new(), }; - ranges.extend( - preferences - .clamped(reader.newest.map(Position::group), None, &resolved) - .ranges, - ); + ranges.extend(preferences.clamped(reader.newest, None, &resolved).ranges); } demand.ranges = ranges; demand @@ -369,7 +365,7 @@ struct Copy { track: track::Consumer, sub: Sub, /// Where the subscription was asked to start, so demand updates never ask for less. - floor: Option, + floor: Option, /// The reader's ranges [`Self::resolved`] are for. from: Vec, /// Each of those ranges resolved, by index, so clamping never widens one: as the @@ -418,7 +414,7 @@ impl Copy { self.from = reader.ranges.clone(); } } - reader.clamped(self.floor, self.end().map(Position::group), &self.resolved) + reader.clamped(self.floor, self.end(), &self.resolved) } /// The first group past where this copy was replaced, if any: one replaced before @@ -454,7 +450,7 @@ impl Copy { /// The local floor `start` as this copy applies it: never below its own resume point, /// since groups under it were handed out by an earlier route, or never owed. fn start(&self, start: u64) -> u64 { - self.floor.map_or(start, |floor| start.max(floor.group)) + self.floor.map_or(start, |floor| start.max(floor)) } /// Whether `within` passed since this copy was superseded; see [`Self::since`]. Parks @@ -495,7 +491,7 @@ impl Copy { { let start = reader.groups.0; // From where it was asked to start, or else from what it first got. - let floor = self.floor.map(|floor| floor.group); + let floor = self.floor; let from = floor.map_or(start, |floor| floor.max(start)); // A group still to land is waited for once something superseded it, as a gap is. if self.since.is_some() @@ -528,7 +524,7 @@ impl Copy { let backs = |sequence: u64| { // A group at or past where it was asked to start may land even before its first // arrival; without a floor, only what follows its first arrival can. - let asked = self.floor.is_some_and(|floor| floor.group <= sequence); + let asked = self.floor.is_some_and(|floor| floor <= sequence); self.track.peek_group(sequence).is_some() || (lands && self @@ -866,15 +862,13 @@ impl Reader { track::Range { start: match range.start { // The farthest offset reaches back to the oldest group, which names no floor. - Anchor::Relative(back) if back != u64::MAX => Anchor::Absolute(Position::group(resolved.start)), - Anchor::Time(_) => Anchor::Absolute(Position::group(resolved.start)), + Anchor::Relative(back) if back != u64::MAX => Anchor::Absolute(resolved.start), + Anchor::Time(_) => Anchor::Absolute(resolved.start), start => start, }, // A time end the copy has not settled stays one: its cap is not the end. end: match (range.end, resolved.end) { - (Some(Anchor::Relative(_) | Anchor::Time(_)), Some(end)) => { - Some(Anchor::Absolute(Position::group(end))) - } + (Some(Anchor::Relative(_) | Anchor::Time(_)), Some(end)) => Some(Anchor::Absolute(end)), (end, _) => end, }, } @@ -980,7 +974,7 @@ impl Reader { copy.datagrams_done = None; // Resume where a fresh copy would: a group it buffered while replaced, below the // newest another route handed out, is not handed out. - copy.floor = copy.floor.max(self.newest.map(Position::group)); + copy.floor = copy.floor.max(self.newest); let start = copy.start(self.groups.0); if let (Sub::Ready(sub), Some(_)) = (&mut copy.sub, copy.floor) { sub.set_range(( @@ -992,7 +986,7 @@ impl Reader { self.copies.push(copy); return; } - let floor = self.newest.map(Position::group); + let floor = self.newest; // The wire request carries the floor from the first message: some sessions read // the demand only once. let wire = self.mirrored.clamped(floor, None, &resolved); @@ -3899,7 +3893,7 @@ mod test { .subscribe(kio::Producer::new(Subscription::default()).into()); let mut reader = sub.reader.lock().unwrap(); let copy = &mut reader.copies[0]; - copy.floor = Some(Position::group(0)); + copy.floor = Some(0); let ranges = vec![track::Range { start: track::Anchor::Time(ts(10_000)), end: None, @@ -3989,7 +3983,7 @@ mod test { assert!(sub.recv_group().now_or_never().is_none()); let start = c.subscription().expect("subscribed").ranges[0].start; assert!( - matches!(start, track::Anchor::Absolute(position) if position.group >= 10), + matches!(start, track::Anchor::Absolute(group) if group >= 10), "the replacement asks from where b resolved the time: {start:?}" ); } @@ -4051,7 +4045,7 @@ mod test { zero.write_frame(ts(3000), b"x".as_ref()).unwrap(); zero.finish().unwrap(); let range = track::Range { - start: track::Anchor::Absolute(Position::group(0)), + start: track::Anchor::Absolute(0), end: Some(track::Anchor::Time(ts(3500))), }; let subscription = Subscription::default() @@ -4083,7 +4077,7 @@ mod test { #[test] fn a_replaced_copy_at_the_last_sequence_has_no_overflow_cap() { - for (last, end) in [(Some(u64::MAX), None), (None, Some(Position::default()))] { + for (last, end) in [(Some(u64::MAX), None), (None, Some(0))] { let routes = Producer::new(); let a = copy(); routes.serve(a.consume()); @@ -4094,8 +4088,8 @@ mod test { old.until = Some(last); assert_eq!(old.wire(&Subscription::default()).end(), end); // A reader's narrower bound survives an unbounded replacement cap. - let requested = Subscription::default().with_end(Position::group(7)); - assert_eq!(old.wire(&requested).end(), Some(end.unwrap_or(Position::group(7)))); + let requested = Subscription::default().with_end(7); + assert_eq!(old.wire(&requested).end(), Some(end.unwrap_or(7))); } } @@ -4300,15 +4294,15 @@ mod test { // One reader has not read anything yet, so it still needs the oldest range. assert_eq!(routes.subscription(demand.clone(), 2).start(), None); assert_eq!(recv(&mut second).sequence, 5); - assert_eq!(routes.subscription(demand.clone(), 2).start(), Some(Position::group(5))); + assert_eq!(routes.subscription(demand.clone(), 2).start(), Some(5)); write(&a, 10); assert_eq!(recv(&mut first).sequence, 10); - assert_eq!(routes.subscription(demand.clone(), 2).start(), Some(Position::group(5))); + assert_eq!(routes.subscription(demand.clone(), 2).start(), Some(5)); let pending = front.consume().subscribe(Subscription::default().with_max_delay(HOUR)); assert_eq!(routes.subscription(front.subscription().unwrap(), 3).start(), None); drop(pending); drop(second); - assert_eq!(routes.subscription(demand, 1).start(), Some(Position::group(10))); + assert_eq!(routes.subscription(demand, 1).start(), Some(10)); } #[moq_clock::test] diff --git a/rs/moq-net/src/model/subscription.rs b/rs/moq-net/src/model/subscription.rs index f94d88aac45..bbca08af25f 100644 --- a/rs/moq-net/src/model/subscription.rs +++ b/rs/moq-net/src/model/subscription.rs @@ -80,8 +80,8 @@ pub struct Subscription { /// for nothing. /// /// A request to the publisher, aggregated across every live subscriber as the union of - /// their ranges, and this subscriber's read filter: it reads only the groups and frames - /// its own ranges cover, whatever another subscriber widens the union to. + /// their ranges, and this subscriber's read filter: it reads only the groups its own + /// ranges cover, whatever another subscriber widens the union to. /// [`crate::track::Subscriber::with_range`] narrows the reads further. See [Local cursor /// vs wire preference](crate::track::Subscriber#local-cursor-vs-wire-preference). pub ranges: Vec, @@ -108,9 +108,12 @@ pub enum Order { Descending, } -/// Where one end of a [`Range`] falls: a fixed position, a count of groups back from the +/// Where one end of a [`Range`] falls: a fixed group, a count of groups back from the /// latest one, or a presentation time. /// +/// Every anchor names a whole group: a subscription always starts at a group's first frame, +/// since a group is the unit of decodability. +/// /// A relative or time anchor resolves at the hop that serves it, when it starts serving /// and again whenever the subscriber replaces its ranges. Before any group exists a /// relative anchor resolves at the first one, so [`Range::live`] starts there. A relay's @@ -124,8 +127,8 @@ pub enum Order { /// rather than resolve it against the part of the track it holds. #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] pub enum Anchor { - /// This exact position. - Absolute(Position), + /// The group with this sequence. + Absolute(u64), /// The group this many before the latest, clamped to group 0: `Relative(0)` is the latest group. Relative(u64), /// The group containing this presentation time: the newest one starting at or before it. @@ -145,10 +148,9 @@ pub enum Anchor { /// A span of a track to deliver: from `start` up to `end`, or onward when `end` is `None`. /// -/// Relative and time anchors cover whole groups: such a start is frame 0 of its group, and -/// such an end includes its group, so `Absolute(0)..Relative(0)` delivers through the latest -/// group. An absolute end is exclusive, like the end of a [`std::ops::Range`]: see -/// [`Position::after`] and [`Position::after_group`]. +/// An absolute end is exclusive, like the end of a [`std::ops::Range`], so `Absolute(2).. +/// Absolute(6)` delivers groups 2 through 5. Relative and time ends include their group, so +/// `Absolute(0)..Relative(0)` delivers through the latest group. #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct Range { /// Where delivery starts. @@ -180,16 +182,16 @@ impl Range { /// The whole groups in `groups`, such as `2..=5`, `2..`, or `..6`. /// /// An omitted start is [`Self::oldest`]'s, and an inclusive end past the last group is - /// no end, as [`Position::after_group`] spells it. + /// no end. pub fn groups(groups: impl RangeBounds) -> Self { let unbounded_start = matches!(groups.start_bound(), Bound::Unbounded); let (start, end) = sequence_bounds(groups); Self { start: match unbounded_start { true => Self::oldest().start, - false => Anchor::Absolute(Position::group(start)), + false => Anchor::Absolute(start), }, - end: end.map(|end| Anchor::Absolute(Position::group(end))), + end: end.map(Anchor::Absolute), } } @@ -227,7 +229,7 @@ pub(super) fn union(ranges: impl IntoIterator) -> Vec { return vec![Range::oldest()]; } - let mut spans: Vec<(Position, Option)> = ranges + let mut spans: Vec<(u64, Option)> = ranges .iter() .filter_map(|range| match (range.start, range.end) { (Anchor::Absolute(start), None) => Some((start, None)), @@ -236,7 +238,7 @@ pub(super) fn union(ranges: impl IntoIterator) -> Vec { }) .collect(); spans.sort_unstable(); - let mut runs: Vec<(Position, Option)> = Vec::new(); + let mut runs: Vec<(u64, Option)> = Vec::new(); for (start, end) in spans { match runs.last_mut() { Some(last) if last.1.is_none_or(|last| start <= last) => { @@ -325,12 +327,12 @@ impl Subscription { self } - /// The lowest absolute position any range asks for, or `None` when one counts back - /// from the latest group (or none is asked for). + /// The lowest absolute group any range asks for, or `None` when one counts back from + /// the latest group (or none is asked for). /// /// The form a wire without ranges carries: a floor below which nothing is requested. - pub(crate) fn start(&self) -> Option { - let mut floor: Option = None; + pub(crate) fn start(&self) -> Option { + let mut floor: Option = None; for range in &self.ranges { let Anchor::Absolute(start) = range.start else { return None; @@ -340,13 +342,13 @@ impl Subscription { floor } - /// The exclusive end past every range, or `None` when one is open or counts back from - /// the latest group. An empty list is the empty range, `Position::group(0)`. + /// The exclusive end group past every range, or `None` when one is open or counts back + /// from the latest group. An empty list is the empty range, ending at group 0. /// /// The form a wire without ranges carries, alongside [`Self::start`]. #[cfg(any(feature = "async", test))] - pub(crate) fn end(&self) -> Option { - let mut end = Some(Position::default()); + pub(crate) fn end(&self) -> Option { + let mut end = Some(0); for range in &self.ranges { let Some(Anchor::Absolute(next)) = range.end else { return None; @@ -359,7 +361,7 @@ impl Subscription { /// Replace the ranges with one from `start` (or [`Range::oldest`]'s) up to [`Self::end`], /// returning `self` for chaining: the form a wire without ranges carries. #[cfg(any(feature = "async", test))] - pub(crate) fn with_start(self, start: impl Into>) -> Self { + pub(crate) fn with_start(self, start: impl Into>) -> Self { let end = self.end(); self.with_span(start.into(), end) } @@ -367,13 +369,13 @@ impl Subscription { /// Replace the ranges with one from [`Self::start`] up to the exclusive `end`, or onward /// for `None`, returning `self` for chaining: the form a wire without ranges carries. #[cfg(any(feature = "async", test))] - pub(crate) fn with_end(self, end: impl Into>) -> Self { + pub(crate) fn with_end(self, end: impl Into>) -> Self { let start = self.start(); self.with_span(start, end.into()) } #[cfg(any(feature = "async", test))] - fn with_span(self, start: Option, end: Option) -> Self { + fn with_span(self, start: Option, end: Option) -> Self { self.with_ranges([Range { start: start.map_or(Range::oldest().start, Anchor::Absolute), end: end.map(Anchor::Absolute), @@ -388,12 +390,7 @@ impl Subscription { /// keeps its anchor, since the floor may lie before it, so the caller drops what it /// resolves to below `floor`; an end not resolved becomes the bound, except a time end, /// which stays so the caller enforces `end` apart from it. - pub(super) fn clamped( - &self, - floor: Option, - end: Option, - resolved: &[Option], - ) -> Self { + pub(super) fn clamped(&self, floor: Option, end: Option, resolved: &[Option]) -> Self { let clamping = floor.is_some() || end.is_some(); let ranges = self .ranges @@ -402,15 +399,11 @@ impl Subscription { .map(|(index, range)| { let resolved = resolved.get(index).copied().flatten().filter(|_| clamping); let start = match (range.start, resolved) { - (Anchor::Relative(_) | Anchor::Time(_), Some(resolved)) => { - Anchor::Absolute(Position::group(resolved.start)) - } + (Anchor::Relative(_) | Anchor::Time(_), Some(resolved)) => Anchor::Absolute(resolved.start), (start, _) => start, }; let requested = match (range.end, resolved.and_then(|resolved| resolved.end)) { - (Some(Anchor::Relative(_) | Anchor::Time(_)), Some(resolved)) => { - Some(Anchor::Absolute(Position::group(resolved))) - } + (Some(Anchor::Relative(_) | Anchor::Time(_)), Some(resolved)) => Some(Anchor::Absolute(resolved)), (requested, _) => requested, }; Range { @@ -591,8 +584,8 @@ mod tests { combined.finish() } - fn absolute(group: u64, frame: u64) -> Anchor { - Anchor::Absolute(Position { group, frame }) + fn absolute(group: u64) -> Anchor { + Anchor::Absolute(group) } fn range(start: Anchor, end: impl Into>) -> Range { @@ -603,16 +596,16 @@ mod tests { combine(subscriptions).unwrap().ranges } - /// A group range spells both positions at once, in whichever form the caller has. + /// A group range spells both ends at once, in whichever form the caller has. #[test] - fn group_ranges_build_whole_group_positions() { - assert_eq!(Range::groups(2..=5), range(absolute(2, 0), absolute(6, 0))); - assert_eq!(Range::groups(2..6), range(absolute(2, 0), absolute(6, 0))); - assert_eq!(Range::groups(..6), range(Range::oldest().start, absolute(6, 0))); - assert_eq!(Range::groups(2..), range(absolute(2, 0), None)); + fn group_ranges_build_absolute_anchors() { + assert_eq!(Range::groups(2..=5), range(absolute(2), absolute(6))); + assert_eq!(Range::groups(2..6), range(absolute(2), absolute(6))); + assert_eq!(Range::groups(..6), range(Range::oldest().start, absolute(6))); + assert_eq!(Range::groups(2..), range(absolute(2), None)); assert_eq!(Range::groups(..), Range::oldest()); - // Through the last group is unbounded, as `after_group` spells it. + // Through the last group is unbounded. assert_eq!(Range::groups(..=u64::MAX).end, None); let sub = Subscription::default().with_range(2..=5).with_range(..); @@ -664,7 +657,7 @@ mod tests { assert_eq!(floor(&Subscription::default(), None), Some(0)); // The lowest range wins, and one unresolved range leaves the whole floor unresolved. - let mixed = Subscription::default().with_ranges([range(absolute(4, 0), None), Range::live()]); + let mixed = Subscription::default().with_ranges([range(absolute(4), None), Range::live()]); assert_eq!(floor(&mixed, Some(3)), Some(3)); assert_eq!(floor(&mixed, Some(9)), Some(4)); assert_eq!(floor(&mixed, None), None); @@ -682,33 +675,25 @@ mod tests { let sub = Subscription::default(); assert_eq!((sub.start(), sub.end()), (None, None)); - let sub = Subscription::default().with_ranges([Range::groups(2..4), range(absolute(7, 1), absolute(9, 0))]); - assert_eq!( - (sub.start(), sub.end()), - (Some(Position::group(2)), Some(Position::group(9))) - ); + let sub = Subscription::default().with_ranges([Range::groups(2..4), Range::groups(7..9)]); + assert_eq!((sub.start(), sub.end()), (Some(2), Some(9))); // An explicit group 0 is a floor, unlike the default. let sub = Subscription::default().with_range(0..); - assert_eq!((sub.start(), sub.end()), (Some(Position::group(0)), None)); + assert_eq!((sub.start(), sub.end()), (Some(0), None)); let sub = Subscription::default().with_ranges([Range::groups(2..4), Range::live()]); assert_eq!((sub.start(), sub.end()), (None, None)); - let sub = Subscription::default().with_ranges([range(absolute(2, 0), Anchor::Relative(0))]); - assert_eq!((sub.start(), sub.end()), (Some(Position::group(2)), None)); + let sub = Subscription::default().with_ranges([range(absolute(2), Anchor::Relative(0))]); + assert_eq!((sub.start(), sub.end()), (Some(2), None)); // Nothing asked for is the empty range. let sub = Subscription::default().with_ranges([]); - assert_eq!(sub.end(), Some(Position::group(0))); + assert_eq!(sub.end(), Some(0)); - let sub = Subscription::default() - .with_start(Position { group: 5, frame: 2 }) - .with_end(Position::after_group(9)); - assert_eq!( - (sub.start(), sub.end()), - (Some(Position { group: 5, frame: 2 }), Some(Position::group(10))) - ); + let sub = Subscription::default().with_start(5).with_end(10); + assert_eq!((sub.start(), sub.end()), (Some(5), Some(10))); assert_eq!(sub.with_start(None).with_end(None), Subscription::default()); } @@ -769,31 +754,6 @@ mod tests { ); } - /// Ranges fold as whole positions: two ranges in the same group are separated only by - /// their frames, so folding group and frame independently would invent a bound. - #[test] - fn combined_ranges_fold_whole_positions() { - let sub = |range: Range| Subscription::default().with_ranges([range]); - - assert_eq!( - ranges(&[ - sub(range(absolute(5, 9), absolute(5, 10))), - sub(range(absolute(5, 2), absolute(5, 3))) - ]), - vec![ - range(absolute(5, 2), absolute(5, 3)), - range(absolute(5, 9), absolute(5, 10)) - ] - ); - assert_eq!( - ranges(&[ - sub(range(absolute(5, 2), absolute(5, 9))), - sub(range(absolute(5, 4), absolute(6, 0))) - ]), - vec![range(absolute(5, 2), absolute(6, 0))] - ); - } - /// Combining is order-independent and stops changing once it covers everyone. #[test] fn combined_ranges_settle() { diff --git a/rs/moq-net/src/model/subscription/window.rs b/rs/moq-net/src/model/subscription/window.rs index e6bcb6ba114..811393ff706 100644 --- a/rs/moq-net/src/model/subscription/window.rs +++ b/rs/moq-net/src/model/subscription/window.rs @@ -402,37 +402,28 @@ impl Window { } /// Whether a datagram is in range, judged on its own and never through a time end's - /// settling: its `timestamp` against each range's times, its `sequence` against the - /// range's absolute groups and the relative ones resolved so far, and its `object` ID - /// against the frames that same range covers in its group. - pub fn admits_datagram(&self, sequence: u64, timestamp: Option, object: u64) -> bool { - let framed = |range: &Range| { - let (start, end) = frames(range, sequence); - object >= start && end.is_none_or(|end| object < end) - }; + /// settling: its `timestamp` against each range's times, and its `sequence` against the + /// range's absolute groups and the relative ones resolved so far. + pub fn admits_datagram(&self, sequence: u64, timestamp: Option) -> bool { let Some(timestamp) = timestamp else { - return self.pending - || self - .spans - .iter() - .any(|span| span.covers(sequence) && framed(&self.ranges[span.index])); + return self.pending || self.spans.iter().any(|span| span.covers(sequence)); }; self.ranges .iter() .enumerate() - .filter(|(_, range)| !range.is_empty() && framed(range)) + .filter(|(_, range)| !range.is_empty()) .any(|(index, range)| { // A relative anchor still waiting on a latest group bounds nothing yet. let span = self.spans.iter().find(|span| span.index == index); let started = match range.start { Anchor::Time(pts) => !later(pts, timestamp), - Anchor::Absolute(start) => sequence >= start.group, + Anchor::Absolute(start) => sequence >= start, Anchor::Relative(_) => span.is_none_or(|span| sequence >= span.start), }; let ended = match range.end { None => true, Some(Anchor::Time(pts)) => !later(timestamp, pts), - Some(Anchor::Absolute(end)) => end.final_sequence().is_none_or(|end| sequence < end), + Some(Anchor::Absolute(end)) => sequence < end, Some(Anchor::Relative(_)) => { span.is_none_or(|span| !matches!(span.end, End::Before(end) if sequence >= end)) } @@ -512,39 +503,6 @@ impl Window { all.into_iter().chain(runs.into_iter().flatten()) } - /// The frames of group `sequence` in range: from a start frame up to an exclusive end, - /// `None` through the group's end. - /// - /// Only an absolute anchor names a frame, so a group a range starts or ends inside reads - /// part of itself and every other group reads whole. Ranges that cover a group - /// differently read the hull of their frames, since a group is handed out once, as one - /// run of frames. Everything while a relative anchor waits. - pub fn frames(&self, sequence: u64) -> (u64, Option) { - if self.pending { - return (0, None); - } - let mut hull = None; - for span in self.spans.iter().filter(|span| span.covers(sequence)) { - let (start, end) = frames(&self.ranges[span.index], sequence); - hull = Some(match hull { - None => (start, end), - Some((low, high)) => (start.min(low), end.zip(high).map(|(end, high)| end.max(high))), - }); - } - hull.unwrap_or((0, None)) - } - - /// The groups some range starts inside, past their frame 0, that are read from there. - pub fn partial_starts(&self) -> impl Iterator + '_ { - self.ranges - .iter() - .filter_map(|range| match range.start { - Anchor::Absolute(start) if start.frame > 0 => Some(start.group), - _ => None, - }) - .filter(|group| self.frames(*group).0 > 0) - } - /// The first run of `range` that is in range. pub fn clip(&self, range: ops::Range) -> Option> { let last = range.end.checked_sub(1).filter(|last| *last >= range.start)?; @@ -623,26 +581,11 @@ impl Window { } } -/// The frames of group `sequence` that `range` names, if it covers that group: from a start -/// frame up to an exclusive end, `None` through the group's end. Only an absolute anchor -/// names a frame; a head-of-group end covers nothing of its group, so it never gets here. -fn frames(range: &Range, sequence: u64) -> (u64, Option) { - let start = match range.start { - Anchor::Absolute(start) if start.group == sequence => start.frame, - _ => 0, - }; - let end = match range.end { - Some(Anchor::Absolute(end)) if end.group == sequence => Some(end.frame), - _ => None, - }; - (start, end) -} - /// The first group a range starts at, and whether it names that group rather than /// reaching back to the oldest one. `None` while it waits on a latest group. fn start(anchor: Anchor, latest: Option, holdings: &impl Holdings) -> Option<(u64, bool)> { match anchor { - Anchor::Absolute(start) => Some((start.group, true)), + Anchor::Absolute(start) => Some((start, true)), // Nothing sits below group 0, so the farthest offset needs no latest group. Anchor::Relative(u64::MAX) => Some((0, false)), Anchor::Relative(back) => Some((latest?.saturating_sub(back), true)), @@ -661,7 +604,7 @@ fn start(anchor: Anchor, latest: Option, holdings: &impl Holdings) -> Optio fn end(anchor: Option, latest: Option) -> Option { Some(match anchor { None => End::Open, - Some(Anchor::Absolute(end)) => end.final_sequence().map_or(End::Open, End::Before), + Some(Anchor::Absolute(end)) => End::Before(end), Some(Anchor::Relative(back)) => latest? .saturating_sub(back) .checked_add(1) @@ -672,7 +615,6 @@ fn end(anchor: Option, latest: Option) -> Option { #[cfg(test)] mod tests { - use super::super::Position; use super::*; use std::cell::Cell; @@ -811,63 +753,12 @@ mod tests { ] ); - // A mid-group end includes its group, a head-of-group end excludes it. - let window = Window::new( - &[range( - Anchor::Absolute(Position { group: 1, frame: 2 }), - Anchor::Absolute(Position { group: 3, frame: 1 }), - )], - None, - &Untimed, - ) - .unwrap(); - assert_eq!(covered(&window, 6), vec![1, 2, 3]); - assert_eq!(window.end(), Some(4)); - // Nothing asked for is nothing in range, floored nowhere. let window = Window::new(&[], None, &Untimed).unwrap(); assert_eq!(covered(&window, 4), Vec::::new()); assert_eq!((window.floor(), window.end()), (Some(0), Some(0))); } - /// Only a group an absolute anchor falls inside reads part of itself, and overlapping - /// ranges read the hull of their frames there. - #[test] - fn frames_bound_only_the_groups_an_anchor_falls_inside() { - let at = |group, frame| Anchor::Absolute(Position { group, frame }); - let window = Window::new(&[range(at(4, 5), at(6, 3))], None, &Untimed).unwrap(); - assert_eq!(window.frames(4), (5, None)); - assert_eq!(window.frames(5), (0, None)); - assert_eq!(window.frames(6), (0, Some(3))); - - // Within one group, both ends bound it. - let window = Window::new(&[range(at(4, 2), at(4, 7))], None, &Untimed).unwrap(); - assert_eq!(window.frames(4), (2, Some(7))); - - // A second range reaching further either way widens the group to cover both. - let window = Window::new(&[range(at(4, 5), at(4, 7)), range(at(4, 2), at(4, 6))], None, &Untimed).unwrap(); - assert_eq!(window.frames(4), (2, Some(7))); - let window = Window::new(&[range(at(4, 5), None), range(at(2, 0), at(4, 3))], None, &Untimed).unwrap(); - assert_eq!(window.frames(4), (0, None)); - - // Relative anchors cover whole groups, and a pending one bounds nothing yet. - let window = Window::new(&[range(Anchor::Relative(1), at(9, 2))], Some(9), &Untimed).unwrap(); - assert_eq!((window.frames(8), window.frames(9)), ((0, None), (0, Some(2)))); - let window = Window::new(&[range(at(4, 5), None), Range::live()], None, &Untimed).unwrap(); - assert_eq!(window.frames(4), (0, None)); - - // A time end covers whole groups, through its cap while it waits to settle. - let groups = Groups::new([Groups::started(4, 4000), Groups::started(5, 5000), (6, Held::Pending)]); - let window = Window::new( - &[range(at(6, 2), None), range(time(4500), time(5000))], - Some(6), - &groups, - ) - .unwrap(); - assert!(!window.is_settled()); - assert_eq!(window.frames(6), (0, None)); - } - /// Relative anchors count back from the latest group, both ends whole groups, and wait /// for a first group when there is none. #[test] @@ -974,12 +865,7 @@ mod tests { #[test] fn an_unsettled_time_end_reads_one_group_on_speculation() { let mut groups = Groups::new([(3, Held::Pending), (4, Held::Pending), (5, Held::Pending)]); - let mut window = Window::new( - &[range(Anchor::Absolute(Position::group(3)), time(3500))], - Some(5), - &groups, - ) - .unwrap(); + let mut window = Window::new(&[range(Anchor::Absolute(3), time(3500))], Some(5), &groups).unwrap(); assert_eq!(covered(&window, 8), vec![3]); groups.held.insert(3, Held::Started(at(3000))); @@ -1035,14 +921,9 @@ mod tests { /// A datagram is one frame, so its own timestamp places it against a time end. #[test] fn a_datagram_is_judged_by_its_own_timestamp() { - let window = Window::new( - &[range(Anchor::Absolute(Position::group(0)), time(3500))], - None, - &Groups::default(), - ) - .unwrap(); - assert!(window.admits_datagram(9, Some(at(3500)), 0)); - assert!(!window.admits_datagram(9, Some(at(3600)), 0)); + let window = Window::new(&[range(Anchor::Absolute(0), time(3500))], None, &Groups::default()).unwrap(); + assert!(window.admits_datagram(9, Some(at(3500)))); + assert!(!window.admits_datagram(9, Some(at(3600)))); } /// Groups whose `racing` one starts past the end after the first time it is asked @@ -1090,12 +971,7 @@ mod tests { racing: 1, asked: Cell::new(false), }; - let window = Window::new( - &[range(Anchor::Absolute(Position::group(0)), time(5000))], - None, - &racing, - ) - .unwrap(); + let window = Window::new(&[range(Anchor::Absolute(0), time(5000))], None, &racing).unwrap(); assert_eq!(window.resolved(), vec![Some(Resolved { start: 0, end: Some(1) })]); } @@ -1119,12 +995,7 @@ mod tests { #[test] fn a_time_end_stops_walking_at_the_last_sequence() { let groups = Groups::new([Groups::started(u64::MAX, 3000)]); - let window = Window::new( - &[range(Anchor::Absolute(Position::group(u64::MAX)), time(5000))], - None, - &groups, - ) - .unwrap(); + let window = Window::new(&[range(Anchor::Absolute(u64::MAX), time(5000))], None, &groups).unwrap(); assert_eq!( window.resolved(), vec![Some(Resolved { @@ -1146,53 +1017,49 @@ mod tests { }; let window = Window::new(&[range(time(0), time(5000))], None, &groups).unwrap(); assert_eq!(window.resolved(), vec![Some(Resolved { start: 0, end: Some(2) })]); - assert!(window.admits_datagram(1, Some(at(4000)), 0)); - assert!(!window.admits_datagram(1, Some(at(7000)), 0)); + assert!(window.admits_datagram(1, Some(at(4000)))); + assert!(!window.admits_datagram(1, Some(at(7000)))); } #[test] fn a_waiting_window_judges_a_datagram_against_both_times() { let window = Window::new(&[range(time(3000), time(3500))], None, &Groups::default()).unwrap(); assert!(window.is_pending(), "no group has started"); - assert!(!window.admits_datagram(0, Some(at(2900)), 0)); - assert!(window.admits_datagram(1, Some(at(3000)), 0)); - assert!(window.admits_datagram(2, Some(at(3500)), 0)); - assert!(!window.admits_datagram(3, Some(at(3600)), 0)); + assert!(!window.admits_datagram(0, Some(at(2900)))); + assert!(window.admits_datagram(1, Some(at(3000)))); + assert!(window.admits_datagram(2, Some(at(3500)))); + assert!(!window.admits_datagram(3, Some(at(3600)))); } #[test] fn a_waiting_window_judges_a_datagram_against_absolute_groups_too() { let window = Window::new( - &[range(time(3000), Some(Anchor::Absolute(Position::group(10))))], + &[range(time(3000), Some(Anchor::Absolute(10)))], None, &Groups::default(), ) .unwrap(); assert!(window.is_pending(), "no group has started"); - assert!(window.admits_datagram(9, Some(at(3500)), 0)); - assert!(!window.admits_datagram(10, Some(at(3500)), 0)); - assert!(!window.admits_datagram(9, Some(at(2900)), 0)); + assert!(window.admits_datagram(9, Some(at(3500)))); + assert!(!window.admits_datagram(10, Some(at(3500)))); + assert!(!window.admits_datagram(9, Some(at(2900)))); } - /// A datagram is in range only when one range admits its sequence, time, and Object ID - /// together, not when each passes a different range. + /// A datagram is in range only when one range admits its sequence and time together, + /// not when each passes a different range. #[test] fn a_datagram_is_judged_against_one_range_at_a_time() { - let pos = |group, frame| Anchor::Absolute(Position { group, frame }); let groups = Groups::new([Groups::started(5, 5000)]); // The time range covers group 5 whole, but only from 5500 on. - let window = Window::new(&[range(pos(5, 1), None), range(time(5500), None)], Some(5), &groups).unwrap(); - assert!(!window.admits_datagram(5, Some(at(5000)), 0)); - assert!(window.admits_datagram(5, Some(at(5000)), 1)); - assert!(window.admits_datagram(5, Some(at(5500)), 0)); - - // Untimed, two ranges splitting a group admit only the Object IDs each names. - let window = Window::new(&[range(pos(4, 1), pos(4, 2)), range(pos(4, 5), None)], None, &Untimed).unwrap(); - assert_eq!(window.frames(4), (1, None), "a group stream reads the hull"); - let admitted: Vec = (0..7) - .filter(|object| window.admits_datagram(4, None, *object)) - .collect(); - assert_eq!(admitted, vec![1, 5, 6]); + let window = Window::new( + &[range(Anchor::Absolute(6), None), range(time(5500), None)], + Some(5), + &groups, + ) + .unwrap(); + assert!(!window.admits_datagram(5, Some(at(5000)))); + assert!(window.admits_datagram(5, Some(at(5500)))); + assert!(window.admits_datagram(6, Some(at(5000)))); } #[test] diff --git a/rs/moq-net/src/model/track.rs b/rs/moq-net/src/model/track.rs index ac06a660396..33f1178095a 100644 --- a/rs/moq-net/src/model/track.rs +++ b/rs/moq-net/src/model/track.rs @@ -3472,44 +3472,6 @@ impl Producer { poll_combined_changed(&subs, bound, &mut self.prev_subscription, waiter).map(Ok) } - /// Widen outgoing compressed ranges that cannot inflate from a cached prefix. - /// Only requesters call this: the original preferences keep filtering local reads, - /// and a server serves the frame bounds it was actually asked for. - /// - /// A range kept mid-group relies on the cached prefix, so it counts as a read of that - /// copy and normal eviction takes it last. Nothing pins it: a prefix still lost - /// before its tail lands fails that group with [`Error::Lagged`], which takes a - /// consumer slower than the cache's turnover. - #[cfg_attr( - not(feature = "async"), - allow(dead_code, reason = "Only session adapters request upstream.") - )] - pub(crate) fn widen_compressed(&self, subscription: &mut Subscription) { - if !self.info.compress { - return; - } - let state = self.state.read(); - for range in &mut subscription.ranges { - if range.is_empty() { - continue; - } - let Anchor::Absolute(start) = &mut range.start else { - continue; - }; - if start.frame == 0 { - continue; - } - let prefix = state - .lookup - .get(&start.group) - .and_then(|slot| slot.copies().find(|group| group.holds_prefix(start.frame))); - match prefix { - Some(group) => group.cache_refresh(), - None => start.frame = 0, - } - } - } - /// Create a [`Dynamic`] handle that serves on-demand fetches of uncached /// (old) groups. Most producers never need this; a relay creates one to fetch /// past groups from upstream. @@ -6076,21 +6038,11 @@ impl Cursor { } } - /// Hand `group` out: bounded to the frames this cursor's ranges cover in it, and expiring - /// against its budget. + /// Hand `group` out whole from its first frame, expiring against its budget. /// - /// Positioned without clamping, so a group missing the head asked for reads it as - /// [`Error::Lagged`], which a route copy recovers from another. A group relayed from - /// moq-transport is handed out whole, since its ranges may name Object IDs rather than - /// frame ordinals, so its reader filters by ID. - fn deliver(&self, mut group: group::Consumer) -> group::Consumer { - if !group.relayed() { - let (start, end) = self.filter.window.frames(group.sequence); - group.skip_to(start); - if let Some(end) = end { - group = group.with_end(end); - } - } + /// A group missing its head reads it as [`Error::Lagged`], which a route copy recovers + /// from another. + fn deliver(&self, group: group::Consumer) -> group::Consumer { let sequence = group.sequence; group.with_expiry(Arc::new(GroupExpiry { state: self.state.weak(), @@ -6108,7 +6060,6 @@ impl Cursor { let asking = { let subscription = self.subscription.read(); miss::Asking { - window, priority: subscription.priority, max_delay: subscription.max_delay, order: subscription.order, @@ -6332,9 +6283,8 @@ impl Cursor { let sequence = datagram.sequence; // A datagram is a whole group, so its own timestamp places it against a time end. - let object = datagram.wire.as_ref().and_then(|wire| wire.object_id).unwrap_or(0); let admits = super::subscription::before_end(sequence, self.end_sequence) - && self.filter.window.admits_datagram(sequence, datagram.timestamp, object); + && self.filter.window.admits_datagram(sequence, datagram.timestamp); if sequence >= self.floor() && admits { return Poll::Ready(Ok(Some(datagram))); } @@ -8105,7 +8055,7 @@ mod test { /// in arrival and sequence order alike, however wide another subscriber made the /// aggregate. #[test] - fn a_range_reads_only_its_frames() { + fn a_range_reads_only_its_groups() { let producer = track_producer("test", Info::default()); let _groups: Vec<_> = (3..8) .map(|sequence| { @@ -8116,23 +8066,22 @@ mod test { .collect(); let _wide = producer.subscribe(replay()); let range = Range { - start: Anchor::Absolute(Position { group: 4, frame: 5 }), - end: Some(Anchor::Absolute(Position { group: 6, frame: 3 })), + start: Anchor::Absolute(4), + end: Some(Anchor::Absolute(6)), }; let bounded = || producer.subscribe(replay().with_ranges([range])); - let want = vec![(4, vec![5, 6, 7]), (5, (0..8).collect()), (6, vec![0, 1, 2])]; + let want = vec![(4, (0..8).collect()), (5, (0..8).collect())]; let mut arrival = bounded(); assert_eq!(read_groups(|waiter| arrival.poll_recv_group(waiter)), want); let mut ordered = bounded().ordered(); assert_eq!(read_groups(|waiter| ordered.poll_next_group(waiter)), want); } - /// A group missing the frames a range asks for is never served short: reading them is - /// [`Error::Lagged`], which a route copy recovers. One holding them from where the range - /// starts is served from there. + /// A group missing its head is never served short: reading it is [`Error::Lagged`], + /// which a route copy recovers. #[test] - fn a_group_missing_its_asked_head_lags() { + fn a_group_missing_its_head_lags() { let producer = track_producer("test", Info::default()); let mut headless = producer.create_group(group::Info { sequence: 4 }).unwrap(); headless.start_at(2).unwrap(); @@ -8142,50 +8091,13 @@ mod test { headless.finish().unwrap(); let noop = kio::Waiter::noop(); - for frame in [0, 1] { - let start = Anchor::Absolute(Position { group: 4, frame }); - let mut sub = producer.subscribe(replay().with_ranges([Range { start, end: None }])); - let Poll::Ready(Ok(Some(mut group))) = sub.poll_recv_group(&noop) else { - panic!("from frame {frame}: no group"); - }; - assert_eq!(group.index(), frame); - assert!( - matches!(group.poll_read_frame(&noop), Poll::Ready(Err(Error::Lagged))), - "from frame {frame}" - ); - } - - let start = Anchor::Absolute(Position { group: 4, frame: 2 }); + let start = Anchor::Absolute(4); let mut sub = producer.subscribe(replay().with_ranges([Range { start, end: None }])); - assert_eq!( - read_groups(|waiter| sub.poll_recv_group(waiter)), - vec![(4, vec![2, 3, 4])] - ); - } - - /// A range replaced between a peek and the take re-bounds the reserved group: it is - /// still the group handed out, positioned by the ranges in force when it is taken. - #[test] - fn a_peeked_group_is_bounded_by_the_ranges_at_handout() { - for ordered in [false, true] { - let producer = track_producer("test", Info::default()); - framed(&producer, 4, 8).finish().unwrap(); - let mut subscriber = producer.subscribe(replay()); - let waiter = kio::Waiter::noop(); - assert_eq!( - subscriber.poll_peek_group(ordered, &waiter).map(Result::unwrap), - Poll::Ready(Some(4)) - ); - let start = Anchor::Absolute(Position { group: 4, frame: 5 }); - subscriber - .control() - .update(replay().with_ranges([Range { start, end: None }])) - .unwrap(); - let Poll::Ready(Ok(Some(group))) = subscriber.poll_group(ordered, &waiter) else { - panic!("ordered={ordered}: the reserved group must be returned"); - }; - assert_eq!((group.sequence, group.index()), (4, 5), "ordered={ordered}"); - } + let Poll::Ready(Ok(Some(mut group))) = sub.poll_recv_group(&noop) else { + panic!("no group"); + }; + assert_eq!(group.index(), 0); + assert!(matches!(group.poll_read_frame(&noop), Poll::Ready(Err(Error::Lagged)))); } /// The floor a reader waiting on each group starts at is the higher of its ranges' and @@ -9056,7 +8968,7 @@ mod test { #[cfg(feature = "async")] fn the_group_range_bounds_datagrams() { let mut producer = track_producer("test", Info::timed()); - let mut subscriber = producer.subscribe(Subscription::default().with_start(Position::group(5))); + let mut subscriber = producer.subscribe(Subscription::default().with_start(5)); subscriber.set_range(..7); for sequence in [4, 5, 7, 6] { producer @@ -9069,33 +8981,6 @@ mod test { assert!(subscriber.poll_recv_datagram(&kio::Waiter::noop()).is_pending()); } - /// A start past the start group's first frame excludes that group's datagram, its only - /// frame, even behind an earlier range; a start at frame 0 keeps it. - #[test] - #[cfg(feature = "async")] - fn a_mid_group_start_drops_the_start_groups_datagram() { - let mut producer = track_producer("test", Info::timed()); - let mut mid = producer.subscribe(Subscription::default().with_start(Position { group: 5, frame: 1 })); - let mut split = producer.subscribe(Subscription::default().with_ranges([ - Range::groups(2..4), - Range { - start: Anchor::Absolute(Position { group: 5, frame: 1 }), - end: None, - }, - ])); - let mut whole = producer.subscribe(Subscription::default().with_start(Position::group(5))); - for sequence in [5, 6] { - producer - .insert_datagram(sequence, Timestamp::ZERO, bytes::Bytes::from_static(b"x")) - .unwrap(); - } - - assert_eq!(recv_datagram(&mut mid).sequence, 6); - assert_eq!(recv_datagram(&mut split).sequence, 6); - assert_eq!(recv_datagram(&mut whole).sequence, 5); - assert_eq!(recv_datagram(&mut whole).sequence, 6); - } - /// A range update judges the datagrams still buffered when they are read, and one it /// dropped stays dropped once the cap rises again: nothing holds it like a parked group. #[test] @@ -10426,20 +10311,20 @@ mod test { // The budget is the only thing that asks for data; a named start only bounds how // far back it may reach. Naming group 1 at real time still delivers the live edge // alone, since the zero budget calls everything older stale. - let named = Subscription::default().with_start(Position::group(1)); + let named = Subscription::default().with_start(1); let mut subscriber = producer.subscribe(named); assert_eq!(drain(&mut subscriber), vec![4]); // A budget reaching further back than the floor is cut off at it. let floored = Subscription::default() - .with_start(Position::group(3)) + .with_start(3) .with_max_delay(Duration::from_secs(10)); let mut subscriber = producer.subscribe(floored); assert_eq!(drain(&mut subscriber), vec![3, 4]); // A floor below what the budget admits changes nothing. let slack = Subscription::default() - .with_start(Position::group(1)) + .with_start(1) .with_max_delay(Duration::from_secs(2)); let mut subscriber = producer.subscribe(slack); assert_eq!(drain(&mut subscriber), vec![2, 3, 4]); @@ -10456,7 +10341,7 @@ mod test { // A resumed subscription names where it left off, which may not exist yet. The // cursor sits at the floor rather than sliding back to what is cached. let resumed = Subscription::default() - .with_start(Position::group(7)) + .with_start(7) .with_max_delay(Duration::from_secs(10)); let mut subscriber = producer.subscribe(resumed); assert_eq!(drain(&mut subscriber), Vec::::new()); @@ -11253,13 +11138,13 @@ mod test { // Asking to start at the beginning is a filter, not a request for reliability. // Backfill needs a budget that covers it; without one the live edge still wins. - let mut subscriber = producer.subscribe(Subscription::default().with_start(Position::group(0))); + let mut subscriber = producer.subscribe(Subscription::default().with_start(0)); subscriber.set_range(0..); assert_eq!(drain(&mut subscriber), vec![3]); let mut patient = producer.subscribe( Subscription::default() - .with_start(Position::group(0)) + .with_start(0) .with_max_delay(Duration::from_secs(10)), ); patient.set_range(0..); @@ -11400,9 +11285,9 @@ mod test { append_untimed(&mut producer); } - let mut subscriber = producer.subscribe(replay().with_start(Position::group(2))); + let mut subscriber = producer.subscribe(replay().with_start(2)); assert_eq!(drain(&mut subscriber), vec![2, 3, 4]); - let mut live = producer.subscribe(Subscription::default().with_start(Position::group(2))); + let mut live = producer.subscribe(Subscription::default().with_start(2)); assert_eq!(drain(&mut live), vec![4]); } @@ -11416,8 +11301,8 @@ mod test { append_untimed(&mut producer); } - let mut subscriber = producer.subscribe(Subscription::default().with_end(Position::group(3))); - subscriber.set_range((Bound::Unbounded, Position::group(3).group_end())); + let mut subscriber = producer.subscribe(Subscription::default().with_end(3)); + subscriber.set_range(..3); assert_eq!(drain(&mut subscriber), vec![2]); } @@ -11869,7 +11754,7 @@ mod test { assert_eq!(next.sequence, 1); // A budget that covers it delivers the range, and nothing above it. - let mut subscriber = producer.subscribe(replay().with_end(Position::after_group(1))); + let mut subscriber = producer.subscribe(replay().with_end(2)); assert_eq!(drain(&mut subscriber), vec![0, 1]); } @@ -11935,7 +11820,7 @@ mod test { let subscription = Subscription::default() .with_max_delay(Duration::ZERO) .with_ranges([Range { - start: Anchor::Absolute(Position::group(0)), + start: Anchor::Absolute(0), end: Some(Anchor::Relative(0)), }]); let mut subscriber = producer.subscribe(subscription); @@ -12185,7 +12070,7 @@ mod test { use std::sync::atomic::{AtomicBool, Ordering}; let mut producer = track_producer("test", Info::timed()); - let _narrow = producer.subscribe(Subscription::default().with_end(Position::after_group(3))); + let _narrow = producer.subscribe(Subscription::default().with_end(4)); let mut wide = producer.subscribe(Subscription::default()); let woken = Arc::new(AtomicBool::new(false)); @@ -12198,14 +12083,13 @@ mod test { assert!(producer.poll_subscription_changed(&waiter).is_pending()); assert!(!woken.load(Ordering::SeqCst), "nothing happened yet"); - wide.update(Subscription::default().with_end(Position::after_group(5))) - .unwrap(); + wide.update(Subscription::default().with_end(6)).unwrap(); assert!( woken.load(Ordering::SeqCst), "the widest subscriber changing must wake the aggregate watcher", ); match producer.poll_subscription_changed(&waiter) { - Poll::Ready(Ok(Some(sub))) => assert_eq!(sub.end(), Position::after_group(5)), + Poll::Ready(Ok(Some(sub))) => assert_eq!(sub.end(), Some(6)), other => panic!("expected the narrowed aggregate, got {other:?}"), } } @@ -14680,7 +14564,13 @@ mod test { let dynamic = producer.dynamic(); let consumer = producer.consume(); - let _tail = consumer.fetch_group(5, group::Fetch::default().with_frame_start(3)); + let _tail = consumer.fetch_group( + 5, + group::Fetch { + frame_start: 3, + ..Default::default() + }, + ); let req = dynamic .requested_fill_group() .now_or_never() @@ -14778,7 +14668,13 @@ mod test { group.finish().unwrap(); // A fetch for the tail is covered and resolves from the cache. - let fetch = consumer.fetch_group(0, group::Fetch::default().with_frame_start(3)); + let fetch = consumer.fetch_group( + 0, + group::Fetch { + frame_start: 3, + ..Default::default() + }, + ); let cached = fetch.now_or_never().expect("covered by the cache").unwrap(); assert_eq!(cached.index(), 3); @@ -15070,12 +14966,24 @@ mod test { let dynamic = producer.dynamic(); let consumer = producer.consume(); - let _narrow = consumer.fetch_group(0, group::Fetch::default().with_frame_start(5)); + let _narrow = consumer.fetch_group( + 0, + group::Fetch { + frame_start: 5, + ..Default::default() + }, + ); let mut narrow = std::pin::pin!(_narrow); assert!(futures::poll!(narrow.as_mut()).is_pending()); // Still queued: widening is honored, because nothing has read the range yet. - let _wider = consumer.fetch_group(0, group::Fetch::default().with_frame_start(2)); + let _wider = consumer.fetch_group( + 0, + group::Fetch { + frame_start: 2, + ..Default::default() + }, + ); let mut wider = std::pin::pin!(_wider); assert!(futures::poll!(wider.as_mut()).is_pending()); @@ -15084,7 +14992,13 @@ mod test { // Handed off now: the handler holds its own copy of the range, so a later wider // caller cannot move what is already being served. - let _widest = consumer.fetch_group(0, group::Fetch::default().with_frame_start(0)); + let _widest = consumer.fetch_group( + 0, + group::Fetch { + frame_start: 0, + ..Default::default() + }, + ); let mut widest = std::pin::pin!(_widest); assert!(futures::poll!(widest.as_mut()).is_pending()); assert_eq!(request.frame_start(), 2, "the in-flight range is already on the wire"); @@ -15406,56 +15320,6 @@ mod test { assert!(consumer.peek_group(1).is_none(), "unread group is evicted instead"); } - /// Requesting a compressed tail from frame N reads the cached prefix it relies on, as a - /// FETCH hit does, so pressure evicts a never-read group first and the prefix stays to - /// seed the tail's inflater. - #[test] - fn a_requested_prefix_survives_eviction() { - let (producer, pool) = pooled_producer(u64::MAX); - let producer = Producer::new( - producer.broadcast.clone(), - "compressed", - Info::timed().with_compress(true), - ); - let consumer = producer.consume(); - // Incompressible, so each group caches about its payload. - let mut seed = 1u32; - let mut append = || { - let payload: Vec = (0..3_000) - .map(|_| { - seed = seed.wrapping_mul(1_103_515_245).wrapping_add(12_345); - (seed >> 16) as u8 - }) - .collect(); - let mut group = producer.append_group().unwrap(); - group.write_frame(Timestamp::ZERO, payload).unwrap(); - group.finish().unwrap(); - }; - - append(); // seq 0 - elapse(&producer, Duration::from_secs(1)); - append(); // seq 1 - elapse(&producer, Duration::from_secs(1)); - let used = pool.used(); - append(); // seq 2 - // Room for the three groups and a little more, so each later append evicts one. - pool.resize(pool.used() + (pool.used() - used) / 3); - elapse(&producer, Duration::from_millis(500)); - - let start = Anchor::Absolute(Position { group: 0, frame: 1 }); - let mut request = Subscription::default().with_ranges([Range { start, end: None }]); - producer.widen_compressed(&mut request); - assert_eq!(request.ranges[0].start, start, "a held prefix keeps the request at N"); - elapse(&producer, Duration::from_millis(500)); - - append(); // seq 3 - elapse(&producer, Duration::from_secs(1)); - append(); // seq 4 - - assert!(consumer.peek_group(0).is_some(), "the requested prefix survives"); - assert!(consumer.peek_group(1).is_none(), "an unread group is evicted instead"); - } - /// A consumer holding an evicted group surfaces the eviction, not a hang or a /// truncated clean end. #[moq_clock::test] @@ -16876,9 +16740,13 @@ mod test { } evict(&producer, 3); let waiter = kio::Waiter::noop(); - let tail = producer - .consume() - .fetch_group(3, group::Fetch::default().with_frame_start(2)); + let tail = producer.consume().fetch_group( + 3, + group::Fetch { + frame_start: 2, + ..Default::default() + }, + ); let Poll::Ready(Ok(first)) = dynamic.poll_requested_fill(&waiter) else { panic!("an evicted group is a miss"); }; @@ -17384,96 +17252,6 @@ mod test { assert_eq!(fill.next(), Some(WIDE - REFUSED - 1)); } - /// A reader starting in an earlier group widens a queued fill to the whole of its first - /// group, not just one starting where it does. - #[test] - fn an_earlier_reader_widens_a_queued_fill_to_its_head() { - let mut producer = track_producer("test", Info::timed()); - let dynamic = producer.dynamic(); - let start = Anchor::Absolute(Position { group: 1, frame: 7 }); - let mut mid = producer.subscribe(replay().with_ranges([Range { start, end: None }])); - let mut whole = producer.subscribe(replay().with_range(0..)); - producer.start_at(4).unwrap(); - write_group(&producer, 4); - let _ = mid.poll_recv_group(&kio::Waiter::noop()); - let _ = whole.poll_recv_group(&kio::Waiter::noop()); - - let Poll::Ready(Ok(fill)) = dynamic.poll_requested_fill(&kio::Waiter::noop()) else { - panic!("the miss below the start is asked"); - }; - assert_eq!((fill.start(), fill.end()), (Position { group: 1, frame: 0 }, 4)); - } - - /// Each fill starts at the frame its own range starts from, not the lowest start of - /// the reader's ranges. - #[test] - fn a_fill_starts_at_its_own_ranges_frame() { - let mut producer = track_producer("test", Info::timed()); - let dynamic = producer.dynamic(); - let at = |group, frame| Anchor::Absolute(Position { group, frame }); - let mut reader = producer.subscribe(replay().with_ranges([ - Range { - start: at(0, 0), - end: Some(at(1, 0)), - }, - Range { - start: at(5, 7), - end: None, - }, - ])); - producer.start_at(8).unwrap(); - write_group(&producer, 8); - let _ = reader.poll_recv_group(&kio::Waiter::noop()); - - let mut asked = Vec::new(); - while let Poll::Ready(Ok(fill)) = dynamic.poll_requested_fill(&kio::Waiter::noop()) { - asked.push((fill.start(), fill.end())); - } - asked.sort(); - assert_eq!( - asked, - [ - (Position { group: 0, frame: 0 }, 1), - (Position { group: 5, frame: 7 }, 8) - ] - ); - } - - /// A range starting inside the group after another range heads its own fill, rather - /// than being read whole as the second group of one. - #[test] - fn an_adjacent_range_starting_mid_group_heads_its_own_fill() { - let mut producer = track_producer("test", Info::timed()); - let dynamic = producer.dynamic(); - let at = |group, frame| Anchor::Absolute(Position { group, frame }); - let mut reader = producer.subscribe(replay().with_ranges([ - Range { - start: at(4, 0), - end: Some(at(5, 0)), - }, - Range { - start: at(5, 7), - end: None, - }, - ])); - producer.start_at(8).unwrap(); - write_group(&producer, 8); - let _ = reader.poll_recv_group(&kio::Waiter::noop()); - - let mut asked = Vec::new(); - while let Poll::Ready(Ok(fill)) = dynamic.poll_requested_fill(&kio::Waiter::noop()) { - asked.push((fill.start(), fill.end())); - } - asked.sort(); - assert_eq!( - asked, - [ - (Position { group: 4, frame: 0 }, 5), - (Position { group: 5, frame: 7 }, 8) - ] - ); - } - /// A descending reader joining a queued fill makes it newest first, as any descending /// subscriber does a subscription. #[test] @@ -18009,23 +17787,6 @@ mod test { assert_eq!(drain(&mut ahead), vec![8]); } - /// A frame-precise start survives the merge with a live range, for the publisher to - /// trim on the wire. Locally it floors reads at its group. - #[test] - #[cfg(feature = "async")] - fn a_frame_precise_start_merged_with_live_keeps_its_frame() { - let producer = quiet_track(3); - let precise = Range { - start: Anchor::Absolute(Position { group: 1, frame: 2 }), - end: None, - }; - let mut from_frame = producer.subscribe(replay().with_ranges([precise])); - let mut live = producer.subscribe(replay().with_ranges([Range::live()])); - assert_eq!(producer.subscription().unwrap().ranges, vec![precise, Range::live()]); - assert_eq!(drain(&mut from_frame), vec![1, 2, 3]); - assert_eq!(drain(&mut live), vec![3]); - } - /// An untimed track replays from group 0 only what the wall clock hasn't made stale. #[test] #[cfg(feature = "async")] @@ -18670,7 +18431,7 @@ mod test { two.write_frame(at(2000), bytes::Bytes::from_static(b"x")).unwrap(); let mut subscriber = producer.subscribe(replay().with_ranges([Range { - start: Anchor::Absolute(Position::group(0)), + start: Anchor::Absolute(0), end: Some(Anchor::Time(at(500))), }])); hole.write_frame(at(1000), bytes::Bytes::from_static(b"x")).unwrap(); @@ -18685,7 +18446,7 @@ mod test { fn a_time_end_waits_on_an_absent_group_below_it() { let at = |millis| Timestamp::from_millis(millis).unwrap(); let ranges = [Range { - start: Anchor::Absolute(Position::group(0)), + start: Anchor::Absolute(0), end: Some(Anchor::Time(at(500))), }]; for dropped in [false, true] { @@ -18763,7 +18524,7 @@ mod test { .map(|sequence| producer.create_group(group::Info { sequence }).unwrap()) .collect(); let mut subscriber = producer.subscribe(replay().with_ranges([Range { - start: Anchor::Absolute(Position::group(0)), + start: Anchor::Absolute(0), end: Some(Anchor::Time(at(3500))), }])); assert_eq!(drain(&mut subscriber), vec![0]); @@ -18808,7 +18569,7 @@ mod test { let at = |millis| Timestamp::from_millis(millis).unwrap(); let mut producer = track_producer("test", Info::timed()); let mut subscriber = producer.subscribe(replay().with_ranges([Range { - start: Anchor::Absolute(Position::group(0)), + start: Anchor::Absolute(0), end: Some(Anchor::Time(at(3500))), }])); for millis in [3000, 3400, 3600, 3700] { diff --git a/rs/moq-net/src/model/track/demand.rs b/rs/moq-net/src/model/track/demand.rs index 63f93789511..c113d52cb77 100644 --- a/rs/moq-net/src/model/track/demand.rs +++ b/rs/moq-net/src/model/track/demand.rs @@ -289,7 +289,7 @@ impl From> for Preferences { #[cfg(test)] mod tests { use super::super::super::subscription::Combined; - use super::super::{Anchor, Position, Range}; + use super::super::{Anchor, Range}; use super::*; use rand::{RngExt, SeedableRng}; use std::sync::atomic::{AtomicUsize, Ordering}; @@ -306,10 +306,7 @@ mod tests { let anchor = |rng: &mut rand::rngs::StdRng| match rng.random_range(0..4) { 0 => Anchor::Relative(rng.random_range(0..3)), 1 => Anchor::Relative(u64::MAX), - _ => Anchor::Absolute(Position { - group: rng.random_range(0..8), - frame: rng.random_range(0..4), - }), + _ => Anchor::Absolute(rng.random_range(0..8)), }; let ranges: Vec = (0..rng.random_range(0..3)) .map(|_| Range { diff --git a/rs/moq-net/src/model/track/demand/ranges.rs b/rs/moq-net/src/model/track/demand/ranges.rs index 88bae86d296..30f4595b89d 100644 --- a/rs/moq-net/src/model/track/demand/ranges.rs +++ b/rs/moq-net/src/model/track/demand/ranges.rs @@ -1,7 +1,7 @@ //! The union of every subscriber's ranges, kept current per change rather than refolded. //! -//! Absolute ranges become signed boundaries over positions, +1 where one starts and -1 -//! where it ends, so a position is covered while the running sum stays above zero. A treap +//! Absolute ranges become signed boundaries over groups, +1 where one starts and -1 +//! where it ends, so a group is covered while the running sum stays above zero. A treap //! keeps each subtree's sum and lowest running sum, so a change touches two boundaries and //! asks one range-minimum question, whatever else the audience holds. Ranges anchored to //! the latest group or to a time can't be placed on that line and are counted by value @@ -11,7 +11,7 @@ use std::cell::OnceCell; use std::collections::BTreeMap; use std::hash::{BuildHasher, RandomState}; -use super::super::{Anchor, Position, Range}; +use super::super::{Anchor, Range}; use super::{add, remove}; /// Every subscriber's ranges, counted so that one change costs O(log n). @@ -32,7 +32,7 @@ pub(super) struct Ranges { enum Shape { Empty, Oldest, - Absolute(Position, Option), + Absolute(u64, Option), Relative(u64), Mixed, } @@ -146,7 +146,7 @@ impl Ranges { } } -/// Signed boundary counts over positions: the coverage of the absolute ranges. +/// Signed boundary counts over groups: the coverage of the absolute ranges. #[derive(Default)] struct Coverage { root: Link, @@ -156,7 +156,7 @@ struct Coverage { type Link = Option>; struct Node { - key: Position, + key: u64, /// Ranges starting here, minus ranges ending here. Never zero. delta: i64, priority: u64, @@ -170,14 +170,14 @@ struct Node { impl Coverage { /// Count `delta` more ranges over `start..end`, open when `end` is `None`. - fn add(&mut self, start: Position, end: Option, delta: i64) { + fn add(&mut self, start: u64, end: Option, delta: i64) { self.shift(start, delta); if let Some(end) = end { self.shift(end, -delta); } } - fn shift(&mut self, key: Position, delta: i64) { + fn shift(&mut self, key: u64, delta: i64) { let (below, rest) = split(self.root.take(), &key, false); let (at, above) = split(rest, &key, true); let delta = at.map_or(0, |node| node.delta) + delta; @@ -185,8 +185,8 @@ impl Coverage { self.root = merge(merge(below, at), above); } - /// The fewest ranges covering any position in `start..end`. - fn min(&mut self, start: Position, end: Option) -> i64 { + /// The fewest ranges covering any group in `start..end`. + fn min(&mut self, start: u64, end: Option) -> i64 { let (through, rest) = split(self.root.take(), &start, true); let (inside, above) = match end { Some(end) => split(rest, &end, false), @@ -199,14 +199,14 @@ impl Coverage { } /// The covered spans in order, the last one open while an open range remains. - fn runs(&self) -> Vec<(Position, Option)> { + fn runs(&self) -> Vec<(u64, Option)> { let mut runs = Vec::new(); walk(&self.root, 0, &mut runs); runs } /// Where the open span starts, if an open range remains. - fn tail(&self) -> Option { + fn tail(&self) -> Option { if sum(&self.root) <= 0 { return None; } @@ -218,7 +218,7 @@ impl Coverage { } impl Node { - fn new(key: Position, delta: i64, priority: u64) -> Box { + fn new(key: u64, delta: i64, priority: u64) -> Box { Box::new(Self { key, delta, @@ -246,7 +246,7 @@ fn low(link: &Link) -> i64 { } /// Split into the keys below `key` (through it when `inclusive`) and the rest. -fn split(link: Link, key: &Position, inclusive: bool) -> (Link, Link) { +fn split(link: Link, key: &u64, inclusive: bool) -> (Link, Link) { let Some(mut node) = link else { return (None, None); }; @@ -281,7 +281,7 @@ fn merge(left: Link, right: Link) -> Link { } } -fn first(link: &Link) -> Option { +fn first(link: &Link) -> Option { let mut node = link.as_ref()?; while let Some(left) = &node.left { node = left; @@ -290,7 +290,7 @@ fn first(link: &Link) -> Option { } /// The first key above `after`. -fn successor(mut link: &Link, after: &Position) -> Option { +fn successor(mut link: &Link, after: &u64) -> Option { let mut found = None; while let Some(node) = link { if node.key > *after { @@ -307,7 +307,7 @@ fn successor(mut link: &Link, after: &Position) -> Option { /// /// A subtree entered covered whose running sum never reaches zero holds no boundary of the /// union, so it is skipped whole: the walk visits O(log n) nodes per span it returns. -fn walk(link: &Link, offset: i64, runs: &mut Vec<(Position, Option)>) { +fn walk(link: &Link, offset: i64, runs: &mut Vec<(u64, Option)>) { let Some(node) = link else { return; }; @@ -326,7 +326,7 @@ fn walk(link: &Link, offset: i64, runs: &mut Vec<(Position, Option)>) } /// The last key where the running sum, starting at `offset`, is zero. -fn last_zero(link: &Link, offset: i64) -> Option { +fn last_zero(link: &Link, offset: i64) -> Option { let node = link.as_ref()?; if offset.saturating_add(node.low) > 0 { return None; @@ -355,10 +355,7 @@ mod tests { let anchor = |rng: &mut rand::rngs::StdRng| match rng.random_range(0..6) { 0 => Anchor::Relative(rng.random_range(0..4)), 1 => Anchor::Relative(u64::MAX), - _ => Anchor::Absolute(Position { - group: rng.random_range(0..12), - frame: rng.random_range(0..3), - }), + _ => Anchor::Absolute(rng.random_range(0..12)), }; let mut ranges = Ranges::default(); let mut held: Vec = Vec::new(); diff --git a/rs/moq-net/src/model/track/miss.rs b/rs/moq-net/src/model/track/miss.rs index 673c4ab8d20..3a1a36ccacb 100644 --- a/rs/moq-net/src/model/track/miss.rs +++ b/rs/moq-net/src/model/track/miss.rs @@ -26,12 +26,9 @@ use rangemap::{RangeInclusiveSet, RangeSet}; use super::fill::{Ask, FillState, Join}; use super::{Drift, Order, Position, TrackState}; use crate::Error; -use crate::model::subscription::Window; /// What one reader asks its fills with. -pub(super) struct Asking<'a> { - /// The reader's resolved ranges, for the frame each run's first group starts from. - pub window: &'a Window, +pub(super) struct Asking { pub priority: i8, pub max_delay: Option, pub order: Order, @@ -433,33 +430,16 @@ impl Misses { state: &kio::Consumer, fills: &kio::Shared, runs: &RangeSet, - asking: &Asking<'_>, + asking: &Asking, given_up: &mut GivenUp, ) { - // A group a range starts inside heads its own fill, since a fill reads every group - // after its first whole. - let mut cuts: Vec = asking.window.partial_starts().collect(); - cuts.sort_unstable(); - cuts.dedup(); - let mut runs: Vec> = runs - .iter() - .flat_map(|run| { - let inside = cuts.iter().copied().filter(|cut| run.start < *cut && *cut < run.end); - let starts = std::iter::once(run.start).chain(inside.clone()); - let ends = inside.chain(std::iter::once(run.end)); - starts.zip(ends).map(|(start, end)| start..end) - }) - .collect(); + let mut runs: Vec> = runs.iter().cloned().collect(); if asking.order == Order::Descending { runs.reverse(); } for run in runs { - let (frame, _) = asking.window.frames(run.start); let ask = Ask { - start: Position { - group: run.start, - frame, - }, + start: Position::group(run.start), end: run.end, priority: asking.priority, max_delay: asking.max_delay, diff --git a/rs/moq-net/tests/integration/compress.rs b/rs/moq-net/tests/integration/compress.rs index a846b1efac3..8f6b53cca01 100644 --- a/rs/moq-net/tests/integration/compress.rs +++ b/rs/moq-net/tests/integration/compress.rs @@ -52,17 +52,45 @@ struct Fixture { _origins: Vec, } -/// A compressed track crosses a relay between the requested protocols. -async fn connect(upstream: &str, downstream: &str) -> Fixture { - connect_with(upstream, downstream, moq_net::track::Subscription::default()).await +/// A track announced by the original publisher, before any relay carries it. +struct Published { + publisher: moq_net::origin::Producer, + broadcast: moq_net::broadcast::Producer, + track: moq_net::track::Producer, } -/// As [`connect`], with the downstream application asking for `subscription`. -async fn connect_with(upstream: &str, downstream: &str, subscription: moq_net::track::Subscription) -> Fixture { +/// Announce one track described by `info`. +fn publish(info: moq_net::track::Info) -> Published { let publisher = produce_origin(1); let broadcast = publisher.create_broadcast("bcast").unwrap(); - let track = broadcast.create_track("data", compressed()).unwrap(); + let track = broadcast.create_track("data", info).unwrap(); broadcast.announce(Default::default()).unwrap(); + Published { + publisher, + broadcast, + track, + } +} + +/// A compressed track crosses a relay between the requested protocols. +async fn connect(upstream: &str, downstream: &str) -> Fixture { + relay( + publish(compressed()), + [upstream, downstream], + moq_net::track::Subscription::default(), + ) + .await +} + +/// `published` crosses a relay over the `[upstream, downstream]` protocols to an application +/// asking for `subscription`. +async fn relay(published: Published, hops: [&str; 2], subscription: moq_net::track::Subscription) -> Fixture { + let Published { + publisher, + broadcast, + track, + } = published; + let [upstream, downstream] = hops; let relay = produce_origin(2); let mut options = MockConnectOptions::new(upstream.parse::().unwrap()); @@ -91,52 +119,74 @@ async fn connect_with(upstream: &str, downstream: &str, subscription: moq_net::t } } -/// A cold subscriber needs the compressed prefix, but its application sees only its -/// requested half-open frame range, including when it changes that range while live. +/// Hops that carry a start group and negotiate compression: lite-07, and moq-transport +/// draft-17 and later. +const RANGED: &[&str] = &[ + "moq-lite-07-wip", + "moq-transport-17", + "moq-transport-18", + "moq-transport-22", +]; + +/// A subscription starting at a group delivers it whole from its first frame across every +/// pair of hops, compressed or not, whether the group began before it subscribed or after. #[moq_clock::test] -async fn application_ranges_keep_their_filter_after_requesting_the_compressed_prefix() { - use moq_net::track::{Anchor, Position, Range, Subscription}; - moq_clock::timeout(TIMEOUT, async { - let range = |group, start, end| { - Subscription::default().with_ranges([Range { - start: Anchor::Absolute(Position { group, frame: start }), - end: Some(Anchor::Absolute(Position { group, frame: end })), - }]) - }; - let mut fixture = connect_with("moq-lite-07-wip", "moq-lite-07-wip", range(5, 2, 4)).await; - assert_eq!( - fixture.track.subscription_changed().await.unwrap().unwrap().ranges[0].start, - Anchor::Absolute(Position::group(5)), - "upstream asks for the prefix" - ); - for sequence in [5, 6] { - if sequence == 6 { - fixture.subscriber.update(range(6, 1, 3)).unwrap(); - let updated = fixture.track.subscription_changed().await.unwrap().unwrap(); - // Old route copies can keep their earlier floor until a newer group arrives. - // The updated end must already permit the newly requested group. - assert_eq!( - updated.ranges[0].end, - Some(Anchor::Absolute(Position { group: 6, frame: 3 })) - ); - } - let mut group = fixture.track.create_group(moq_net::group::Info { sequence }).unwrap(); - for index in 0..5 { - group - .write_frame(Timestamp::from_millis(index).unwrap(), payload(index)) - .unwrap(); - } - group.finish().unwrap(); - let mut reading = fixture.subscriber.recv_group().await.unwrap().unwrap(); - let (start, end) = if sequence == 5 { (2, 4) } else { (1, 3) }; - for index in start..end { - assert_eq!(reading.read_frame().await.unwrap().unwrap().payload, payload(index)); +async fn a_start_group_arrives_whole() { + use moq_net::track::{Anchor, Subscription}; + let write = |group: &mut moq_net::group::Producer, frames: std::ops::Range| { + for index in frames { + group + .write_frame(Timestamp::from_millis(index).unwrap(), payload(index)) + .unwrap(); + } + }; + for &upstream in RANGED { + for &downstream in RANGED { + let hops = [upstream, downstream]; + let lite = hops.iter().all(|hop| *hop == "moq-lite-07-wip"); + for compress in [true, false] { + // Frames of the group written before the subscription, which a joining FETCH + // or fill delivers. + for head in [0, 3] { + let case = format!("{upstream} -> {downstream}, compress {compress}, head {head}"); + moq_clock::timeout(TIMEOUT, async { + let info = match compress { + true => compressed(), + false => moq_net::track::Info::default().with_timescale(moq_net::Timescale::MILLI), + }; + let published = publish(info); + let mut group = published + .track + .create_group(moq_net::group::Info { sequence: 5 }) + .unwrap(); + write(&mut group, 0..head); + let mut fixture = relay(published, hops, Subscription::default().with_range(5..)).await; + if lite { + assert_eq!( + fixture.track.subscription_changed().await.unwrap().unwrap().ranges[0].start, + Anchor::Absolute(5), + "{case}: upstream asks for the start group" + ); + } + write(&mut group, head..5); + group.finish().unwrap(); + let mut reading = fixture.subscriber.recv_group().await.unwrap().unwrap(); + assert_eq!(reading.sequence, 5, "{case}"); + for index in 0..5 { + assert_eq!( + reading.read_frame().await.unwrap().unwrap().payload, + payload(index), + "{case}" + ); + } + assert!(reading.read_frame().await.unwrap().is_none(), "{case}"); + }) + .await + .unwrap_or_else(|_| panic!("{case}: the group never arrived whole")); + } } - assert!(reading.read_frame().await.unwrap().is_none()); } - }) - .await - .expect("the requested frames arrive"); + } } /// Whole and chunked frames on a compressed track read back as written on every version, diff --git a/rs/moq-net/tests/integration/head_fetch.rs b/rs/moq-net/tests/integration/head_fetch.rs deleted file mode 100644 index 7dcb5459d49..00000000000 --- a/rs/moq-net/tests/integration/head_fetch.rs +++ /dev/null @@ -1,171 +0,0 @@ -#![cfg(feature = "async")] - -//! A relay that fetches the head of a group it is receiving mid-group keeps delivering -//! that group to arrival-order subscribers, along with the frames written into it after. -//! -//! A mesh peer subscribing from partway into an open group (a catalog: snapshot, then -//! deltas) has its relay pull the group headless. Reading the peer's copy from frame 0 -//! then fetches the head through the relay, whose cache must not trade the live group -//! for the fetched copy. - -use crate::support; - -use std::time::Duration; - -use moq_net::track::{Anchor, Position, Range, Subscription}; -use moq_net::{Hop, Timestamp, Version}; -use support::harness::{MockConnectOptions, connect_mock}; - -/// A millisecond track: the frames written here carry timestamps. -fn timed() -> moq_net::track::Info { - moq_net::track::Info::default().with_timescale(moq_net::Timescale::MILLI) -} - -const TIMEOUT: Duration = Duration::from_secs(5); - -const VERSIONS: &[&str] = &[ - "moq-lite-03", - "moq-lite-05", - "moq-lite-06", - "moq-lite-07-wip", - "moq-transport-14", - "moq-transport-17", - "moq-transport-22", -]; - -fn produce_origin(hop: u64) -> moq_net::origin::Producer { - let (producer, driver) = moq_net::origin::Producer::new(moq_net::origin::Config::new(Hop::new(hop).unwrap())); - support::harness::spawn(driver); - producer -} - -async fn resolve(origin: &moq_net::origin::Producer) -> moq_net::broadcast::Consumer { - let consumer = origin.consume(); - moq_clock::timeout(TIMEOUT, consumer.routed("bcast")) - .await - .expect("announce timeout") - .expect("routed"); - moq_clock::timeout(TIMEOUT, consumer.request_broadcast("bcast", None)) - .await - .expect("resolve timeout") - .expect("broadcast resolves") -} - -async fn read(group: &mut moq_net::group::Consumer) -> Vec { - moq_clock::timeout(TIMEOUT, group.read_frame()) - .await - .expect("no frame") - .expect("read_frame") - .expect("group ended") - .payload - .to_vec() -} - -/// Publisher P holds an open group, relay R pulls P, peer M pulls R from the group's -/// second frame and then reads its copy from the first. A fresh subscriber on R must -/// still get the whole group, and a frame P writes afterward. -async fn round(version: &str) { - let version: Version = version.parse().unwrap(); - let publisher = produce_origin(1); - let broadcast = publisher.create_broadcast("bcast").unwrap(); - let track = broadcast.create_track("catalog.json", timed()).unwrap(); - broadcast.announce(Default::default()).unwrap(); - - let mut group = track.append_group().unwrap(); - let sequence = group.sequence; - for payload in [&b"snapshot"[..], b"delta1", b"delta2"] { - group.write_frame(Timestamp::now(), payload).unwrap(); - } - - let relay = produce_origin(2); - let mut options = MockConnectOptions::new(version); - options.server_publish = Some(publisher.consume()); - options.client_subscribe = Some(relay.clone()); - let upstream = connect_mock(options).await; - - let peer = produce_origin(3); - let mut options = MockConnectOptions::new(version); - options.server_publish = Some(relay.consume()); - options.client_subscribe = Some(peer.clone()); - let downstream = connect_mock(options).await; - - let remote = resolve(&relay).await; - let peer_remote = resolve(&peer).await; - - let start = Position { - group: sequence, - frame: 1, - }; - let mut peer_sub = peer_remote - .track("catalog.json") - .unwrap() - .subscribe(Subscription::default().with_ranges([Range { - start: Anchor::Absolute(start), - end: None, - }])) - .await - .expect("peer subscribe"); - let mut peer_group = moq_clock::timeout(TIMEOUT, peer_sub.recv_group()) - .await - .expect("peer got no group") - .unwrap() - .unwrap(); - // The peer reads its headless copy from frame 0: a fetch of the head through R. - let _ = moq_clock::timeout(Duration::from_millis(500), peer_group.read_frame()).await; - - // A fresh reader on R wants the latest group from its snapshot. - let mut sub = remote.track("catalog.json").unwrap().subscribe(None).await.unwrap(); - let mut got = moq_clock::timeout(TIMEOUT, sub.recv_group()) - .await - .unwrap_or_else(|_| panic!("{version}: no group")) - .unwrap() - .unwrap(); - assert_eq!(got.sequence, sequence, "{version}"); - assert_eq!(read(&mut got).await, b"snapshot", "{version}"); - assert_eq!(read(&mut got).await, b"delta1", "{version}"); - assert_eq!(read(&mut got).await, b"delta2", "{version}"); - - // The publisher keeps writing into the group: the reader on R hears it. - moq_clock::sleep(Duration::from_secs(1)).await; - group.write_frame(Timestamp::now(), &b"delta3"[..]).unwrap(); - assert_eq!( - read(&mut got).await, - b"delta3", - "{version}: frame written after the fetch" - ); - - // Every reader of the fetched copy leaves, so its fetch is abandoned, while both - // subscriptions keep the live feed running. The live group still holds R's slot: a - // later reader gets the whole group again. - drop((got, peer_group)); - moq_clock::sleep(Duration::from_secs(1)).await; - let mut late_sub = remote.track("catalog.json").unwrap().subscribe(None).await.unwrap(); - let mut late = moq_clock::timeout(TIMEOUT, late_sub.recv_group()) - .await - .unwrap_or_else(|_| panic!("{version}: no group after the fetch was abandoned")) - .unwrap() - .unwrap(); - assert_eq!(late.sequence, sequence, "{version}"); - for payload in [&b"snapshot"[..], b"delta1", b"delta2", b"delta3"] { - assert_eq!( - read(&mut late).await, - payload, - "{version}: after the fetch was abandoned" - ); - } - - drop(( - late, late_sub, sub, peer_sub, group, track, broadcast, downstream, upstream, publisher, relay, peer, - )); -} - -#[moq_clock::test] -async fn a_fetched_head_keeps_the_live_group_visible() { - let mut failures = Vec::new(); - for version in VERSIONS { - if moq_clock::sim::spawn(round(version)).await.is_err() { - failures.push(*version); - } - } - assert!(failures.is_empty(), "failed: {failures:?}"); -} diff --git a/rs/moq-net/tests/integration/late_lower_group.rs b/rs/moq-net/tests/integration/late_lower_group.rs index 2cf29eee790..a7aa035cdbd 100644 --- a/rs/moq-net/tests/integration/late_lower_group.rs +++ b/rs/moq-net/tests/integration/late_lower_group.rs @@ -11,7 +11,7 @@ use crate::support; use std::time::Duration; -use moq_net::track::{Anchor, Info, Position, Range, Subscription}; +use moq_net::track::{Anchor, Info, Range, Subscription}; use moq_net::{Timestamp, Version}; use support::harness::{MockConnectOptions, connect_mock}; @@ -28,7 +28,7 @@ const LITE: &[&str] = &["moq-lite-05", "moq-lite-06", "moq-lite-07-wip"]; const TRANSPORT: &[&str] = &["moq-transport-14", "moq-transport-17", "moq-transport-22"]; /// From `start`, or the oldest group still around when there is none. -fn floored(start: Option) -> Range { +fn floored(start: Option) -> Range { Range { start: start.map_or(Range::oldest().start, Anchor::Absolute), end: None, @@ -79,7 +79,7 @@ async fn read_all(sub: &mut moq_net::track::Subscriber) -> Vec<(u64, usize)> { } /// Group 1, then a fresh group 0, over `version`. `relay` inserts one hop. -async fn session(version: &str, relay: bool, start: Option) -> Vec<(u64, usize)> { +async fn session(version: &str, relay: bool, start: Option) -> Vec<(u64, usize)> { let version: Version = version.parse().unwrap(); let publisher = produce_origin(1); let broadcast = publisher.create_broadcast("bcast").unwrap(); @@ -156,7 +156,7 @@ async fn session(version: &str, relay: bool, start: Option) -> Vec<(u6 got } -async fn in_process(start: Option) -> Vec<(u64, usize)> { +async fn in_process(start: Option) -> Vec<(u64, usize)> { let publisher = produce_origin(1); let broadcast = publisher.create_broadcast("bcast").unwrap(); let track = broadcast @@ -190,14 +190,14 @@ async fn an_explicit_floor_receives_a_group_created_below_the_first_served_group let want = vec![(1, 4), (0, 4)]; let mut failures = Vec::new(); - let got = in_process(Some(Position::group(0))).await; + let got = in_process(Some(0)).await; if got != want { failures.push(format!("in-process: {got:?}")); } for version in LITE.iter().chain(TRANSPORT) { for relay in [false, true] { - let got = session(version, relay, Some(Position::group(0))).await; + let got = session(version, relay, Some(0)).await; if got != want { failures.push(format!("{version} relay={relay}: {got:?}")); } diff --git a/rs/moq-net/tests/integration/main.rs b/rs/moq-net/tests/integration/main.rs index 896e7e72599..b7d86740ae3 100644 --- a/rs/moq-net/tests/integration/main.rs +++ b/rs/moq-net/tests/integration/main.rs @@ -29,7 +29,6 @@ mod finished_broadcast_mock; mod finite_relay; mod goaway; mod group_order; -mod head_fetch; mod history_groups; mod idle_cancel_refetch; mod idle_session; diff --git a/rs/moq-net/tests/integration/range_anchors.rs b/rs/moq-net/tests/integration/range_anchors.rs index ee22231a19b..d1d1c3e2c7d 100644 --- a/rs/moq-net/tests/integration/range_anchors.rs +++ b/rs/moq-net/tests/integration/range_anchors.rs @@ -9,7 +9,7 @@ use crate::support; use std::time::Duration; -use moq_net::track::{Anchor, Info, Order, Position, Range, Subscription}; +use moq_net::track::{Anchor, Info, Order, Range, Subscription}; use moq_net::{Error, Hop, Timescale, Timestamp, Version}; use support::harness::peer; @@ -107,7 +107,7 @@ fn range(start: Anchor, end: Option) -> Range { /// Relative and time anchors at each end, with what a reader of the held track reads. fn anchored() -> [(Range, Vec); 6] { - let zero = Anchor::Absolute(Position::group(0)); + let zero = Anchor::Absolute(0); [ (range(Anchor::Relative(2), None), vec![2, 3, 4]), (range(Anchor::Time(millis(2500)), None), vec![2, 3, 4]), diff --git a/rs/moq-transcode/src/lib.rs b/rs/moq-transcode/src/lib.rs index 2feba4d52d5..c194ca66861 100644 --- a/rs/moq-transcode/src/lib.rs +++ b/rs/moq-transcode/src/lib.rs @@ -1432,70 +1432,6 @@ mod tests { transcoder.abort(); } - /// A fetch that starts partway through a group is refused rather than served. - /// A fresh encode of that group need not match the bytes of the head a reader - /// holds, so its tail cannot continue it. The reader moves on to the next group. - #[tokio::test] - async fn a_mid_group_fetch_is_refused() { - let source = source_catalog(320, 240); - // Serve any source group on demand, so a fetch that is not refused has - // something to transcode and resolves. - let source_fetches = source._track.dynamic(); - let server = tokio::spawn(async move { - while let Ok(request) = source_fetches.requested_fill().await { - let mut group = request.accept(request.start().group, None).unwrap(); - write_keyframe(&mut group); - group.finish().unwrap(); - } - }); - - let config = Config { - ladder: Ladder::new([Rung::new(120, moq_net::bandwidth::Rate::from_bps(100_000))]).unwrap(), - encoder: moq_video::encode::Kind::Software, - decoder: moq_video::decode::Kind::Software, - source: None, - ..Default::default() - }; - let output = moq_net::broadcast::Info::default().produce(); - let consumer = output.consume(); - let transcoder = tokio::spawn(run(source.broadcast.consume(), output, config)); - - let catalog = loop { - match consumer.track(hang::Catalog::DEFAULT_NAME) { - Ok(track) => break track, - Err(moq_net::Error::NotFound) => tokio::task::yield_now().await, - Err(err) => panic!("catalog track: {err}"), - } - }; - let mut catalogs = moq_mux::catalog::hang::Consumer::<()>::new(catalog.subscribe(None).await.unwrap()); - await_catalog(&mut catalogs, |snapshot| { - snapshot.video.renditions.contains_key("video/120p") - }) - .await; - - // The source publishes no live groups, so group 7 is a cache miss that - // reaches the rung's fetch handler. - let rung = consumer.track("video/120p").unwrap(); - rung.query().await.unwrap(); - - let partial = rung - .fetch_group(7, moq_net::group::Fetch::default().with_frame_start(2)) - .await; - match partial { - Err(moq_net::Error::Unsupported) => {} - Err(err) => panic!("expected an Unsupported refusal, got {err}"), - Ok(_) => panic!("served a fetch that starts mid-group"), - } - - // The whole group still serves. - let mut whole = rung.fetch_group(7, None).await.unwrap(); - while whole.read_frame().await.unwrap().is_some() {} - assert!(whole.finished().await.unwrap() > 0, "the whole group had no frames"); - - server.abort(); - transcoder.abort(); - } - /// A fetch dropped before its group is encoded never opens that encode, and /// the request goes with it. A later fetch of the same group is a new /// request: the abandoned one must not still be encoding into the cache, diff --git a/swift/Sources/Moq/Aliases.swift b/swift/Sources/Moq/Aliases.swift index 3a99de9ee17..06d4d4ea615 100644 --- a/swift/Sources/Moq/Aliases.swift +++ b/swift/Sources/Moq/Aliases.swift @@ -121,7 +121,7 @@ public typealias ProtocolError = MoqFFI.MoqProtocolError extension Subscription { /// A span of a track to deliver: from `start` up to `end`, or onward when `end` is `nil`. public typealias Range = MoqFFI.MoqRange - /// Where one end of a `Range` falls: `.absolute(group:frame:)`, `.relative(groups:)` + /// Where one end of a `Range` falls: `.absolute(group:)`, `.relative(groups:)` /// back from the latest group, or `.time(timestampUs:)` for the group containing it. public typealias Anchor = MoqFFI.MoqAnchor /// Which pending group the publisher sends first: `.ascending` or `.descending`.