Skip to content

fix(net): a fetch stream states the track's publisher priority - #7583

Closed
kkroo wants to merge 2 commits into
moq-dev:mainfrom
Blockcast:fix/ietf-fetch-publisher-priority
Closed

kkroo wants to merge 2 commits into
moq-dev:mainfrom
Blockcast:fix/ietf-fetch-publisher-priority

Conversation

@kkroo

@kkroo kkroo commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Problem

A moq-transport fetch stream's first object carries the Publisher Priority that every later object inherits, and ietf/publisher.rs hardcoded it as Some(0): the most urgent value on the lower-first wire, whatever the track's priority is. Subgroup headers and datagrams on the same publisher send to_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) in FetchFrame.encode), and its draft-18 joining-FETCH test asserted that 0 beside a FETCH_OK stating 127.

Approach

Thread the track's wire priority into the three Rust fetch writers (write_fetched_group for standalone/joining FETCH, write_fetch_group for draft-22 fills) and into JS FetchFrame, through FetchAnswer, RunFill, and the joining subscription's Joinable. No conversion is added: each site uses the existing priority::to_wire/toWire, as the subgroup path does.

Tests, each red on the hardcoded value:

  • Rust a_standalone_fetch_states_the_track_priority (draft-18 and draft-22) and a_fill_states_the_track_priority: a track at model priority 72 must put wire 55 on the first fetch object and nothing on the rest. Reverting the one production line to Some(0) fails both with left: [Some(0), ...].
  • a_joining_fetch_is_answered_with_fetch_ok (Rust) and the draft-18 joining-FETCH test (JS) asserted the old 0 beside a FETCH_OK stating 127; both now expect 127.
  • JS a FETCH states the track's priority on its first object, and the FetchFrame byte test now at priority 55.

Impact

  • Wire: a fetch stream's first object now carries the track's Publisher Priority instead of 0 (Rust and JS publishers). Not breaking: the field was always present; only its value changes.
  • FetchFrame (JS, ietf/object.ts) takes a required priority in 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.

HangPlayerEngineer and others added 2 commits October 28, 2026 13:38
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>
@kixelated

Copy link
Copy Markdown
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 main on top.

Why a new PR: this branch lives on the organization-owned fork Blockcast/moq, and GitHub doesn't let maintainers push to organization-owned forks, even with "Allow edits by maintainers" checked, so it couldn't be updated in place. Main had since made FETCH and fill single-group, which conflicted with this branch. #7790 re-applies your change onto that code and drops the now-dead multi-group prior_group plumbing.

(Written by Claude Opus 5.5)

@kixelated kixelated closed this Nov 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants