Skip to content

fix(mux): apply fMP4 edit lists to frame timestamps - #6686

Merged
kixelated merged 4 commits into
mainfrom
quest/m1/fmp4-edit-lists
Oct 24, 2026
Merged

kixelated merged 4 commits into
mainfrom
quest/m1/fmp4-edit-lists

Conversation

@kixelated

@kixelated kixelated commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator

Quest: quest/m1/fmp4-edit-lists.md (rewritten here to its remaining interop step).

Summary

ffmpeg's fragmented MP4 carries an edit list on every AAC track (media_time 1024 at tfdt 0 under -movflags cmaf), and on H.264 with B-frames (media_time equal to the first composition offset). The importer forwarded the edts in the passthrough init and ignored it otherwise, so every subscriber played ffmpeg's AAC 1024 samples late and its B-frame video one composition delay late.

The importer now reduces each track's edit list to one shift (new private fmp4/edit.rs), applies it to every frame timestamp, and drops the samples before the edit, so no consumer needs edit list code:

  • Unwrapped tracks publish each kept sample at its presentation time. A fragment wholly before the edit publishes nothing.
  • Passthrough tracks stamp each fragment at its earliest kept presentation time. A fragment that loses samples is restated as one explicit trun starting at the first kept sample, so every kept sample keeps its decode and composition time. The catalog init drops the edts.
  • Opus applies only the part of the edit beyond the OpusHead pre-skip, which its frame timestamps already count, so ffmpeg's Opus (media_time equal to the pre-skip) is unchanged. An edit starting inside the pre-skip delays the track by the rest of it, since a consumer trims the whole pre-skip while in ISO BMFF the edit alone trims.
  • WebVTT and TTML tracks take the same edit, so captions stay on the media's timeline.

Decisions made without asking

  • Priming before the edit is dropped, not published at a negative offset: frame timestamps are unsigned. A sample straddling the edit's start is dropped too, so Apple's 2112-sample AAC priming loses 960 samples (20 ms) at the very start of the stream, and every published timestamp stays exact. A source joined mid-stream loses nothing.
  • A sync sample the edit drops takes the samples decoding from it along, until the next sync sample, so a passthrough track never opens a group on an undecodable P frame (an edit after a GOP's I frame, as a cut at a non-sync sample writes). Unwrapped video already skipped to a sync sample.
  • Refused with the new Error::UnsupportedEditList: more than one media edit, any edit after the media edit (a trailing empty edit is invalid ISO BMFF anyway), a list of only empty edits, and a media rate other than 1.
  • Error::UnsupportedEditList(&'static str) carries a fixed reason, like InvalidAspect and InvalidSubtitle beside it: a caller can't act on which shape was refused, so a typed reason would be API with no consumer.
  • The edit's duration is ignored. It trims the end, and a live source has none.
  • A dropped passthrough sample's duration goes to the kept sample before it, or into tfdt when it led the fragment, and the restated fragment drops the per-sample tables (sbgp, subs, sample auxiliary information) that would index the wrong samples. No consumer reads them, only a stream's first fragments are restated, and encrypted sample entries are refused already.
  • A cue starting before the edit is dropped rather than clipped, like a media sample.
  • Kept the export's edts strip in extract_init: other publishers can still put one in a CMAF init.
  • Left test/interop/ and quest/m1/README.md alone, since moq-dev/moq#6647 rewrites the neighbouring lines. The quest now holds only the interop step, to land after it.

Interop finding: GStreamer's AAC decoder

With the opus condition dropped from phase_checked locally, just test interop --all --publishers rust passed the AAC phase on every phase-checked subscriber except GStreamer, which read the AAC tone +878us out, 1024 samples early. That is the decoder: faad, which decodebin picks there, drops the first frame's output and stamps the rest one frame early, while ffmpeg's decoder keeps the priming at the input timestamps. An ffmpeg ADTS encode decodes with its first audible sample at 2 through faad, at 1026 through ffmpeg, and at 2770 through fdkaacdec. The old 1024-sample error happened to cancel faad's. The quest now records the choice between compensating in moqsrc and skipping GStreamer's AAC phase with the reason.

Public API and wire impact

  • Public API: adds container::fmp4::Error::UnsupportedEditList(&'static str). The enum is #[non_exhaustive], so this is additive.
  • Wire: none. Behavior change, noted in the moq-mux changelog: frame timestamps of an fMP4 import now include the edit, its first AAC frame of priming is gone, and its passthrough init has no edts. A consumer that offset AAC priming on its own must stop, or it shifts the track twice. GStreamer playback through faad now lands AAC 21 ms early (see above).

Checks

  • just check on the final head: passed (4087 tests).
  • cargo nextest run -p moq-mux: passed (1726 tests), including the new edit::tests, audio_presents_from_its_edit, and video_drops_the_samples_before_its_edit, whose dropped-I-frame case failed in passthrough before the fix.
  • just test interop --publishers rust with the AAC phase check enabled locally: passed.
  • just test interop --all --publishers rust with the AAC phase check enabled locally: passed except rust -> gst on AAC, as above. Not run with the committed interop.sh, whose only difference is that check. Not rerun after the Opus and sync-sample fixes, which change nothing for ffmpeg's output (media_time equal to the pre-skip, edits at the first I frame).

Follow-ups

  • The rewritten quest: drop the opus condition after moq-dev/moq#6647, with a decision on GStreamer's AAC decoder delay. Its quest/m1/README.md summary line goes stale until then.
  • draft-lcurley-moq-hang says nothing about an edts in a CMAF init. It could say a consumer ignores one, matching what the importer and the export now do.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

kixelated and others added 3 commits October 24, 2026 12:13
The fMP4 importer reduces each track's edit list to one shift, applies it
to every frame timestamp, and drops the samples before the edit (such as
AAC priming), so consumers need no edit list code. The passthrough init
no longer carries the edts. Opus applies only the part beyond the
OpusHead pre-skip, which its timestamps already count. An edit list with
several media edits or a rate other than 1 is refused.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kixelated kixelated changed the title fMP4 import applies edit lists to timestamps fix(mux): apply fMP4 edit lists to frame timestamps Oct 24, 2026
@kixelated
kixelated marked this pull request as ready for review October 24, 2026 19:42
@coderabbitai

coderabbitai Bot commented Oct 24, 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 11 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: f55c4e79-23d4-42f1-b2de-212884bee86e
Commits

Reviewing files that changed from the base of the PR and between 3818519 and 2de40c9.

Files selected for processing: 7
  • quest/m1/fmp4-edit-lists.md
  • rs/moq-mux/CHANGELOG.md
  • rs/moq-mux/src/container/fmp4/edit.rs
  • rs/moq-mux/src/container/fmp4/import.rs
  • rs/moq-mux/src/container/fmp4/import_test.rs
  • rs/moq-mux/src/container/fmp4/mod.rs
  • rs/moq-mux/src/container/fmp4/text.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

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-24T19:57:09.647728Z 2de40c9 New commits
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

An Opus edit starting inside the OpusHead pre-skip saturated to no shift,
misplacing every kept sample by the rest of the pre-skip; it now delays
the track by that much instead. A sync sample the edit drops also drops
the samples decoding from it, so a passthrough track no longer opens a
group on an undecodable P frame.

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

Copy link
Copy Markdown
Collaborator Author

Merging 2de40c9 with the maintainer's approval. The non-Grok review of this head is clean, every finding is fixed or answered, and the pre-land quest check passes against origin/main.

(Written by Claude Opus 5.5)

@kixelated
kixelated added this pull request to the merge queue Oct 24, 2026
Merged via the queue into main with commit 916014f Oct 24, 2026
10 checks passed
@kixelated
kixelated deleted the quest/m1/fmp4-edit-lists branch October 24, 2026 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant