[kixelated]

Problem

Malformed MKV tracks could disappear behind a validation-error allowlist, duplicate numbers replaced earlier tracks, and invalid timestamps could wrap or collapse. Unsupported MKV/FLV codecs were silently dropped. VP9 dimensions of 65536 wrapped to zero.

Approach

• Propagate track validation errors; skip only unsupported roles. Refuse duplicate/zero track numbers, zero TimestampScale, signed-offset underflow and unsigned overflow with existing typed errors.
• Keep VP9 dimensions as catalog-width u32.
• Replace MKV replay offsets with the already-locked published ebml-iterable 0.7.1 incremental decoder. Seven-byte chunks exposed an independent prerequisite: a full master's payload offset retained orphan TrackEntry fields and bypassed Tracks validation. The existing buffered-master set, live unknown-size input and EOF allocation release remain covered; incomplete EOF now fails.
• Refuse unsupported legacy/enhanced FLV codecs before any configured track can accept their packets. Existing Container, RTMP, C and FFI callers propagate these failures.
• Track finish() on the MKV importer itself: a finish() that failed while the decoder still queued tags otherwise let a later decode() reach the decoder's append-after-EOF assertion and panic. Input after finish() fails with mkv::Error::TracksAfterFinish, whose doc now says so.
• Delete the finished quest.

Impact

• Public API shape, exported error variants and MoQ wire are unchanged.
• Input behavior intentionally tightens under the recorded 2026-10-25 codec decision: malformed metadata and unsupported media codecs now fail instead of producing a partial import. Supply valid metadata and supported codecs, or transcode/remove unsupported media tracks before import. Unsupported roles remain skippable. Recorded in rs/moq-mux/MIGRATE.md.
• Any mkv::Import::decode after finish() fails with mkv::Error::TracksAfterFinish, not only one carrying Tracks. Stop decoding once the input ends.
• Direct dependency uses the existing locked registry version; no dependency version resolution changes.

Validation

• All six original refusal reproductions failed before source changes.
• Complete mux nextest suite: 1,797 passed, 3 pre-existing ignored tests skipped.
• Real ffmpeg 9.0.2 live WebM fixture: six Opus packets at 48 kHz, public constructor/streaming path plus emitted frames at chunks 1/7/64/full. Fixture and provenance are retained.
• Existing finished-import allocation test passes unchanged. Tiny live clusters verify exact consumed/buffered byte accounting and release completed prefixes.
• New admitted mkv-chunks Criterion benchmark passes: approximately 95/73/68/68 microseconds for chunks 1/7/64/1813 on a shared 24-CPU host (initial load 6.52/10.63/11.66; 1-second warmup/measurement, 10 samples, compiler 1.98.1). Descriptive observations only, no comparative speedup claim.
• cargo fmt --all --check, diff check and quest check pass.
• Required just fix stops at unchanged rs/moq-mux/benches/catalog.rs:226: missing hang::catalog::MAX_RENDITIONS. Untouched base 83d97bea0a reproduces E0425 with its catalog bench check. Formatting was applied separately; the failure is not hidden.
• Full strict just ci attempt began at source checkpoint 77bb8facbb6b1fbe84cfc17b75a002b618926fd0: Rust lint, feature extremes/boundaries and all 10,551 workspace tests passed (111 existing tests skipped); all other selected lanes passed. Overall exit 1 after 1,218 seconds solely because the selected flake check cannot build github-runner.drv: unshare cannot write /proc/self/uid_map on this local Nix builder. No gate was bypassed.
• Final head 91272b3a20821dadfb6af7588efa427f8c28e8b4 adds only the quest clarification of that observed condition. Production source, manifests, fixture and benchmark are byte-identical to the source checkpoint. Final markdown, quest and diff checks pass; the known-failing expensive matrix was not repeated for prose alone.
• Ready-for-review pass (Claude): merged current main; reproduced the post-failed-finish() panic with input_after_a_failed_finish_is_refused before fixing it. nix develop --command just check origin/main passes: Rust selected the workspace (lint, compile, 10,574 tests passed, 112 skipped), plus markdown, gh, drill, quest, shell, toml, nix and justfile lanes. The flake lane is skipped because this host also cannot create user namespaces; the merge queue runs it on a compatible builder.

Alternatives

Retaining the replay cursor also retained the metadata-boundary bug and repeated parsing. Reusing the maintained incremental decoder removes that machinery without a new parser or buffering policy. No API decisions were made.

Follow-ups

Keep the unrelated catalog benchmark repair separate. Recheck pending moq-dev/moq#6750, moq-dev/moq#6751 and moq-dev/moq#6752 on integration: inspected patches have no competing importer invariant. moq-dev/moq#6864 already landed and merged cleanly. Consider a separate release backport after main for malformed-input timestamp panic/corruption and silently missing tracks, preserving the published API.

Original author: GPT-6 (Codex).

(Written by Claude Opus 5.5)

[coderabbitai]

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 17 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: b828921a-4871-4b95-a533-a1bea9da8aca

Commits

Reviewing files that changed from the base of the PR and between 04eb72b and 80a621c.

Files ignored due to path filters: 2

• Cargo.lock is excluded by !**/*.lock
• rs/moq-mux/src/container/mkv/test_data/opus.webm is excluded by !**/*.webm

Files selected for processing: 15

• Cargo.toml
• quest/m1/cut-audit/README.md
• quest/m1/cut-audit/mkv-import-errors.md
• rs/moq-mux/Cargo.toml
• rs/moq-mux/MIGRATE.md
• rs/moq-mux/benches/containers.rs
• rs/moq-mux/src/codec/vp9/import.rs
• rs/moq-mux/src/codec/vp9/mod.rs
• rs/moq-mux/src/container/flv/import.rs
• rs/moq-mux/src/container/flv/import_test.rs
• rs/moq-mux/src/container/flv/mod.rs
• rs/moq-mux/src/container/mkv/import.rs
• rs/moq-mux/src/container/mkv/import_test.rs
• rs/moq-mux/src/container/mkv/mod.rs
• rs/moq-mux/src/container/mkv/test_data/README.md

• 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[chatgpt-codex-connector]

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-27T05:59:21.643004Z	80a621c	Manual request

ℹ️ 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.

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: 80a621c

No actionable bugs found in the changed areas. Propagating track-validation failures and checking unsigned timestamp arithmetic addresses the silent-loss/wrap paths directly. Reusing the existing locked incremental EBML decoder is simpler than maintaining replay offsets; the tiny-chunk, live-WebM, truncated-EOF and failed-finish regressions justify that change. FLV codec refusal is checked before payload dispatch, and VP9 dimensions retain the full 16-bit-plus-one range.

Overall direction is sound and the stricter input behavior is documented. Inspected the diff, relevant importer/exporter and container-dispatch code, and regression tests. Verification was static; I did not run tests, benchmarks or CI, or independently validate the upstream decoder implementation.

[kixelated]

@codex review

(Written by Claude Opus 5.5)

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Delightful!

Reviewed commit: 80a621c73c

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[kixelated]

Landing summary for head 80a621c73cc1af4615b952522de0f8e23913ebcf:

• Merged current main (no rebase). The MAX_RENDITIONS catalog bench break on main did not affect just check or just ci, so nothing related was folded in.
• Fixed a panic found in adversarial review: a finish() that failed while the decoder still queued tags let a later decode() hit ebml-iterable's append-after-EOF assertion. The importer now tracks finish() itself; input_after_a_failed_finish_is_refused reproduced the panic first.
• Added the rs/moq-mux/MIGRATE.md entry for the stricter MKV/FLV input behavior and deleted the finished quest.
• Validation: just check origin/main passed (workspace Rust lint/compile, 10,574 tests). Strict just ci origin/main passed every lane except check flake, which cannot build github-runner.drv on this host (unshare cannot write /proc/self/uid_map); the merge queue runs that gate on a compatible builder.
• Reviews: Codex reacted with a thumbs up on this head, no findings.

(Written by Claude Opus 5.5)