Skip to content

moq-mux: add seek(sequence) on importers for explicit group boundaries#1515

Merged
kixelated merged 1 commit into
mainfrom
claude/moq-mux-group-start-cnY8i
May 27, 2026
Merged

moq-mux: add seek(sequence) on importers for explicit group boundaries#1515
kixelated merged 1 commit into
mainfrom
claude/moq-mux-group-start-cnY8i

Conversation

@kixelated

Copy link
Copy Markdown
Collaborator

Summary

Adds seek(sequence: u64) to every moq-mux importer (and to the underlying container::Producer<C>) so callers can open the next group at an explicit sequence instead of always starting at 0. Useful for joining mid-stream and signalling discontinuities.

Default auto-rotate behavior (video → new group on keyframe, AAC/Opus → one frame per group, fMP4 → new group when a traf contains a sync sample) is unchanged when seek is not invoked, so existing callers (moq-cli, moq-gst, libmoq, moq-ffi, moq-boy, hang) are unaffected.

The standalone AAC/Opus "one-frame-per-group" finalize stays as-is for now. A multi-frame-per-group knob for LL-HLS-style import was discussed and deferred; seek alone solves the join/discontinuity case.

How it works

  • container::Producer<C> gains a pending_sequence: Option<u64> field. seek(n) flushes + finishes the current group and stores Some(n). The next write opens the new group via TrackProducer::create_group(Group { sequence: n }) instead of append_group(), then clears the pending slot so subsequent groups auto-increment from n.
  • Codec importers (h264, h265, av1, aac, opus) forward seek to their inner container::Producer.
  • container::fmp4::Import and container::mkv::Import fan seek out across every per-track entry so all renditions advance together. (Broadcast-wide by design; per-track control intentionally not exposed.)
  • import::Framed and import::Stream dispatchers forward to the contained importer.

Test plan

  • cargo test -p moq-mux (144/144 passing, including 2 new unit tests on container::Producer and 1 new end-to-end test on fmp4::Import).
  • cargo check --all-targets --workspace (excluding moq-boy/moq-gst which need system ffmpeg/gstreamer in this sandbox).
  • cargo clippy --all-targets --workspace -- -D warnings (same exclusions).
  • cargo fmt --all --check.

(Written by Claude)

https://claude.ai/code/session_01E5uGooj9DFQm7Hc1fy9PYL


Generated by Claude Code

Lets callers join mid-stream or signal discontinuities by opening the next
group at a chosen sequence instead of always starting at 0. The default
auto-rotate behavior on every importer is unchanged when seek is not
invoked.

`container::Producer<C>::seek` stashes a pending sequence used by the next
group opened in `write`. fmp4/mkv `Import::seek` fans out across every
track so all renditions advance together. Framed and Stream dispatchers
forward to the underlying importer.

https://claude.ai/code/session_01E5uGooj9DFQm7Hc1fy9PYL
@coderabbitai

coderabbitai Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

This PR adds a seek(sequence: u64) method throughout the moq-mux media import pipeline. The core mechanism in Producer<C> introduces a pending_sequence field that schedules the next group at an explicit sequence number instead of auto-incrementing. Container implementations (fMP4, MKV) and codec importers (AAC, AV1, H264, H265, Opus) delegate to their underlying track producers. Top-level importers (Framed, Stream) dispatch seek calls to the active decoder variant. The fMP4 container adds per-track pending sequence state and tests validate that seek sets the initial group sequence and subsequent groups auto-increment correctly.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: adding seek(sequence) methods to moq-mux importers for explicit group boundary control, which is the primary focus across all modified files.
Description check ✅ Passed The description is comprehensive and directly related to the changeset, explaining the motivation (join mid-stream, signal discontinuities), implementation approach across all layers, 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
  • Commit simplified code in branch claude/moq-mux-group-start-cnY8i

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.

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@rs/moq-mux/src/container/fmp4/import_test.rs`:
- Line 10: The test helper is silently swallowing errors by turning Err into
None using .now_or_never().and_then(|r| r.ok().flatten()); update the loop to
explicitly handle the Result from consumer.recv_group(): call
consumer.recv_group().now_or_never(), match on Some(Ok(Some(group))) to
continue, Some(Ok(None)) to break, and treat Some(Err(e)) as a test-failure
(panic or propagate) so recv_group errors fail fast instead of being hidden.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 99340bf4-8eb6-42db-be3b-244c3add73df

📥 Commits

Reviewing files that changed from the base of the PR and between b6e7f4c and 670b7de.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (11)
  • rs/moq-mux/Cargo.toml
  • 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/container/fmp4/import.rs
  • rs/moq-mux/src/container/fmp4/import_test.rs
  • rs/moq-mux/src/container/mkv/import.rs
  • rs/moq-mux/src/container/producer.rs
  • rs/moq-mux/src/import.rs

#[cfg(test)]
fn drain_group_sequences(consumer: &mut moq_net::TrackConsumer) -> Vec<u64> {
let mut sequences = Vec::new();
while let Some(group) = consumer.recv_group().now_or_never().and_then(|r| r.ok().flatten()) {

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.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Don’t silently swallow recv_group errors in the test helper.

Line 10 converts Err into None, which can hide real failures and let tests pass for the wrong reason. Fail fast on Some(Err(_)).

Suggested fix
 fn drain_group_sequences(consumer: &mut moq_net::TrackConsumer) -> Vec<u64> {
 	let mut sequences = Vec::new();
-	while let Some(group) = consumer.recv_group().now_or_never().and_then(|r| r.ok().flatten()) {
-		sequences.push(group.sequence);
-	}
+	loop {
+		match consumer.recv_group().now_or_never() {
+			Some(Ok(Some(group))) => sequences.push(group.sequence),
+			Some(Ok(None)) | None => break,
+			Some(Err(err)) => panic!("recv_group failed: {err:?}"),
+		}
+	}
 	sequences
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@rs/moq-mux/src/container/fmp4/import_test.rs` at line 10, The test helper is
silently swallowing errors by turning Err into None using
.now_or_never().and_then(|r| r.ok().flatten()); update the loop to explicitly
handle the Result from consumer.recv_group(): call
consumer.recv_group().now_or_never(), match on Some(Ok(Some(group))) to
continue, Some(Ok(None)) to break, and treat Some(Err(e)) as a test-failure
(panic or propagate) so recv_group errors fail fast instead of being hidden.

@kixelated kixelated merged commit e9a1086 into main May 27, 2026
1 check passed
@kixelated kixelated deleted the claude/moq-mux-group-start-cnY8i branch May 27, 2026 14:56
@moq-bot moq-bot Bot mentioned this pull request May 26, 2026
kixelated added a commit that referenced this pull request May 27, 2026
…drasekhar-da73a5

Conflicts in moq-mux Import structs where main (#1515) added a `seek`
method using anyhow::Result; reconciled to return the per-module
crate::Result/Result variant matching the rest of this branch's refactor.
The h264/h265/av1 seek variants swap `.context("not initialized")` for
`.ok_or(Error::NotInitialized)?` using each module's NotInitialized
variant.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@kixelated kixelated mentioned this pull request May 27, 2026
2 tasks
@moq-bot moq-bot Bot mentioned this pull request May 30, 2026
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.

2 participants