Skip to content

fix(moq-mux): move a sub-tick presentation tie up one tick on the declared clock - #8715

Merged
kixelated merged 2 commits into
mainfrom
fix/declared-clock-sub-tick-tie
Nov 14, 2026
Merged

kixelated merged 2 commits into
mainfrom
fix/declared-clock-sub-tick-tie

Conversation

@kixelated

@kixelated kixelated commented Nov 14, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

A rendition that declares reorder clocks each group with DeclaredClock at its container's tick: 1 ms for FLV, 90 kHz for MKV and MPEG-TS. Two frames whose source timestamps are distinct but round onto one tick take the same presentation slot. The clock then authors two equal DTS and fails the export with video presentation slots cannot author increasing DTS at or before PTS, which ends the FLV/RTMP play.

This is easy to hit: an MP4 with a sample that repeats its predecessor's timestamp, looped through ffmpeg -c copy into MPEG-TS, publishes the repeat one 90 kHz tick (11 µs) later. Every FLV viewer then drops each time the stream reaches that frame. The FLV export authored a tie one tick apart with author_dts before moq-dev/moq#6453 moved it onto the declared clock.

Approach

DeclaredClock::push takes the frame's source timestamp along with its tick. A frame moves up to the tick after every frame still in the clock's lookback window that is presented before it, so a run of frames inside one tick presents one tick apart, in source order. Identical source timestamps still fail, as does a frame presented below one already on its tick, since neither leaves a slot to take. pop returns the PTS the frame presents at, and the FLV, MKV, and MPEG-TS exports write it.

Tests: a clock case at depth 0 and 1 (pairs, runs, identical and unplaceable ties), and an FLV export case, which fails on main with the error above.

Impact

  • No public API or wire change. FLV, MKV, and MPEG-TS exports of a declared-reorder rendition now present a sub-tick tie one tick later instead of failing.

Alternatives

  • Fail as today: refusing a tie that only rounding created ends every FLV viewer at a frame the source itself orders.
  • Bump the DTS instead of the PTS, as author_dts did: at depth 0 that decodes after presenting, a negative composition time.

Caused by

  • moq-dev/moq#6453 - moved FLV and MKV onto the declared clock, which refuses the tie that author_dts bumped.

Follow-ups

None.

Models

Size: S
Implementer: Claude Code, claude-opus-5-5, medium effort (iterate, S)

(Written by Claude Opus 5.5 with medium)

Review

Rewritten by quest review after each round.

  • Reviewer: Codex, gpt-6.1-sol, medium effort
  • Session: 01a1d82c-4208-7c41-b3bc-86d81e14df82, round 2
  • Head: ddbcc8b
  • Base: origin/main at 6f2f3ff
  • Score: 10/10, The updated tick advancement fixes ascending collision runs, and code inspection found no new issues in the incremental change.
  • Findings: none
  • The last round's findings, at 93d66a3:
    • Fixed: medium, rs/moq-mux/src/container/declared.rs:96: Preserve collision history when advancing presentation ticks; push now advances past every retained frame with an earlier source timestamp, preserving displacement across evictions; the added depth-zero case maps [40000, 40011, 40022, 40033, 80000] µs to ticks [40, 41, 42, 43, 80].

kixelated and others added 2 commits November 13, 2026 19:48
…lared clock

Two frames with distinct source timestamps can round onto one container
tick (one FLV millisecond, one 90 kHz tick). The declared clock then
authored two equal DTS and failed the export. The later-presented frame
now moves up to the next free tick, and the FLV, MKV, and MPEG-TS
exports present it there. Identical source timestamps still fail.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…view

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Nov 14, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You'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 18 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 5ea6f0d7-72fc-4f00-811d-5e896d93723c
Commits

Reviewing files that changed from the base of the PR and between 6f2f3ff and ddbcc8b.

Files selected for processing: 5

  • rs/moq-mux/src/container/declared.rs
  • rs/moq-mux/src/container/flv/export.rs
  • rs/moq-mux/src/container/flv/export_test.rs
  • rs/moq-mux/src/container/mkv/export.rs
  • rs/moq-mux/src/container/ts/export.rs
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

Share

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

I reviewed how DeclaredClock now resolves presentation ties that come from rounding, and how the FLV, MKV, and MPEG-TS exports write the moved PTS.

  • Tie lift in DeclaredClock::push: highest now carries each frame's source nanoseconds. A frame moves to the tick after every retained frame with an earlier source at or above its tick. Because the loop walks highest in ascending order, a run of ties keeps moving up. Ticks in highest and slots stay unique, so pop's DTS invariant holds.
  • New ClockError::Tie: identical source timestamps, or a frame presented below a later-sourced frame already on its tick, fail explicitly. Before, they reached the Presentation error.
  • Exports rewrite the moved PTS: FLV writes from_millis(pts) and MKV writes from_scale(pts, 90_000). TS writes ceil(pts·100/9) µs, which I checked always reads back through to_ticks as pts.
  • Tests: there are new clock cases at depth 0 and 1, and the FLV test helper now takes µs. I ran cargo test -p moq-mux --lib -- declared export_declared_reorder locally. All 27 tests that can run here pass, including sub_tick_ties_move_to_the_next_free_tick and both export_declared_reorder_decodes_each_group_from_its_frames tests. The one failure is ts::recovery_test::a_declared_discontinuity_survives_loss_either_side, which panics with "ffmpeg is in the dev shell" because ffmpeg isn't installed in this environment.

ℹ️ Remaining tie cases still end the export

These cases don't block the merge, but they behave the same as before:

  • MKV: the clock runs at 90 kHz, but feed_frame writes blocks in ms. An 11 µs lift therefore lands both frames on the same MKV block timestamp. MKV no longer fails, but it still can't present the two frames apart. MKV already collapsed sub-ms timestamps like this before this PR.
  • TS steps_back: it checks reorders_past(to_ticks(pts)) with the tick before any lift. A backward step smaller than one tick is therefore not treated as a timeline restart. push refuses it as Tie, so the export fails, as it did before through Presentation.
  • Identical source timestamps: an MP4 published directly, not through ffmpeg's TS path, can repeat a sample's timestamp exactly. That still ends the FLV play. At depth 0, decode order would decide which frame presents first, if you ever want to cover that case too.

Pullfrog  | View workflow run | Using claude-opus-5-5 | 𝕏

@kixelated
kixelated added this pull request to the merge queue Nov 14, 2026
Merged via the queue into main with commit 88508d5 Nov 14, 2026
7 checks passed
@kixelated
kixelated deleted the fix/declared-clock-sub-tick-tie branch November 14, 2026 18:39
kixelated added a commit that referenced this pull request Nov 14, 2026
Resolves the conflicts with main's declared-clock tie fix (#8715) and the
decoded-picture arrival fix (#8720), and ports main's new QUIC sim tests to
start keys and fallible time conversions.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

1 participant