Repository navigation
A replayed media file never repeats a timestamp - #113
Conversation
Quest-Claim: 94e6f45a60d0d01c
A cut's play begins after the last picture already encoded onto the media source's own track. Pictures encode a bus latency ahead of when they show, so one handed over just before a restart was published at the same moment the restart's keyframe was stamped with. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 53 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
kixelated
left a comment
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed head: ef33e9d
No new defects found in the interrupted-play fix. I read source.rs in full, followed the publication/encoder timestamp paths, and checked the existing media restart and A/V tests. Own retains the last submitted picture's show time across cuts; a fresh play selects a later begin, shared by both audio and video. Continuous loops keep their existing scheduled begin rather than accumulating the restart adjustment. The strict timestamp assertion remains unchanged. This respects the recorded decision to leave an already-encoded, subsequently cut picture on the published track.
Separate pre-existing follow-up, not a defect introduced by this PR: a video-only file that decodes to one picture gets a zero final-picture duration at rs/moq-scene/src/source.rs:1726. With looping enabled, next = Some(end) at :1552 then reuses the same begin, so successive loops can submit the same timestamp. The importer does not exclude this input. A separate fix should establish the lone frame's duration and cover that input; the same behavior is present on the base branch.
Direction: no revisions requested for this cut/restart change. Keep the single-frame loop-duration issue separate.
Verification limits: GitHub-only static review. I did not run quest context, builds, tests, or reproduce either scenario. The repeated-run and just ci results are reported by the author, not independently verified here. The Check workflow for this head was in progress when inspected.
When a media source starts over while it's on program, it now publishes picture timestamps that always increase.
publish::tests::a_media_file_is_published_on_one_timelinepasses on every run.Root cause
This is a real bug in
rs/moq-scene/src/source.rs. The test doesn't read too early. Media pictures are encoded onto the source's own track one bus latency (50 ms) before they show, and each is stamped with the time it will show. Whenfeed.play()restarts the clip, the cut clears the pictures waiting to be displayed. A picture handed over just before the cut is already on the track, though. The new play starts atnow + BUS_LATENCY, which can be that same moment. The failing runs show this:... 10450ms, 10550ms, 10550ms, 10650ms ....Caused by
Fix
Ownnow records when the last picture encoded onto the track shows (stamped). After a cut, the new play starts at whichever is later:now + BUS_LATENCYorstamped + 1 ms. Loops aren't affected, because each loop already starts where the last play ended.Verification
mainat 4c4a219 (macOS).just cipasses.just cipasses.Choices made without asking
aheadalready accounts for.Decisions
media's display loop and drop encoding ahead). Maintainer, 2026-11-08.🤖 Generated with Claude Code
(Written by Claude Opus 5.5 with low)
Review
Rewritten by
quest reviewafter each round.01a1bc60-8dab-7252-a4f6-84c8ec0132cc, round 1The change preserves increasing publication timestamps across media restarts without shifting continuous loops, with no actionable defects found by source inspection and runtime validation remaining outside this review.