Repository navigation
Conversation
The first object on a moq-transport fetch stream carries the Publisher Priority every later object inherits. Both publishers hardcoded it as 0, the most urgent value, whatever the track's priority, while subgroup headers, datagrams and FETCH_OK all state the track's. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The two remaining moq-transport priority crossings without a test are the forks a draft-22 fill reads the cache through: the SUBSCRIBE fill's snapshot and the fork an update's fill holds. Replacing either fromWire(msg.subscriberPriority) with the bare wire byte left every test green. Every other crossing, Rust and JS, already fails under that mutation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator
|
Thank you @kkroo for this fix and the thorough tests! It continues in moq-dev/moq#7790, which keeps your two commits and authorship unchanged and adds a merge of Why a new PR: this branch lives on the organization-owned fork (Written by Claude Opus 5.5) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
A moq-transport fetch stream's first object carries the Publisher Priority that every later object inherits, and
ietf/publisher.rshardcoded it asSome(0): the most urgent value on the lower-first wire, whatever the track'spriorityis. Subgroup headers and datagrams on the same publisher sendto_wire(track.info().priority), and FETCH_OK's own properties state that priority too, so a fetched object disagreed with the FETCH_OK above it. A relay or peer ranking by publisher priority could not tell a catalog fetch from a video one, and every fetch outranked every group stream. The JS publisher had the same gap (w.u8(0)inFetchFrame.encode), and its draft-18 joining-FETCH test asserted that0beside a FETCH_OK stating127.Approach
Thread the track's wire priority into the three Rust fetch writers (
write_fetched_groupfor standalone/joining FETCH,write_fetch_groupfor draft-22 fills) and into JSFetchFrame, throughFetchAnswer,RunFill, and the joining subscription'sJoinable. No conversion is added: each site uses the existingpriority::to_wire/toWire, as the subgroup path does.Tests, each red on the hardcoded value:
a_standalone_fetch_states_the_track_priority(draft-18 and draft-22) anda_fill_states_the_track_priority: a track at model priority 72 must put wire55on the first fetch object and nothing on the rest. Reverting the one production line toSome(0)fails both withleft: [Some(0), ...].a_joining_fetch_is_answered_with_fetch_ok(Rust) and the draft-18 joining-FETCH test (JS) asserted the old0beside a FETCH_OK stating 127; both now expect 127.a FETCH states the track's priority on its first object, and theFetchFramebyte test now at priority 55.Impact
FetchFrame(JS,ietf/object.ts) takes a requiredpriorityin its constructor.Alternatives
Omitting the priority on the first object was not an option: there is no prior object to inherit from.
Follow-ups
The subscriber does not read FETCH_OK's priority, which is by design since #4974: it learns a track from SUBSCRIBE_OK or TRACK_STATUS_OK before any FETCH.