diff --git a/cpp/moq/MIGRATE.md b/cpp/moq/MIGRATE.md index 025937be93a..00ff7db554b 100644 --- a/cpp/moq/MIGRATE.md +++ b/cpp/moq/MIGRATE.md @@ -4,6 +4,12 @@ How to update your code for each breaking change, newest first. ## Unreleased +### Priorities are signed (#6142) + +`TrackInfo::priority`, `Subscription::priority`, and `FetchGroupOptions::priority` are `int8_t`, and `GroupRequest::priority()` returns one. +Higher is first and 0 is the unset midpoint; `TrackInfo::priority` defaulted to 127 and now defaults to 0. +To keep a value's moq-lite byte, subtract 128 from it, widening first (`static_cast(int(old) - 128)`): 255 becomes 127, 128 becomes 0, and 0 becomes -128. + ### A client asks only about what it watches (#6165) On moq-lite, a client session asks its relay for announcements only under paths something watches, through `announced` or `announced_broadcast`, instead of under its whole scope at connect. diff --git a/dart/moq/MIGRATE.md b/dart/moq/MIGRATE.md index 2c305a2cb5a..2b1f516eb10 100644 --- a/dart/moq/MIGRATE.md +++ b/dart/moq/MIGRATE.md @@ -4,6 +4,12 @@ How to update your code for each breaking change, newest first. ## Unreleased +### Priorities are signed (#6142) + +`TrackInfo.priority`, `Subscription.priority`, and `FetchGroupOptions.priority` take `-128..=127`, higher first, and 0 is the unset midpoint. +`TrackInfo.priority` defaulted to 127 and now defaults to 0. +To keep a value's moq-lite byte, subtract 128 from it: 255 becomes 127, 128 becomes 0, and 0 becomes -128. + ### A client asks only about what it watches (#6165) On moq-lite, a client session asks its relay for announcements only under paths something watches, through `announced()` or `announcedBroadcast()`, instead of under its whole scope at connect. diff --git a/dart/moq_ffi/MIGRATE.md b/dart/moq_ffi/MIGRATE.md index d8d452d9e39..8df537cb61c 100644 --- a/dart/moq_ffi/MIGRATE.md +++ b/dart/moq_ffi/MIGRATE.md @@ -4,6 +4,13 @@ How to update your code for each breaking change, newest first. ## Unreleased +### Priorities are signed (#6142) + +`MoqTrackInfo.priority`, `MoqSubscription.priority`, `MoqFetchGroupOptions.priority`, and `MoqGroupRequest.priority()` take `-128..=127`, higher first, and 0 is the unset midpoint. +`MoqTrackInfo.priority` defaulted to 127 and now defaults to 0. +To keep a value's moq-lite byte, subtract 128 from it: 255 becomes 127, 128 becomes 0, and 0 becomes -128. +Regenerate the bindings for the changed checksums. + ### A client asks only about what it watches (#6165) On moq-lite, a client session asks its relay for announcements only under paths something watches, through `announced()` or `announcedBroadcast()`, instead of under its whole scope at connect. diff --git a/dart/moq_ffi/lib/src/moq.dart b/dart/moq_ffi/lib/src/moq.dart index 05440ca2688..b94b2a2f438 100644 --- a/dart/moq_ffi/lib/src/moq.dart +++ b/dart/moq_ffi/lib/src/moq.dart @@ -579,7 +579,7 @@ class FfiConverterMoqFetchGroupOptions { static LiftRetVal read(Uint8List buf) { int new_offset = buf.offsetInBytes; - final priority_lifted = FfiConverterUInt8.read( + final priority_lifted = FfiConverterInt8.read( Uint8List.view(buf.buffer, new_offset), ); final priority = priority_lifted.value; @@ -597,7 +597,7 @@ class FfiConverterMoqFetchGroupOptions { static RustBuffer lower(MoqFetchGroupOptions value) { final total_length = - FfiConverterUInt8.allocationSize(value.priority) + + FfiConverterInt8.allocationSize(value.priority) + FfiConverterOptionalUInt64.allocationSize(value.maxDelayUs) + 0; final buf = Uint8List(total_length); @@ -607,7 +607,7 @@ class FfiConverterMoqFetchGroupOptions { static int write(MoqFetchGroupOptions value, Uint8List buf) { int new_offset = buf.offsetInBytes; - new_offset += FfiConverterUInt8.write( + new_offset += FfiConverterInt8.write( value.priority, Uint8List.view(buf.buffer, new_offset), ); @@ -619,7 +619,7 @@ class FfiConverterMoqFetchGroupOptions { } static int allocationSize(MoqFetchGroupOptions value) { - return FfiConverterUInt8.allocationSize(value.priority) + + return FfiConverterInt8.allocationSize(value.priority) + FfiConverterOptionalUInt64.allocationSize(value.maxDelayUs) + 0; } @@ -704,7 +704,7 @@ class FfiConverterMoqSubscription { static LiftRetVal read(Uint8List buf) { int new_offset = buf.offsetInBytes; - final priority_lifted = FfiConverterUInt8.read( + final priority_lifted = FfiConverterInt8.read( Uint8List.view(buf.buffer, new_offset), ); final priority = priority_lifted.value; @@ -737,7 +737,7 @@ class FfiConverterMoqSubscription { static RustBuffer lower(MoqSubscription value) { final total_length = - FfiConverterUInt8.allocationSize(value.priority) + + FfiConverterInt8.allocationSize(value.priority) + FfiConverterUInt64.allocationSize(value.maxDelayUs) + FfiConverterOptionalSequenceMoqRange.allocationSize(value.ranges) + FfiConverterOptionalMoqOrder.allocationSize(value.order) + @@ -749,7 +749,7 @@ class FfiConverterMoqSubscription { static int write(MoqSubscription value, Uint8List buf) { int new_offset = buf.offsetInBytes; - new_offset += FfiConverterUInt8.write( + new_offset += FfiConverterInt8.write( value.priority, Uint8List.view(buf.buffer, new_offset), ); @@ -769,7 +769,7 @@ class FfiConverterMoqSubscription { } static int allocationSize(MoqSubscription value) { - return FfiConverterUInt8.allocationSize(value.priority) + + return FfiConverterInt8.allocationSize(value.priority) + FfiConverterUInt64.allocationSize(value.maxDelayUs) + FfiConverterOptionalSequenceMoqRange.allocationSize(value.ranges) + FfiConverterOptionalMoqOrder.allocationSize(value.order) + @@ -2739,7 +2739,7 @@ class MoqTrackInfo { final int? timescale; final bool compress; MoqTrackInfo({ - this.priority = 127, + this.priority = 0, this.maxAgeUs = null, this.refresh = null, this.timescale = null, @@ -2754,7 +2754,7 @@ class FfiConverterMoqTrackInfo { static LiftRetVal read(Uint8List buf) { int new_offset = buf.offsetInBytes; - final priority_lifted = FfiConverterUInt8.read( + final priority_lifted = FfiConverterInt8.read( Uint8List.view(buf.buffer, new_offset), ); final priority = priority_lifted.value; @@ -2793,7 +2793,7 @@ class FfiConverterMoqTrackInfo { static RustBuffer lower(MoqTrackInfo value) { final total_length = - FfiConverterUInt8.allocationSize(value.priority) + + FfiConverterInt8.allocationSize(value.priority) + FfiConverterOptionalUInt64.allocationSize(value.maxAgeUs) + FfiConverterOptionalUInt32.allocationSize(value.refresh) + FfiConverterOptionalUInt64.allocationSize(value.timescale) + @@ -2806,7 +2806,7 @@ class FfiConverterMoqTrackInfo { static int write(MoqTrackInfo value, Uint8List buf) { int new_offset = buf.offsetInBytes; - new_offset += FfiConverterUInt8.write( + new_offset += FfiConverterInt8.write( value.priority, Uint8List.view(buf.buffer, new_offset), ); @@ -2830,7 +2830,7 @@ class FfiConverterMoqTrackInfo { } static int allocationSize(MoqTrackInfo value) { - return FfiConverterUInt8.allocationSize(value.priority) + + return FfiConverterInt8.allocationSize(value.priority) + FfiConverterOptionalUInt64.allocationSize(value.maxAgeUs) + FfiConverterOptionalUInt32.allocationSize(value.refresh) + FfiConverterOptionalUInt64.allocationSize(value.timescale) + @@ -11764,7 +11764,7 @@ class MoqGroupRequest implements MoqGroupRequestInterface { uniffiClonePointer(), status, ), - FfiConverterUInt8.lift, + FfiConverterInt8.lift, null, ); } @@ -13340,6 +13340,29 @@ class FfiConverterDouble64 { } } +class FfiConverterInt8 { + static int lift(int value) => value; + static LiftRetVal read(Uint8List buf) { + return LiftRetVal(buf.buffer.asByteData(buf.offsetInBytes).getInt8(0), 1); + } + + static int lower(int value) { + if (value < -128 || value > 127) { + throw ArgumentError("Value out of range for i8: " + value.toString()); + } + return value; + } + + static int allocationSize([int value = 0]) { + return 1; + } + + static int write(int value, Uint8List buf) { + buf.buffer.asByteData(buf.offsetInBytes).setInt8(0, lower(value)); + return 1; + } +} + class FfiConverterMapStringToMoqAudio { static Map lift(RustBuffer buf) { return FfiConverterMapStringToMoqAudio.read(buf.asUint8List()).value; @@ -15515,29 +15538,6 @@ class FfiConverterUInt64 { } } -class FfiConverterUInt8 { - static int lift(int value) => value; - static LiftRetVal read(Uint8List buf) { - return LiftRetVal(buf.buffer.asByteData(buf.offsetInBytes).getUint8(0), 1); - } - - static int lower(int value) { - if (value < 0 || value > 255) { - throw ArgumentError("Value out of range for u8: " + value.toString()); - } - return value; - } - - static int allocationSize([int value = 0]) { - return 1; - } - - static int write(int value, Uint8List buf) { - buf.buffer.asByteData(buf.offsetInBytes).setUint8(0, lower(value)); - return 1; - } -} - class FfiConverterUint8List { static Uint8List lift(RustBuffer value) { return FfiConverterUint8List.read(value.asUint8List()).value; @@ -17491,7 +17491,7 @@ external Pointer uniffi_moq_ffi_fn_method_moqgrouprequest_demand( Pointer uniffiStatus, ); -@Native, Pointer)>( +@Native, Pointer)>( assetId: _uniffiAssetId, ) external int uniffi_moq_ffi_fn_method_moqgrouprequest_priority( @@ -19692,7 +19692,7 @@ void _checkApiChecksums() { if (uniffi_moq_ffi_checksum_method_moqgrouprequest_demand() != 31288) { throw UniffiInternalError.panicked("UniFFI API checksum mismatch"); } - if (uniffi_moq_ffi_checksum_method_moqgrouprequest_priority() != 1745) { + if (uniffi_moq_ffi_checksum_method_moqgrouprequest_priority() != 15373) { throw UniffiInternalError.panicked("UniFFI API checksum mismatch"); } if (uniffi_moq_ffi_checksum_method_moqgrouprequest_sequence() != 29523) { diff --git a/doc/concept/moq-lite.md b/doc/concept/moq-lite.md index ee43da035d3..ee3f10472ee 100644 --- a/doc/concept/moq-lite.md +++ b/doc/concept/moq-lite.md @@ -236,7 +236,7 @@ Each subscription carries the knobs that decide behavior under congestion: | Knob | Effect | | --- | --- | -| **Priority** (0..255) | Higher-priority tracks get bandwidth first. Audio above video, base layer above enhancement. From a native Rust publisher over QUIC or WebTransport (iroh excepted), subscriptions at the same 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. | +| **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. | | **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. | diff --git a/doc/concept/standard.md b/doc/concept/standard.md index 9bd982e143b..586d5a57213 100644 --- a/doc/concept/standard.md +++ b/doc/concept/standard.md @@ -35,7 +35,7 @@ subset drops. What a peer actually observes against this implementation: - **Refused, not fatal.** A legal request this stack does not serve is rejected on its own and the session stays up: `FORWARD=0` on a `SUBSCRIBE`, JavaScript's `SUBSCRIBE_TRACKS`, and the fetch forms above get `NOT_SUPPORTED`. Range Filters get `INVALID_FILTER` on draft 19 and later because the advertised filter limit is zero. On draft 21 and later, namespaces whose first tuple field is exactly `.` or `.session` get `DOES_NOT_EXIST` before application lookup; other dot prefixes still reach the application. Subscription updates accept priority, negotiated subscriber max age, and delivery timeouts; Rust also renews request tokens and judges `NEW_GROUP_REQUEST` as a join (see dynamic groups below). Other supported-but-unimplemented preferences end that subscription; a `TRACK_PROPERTY_FILTER` there closes the session with `PROTOCOL_VIOLATION`, since it belongs only to `SUBSCRIBE_TRACKS`. On draft 18 and later a namespace update may change its prefix; suffixes after the acknowledgement are relative to that new prefix. - **Objects and datagrams.** Only subgroup 0 is accepted: a stream for any other subgroup, whether its header gives the ID or takes it from the first Object ID, is stopped and the session stays up. Rust relays preserve Object IDs, gaps, object properties, FIRST\_OBJECT, and END\_OF\_GROUP status objects between moq-transport peers for subgroup 0, including across a moq-lite-07 hop between two relays that negotiate the [ietf](/draft/moq-ietf) extension. Datagrams retain their IDs, properties, priority, and status. Plain Rust readers and moq-lite peers see consecutive frames and normal datagram payloads; JavaScript supports only normal datagrams at Object ID 0. A malformed datagram closes the session; a relayed one the downstream draft cannot carry is dropped. A relayed Timestamp leaves in the units the downstream response declares, and is dropped where it declares none (drafts 14 through 16), as for a plain frame; an immutable one that would change or vanish is refused. On a timed track the frame holds each object's time, so a mutable Timestamp that only repeats the immutable one is not restated; a status object has no frame, so it keeps its Timestamp among its properties. Only property 0x10 is read as a Timestamp. Property 0x06 stays opaque and keeps its value regardless of timestamp units; it is dropped across the draft-19 boundary, or refused when immutable, because draft 19 assigns it to SUBGROUP\_DELIVERY\_TIMEOUT. From draft 15, a status object carrying properties is refused, and one relayed from draft 14 leaves without them. Draft 14 allows them, but an END\_OF\_TRACK carrying properties ends the subscription as unsupported, since a track end has nowhere to keep them. A `FETCH` carries the END\_OF\_GROUP object on drafts 14 and 15; later drafts have no FETCH status, so their response ends at the last frame. A group cannot resume across route changes by frame ordinal once Object IDs skip ahead of the ordinals on either route; recovery refuses it instead of splicing different objects. - **Dynamic groups.** A track advertises `DYNAMIC_GROUPS` when its publisher sets a refresh ratio, and a track received with `DYNAMIC_GROUPS` gets a ratio of 0. A `NEW_GROUP_REQUEST` (draft 15 and later) counts as one more subscriber joining, which a `SUBSCRIBE` already is: it asks for a new group only when the newest group's bytes after its first frame exceed the ratio times the first frame. Joins coalesce until the asked-for group arrives. A Rust relay sends `NEW_GROUP_REQUEST` upstream, on the `SUBSCRIBE` or in a `REQUEST_UPDATE`, once a join wants a group past the newest it holds; a relay holding nothing yet asks for none. On a track without a ratio, an update carrying one ends the subscription with `UPDATE_FAILED`. On drafts 15 and 16, whose `TRACK_STATUS` answer has no room for it, a relay copy first learned from `TRACK_STATUS` (a fetch-only reader) does not advertise it to later subscribers. A moq-lite-07 hop (`moq-lite-07-wip`, opt-in) carries the same demand natively: the ratio in `TRACK_INFO` and the asked-for group in `SUBSCRIBE_UPDATE`. Earlier moq-lite versions carry neither, so a track received over one never refreshes. -- **Priority.** Higher is served first. The IETF default of 128 is this stack's 127, and a track that never sets one is 127. A relay's upstream subscription carries the highest priority its subscribers ask for and updates it as they come and go; its group order stays as the `SUBSCRIBE` asked, since no draft lets an update change it. +- **Priority.** Higher is served first. This stack's priorities run from -128 to 127 with 0 unset, and go out on the IETF wire, which ranks lower first, as `127 - priority`: an unset priority is IETF 127, and an absent IETF priority is 0. A relay's upstream subscription carries the highest priority its subscribers ask for and updates it as they come and go; its group order stays as the `SUBSCRIBE` asked, since no draft lets an update change it. - **Size.** An object extension block larger than 64 KiB ends that subgroup stream. The session stays up. This cap is ours, not the draft's. Several project drafts extend the IETF wire without breaking it, since `SETUP` diff --git a/doc/setup/upgrade.md b/doc/setup/upgrade.md index 1a0136a8e3f..49f2be6b650 100644 --- a/doc/setup/upgrade.md +++ b/doc/setup/upgrade.md @@ -32,6 +32,14 @@ ignored; its guide says what replaces it. These land with the next breaking release. +- **The default priority byte moves again.** Priorities are signed in every + API, -128 to 127 with 0 unset, so nobody has to know that 127 is the middle + of a byte. The wire keeps one byte and one urgency on both protocols, so an + unset priority now goes out as 128 on moq-lite (was 127) and 127 on + moq-transport (was 128); the previous release moved it to match moxygen. + hang's media priorities now sit above that default rather than below it. + Mixed versions only differ in how an unset priority ranks against a set one, + so upgrade publishers and players in either order. - **WebSocket needs a QMux subprotocol** (#6231). Relays answer a WebSocket upgrade that offers no `qmux-NN` subprotocol with 400 instead of falling back to the legacy `webtransport` format, so upgrade such clients before relays. diff --git a/drafts/draft-lcurley-moq-hang.md b/drafts/draft-lcurley-moq-hang.md index 5c0f33d14c8..63c038b28e1 100644 --- a/drafts/draft-lcurley-moq-hang.md +++ b/drafts/draft-lcurley-moq-hang.md @@ -992,7 +992,7 @@ The `store` field, if present, is the object-store URL the recording objects liv Authorization for `replay` and `store` is external. The `version` field is the version of that recording object format. -It is 2 for this specification. +It is 3 for this specification. A publisher that exposes a store MUST set it; a publisher that does not MUST omit it. A catalog that composes another broadcast's renditions MUST preserve that child's `archive` entry instead of synthesizing one. @@ -1189,6 +1189,7 @@ This document has no IANA actions. ## moq-hang-04 {:numbered="false"} +- The archive `version` is 3: the recording format stores a track's `priority` signed, -128 through 127 with 0 unset, instead of its moq-lite byte. - A reader refuses a JSON frame in which an object repeats a member name. - Added the root `version` ({{catalog-version}}): the oldest draft that reads the catalog, which a consumer refuses when above its own. A `track` other than the entry's ID or a video `warmup` needs 4. Consumers MUST ignore unknown fields. - A data track marks a discontinuity with a group of one object with no bytes, since its empty payload is data. An object with no bytes on an audio or video track, or sharing its group, is malformed. diff --git a/drafts/draft-lcurley-moq-lite.md b/drafts/draft-lcurley-moq-lite.md index 422beba5388..0c747eb7012 100644 --- a/drafts/draft-lcurley-moq-lite.md +++ b/drafts/draft-lcurley-moq-lite.md @@ -667,6 +667,7 @@ A publisher SHOULD order a [datagram](#datagrams) among the subscription's strea ### Priority The `Subscriber Priority` is scoped to the connection and MAY change over the life of the subscription via SUBSCRIBE_UPDATE. The `Publisher Priority` is fixed for the lifetime of the Track (see [TRACK_INFO](#track-info)) and SHOULD be used only to resolve conflicts or ties. +Both are a byte where higher is more urgent, and 128 is the midpoint an endpoint SHOULD use for a priority it was never given. A conflict can occur when a relay tries to serve multiple downstream subscriptions from a single upstream subscription. The relay cannot pick any one subscriber's priority, so the upstream subscription SHOULD use the publisher priority instead of some combination of different subscriber priorities. @@ -1217,7 +1218,7 @@ A unique identifier chosen by the subscriber. A Subscribe ID MUST NOT be reused within the same session, even if the prior subscription has been closed. **Subscriber Priority**: -The priority of the subscription within the session, represented as a u8. +The priority of the subscription within the session, represented as a u8 with 128 as the midpoint. The publisher SHOULD transmit *higher* values first during congestion. See the [Prioritization](#prioritization) section for more information. @@ -1339,7 +1340,7 @@ This is what lets the properties live on their own stream, fetched once and cach Publisher properties fan *out* at a relay (one upstream subscription serving many downstreams), so a change would have to propagate everywhere; subscriber properties fan *in*, which the relay already merges, so they MAY change freely via SUBSCRIBE_UPDATE. **Publisher Priority**: -The publisher's priority for this Track, represented as a u8, used only to resolve ties between subscriptions of equal subscriber priority. +The publisher's priority for this Track, represented as a u8 with 128 as the midpoint, used only to resolve ties between subscriptions of equal subscriber priority. See the [Prioritization](#prioritization) section for more information. **Publisher Max Age**: @@ -1767,6 +1768,7 @@ The `Message Length` describes the payload size on the wire. - Added Padding Stream (0x2), read and discarded incrementally by either endpoint. +- Named 128 the midpoint priority for an unset Subscriber or Publisher Priority. - Added the Tracks Stream (0x8) with TRACKS, TRACKS_ADD, TRACKS_REMOVE, and SYNCED: a subscriber lists one Broadcast's Tracks, each with its TRACK_INFO, then SUBSCRIBEs to those it wants. A relay answers at once with what it holds. Older versions reset the stream type. - Added TRACK_INFO `Refresh` and SUBSCRIBE_UPDATE `Wanted Group`, carrying ratio-based refresh demand through relays. diff --git a/go/ffi/MIGRATE.md b/go/ffi/MIGRATE.md index c0dd098ee1a..5319a4adb04 100644 --- a/go/ffi/MIGRATE.md +++ b/go/ffi/MIGRATE.md @@ -4,6 +4,13 @@ How to update your code for each breaking change, newest first. ## Unreleased +### Priorities are signed (#6142) + +`MoqTrackInfo.Priority`, `MoqSubscription.Priority`, `MoqFetchGroupOptions.Priority`, and `MoqGroupRequest.Priority()` are `int8`, higher first, and 0 is the unset midpoint. +`MoqTrackInfo.Priority` defaulted to 127 and now defaults to 0. +To keep a value's moq-lite byte, subtract 128 from it: 255 becomes 127, 128 becomes 0, and 0 becomes -128. +Regenerate the bindings for the changed checksums. + ### A client asks only about what it watches (#6165) On moq-lite, a client session asks its relay for announcements only under paths something watches, through `Announced()` or `AnnouncedBroadcast()`, instead of under its whole scope at connect. diff --git a/go/wrapper/MIGRATE.md b/go/wrapper/MIGRATE.md index fda653cd3a4..eae8355de43 100644 --- a/go/wrapper/MIGRATE.md +++ b/go/wrapper/MIGRATE.md @@ -4,6 +4,12 @@ How to update your code for each breaking change, newest first. ## Unreleased +### Priorities are signed (#6142) + +`TrackInfo.Priority`, `Subscription.Priority`, and `FetchGroupOptions.Priority` are `int8`, and `GroupRequest.Priority()` returns one. +Higher is first and the zero value is the unset midpoint, so a `TrackInfo` that leaves `Priority` zero now gets the default rather than the least urgent priority; drop any `Priority: 127` that asked for the midpoint. +To keep a value's moq-lite byte, subtract 128 from it: 255 becomes 127, 128 becomes 0, and 0 becomes -128. + ### A client asks only about what it watches (#6165) On moq-lite, a client session asks its relay for announcements only under paths something watches, through `Announced()` or `AnnouncedBroadcast()`, instead of under its whole scope at connect. diff --git a/go/wrapper/json/json_test.go b/go/wrapper/json/json_test.go index 16f57e22fb9..810040fa7c0 100644 --- a/go/wrapper/json/json_test.go +++ b/go/wrapper/json/json_test.go @@ -55,7 +55,7 @@ func TestTracks(t *testing.T) { broadcast, consumer := newBroadcast(t) scale := uint64(1_000_000) - track, err := broadcast.PublishTrack("status", &moq.TrackInfo{Priority: 127, Timescale: &scale}) + track, err := broadcast.PublishTrack("status", &moq.TrackInfo{Timescale: &scale}) if err != nil { t.Fatal(err) } diff --git a/go/wrapper/moq_test.go b/go/wrapper/moq_test.go index a45561b7e87..2c458e0bc6d 100644 --- a/go/wrapper/moq_test.go +++ b/go/wrapper/moq_test.go @@ -149,7 +149,7 @@ func TestBroadcastFinishLingersUntilCancelled(t *testing.T) { t.Fatal(err) } maxAge := time.Hour - track, err := broadcast.PublishTrack("status", &moq.TrackInfo{Priority: 127, MaxAge: &maxAge}) + track, err := broadcast.PublishTrack("status", &moq.TrackInfo{MaxAge: &maxAge}) if err != nil { t.Fatal(err) } @@ -182,7 +182,7 @@ func TestBroadcastFinishWaitOnDoneContextCloses(t *testing.T) { t.Fatal(err) } maxAge := time.Hour - track, err := broadcast.PublishTrack("status", &moq.TrackInfo{Priority: 127, MaxAge: &maxAge}) + track, err := broadcast.PublishTrack("status", &moq.TrackInfo{MaxAge: &maxAge}) if err != nil { t.Fatal(err) } @@ -1691,7 +1691,7 @@ func ts(d time.Duration) *time.Duration { return &d } // timedInfo declares the microsecond timeline used by timed raw fixtures. func timedInfo() *moq.TrackInfo { scale := uint64(1_000_000) - return &moq.TrackInfo{Priority: 127, Timescale: &scale} + return &moq.TrackInfo{Timescale: &scale} } // untilWanted blocks until some subscriber wants the track, or none does. diff --git a/go/wrapper/publish.go b/go/wrapper/publish.go index 16c70cdf307..3df32231205 100644 --- a/go/wrapper/publish.go +++ b/go/wrapper/publish.go @@ -385,7 +385,7 @@ func (r *GroupRequest) Sequence() uint64 { } // Priority is the consumer's delivery priority for this fetch. -func (r *GroupRequest) Priority() uint8 { +func (r *GroupRequest) Priority() int8 { return r.inner.Priority() } diff --git a/go/wrapper/records.go b/go/wrapper/records.go index 455fd68729a..8eaec884773 100644 --- a/go/wrapper/records.go +++ b/go/wrapper/records.go @@ -63,7 +63,7 @@ func datagramFromFFI(d *ffi.MoqDatagram) *Datagram { // still allows, while an empty non-nil slice asks for nothing), and the order the // publisher sends pending groups in (nil for newest first). type Subscription struct { - Priority uint8 + Priority int8 MaxDelay time.Duration Ranges []Range Order *Order @@ -95,11 +95,11 @@ func subscriptionFFI(s *Subscription) (*ffi.MoqSubscription, error) { // non-latest group is cached (nil for no limit), how many times its first frame // a group's later bytes may grow before a joining subscriber asks for a new group // (nil never asks), and the timescale in ticks per second (nil means untimed on -// both publish and receive). A zero Priority is the least urgent; set 127 for the +// both publish and receive). Priority is higher first; the zero Priority is the // midpoint a nil TrackInfo uses. Compress DEFLATEs each group on the wire to peers // that support it; readers always see the original frames. type TrackInfo struct { - Priority uint8 + Priority int8 MaxAge *time.Duration Refresh *uint32 Timescale *uint64 diff --git a/js/hang/src/catalog/archive.ts b/js/hang/src/catalog/archive.ts index 7a57a968d53..ea80d0a408b 100644 --- a/js/hang/src/catalog/archive.ts +++ b/js/hang/src/catalog/archive.ts @@ -11,7 +11,7 @@ const timescaleSchema = nonzeroU53Schema.check(z.lte(4_294_967_295)); /** * The recording object format advertised in {@link Archive.version} when a store is present. */ -export const ARCHIVE_VERSION = 2; +export const ARCHIVE_VERSION = 3; /** * Discovers each track's timeline and any durable archive. diff --git a/js/hang/src/catalog/priority.ts b/js/hang/src/catalog/priority.ts index b3fead65d8d..50534d47b34 100644 --- a/js/hang/src/catalog/priority.ts +++ b/js/hang/src/catalog/priority.ts @@ -1,5 +1,8 @@ // Defined in one place so the relative ordering stays consistent. -/** Delivery priority per track kind; higher is sent first. */ +/** + * Delivery priority per track kind, in `-128..=127` with higher sent first. All sit above 0, the + * unset default, so hang media outranks a track that never set a priority. + */ export const PRIORITY = { catalog: 100, text: 90, diff --git a/js/hang/src/catalog/root.test.ts b/js/hang/src/catalog/root.test.ts index 0d906ab80f0..1a20f9676e8 100644 --- a/js/hang/src/catalog/root.test.ts +++ b/js/hang/src/catalog/root.test.ts @@ -192,7 +192,7 @@ test("archive round-trips at the root", () => { timescale: 1000, replay: "recordings/clip", store: "https://objects.example/rec/", - version: 2, + version: ARCHIVE_VERSION, }); expect(JSON.stringify(parsed)).not.toContain('"timeline":'); }); diff --git a/js/net/MIGRATE.md b/js/net/MIGRATE.md index 4ecb1d19071..f5cbe4360c1 100644 --- a/js/net/MIGRATE.md +++ b/js/net/MIGRATE.md @@ -4,6 +4,13 @@ How to update your code for each breaking change, newest first. ## Unreleased +### Priorities are signed (#6142) + +`Track.Info.priority`, `Track.Subscription.priority`, and a fetch's `priority` take `-128..=127`, higher first, and 0 is the unset midpoint; anything outside throws a `RangeError`. +The track's default was 127, the midpoint, and the subscription's was 0, the least urgent; both are 0 now. +To keep a value's moq-lite byte, subtract 128 from it: 255 becomes 127, 128 becomes 0, and 0 becomes -128. +The wire messages (`Lite.Subscribe`, `Lite.TrackInfo`, `Ietf.Subscribe`, ...) hold the same signed priority, mapped to the wire byte when encoded. + ### A dropped or cut group ends its reader with an error (#6143) A group the publisher cut, or named in `SUBSCRIBE_DROP`, after a `Track.Subscriber` saw it is now handed out, and its reads reject with the publisher's `StreamError`; before, it was skipped or ended cleanly. diff --git a/js/net/src/ietf/object.ts b/js/net/src/ietf/object.ts index ae61a0d96df..e7b068b795f 100644 --- a/js/net/src/ietf/object.ts +++ b/js/net/src/ietf/object.ts @@ -2,6 +2,7 @@ import { ProtocolViolation, StreamCode, Stream as StreamError } from "../error.t import { asIetf, type Cursor, type Reader, Writer } from "../stream.ts"; import { type Timescale, Timestamp } from "../time.ts"; import { U64 } from "../util/u64.ts"; +import { ABSENT_WIRE } from "./priority.ts"; import { type IetfVersion, Version } from "./version.ts"; // Implementation limit for object extension blocks, independent of the IETF draft. @@ -244,7 +245,7 @@ export class Group { const trackAlias = await r.u62(); const groupId = await r.u53(); const subGroupId = flags.hasSubgroup ? await r.u53() : 0; - const publisherPriority = hasPriority ? await r.u8() : 128; // Default priority when absent + const publisherPriority = hasPriority ? await r.u8() : ABSENT_WIRE; return new Group({ trackAlias, groupId, subGroupId, publisherPriority, flags }); } diff --git a/js/net/src/ietf/priority.test.ts b/js/net/src/ietf/priority.test.ts index b04da83354e..04ff32b0787 100644 --- a/js/net/src/ietf/priority.test.ts +++ b/js/net/src/ietf/priority.test.ts @@ -1,24 +1,33 @@ import { expect, test } from "bun:test"; +import * as Lite from "../lite/priority.ts"; import { infoDefaults } from "../track.ts"; -import { fromWire, toWire } from "./priority.ts"; +import { ABSENT_WIRE, fromWire, toWire } from "./priority.ts"; -test("IETF subscriber priority is lower first", () => { - expect(fromWire(0)).toBe(0xff); - expect(fromWire(0xff)).toBe(0); - expect(toWire(0xff)).toBe(0); - expect(toWire(0)).toBe(0xff); +test("IETF priority is lower first", () => { + expect(fromWire(0)).toBe(127); + expect(fromWire(0xff)).toBe(-128); + expect(toWire(127)).toBe(0); + expect(toWire(-128)).toBe(0xff); for (let priority = 0; priority < 0xff; priority++) { expect(fromWire(priority)).toBeGreaterThan(fromWire(priority + 1)); } }); -test("subscriber priority round trips", () => { +test("priority round trips", () => { for (let priority = 0; priority <= 0xff; priority++) { - expect(fromWire(toWire(priority))).toBe(priority); + expect(toWire(fromWire(priority))).toBe(priority); } }); -test("an unset track priority is the draft's usual publisher priority", () => { - expect(toWire(infoDefaults().priority)).toBe(128); +// Both wires carry one urgency: the IETF byte is the moq-lite byte flipped. +test("IETF mirrors moq-lite", () => { + for (let priority = -128; priority <= 127; priority++) { + expect(toWire(priority)).toBe(0xff - Lite.toWire(priority)); + } +}); + +test("an unset track priority goes out as the absent byte", () => { + expect(toWire(infoDefaults().priority)).toBe(127); + expect(fromWire(ABSENT_WIRE)).toBe(0); }); diff --git a/js/net/src/ietf/priority.ts b/js/net/src/ietf/priority.ts index 43c4732a413..991475e0ed3 100644 --- a/js/net/src/ietf/priority.ts +++ b/js/net/src/ietf/priority.ts @@ -1,9 +1,22 @@ -/** Convert an IETF subscriber priority into the model's higher-first priority. */ +/** + * Priority conversion between the moq-transport wire and the model, mirroring the Rust + * `ietf::priority`. + * + * moq-transport ranks lower first, so the wire carries `127 - priority`: the unset priority 0 + * goes out as 127, and an absent priority decodes as 0. + * + * @module + */ + +/** The wire byte for the unset priority 0, which an absent priority stands for. */ +export const ABSENT_WIRE = 127; + +/** Convert an IETF priority byte into the model's higher-first priority. */ export function fromWire(priority: number): number { - return 0xff - priority; + return 127 - priority; } -/** Convert the model's higher-first priority into an IETF subscriber priority. */ +/** Convert the model's higher-first priority into an IETF priority byte. */ export function toWire(priority: number): number { - return 0xff - priority; + return 127 - priority; } diff --git a/js/net/src/ietf/properties.ts b/js/net/src/ietf/properties.ts index 35829832178..2f7d91fa61a 100644 --- a/js/net/src/ietf/properties.ts +++ b/js/net/src/ietf/properties.ts @@ -30,7 +30,7 @@ export interface Properties { /// `undefined` declares no timeline, so the track and its objects are untimed. timescale?: Timescale; - /// Publisher priority for a group header without its priority flag. The wire default is 128. + /// Publisher priority for a group header without its priority flag. The wire default is 127, the unset priority 0. priority?: number; /// The publisher's preference for prioritizing groups within a subscription, diff --git a/js/net/src/ietf/publisher.test.ts b/js/net/src/ietf/publisher.test.ts index 9a020f169b7..88d71ecee19 100644 --- a/js/net/src/ietf/publisher.test.ts +++ b/js/net/src/ietf/publisher.test.ts @@ -2513,7 +2513,7 @@ test("drafts 14 to 16 apply an update and still end a subscription on UNSUBSCRIB expect(await peer.reader.u53()).toBe(RequestOk.id); expect((await RequestOk.decode(peer.reader, version)).requestId).toBe(2n); } - while (track.subscription.peek()?.priority !== 245) await track.subscription.changed(); + while (track.subscription.peek()?.priority !== 117) await track.subscription.changed(); await peer.writer.u53(Unsubscribe.id); await new Unsubscribe({ requestId: 0n }).encode(peer.writer, version); @@ -3786,14 +3786,14 @@ test("REQUEST_UPDATE applies priority and preserves it when omitted", async () = await client.writer.write(new Uint8Array([0x02, 0, 4, 2, 1, 0x20, 10])); expect(await client.reader.u53()).toBe(RequestOk.id); await RequestOk.decode(client.reader, fx.version); - expect(track.subscription.peek()?.priority).toBe(245); + expect(track.subscription.peek()?.priority).toBe(117); // Only the priority changes; retention and the group range survive. expect(track.subscription.peek()?.maxDelay).toBe(before?.maxDelay); expect(track.subscription.peek()?.groups).toEqual(before?.groups); await client.writer.write(new Uint8Array([0x02, 0, 2, 4, 0])); expect(await client.reader.u53()).toBe(RequestOk.id); await RequestOk.decode(client.reader, fx.version); - expect(track.subscription.peek()?.priority).toBe(245); + expect(track.subscription.peek()?.priority).toBe(117); } finally { client.close(); track.close(); diff --git a/js/net/src/ietf/publisher.ts b/js/net/src/ietf/publisher.ts index 3047bb5d1db..0e3f9ed97a1 100644 --- a/js/net/src/ietf/publisher.ts +++ b/js/net/src/ietf/publisher.ts @@ -723,7 +723,7 @@ export class Publisher { const groups = new Set>(); const streams: StreamCount = { opened: 0 }; // One ranking for the whole subscription, shared by every group and the fill. - const ranked = new Priority(track); + const ranked = new Priority(track, info.priority); ranking = ranked; // Serve groups until the track ends or the requester cancels. diff --git a/js/net/src/ietf/subscribe.ts b/js/net/src/ietf/subscribe.ts index d00a83d75f5..74e1be225b6 100644 --- a/js/net/src/ietf/subscribe.ts +++ b/js/net/src/ietf/subscribe.ts @@ -7,6 +7,7 @@ import * as Filter from "./filter.ts"; import * as Message from "./message.ts"; import * as Namespace from "./namespace.ts"; import { type MessageLocation, MSG_PARAM_FORWARD, MSG_PARAM_SUBSCRIBER_PRIORITY, Parameters } from "./parameters.ts"; +import { ABSENT_WIRE } from "./priority.ts"; import * as Properties from "./properties.ts"; import { type IetfVersion, Version } from "./version.ts"; @@ -220,7 +221,7 @@ export class Subscribe { } // v15+: fields are in parameters const params = await Parameters.decode(r, version, "subscribe"); - const subscriberPriority = params.subscriberPriority ?? 128; + const subscriberPriority = params.subscriberPriority ?? ABSENT_WIRE; const groupOrder = decodeGroupOrder(params.groupOrder ?? GroupOrder.Descending); // FILL_PARAMETERS and INCLUDE_PROPERTIES are draft-20 additions, and the Range @@ -459,7 +460,7 @@ export class SubscribeUpdate { await w.u62(0n); // start_group await w.u62(0n); // start_object await w.u62(0n); // end_group - await w.u8(this.parameters.subscriberPriority ?? 128); + await w.u8(this.parameters.subscriberPriority ?? ABSENT_WIRE); await w.bool(this.parameters.forward ?? true); await this.parameters.encode(w, version, [MSG_PARAM_SUBSCRIBER_PRIORITY, MSG_PARAM_FORWARD]); } else { diff --git a/js/net/src/ietf/subscriber.test.ts b/js/net/src/ietf/subscriber.test.ts index e9441b33b24..2c29c7712e2 100644 --- a/js/net/src/ietf/subscriber.test.ts +++ b/js/net/src/ietf/subscriber.test.ts @@ -1226,9 +1226,9 @@ test("a compact END_OF_TRACK at object 5 with properties is subgroup 5, refused } }); -test("older peer without priority property inherits wire priority 128", async () => { +test("older peer without priority property inherits the unset priority", async () => { const { subscriber, track } = await subscribeTrack(); - expect((await track.info()).priority).toBe(0xff - 128); + expect((await track.info()).priority).toBe(0); const group = new GroupMessage({ trackAlias: ALIAS, groupId: 3, @@ -1237,7 +1237,7 @@ test("older peer without priority property inherits wire priority 128", async () flags: { ...groupFlags(true), hasPriority: false }, }); await subscriber.handleGroup(group, new Reader(undefined, encodeObjects([0]), VERSION)); - expect(group.publisherPriority).toBe(128); + expect(group.publisherPriority).toBe(127); track.close(); }); @@ -1257,7 +1257,7 @@ test("an info-only lookup waits for SUBSCRIBE_OK instead of abandoning", async ( trackAlias: ALIAS, properties: { priority: 37 }, }).encode(peer.writer, VERSION); - expect((await info).priority).toBe(0xff - 37); + expect((await info).priority).toBe(127 - 37); }); test("SUBSCRIBE_OK's largest is known before any group arrives", async () => { @@ -1351,7 +1351,7 @@ test.each(["acceptance", "timeout"])("returning demand preserves pending setup u if (outcome === "acceptance") { await peer.writer.u53(SubscribeOk.id); await new SubscribeOk({ requestId: request.requestId, trackAlias: ALIAS }).encode(peer.writer, VERSION); - expect(await info).toMatchObject({ priority: 127 }); + expect(await info).toMatchObject({ priority: 0 }); expect(returned.closed.peek()).toBeUndefined(); } else { jest.advanceTimersByTime(4_000); @@ -1418,7 +1418,7 @@ test("setup abandonment closes before a queued viewer can attach", async () => { const request = await Subscribe.decode(next.reader, VERSION); await next.writer.u53(SubscribeOk.id); await new SubscribeOk({ requestId: request.requestId, trackAlias: ALIAS }).encode(next.writer, VERSION); - expect(await info).toMatchObject({ priority: 127 }); + expect(await info).toMatchObject({ priority: 0 }); }); test("abandonment cancels once and a late acceptance cannot capture a reused alias", async () => { @@ -1524,7 +1524,7 @@ test("early group waits for SUBSCRIBE_OK priority before track acceptance", asyn properties: { priority: 37 }, }).encode(peer.writer, VERSION); await arriving; - expect((await track.info()).priority).toBe(0xff - 37); + expect((await track.info()).priority).toBe(127 - 37); expect(group.publisherPriority).toBe(37); const ordered = track.ordered(); expect((await ordered.nextGroup())?.sequence).toBe(3); @@ -1537,7 +1537,7 @@ test("early group waits for SUBSCRIBE_OK priority before track acceptance", asyn }); await subscriber.handleGroup(explicit, new Reader(undefined, encodeObjects([0]), VERSION)); expect(explicit.publisherPriority).toBe(9); - expect((await track.info()).priority).toBe(0xff - 37); + expect((await track.info()).priority).toBe(127 - 37); expect((await ordered.nextGroup())?.sequence).toBe(4); ordered.close(); track.close(); @@ -2365,7 +2365,7 @@ for (const negotiated of [false, true]) { expect(await peer.reader.u53()).toBe(SubscribeUpdate.id); const update = await SubscribeUpdate.decode(peer.reader, VERSION); expect(update.parameters.maxAge).toBe(negotiated ? 900 : undefined); - expect(update.parameters.subscriberPriority).toBe(246); + expect(update.parameters.subscriberPriority).toBe(127 - 9); tolerant.close(); track.close(); peer.close(); @@ -2412,7 +2412,7 @@ test("a draft 16 subscription update waits for the last one's answer", async () await new RequestOk({ requestId: first.ownRequestId }).encode(peer.writer, version); expect(await next).toBe(SubscribeUpdate.id); const second = await SubscribeUpdate.decode(peer.reader, version); - expect(second.parameters.subscriberPriority).toBe(235); + expect(second.parameters.subscriberPriority).toBe(127 - 20); higher.close(); highest.close(); } finally { diff --git a/js/net/src/ietf/subscriber.ts b/js/net/src/ietf/subscriber.ts index 6f2d304f743..614b3125702 100644 --- a/js/net/src/ietf/subscriber.ts +++ b/js/net/src/ietf/subscriber.ts @@ -55,7 +55,7 @@ import { ObjectIdGap, } from "./object.ts"; import { Parameters } from "./parameters.ts"; -import { fromWire, toWire } from "./priority.ts"; +import { ABSENT_WIRE, fromWire, toWire } from "./priority.ts"; import { type Publish, PublishDone, PublishDoneStatus, publishDoneClean } from "./publish.ts"; import { type AdvertParams, @@ -1217,7 +1217,7 @@ export class Subscriber { // No TIMESCALE (always so on drafts 14-16, which can't carry it) means no timeline, // and the track must not claim one when served onward. timescale: ok.properties.timescale, - priority: fromWire(ok.properties.priority ?? 128), + priority: fromWire(ok.properties.priority ?? ABSENT_WIRE), maxAge: maxCacheDuration === undefined ? undefined : Milli(Number(maxCacheDuration)), // A moq-transport peer may stamp arrival times, and nothing on the wire says it didn't. unchecked: ok.properties.timescale != null, diff --git a/js/net/src/lite/fetch.ts b/js/net/src/lite/fetch.ts index 823f85a6268..d433ddf134a 100644 --- a/js/net/src/lite/fetch.ts +++ b/js/net/src/lite/fetch.ts @@ -4,6 +4,7 @@ import * as Path from "../path.ts"; import type { Reader, Writer } from "../stream.ts"; import { decodeEpoch, encodeEpoch } from "./epoch.ts"; import * as Message from "./message.ts"; +import { fromWire, toWire } from "./priority.ts"; import { hasFetchMaxDelay, hasFrameBounds, Version } from "./version.ts"; function guardFetch(version: Version) { @@ -68,7 +69,7 @@ export class Fetch { await w.string(Path.encode(this.broadcast)); await encodeEpoch(w, version, this.epoch); await w.string(this.track); - await w.u8(this.priority); + await w.u8(toWire(this.priority)); if (hasFetchMaxDelay(version)) await w.u53(this.maxDelay === undefined ? 0 : this.maxDelay + 1); else if (this.maxDelay !== undefined) throw new Error("fetch maxDelay not supported for this version"); await w.u53(this.group); @@ -86,7 +87,7 @@ export class Fetch { const broadcast = Path.decode(await r.string()); const epoch = await decodeEpoch(r, version); const track = await r.string(); - const priority = await r.u8(); + const priority = fromWire(await r.u8()); const encodedMaxDelay = hasFetchMaxDelay(version) ? await r.u53() : 0; const maxDelay = encodedMaxDelay > 0 ? ((encodedMaxDelay - 1) as Milli) : undefined; const group = await r.u53(); diff --git a/js/net/src/lite/priority.ts b/js/net/src/lite/priority.ts new file mode 100644 index 00000000000..2de185440d0 --- /dev/null +++ b/js/net/src/lite/priority.ts @@ -0,0 +1,19 @@ +/** + * Priority conversion between the moq-lite wire and the model, mirroring the Rust + * `lite::priority`. + * + * The wire carries `priority + 128`: one-to-one and order preserving, so a byte means what it + * always meant and the unset priority 0 is byte 128. + * + * @module + */ + +/** Convert a moq-lite priority byte into the model's priority. */ +export function fromWire(byte: number): number { + return byte - 128; +} + +/** Convert the model's priority into a moq-lite priority byte. */ +export function toWire(priority: number): number { + return priority + 128; +} diff --git a/js/net/src/lite/publisher.test.ts b/js/net/src/lite/publisher.test.ts index c7bb4b288a0..e7d620bff4f 100644 --- a/js/net/src/lite/publisher.test.ts +++ b/js/net/src/lite/publisher.test.ts @@ -247,18 +247,24 @@ async function servingNextGroup(opened: ReadableStreamDefaultReader new GroupProducer(sequence)); const pair = createMockTransportPair(ALPN_05); const origin = new OriginProducer(); - const publisher = new Publisher(pair.server, Version.DRAFT_05, randomHop(), origin.consume()); + const publisher = new Publisher(pair.server, version, randomHop(), origin.consume()); const broadcast = publish(origin, Path.from("test")); - const track = broadcast.createTrack("video", { timescale: Timescale.MILLI }); + const track = broadcast.createTrack("video", { timescale: Timescale.MILLI, priority: options.publisher }); - const client = await Stream.open(pair.client, { version: Version.DRAFT_05 }); - const server = await Stream.accept(pair.server, Version.DRAFT_05); + const client = await Stream.open(pair.client, { version }); + const server = await Stream.accept(pair.server, version); if (!server) throw new Error("publisher never accepted the subscribe stream"); const msg = replaySubscribe({ @@ -275,7 +281,7 @@ async function groupSendOrders(options: { priority: number; sequences: number[]; try { for (const [index, group] of groups.entries()) { if (index === 1 && update !== undefined) { - await replayUpdate({ priority: update }).encode(client.writer, Version.DRAFT_05); + await replayUpdate({ priority: update }).encode(client.writer, version); // Wait for the publisher to apply it, rather than assuming it beat the next group. while (track.subscription.peek()?.priority !== update) await track.subscription.changed(); } @@ -324,6 +330,13 @@ test("lite draft-05: equal priorities tie regardless of group numbering", async expect(fresh).toEqual(ongoing); }); +// The track's publisher priority breaks ties on every draft, as in Rust, including the drafts +// before lite-05 that acknowledge with SUBSCRIBE_OK instead of TRACK_INFO. +test.each([Version.DRAFT_03, Version.DRAFT_05])("lite %s: the publisher priority ranks each group", async (version) => { + const orders = await groupSendOrders({ priority: 7, publisher: 5, sequences: [0], version }); + expect(orders).toEqual([sendOrder({ priority: 7, publisher: 5 })]); +}); + // SUBSCRIBE_UPDATE re-ranks the subscription, so every group it is serving picks up the new // priority, including one opened after the update landed. test("lite draft-05: a subscribe update re-ranks the whole subscription", async () => { diff --git a/js/net/src/lite/publisher.ts b/js/net/src/lite/publisher.ts index 63015bf351a..7668218dcf4 100644 --- a/js/net/src/lite/publisher.ts +++ b/js/net/src/lite/publisher.ts @@ -891,6 +891,12 @@ export class Publisher { const served: Run[] | undefined = hasTrackStream(this.version) ? [] : undefined; try { + // Every draft waits for the track's info, as Rust does: it surfaces a rejected track + // (accept never called, track closed) as an error here, which resets the stream, and + // its priority breaks ties between this session's subscriptions at one subscriber + // priority. + const info = await untilAborted(track.info(), left.signal); + const publisher = info.priority; // Older drafts carry no timestamps. let stamp: Stamp | undefined; let compress = false; @@ -902,10 +908,7 @@ export class Publisher { // // The timescale is an immutable property, so serving MUST use exactly // what TRACK_INFO advertised. It comes from the producer's accept(), so - // they always agree. Awaiting info() also surfaces a rejected track - // (accept never called, track closed) as an error here, which resets the - // stream. - const info = await untilAborted(track.info(), left.signal); + // they always agree. stamp = stamping(this.version, liteTimescale(info)); compress = hasCompression(this.version) && !!info.compress; } else { @@ -954,6 +957,7 @@ export class Publisher { sub: msg.id, broadcast: msg.broadcast, stamp, + publisher, compress, served, bounds: { @@ -1124,12 +1128,14 @@ export class Publisher { sub: bigint; broadcast: Path.Valid; stamp: Stamp | undefined; + /** The track's publisher priority, breaking ties between subscriptions. */ + publisher: number; compress: boolean; served: Run[] | undefined; bounds: FrameBounds; }, ): Promise { - const { sub, broadcast, stamp, compress, served, bounds } = serving; + const { sub, broadcast, stamp, publisher, compress, served, bounds } = serving; // Lite-05+ resolves the range on the subscribe stream: SUBSCRIBE_START once the // first group is known, SUBSCRIBE_END when the track finishes. const emitRange = hasTrackStream(this.version); @@ -1178,7 +1184,7 @@ export class Publisher { }; // One ranking for the whole subscription, shared by every group it serves. - const priority = new Priority(track); + const priority = new Priority(track, publisher); // Every group this subscription started serving, until its stream finishes or resets. const groups = new Set>(); diff --git a/js/net/src/lite/subscribe.test.ts b/js/net/src/lite/subscribe.test.ts index 5f029bcc6c9..b9010a0b1d3 100644 --- a/js/net/src/lite/subscribe.test.ts +++ b/js/net/src/lite/subscribe.test.ts @@ -276,8 +276,8 @@ test.each(["ascending", "descending"] as const)("lite-07 round-trips a %s group expect(got.order).toBe(order); const update = await encodeMessage(version, new SubscribeUpdate({ priority: 3, order, maxDelay: 250 })); - // The length prefix, the priority, then the order. - expect(update[1]).toBe(3); + // The length prefix, the priority byte, then the order. + expect(update[1]).toBe(3 + 128); expect(update[2]).toBe(order === "ascending" ? 1 : 0); const decoded = await SubscribeUpdate.decode(new Reader(undefined, update, version), version); expect(decoded.order).toBe(order); diff --git a/js/net/src/lite/subscribe.ts b/js/net/src/lite/subscribe.ts index 7651c5e62f7..3f5bf3c17b6 100644 --- a/js/net/src/lite/subscribe.ts +++ b/js/net/src/lite/subscribe.ts @@ -5,6 +5,7 @@ import type { Reader, Writer } from "../stream.ts"; import type { Location, Order } from "../track.ts"; import { decodeEpoch, encodeEpoch } from "./epoch.ts"; import * as Message from "./message.ts"; +import { fromWire, toWire } from "./priority.ts"; import { hasDropCode, hasFrameBounds, @@ -230,10 +231,10 @@ export class SubscribeUpdate { switch (version) { case Version.DRAFT_01: case Version.DRAFT_02: - await w.u8(this.priority); + await w.u8(toWire(this.priority)); break; default: - await w.u8(this.priority); + await w.u8(toWire(this.priority)); await encodeOrder(w, version, this.order); await w.u53(this.maxDelay); await encodeStartGroup(w, version, this.startGroup); @@ -249,9 +250,9 @@ export class SubscribeUpdate { switch (version) { case Version.DRAFT_01: case Version.DRAFT_02: - return new SubscribeUpdate({ priority: await r.u8() }); + return new SubscribeUpdate({ priority: fromWire(await r.u8()) }); default: { - const priority = await r.u8(); + const priority = fromWire(await r.u8()); const order = await decodeOrder(r, version); const maxDelay = await r.u53(); const startGroup = await decodeStartGroup(r, version); @@ -351,7 +352,7 @@ export class Subscribe { await w.string(Path.encode(this.broadcast)); await encodeEpoch(w, version, this.epoch); await w.string(this.track); - await w.u8(this.priority); + await w.u8(toWire(this.priority)); switch (version) { case Version.DRAFT_01: @@ -372,7 +373,7 @@ export class Subscribe { const broadcast = Path.decode(await r.string()); const epoch = await decodeEpoch(r, version); const track = await r.string(); - const priority = await r.u8(); + const priority = fromWire(await r.u8()); switch (version) { case Version.DRAFT_01: @@ -446,12 +447,12 @@ export class SubscribeOk { // noop break; case Version.DRAFT_01: - await w.u8(this.priority ?? 0); + await w.u8(toWire(this.priority)); break; // Draft-05+ never sends SUBSCRIBE_OK, but keep the field layout matching // Draft-03/04 so a stray future use stays well-formed. default: - await w.u8(this.priority); + await w.u8(toWire(this.priority)); await padGroupOrder(w, version); await w.u53(this.maxDelay); await w.u53(this.startGroup !== undefined ? this.startGroup + 1 : 0); @@ -471,10 +472,10 @@ export class SubscribeOk { // noop break; case Version.DRAFT_01: - priority = await r.u8(); + priority = fromWire(await r.u8()); break; default: - priority = await r.u8(); + priority = fromWire(await r.u8()); await skipGroupOrder(r, version); maxDelay = await r.u53(); startGroup = await r.u53(); diff --git a/js/net/src/lite/track.test.ts b/js/net/src/lite/track.test.ts index 9663d6c5360..62d14358343 100644 --- a/js/net/src/lite/track.test.ts +++ b/js/net/src/lite/track.test.ts @@ -53,7 +53,7 @@ test("TrackInfo for Rust's default Info matches cross-language wire bytes", asyn const model = infoDefaults(); const info = new TrackInfo({ ...model, timescale: model.timescale ?? undefined }); expect(await bytes((w) => info.encode(w, Version.DRAFT_05), Version.DRAFT_05)).toEqual( - new Uint8Array([0x0c, 0x7f, 0x00, 0xc0, 0x1f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x43, 0xe8]), + new Uint8Array([0x0c, 0x80, 0x00, 0xc0, 0x1f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x43, 0xe8]), ); }); @@ -111,7 +111,7 @@ test("TRACK_INFO is rejected before draft-05", async () => { test("TrackInfo round-trips valid boundary metadata", async () => { const info = new TrackInfo({ - priority: 255, + priority: 127, maxAge: Number.MAX_SAFE_INTEGER - 1, timescale: Number.MAX_SAFE_INTEGER, }); @@ -121,15 +121,15 @@ test("TrackInfo round-trips valid boundary metadata", async () => { Version.DRAFT_05, ); const got = await TrackInfo.decode(reader, Version.DRAFT_05); - expect(got.priority).toBe(255); + expect(got.priority).toBe(127); expect(got.maxAge).toBe(Number.MAX_SAFE_INTEGER - 1); expect(got.timescale).toBe(Timescale(Number.MAX_SAFE_INTEGER)); }); test("TrackInfo refuses invalid fields at construction", () => { for (const fields of [ - { priority: 256, timescale: 1000 }, - { priority: -1, timescale: 1000 }, + { priority: 128, timescale: 1000 }, + { priority: -129, timescale: 1000 }, { priority: 1.5, timescale: 1000 }, { timescale: 0 }, { timescale: -1 }, diff --git a/js/net/src/lite/track.ts b/js/net/src/lite/track.ts index 177cd337136..901f930c6ba 100644 --- a/js/net/src/lite/track.ts +++ b/js/net/src/lite/track.ts @@ -5,6 +5,7 @@ import type { Reader, Writer } from "../stream.ts"; import { Timescale } from "../time.ts"; import { decodeEpoch, encodeEpoch } from "./epoch.ts"; import * as Message from "./message.ts"; +import { fromWire, toWire } from "./priority.ts"; import { hasCompression, hasGroupOrder, hasRefresh, hasTrackStream, Version, wireTimescale } from "./version.ts"; // TRACK_INFO `Compression` codec ids: none, or per-group DEFLATE (draft-lcurley-moq-flate). @@ -107,8 +108,8 @@ export class TrackInfo { refresh?: number; compress?: boolean; }) { - if (!Number.isInteger(priority) || priority < 0 || priority > 255) { - throw new RangeError(`priority must be an integer in 0..=255: ${priority}`); + if (!Number.isInteger(priority) || priority < -128 || priority > 127) { + throw new RangeError(`priority must be an integer in -128..=127: ${priority}`); } if (maxAge !== undefined && (!Number.isSafeInteger(maxAge) || maxAge < 0)) { throw new RangeError(`maxAge must be a safe non-negative integer: ${maxAge}`); @@ -123,7 +124,7 @@ export class TrackInfo { } async #encode(w: Writer, version: Version) { - await w.u8(this.priority); + await w.u8(toWire(this.priority)); // The retired `Ordered` byte: lite-05 keeps it in its layout, written as 0. if (hasGroupOrder(version)) await w.bool(false); const legacy = version === Version.DRAFT_05 || version === Version.DRAFT_06; @@ -135,7 +136,7 @@ export class TrackInfo { } static async #decode(r: Reader, version: Version): Promise { - const priority = await r.u8(); + const priority = fromWire(await r.u8()); if (hasGroupOrder(version)) await r.bool(); const encoded = await r.u62(); const legacy = version === Version.DRAFT_05 || version === Version.DRAFT_06; @@ -167,8 +168,8 @@ export class TrackInfo { guardTrack(version); // Re-check after construction: fields are public, and a mutated value must not // reach the length-prefixed writer. - if (!Number.isInteger(this.priority) || this.priority < 0 || this.priority > 255) { - throw new RangeError(`priority must be an integer in 0..=255: ${this.priority}`); + if (!Number.isInteger(this.priority) || this.priority < -128 || this.priority > 127) { + throw new RangeError(`priority must be an integer in -128..=127: ${this.priority}`); } if (this.maxAge !== undefined && (!Number.isSafeInteger(this.maxAge) || this.maxAge < 0)) { throw new RangeError(`maxAge must be a safe non-negative integer: ${this.maxAge}`); diff --git a/js/net/src/priority.test.ts b/js/net/src/priority.test.ts index b71563bb9d1..28f472e2aef 100644 --- a/js/net/src/priority.test.ts +++ b/js/net/src/priority.test.ts @@ -6,16 +6,28 @@ import { type SendStream, Writer } from "./stream.ts"; import { Timescale } from "./time.ts"; import type { Subscription } from "./track.ts"; -// The last position that still fits below the track priority. -const MAX_POSITION = 2 ** 44 - 1; +// The last position that still fits below the track priorities. +const MAX_POSITION = 2 ** 36 - 1; // The transport sends higher values first, so a track the subscriber cares about more has to // outrank one it cares about less, wherever their groups sit in their own queues. test("track priority dominates the position", () => { expect(sendOrder({ priority: 2, position: 1_000_000 })).toBeGreaterThan(sendOrder({ priority: 1, position: 0 })); - expect(sendOrder({ priority: 255, position: MAX_POSITION })).toBeGreaterThan( - sendOrder({ priority: 254, position: 0 }), + expect(sendOrder({ priority: 127, position: MAX_POSITION })).toBeGreaterThan( + sendOrder({ priority: 126, position: 0 }), ); + expect(sendOrder({ priority: 0, position: 0 })).toBeGreaterThan(sendOrder({ priority: -1, position: 0 })); +}); + +// The publisher's priority breaks a tie at one subscriber priority, as in the Rust +// `Priority::cmp`, but never outranks a higher subscriber priority. +test("the publisher priority breaks subscriber ties", () => { + expect(sendOrder({ priority: 0, publisher: 80 })).toBeGreaterThan(sendOrder({ priority: 0, publisher: 60 })); + expect(sendOrder({ priority: 0, publisher: 1, position: MAX_POSITION })).toBeGreaterThan( + sendOrder({ priority: 0, publisher: 0, position: 0 }), + ); + expect(sendOrder({ priority: 1, publisher: -128 })).toBeGreaterThan(sendOrder({ priority: 0, publisher: 127 })); + expect(sendOrder({ priority: 0 })).toBe(sendOrder({ priority: 0, publisher: 0 })); }); // Position 0 is the group its subscription wants sent next, so it goes first. @@ -33,15 +45,15 @@ test("equal priorities tie at the same position", () => { // Every send order stays a safe integer, so two distinct ranks never collapse into one. test("send orders stay safe integers", () => { - expect(sendOrder({ priority: 255, position: 0 })).toBe(-1); - expect(sendOrder({ priority: 0, position: MAX_POSITION })).toBe(-(2 ** 52)); - expect(Number.isSafeInteger(sendOrder({ priority: 0, position: MAX_POSITION }))).toBe(true); + expect(sendOrder({ priority: 127, publisher: 127, position: 0 })).toBe(-1); + expect(sendOrder({ priority: -128, publisher: -128, position: MAX_POSITION })).toBe(-(2 ** 52)); + expect(Number.isSafeInteger(sendOrder({ priority: -128, publisher: -128, position: MAX_POSITION }))).toBe(true); }); // Protocol streams use the transport's default order of 0. Keeping every group below it // prevents a continuous supply of media from starving subscriptions, announcements, or probes. test("protocol streams outrank group data", () => { - expect(sendOrder({ priority: 255, position: 0 })).toBeLessThan(0); + expect(sendOrder({ priority: 127, publisher: 127, position: 0 })).toBeLessThan(0); }); // A position past the space left for it must never carry into the track's bits and outrank a @@ -50,8 +62,8 @@ test("out of range ranks stay inside their own bits", () => { expect(sendOrder({ priority: 1, position: Number.MAX_SAFE_INTEGER })).toBeLessThan( sendOrder({ priority: 2, position: 0 }), ); - expect(sendOrder({ priority: 300, position: 0 })).toBe(sendOrder({ priority: 255, position: 0 })); - expect(sendOrder({ priority: -1, position: -1 })).toBe(sendOrder({ priority: 0, position: 0 })); + expect(sendOrder({ priority: 300, position: 0 })).toBe(sendOrder({ priority: 127, position: 0 })); + expect(sendOrder({ priority: -300, position: -1 })).toBe(sendOrder({ priority: -128, position: 0 })); }); // The ranking itself, driven directly: a subscription outlives its own serving loop, since a diff --git a/js/net/src/priority.ts b/js/net/src/priority.ts index a0f82d60aef..42d1e4c31b8 100644 --- a/js/net/src/priority.ts +++ b/js/net/src/priority.ts @@ -9,15 +9,12 @@ import type { Dispose } from "@moq/signals"; import type { Writer } from "./stream.ts"; import type * as track from "./track.ts"; -// The transport compares send orders as a single number, so the two ranks are packed into -// disjoint ranges: the track priority takes the high bits so it always dominates, and the -// position below it. Group data stays negative so protocol streams at the transport's default -// order of 0 always make progress. A u8 priority and 44-bit position fill the negative half of -// the safe integer range exactly. -const GROUP_SPAN = 2 ** 44; - -/** The highest track priority the wire carries (a u8). */ -const MAX_PRIORITY = 0xff; +// The transport compares send orders as a single number, so the ranks are packed into +// disjoint ranges: the subscriber's priority takes the high bits so it always dominates, the +// publisher's below it, and the position below that. Group data stays negative so protocol +// streams at the transport's default order of 0 always make progress. Two signed byte +// priorities and a 36-bit position fit the negative half of the safe integer range. +const GROUP_SPAN = 2 ** 36; /** * What to rank a group stream by. @@ -25,9 +22,15 @@ const MAX_PRIORITY = 0xff; * @internal */ export interface Rank { - /** The subscriber's track priority, which outweighs everything below it. */ + /** The subscriber's track priority (`-128..=127`), which outweighs everything below it. */ priority: number; + /** + * The publisher's track priority (`-128..=127`), breaking ties between subscriptions at one + * subscriber priority. Defaults to 0, the unset priority. + */ + publisher?: number; + /** * Where the group sits in the queue its own subscription wants sent, 0 being the one to * send next. Defaults to 0, which is right for a stream that is the only one of its kind. @@ -47,14 +50,15 @@ export interface Rank { * * @internal */ -export function sendOrder({ priority, position = 0 }: Rank): number { - return -((MAX_PRIORITY - clamp(priority, MAX_PRIORITY)) * GROUP_SPAN + clamp(position, GROUP_SPAN - 1) + 1); +export function sendOrder({ priority, publisher = 0, position = 0 }: Rank): number { + const rank = (127 - clamp(priority, -128, 127)) * 256 + (127 - clamp(publisher, -128, 127)); + return -(rank * GROUP_SPAN + clamp(position, 0, GROUP_SPAN - 1) + 1); } -// Both ranks are bounded: the priority by its wire type, the position by the space left below -// it. Anything outside is a caller bug rather than a value worth preserving. -function clamp(value: number, max: number): number { - return Math.min(Math.max(Math.trunc(value), 0), max); +// Every rank is bounded: the priorities by their wire type, the position by the space left +// below them. Anything outside is a caller bug rather than a value worth preserving. +function clamp(value: number, min: number, max: number): number { + return Math.min(Math.max(Math.trunc(value), min), max); } // private: plumbing @@ -74,13 +78,18 @@ function clamp(value: number, max: number): number { */ export class Priority { #track: track.Subscriber; + #publisher: number; #streams = new Map(); #dispose: Dispose; #closed = false; - /** Follow `track`'s subscription until {@link close}. */ - constructor(track: track.Subscriber) { + /** + * Follow `track`'s subscription until {@link close}, breaking ties at one subscriber + * priority by the track's `publisher` priority. + */ + constructor(track: track.Subscriber, publisher = 0) { this.#track = track; + this.#publisher = publisher; this.#dispose = track.subscription.subscribe(() => this.#rerank()); } @@ -89,6 +98,7 @@ export class Priority { const subscription = this.#track.subscription.peek(); return sendOrder({ priority: subscription?.priority ?? 0, + publisher: this.#publisher, position: this.#position(sequence, object, subscription?.order), }); } @@ -147,7 +157,11 @@ export class Priority { const priority = subscription?.priority ?? 0; for (const [stream, { sequence, object }] of this.#streams) { stream.setPriority( - sendOrder({ priority, position: this.#position(sequence, object, subscription?.order) }), + sendOrder({ + priority, + publisher: this.#publisher, + position: this.#position(sequence, object, subscription?.order), + }), ); } } diff --git a/js/net/src/subscribe_priority.test.ts b/js/net/src/subscribe_priority.test.ts index 682259f1391..3b160162b01 100644 --- a/js/net/src/subscribe_priority.test.ts +++ b/js/net/src/subscribe_priority.test.ts @@ -16,8 +16,8 @@ import { wireOf } from "./wire.ts"; const url = new URL("https://localhost:4443/test"); const path = Path.from("test"); -const LOW = 1; -const HIGH = 200; +const LOW = -100; +const HIGH = 100; // Dial a session that consumes what `origin` serves. Draft-14's ALPN is the generic one, so // the server is told the version instead. diff --git a/js/net/src/track.test.ts b/js/net/src/track.test.ts index 0206488b98d..96a58cdedc2 100644 --- a/js/net/src/track.test.ts +++ b/js/net/src/track.test.ts @@ -450,14 +450,15 @@ test("a maxDelay or maxAge that is not a duration is refused, not rounded into o expect(() => producer.subscribe({ maxDelay: Milli(Number.MAX_SAFE_INTEGER + 1) })).toThrow(RangeError); }); -test("priority 255 is accepted and 256 is refused", () => { +test("priority -128..=127 is accepted and anything outside is refused", () => { const producer = new TrackProducer("video"); - producer.accept({ priority: 255 }); - expect(producer.priority).toBe(255); - expect(() => producer.accept({ priority: 256 })).toThrow(RangeError); - expect(() => producer.accept({ priority: -1 })).toThrow(RangeError); + producer.accept({ priority: 127 }); + expect(producer.priority).toBe(127); + expect(new TrackProducer("audio").subscribe({ priority: -128 }).subscription.peek()?.priority).toBe(-128); + expect(() => producer.accept({ priority: 128 })).toThrow(RangeError); + expect(() => producer.accept({ priority: -129 })).toThrow(RangeError); expect(() => producer.accept({ priority: 1.5 })).toThrow(RangeError); - expect(() => producer.subscribe({ priority: 256 })).toThrow(RangeError); + expect(() => producer.subscribe({ priority: 128 })).toThrow(RangeError); }); test("infoDefaults re-validates timescale and the safe maxAge bound", () => { diff --git a/js/net/src/track.ts b/js/net/src/track.ts index 9c90c9a8ba9..8c65a991c0f 100644 --- a/js/net/src/track.ts +++ b/js/net/src/track.ts @@ -52,9 +52,9 @@ const CACHE_WINDOW = Nano.fromMilli(Milli(30_000)); // The cache scans at most this many times per idle cache window. const PRUNE_SLICES = 8; -// The higher-first midpoint. IETF flips priority (lower first), so this goes out as 128, the -// draft's usual publisher priority, while moq-lite carries 127 as written: one urgency on both. -const DEFAULT_PRIORITY = 127; +// The unset priority, the midpoint of the signed range: moq-lite carries it as 128 and +// moq-transport as 127, one urgency on both. +const DEFAULT_PRIORITY = 0; /** * Sanity cap on a datagram payload: the QUIC DATAGRAM frame ceiling. The real limit is @@ -120,7 +120,7 @@ export interface Info { * or non-finite value and a result past `Number.MAX_SAFE_INTEGER`. */ maxAge?: Milli; - /** Tie-break priority between subscriptions of equal subscriber priority (`0..=255`, higher first). Defaults to `127`. */ + /** Tie-break priority between subscriptions of equal subscriber priority (`-128..=127`, higher first). Defaults to `0`. */ priority: number; /** * How many times its first frame a group's later stored bytes may grow before a joining @@ -158,9 +158,9 @@ function wireMillis(field: string, value: Milli): Milli { return Milli(millis); } -function priorityByte(value: number): number { - if (!Number.isInteger(value) || value < 0 || value > 255) { - throw new RangeError(`priority must be an integer in 0..=255: ${value}`); +function priorityValue(value: number): number { + if (!Number.isInteger(value) || value < -128 || value > 127) { + throw new RangeError(`priority must be an integer in -128..=127: ${value}`); } return value; } @@ -182,7 +182,7 @@ export function infoDefaults(info: Partial = {}): Info { compress: info.compress ?? false, timescale, maxAge: info.maxAge === undefined ? undefined : wireMillis("maxAge", info.maxAge), - priority: priorityByte(info.priority ?? DEFAULT_PRIORITY), + priority: priorityValue(info.priority ?? DEFAULT_PRIORITY), refresh: info.refresh == null ? undefined : refreshRatio(info.refresh), ...(info.unchecked ? { unchecked: true } : {}), }; @@ -212,7 +212,7 @@ export interface Groups { * The obsolete subscriber `maxAge` field is refused at runtime; use `maxDelay`. */ export interface Subscription { - /** Delivery priority relative to this session's other subscriptions (`0..=255`). Defaults to `0`. */ + /** Delivery priority relative to this session's other subscriptions (`-128..=127`, higher first). Defaults to `0`. */ priority?: number; /** * How long (milliseconds) a non-latest group stays worth delivering, by the same two @@ -261,7 +261,7 @@ function subscriptionDefaults(subscription: Subscription = {}): Subscription { if ("maxAge" in subscription) throw new TypeError("Subscriber maxAge is unsupported; use maxDelay"); const bounds = groupBounds(subscription.groups ?? {}); return { - priority: priorityByte(subscription.priority ?? 0), + priority: priorityValue(subscription.priority ?? DEFAULT_PRIORITY), maxDelay: wireMillis("maxDelay", subscription.maxDelay ?? Milli.zero), order: orderOf(subscription.order), groups: { @@ -674,9 +674,9 @@ export class Demand { return this.#state.closed; } - /** Publisher priority from the committed {@link Info}, or 0 before {@link Request.accept}. */ + /** Publisher priority from the committed {@link Info}, or the default before {@link Request.accept}. */ get priority(): number { - return this.#state.info.peek()?.priority ?? 0; + return this.#state.info.peek()?.priority ?? DEFAULT_PRIORITY; } } diff --git a/kt/moq/MIGRATE.md b/kt/moq/MIGRATE.md index 0cfe339b860..500678d4768 100644 --- a/kt/moq/MIGRATE.md +++ b/kt/moq/MIGRATE.md @@ -4,6 +4,12 @@ How to update your code for each breaking change, newest first. ## Unreleased +### Priorities are signed (#6142) + +`TrackInfo.priority`, `Subscription.priority`, and `FetchGroupOptions.priority` are `Byte`, higher first, and 0 is the unset midpoint; write `priority = 3` instead of `3u`. +`TrackInfo.priority` defaulted to 127 and now defaults to 0. +To keep a value's moq-lite byte, subtract 128 from it: 255 becomes 127, 128 becomes 0, and 0 becomes -128. + ### A client asks only about what it watches (#6165) On moq-lite, a client session asks its relay for announcements only under paths something watches, through `announced()` or `announcedBroadcast()`, instead of under its whole scope at connect. diff --git a/kt/moq/src/jvmAndAndroidTest/kotlin/dev/moq/SmokeTest.kt b/kt/moq/src/jvmAndAndroidTest/kotlin/dev/moq/SmokeTest.kt index 919113a6a63..c045c86dcc1 100644 --- a/kt/moq/src/jvmAndAndroidTest/kotlin/dev/moq/SmokeTest.kt +++ b/kt/moq/src/jvmAndAndroidTest/kotlin/dev/moq/SmokeTest.kt @@ -265,7 +265,7 @@ class SmokeTest { val fetched = broadcast.consume().fetchGroup( "events", 0uL, - FetchGroupOptions(priority = 3u), + FetchGroupOptions(priority = 3), ) assertEquals(0uL, fetched.sequence()) assertEquals("cached", fetched.readFrame()?.payload?.decodeToString()) @@ -372,7 +372,7 @@ class SmokeTest { // Fetch while the track is still published: finishing the media producer // unpublishes it, and the fetch would then miss with NotFound. - val fetched = dev.moq.media.GroupConsumer.fetch(consumer, dev.moq.media.GroupConfig(rendition = dev.moq.media.AudioRendition(name, audio), sequence = 0uL, options = FetchGroupOptions(priority = 3u))) + val fetched = dev.moq.media.GroupConsumer.fetch(consumer, dev.moq.media.GroupConfig(rendition = dev.moq.media.AudioRendition(name, audio), sequence = 0uL, options = FetchGroupOptions(priority = 3))) // Close the group so the fetched stream terminates instead of waiting for more. media.finish() diff --git a/py/moq-rs/MIGRATE.md b/py/moq-rs/MIGRATE.md index 39798153e67..3d50bab2cab 100644 --- a/py/moq-rs/MIGRATE.md +++ b/py/moq-rs/MIGRATE.md @@ -4,6 +4,12 @@ How to update your code for each breaking change, newest first. ## Unreleased +### Priorities are signed (#6142) + +`TrackInfo.priority`, `Subscription.priority`, and `FetchGroupOptions.priority` take `-128..=127`, higher first, and 0 is the unset midpoint. +`TrackInfo.priority` defaulted to 127 and now defaults to 0. +To keep a value's moq-lite byte, subtract 128 from it: 255 becomes 127, 128 becomes 0, and 0 becomes -128. + ### A client asks only about what it watches (#6165) On moq-lite, a client session asks its relay for announcements only under paths something watches, through `announced()` or `announced_broadcast()`, instead of under its whole scope at connect. diff --git a/py/moq-rs/moq/_records.py b/py/moq-rs/moq/_records.py index 05e5342d789..f1e4be00755 100644 --- a/py/moq-rs/moq/_records.py +++ b/py/moq-rs/moq/_records.py @@ -93,6 +93,7 @@ def _from_ffi(datagram: MoqDatagram) -> Datagram: class Subscription: """Subscriber-side delivery preferences, mirroring moq-net's ``track::Subscription``. + ``priority`` is in ``-128..=127``, higher first, and defaults to 0, the midpoint. ``max_delay`` is how far a non-latest group may fall behind before it is skipped; zero skips at once, and it caps every range. ``ranges`` are the spans of the track to deliver, ``None`` for everything ``max_delay`` still allows. ``order`` is which pending @@ -121,7 +122,8 @@ def _subscription(subscription: Subscription | None) -> MoqSubscription | None: class TrackInfo: """Publisher-side track properties, mirroring moq-net's ``track::Info``. - ``priority`` defaults to 127, the middle of the range. ``max_age`` is how long the + ``priority`` is in ``-128..=127``, higher first, and defaults to 0, the midpoint. It breaks + ties between subscriptions of equal subscriber priority. ``max_age`` is how long the publisher caches a non-latest group, ``None`` for no limit. ``refresh`` is how many times its first frame a group's later bytes may grow before a joining subscriber asks for a new group, ``None`` to never ask. ``timescale`` is ticks per second: @@ -130,7 +132,7 @@ class TrackInfo: always see the original frames. """ - priority: int = 127 + priority: int = 0 max_age: timedelta | None = None refresh: int | None = None timescale: int | None = None diff --git a/py/moq-rs/tests/test_local.py b/py/moq-rs/tests/test_local.py index 5166dbe6823..c3318208144 100644 --- a/py/moq-rs/tests/test_local.py +++ b/py/moq-rs/tests/test_local.py @@ -484,7 +484,7 @@ async def test_publish_track_info_and_subscription(): def test_track_info_keeps_native_default_priority(): - """A partially configured TrackInfo keeps moq-ffi's default priority of 127.""" + """A partially configured TrackInfo keeps moq-ffi's default priority of 0.""" native = moq_ffi.MoqTrackInfo().priority assert moq.TrackInfo()._ffi().priority == native assert moq.TrackInfo(max_age=timedelta(seconds=2))._ffi().priority == native diff --git a/quest/m1/README.md b/quest/m1/README.md index c46e8eec290..e89770e38f3 100644 --- a/quest/m1/README.md +++ b/quest/m1/README.md @@ -88,7 +88,6 @@ lite-07 stream type here. - [Rust identity enforcement](/quest/m1/catalog-identity-rs.md) - `moq-mux` refuses a redefined rendition, and every Rust publisher mints a new rendition name for a decoder identity change and updates everything else in place - [IETF takeover](/quest/m1/ietf-takeover.md) - carry the shared takeover contract through the negotiated IETF epoch extension - [Identity and recovery guide](/quest/m1/identity-guide.md) - explain pinned catalogs, lazy jobs, takeover, restart and version compatibility -- [Scope track priority](/quest/m1/track-priority-scope.md) - priority orders one owner's streams, sent and received, and a shared cluster session is fair across tenants - [An evicted group is judged stale](/quest/m1/evicted-group-stale.md) - a handed-out group the cache evicted goes stale once its successor's arrival plus the budget passes, so its stalled stream resets - [Native probing](/quest/m1/probe-native.md) - moq-quic retransmits in-flight data during BBR3's ProbeBW_UP so an app-limited sender validates headroom up to the target - [Native rung disable](/quest/m1/simulcast-rung-disable.md) - Rust/FFI and OBS publishers stop encoding the top renditions their grant cannot fund and resume them once probing raises the estimate diff --git a/quest/m1/c/README.md b/quest/m1/c/README.md index a466cd6f0ba..21105a6f61d 100644 --- a/quest/m1/c/README.md +++ b/quest/m1/c/README.md @@ -51,10 +51,11 @@ Rust and JS so C cannot retain the unsafe accept path while this line waits. That change owns its C tests and `MIGRATE.md` entries; other API reshaping stays here. Decided 2026-10-20 (maintainer): m1 quests that change the C API -([exact binding timestamps](/quest/m1/binding-timestamps.md), -[scope track priority](/quest/m1/track-priority-scope.md), +([exact binding timestamps](/quest/m1/binding-timestamps.md) and [Rust identity enforcement](/quest/m1/catalog-identity-rs.md)) Require this line and land their C half on the generated package, so C users break once. +Signed priorities already reached the hand-written ABI as `int8_t` in +[moq-dev/moq#6142](https://github.com/moq-dev/moq/pull/6142). [Dynamic subscribe ranges](/quest/m1/subscribe-ranges/dynamic.md) is the settled exception from its later plan in [moq-dev/moq#6123](https://github.com/moq-dev/moq/pull/6123): if generation diff --git a/quest/m1/quic/README.md b/quest/m1/quic/README.md index 73b76f896ff..dc3e48e07bb 100644 --- a/quest/m1/quic/README.md +++ b/quest/m1/quic/README.md @@ -10,7 +10,7 @@ reliable stream resets (#5690), hierarchical send groups (#5711, #5770), one `Limits` type for relay peers (#5871), and endpoint sharding (#5688) are on `main`. What remains is bounded incomplete-message memory; per-broadcast fairness gives the crate-private send domains (#5872) their consumer in -[Scope track priority](/quest/m1/track-priority-scope.md). +[Broadcast fairness](/quest/m2/broadcast-fairness.md). qmux already runs on the shared stream state machine, outside this epic. Per-stream acknowledgment progress, per-stream deadlines, and the experiments (the egress profile, ECN measurement, deadline keep-alive) live in @@ -74,8 +74,8 @@ cache limits remain separate; bitrate pacing composes with this memory cap. ## Related -- [Scope track priority](/quest/m1/track-priority-scope.md) - the - per-broadcast fairness policy on cluster sessions +- [Broadcast fairness](/quest/m2/broadcast-fairness.md) - the + per-broadcast fairness policy on cluster sessions, a consumer for send domains - [Native probing](/quest/m1/probe-native.md) - moq-quic retransmits in-flight data during ProbeBW_UP to probe up to a session's target - [L4S on the backbone](/quest/m3/quic-ecn.md) - an ECT(1) option in `moq-quic`, an `ecn` config knob, and a dualpi2 measurement - [Careful resume on reconnect](/quest/m3/quic-careful-resume.md) - a redial starts at the previous connection's rate diff --git a/quest/m1/quic/receive-memory.md b/quest/m1/quic/receive-memory.md index b99433a0b28..1409f9fb367 100644 --- a/quest/m1/quic/receive-memory.md +++ b/quest/m1/quic/receive-memory.md @@ -26,7 +26,7 @@ Decided 2026-10-20 (maintainer): three quests break moq-net's `transport::poll` traits, so they land in order and each rebases its trait change on the last: this quest first, then [Rust request credit](/quest/m1/rs-request-credit.md), then -[Scope track priority](/quest/m1/track-priority-scope.md). +the send domain in [broadcast fairness](/quest/m2/broadcast-fairness.md). That primitive landed as a provisional seam to finalize here: both native drivers' receive streams have a copying `poll_read_held` that yields a @@ -115,5 +115,5 @@ separate guide quest is needed. ## Related - [Unbounded groups](/quest/m2/unbounded-groups/README.md) - removes the frame cap; frame payload credit becomes per segment -- [Scope track priority](/quest/m1/track-priority-scope.md) - broadcast fairness domains and subscription priority semantics +- [Broadcast fairness](/quest/m2/broadcast-fairness.md) - broadcast fairness domains on the same sessions - [QUIC bitrate caps](/quest/m2/rate-quic.md) - pace credit grants inside the independently bounded memory allowance diff --git a/quest/m1/rs-request-credit.md b/quest/m1/rs-request-credit.md index 2424930b630..ebb544e7f5e 100644 --- a/quest/m1/rs-request-credit.md +++ b/quest/m1/rs-request-credit.md @@ -37,7 +37,7 @@ variant, and possibly a `transport::poll` session method. Wire: none. Decided 2026-10-20 (maintainer): the `transport::poll` trait breaks land in order, [receive memory](/quest/m1/quic/receive-memory.md) first, then this -quest, then [Scope track priority](/quest/m1/track-priority-scope.md), so +quest, then the send domain in [broadcast fairness](/quest/m2/broadcast-fairness.md), so implementers rebase on one shape at a time. ## Required diff --git a/quest/m1/simulcast-rung-disable.md b/quest/m1/simulcast-rung-disable.md index 84cdce3db26..f1d08a3244e 100644 --- a/quest/m1/simulcast-rung-disable.md +++ b/quest/m1/simulcast-rung-disable.md @@ -57,8 +57,8 @@ with padding. has a sender when its transport's `poll::Session::probe` returns one, which moq-net keeps private until this quest exposes it to the publishers. - Assign descending track priority down the renditions so the allocator - fills lower rungs first; [Scope track priority](/quest/m1/track-priority-scope.md) - settles how that number also orders sends. + fills lower rungs first; the same number breaks ties in send order + between subscriptions at one subscriber priority. - OBS: each OBS video encoder is its own rendition (`video_tracks` in `cpp/obs/src/moq-output.cpp`), so apply the rule per encoder through the generated C++ package, on top of the per-encoder diff --git a/quest/m1/subscribe-ranges/README.md b/quest/m1/subscribe-ranges/README.md index 9ec1b4db696..a8e1a1611a2 100644 --- a/quest/m1/subscribe-ranges/README.md +++ b/quest/m1/subscribe-ranges/README.md @@ -103,5 +103,4 @@ every sequence delivered or dropped. ## Related -- [Track priority scope](/quest/m1/track-priority-scope.md) - group order within a track now follows the subscription - [IETF fill timeout](/quest/m1/ietf-fill-timeout.md) - bounds upstream waiting independently of content age diff --git a/quest/m1/track-priority-scope.md b/quest/m1/track-priority-scope.md deleted file mode 100644 index 75f8200e613..00000000000 --- a/quest/m1/track-priority-scope.md +++ /dev/null @@ -1,174 +0,0 @@ -# [XL] Scope and sign track priority - -## Goal - -Priority orders streams only within one scheduling domain: the streams a single -party owns both ends of, the first mile from publisher to ingest relay and the -last mile from edge relay to viewer. On a shared relay-to-relay session the -domain is the broadcast: when the relay enables it, broadcasts share the link -fairly and priority orders streams only inside each broadcast, so one -customer's viewers asking for 255 never starve another customer's broadcast. - -In the same API break, every priority becomes an `i8`, higher first, with 0 as -the unset midpoint: `track::Info`, `Subscription`, and `group::Fetch` in Rust, -their JS counterparts, moq-ffi, moq-c, and every wrapper. Nobody has to know -that 127 is the middle of a `u8`. The wire stays a byte. - -## Plan - -Where the model stands today, all in `rs/moq-net`: - -- One `PriorityQueue` per session, shared by the moq-lite and moq-transport - publishers, a strict global sort over every in-flight group on the - connection: a subscription at - priority 200 pre-empts one at 100 indefinitely, and only the top 255 groups - get a distinct rank. -- `Priority` already ranks a group by its track priority, then its subscription - id, then group within that subscription in its order (`priority.rs`). JS - packs track priority and the group's position within its own subscription - into one send order (`js/net/src/priority.ts`), so two tracks at equal - priority interleave rather than one draining first. -- Group order within a track follows each subscription's `order` (#5385), - newest first by default. -- A relay forwards the max of its downstream subscriber priorities upstream - (`model/subscription.rs`, `lite/subscriber.rs`, and on moq-transport - upstreams `ietf/subscriber.rs` since #5922), never the publisher's track - priority, so one viewer asking for 255 raises that track above every other - tenant's on the cluster link (`rs/moq-relay/src/cluster.rs`: one - bidirectional session per peer pair carries every broadcast). The draft - already says the upstream leg SHOULD use the publisher priority. - -Decided 2026-09-19: the domain on a relay-to-relay session is the broadcast, -keyed by its path, and it is a relay setting (`cluster.fair = true`), off by -default so a single-tenant deployment keeps strict priority across the link. -A tenant-keyed domain (one bucket per customer, however many broadcasts they -run) needs the tenant identity from the auth line first and is a later -refinement of the same tier, not a different design. Opening more -subscriptions must never buy a broadcast more than its bucket allows. Until -tenant keying lands, a peer that opens more broadcasts does get more shares, -on send and receive alike; tenant keying is what closes that. - -Decided 2026-10-08: the signed priority type lands here, so the priority API -breaks once instead of twice. Subscribe ranges owns group order, so this quest -neither forbids an order knob nor edits the doc's Order row. - -Direction to settle in the draft first, then the code: - -- On the last mile the viewer owns the whole session, so its audio and video - subscriptions share one domain and the wire is unchanged. On a relay-to-relay - session the relay knows every subscription's broadcast, so no carrier is - needed on the wire; the draft only has to say that a relay MAY scope - priority to the broadcast. -- On a relay-to-relay session with fairness on, ordering across broadcasts is - byte-fair (the scheduler's fair tier, one domain per broadcast and one send group - per subscription inside that domain), the - publisher's track priority orders streams within a broadcast, and - downstream subscriber priorities are not forwarded. Attribute FETCH streams - to a domain the same way: cache-miss and history fetches carry Subscriber - Priority outside any subscription, so leaving them outside every bucket - bypasses tenant isolation; settle their carrier beside the subscription one, - or narrow the goal to subscription delivery. -- Rank by track priority, then subscription, then the subscription's own group - order (`Subscription::order`, newest first by default). -- Decided 2026-10-20 (maintainer): the publisher's `track::Info::priority` - breaks a tie between equal subscriber priorities in `Priority::cmp` - (`rs/moq-net/src/priority.rs`), so the same number decides what to produce - and, among equal subscriber priorities, what to send first. The - [ladder controller](/quest/m3/ladder/controller.md) only consumes it. - `Info::priority` in `rs/moq-net/src/model/track.rs` already claims the - tiebreak; fix the allocator doc in `rs/moq-net/src/model/bandwidth.rs`, - which presents send order as unrelated. -- A per-session cap on distinct ranks is a scheduling detail; whatever replaces - the 255-entry sort must stay O(log n) per group under chat-shaped churn. -- moq-quic already has the fair tier: send domains sharing bytes by deficit - round robin, with groups inside them. They are crate-private (exercised only - by its tests) until this quest gives them a consumer. Exposing them also - gives moq-net's `poll::Session::send_group(priority)` a domain, so land that - in the same trait break and implementers break once. - -Receive ranks, folded in from -[#5878](https://github.com/moq-dev/moq/pull/5878)'s follow-ups by the -maintainer on 2026-10-20. When the receive window is stuck, #5878 evicts the -lowest subscription priority first across every broadcast on the session, -then the group its subscription would deliver last, and sets a stream's rank -once, when its header decodes: - -- With fairness on, rank received groups within their broadcast's domain - first, as sends are, so one broadcast's priorities never evict another's - partial groups. Received FETCH streams map to a domain through the same - carrier the send side settles, or fall out with it if the goal narrows to - subscription delivery. -- Ranking inside domains still leaves the session-wide window to share, so - a broadcast that fills it would block every other one. Add a fair domain - tier ahead of the ranks, mirroring the byte-fair send tier: a stuck window - evicts from the domain holding the most beyond an even share of the window - across the domains with partial groups, then by rank within that domain. - A broadcast that fills the window alone may keep it until another arrives. - A full window admits no newcomer's bytes, so settle what enters a broadcast - into the fair set first (its subscription, or the header of a stream the - stuck-window eviction made room for), and test that it gets in. -- Re-rank a receive stream when its subscription's priority or group order - changes, instead of keeping the rank from its header. - -Signed priority, decided with the maintainer: - -- moq-lite carries `p + 128` (flip the top bit). The mapping is one-to-one - and keeps order, so a given byte means what it means today; only the API - number changes. The default becomes byte 128. -- IETF carries `127 - p`. Both mappings are one-to-one over the whole `i8` - range, with no saturation. An unset priority goes out as IETF 127, not the - draft's usual 128, and an absent IETF priority decodes to 0, the unset - default. Pin both ends in tests. -- Keep one urgency on both wires (#4273), so the IETF byte is always `255 -` - the lite byte. Mapping IETF as `128 - p` to hit the draft's 128 would break - that, which is why the default is one step off the draft there. -- hang's built-in priorities (`hang::catalog::PRIORITY`, today all below the - 127 default) move above 0, so hang media outranks a track that never set - one. Rust and JS keep matching values; the spacing is the implementer's call. -- moq-c's `moq_track_info.priority` has no presence flag, so a zeroed struct - means least urgent today. With `i8`, zero is the default. -- Look for anything that does arithmetic on priority (the lite send queue, JS - send-order packing, the bandwidth allocator, the relay's max-of-subscribers) - and keep its ordering, not just its type. -- moq-archive's `.info` priority follows and breaks in place with the - archive line's other formats (decided 2026-10-20, maintainer): no format - version or compatibility mapping for recordings made before the cut. -- The moxygen default (127, #4273) already shipped, so the default byte moves - a second time. Add an upgrade note to the release's upgrade page saying why. - Rejected: a mapping that keeps today's bytes (lite byte = p + 127). - -Decided 2026-10-20 (maintainer): the `transport::poll` trait breaks land in -order, [receive memory](/quest/m1/quic/receive-memory.md), then -[Rust request credit](/quest/m1/rs-request-credit.md), then this quest, which -adds the send domain last. The publisher serve loops it rewrites already -park one group per subscription for a stalled subscriber -([moq-dev/moq#6140](https://github.com/moq-dev/moq/pull/6140)); keep that -bound. The moq-c half lands on the -[generated C package](/quest/m1/c/README.md), not the frozen hand-written -ABI, so C users break once. - -Prove scoping with the existing lite publisher tests extended to two -broadcasts on one session with fairness on: neither starves, a downstream 255 -does not change the upstream order, and with fairness off the strict order is -unchanged. On the receive side, with fairness on, a high priority in one -broadcast never makes a stuck window evict another broadcast's partial groups -first, a broadcast that filled the window before another began receiving -yields down to its even share instead of blocking the newcomer, and a priority -or order update moves an already-ranked stream. The io_uring workers apply the same setting. Move -`doc/concept/moq-lite.md` (the 0..255 knob) and `doc/concept/standard.md` -(IETF 128 maps to 127) to the new range and mapping. - -Public API impact: `u8` priority fields become `i8` in every language, and -`Subscribe.priority` on a cluster session changes meaning. Wire impact: none -in format, but the default byte moves from 127 to 128 on moq-lite and from 128 -to 127 on IETF. Report both with the draft change. - -## Required - -- [Generated C bindings](/quest/m1/c/README.md) - the `i8` priority reaches C through the generated package, so C users break once -- [Rust request credit](/quest/m1/rs-request-credit.md) - the previous `transport::poll` break; this one adds the send domain on top - -## Related - -- [Subscribe ranges](/quest/m1/subscribe-ranges/README.md) - owns the group - order a subscription asks for diff --git a/quest/m2/README.md b/quest/m2/README.md index d5d9f8cc95f..c6dbabf0107 100644 --- a/quest/m2/README.md +++ b/quest/m2/README.md @@ -35,6 +35,7 @@ ranked just below deterministic TS legs, which it requires. - [Poll-native async](/quest/m2/ffi-poll-migrate.md) - moq-ffi's pure-kio async exports poll their state directly instead of going through `ffi::Task` - [Foreign poll for hot reads](/quest/m2/ffi-poll-protocol.md) - the fork's generators give each wrapper a sync poll plus a registered waker for track and group reads, so a ready read skips the event-loop round trip and a cancel never loses a frame - [QoS](/quest/m2/qos/README.md) - the relay's egress `lag` histogram and `dropped` counters on the stats totals and prefix tracks +- [Broadcast fairness](/quest/m2/broadcast-fairness.md) - broadcasts share a cluster link fairly, so one tenant's priority never starves another's - [Plan: watch worker](/quest/m2/plan-watch-worker.md) - prototype an invisible page worker against app-spawned workers, and land the jank harness that decides - [Watch worker](/quest/m2/watch-worker.md) - watch playback runs in a worker onto an OffscreenCanvas, so main-thread jank never stalls video or audio - [Vulkan Video encode on AMD](/quest/m2/vulkan-encode.md) - H.264 and H.265 from an external Vulkan image on RADV, hand-rolled on ash, proven on an RX 9070 diff --git a/quest/m2/audio-bufferbloat-av.md b/quest/m2/audio-bufferbloat-av.md index e71ba417bd6..fc002775c8e 100644 --- a/quest/m2/audio-bufferbloat-av.md +++ b/quest/m2/audio-bufferbloat-av.md @@ -41,8 +41,9 @@ Recommendations: - Budgets come from nightly runs by the rule in `budgets.json`'s note, not a developer machine. The native `pulse` ceilings there are provisional developer-host numbers; re-measure them from nightlies before enforcing. -- [Track priority scope](/quest/m1/track-priority-scope.md) renumbers - priorities; assert on behavior, not on the 80 and 60 constants. +- Priorities are signed since + [moq-dev/moq#6142](https://github.com/moq-dev/moq/pull/6142); assert on + behavior, not on the 80 and 60 constants. - Update the harness README's "Not covered" list, which names video. Public API: none. Wire: none. diff --git a/quest/m2/broadcast-fairness.md b/quest/m2/broadcast-fairness.md new file mode 100644 index 00000000000..ea2b07f2188 --- /dev/null +++ b/quest/m2/broadcast-fairness.md @@ -0,0 +1,106 @@ +# [L] Fair share across broadcasts on cluster links + +## Goal + +On a shared relay-to-relay session, broadcasts share the link fairly and +priority orders streams only inside each broadcast, so one tenant's viewers +asking for the highest priority never starve another tenant's broadcast. +Opening more subscriptions must never buy a broadcast more than its bucket +allows. Until tenant keying lands, a peer that opens more broadcasts does get +more shares, on send and receive alike; tenant keying is what closes that. +Single-tenant deployments and the first and last mile keep +strict priority across the session, as today. + +## Plan + +Needs a fresh `/quest-plan` before starting: there is no settled design. +Every transport already shares moq-quic's scheduler since qmux moved onto it +([moq-dev/moq#6231](https://github.com/moq-dev/moq/pull/6231)); revisit once the +auth line carries a tenant identity. + +Today every session schedules strictly: one `PriorityQueue` or one moq-quic +send group per subscription, ranked by the subscriber's priority, then the +publisher's track priority, then subscription id +(`rs/moq-net/src/priority.rs`). A relay forwards the max of its downstream +subscriber priorities upstream (`model/subscription.rs`, `lite/subscriber.rs`, +and on moq-transport upstreams `ietf/subscriber.rs`), and +`rs/moq-relay/src/cluster.rs` runs one bidirectional session per peer pair +carrying every broadcast, so a viewer asking for 127 raises its track above +every other tenant's on the cluster link. + +Direction carried over from 2026-09-19, not re-confirmed: + +- The domain on a relay-to-relay session is the broadcast, keyed by its path, + behind a relay setting (`cluster.fair`), off by default. +- A tenant-keyed domain (one bucket per customer, however many broadcasts they + run) needs the tenant identity from the auth line and refines the same tier. +- moq-quic already has the fair tier: send domains sharing bytes by deficit + round robin, with groups inside them, crate-private until a consumer lands. + Exposing them gives `transport::poll::Session::send_group` a domain, a + trait break for every implementer. +- The draft would say a relay MAY scope priority to the broadcast on a + relay-to-relay connection; no carrier is needed on the wire. +- Decided 2026-10-20 (maintainer): the `transport::poll` trait breaks land in + order, [receive memory](/quest/m1/quic/receive-memory.md), then + [Rust request credit](/quest/m1/rs-request-credit.md), then the send domain. + +Receive ranks, folded in from +[moq-dev/moq#5878](https://github.com/moq-dev/moq/pull/5878)'s follow-ups by +the maintainer on 2026-10-20. When the receive window is stuck, #5878 evicts +the lowest subscription priority first across every broadcast on the session, +then the group its subscription would deliver last, and sets a stream's rank +once, when its header decodes: + +- With fairness on, rank received groups within their broadcast's domain + first, as sends are, so one broadcast's priorities never evict another's + partial groups. Received FETCH streams map to a domain through the same + carrier the send side settles. +- Ranking inside domains still leaves the session-wide window to share, so a + broadcast that fills it would block every other one. Add a fair domain tier + ahead of the ranks, mirroring the byte-fair send tier: a stuck window evicts + from the domain holding the most beyond an even share of the window across + the domains with partial groups, then by rank within that domain. A + broadcast that fills the window alone may keep it until another arrives. A + full window admits no newcomer's bytes, so settle what enters a broadcast + into the fair set first, and test that it gets in. +- Re-rank a receive stream when its subscription's priority or group order + changes, instead of keeping the rank from its header. This one does not + need fairness; split it out if it should land first. + +Dropped 2026-10-20 from +[moq-dev/moq#6142](https://github.com/moq-dev/moq/pull/6142), which landed +only signed priorities and the publisher-priority tiebreak. Its first +revision prototyped a send domain per broadcast in the lite publisher +(`session::Schedule { Strict, Broadcast }`, `poll::Session::send_domain`, +public moq-quic domains, `cluster.fair`), falling back to strict on a +transport without domains. The maintainer: "IDK I feel like we don't have a +good plan here". Open questions it left: + +- The IETF publisher schedules strictly; it needs the same domains. +- Tenant-keyed domains instead of, or above, broadcast domains. +- FETCH placement: cache-miss and history fetches carry Subscriber Priority + outside any subscription, so leaving them outside every bucket bypasses + isolation. +- Subscriber aggregate forwarding: under a fair schedule the downstream max + should stop being forwarded upstream at its source, not just be ignored by + the next fair hop. +- Transports without send domains (WebSocket, iroh, today's qmux) and + moq-transport sessions: refuse, or degrade to strict. +- A cheaper capability probe than opening and dropping a domain. + +Prove it with two broadcasts on one cluster session over a bufferbloated +link: neither starves, a downstream 127 does not change the upstream order, +and with fairness off the strict order is unchanged. On the receive side, a +high priority in one broadcast never makes a stuck window evict another +broadcast's partial groups first, a broadcast that filled the window before +another began receiving yields down to its even share, and a priority or order +update moves an already-ranked stream. + +## Required + +- [Rust request credit](/quest/m1/rs-request-credit.md) - the previous `transport::poll` break; the send domain goes on top + +## Related + +- [One parked group per subscription](https://github.com/moq-dev/moq/pull/6140) - the parked group each subscription keeps must still yield the next stream by broadcast +- [Bound incomplete messages](/quest/m1/quic/receive-memory.md) - the receive-side counterpart on the same moq-quic sessions diff --git a/quest/m3/ladder/README.md b/quest/m3/ladder/README.md index bd37a8deeba..024bdbec998 100644 --- a/quest/m3/ladder/README.md +++ b/quest/m3/ladder/README.md @@ -34,12 +34,9 @@ What remains is the publisher side. The allocator estimate by `track::Info::priority`, filling a tier before the next sees a bit and splitting max-min fair within one. A controller that assigns descending priorities down the ladder gets correct allocation from that -alone. Send order is a different number today: `Priority::cmp` -(`rs/moq-net/src/priority.rs`) ranks first by `Priority.track`, which is -the subscriber's priority from SUBSCRIBE (`msg.priority` in -`rs/moq-net/src/lite/publisher.rs`), not the publisher's `Info::priority`. -[Scope track priority](/quest/m1/track-priority-scope.md) owns making the -publisher's number the tiebreak after it, so the same number decides what to +alone. Send order ranks by the subscriber's priority from SUBSCRIBE first +and the publisher's `Info::priority` after it (`Priority::cmp` in +`rs/moq-net/src/priority.rs`, [moq-dev/moq#6142](https://github.com/moq-dev/moq/pull/6142)), so the same number decides what to produce and, among equal subscriber priorities, what to send first; the controller only consumes that tiebreak. diff --git a/quest/m3/ladder/controller.md b/quest/m3/ladder/controller.md index 9f61396bc03..c1d26cd668e 100644 --- a/quest/m3/ladder/controller.md +++ b/quest/m3/ladder/controller.md @@ -33,9 +33,7 @@ stamped `PRIORITY.video` today (`rs/moq-transcode/src/rung.rs`, before the next sees a bit, so that alone protects lower rungs' allocation. For send order, consume the publisher-priority tiebreak in `Priority::cmp` -that [Scope track priority](/quest/m1/track-priority-scope.md) settles; that -quest owns the code change and the doc fixes (decided in the 2026-10-06 -audit). The `BitrateUnsupported` fallback leans on it: without it, an +([moq-dev/moq#6142](https://github.com/moq-dev/moq/pull/6142)). The `BitrateUnsupported` fallback leans on it: without it, an encoder that cannot retune degrades the whole ladder equally instead of protecting the bottom. A subscriber asking for a higher rendition ahead of a lower one still gets what it asked for. @@ -67,10 +65,6 @@ bandwidth input preserving existing behavior exactly; and for send order, equal subscriber priority where the lower rung wins, conflicting subscriber priority where the subscriber's order wins, and a custom ladder order. -## Required - -- [Scope track priority](/quest/m1/track-priority-scope.md) - settles the priority ranking the controller assigns before the encoder side reads it - ## Related - [Encoder feedback](/quest/m3/stats-encoder-feedback.md) - viewer stalls for `moq import`; its `moq transcode --echo` leg, split out if revived, feeds this controller diff --git a/rs/hang/MIGRATE.md b/rs/hang/MIGRATE.md index dde7dbb8fe0..33cd6fd87b4 100644 --- a/rs/hang/MIGRATE.md +++ b/rs/hang/MIGRATE.md @@ -4,6 +4,11 @@ How to update your code for each breaking change, newest first. ## Unreleased +### Priorities are signed (#6142) + +`catalog::Priorities` fields and `container::track_info`'s priority are `i8`, higher first, with 0 unset. +`PRIORITY` keeps its values (100, 90, 80, 60), which now sit above the unset default instead of below it, so their moq-lite bytes move from 100..60 to 228..188. + ### `catalog::Reference` names its epoch (#5699) Every rendition and data track config gains an optional `epoch` beside `broadcast`, and `Archive` gains `replay_epoch` beside `replay`. diff --git a/rs/hang/src/catalog/archive.rs b/rs/hang/src/catalog/archive.rs index ffd463b5075..c988e6b007b 100644 --- a/rs/hang/src/catalog/archive.rs +++ b/rs/hang/src/catalog/archive.rs @@ -71,7 +71,7 @@ pub struct Archive { impl Archive { /// The recording object format advertised in [`version`](Self::version). - pub const VERSION: u32 = 2; + pub const VERSION: u32 = 3; /// The default timeline timescale, milliseconds. pub const fn default_timescale() -> u32 { @@ -129,7 +129,7 @@ mod test { let json = serde_json::to_string(&archive).unwrap(); assert_eq!( json, - r#"{"timelines":{"video":{"track":"video.timeline.z","duration":4000}},"timescale":1000,"replay":"recordings/clip","store":"https://objects.example/rec/","version":2}"# + r#"{"timelines":{"video":{"track":"video.timeline.z","duration":4000}},"timescale":1000,"replay":"recordings/clip","store":"https://objects.example/rec/","version":3}"# ); assert_eq!(serde_json::from_str::(&json).unwrap(), archive); } diff --git a/rs/hang/src/catalog/priority.rs b/rs/hang/src/catalog/priority.rs index d8ba1902ed4..b57e7bacd3a 100644 --- a/rs/hang/src/catalog/priority.rs +++ b/rs/hang/src/catalog/priority.rs @@ -5,18 +5,19 @@ #[non_exhaustive] pub struct Priorities { /// Catalog track priority. - pub catalog: u8, + pub catalog: i8, /// Text (caption/subtitle) track priority. - pub text: u8, + pub text: i8, /// Audio track priority. - pub audio: u8, + pub audio: i8, /// Video track priority. - pub video: u8, + pub video: i8, } /// Default delivery priorities for hang tracks, with higher values sent first. /// -/// Text sits just below the catalog and above audio/video: cues are tiny and accessibility-critical, +/// All of them sit above 0, the unset default, so hang media outranks a track that never set +/// a priority. Text sits just below the catalog and above audio/video: cues are tiny and accessibility-critical, /// so they should never be starved behind a media backlog. pub const PRIORITY: Priorities = Priorities { catalog: 100, diff --git a/rs/hang/src/container/frame.rs b/rs/hang/src/container/frame.rs index 62019211256..8b7c16d8af8 100644 --- a/rs/hang/src/container/frame.rs +++ b/rs/hang/src/container/frame.rs @@ -34,10 +34,10 @@ pub const TIMESCALE: Timescale = Timescale::MICRO; /// [`PRIORITY`](crate::catalog::PRIORITY) so every publisher orders its kinds the /// same way (`PRIORITY.video` for a video track, and so on). It's a parameter /// rather than a default because there is no correct value for "some media -/// track": leaving it at zero is what puts audio and video in one undifferentiated +/// track": leaving it at the unset 0 is what puts audio and video in one undifferentiated /// tier, both on the wire and in /// [`bandwidth::Allocator`](moq_net::bandwidth::Allocator). -pub fn track_info(priority: u8) -> moq_net::track::Info { +pub fn track_info(priority: i8) -> moq_net::track::Info { moq_net::track::Info::default() .with_timescale(TIMESCALE) .with_max_age(MAX_AGE) diff --git a/rs/moq-archive/MIGRATE.md b/rs/moq-archive/MIGRATE.md index 35fa7a38048..c08212303ca 100644 --- a/rs/moq-archive/MIGRATE.md +++ b/rs/moq-archive/MIGRATE.md @@ -4,6 +4,13 @@ How to update your code for each breaking change, newest first. ## Unreleased +### Recording format version 3 stores a signed priority (#6142) + +`Info::priority`, `Info::new`, and `Error::Priority` use `i8`, higher first, with 0 unset. +`VERSION` is 3, whose `.info` stores that `i8` as written; version 2 stored the moq-lite byte, `priority + 128`. +Readers still read version 2 recordings, converting the byte, so a stored 127 reads as -1; a decoded version 2 `Info` keeps `version: 2` and refuses to `encode`. +A `Producer` refuses to resume a version 2 recording with `Error::Version(2)`, so start a new prefix. + ### `Producer` replaces `Writer` (#6160) `Writer`, its `Control`, and `writer::{Config, Retention}` are removed; record with `Producer::new(store, broadcast, producer::Config::default()).run().await` instead. diff --git a/rs/moq-archive/src/error.rs b/rs/moq-archive/src/error.rs index 6bbd956dd41..9a9b075965c 100644 --- a/rs/moq-archive/src/error.rs +++ b/rs/moq-archive/src/error.rs @@ -10,8 +10,8 @@ pub enum Error { #[error("not found: {0}")] NotFound(String), - /// The recording format version is not the current one. - #[error("unknown version {0}")] + /// The recording format version is unknown, or older than the current one when writing. + #[error("unsupported version {0}")] Version(u64), /// A group ID, segment ID, or frame timestamp is outside 0 through 2^53 - 1. @@ -26,9 +26,9 @@ pub enum Error { #[error("priority {existing} does not match {intended}")] Priority { /// The priority already recorded. - existing: u8, + existing: i8, /// The priority the caller wanted to record. - intended: u8, + intended: i8, }, /// An existing `.info` has a different timescale, `None` when untimed. diff --git a/rs/moq-archive/src/info.rs b/rs/moq-archive/src/info.rs index ff90254914d..38024bd3733 100644 --- a/rs/moq-archive/src/info.rs +++ b/rs/moq-archive/src/info.rs @@ -2,48 +2,58 @@ use bytes::Bytes; use serde::{Deserialize, Serialize}; use crate::path::check_id; -use crate::{Error, Result, VERSION}; +use crate::{Error, Result, VERSION, VERSION_MIN}; /// Immutable track properties stored at `/.info`. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -#[serde(from = "Stored")] +#[serde(try_from = "Stored")] pub struct Info { - /// Recording format version; must be [`VERSION`]. + /// Recording format version: [`VERSION`], or 2 for a recording read from before + /// priorities were signed, which only a reader accepts. pub version: u64, - /// Publisher tie-break priority, 0 through 255. - pub priority: u8, + /// The publisher's track priority, higher first. + pub priority: i8, /// Units per second for this track's timestamps, 1 through 2^53 - 1, or `None` when the track /// is untimed and its frames store no timestamps. #[serde(default, skip_serializing_if = "Option::is_none")] pub timescale: Option, } -/// [`Info`] as stored, also taking the `epoch` an older `Writer` recorded, so [`Reader`](crate::Reader) -/// still replays those recordings; it goes when `Reader` does. +/// [`Info`] as stored, before its priority is read in its version's encoding. It also takes the +/// `epoch` an older `Writer` recorded, so [`Reader`](crate::Reader) still replays those +/// recordings; that goes when `Reader` does. #[derive(Deserialize)] #[serde(deny_unknown_fields)] struct Stored { version: u64, - priority: u8, + priority: i64, #[serde(default)] timescale: Option, #[serde(default, rename = "epoch")] _epoch: Option, } -impl From for Info { - fn from(stored: Stored) -> Self { - Self { +impl TryFrom for Info { + type Error = String; + + fn try_from(stored: Stored) -> std::result::Result { + let priority = match stored.version { + // Version 2 stored the moq-lite byte, `priority + 128`. + 2 => u8::try_from(stored.priority).map(|byte| (byte ^ 0x80) as i8), + _ => i8::try_from(stored.priority), + } + .map_err(|_| format!("priority {} is out of range", stored.priority))?; + Ok(Self { version: stored.version, - priority: stored.priority, + priority, timescale: stored.timescale, - } + }) } } impl Info { /// The current format version with `priority` and `timescale`, `None` for an untimed track. - pub fn new(priority: u8, timescale: Option) -> Result { + pub fn new(priority: i8, timescale: Option) -> Result { let info = Self { version: VERSION, priority, @@ -53,8 +63,11 @@ impl Info { Ok(info) } - /// Encode as compact JSON. + /// Encode as compact JSON, refusing any version but the current one. pub fn encode(&self) -> Result { + if self.version != VERSION { + return Err(Error::Version(self.version)); + } self.validate()?; Ok(Bytes::from(serde_json::to_vec(self)?)) } @@ -67,7 +80,7 @@ impl Info { } fn validate(&self) -> Result<()> { - if self.version != VERSION { + if !(VERSION_MIN..=VERSION).contains(&self.version) { return Err(Error::Version(self.version)); } if let Some(timescale) = self.timescale { @@ -89,7 +102,7 @@ mod tests { fn roundtrip() { let info = Info::new(0, Some(1_000_000)).unwrap(); let bytes = info.encode().unwrap(); - assert_eq!(bytes.as_ref(), br#"{"version":2,"priority":0,"timescale":1000000}"#); + assert_eq!(bytes.as_ref(), br#"{"version":3,"priority":0,"timescale":1000000}"#); assert_eq!(Info::decode(&bytes).unwrap(), info); } @@ -97,24 +110,57 @@ mod tests { fn an_untimed_track_omits_its_timescale() { let info = Info::new(3, None).unwrap(); let bytes = info.encode().unwrap(); - assert_eq!(bytes.as_ref(), br#"{"version":2,"priority":3}"#); + assert_eq!(bytes.as_ref(), br#"{"version":3,"priority":3}"#); assert_eq!(Info::decode(&bytes).unwrap(), info); } #[test] fn whitespace_and_member_order_do_not_affect_equality() { - let info = Info::decode(br#"{"timescale":1000,"priority":7,"version":2}"#).unwrap(); + let info = Info::decode(br#"{"timescale":1000,"priority":7,"version":3}"#).unwrap(); assert_eq!( info, - Info::decode(br#"{ "version": 2, "priority": 7, "timescale": 1000 }"#).unwrap() + Info::decode(br#"{ "version": 3, "priority": 7, "timescale": 1000 }"#).unwrap() ); assert_eq!(info.priority, 7); assert_eq!(info.timescale, Some(1000)); } + #[test] + fn priority_is_stored_signed() { + for priority in [i8::MIN, -1, 0, i8::MAX] { + let info = Info::new(priority, None).unwrap(); + let json = format!(r#"{{"version":3,"priority":{priority}}}"#); + assert_eq!(info.encode().unwrap().as_ref(), json.as_bytes()); + assert_eq!(Info::decode(json.as_bytes()).unwrap(), info); + } + for priority in [-129, 128] { + let json = format!(r#"{{"version":3,"priority":{priority}}}"#); + assert!( + matches!(Info::decode(json.as_bytes()), Err(Error::Json(_))), + "{priority}" + ); + } + } + + /// Version 2 stored the priority as its moq-lite byte, `priority + 128`, so an old + /// recording keeps its meaning; nothing writes version 2 any more. + #[test] + fn a_version_2_info_reads_its_lite_byte() { + for (byte, priority) in [(0, i8::MIN), (127, -1), (128, 0), (255, i8::MAX)] { + let json = format!(r#"{{"version":2,"priority":{byte},"timescale":1000}}"#); + let info = Info::decode(json.as_bytes()).unwrap(); + assert_eq!((info.version, info.priority, info.timescale), (2, priority, Some(1000))); + assert!(matches!(info.encode(), Err(Error::Version(2)))); + } + for byte in [-1, 256] { + let json = format!(r#"{{"version":2,"priority":{byte}}}"#); + assert!(matches!(Info::decode(json.as_bytes()), Err(Error::Json(_))), "{byte}"); + } + } + #[test] fn unknown_version_is_refused() { - for version in [1, 3] { + for version in [1, VERSION + 1] { let json = format!(r#"{{"version":{version},"priority":0,"timescale":1000}}"#); assert!(matches!(Info::decode(json.as_bytes()), Err(Error::Version(v)) if v == version)); } @@ -130,22 +176,18 @@ mod tests { #[test] fn an_older_writers_epoch_is_ignored() { - let info = Info::decode(br#"{"version":2,"priority":0,"timescale":1000,"epoch":"x"}"#).unwrap(); - assert_eq!(info, Info::new(0, Some(1000)).unwrap()); - assert_eq!( - info.encode().unwrap().as_ref(), - br#"{"version":2,"priority":0,"timescale":1000}"# - ); + let info = Info::decode(br#"{"version":2,"priority":128,"timescale":1000,"epoch":"x"}"#).unwrap(); + assert_eq!((info.version, info.priority, info.timescale), (2, 0, Some(1000))); } #[test] fn unknown_members_and_missing_fields_are_refused() { assert!(matches!( - Info::decode(br#"{"version":2,"priority":0,"timescale":1,"extra":true}"#), + Info::decode(br#"{"version":3,"priority":0,"timescale":1,"extra":true}"#), Err(Error::Json(_)) )); assert!(matches!( - Info::decode(br#"{"version":2,"timescale":1}"#), + Info::decode(br#"{"version":3,"timescale":1}"#), Err(Error::Json(_)) )); } diff --git a/rs/moq-archive/src/lib.rs b/rs/moq-archive/src/lib.rs index 2871111019b..12e47d66db3 100644 --- a/rs/moq-archive/src/lib.rs +++ b/rs/moq-archive/src/lib.rs @@ -8,7 +8,7 @@ //! //! A [`Producer`] writes, under its prefix: //! -//! - `/.info`: the track's properties as JSON, `{"version":2,"priority":127,"timescale":1000}`, +//! - `/.info`: the track's properties as JSON, `{"version":3,"priority":0,"timescale":1000}`, //! written before its first object. `timescale` is absent on an untimed track. Properties never //! change for a track name. The source's max age is not stored: it bounds the live cache, not //! the archive. @@ -32,7 +32,7 @@ //! //! ```text //! Object { -//! Version (i) = 2, +//! Version (i) = 3, //! Frame Start (i), // the first group's first frame index; 0 when it starts with a datagram //! Entry Count (i), //! Entry (..) ..., @@ -82,7 +82,11 @@ pub use segment::{Frame, Group, Object}; pub use store::Store; /// Recording format version written into `.info` and the binary envelope. -pub const VERSION: u64 = 2; +pub const VERSION: u64 = 3; + +/// The oldest recording format version [`Reader`] accepts. Version 2 stored `.info`'s +/// priority as its moq-lite byte and is otherwise identical. +pub(crate) const VERSION_MIN: u64 = 2; /// Largest group, segment, or timestamp value the format allows (`2^53 - 1`). pub const ID_MAX: u64 = (1 << 53) - 1; diff --git a/rs/moq-archive/src/producer/resume.rs b/rs/moq-archive/src/producer/resume.rs index 4d8ca41958b..18bfeb8e7e1 100644 --- a/rs/moq-archive/src/producer/resume.rs +++ b/rs/moq-archive/src/producer/resume.rs @@ -9,7 +9,7 @@ use object_store::path::Path; use object_store::{ObjectStore, ObjectStoreExt}; use crate::layout::{self, End, Name, Object, Start}; -use crate::{Error, Info, Result, Store}; +use crate::{Error, Info, Result, Store, VERSION}; /// One track as stored. pub(super) struct Stored { @@ -49,6 +49,10 @@ async fn load_track(store: &Store, dir: &Path) -> Result = store.inner().list(Some(dir)).try_collect().await?; let info_path = crate::path::push(dir, layout::TRACK_INFO)?; let info = Info::decode(&store.inner().get(&info_path).await?.bytes().await?)?; + // A reader still reads an older format, but nothing appends to it. + if info.version != VERSION { + return Err(Error::Version(info.version)); + } let timed = info.timescale.is_some(); let mut objects: Vec<(Start, Path)> = Vec::new(); diff --git a/rs/moq-archive/src/producer/tests.rs b/rs/moq-archive/src/producer/tests.rs index ad06608f94e..1325262581d 100644 --- a/rs/moq-archive/src/producer/tests.rs +++ b/rs/moq-archive/src/producer/tests.rs @@ -569,6 +569,20 @@ async fn changed_properties_fail_the_resume() { ); } +/// A reader still reads a version 2 `.info`, but a producer never appends to one. +#[tokio::test(start_paused = true)] +async fn an_older_version_fails_the_resume() { + let store = store(); + let info = object_store::path::Path::from("rec/video/.info"); + let v2 = br#"{"version":2,"priority":128,"timescale":1000}"#; + store.inner().put(&info, v2.as_ref().into()).await.unwrap(); + let broadcast = broadcast::Info::new().produce(); + let _video = broadcast.create_track("video", timed()).unwrap(); + let err = spawn(&store, &broadcast, config()).await.unwrap().unwrap_err(); + assert!(matches!(err, Error::Version(2)), "{err}"); + assert_eq!(get(&store, "video/.info").await.as_deref(), Some(&v2[..])); +} + #[tokio::test(start_paused = true)] async fn track_names_stay_one_segment_through_a_resume() { let store = store(); diff --git a/rs/moq-archive/src/reader/tests.rs b/rs/moq-archive/src/reader/tests.rs index 150d9d0c25e..19a6cedb6b8 100644 --- a/rs/moq-archive/src/reader/tests.rs +++ b/rs/moq-archive/src/reader/tests.rs @@ -645,7 +645,7 @@ async fn a_track_without_usable_info_is_not_found() { archive .raw( &Key::info("future").unwrap(), - br#"{"version":3,"priority":0,"timescale":1000}"#, + br#"{"version":4,"priority":0,"timescale":1000}"#, ) .await; diff --git a/rs/moq-archive/src/segment.rs b/rs/moq-archive/src/segment.rs index 9f98a119719..7f69c463e0b 100644 --- a/rs/moq-archive/src/segment.rs +++ b/rs/moq-archive/src/segment.rs @@ -3,7 +3,7 @@ use hang::timeline::Position; use moq_net::varint; use crate::path::check_id; -use crate::{Error, Result, VERSION}; +use crate::{Error, Result, VERSION, VERSION_MIN}; /// One group's stored frames in a segment object, in sequence order. #[derive(Debug, Clone, PartialEq, Eq)] @@ -116,11 +116,13 @@ impl Object { /// Decode a complete table before slicing any payload, and refuse an unknown version. /// + /// Version 2 objects are identical to the current ones. + /// /// `timed` is whether the track's `.info` declares a timescale, which says whether the table /// stores a timestamp per frame. pub fn decode(mut buf: impl Buf, timed: bool) -> Result { let version = read_varint(&mut buf)?; - if version != VERSION { + if !(VERSION_MIN..=VERSION).contains(&version) { return Err(Error::Version(version)); } @@ -368,7 +370,7 @@ mod tests { .unwrap(); let mut buf = &bytes[..]; - assert_eq!(read_varint(&mut buf).unwrap(), 2); // version + assert_eq!(read_varint(&mut buf).unwrap(), VERSION); // version assert_eq!(read_varint(&mut buf).unwrap(), 0); // frame start assert_eq!(read_varint(&mut buf).unwrap(), 3); // group count assert_eq!(read_varint(&mut buf).unwrap(), 5); // absolute @@ -496,7 +498,7 @@ mod tests { #[test] fn unknown_version_is_refused() { // Version 1 objects held whole groups under range-named keys. - for version in [1, 3] { + for version in [1, VERSION + 1] { let mut bytes = BytesMut::new(); write_varint(&mut bytes, version).unwrap(); write_varint(&mut bytes, 1).unwrap(); @@ -504,6 +506,21 @@ mod tests { } } + #[test] + fn a_version_2_object_decodes_unchanged() { + let original = object(vec![Group { + sequence: 3, + frames: vec![untimed(b"a")], + }]); + let mut bytes = BytesMut::new(); + write_varint(&mut bytes, 2).unwrap(); + let encoded = original.encode().unwrap(); + let mut rest = &encoded[..]; + assert_eq!(read_varint(&mut rest).unwrap(), VERSION); + bytes.extend_from_slice(rest); + assert_eq!(Object::decode(bytes.freeze(), false).unwrap(), original); + } + #[test] fn payload_must_be_contiguous() { let mut good = object(vec![Group { diff --git a/rs/moq-archive/src/store.rs b/rs/moq-archive/src/store.rs index d3d7cee6809..3e8b3d8b1a1 100644 --- a/rs/moq-archive/src/store.rs +++ b/rs/moq-archive/src/store.rs @@ -146,6 +146,9 @@ impl Store { } /// Create `.info`, or accept an existing object with the same parsed properties. + /// + /// An existing recording in an older format is refused with [`Error::Version`]: a reader + /// still reads it, but nothing appends to it. pub async fn put_info(&self, track: &str, info: &Info) -> Result { let key = Key::info(track)?; let path = self.path(&key)?; @@ -155,6 +158,9 @@ impl Store { Create::Exists => { let existing = self.get_bytes(&path).await?; let parsed = Info::decode(&existing)?; + if parsed.version != info.version { + return Err(Error::Version(parsed.version)); + } if parsed.priority != info.priority { return Err(Error::Priority { existing: parsed.priority, @@ -409,7 +415,7 @@ mod tests { .inner() .put( &path, - br#"{ "timescale": 1000, "priority": 1, "version": 2 }"#.as_ref().into(), + br#"{ "timescale": 1000, "priority": 1, "version": 3 }"#.as_ref().into(), ) .await .unwrap(); @@ -420,7 +426,7 @@ mod tests { .bytes() .await .unwrap(); - assert_eq!(&kept[..], br#"{ "timescale": 1000, "priority": 1, "version": 2 }"#); + assert_eq!(&kept[..], br#"{ "timescale": 1000, "priority": 1, "version": 3 }"#); } #[tokio::test] @@ -433,8 +439,11 @@ mod tests { &br#"{"version":1,"priority":0,"timescale":1000}"#[..], (|err| matches!(err, Error::Version(1))) as fn(&Error) -> bool, ), + ("v2", br#"{"version":2,"priority":128,"timescale":1000}"#, |err| { + matches!(err, Error::Version(2)) + }), ("junk", b"not json", |err| matches!(err, Error::Json(_))), - ("zero", br#"{"version":2,"priority":0,"timescale":0}"#, |err| { + ("zero", br#"{"version":3,"priority":0,"timescale":0}"#, |err| { matches!(err, Error::Timescale(0)) }), ] { diff --git a/rs/moq-c/MIGRATE.md b/rs/moq-c/MIGRATE.md index 6335f6c8e98..165238a7348 100644 --- a/rs/moq-c/MIGRATE.md +++ b/rs/moq-c/MIGRATE.md @@ -4,6 +4,12 @@ How to update your code for each breaking change, newest first. ## Unreleased +### Priorities are signed (#6142) + +`moq_track_info.priority` and `moq_subscription.priority` are `int8_t`, and `moq_group_request_priority` writes an `int8_t`, which changes the ABI. +Higher is first and 0 is the unset midpoint, so a zeroed `moq_track_info` now uses the default priority rather than the least urgent one. +To keep a value's moq-lite byte, subtract 128 from it: 255 becomes 127, 128 becomes 0, and 0 becomes -128. + ### A client asks only about what it watches (#6165) On moq-lite, a client session asks its relay for announcements only under paths something watches, through `moq_origin_announced` or `moq_origin_announced_broadcast`, instead of under its whole scope at connect. diff --git a/rs/moq-c/README.md b/rs/moq-c/README.md index 127ddea1642..57779201691 100644 --- a/rs/moq-c/README.md +++ b/rs/moq-c/README.md @@ -105,7 +105,7 @@ int32_t moq_track_request_audio(uint32_t request, const moq_audio_init *config); int32_t moq_track_request_abort(uint32_t request, uint16_t error_code); int32_t moq_track_request_free(uint32_t request); int32_t moq_group_request_sequence(uint32_t request, uint64_t *dst); -int32_t moq_group_request_priority(uint32_t request, uint8_t *dst); +int32_t moq_group_request_priority(uint32_t request, int8_t *dst); int32_t moq_group_request_frame_start(uint32_t request, uint64_t *dst); int32_t moq_group_request_accept(uint32_t request); int32_t moq_group_request_abort(uint32_t request, uint16_t error_code); diff --git a/rs/moq-c/src/api.rs b/rs/moq-c/src/api.rs index 0ff9e6c2550..647f3e1f37b 100644 --- a/rs/moq-c/src/api.rs +++ b/rs/moq-c/src/api.rs @@ -521,14 +521,14 @@ pub struct moq_datagram { /// Publisher-side raw track properties. /// -/// A null [moq_publish_track] `info` pointer uses the moq-net defaults. -/// A zero-initialized struct also uses those defaults, except `priority`, which -/// has no presence flag: zero is the least urgent, and 127 is the moq-net default. +/// A null [moq_publish_track] `info` pointer uses the moq-net defaults, and so does a +/// zero-initialized struct. #[repr(C)] #[allow(non_camel_case_types)] pub struct moq_track_info { /// Priority, used to break ties between subscriptions of equal subscriber priority. - pub priority: u8, + /// Higher is more urgent; the default 0 is the midpoint. + pub priority: i8, /// Maximum age of a non-latest group before the publisher evicts it, in microseconds. /// The publisher-side half of `moq_subscription.max_delay_us`. @@ -597,8 +597,9 @@ pub enum moq_demand { #[repr(C)] #[allow(non_camel_case_types)] pub struct moq_subscription { - /// Delivery priority. Higher values preempt lower ones under contention. - pub priority: u8, + /// Delivery priority. Higher values preempt lower ones under contention; the default 0 is + /// the midpoint. + pub priority: i8, /// How far a non-latest group may fall behind the live edge before it is skipped, in microseconds. /// Zero skips immediately. Enforced by the publisher's cache and by any local buffering. @@ -3156,9 +3157,9 @@ pub unsafe extern "C" fn moq_group_request_sequence(request: u32, dst: *mut u64) /// Returns a zero on success, or a negative code on failure. /// /// # Safety -/// - `dst` must point at a writable `uint8_t`. +/// - `dst` must point at a writable `int8_t`. #[unsafe(no_mangle)] -pub unsafe extern "C" fn moq_group_request_priority(request: u32, dst: *mut u8) -> i32 { +pub unsafe extern "C" fn moq_group_request_priority(request: u32, dst: *mut i8) -> i32 { ffi::enter(move || { let request = ffi::parse_id(request)?; let dst = unsafe { dst.as_mut() }.ok_or(Error::InvalidPointer)?; diff --git a/rs/moq-c/src/consume.rs b/rs/moq-c/src/consume.rs index 8e46e7f44c5..e6abd8a1385 100644 --- a/rs/moq-c/src/consume.rs +++ b/rs/moq-c/src/consume.rs @@ -53,7 +53,7 @@ impl ConsumeCatalog { /// A catalog media section whose renditions are consumed by index. pub(crate) trait Media: hang::catalog::Reference + Clone { /// The delivery priority its tracks subscribe with. - const PRIORITY: u8; + const PRIORITY: i8; /// Its renditions, keyed by ID. fn renditions(catalog: &moq_mux::catalog::hang::Catalog) -> &BTreeMap; @@ -63,7 +63,7 @@ pub(crate) trait Media: hang::catalog::Reference + Clone { } impl Media for hang::catalog::VideoConfig { - const PRIORITY: u8 = hang::catalog::PRIORITY.video; + const PRIORITY: i8 = hang::catalog::PRIORITY.video; fn renditions(catalog: &moq_mux::catalog::hang::Catalog) -> &BTreeMap { &catalog.video.renditions @@ -75,7 +75,7 @@ impl Media for hang::catalog::VideoConfig { } impl Media for hang::catalog::AudioConfig { - const PRIORITY: u8 = hang::catalog::PRIORITY.audio; + const PRIORITY: i8 = hang::catalog::PRIORITY.audio; fn renditions(catalog: &moq_mux::catalog::hang::Catalog) -> &BTreeMap { &catalog.audio.renditions diff --git a/rs/moq-c/src/publish.rs b/rs/moq-c/src/publish.rs index 55aeee33ee8..9e1f14a074f 100644 --- a/rs/moq-c/src/publish.rs +++ b/rs/moq-c/src/publish.rs @@ -816,7 +816,7 @@ impl Publish { } /// The sequence, priority, and first frame of a requested group. - pub fn group_request_info(&self, request: Id) -> Result<(u64, u8, u64), Error> { + pub fn group_request_info(&self, request: Id) -> Result<(u64, i8, u64), Error> { let request = self.group_request.get(request).ok_or(Error::NotFound)?; Ok((request.sequence(), request.priority(), request.frame_start())) } diff --git a/rs/moq-c/src/test.rs b/rs/moq-c/src/test.rs index 828a2b2c1a7..2e679bc5e65 100644 --- a/rs/moq-c/src/test.rs +++ b/rs/moq-c/src/test.rs @@ -2844,7 +2844,7 @@ fn track_demand_reports_the_wanted_group() { let broadcast = publish_broadcast(origin, b"track-demand-wanted"); let name = b"data"; let info = moq_track_info { - priority: 127, + priority: 0, max_age_us: 0, max_age_present: false, timescale: 0, @@ -3171,7 +3171,7 @@ fn fetch_group( broadcast: u32, name: &str, sequence: u64, - priority: u8, + priority: i8, frame_start: u64, ) -> mpsc::Receiver), moq_net::Error>> { let consumer = State::lock() @@ -3212,7 +3212,7 @@ fn track_dynamic_serves_a_fetch_miss() { let fetch = fetch_group(broadcast, "data", 5, 11, 0); let request = id(group_cb.recv()); let mut sequence = 0u64; - let mut priority = 0u8; + let mut priority = 0i8; let mut frame_start = 1u64; assert_eq!(unsafe { moq_group_request_sequence(request, &mut sequence) }, 0); assert_eq!(unsafe { moq_group_request_priority(request, &mut priority) }, 0); diff --git a/rs/moq-cli/src/report.rs b/rs/moq-cli/src/report.rs index d1a95ef3797..b6553eddb6f 100644 --- a/rs/moq-cli/src/report.rs +++ b/rs/moq-cli/src/report.rs @@ -38,7 +38,7 @@ const MAX_REPORT: usize = 1024 * 1024; const DRAIN: Duration = Duration::from_secs(5); /// Below every media track, so a report never delays the media it describes. -const PRIORITY: u8 = 0; +const PRIORITY: i8 = i8::MIN; /// A snapshot track and its compressed `.z` sibling, written together. Either is `None` once a /// write failed and closed it. diff --git a/rs/moq-ffi/MIGRATE.md b/rs/moq-ffi/MIGRATE.md index 124800378b7..be16b1d0ccd 100644 --- a/rs/moq-ffi/MIGRATE.md +++ b/rs/moq-ffi/MIGRATE.md @@ -5,6 +5,12 @@ These are the UniFFI names; each language wrapper has its own `MIGRATE.md` in it ## Unreleased +### Priorities are signed (#6142) + +`MoqTrackInfo.priority`, `MoqSubscription.priority`, `MoqFetchGroupOptions.priority`, and `MoqGroupRequest.priority()` are `i8`, higher first, and 0 is the unset midpoint. +`MoqTrackInfo.priority` defaulted to 127 and now defaults to 0. +To keep a value's moq-lite byte, subtract 128 from it: 255 becomes 127, 128 becomes 0, and 0 becomes -128. + ### A client asks only about what it watches (#6165) On moq-lite, a client session asks its relay for announcements only under paths something watches, through `announced` or `announced_broadcast`, instead of under its whole scope at connect. diff --git a/rs/moq-ffi/src/bench.rs b/rs/moq-ffi/src/bench.rs index adf13710738..0d96b97e224 100644 --- a/rs/moq-ffi/src/bench.rs +++ b/rs/moq-ffi/src/bench.rs @@ -240,7 +240,7 @@ async fn rust_ffi() { }); let broadcast = origin.create_broadcast("bench".into()).unwrap(); let timed = MoqTrackInfo { - priority: 127, + priority: 0, max_age_us: None, refresh: None, timescale: Some(1_000_000), diff --git a/rs/moq-ffi/src/consumer.rs b/rs/moq-ffi/src/consumer.rs index 2a5c6c3e204..11e0c469509 100644 --- a/rs/moq-ffi/src/consumer.rs +++ b/rs/moq-ffi/src/consumer.rs @@ -28,8 +28,9 @@ fn raw_frame(frame: moq_net::frame::Frame) -> Result { #[derive(Clone, uniffi::Record)] pub struct MoqSubscription { /// Delivery priority; higher values preempt lower ones under bandwidth contention. + /// The default 0 is the midpoint. #[uniffi(default = 0)] - pub priority: u8, + pub priority: i8, /// Maximum age of a non-latest group before it is skipped, in microseconds. /// `0` skips immediately; a larger value tolerates that much reordering. /// @@ -132,8 +133,9 @@ impl From for moq_net::track::Order { #[derive(Clone, uniffi::Record)] pub struct MoqFetchGroupOptions { /// Delivery priority for the fetch stream; higher values preempt lower ones. + /// The default 0 is the midpoint. #[uniffi(default = 0)] - pub priority: u8, + pub priority: i8, /// Maximum age behind the live edge in microseconds; null uses publisher retention alone. #[uniffi(default = None)] pub max_delay_us: Option, diff --git a/rs/moq-ffi/src/producer.rs b/rs/moq-ffi/src/producer.rs index 401ac753d3d..cc78d7529fd 100644 --- a/rs/moq-ffi/src/producer.rs +++ b/rs/moq-ffi/src/producer.rs @@ -13,13 +13,13 @@ use crate::media::MoqFrame; /// Publisher-side track properties, mirroring [`moq_net::track::Info`]. /// /// Construct with the fields you care about; the rest use raw-track defaults -/// (priority 127, no publisher age limit, untimed). +/// (priority 0, no publisher age limit, untimed). #[derive(Clone, uniffi::Record)] pub struct MoqTrackInfo { /// Priority, used only to break ties between subscriptions of equal subscriber priority. - /// Higher is more urgent; the default 127 is the midpoint. - #[uniffi(default = 127)] - pub priority: u8, + /// Higher is more urgent; the default 0 is the midpoint. + #[uniffi(default = 0)] + pub priority: i8, /// Maximum age of a non-latest group before the publisher evicts it, in /// microseconds. Null imposes no publisher age limit. This is the publisher-side half of /// [`MoqSubscription::max_delay_us`](crate::consumer::MoqSubscription::max_delay_us). @@ -485,7 +485,7 @@ impl MoqTrackDynamic { #[derive(uniffi::Object)] pub struct MoqGroupRequest { sequence: u64, - priority: u8, + priority: i8, inner: std::sync::Mutex>, } @@ -511,7 +511,7 @@ impl MoqGroupRequest { } /// The consumer's delivery priority for this fetch. - pub fn priority(&self) -> u8 { + pub fn priority(&self) -> i8 { self.priority } diff --git a/rs/moq-ffi/src/test.rs b/rs/moq-ffi/src/test.rs index ee5132d4e7e..1a1ff215d64 100644 --- a/rs/moq-ffi/src/test.rs +++ b/rs/moq-ffi/src/test.rs @@ -31,7 +31,7 @@ fn timed() -> moq_net::track::Info { /// An explicitly timed raw track, whose frame API accepts microsecond timestamps. fn timed_info() -> MoqTrackInfo { MoqTrackInfo { - priority: 127, + priority: 0, max_age_us: None, refresh: None, timescale: Some(1_000_000), @@ -944,7 +944,7 @@ async fn data_writes_preserve_optional_timestamps() { .publish_track( "data".into(), Some(MoqTrackInfo { - priority: 127, + priority: 0, max_age_us: None, refresh: None, timescale: timestamp_us.map(|_| 1_000_000), @@ -1086,7 +1086,7 @@ async fn json_snapshot_roundtrip() { compression: true, }; let info = MoqTrackInfo { - priority: 127, + priority: 0, max_age_us: None, timescale: Some(1_000_000), refresh: None, diff --git a/rs/moq-mux/MIGRATE.md b/rs/moq-mux/MIGRATE.md index ff058f700a2..6b367a8d6f6 100644 --- a/rs/moq-mux/MIGRATE.md +++ b/rs/moq-mux/MIGRATE.md @@ -4,6 +4,10 @@ How to update your code for each breaking change, newest first. ## Unreleased +### Priorities are signed (#6142) + +`track_info` on the catalog producer and tracks takes an `i8` priority, higher first, with 0 unset; pass `hang::catalog::PRIORITY` as before. + ### fMP4 import unwraps to Legacy (#6250) `container::fmp4::Import` now publishes one `Legacy` frame per sample instead of one `Cmaf` frame per fragment. diff --git a/rs/moq-mux/src/catalog/producer.rs b/rs/moq-mux/src/catalog/producer.rs index 01c286a7638..e0e236c1023 100644 --- a/rs/moq-mux/src/catalog/producer.rs +++ b/rs/moq-mux/src/catalog/producer.rs @@ -483,7 +483,7 @@ impl Producer { /// `priority` should come from [`PRIORITY`](hang::catalog::PRIORITY) for the kind of media /// the track carries. Chain [`with_timescale`](moq_net::track::Info::with_timescale) for a /// container that carries the source's own scale (CMAF and Matroska both do). - pub fn track_info(&self, priority: u8) -> moq_net::track::Info { + pub fn track_info(&self, priority: i8) -> moq_net::track::Info { let info = hang::container::track_info(priority); match self.max_age { Some(max_age) => info.with_max_age(max_age), diff --git a/rs/moq-mux/src/catalog/tracks.rs b/rs/moq-mux/src/catalog/tracks.rs index 5e1e447eaa6..cf5d62fff15 100644 --- a/rs/moq-mux/src/catalog/tracks.rs +++ b/rs/moq-mux/src/catalog/tracks.rs @@ -396,7 +396,7 @@ impl Reserved { /// Track properties for a media track under this catalog, carrying any retention it declares. /// See [`Producer::track_info`](super::Producer::track_info). - pub fn track_info(&self, priority: u8) -> moq_net::track::Info { + pub fn track_info(&self, priority: i8) -> moq_net::track::Info { self.timebase.catalog.track_info(priority) } diff --git a/rs/moq-mux/src/container/fmp4/mod.rs b/rs/moq-mux/src/container/fmp4/mod.rs index 91b5c3802db..f7202402b35 100644 --- a/rs/moq-mux/src/container/fmp4/mod.rs +++ b/rs/moq-mux/src/container/fmp4/mod.rs @@ -332,7 +332,7 @@ impl Kind { } /// The publisher priority for this kind of media. - fn priority(self) -> u8 { + fn priority(self) -> i8 { match self { Self::Video => hang::catalog::PRIORITY.video, Self::Audio => hang::catalog::PRIORITY.audio, diff --git a/rs/moq-mux/src/container/mkv/import.rs b/rs/moq-mux/src/container/mkv/import.rs index 46d95718e63..00a538667bc 100644 --- a/rs/moq-mux/src/container/mkv/import.rs +++ b/rs/moq-mux/src/container/mkv/import.rs @@ -85,7 +85,7 @@ enum TrackKind { impl TrackKind { /// The publisher priority for this kind of media, so audio isn't stuck behind a /// video backlog on a busy connection. - fn priority(&self) -> u8 { + fn priority(&self) -> i8 { match self { Self::Video => hang::catalog::PRIORITY.video, Self::Audio => hang::catalog::PRIORITY.audio, diff --git a/rs/moq-mux/src/import/track.rs b/rs/moq-mux/src/import/track.rs index 4647726e3ce..582d4275fa1 100644 --- a/rs/moq-mux/src/import/track.rs +++ b/rs/moq-mux/src/import/track.rs @@ -27,7 +27,7 @@ fn video_hint(init: &VideoInit, default_codec: Option /// as the moq-hang draft requires, so each frame timestamp lands on a media tick. fn track_info( reserved: &crate::catalog::Reserved, - priority: u8, + priority: i8, container: &hang::catalog::Container, ) -> Result { let info = reserved.track_info(priority); diff --git a/rs/moq-net/MIGRATE.md b/rs/moq-net/MIGRATE.md index 1653e37cc01..6023c18cb9e 100644 --- a/rs/moq-net/MIGRATE.md +++ b/rs/moq-net/MIGRATE.md @@ -4,6 +4,15 @@ How to update your code for each breaking change, newest first. ## Unreleased +### Priorities are signed, with 0 unset (#6142) + +`track::Info::priority`, `track::Subscription::priority`, `group::Fetch::priority`, and `group::Request::priority()` are `i8`, higher first, and 0 is the unset midpoint. +The track's default was 127, the midpoint, and the subscription's was 0, the least urgent; both are 0 now. +To keep a value's moq-lite byte, subtract 128 from it: 255 becomes 127, 128 becomes 0, and 0 becomes -128. +The publisher's `Info::priority` now breaks ties between subscriptions at one subscriber priority. + +On the wire, moq-lite carries `priority + 128` and moq-transport `127 - priority`, so an unset priority goes out as 128 on moq-lite (was 127) and 127 on moq-transport (was 128), and an absent moq-transport priority reads as 0. + ### A client session asks only about what its readers watch (#6165) On moq-lite, a client session no longer asks its peer for announcements under its whole subscribe scope at connect. diff --git a/rs/moq-net/benches/priority.rs b/rs/moq-net/benches/priority.rs index 0bb25f0914b..176f356bd3b 100644 --- a/rs/moq-net/benches/priority.rs +++ b/rs/moq-net/benches/priority.rs @@ -21,7 +21,7 @@ const OVERFLOW_DEPTHS: [usize; 3] = [256, 1_024, 4_096]; fn filled(depth: usize) -> (PriorityQueue, Vec) { let queue = PriorityQueue::default(); let handles = (0..depth) - .map(|group| queue.insert(Priority::new(100, 0, group as u64))) + .map(|group| queue.insert(Priority::new(-28, 0, group as u64))) .collect(); (queue, handles) } @@ -33,7 +33,7 @@ fn bench_insert(c: &mut Criterion) { b.iter_batched_ref( || filled(depth), |(queue, handles)| { - handles.push(queue.insert(Priority::new(u8::MAX, 0, u64::MAX))); + handles.push(queue.insert(Priority::new(i8::MAX, 0, u64::MAX))); }, BatchSize::SmallInput, ); @@ -45,9 +45,9 @@ fn bench_insert(c: &mut Criterion) { fn overflow(depth: usize) -> Vec { let queue = PriorityQueue::default(); let mut handles: Vec<_> = (0..255) - .map(|group| queue.insert(Priority::new(200, 0, group))) + .map(|group| queue.insert(Priority::new(72, 0, group))) .collect(); - handles.extend((255..depth as u64).map(|group| queue.insert(Priority::new(100, 0, group)))); + handles.extend((255..depth as u64).map(|group| queue.insert(Priority::new(-28, 0, group)))); handles } diff --git a/rs/moq-net/benches/schedule.rs b/rs/moq-net/benches/schedule.rs index e02c647e6c1..d049f4729f6 100644 --- a/rs/moq-net/benches/schedule.rs +++ b/rs/moq-net/benches/schedule.rs @@ -49,7 +49,7 @@ const PACKET: usize = 1200; /// `cadence` ticks, numbered from `first` in steps of `step`. #[derive(Clone, Copy)] struct Track { - priority: u8, + priority: i8, rate: usize, cadence: u64, first: u64, @@ -57,7 +57,7 @@ struct Track { } impl Track { - const fn new(priority: u8, rate: usize, cadence: u64) -> Self { + const fn new(priority: i8, rate: usize, cadence: u64) -> Self { Self { priority, rate, @@ -349,7 +349,7 @@ impl Sim { } fn report(&self, workload: &str) { - let priorities: Vec = { + let priorities: Vec = { let mut p: Vec<_> = self.tracks.iter().map(|t| t.priority).collect(); p.sort(); p.dedup(); diff --git a/rs/moq-net/benches/track.rs b/rs/moq-net/benches/track.rs index f471d193563..013222aa756 100644 --- a/rs/moq-net/benches/track.rs +++ b/rs/moq-net/benches/track.rs @@ -724,13 +724,13 @@ fn bench_subscriber_preferences(c: &mut Criterion) { let held = |i: usize| { let i = i as u64; track::Subscription::default() - .with_priority((i % 255) as u8) + .with_priority(((i % 255) as i16 - 128) as i8) .with_max_delay(Duration::from_micros(i)) .with_groups(100 + i..200 + i) .with_order(track::Order::Ascending) }; let extreme = track::Subscription::default() - .with_priority(255) + .with_priority(i8::MAX) .with_max_delay(Duration::from_secs(10)); let mut group = c.benchmark_group("track_subscriber_preferences"); for steady in STEADY { diff --git a/rs/moq-net/src/ietf/fetch.rs b/rs/moq-net/src/ietf/fetch.rs index aff3d5cb227..5174df19208 100644 --- a/rs/moq-net/src/ietf/fetch.rs +++ b/rs/moq-net/src/ietf/fetch.rs @@ -311,7 +311,7 @@ impl Message for Fetch<'_> { Ok(Self { request_id, - subscriber_priority: subscriber_priority.unwrap_or(128), + subscriber_priority: subscriber_priority.unwrap_or(super::priority::ABSENT_WIRE), // No preference: the publisher picks the order. group_order: group_order.unwrap_or(GroupOrder::Any), fetch_type, diff --git a/rs/moq-net/src/ietf/group.rs b/rs/moq-net/src/ietf/group.rs index 5e27e089b33..7ddc17b82ca 100644 --- a/rs/moq-net/src/ietf/group.rs +++ b/rs/moq-net/src/ietf/group.rs @@ -324,7 +324,7 @@ impl Decode for GroupHeader { let publisher_priority = if flags.has_priority { r.u8()? } else { - 128 // Default priority when absent + super::priority::ABSENT_WIRE }; let result = Self { diff --git a/rs/moq-net/src/ietf/parameters.rs b/rs/moq-net/src/ietf/parameters.rs index 976a4c05e6e..b0301456d83 100644 --- a/rs/moq-net/src/ietf/parameters.rs +++ b/rs/moq-net/src/ietf/parameters.rs @@ -560,7 +560,7 @@ macro_rules! encode_params { /// 0x20 => subscriber_priority: Option, /// ); /// // forward: Option and subscriber_priority: Option are now in scope -/// let subscriber_priority = subscriber_priority.unwrap_or(128); +/// let subscriber_priority = subscriber_priority.unwrap_or(super::priority::ABSENT_WIRE); /// ``` macro_rules! decode_params { ($r:expr, $version:expr, $($key:expr => $name:ident: $ty:ty $(where $gate:expr)?),* $(,)?) => { diff --git a/rs/moq-net/src/ietf/priority.rs b/rs/moq-net/src/ietf/priority.rs index abc97c9ac00..7a3cf596178 100644 --- a/rs/moq-net/src/ietf/priority.rs +++ b/rs/moq-net/src/ietf/priority.rs @@ -1,13 +1,23 @@ //! Priority conversion between the IETF wire and the MoQ model. +//! +//! The wire is lower first, so `127 - priority` keeps one urgency with moq-lite's +//! `priority + 128`: the IETF byte is always `255 -` the lite byte. The unset priority 0 +//! goes out as 127, one step off the draft's usual 128, to keep that. -/// Convert an IETF subscriber priority into the model's higher-first priority. -pub(super) const fn from_wire(priority: u8) -> u8 { - u8::MAX - priority +/// The model priority an absent IETF priority stands for: the unset default. +pub(super) const ABSENT: i8 = 0; + +/// The wire byte of [`ABSENT`], for a message that must carry a priority it was not given. +pub(super) const ABSENT_WIRE: u8 = to_wire(ABSENT); + +/// Convert an IETF priority into the model's higher-first priority. +pub(super) const fn from_wire(priority: u8) -> i8 { + (127 - priority as i16) as i8 } -/// Convert the model's higher-first priority into an IETF subscriber priority. -pub(super) const fn to_wire(priority: u8) -> u8 { - u8::MAX - priority +/// Convert the model's higher-first priority into an IETF priority. +pub(super) const fn to_wire(priority: i8) -> u8 { + (127 - priority as i16) as u8 } #[cfg(test)] @@ -16,15 +26,33 @@ mod tests { #[test] fn ietf_priority_is_lower_first() { - assert_eq!(from_wire(0), u8::MAX); - assert_eq!(from_wire(u8::MAX), 0); - assert_eq!(to_wire(u8::MAX), 0); - assert_eq!(to_wire(0), u8::MAX); + assert_eq!(from_wire(0), i8::MAX); + assert_eq!(from_wire(u8::MAX), i8::MIN); + assert_eq!(to_wire(i8::MAX), 0); + assert_eq!(to_wire(i8::MIN), u8::MAX); assert!((0u8..u8::MAX).all(|priority| from_wire(priority) > from_wire(priority + 1))); } + /// The unset default goes out as 127, and an absent priority decodes to it. + #[test] + fn unset_is_127() { + assert_eq!(to_wire(0), 127); + assert_eq!(from_wire(127), 0); + assert_eq!(ABSENT, 0); + assert_eq!(ABSENT_WIRE, 127); + } + + /// One urgency on both wires: the IETF byte mirrors the moq-lite byte. + #[test] + fn mirrors_lite() { + for priority in i8::MIN..=i8::MAX { + assert_eq!(to_wire(priority), u8::MAX - crate::lite::priority::to_wire(priority)); + } + } + #[test] fn priority_round_trips() { - assert!((0u8..=u8::MAX).all(|priority| from_wire(to_wire(priority)) == priority)); + assert!((i8::MIN..=i8::MAX).all(|priority| from_wire(to_wire(priority)) == priority)); + assert!((0u8..=u8::MAX).all(|priority| to_wire(from_wire(priority)) == priority)); } } diff --git a/rs/moq-net/src/ietf/publish.rs b/rs/moq-net/src/ietf/publish.rs index 6c5bfc4de72..94f8d0d1849 100644 --- a/rs/moq-net/src/ietf/publish.rs +++ b/rs/moq-net/src/ietf/publish.rs @@ -422,7 +422,7 @@ impl Message for PublishOk { match version { Version::Draft14 => { w.bool(self.forward); - w.u8(self.subscriber_priority.unwrap_or(128)); + w.u8(self.subscriber_priority.unwrap_or(super::priority::ABSENT_WIRE)); self.group_order.unwrap_or(GroupOrder::Descending).encode(w, version)?; // Same as SUBSCRIBE: the Location an absolute filter carries is dropped on // decode, so encoding one would truncate the message. diff --git a/rs/moq-net/src/ietf/publisher.rs b/rs/moq-net/src/ietf/publisher.rs index 75d2ae64de3..d697db48076 100644 --- a/rs/moq-net/src/ietf/publisher.rs +++ b/rs/moq-net/src/ietf/publisher.rs @@ -184,7 +184,7 @@ async fn read_fetch( sequence: u64, skip: u64, until: Option, - priority: u8, + priority: i8, compression: bool, ) -> Result { let info = track.query().await?; @@ -2449,12 +2449,14 @@ where // Its own entry in the queue, or its own send group, so it competes with group // streams by its priority alone, its request id breaking ties in the queue. - // Nothing updates a FETCH's priority. + // Nothing updates a FETCH's priority, and it ranks before the track's info + // resolves, so the publisher's priority is the unset default. let ranking = Ranking { queue: self.priority.clone(), - group: self.session.clone().send_group(priority.into()), + group: self.session.clone().send_group(crate::priority::group(priority, 0)), request_id: msg.request_id, priority: kio::Producer::new(priority).consume(), + publisher: 0, order: Order::Descending, }; (track, start, end, false, ranking, gate) @@ -4320,7 +4322,7 @@ struct TrackServe { /// The groups with a stream, serving or awaiting the acknowledgement of their FIN. children: kio::Tasks>, /// The subscriber priority, which every group stream follows while it is open. - priority: kio::Producer, + priority: kio::Producer, /// Where the group streams are ranked: the subscription's send group, or the session's /// queue once [`Self::ranked`] on a transport without groups. ranking: Ranking>, @@ -4369,12 +4371,14 @@ impl TrackServe { let timescale = timescale.filter(|_| ietf::Properties::sends_timescale(version)); let dynamic = track.info().refresh.is_some(); let priority = kio::Producer::new(track.subscription().priority); + let publisher = track.info().priority; let mut session = session; let ranking = Ranking { queue: PriorityQueue::default(), - group: session.send_group(track.subscription().priority.into()), + group: session.send_group(crate::priority::group(track.subscription().priority, publisher)), request_id, priority: priority.consume(), + publisher, order: track.subscription().order, }; @@ -4414,7 +4418,7 @@ impl TrackServe { } /// Move the subscriber priority, for the group streams already open as well as later ones. - fn set_priority(&mut self, priority: u8) -> Result<(), Error> { + fn set_priority(&mut self, priority: i8) -> Result<(), Error> { let mut subscription = self.track.subscription(); subscription.priority = priority; self.track.update(subscription)?; @@ -4424,7 +4428,7 @@ impl TrackServe { } // One move for the whole subscription; its streams keep their place within it. if let Some(group) = &mut self.ranking.group { - group.set_priority(priority.into()); + group.set_priority(crate::priority::group(priority, self.ranking.publisher)); } Ok(()) } @@ -4759,7 +4763,10 @@ struct Ranking { /// The request's send group, which replaces the queue on a transport that has them. group: Option, request_id: RequestId, - priority: kio::Consumer, + priority: kio::Consumer, + /// The publisher's priority for the track, breaking ties between requests at one + /// subscriber priority. + publisher: i8, /// The request's GROUP_ORDER, fixed for its life: no update carries it. order: Order, } @@ -4777,6 +4784,7 @@ impl Ranking { Rank::Queue(Queued { handle: self.queue.insert( Priority::new(applied, self.request_id.0, start.group) + .with_publisher(self.publisher) .with_order(self.order) .with_start(start.object), ), @@ -4882,7 +4890,7 @@ enum Rank { impl Rank { /// A group ranked only against itself, for driving one group machine alone. #[cfg(test)] - fn solo(priority: kio::Consumer) -> Self { + fn solo(priority: kio::Consumer) -> Self { Self::Queue(Queued::solo(priority)) } @@ -4913,9 +4921,9 @@ impl Rank { struct Queued { handle: PriorityHandle, /// The subscription's priority, followed while the stream is open. - priority: kio::Consumer, + priority: kio::Consumer, /// The priority last applied to [`Self::handle`]. - applied: u8, + applied: i8, /// The subscription's GROUP_ORDER, fixed for its life: no update carries it. order: Order, } @@ -4923,7 +4931,7 @@ struct Queued { impl Queued { /// A group ranked only against itself, for driving one group machine alone. #[cfg(test)] - fn solo(priority: kio::Consumer) -> Self { + fn solo(priority: kio::Consumer) -> Self { let applied = *priority.read(); Self { handle: PriorityQueue::default().insert(Priority::new(applied, 0, 0)), @@ -5513,7 +5521,7 @@ mod group_priority_test { None, ) .ranked(queue.clone()); - let _higher = queue.insert(Priority::new(u8::MAX, 0, 0)); + let _higher = queue.insert(Priority::new(i8::MAX, 0, 0)); serve.write_end_of_track(track::Position::group(0)).await.unwrap(); assert_eq!(log.priorities(), [254], "second, behind the higher priority"); } @@ -5697,7 +5705,7 @@ mod group_priority_test { } /// A group of `subscribe` ranked in `queue` at `priority`'s current value. - fn rank_in(queue: &PriorityQueue, priority: &kio::Producer, subscribe: u64) -> Rank { + fn rank_in(queue: &PriorityQueue, priority: &kio::Producer, subscribe: u64) -> Rank { let applied = *priority.read(); Rank::Queue(Queued { handle: queue.insert(Priority::new(applied, subscribe, 0)), @@ -5736,8 +5744,8 @@ mod group_priority_test { }; // Queue both before either opens, so each stream is handed its final rank. - let high = kio::Producer::new(200); - let low = kio::Producer::new(100); + let high = kio::Producer::new(72); + let low = kio::Producer::new(-28); let mut serves = [(&low, 1), (&high, 0)].map(|(priority, subscribe)| { GroupServe::new( SinkSession::new(log.clone()), @@ -5778,12 +5786,13 @@ mod group_priority_test { ); } - /// A track that never set a priority is the draft's usual publisher priority, 128, - /// not 255, the least urgent value a peer like moxygen would deprioritize. + /// A track that never set a priority goes out at the midpoint, 127, one step off the + /// draft's usual 128 so it mirrors moq-lite's 128, and not 255, the least urgent value a + /// peer like moxygen would deprioritize. #[moq_clock::test] async fn group_header_defaults_to_the_midpoint() { let header = serve_group_header(track::Info::timed()).await; - assert_eq!(header.publisher_priority, 128); + assert_eq!(header.publisher_priority, 127); } /// Drafts 14-16 cannot send TIMESCALE, so a group served with a track timescale still @@ -5960,7 +5969,7 @@ mod group_priority_test { let waiter = kio::Waiter::noop(); assert!(serve.poll(&waiter).is_pending()); - assert_eq!(log.groups(), [(0, 3)]); + assert_eq!(log.groups(), [(0, crate::priority::group(3, 0))]); assert_eq!( log.joins(), [(0, 0), (1, 0)], @@ -5971,7 +5980,11 @@ mod group_priority_test { serve.set_priority(9).unwrap(); assert!(serve.poll(&waiter).is_pending()); - assert_eq!(log.groups(), [(0, 3), (0, 9)], "one move for the subscription"); + assert_eq!( + log.groups(), + [(0, crate::priority::group(3, 0)), (0, crate::priority::group(9, 0))], + "one move for the subscription" + ); assert_eq!(log.orders().len(), 2, "the streams keep their place in the group"); } @@ -6126,11 +6139,11 @@ mod group_priority_test { /// `hang::catalog::PRIORITY` isn't reachable from `moq-net` (hang depends on it, not the /// other way round), so the two ranks it publishes audio and video at are spelled here. - fn hang_audio_priority() -> u8 { + fn hang_audio_priority() -> i8 { 80 } - fn hang_video_priority() -> u8 { + fn hang_video_priority() -> i8 { 60 } @@ -6346,11 +6359,11 @@ mod group_priority_test { let consumer = group.consume(); group.finish().unwrap(); - // Another subscription's group at 150 sits in the same queue, so each update + // Another subscription's group at 22 sits in the same queue, so each update // moves this stream across it. let queue = PriorityQueue::default(); - let _other = queue.insert(Priority::new(150, 1, 0)); - let priority = kio::Producer::new(200); + let _other = queue.insert(Priority::new(22, 1, 0)); + let priority = kio::Producer::new(72); let mut serve = GroupServe::new( session, ietf::GroupHeader { @@ -6373,11 +6386,11 @@ mod group_priority_test { ); assert_eq!(log.priorities(), vec![255]); - *priority.write().ok().unwrap() = 100; + *priority.write().ok().unwrap() = -28; assert!(futures::poll!(serving.as_mut()).is_pending()); assert_eq!(log.priorities(), vec![255, 254]); - *priority.write().ok().unwrap() = 175; + *priority.write().ok().unwrap() = 47; assert!(futures::poll!(serving.as_mut()).is_pending()); assert_eq!(log.priorities(), vec![255, 254, 255]); } @@ -6893,7 +6906,7 @@ mod serve_tests { None, )); assert!(futures::poll!(run.as_mut()).is_pending()); - assert_eq!(h.track.subscription().unwrap().priority, 245); + assert_eq!(h.track.subscription().unwrap().priority, 117); assert_eq!(occurrences(&session.log, &[0x07, 0, 1, 0]), 2); } @@ -6941,7 +6954,7 @@ mod serve_tests { for _ in 0..4 { assert!(futures::poll!(run.as_mut()).is_pending()); } - assert_eq!(h.track.subscription().unwrap().priority, 245); + assert_eq!(h.track.subscription().unwrap().priority, 117); let after = h.log.priorities(); assert_eq!(after.last(), Some(&255), "above the other subscription: {after:?}"); } @@ -7359,7 +7372,7 @@ mod serve_tests { None, )); assert!(futures::poll!(run.as_mut()).is_pending(), "{version}: update ended it"); - assert_eq!(h.track.subscription().unwrap().priority, 245, "{version}"); + assert_eq!(h.track.subscription().unwrap().priority, 117, "{version}"); // Drafts 15 and 16 answer with REQUEST_OK naming the update; draft 14 answers nothing. let answered = occurrences(&session.log, &[ietf::RequestOk::ID as u8, 0, 2, 10, 0]); @@ -9135,16 +9148,16 @@ mod serve_tests { } /// Subscriber priority 128 on the wire, which every request in these tests carries. - const PRIORITY: u8 = 127; + const PRIORITY: i8 = super::super::priority::from_wire(128); /// The streams a FETCH or fill competes with: one at a higher priority and one at a lower /// one, plus two at its own priority on either side of `request_id`. fn rival_streams(queue: &PriorityQueue, request_id: RequestId) -> [PriorityHandle; 4] { [ - queue.insert(Priority::new(200, 1, 0)), + queue.insert(Priority::new(72, 1, 0)), queue.insert(Priority::new(PRIORITY, request_id.0 - 1, 0)), queue.insert(Priority::new(PRIORITY, request_id.0 + 1, 0)), - queue.insert(Priority::new(50, 2, 0)), + queue.insert(Priority::new(-78, 2, 0)), ] } @@ -9163,6 +9176,7 @@ mod serve_tests { group: None, request_id: RequestId(REQUEST_ID), priority: priority.consume(), + publisher: 0, order: Order::Descending, }; let [higher, _older, _later, _lower] = rival_streams(&queue, ranking.request_id); @@ -9190,7 +9204,7 @@ mod serve_tests { let Ok(mut current) = priority.write() else { unreachable!("this test holds the producer") }; - *current = 255; + *current = i8::MAX; drop(current); assert!(futures::poll!(fill.as_mut()).is_pending()); assert_eq!( @@ -9218,6 +9232,7 @@ mod serve_tests { group: None, request_id: RequestId(REQUEST_ID), priority: kio::Producer::new(PRIORITY).consume(), + publisher: 0, order: Order::Descending, }; // The live stream, from object 1 on, queued first so the slab id alone would favor it. @@ -10024,7 +10039,7 @@ mod serve_tests { async fn fetch_ok_carries_the_track_properties() { let info = track::Info { max_age: Some(Duration::from_secs(5)), - priority: 200, + priority: 72, timescale: Some(crate::Timescale::MILLI), ..Default::default() }; diff --git a/rs/moq-net/src/ietf/publisher/tracks.rs b/rs/moq-net/src/ietf/publisher/tracks.rs index 947fe1103cc..01f220e8aa7 100644 --- a/rs/moq-net/src/ietf/publisher/tracks.rs +++ b/rs/moq-net/src/ietf/publisher/tracks.rs @@ -281,7 +281,7 @@ where prefix: prefix.clone(), forward: msg.forward, params: Params { - priority: msg.subscriber_priority.unwrap_or(128), + priority: msg.subscriber_priority.unwrap_or(crate::ietf::priority::ABSENT_WIRE), order: msg.group_order, filter: msg.filter.unwrap_or(Filter::Unfiltered), max_age: None, diff --git a/rs/moq-net/src/ietf/subscribe.rs b/rs/moq-net/src/ietf/subscribe.rs index 51973791038..5fca49b1b10 100644 --- a/rs/moq-net/src/ietf/subscribe.rs +++ b/rs/moq-net/src/ietf/subscribe.rs @@ -148,7 +148,7 @@ impl Message for Subscribe<'_> { // A zero cap still distinguishes a requested wait from an immediate miss. let rendezvous = rendezvous_timeout.is_some_and(|millis| millis > 0); - let subscriber_priority = subscriber_priority.unwrap_or(128); + let subscriber_priority = subscriber_priority.unwrap_or(super::priority::ABSENT_WIRE); let group_order = group_order.unwrap_or(GroupOrder::Descending); // An absent LOCATION_FILTER means the subscription is unfiltered. let filter = filter.unwrap_or(Filter::Unfiltered); @@ -448,7 +448,7 @@ impl Message for SubscribeUpdate { .encode(w, version)?; self.start_location.encode(w, version)?; w.varint(self.end_group)?; - w.u8(self.subscriber_priority.unwrap_or(128)); + w.u8(self.subscriber_priority.unwrap_or(super::priority::ABSENT_WIRE)); w.bool(self.forward.unwrap_or(true)); encode_params!(w, version, 0x03 => self.authorization_token.clone(), diff --git a/rs/moq-net/src/ietf/subscriber.rs b/rs/moq-net/src/ietf/subscriber.rs index a4e3a4b4ce4..2dbe854771a 100644 --- a/rs/moq-net/src/ietf/subscriber.rs +++ b/rs/moq-net/src/ietf/subscriber.rs @@ -481,7 +481,11 @@ fn accepted_info(properties: &ietf::Properties, version: Version) -> track::Info .with_compress(properties.compress) .with_timescale(properties.timescale) .with_max_age(properties.max_cache_duration) - .with_priority(super::priority::from_wire(properties.priority.unwrap_or(128))) + .with_priority( + properties + .priority + .map_or(super::priority::ABSENT, super::priority::from_wire), + ) // A publisher answering NEW_GROUP_REQUEST refreshes on any join with a delta to skip; // this session carries the request back to it. .with_refresh(properties.dynamic_groups.then_some(0)); @@ -7821,7 +7825,7 @@ mod tests { }); settle().await; - let _high = track.subscribe(track::Subscription::default().with_priority(200)); + let _high = track.subscribe(track::Subscription::default().with_priority(72)); settle().await; let writes = log.writes.lock().unwrap().clone(); @@ -7846,7 +7850,7 @@ mod tests { assert_eq!(update.subscription_request_id, subscribe); assert_eq!(update.start_location, ietf::Location { group: 5, object: 3 }); assert_eq!(update.end_group, 0, "still open ended"); - assert_eq!(update.subscriber_priority, Some(super::super::priority::to_wire(200))); + assert_eq!(update.subscriber_priority, Some(super::super::priority::to_wire(72))); assert_eq!(update.forward, Some(true)); } @@ -8062,7 +8066,7 @@ mod tests { .expect("the subscription is registered"); assert_eq!( track.producer.as_ref().unwrap().publisher_priority(), - 255 - priority.unwrap_or(128), + priority.map_or(0, super::super::priority::from_wire), "SUBSCRIBE_OK priority must be committed before alias registration" ); // The answer's largest is known before any group arrives. @@ -12014,7 +12018,7 @@ struct Update { /// Draft-14's mandatory Start Location, repeated as it stands; unused on later drafts. start_location: ietf::Location, /// The aggregate subscriber priority, in the model's higher-first terms. - priority: Option, + priority: Option, /// Subscriber age budget, in milliseconds. max_age: Option, /// NEW_GROUP_REQUEST. diff --git a/rs/moq-net/src/lite/fetch.rs b/rs/moq-net/src/lite/fetch.rs index 6472ed3108b..df57064f947 100644 --- a/rs/moq-net/src/lite/fetch.rs +++ b/rs/moq-net/src/lite/fetch.rs @@ -17,7 +17,7 @@ pub struct Fetch<'a> { /// Lite07+ only. pub epoch: Option, pub track: Cow<'a, str>, - pub priority: u8, + pub priority: i8, /// Reader freshness budget. Lite07+ only; omission uses publisher retention alone. pub max_delay: Option, pub group: u64, @@ -40,7 +40,7 @@ impl Message for Fetch<'_> { let broadcast = Path::decode(r, version)?; let epoch = super::epoch::decode_epoch(r, version)?; let track = Cow::Owned(r.string()?); - let priority = r.u8()?; + let priority = super::priority::from_wire(r.u8()?); let max_delay = match version.has_fetch_max_delay() { true => r.varint_opt()?.map(std::time::Duration::from_millis), false => None, @@ -79,7 +79,7 @@ impl Message for Fetch<'_> { self.broadcast.encode(w, version)?; super::epoch::encode_epoch(w, version, self.epoch.as_ref())?; w.string(&self.track)?; - w.u8(self.priority); + w.u8(super::priority::to_wire(self.priority)); match version.has_fetch_max_delay() { true => w.varint_opt( self.max_delay diff --git a/rs/moq-net/src/lite/mod.rs b/rs/moq-net/src/lite/mod.rs index c2453f181b8..6be258bd2ec 100644 --- a/rs/moq-net/src/lite/mod.rs +++ b/rs/moq-net/src/lite/mod.rs @@ -20,6 +20,7 @@ pub(crate) mod ietf; mod info; pub mod message; mod parameters; +pub(crate) mod priority; mod probe; #[cfg(feature = "async")] mod publisher; diff --git a/rs/moq-net/src/lite/priority.rs b/rs/moq-net/src/lite/priority.rs new file mode 100644 index 00000000000..0bf024df32c --- /dev/null +++ b/rs/moq-net/src/lite/priority.rs @@ -0,0 +1,34 @@ +//! Priority conversion between the moq-lite wire and the MoQ model. +//! +//! The wire carries `priority + 128` (the top bit flipped): one-to-one and order +//! preserving, so a byte means what it always meant and the unset priority 0 is byte 128. + +/// Convert a moq-lite priority byte into the model's priority. +pub(crate) const fn from_wire(priority: u8) -> i8 { + (priority ^ 0x80) as i8 +} + +/// Convert the model's priority into a moq-lite priority byte. +pub(crate) const fn to_wire(priority: i8) -> u8 { + (priority as u8) ^ 0x80 +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn lite_priority_is_offset() { + assert_eq!(to_wire(0), 128); + assert_eq!(to_wire(i8::MIN), 0); + assert_eq!(to_wire(i8::MAX), u8::MAX); + assert_eq!(from_wire(128), 0); + assert!((i8::MIN..i8::MAX).all(|priority| to_wire(priority) < to_wire(priority + 1))); + } + + #[test] + fn priority_round_trips() { + assert!((i8::MIN..=i8::MAX).all(|priority| from_wire(to_wire(priority)) == priority)); + assert!((0u8..=u8::MAX).all(|priority| to_wire(from_wire(priority)) == priority)); + } +} diff --git a/rs/moq-net/src/lite/publisher.rs b/rs/moq-net/src/lite/publisher.rs index 5278ac62f09..168aff54b6e 100644 --- a/rs/moq-net/src/lite/publisher.rs +++ b/rs/moq-net/src/lite/publisher.rs @@ -2157,12 +2157,14 @@ impl Request for SubscribeServe { let steer_tx = kio::Producer::new(steer); let mut session = shared.session.clone(); - let group = session.send_group(msg.priority.into()); + let publisher = track.info().priority; + let group = session.send_group(crate::priority::group(msg.priority, publisher)); let sub = Subscription { session, id: msg.id, track_name: Arc::from(track.name()), priority: shared.priority.clone(), + publisher, group, steer: steer_tx.consume(), steer_seen: steer, @@ -3597,7 +3599,7 @@ impl From<&lite::SubscribeUpdate> for Bounds { /// The part of each group's rank a SUBSCRIBE_UPDATE can move. #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] struct Steer { - priority: u8, + priority: i8, order: Order, } @@ -3611,6 +3613,8 @@ struct Subscription { id: u64, track_name: Arc, priority: PriorityQueue, + /// The publisher's priority for the track, which breaks subscriber priority ties. + publisher: i8, /// The subscription's send group, ranking it against the session's others, or `None` /// on a transport without groups, where `priority` ranks every group stream instead. group: Option>, @@ -3859,7 +3863,7 @@ impl TrackRun { } // One move for the whole subscription; its streams keep their order within it. if let Some(group) = &mut self.ctx.group { - group.set_priority(upd.priority.into()); + group.set_priority(crate::priority::group(upd.priority, self.ctx.publisher)); } // Feed the full update into the model subscriber so the producer's // aggregate reflects it (and a relay re-forwards it upstream). @@ -4244,9 +4248,11 @@ impl TrackRun { // subscription on the session, and only groups of the same one may be // ranked against each other by sequence, in that subscription's order. None => Rank::Queue( - self.ctx - .priority - .insert(Priority::new(steer.priority, self.ctx.id, sequence).with_order(steer.order)), + self.ctx.priority.insert( + Priority::new(steer.priority, self.ctx.id, sequence) + .with_publisher(self.ctx.publisher) + .with_order(steer.order), + ), ), }; debug_assert!(self.opening.is_none(), "one group waits for stream credit at a time"); @@ -4849,6 +4855,7 @@ mod serve_group_test { id: 0, track_name: "test".into(), priority: PriorityQueue::default(), + publisher: 0, group: None, steer: steer.consume(), steer_seen: Steer::default(), @@ -4895,6 +4902,7 @@ mod serve_group_test { id: 0, track_name: "test".into(), priority: PriorityQueue::default(), + publisher: 0, group: None, steer: steer.consume(), steer_seen: Steer::default(), @@ -4945,6 +4953,7 @@ mod serve_group_test { id: 0, track_name: "test".into(), priority: PriorityQueue::default(), + publisher: 0, group: None, steer: steer.consume(), steer_seen: Steer::default(), @@ -4995,6 +5004,7 @@ mod serve_group_test { id: 0, track_name: "test".into(), priority: PriorityQueue::default(), + publisher: 0, group: None, steer: steer.consume(), steer_seen: Steer::default(), @@ -5063,6 +5073,7 @@ mod serve_group_test { id: 0, track_name: "test".into(), priority: PriorityQueue::default(), + publisher: 0, group: None, steer: steer.consume(), steer_seen: Steer::default(), @@ -5236,6 +5247,7 @@ mod serve_group_test { id: 0, track_name: "test".into(), priority: PriorityQueue::default(), + publisher: 0, group: None, steer: steer.consume(), steer_seen: Steer::default(), @@ -5311,6 +5323,7 @@ mod serve_group_test { id: 0, track_name: "test".into(), priority: PriorityQueue::default(), + publisher: 0, group: None, steer: steer.consume(), steer_seen: Steer::default(), @@ -5360,6 +5373,7 @@ mod serve_group_test { id: 0, track_name: "test".into(), priority: PriorityQueue::default(), + publisher: 0, group: None, steer: steer.consume(), steer_seen: Steer::default(), @@ -5408,6 +5422,7 @@ mod serve_group_test { id: 0, track_name: "test".into(), priority: PriorityQueue::default(), + publisher: 0, group: None, steer: steer.consume(), steer_seen: Steer::default(), @@ -5468,6 +5483,7 @@ mod serve_group_test { id: 0, track_name: "test".into(), priority: PriorityQueue::default(), + publisher: 0, group: None, steer: steer.consume(), steer_seen: Steer::default(), @@ -5577,7 +5593,11 @@ mod serve_group_test { 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.groups(), [(0, 3), (0, 7)], "one move for the subscription"); + assert_eq!( + log.groups(), + [(0, 3), (0, crate::priority::group(7, 0))], + "one move for the subscription" + ); let mut flipped = log.orders()[2..].to_vec(); flipped.sort(); assert_eq!(flipped, [(0, 1), (1, -1)], "older groups first"); @@ -6600,6 +6620,7 @@ mod serve_group_test { id: 0, track_name: "test".into(), priority: PriorityQueue::default(), + publisher: 0, group: None, steer: steer.consume(), steer_seen: Steer::default(), @@ -6882,6 +6903,7 @@ mod tests { use super::*; use crate::lite::test_transport::{Close, ScriptedSession, SinkSend, SinkSession}; use crate::model::ProduceTest; + use crate::{broadcast, coding::Encode}; use futures::FutureExt; /// A peer that declares no origin in its SETUP is split-horizoned by the identity @@ -8375,4 +8397,105 @@ mod tests { } } } + + /// A publisher on a session with send groups. + fn grouped_publisher(origin: origin::Consumer) -> Publisher { + let peer_setup = crate::lite::PeerSetup::new(false, crate::transport::Binding::InBand); + peer_setup.set(crate::lite::Setup::default()); + let (_, goaway) = crate::goaway::Handle::new(true); + Publisher::new(PublisherConfig { + runtime: crate::time::sim(), + session: ScriptedSession::new(Vec::new()).with_groups(), + origin, + version: Version::Lite07, + peer_setup, + goaway, + peer_hop: None, + auth: crate::auth::Handle::new(false), + peer_grant: Default::default(), + client: false, + subscriptions: Default::default(), + fetches: Default::default(), + estimate: kio::Producer::default().weak(), + }) + } + + /// Subscribe to `broadcast`'s `track` at the subscriber `priority`, resolving the track + /// with `info`, until the subscription runs. + async fn subscribe_at( + publisher: &Publisher, + dynamic: &mut broadcast::Dynamic, + broadcast: &'static str, + track: &'static str, + priority: i8, + info: track::Info, + ) -> ( + RequestServe>, + track::Producer, + ) { + let version = Version::Lite07; + let msg = lite::Subscribe { + order: crate::track::Order::Descending, + epoch: None, + id: 0, + broadcast: crate::Path::new(broadcast), + track: track.into(), + priority, + max_delay: Duration::ZERO, + start_group: None, + end_group: None, + start_frame: 0, + end_frame: None, + }; + let mut script = Vec::new(); + msg.encode(&mut crate::coding::Encoder::new(&mut script, version.into()), version) + .unwrap(); + let mut session = ScriptedSession::new(script); + let (send, recv) = futures::future::poll_fn(|cx| { + ::poll_open_bi(&mut session, cx, Default::default()) + }) + .await + .unwrap(); + let stream = Stream:: { + writer: Writer::new(send, version), + reader: crate::coding::Reader::new(recv, version), + }; + let mut serve = RequestServe::<_, SubscribeServe<_>>::new(publisher.shared.clone(), stream); + assert!(!drive(&mut serve).await); + let track = dynamic + .requested_track() + .now_or_never() + .expect("track requested") + .unwrap() + .accept(info); + assert!(!drive(&mut serve).await, "the subscription ended"); + assert!( + matches!(serve.state, RequestState::Serve(SubscribeServe::Run(_))), + "the subscription is not running" + ); + (serve, track) + } + + /// A subscription's send group ranks by the peer's subscriber priority first, and the + /// publisher's track priority breaks a tie between two at one subscriber priority. + #[moq_clock::test] + async fn send_group_ranks_by_the_subscriber_then_the_publisher() { + let origin = crate::origin::Config::new(Hop::new(1).unwrap()).produce(); + let broadcast = origin.create_broadcast("a").unwrap(); + let mut dynamic = broadcast.dynamic(); + broadcast.announce(Default::default()).unwrap(); + let publisher = grouped_publisher(origin.consume()); + let log = publisher.shared.session.log.clone(); + + let video = track::Info::timed().with_priority(60); + let audio = track::Info::timed().with_priority(80); + let _video = subscribe_at(&publisher, &mut dynamic, "a", "video", 0, video).await; + let _audio = subscribe_at(&publisher, &mut dynamic, "a", "audio", 0, audio).await; + let _urgent = subscribe_at(&publisher, &mut dynamic, "a", "chat", 1, track::Info::timed()).await; + + let groups = log.groups(); + assert_eq!(groups.len(), 3); + assert!(groups[1].1 > groups[0].1, "the publisher's priority breaks the tie"); + assert!(groups[2].1 > groups[1].1, "the subscriber's priority dominates"); + } } diff --git a/rs/moq-net/src/lite/subscribe.rs b/rs/moq-net/src/lite/subscribe.rs index de03c84cd48..279a07ffdf8 100644 --- a/rs/moq-net/src/lite/subscribe.rs +++ b/rs/moq-net/src/lite/subscribe.rs @@ -21,7 +21,7 @@ pub struct Subscribe<'a> { /// Lite07+ only. pub epoch: Option, pub track: Cow<'a, str>, - pub priority: u8, + pub priority: i8, pub max_delay: std::time::Duration, /// The minimum group to deliver (a floor). On lite-06 the wire carries the raw /// sequence and `None` is interchangeable with `Some(0)`: a floor of 0 constrains @@ -58,7 +58,7 @@ impl Message for Subscribe<'_> { let broadcast = Path::decode(r, version)?; let epoch = super::epoch::decode_epoch(r, version)?; let track = Cow::Owned(r.string()?); - let priority = r.u8()?; + let priority = super::priority::from_wire(r.u8()?); let (order, max_delay, start_group, end_group) = match version { Version::Lite01 | Version::Lite02 => (Order::Descending, std::time::Duration::ZERO, None, None), @@ -94,7 +94,7 @@ impl Message for Subscribe<'_> { self.broadcast.encode(w, version)?; super::epoch::encode_epoch(w, version, self.epoch.as_ref())?; w.string(&self.track)?; - w.u8(self.priority); + w.u8(super::priority::to_wire(self.priority)); match version { Version::Lite01 | Version::Lite02 => {} @@ -261,7 +261,7 @@ fn encode_frame_bounds( /// to [`super::TrackInfo`]. #[derive(Clone, Debug)] pub struct SubscribeOk { - pub priority: u8, + pub priority: i8, pub max_delay: std::time::Duration, pub start_group: Option, pub end_group: Option, @@ -271,13 +271,13 @@ impl Message for SubscribeOk { fn encode_msg(&self, w: &mut Encoder<'_>, version: Version) -> Result<(), EncodeError> { match version { Version::Lite01 => { - w.u8(self.priority); + w.u8(super::priority::to_wire(self.priority)); } Version::Lite02 => {} // Lite05+ never sends SUBSCRIBE_OK, but keep the field layout matching // Lite03/04 so a stray future use stays well-formed. _ => { - w.u8(self.priority); + w.u8(super::priority::to_wire(self.priority)); pad_group_order(w, version)?; w.varint(u64::try_from(self.max_delay.as_millis()).map_err(|_| EncodeError::BoundsExceeded)?)?; w.varint_opt(self.start_group)?; @@ -291,7 +291,7 @@ impl Message for SubscribeOk { fn decode_msg(r: &mut Decoder<'_>, version: Version) -> Result { match version { Version::Lite01 => Ok(Self { - priority: r.u8()?, + priority: super::priority::from_wire(r.u8()?), max_delay: std::time::Duration::ZERO, start_group: None, end_group: None, @@ -303,7 +303,7 @@ impl Message for SubscribeOk { end_group: None, }), _ => { - let priority = r.u8()?; + let priority = super::priority::from_wire(r.u8()?); skip_group_order(r, version)?; let max_delay = std::time::Duration::from_millis(r.varint()?); let start_group = r.varint_opt()?; @@ -405,7 +405,7 @@ impl Message for SubscribeEnd { pub struct SubscribeUpdate { /// A join requesting this group or later, judged against the local refresh ratio. pub wanted_group: Option, - pub priority: u8, + pub priority: i8, pub max_delay: std::time::Duration, pub start_group: Option, pub end_group: Option, @@ -426,7 +426,7 @@ impl Message for SubscribeUpdate { _ => {} } - let priority = r.u8()?; + let priority = super::priority::from_wire(r.u8()?); let order = decode_order(r, version)?; let max_delay = std::time::Duration::from_millis(r.varint()?); let start_group = decode_start_group(r, version)?; @@ -463,7 +463,7 @@ impl Message for SubscribeUpdate { _ => {} } - w.u8(self.priority); + w.u8(super::priority::to_wire(self.priority)); encode_order(w, version, self.order)?; w.varint(u64::try_from(self.max_delay.as_millis()).map_err(|_| EncodeError::BoundsExceeded)?)?; diff --git a/rs/moq-net/src/lite/subscriber.rs b/rs/moq-net/src/lite/subscriber.rs index 2f5bc62050c..f4d0dc49550 100644 --- a/rs/moq-net/src/lite/subscriber.rs +++ b/rs/moq-net/src/lite/subscriber.rs @@ -5803,7 +5803,7 @@ struct SubStream { /// downstream aggregate changes. max_delay: Duration, start: Option, - priority: u8, + priority: i8, order: crate::track::Order, /// The start the SUBSCRIBE itself carried, fixed for the stream's life. A /// SUBSCRIBE_START describes this demand and no fresh one follows an update, diff --git a/rs/moq-net/src/lite/test_transport.rs b/rs/moq-net/src/lite/test_transport.rs index f581584264b..a62306b3d3b 100644 --- a/rs/moq-net/src/lite/test_transport.rs +++ b/rs/moq-net/src/lite/test_transport.rs @@ -917,6 +917,8 @@ pub struct ScriptedSession { /// Set by [`Self::with_unacked_fin`]. unacked_fin: bool, datagram_size: usize, + /// Set by [`Self::with_groups`]. + groups: bool, } impl ScriptedSession { @@ -943,9 +945,16 @@ impl ScriptedSession { incoming_bidis: Arc::new(Mutex::new(std::collections::VecDeque::new())), unacked_fin: false, datagram_size: 0, + groups: false, } } + /// Open send groups, as native QUIC does, recording them in the log. + pub fn with_groups(mut self) -> Self { + self.groups = true; + self + } + /// Never acknowledge an opened stream's FIN, like a peer that stopped reading. pub fn with_unacked_fin(mut self) -> Self { self.unacked_fin = true; @@ -1103,8 +1112,8 @@ impl poll::Session for ScriptedSession { Poll::Ready(Ok(SinkSend::new(self.log.clone()).opened(options))) } - fn send_group(&mut self, _: i64) -> Option { - None + fn send_group(&mut self, priority: i64) -> Option { + self.groups.then(|| SinkGroup::open(self.log.clone(), priority)) } fn send_datagram( diff --git a/rs/moq-net/src/lite/track.rs b/rs/moq-net/src/lite/track.rs index 658f248991c..eeaa1f8a112 100644 --- a/rs/moq-net/src/lite/track.rs +++ b/rs/moq-net/src/lite/track.rs @@ -62,7 +62,7 @@ pub struct TrackInfo { /// Refresh byte ratio, advertised on lite-07 only. pub refresh: Option, /// The publisher's tie-break priority for this track. - pub priority: u8, + pub priority: i8, /// Publisher Max Age: an upper bound on how long the publisher caches a /// non-latest group past the arrival of a newer one. Encoded as milliseconds. pub max_age: Option, @@ -138,7 +138,7 @@ impl Message for TrackInfo { return Err(DecodeError::Version); } - let priority = r.u8()?; + let priority = super::priority::from_wire(r.u8()?); super::subscribe::skip_group_order(r, version)?; let encoded = r.varint()?; let max_age = match version { @@ -183,7 +183,7 @@ impl Message for TrackInfo { return Err(EncodeError::Version); } - w.u8(self.priority); + w.u8(super::priority::to_wire(self.priority)); super::subscribe::pad_group_order(w, version)?; let encoded = match (version, self.max_age) { (Version::Lite05 | Version::Lite06, None) => LEGACY_UNLIMITED, @@ -325,7 +325,7 @@ mod test { assert_eq!( buf, [ - 0x0c, 0x7f, 0x00, 0xc0, 0x1f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x43, 0xe8 + 0x0c, 0x80, 0x00, 0xc0, 0x1f, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x43, 0xe8 ] ); } @@ -431,13 +431,18 @@ mod test { fn track_info_roundtrips_varint_and_priority_bounds() { let info = TrackInfo { refresh: None, - priority: 255, + priority: i8::MIN, max_age: None, timescale: Some(Timescale::new((1u64 << 62) - 1).unwrap()), compress: false, }; let got = info_roundtrip(Version::Lite05, &info); - assert_eq!(got.priority, 255); + assert_eq!(got.priority, i8::MIN); + let info = TrackInfo { + priority: i8::MAX, + ..info + }; + assert_eq!(info_roundtrip(Version::Lite05, &info).priority, i8::MAX); assert_eq!(got.max_age, info.max_age); assert_eq!(got.timescale, info.timescale); } diff --git a/rs/moq-net/src/model/bandwidth.rs b/rs/moq-net/src/model/bandwidth.rs index 8a5d5174191..609f540892e 100644 --- a/rs/moq-net/src/model/bandwidth.rs +++ b/rs/moq-net/src/model/bandwidth.rs @@ -294,12 +294,13 @@ impl Allocator { /// bit; within a tier the split is max-min fair, so a share asking for less /// than an even cut takes all of it and leaves the difference to the others. /// - /// That is the *publisher's* priority, which is not what orders the local send - /// queue: that ranks by each subscription's own priority, so a subscriber - /// asking for video ahead of audio is served that way whatever this decides. - /// The publisher's is still the right one to divide by, since allocation is a - /// decision about what to *produce*, and there is no single subscriber - /// priority to read when several are watching one track. + /// That is the *publisher's* priority, which the send order only falls back on: + /// it ranks by each subscription's own priority first, so a subscriber asking for + /// video ahead of audio is served that way whatever this decides, and this one + /// breaks the ties (or alone orders a broadcast's tracks on a session scheduled + /// per broadcast). The publisher's is still the right one to divide by, since + /// allocation is a decision about what to *produce*, and there is no single + /// subscriber priority to read when several are watching one track. /// /// That last part carries a publisher that leaves `priority` at its default: /// one tier of audio and video still serves audio's small reservation in full @@ -475,7 +476,7 @@ impl Registry { /// One registered track's standing reservation. struct Entry { demand: track::Demand, - priority: u8, + priority: i8, max: Rate, } @@ -535,7 +536,7 @@ struct Share { #[derive(Copy, Clone, Debug, Eq, PartialEq)] struct Want { id: u64, - priority: u8, + priority: i8, max: Rate, } @@ -755,15 +756,15 @@ mod tests { use crate::broadcast; /// Priorities matching `hang`'s, which is what the allocator sees in practice. - const AUDIO: u8 = 80; - const VIDEO: u8 = 60; + const AUDIO: i8 = 80; + const VIDEO: i8 = 60; /// Bits per second, so the tables below stay readable. fn bps(bps: u64) -> Rate { Rate::from_bps(bps) } - fn want(id: u64, priority: u8, max: u64) -> Want { + fn want(id: u64, priority: i8, max: u64) -> Want { Want { id, priority, @@ -772,7 +773,7 @@ mod tests { } /// A standalone track at `priority`, plus the broadcast keeping it alive. - fn track(priority: u8) -> (broadcast::Producer, track::Producer) { + fn track(priority: i8) -> (broadcast::Producer, track::Producer) { let broadcast = broadcast::Info::default().produce(); let track = broadcast .create_track("t", track::Info::timed().with_priority(priority)) diff --git a/rs/moq-net/src/model/group.rs b/rs/moq-net/src/model/group.rs index 19f662c8f18..89311b0ea5a 100644 --- a/rs/moq-net/src/model/group.rs +++ b/rs/moq-net/src/model/group.rs @@ -3212,8 +3212,8 @@ impl Cursor { #[derive(Clone, Debug, Default)] #[non_exhaustive] pub struct Fetch { - /// Delivery priority for the fetched group's stream. Defaults to 0. - pub priority: u8, + /// Delivery priority for the fetched group's stream, higher first. Defaults to 0. + pub priority: i8, /// Maximum age behind the live edge this caller accepts. /// @@ -3236,7 +3236,7 @@ pub struct Fetch { impl Fetch { /// Set the delivery priority, returning `self` for chaining. - pub fn with_priority(mut self, priority: u8) -> Self { + pub fn with_priority(mut self, priority: i8) -> Self { self.priority = priority; self } @@ -3269,7 +3269,7 @@ pub struct Request { pub(crate) state: kio::Producer, pub(super) fetch: kio::Shared, pub(super) sequence: u64, - pub(super) priority: u8, + pub(super) priority: i8, pub(super) max_delay: Option, pub(super) frame_start: u64, // pending: /quest/m1/public-model/deferred.md diff --git a/rs/moq-net/src/model/subscription.rs b/rs/moq-net/src/model/subscription.rs index ca15f330985..0554c86003a 100644 --- a/rs/moq-net/src/model/subscription.rs +++ b/rs/moq-net/src/model/subscription.rs @@ -17,8 +17,9 @@ pub(super) use window::{Containing, Held, Holdings, Resolved, Unheld, Window, la #[derive(Clone, Debug, PartialEq, Eq)] #[non_exhaustive] pub struct Subscription { - /// Delivery priority. Higher values preempt lower ones when bandwidth is constrained. - pub priority: u8, + /// Delivery priority, higher first, defaulting to 0. Higher values preempt lower ones + /// when bandwidth is constrained. + pub priority: i8, /// How long a superseded group stays worth delivering to this subscriber. /// /// [`Duration::ZERO`] (the default) skips immediately: group 8 arriving means group 7 @@ -271,7 +272,7 @@ impl Default for Subscription { impl Subscription { /// Set the delivery priority, returning `self` for chaining. - pub fn with_priority(mut self, priority: u8) -> Self { + pub fn with_priority(mut self, priority: i8) -> Self { self.priority = priority; self } diff --git a/rs/moq-net/src/model/track.rs b/rs/moq-net/src/model/track.rs index 84fb2f216a7..a8c593de4f0 100644 --- a/rs/moq-net/src/model/track.rs +++ b/rs/moq-net/src/model/track.rs @@ -40,9 +40,9 @@ use std::{ time::Duration, }; -// The higher-first midpoint. IETF flips priority (lower first), so this goes out as 128, the -// draft's usual publisher priority, while moq-lite carries 127 as written: one urgency on both. -const DEFAULT_PRIORITY: u8 = 127; +// The unset priority, the midpoint of the signed range. It goes out as byte 128 on moq-lite +// and 127 on IETF, which flips priority (lower first): one urgency on both. +const DEFAULT_PRIORITY: i8 = 0; /// Maximum number of datagrams retained in the per-track send buffer. /// @@ -133,10 +133,11 @@ pub struct Info { /// `2^53 - 1` milliseconds to no limit for older JavaScript readers. Sub-millisecond precision is truncated /// (`Duration::as_millis`) at encode time. pub max_age: Option, - /// The publisher's priority for this track, used only to break ties between - /// subscriptions of equal subscriber priority. Reported in TRACK_INFO (Lite05+). - /// Higher is more urgent. Defaults to 127, the midpoint. - pub priority: u8, + /// The publisher's priority for this track, higher first, defaulting to 0. + /// + /// It breaks ties between subscriptions of equal subscriber priority. Reported in + /// TRACK_INFO (Lite05+). + pub priority: i8, /// Whether frames are DEFLATE compressed per group, so later frames compress against /// earlier ones in the same group. Defaults to `false`. /// @@ -209,8 +210,8 @@ impl Info { self } - /// Set the publisher's tie-break priority, returning `self` for chaining. - pub fn with_priority(mut self, priority: u8) -> Self { + /// Set the publisher's priority, returning `self` for chaining. + pub fn with_priority(mut self, priority: i8) -> Self { self.priority = priority; self } @@ -714,7 +715,7 @@ pub(super) type FetchState = Requests; /// One fetch attempt for a sequence, shared by every [`Fetching`] that joined it. pub(super) struct PendingFetch { // The most demanding delivery priority across the joined fetches. - priority: u8, + priority: i8, // The most tolerant reader budget. None is unbounded and therefore dominates. max_delay: Option, @@ -2561,7 +2562,7 @@ impl Producer { #[cfg(test)] #[cfg(feature = "async")] // private: test - pub(crate) fn publisher_priority(&self) -> u8 { + pub(crate) fn publisher_priority(&self) -> i8 { self.info.priority } @@ -3880,7 +3881,7 @@ impl Demand { } /// The publisher's tie-break priority, as set in [`Info::priority`]. - pub(super) fn priority(&self) -> u8 { + pub(super) fn priority(&self) -> i8 { // Always Some once the track exists; a closed one reads its last value. self.state .read() @@ -4727,7 +4728,7 @@ impl group::Request { } /// The delivery priority the consumer requested for this group. - pub fn priority(&self) -> u8 { + pub fn priority(&self) -> i8 { self.priority } @@ -4879,7 +4880,7 @@ pub struct Fetching { // rather than a short answer. frame_start: u64, // This caller's priority, for a fetch that moves to a front's routes. - priority: u8, + priority: i8, // This caller's own age cap. None means only the publisher's max age. max_delay: Option, // The attempt this fetch joined; `None` on a cache hit, or when no handler diff --git a/rs/moq-net/src/model/track/demand.rs b/rs/moq-net/src/model/track/demand.rs index 65093a1e956..80618c813b2 100644 --- a/rs/moq-net/src/model/track/demand.rs +++ b/rs/moq-net/src/model/track/demand.rs @@ -17,7 +17,7 @@ use ranges::Ranges; #[derive(Default)] pub(super) struct Subscriptions { count: usize, - priority: BTreeMap, + priority: BTreeMap, max_delay: BTreeMap, ranges: Ranges, descending: usize, @@ -32,7 +32,7 @@ pub(super) struct Subscriptions { } /// Everything the aggregate holds besides its ranges. -type Scalars = (u8, Duration, Order); +type Scalars = (i8, Duration, Order); impl Subscriptions { pub(super) fn combined(&self) -> Option { diff --git a/rs/moq-net/src/priority.rs b/rs/moq-net/src/priority.rs index d7340d07fca..98db1b09a8d 100644 --- a/rs/moq-net/src/priority.rs +++ b/rs/moq-net/src/priority.rs @@ -17,14 +17,16 @@ use crate::track::Order; // - On remove from Vec: pop highest priority item from overflow to backfill // - On remove from overflow: remove its priority-and-id key in O(log n) // -// Priority ordering: higher track value = higher priority, then subscription, then -// group sequence in that subscription's own direction. +// Priority ordering: higher track value = higher priority, then the publisher's priority, +// then subscription, then group sequence in that subscription's own direction. -/// A group stream's rank: its subscription's track priority, the subscription it -/// belongs to, its group sequence in that subscription's [`Order`], and the first object -/// it carries. +/// A group stream's rank: its subscription's track priority, the publisher's priority for +/// the track, the subscription it belongs to, its group sequence in that subscription's +/// [`Order`], and the first object it carries. /// -/// Higher `track` always wins. `group` breaks ties only *within* one `subscribe`: +/// Higher `track` always wins, and a higher `publisher` breaks a tie between two +/// subscriptions at the same `track`, so hang audio goes ahead of video when a subscriber +/// asks for both at one priority. `group` breaks ties only *within* one `subscribe`: /// newest first by default, so a congested track sheds its backlog rather than its live /// edge, or oldest first for an ascending subscription draining one. /// @@ -36,7 +38,8 @@ use crate::track::Order; /// since the rest of a group is undecodable without its start. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub struct Priority { - track: u8, + track: i8, + publisher: i8, subscribe: u64, order: Order, group: u64, @@ -45,9 +48,10 @@ pub struct Priority { impl Priority { /// Rank a group for transmission: the newest group of a track transmits first. - pub fn new(track: u8, subscribe: u64, group: u64) -> Self { + pub fn new(track: i8, subscribe: u64, group: u64) -> Self { Self { track, + publisher: 0, subscribe, order: Order::Descending, group, @@ -55,6 +59,12 @@ impl Priority { } } + /// Break a tie between subscriptions at the same track priority by the publisher's. + pub fn with_publisher(mut self, publisher: i8) -> Self { + self.publisher = publisher; + self + } + /// Rank groups of the same subscription in `order` instead. pub fn with_order(mut self, order: Order) -> Self { self.order = order; @@ -74,6 +84,7 @@ impl Ord for Priority { other .track .cmp(&self.track) + .then(other.publisher.cmp(&self.publisher)) // Which subscription wins a priority tie is arbitrary, but it has to be // stable and independent of either one's direction. The older (lower id) // subscription goes first. @@ -115,6 +126,13 @@ pub fn in_group(group: u64, start: u64, order: Order) -> i64 { group.saturating_add(i64::from(start == 0)) } +/// A subscription's send group priority: its subscriber priority, then the publisher's. +/// +/// Packed so the subscriber's always dominates, exactly as [`Priority`] ranks them. +pub fn group(subscriber: i8, publisher: i8) -> i64 { + i64::from(subscriber) * 256 + i64::from(publisher) +} + #[derive(Debug, Clone)] struct PriorityItem { id: usize, @@ -388,7 +406,7 @@ impl PriorityState { } /// Change one item's track priority and order, leaving the rest of its rank alone. - fn steer(&mut self, id: usize, track: u8, order: Order) { + fn steer(&mut self, id: usize, track: i8, order: Order) { let mut item = self.extract(id); item.priority.track = track; item.priority.order = order; @@ -419,7 +437,7 @@ pub struct PriorityHandle { id: usize, /// This item's track priority and order, the only parts of its rank a handle owns; /// the rest is fixed when the item is queued. - track: u8, + track: i8, order: Order, /// Last value observed via [`current`](Self::current)/[`next`](Self::next), so /// `next` fires only on a real change. @@ -483,7 +501,7 @@ impl PriorityHandle { /// Change this item's track priority and order and re-sort the queue, returning its /// new rank. No-op if neither changed. - pub fn steer(&mut self, track: u8, order: Order) -> u8 { + pub fn steer(&mut self, track: i8, order: Order) -> u8 { if self.track == track && self.order == order { return self.current(); } @@ -514,7 +532,7 @@ mod tests { /// A live subscription's group rank, on the one subscription these tests share. /// Group order within a subscription is what most of them exercise, so scoping /// them together is what keeps them testing it. - fn live(track: u8, group: u64) -> Priority { + fn live(track: i8, group: u64) -> Priority { Priority::new(track, 0, group) } @@ -573,14 +591,14 @@ mod tests { #[test] fn a_reorder_wakes_only_the_ranks_that_moved() { let queue = PriorityQueue::default(); - let parked = Parked::new((0..64).map(|group| queue.insert(live(100, group))).collect()); + let parked = Parked::new((0..64).map(|group| queue.insert(live(-28, group))).collect()); // Lowest priority yet, so it lands at the tail and shifts nobody. - let tail = queue.insert(live(50, 0)); + let tail = queue.insert(live(-78, 0)); assert_eq!(parked.counts(), vec![0; 64], "a tail insert moved no other rank"); // Highest priority yet, so every existing entry shifts down one. - let front = queue.insert(live(u8::MAX, u64::MAX)); + let front = queue.insert(live(i8::MAX, u64::MAX)); assert_eq!(parked.counts(), vec![1; 64], "a front insert moved every other rank"); drop((tail, front)); @@ -590,8 +608,8 @@ mod tests { #[test] fn an_overflow_removal_wakes_nobody() { let queue = PriorityQueue::default(); - let fillers = (0..MAX_VEC_SIZE as u64).map(|group| queue.insert(live(200, group))); - let overflow = (0..2).map(|group| queue.insert(live(100, group))); + let fillers = (0..MAX_VEC_SIZE as u64).map(|group| queue.insert(live(72, group))); + let overflow = (0..2).map(|group| queue.insert(live(-28, group))); let mut parked = Parked::new(fillers.chain(overflow).collect()); let doomed = parked.handles.pop().expect("overflow handle"); @@ -608,7 +626,7 @@ mod tests { #[test] fn test_single_item() { let queue = PriorityQueue::default(); - let mut handle = queue.insert(live(100, 5)); + let mut handle = queue.insert(live(-28, 5)); assert_eq!(handle.current(), 0); // First item is always index 0 } @@ -618,8 +636,8 @@ mod tests { #[test] fn test_send_order_inverts_rank() { let queue = PriorityQueue::default(); - let mut top = queue.insert(live(200, 0)); - let mut low = queue.insert(live(100, 0)); + let mut top = queue.insert(live(72, 0)); + let mut low = queue.insert(live(-28, 0)); assert_eq!(top.current(), 0); assert_eq!(top.send_order(), 255, "most urgent rank gets the highest send order"); @@ -632,9 +650,9 @@ mod tests { let queue = PriorityQueue::default(); // Insert items with different track priorities - let mut low = queue.insert(live(50, 0)); - let mut high = queue.insert(live(255, 0)); - let mut mid = queue.insert(live(100, 0)); + let mut low = queue.insert(live(-78, 0)); + let mut high = queue.insert(live(127, 0)); + let mut mid = queue.insert(live(-28, 0)); // With sorted vec, indices map exactly to priority order assert_eq!(high.current(), 0); // Highest priority @@ -647,9 +665,9 @@ mod tests { let queue = PriorityQueue::default(); // Same track priority, different groups - let mut group10 = queue.insert(live(100, 10)); - let mut group5 = queue.insert(live(100, 5)); - let mut group1 = queue.insert(live(100, 1)); + let mut group10 = queue.insert(live(-28, 10)); + let mut group5 = queue.insert(live(-28, 5)); + let mut group1 = queue.insert(live(-28, 1)); // Exact index mapping for sorted vec assert_eq!(group10.current(), 0); @@ -662,12 +680,13 @@ mod tests { #[test] fn test_ord_is_total() { let mixed = [ - Priority::new(100, 7, 1), - Priority::new(100, 7, 2), - Priority::new(100, 7, 1).with_order(Order::Ascending), - Priority::new(100, 7, 2).with_order(Order::Ascending), - Priority::new(100, 8, 1), - Priority::new(200, 7, 1), + Priority::new(-28, 7, 1), + Priority::new(-28, 7, 2), + Priority::new(-28, 7, 1).with_order(Order::Ascending), + Priority::new(-28, 7, 2).with_order(Order::Ascending), + Priority::new(-28, 8, 1), + Priority::new(-28, 8, 1).with_publisher(5), + Priority::new(72, 7, 1), ]; for a in mixed { @@ -687,7 +706,7 @@ mod tests { #[test] fn test_ascending_ranks_oldest_first() { let queue = PriorityQueue::default(); - let ascending = |group| Priority::new(100, 0, group).with_order(Order::Ascending); + let ascending = |group| Priority::new(-28, 0, group).with_order(Order::Ascending); let mut group5 = queue.insert(ascending(5)); let mut group1 = queue.insert(ascending(1)); let mut group10 = queue.insert(ascending(10)); @@ -698,7 +717,7 @@ mod tests { // Steering the subscription back to descending flips it once every group moved. for group in [&mut group1, &mut group5, &mut group10] { - group.steer(100, Order::Descending); + group.steer(-28, Order::Descending); } assert_eq!(group10.current(), 0); assert_eq!(group5.current(), 1); @@ -711,7 +730,7 @@ mod tests { fn test_earlier_start_ranks_first_within_a_group() { for order in [Order::Descending, Order::Ascending] { let queue = PriorityQueue::default(); - let stream = |group, start| Priority::new(100, 0, group).with_order(order).with_start(start); + let stream = |group, start| Priority::new(-28, 0, group).with_order(order).with_start(start); // Queued first, so the slab id alone would rank it ahead. let mut rest = queue.insert(stream(5, 3)); let mut head = queue.insert(stream(5, 0)); @@ -729,14 +748,29 @@ mod tests { } } + /// The publisher's priority breaks a tie between subscriptions at one track priority, and + /// only a tie: the subscriber's priority still wins outright. + #[test] + fn test_publisher_priority_breaks_ties() { + let queue = PriorityQueue::default(); + // Older subscriptions win a full tie, so the tiebreak has to beat that. + let mut video = queue.insert(Priority::new(0, 1, 9).with_publisher(20)); + let mut audio = queue.insert(Priority::new(0, 2, 1).with_publisher(40)); + let mut urgent = queue.insert(Priority::new(1, 3, 1).with_publisher(i8::MIN)); + + assert_eq!(urgent.current(), 0, "subscriber priority dominates"); + assert_eq!(audio.current(), 1, "a higher publisher priority wins the tie"); + assert_eq!(video.current(), 2); + } + #[test] fn test_track_priority_overrides_group() { let queue = PriorityQueue::default(); // Lower track priority but higher group - let mut low_track_high_group = queue.insert(live(50, 1000)); + let mut low_track_high_group = queue.insert(live(-78, 1000)); // Higher track priority but lower group - let mut high_track_low_group = queue.insert(live(255, 1)); + let mut high_track_low_group = queue.insert(live(127, 1)); // Track priority should take precedence assert_eq!(high_track_low_group.current(), 0); @@ -747,9 +781,9 @@ mod tests { fn test_removal_on_drop() { let queue = PriorityQueue::default(); - let mut first = queue.insert(live(255, 0)); - let mut second = queue.insert(live(100, 0)); - let mut third = queue.insert(live(50, 0)); + let mut first = queue.insert(live(127, 0)); + let mut second = queue.insert(live(-28, 0)); + let mut third = queue.insert(live(-78, 0)); assert_eq!(first.current(), 0); assert_eq!(second.current(), 1); @@ -767,8 +801,8 @@ mod tests { fn test_removal_of_highest_priority() { let queue = PriorityQueue::default(); - let mut first = queue.insert(live(255, 0)); - let mut second = queue.insert(live(100, 0)); + let mut first = queue.insert(live(127, 0)); + let mut second = queue.insert(live(-28, 0)); assert_eq!(first.current(), 0); assert_eq!(second.current(), 1); @@ -784,8 +818,8 @@ mod tests { fn test_removal_of_lowest_priority() { let queue = PriorityQueue::default(); - let mut first = queue.insert(live(255, 0)); - let mut second = queue.insert(live(100, 0)); + let mut first = queue.insert(live(127, 0)); + let mut second = queue.insert(live(-28, 0)); assert_eq!(first.current(), 0); assert_eq!(second.current(), 1); @@ -802,7 +836,7 @@ mod tests { let queue = PriorityQueue::default(); // Insert items from high to low group to make them ordered in heap - let mut handles: Vec<_> = (0..10).rev().map(|i| queue.insert(live(100, i))).collect(); + let mut handles: Vec<_> = (0..10).rev().map(|i| queue.insert(live(-28, i))).collect(); // Highest group (9, at handles[0]) should be at heap index 0 assert_eq!(handles[0].current(), 0); @@ -818,7 +852,7 @@ mod tests { let queue = PriorityQueue::default(); // Insert more than 255 items (insert high to low so first item is highest priority) - let mut handles: Vec<_> = (0..300).rev().map(|i| queue.insert(live(100, i))).collect(); + let mut handles: Vec<_> = (0..300).rev().map(|i| queue.insert(live(-28, i))).collect(); // Highest priority item (group=299, handles[0]) should be at heap index 0 assert_eq!(handles[0].current(), 0); @@ -839,18 +873,18 @@ mod tests { let queue = PriorityQueue::default(); // Mix of different track priorities and groups - let mut high_track_high_group = queue.insert(live(255, 10)); - let mut high_track_low_group = queue.insert(live(255, 1)); - let mut mid_track_high_group = queue.insert(live(100, 5)); - let mut mid_track_low_group = queue.insert(live(100, 1)); - let mut low_track_high_group = queue.insert(live(50, 100)); + let mut high_track_high_group = queue.insert(live(127, 10)); + let mut high_track_low_group = queue.insert(live(127, 1)); + let mut mid_track_high_group = queue.insert(live(-28, 5)); + let mut mid_track_low_group = queue.insert(live(-28, 1)); + let mut low_track_high_group = queue.insert(live(-78, 100)); // Exact index mapping with sorted vec - assert_eq!(high_track_high_group.current(), 0); // track=255, group=10 - assert_eq!(high_track_low_group.current(), 1); // track=255, group=1 - assert_eq!(mid_track_high_group.current(), 2); // track=100, group=5 - assert_eq!(mid_track_low_group.current(), 3); // track=100, group=1 - assert_eq!(low_track_high_group.current(), 4); // track=50, group=100 + assert_eq!(high_track_high_group.current(), 0); // track=127, group=10 + assert_eq!(high_track_low_group.current(), 1); // track=127, group=1 + assert_eq!(mid_track_high_group.current(), 2); // track=-28, group=5 + assert_eq!(mid_track_low_group.current(), 3); // track=-28, group=1 + assert_eq!(low_track_high_group.current(), 4); // track=-78, group=100 } #[moq_clock::test] @@ -858,10 +892,10 @@ mod tests { let queue = PriorityQueue::default(); // Fill vec to capacity - let mut fillers: Vec<_> = (0..255).rev().map(|i| queue.insert(live(100, i + 100))).collect(); + let mut fillers: Vec<_> = (0..255).rev().map(|i| queue.insert(live(-28, i + 100))).collect(); // This goes to overflow - let mut overflow_item = queue.insert(live(100, 50)); + let mut overflow_item = queue.insert(live(-28, 50)); assert_eq!(overflow_item.current(), u8::MAX); // Spawn task to wait for promotion from overflow @@ -882,9 +916,9 @@ mod tests { fn test_interleaved_insertions_and_removals() { let queue = PriorityQueue::default(); - let mut h1 = queue.insert(live(200, 0)); - let h2 = queue.insert(live(150, 0)); - let mut h3 = queue.insert(live(100, 0)); + let mut h1 = queue.insert(live(72, 0)); + let h2 = queue.insert(live(22, 0)); + let mut h3 = queue.insert(live(-28, 0)); // h1 has highest priority assert_eq!(h1.current(), 0); @@ -896,7 +930,7 @@ mod tests { // h3 should have moved up assert!(h3.current() < 2); - let mut h4 = queue.insert(live(250, 0)); + let mut h4 = queue.insert(live(122, 0)); // h4 has highest priority now assert_eq!(h4.current(), 0); @@ -914,9 +948,9 @@ mod tests { let queue = PriorityQueue::default(); // Items with identical track and group should still be ordered consistently - let mut h1 = queue.insert(live(100, 5)); - let mut h2 = queue.insert(live(100, 5)); - let mut h3 = queue.insert(live(100, 5)); + let mut h1 = queue.insert(live(-28, 5)); + let mut h2 = queue.insert(live(-28, 5)); + let mut h3 = queue.insert(live(-28, 5)); // All three should have valid indices let indices = [h1.current(), h2.current(), h3.current()]; @@ -931,9 +965,9 @@ mod tests { let queue = PriorityQueue::default(); // Create a heap with known structure - let mut root = queue.insert(live(255, 0)); - let left = queue.insert(live(100, 0)); - let mut right = queue.insert(live(100, 0)); + let mut root = queue.insert(live(127, 0)); + let left = queue.insert(live(-28, 0)); + let mut right = queue.insert(live(-28, 0)); assert_eq!(root.current(), 0); @@ -952,21 +986,21 @@ mod tests { // Insert in random order let mut handles = vec![ - queue.insert(live(100, 5)), - queue.insert(live(200, 3)), - queue.insert(live(50, 10)), - queue.insert(live(200, 8)), - queue.insert(live(100, 1)), + queue.insert(live(-28, 5)), + queue.insert(live(72, 3)), + queue.insert(live(-78, 10)), + queue.insert(live(72, 8)), + queue.insert(live(-28, 1)), ]; // Verify highest priority is at index 0 - // track=200, group=8 should be highest + // track=72, group=8 should be highest assert_eq!(handles[3].current(), 0); // Remove highest priority drop(handles.remove(3)); - // Next highest should now be at 0 (track=200, group=3) + // Next highest should now be at 0 (track=72, group=3) assert_eq!(handles[1].current(), 0); } @@ -975,10 +1009,10 @@ mod tests { let queue = PriorityQueue::default(); // Fill vec to capacity - 1 - let _fillers: Vec<_> = (0..254).map(|i| queue.insert(live(100, i + 100))).collect(); + let _fillers: Vec<_> = (0..254).map(|i| queue.insert(live(-28, i + 100))).collect(); // Insert one more that will be at the edge - let mut at_edge = queue.insert(live(100, 50)); + let mut at_edge = queue.insert(live(-28, 50)); assert_eq!(at_edge.current(), 254); // Spawn task to wait for demotion notification @@ -987,7 +1021,7 @@ mod tests { moq_clock::sleep(std::time::Duration::from_millis(10)).await; // Insert very high priority item, kicking at_edge to overflow - let _high = queue.insert(live(255, 1000)); + let _high = queue.insert(live(127, 1000)); let new_priority = task.await.unwrap(); assert_eq!(new_priority, u8::MAX, "Should be demoted to overflow"); @@ -997,16 +1031,16 @@ mod tests { fn test_empty_after_all_removed() { let queue = PriorityQueue::default(); - let h1 = queue.insert(live(100, 0)); - let h2 = queue.insert(live(200, 0)); - let h3 = queue.insert(live(50, 0)); + let h1 = queue.insert(live(-28, 0)); + let h2 = queue.insert(live(72, 0)); + let h3 = queue.insert(live(-78, 0)); drop(h1); drop(h2); drop(h3); // Queue should be empty, next insert should get index 0 - let mut h4 = queue.insert(live(100, 0)); + let mut h4 = queue.insert(live(-28, 0)); assert_eq!(h4.current(), 0); } @@ -1014,11 +1048,11 @@ mod tests { fn test_steer_track_reorders() { let queue = PriorityQueue::default(); - // Subscription 1 (track=255), Subscription 2 (track=55) - let mut s1_g1 = queue.insert(live(255, 1)); - let mut s1_g2 = queue.insert(live(255, 2)); - let mut s2_g1 = queue.insert(live(55, 1)); - let mut s2_g2 = queue.insert(live(55, 2)); + // Subscription 1 (track=127), Subscription 2 (track=-73) + let mut s1_g1 = queue.insert(live(127, 1)); + let mut s1_g2 = queue.insert(live(127, 2)); + let mut s2_g1 = queue.insert(live(-73, 1)); + let mut s2_g2 = queue.insert(live(-73, 2)); assert_eq!(s1_g2.current(), 0); // s1 highest assert_eq!(s1_g1.current(), 1); @@ -1026,10 +1060,10 @@ mod tests { assert_eq!(s2_g1.current(), 3); // Swap track priorities for each handle individually. - s1_g1.steer(55, Order::Descending); - s1_g2.steer(55, Order::Descending); - s2_g1.steer(255, Order::Descending); - s2_g2.steer(255, Order::Descending); + s1_g1.steer(-73, Order::Descending); + s1_g2.steer(-73, Order::Descending); + s2_g1.steer(127, Order::Descending); + s2_g2.steer(127, Order::Descending); assert_eq!(s2_g2.current(), 0); // s2 now highest assert_eq!(s2_g1.current(), 1); @@ -1042,8 +1076,8 @@ mod tests { let queue = PriorityQueue::default(); // h_low at index 1, will be promoted to 0 when h_high is demoted. - let mut h_high = queue.insert(live(255, 1)); - let mut h_low = queue.insert(live(50, 1)); + let mut h_high = queue.insert(live(127, 1)); + let mut h_low = queue.insert(live(-78, 1)); assert_eq!(h_low.current(), 1); @@ -1052,7 +1086,7 @@ mod tests { moq_clock::sleep(std::time::Duration::from_millis(10)).await; // Demote h_high below h_low. - h_high.steer(10, Order::Descending); + h_high.steer(-118, Order::Descending); let new_priority = task.await.unwrap(); assert_eq!(new_priority, 0, "h_low should be promoted to the top"); @@ -1062,16 +1096,16 @@ mod tests { fn test_steer_track_self() { let queue = PriorityQueue::default(); - let mut h_high = queue.insert(live(255, 1)); - let mut h_mid = queue.insert(live(100, 1)); - let mut h_low = queue.insert(live(50, 1)); + let mut h_high = queue.insert(live(127, 1)); + let mut h_mid = queue.insert(live(-28, 1)); + let mut h_low = queue.insert(live(-78, 1)); assert_eq!(h_high.current(), 0); assert_eq!(h_mid.current(), 1); assert_eq!(h_low.current(), 2); // Demote h_high below the others. - h_high.steer(10, Order::Descending); + h_high.steer(-118, Order::Descending); assert_eq!(h_mid.current(), 0); assert_eq!(h_low.current(), 1); @@ -1082,19 +1116,19 @@ mod tests { fn test_steer_track_swaps_demoted_vec_item_with_overflow() { let queue = PriorityQueue::default(); - // Fill vec with 255 items at track=100, groups 1..=255. + // Fill vec with 255 items at track=-28, groups 1..=255. // f1 (group=1) is the vec tail (lowest priority of the fillers). - let mut fillers: Vec<_> = (1..=255u64).map(|g| queue.insert(live(100, g))).collect(); + let mut fillers: Vec<_> = (1..=255u64).map(|g| queue.insert(live(-28, g))).collect(); // Insert a higher-track item; this kicks f1 out of vec into overflow. - let mut top = queue.insert(live(200, 0)); + let mut top = queue.insert(live(72, 0)); assert_eq!(top.current(), 0); assert_eq!(fillers[0].current(), u8::MAX, "f1 was kicked into overflow"); // Lower top's track below every filler. Without the swap, top would land // in vec at the tail while f1 stays in overflow despite having higher // priority, breaking the "every overflow item < every vec item" invariant. - top.steer(0, Order::Descending); + top.steer(-128, Order::Descending); assert!(fillers[0].current() < u8::MAX, "f1 should be promoted back into vec"); assert_eq!(top.current(), u8::MAX, "demoted top should land in overflow"); @@ -1105,15 +1139,15 @@ mod tests { let queue = PriorityQueue::default(); // Three items, all in vec; no overflow involvement. - let mut a = queue.insert(live(200, 0)); - let mut b = queue.insert(live(100, 0)); - let mut c = queue.insert(live(50, 0)); + let mut a = queue.insert(live(72, 0)); + let mut b = queue.insert(live(-28, 0)); + let mut c = queue.insert(live(-78, 0)); assert_eq!(a.current(), 0); assert_eq!(b.current(), 1); assert_eq!(c.current(), 2); // Lowering A's priority below B but above C should leave A at index 1. - a.steer(75, Order::Descending); + a.steer(-53, Order::Descending); assert_eq!(b.current(), 0); assert_eq!(a.current(), 1); assert_eq!(c.current(), 2); @@ -1123,13 +1157,13 @@ mod tests { fn test_remove_promotes_highest_priority_overflow_item() { let queue = PriorityQueue::default(); - // Fill vec to capacity with track=200. - let fillers: Vec<_> = (100..355u64).map(|g| queue.insert(live(200, g))).collect(); + // Fill vec to capacity with track=72. + let fillers: Vec<_> = (100..355u64).map(|g| queue.insert(live(72, g))).collect(); // Three overflow items with distinct priorities (same track, different groups). - let mut low = queue.insert(live(100, 1)); - let mut mid = queue.insert(live(100, 2)); - let mut high = queue.insert(live(100, 3)); + let mut low = queue.insert(live(-28, 1)); + let mut mid = queue.insert(live(-28, 2)); + let mut high = queue.insert(live(-28, 3)); assert_eq!(low.current(), u8::MAX); assert_eq!(mid.current(), u8::MAX); assert_eq!(high.current(), u8::MAX); @@ -1152,8 +1186,8 @@ mod tests { let queue = PriorityQueue::default(); // Fill vec, then insert top, kicking f1 (filler at group=1) into overflow. - let mut fillers: Vec<_> = (1..=255u64).map(|g| queue.insert(live(100, g))).collect(); - let mut top = queue.insert(live(200, 0)); + let mut fillers: Vec<_> = (1..=255u64).map(|g| queue.insert(live(-28, g))).collect(); + let mut top = queue.insert(live(72, 0)); assert_eq!(top.current(), 0); // Take ownership of f1 so we can await its promotion notification. @@ -1164,7 +1198,7 @@ mod tests { moq_clock::sim::yield_now().await; // Demoting top below every filler swaps it with f1 in overflow. - top.steer(0, Order::Descending); + top.steer(-128, Order::Descending); let promoted = task.await.unwrap(); assert!(promoted < u8::MAX, "f1 should be notified of promotion"); diff --git a/rs/moq-net/src/test_interop.rs b/rs/moq-net/src/test_interop.rs index e84d2c3ac49..be8e2770b3d 100644 --- a/rs/moq-net/src/test_interop.rs +++ b/rs/moq-net/src/test_interop.rs @@ -503,8 +503,8 @@ fn ietf_track_status_interop() { compress: false, max_cache_duration: Some(std::time::Duration::from_millis(5000)), timescale: Some(crate::Timescale::MILLI), - // The JS track's priority 2, lower-first on the wire. - priority: Some(u8::MAX - 2), + // The JS track's priority 2, lower-first on the wire as `127 - 2`. + priority: Some(125), group_order: Some(ietf::GroupOrder::Descending), }, }; diff --git a/rs/moq-net/tests/integration/subscribe_priority.rs b/rs/moq-net/tests/integration/subscribe_priority.rs index 2af297a313b..1ec161c36b1 100644 --- a/rs/moq-net/tests/integration/subscribe_priority.rs +++ b/rs/moq-net/tests/integration/subscribe_priority.rs @@ -27,8 +27,8 @@ const VERSIONS: &[&str] = &[ "moq-transport-22", ]; -const LOW: u8 = 1; -const HIGH: u8 = 200; +const LOW: i8 = -100; +const HIGH: i8 = 100; fn produce_origin(hop: u64) -> origin::Producer { let (producer, driver) = origin::Producer::new(origin::Config::new(Hop::new(hop).unwrap())); @@ -37,7 +37,7 @@ fn produce_origin(hop: u64) -> origin::Producer { } /// Wait until the subscription `track` serves asks for `want`. -async fn priority(track: &moq_net::track::Producer, want: u8) -> Result<(), String> { +async fn priority(track: &moq_net::track::Producer, want: i8) -> Result<(), String> { let mut track = track.clone(); let reached = moq_clock::timeout(TIMEOUT, async { while track @@ -85,7 +85,7 @@ async fn follows(version: Version, relay: bool) -> Result<(), String> { .await .expect("resolve timeout") .expect("broadcast resolves"); - let subscribe = |priority: u8| { + let subscribe = |priority: i8| { let track = remote.track("video").unwrap(); async move { moq_clock::timeout( diff --git a/rs/moq-room/MIGRATE.md b/rs/moq-room/MIGRATE.md index de8cc3119cd..1149a75e95a 100644 --- a/rs/moq-room/MIGRATE.md +++ b/rs/moq-room/MIGRATE.md @@ -4,6 +4,10 @@ How to update your code for each breaking change, newest first. ## Unreleased +### `chat::PRIORITY` is signed (#6142) + +`chat::PRIORITY` is an `i8`, still 10. + ### Chat events carry their send time `chat::Subscriber::recv` / `poll_recv` return `moq_net::Timed`: `.value` is the event they used to return, and `.at` is its frame's timestamp. diff --git a/rs/moq-room/src/chat.rs b/rs/moq-room/src/chat.rs index 0c7efb389e6..d98d56e17eb 100644 --- a/rs/moq-room/src/chat.rs +++ b/rs/moq-room/src/chat.rs @@ -14,7 +14,7 @@ use std::{ /// Name of the track carrying the chat window. pub const TRACK: &str = "chat"; /// Delivery priority, below audio and video. -pub const PRIORITY: u8 = 10; +pub const PRIORITY: i8 = 10; /// How long a published message stays in the window. pub const HISTORY: Duration = Duration::from_secs(10); /// A message entering, leaving, or missed from the window. diff --git a/swift/MIGRATE.md b/swift/MIGRATE.md index 3402b613c65..b57dcd64d06 100644 --- a/swift/MIGRATE.md +++ b/swift/MIGRATE.md @@ -4,6 +4,12 @@ How to update your code for each breaking change, newest first. ## Unreleased +### Priorities are signed (#6142) + +`TrackInfo.priority`, `Subscription.priority`, `FetchGroupOptions.priority`, and `GroupRequest.priority` are `Int8`, higher first, and 0 is the unset midpoint. +`TrackInfo.priority` defaulted to 127 and now defaults to 0. +To keep a value's moq-lite byte, subtract 128 from it, widening first so it doesn't trap (`Int8(Int16(old) - 128)`): 255 becomes 127, 128 becomes 0, and 0 becomes -128. + ### A client asks only about what it watches (#6165) On moq-lite, a client session asks its relay for announcements only under paths something watches, through `announced()` or `announcedBroadcast()`, instead of under its whole scope at connect. diff --git a/swift/Sources/Moq/Dynamic.swift b/swift/Sources/Moq/Dynamic.swift index 8dc8c5539f9..211d7157dc3 100644 --- a/swift/Sources/Moq/Dynamic.swift +++ b/swift/Sources/Moq/Dynamic.swift @@ -85,7 +85,7 @@ public final class GroupRequest: Sendable { } /// The consumer's delivery priority for this fetch. - public var priority: UInt8 { + public var priority: Int8 { ffi.priority() }