Repository navigation
Conversation
Quest-Claim: b05d29a00866f339
Timestamps keep moving forward across a declared end, so the shared clock needs no segment mapping. Delete moq_mux::sync's transitions (Transition, Mapping, Resume, declare/end/plan/apply/abandon/gap/retire/map) and the segment state behind a second lock. moq play's video track resets its part of the clock at the first later group after a declared end, and audio starts the next utterance on the same clock, as @moq/watch does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ecoder Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
Reviewed the full diff: the segment-transition model is removed from moq_mux::sync, and moq play's video and audio paths now follow @moq/watch's reset rule.
moq_mux::syncslimmed to the clock —Transition,Mapping,Resume,sync::Error/Resultand the transition methods onTrackare gone.Tracknow holds only{clock, role}, so each value sits behind one lock. No other crate in the workspace uses the removed API, andreleasehas nosync.rs, so no migration note is needed.- Video resets at a later group after a declared end — receive opens a new
Segmentonly forgroup > end, which matches JS#onRead'sread.group <= endedguard, and callsrole.reset()there.Step::Nextadvances once the front drains and the codec has flushed. Generation bookkeeping stays sound becauseStep::Nextis reachable only afterFlushclears the decoder's generation. - Backpressure kept across queued segments — receive still pauses while a segment behind the front has waited past
max_delay + held. The new slow-decoder test covers this. - Max-delay decoder unfenced — the
fence/origin/advancinggating is removed, since nothing waits on a mapping any more. - Audio carries its clock through a declared end —
Crossing::End { ahead }replaces the resume wait.aheadreproduces the old deferred-endsink.end()path for an utterance whose end was read before its first PCM. - Tests rewritten to the new model — tests built on held replacements are deleted, and the remaining ones now publish live with continuing timestamps.
claude-opus-5-5 | 𝕏
|
No actionable comments were generated in the recent review. WalkthroughThe sync module removes encoder-segment mapping and transition APIs. Tracks contribute received timestamps directly to the shared clock. Audio playback handles declared ends using original frame timestamps. Video playback advances through buffered segments without presentation transition resumption. Tests and supporting documentation were updated to reflect these changes. Merge Risk:
|

Completes quest/m2/sync-segments.md and deletes it.
Problem
moq_mux::syncmapped encoder segments onto the shared clock across declared ends. It assumed a replacement encoder might restart its timestamps, which the protocol forbids (doc/concept/identity.md: keep timestamps moving past the old track's). The model had these parts:Transitionper restart, shared by roles and paired by their earliest plan;Mappingthat rebased each replacement;Resumethat waited for the old tail;Its state was split two ways:
Pendingkept three parallelmembers/resumed/plansarrays.@moq/watchruns none of this, so Rust ran a third model that the deleted mux-sync quest said not to invent. The maintainer picked option (a) on moq-dev/moq#8764: delete the transitions and reset the way JS does.Approach
moq_mux::sync(1,894 → 979 lines):Transition,Mapping,Resume,sync::Error/Result, andTrack::{declare, end, plan, apply, abandon, gap, retire, map, resume}, with the state behind them.Trackkeepsreceived,reset,set_target, andpoll_delay_changed, and holds only the clock and its role. Each value now sits behind one lock.moq playfollows@moq/watch's rule:Track::reset) at the first frame of a group later than a declared end's, asjs/watch/src/video/decoder.ts#onBreakdoes.js/watch/src/audio/decoder.tsdoes. The sink plays comfort noise afterend()and starts the next utterance withnext(). An end already queued past that utterance's first frame still declares its silence (Crossing::End { ahead }).Buffer::end,Buffer::clear, and the hold behind a retired tail.JS is unchanged: it already ran this rule.
Impact
moq-mux: removessync::{Transition, Mapping, Resume, Error, Result}and the transition methods onsync::Track.moq_mux::syncis unreleased (thereleasebranch has nosync.rs), so this needs no!and no MIGRATE entry. The CHANGELOG'spoll_changedline drops its mention of transition bookkeeping.moq play: media after a declared end plays on the timestamps it carries.Choices made without asking
Track::reset's name (it matches JSTrack.reset()).retirebecame plain drop, since nothing waits on a tail any more.a_held_video_resume_leaves_the_excess_to_the_subscription,a_held_audio_resume_leaves_the_excess_to_the_subscription, and every transition test insync.rs. Addedsegments_behind_a_slow_decoder_leave_the_excess_to_the_subscriptionfor the backpressure (it decodes all 100 pictures with the check disabled, under 50 with it). Rewrote the rest to the new model:an_emptied_superseded_segment_is_discardedandan_evicted_replacement_gop_gives_way_to_the_next_replacementnow publish live, with no audio hold.a_restart_the_max_delay_decoder_reads_first_plays_on_the_clockasserts the replacement shows at its due time.quick_resumessends the replacement live, since timestamps run on through the end.video.rs'sMappedtoOrdered, since nothing maps any more.Validation
just test push-to-talkpasses. It runs the nativemoq playagainst the JS push-to-talk publisher, 22 presses. For every press, the native player resumed on the utterance's head. The head lag stayed flat at -551 to -555 ms, with two at -531 and -534 and no drift; fix(mux): back-to-back push-to-talk presses keep their latency #8589 measured -608 flat. Native playback had 0 underruns and 0 stalls, and every separated mute played comfort noise.cargo test -p moq-cli --features play --bin moq play::: 86 passed, 1 failed. The failure isa_wide_delay_observes_the_whole_tune_in_burst, which this host can't build:NoEncoder("nothing compiled in for H264 at Software"). The test doesn't touch sync.cargo test -p moq-mux --lib sync: 52 passed.just check: passes.Caused by
js/watch's sync, rather than porting JS's reset (found by reviewer: the pre-cut feature review, moq-dev/moq#8764).Follow-ups
None.
Models
Size: M
Implementer: Claude Code, claude-opus-5-5, unknown effort (arm A)
(Written by Claude Opus 5.5)
Review
Rewritten by
quest reviewafter each round.Head: e265b74
Base: origin/main at 0e6e469
Risk: medium,
the base has no RISK.md to name what is always high or customer-facing, so every change is mediumClassified: medium by default
Reviewer: Codex, gpt-6.1-sol, high effort
Session:
01a1dfe6-16ec-7dd2-af0a-f4a001f71390, round 2Score: 10/10,
The restored queue-wide backpressure fixes the retention issue, and source review found no new problems in the incremental change.Rounds: 2; first round 1 (high 1); findings 1 (high 1): fixed 1, refuted 0, open 0
Findings: none
The last round's findings, at ec5c2dc:
rs/moq-cli/src/play/video.rs:235:Preserve backpressure across queued video segments;In rs/moq-cli/src/play/video.rs:228, receive pauses when nonempty segments behind the front exceed the age budget and resumes on queue advancement, leaving excess media upstream for subscription eviction; the added slow-decoder regression test covers this scenario, though tests were not run during this review.