feat(moq-mux): cap pending AU growth and add discontinuity() by arielmol · Pull Request #1770 · moq-dev/moq

arielmol · 2026-06-16T22:43:20Z

Summary

Two additive, internal hardening changes to the moq-mux codec importers.
No wire-protocol or breaking API change, so this targets main.

Bound the in-progress access unit. H.264/H.265/AV1 append every
NAL/OBU to a pending buffer and only flush on a slice/frame; a stream
that never sends one (repeated AUD/SEI/SPS/PPS, or non-frame OBUs) grew
it without limit. Every append, including the SPS/PPS/VPS re-inserted
before a keyframe, now goes through a checked helper that bails past
MAX_PENDING_FRAME_BYTES (16 MiB). moq-gst's per-buffer limit does not
bound accumulation across buffers, so this is the right layer for it.
Add discontinuity() to signal a timeline boundary (e.g. a
GStreamer FLUSH). It closes the open group via
Producer::finish_group() so the next frame starts a fresh group, and
for H.264/H.265/AV1 also drops the partial access unit. It does not use
seek(0) (which would rewrite the group sequence) and does not touch
lenient_start, so it stays forward compatible with the stricter keyframe
contract in fix(moq-mux): reject frames before the first keyframe instead of dropping #1766.

Public API (all additive, non-breaking)

Framed::discontinuity()
discontinuity() on the h264/h265/av1/vp8/vp9/aac/opus
codec importers.

Not in this PR

The moq-gst Pad::flush() caller that will invoke Framed::discontinuity()
ships with the moqsink promotion (it lives on a separate branch). This PR is
the moq-mux half only.

Test plan

cargo test -p moq-mux: 243 passed (10 new: cap + discontinuity per
codec, an end-to-end no-leak test, and a group-boundary test).
The group-boundary test was verified faithful: deleting
finish_group() from discontinuity() makes it fail (2 frames vs 1).
cargo clippy -p moq-mux --all-targets clean, cargo fmt --check clean.

(Written by Claude)

Two additive, internal hardening changes to the codec importers (no wire or breaking API change). Bound the in-progress access unit: H.264/H.265/AV1 append every NAL/OBU to a pending `chunks` buffer and only flush it on a slice/frame, so a stream that never sends one (repeated AUD/SEI/SPS/PPS, or non-frame OBUs) grows it without limit. Every append now goes through a checked helper that bails past MAX_PENDING_FRAME_BYTES (16 MiB), including the cached SPS/PPS/VPS re-inserted before a keyframe. Add `discontinuity()` to signal a timeline boundary (e.g. a GStreamer FLUSH): it closes the open group via `Producer::finish_group()` so the next frame starts a fresh group, and for H.264/H.265/AV1 also drops the partial access unit. Exposed on `Framed` and on every framed codec importer; the moq-gst caller lands with the moqsink promotion (it lives on another branch). CONTEXT Discarded paths: - `seek(0)` as the discontinuity primitive: it rewrites the group sequence explicitly and fabricates false semantics. `finish_group()` closes the group without inventing a sequence. - No-op discontinuity for codecs without a partial AU (VP8/VP9/AAC/Opus): they still `finish_group()` so a FLUSH closes a clean group on every track, not only the video ones. - Toggling lenient_start or dropping deltas until a keyframe inside discontinuity(): that revives `with_lenient_start` under another name. discontinuity() defers to the Producer keyframe contract, forward compatible with the stricter contract in PR moq-dev#1766. - Leaving the SPS/PPS/VPS re-insertion on a raw `extend_from_slice`: a large cached parameter set would allocate past the cap before the final append checked it. Routing it through the helper validates before mutating. - Including the moq-gst `Pad::flush()` caller here: moqsinkspike lives on the gstsink branch, not main, so the caller ships with the promotion PR. - A `Stream` dispatcher discontinuity(): moqsinkspike only uses `Framed`. Key decisions: - discontinuity() means "timeline boundary", not "clear partial AU if any": all codecs close the group, only H.264/H.265/AV1 also clear the partial AU. - The cap is hardening against malformed input (a normal stream flushes every frame). moq-gst's per-buffer 16 MiB limit does not bound accumulation across buffers. - A discontinuity test followed by a keyframe does not prove finish_group() (the keyframe closes the group anyway), so the Framed-level test feeds a delta after discontinuity() and asserts it does not extend the closed group. Verified faithful by deleting finish_group() and watching it fail. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

coderabbitai · 2026-06-16T22:46:35Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: de993b13-00e8-454f-8238-d66a9e778e6e

📥 Commits

Reviewing files that changed from the base of the PR and between 2788d79 and 8461707.

📒 Files selected for processing (8)

rs/moq-mux/src/codec/aac/import.rs
rs/moq-mux/src/codec/av1/import.rs
rs/moq-mux/src/codec/h264/import.rs
rs/moq-mux/src/codec/h265/import.rs
rs/moq-mux/src/codec/opus/import.rs
rs/moq-mux/src/codec/vp8/import.rs
rs/moq-mux/src/codec/vp9/import.rs
rs/moq-mux/src/import.rs

Walkthrough

A discontinuity() method is added to all seven codec importers (H264, H265, AV1, AAC, Opus, VP8, VP9) and to the top-level Framed struct. When called, it finishes the current track group so the next decoded frame starts a fresh group; for video codecs it also clears any partially accumulated access unit or temporal unit. H264, H265, and AV1 additionally gain a MAX_PENDING_FRAME_BYTES cap enforced through new append_nal/append_obu helpers (with a companion clear_partial reset), replacing direct buffer extension in their decode paths. Container formats (fMP4, MKV, TS, FLV) treat discontinuity() as a no-op.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately captures both main changes: capping pending AU growth and adding the discontinuity() method across multiple codec importers.
Description check	✅ Passed	The description clearly relates to the changeset, providing detailed context on the two additive hardening changes, public API additions, and test coverage.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

✨ Simplify code

Create PR with simplified code

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 and usage tips.}

arielmol · 2026-06-17T15:44:51Z

Well the rabbit left no comments? 🐰

kixelated · 2026-06-17T20:48:15Z

+	/// Signal a timeline discontinuity: close the current group so the next
+	/// frame starts a fresh group.
+	pub fn discontinuity(&mut self) -> anyhow::Result<()> {
+		self.track.finish_group()?;


? each frame should be its own group

Right, for AAC/Opus each frame already calls finish_group() after write() so discontinuity(..) here just refinishes a closed group already (no-op), I'll drop discontinuuty from the audio importers and keep it only on video

kixelated · 2026-06-17T20:49:03Z

+/// Cap on a single in-progress temporal unit. Without a frame OBU the importer
+/// never flushes `chunks`, so a stream of non-frame OBUs (sequence headers,
+/// metadata) would grow it unboundedly; bail instead.
+const MAX_PENDING_FRAME_BYTES: usize = 16 * 1024 * 1024;


Maybe we should export moq_net::MAX_FRAME_SIZE and use that instead?

Maybe we should export moq_net::MAX_FRAME_SIZE and use that instead?

Agreed

kixelated · 2026-06-17T20:50:56Z

+	/// Signal a timeline discontinuity: drop the in-progress temporal unit and
+	/// close the current group so the next frame starts a fresh group with a
+	/// keyframe. Tolerant before the track exists.
+	pub fn discontinuity(&mut self) -> anyhow::Result<()> {


IDK I really don't like this TS stuff leaking everywhere. Partially writing a frame is just gross.

I'm architecting moq-mux on dev now. There's a separate av1::Split module that finds frame boundaries, while the dumb av1::Import takes entire frames (not a stream). TS and stdin can use av1::Split, while everybody else uses av1::Import directly instead.

Your call, just two questions:

is it ok if I land just the pending AU cap here for now?

once Split exists, where should a FLUSH hook in? SPlit or still surfaced via Framed? (moqsink will need this)

kixelated reviewed Jun 17, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(moq-mux): cap pending AU growth and add discontinuity()#1770

feat(moq-mux): cap pending AU growth and add discontinuity()#1770
arielmol wants to merge 1 commit into
moq-dev:mainfrom
arielmol:moqmux

arielmol commented Jun 16, 2026

Uh oh!

coderabbitai Bot commented Jun 16, 2026

Uh oh!

arielmol commented Jun 17, 2026

Uh oh!

kixelated Jun 17, 2026

Uh oh!

arielmol Jun 17, 2026

Uh oh!

kixelated Jun 17, 2026

Uh oh!

arielmol Jun 17, 2026 •

edited

Loading

Uh oh!

kixelated Jun 17, 2026 •

edited

Loading

Uh oh!

arielmol Jun 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

arielmol commented Jun 16, 2026

Summary

Public API (all additive, non-breaking)

Not in this PR

Test plan

Uh oh!

coderabbitai Bot commented Jun 16, 2026

Walkthrough

Uh oh!

arielmol commented Jun 17, 2026

Uh oh!

kixelated Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

arielmol Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

kixelated Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

arielmol Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kixelated Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

arielmol Jun 17, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

arielmol Jun 17, 2026 •

edited

Loading

kixelated Jun 17, 2026 •

edited

Loading