Skip to content

Conversation

@bradh
Copy link
Collaborator

@bradh bradh commented Jan 15, 2026

No description provided.

@coderabbitai
Copy link

coderabbitai bot commented Jan 15, 2026

Walkthrough

This pull request adds two new atom extension types, Cprt and Kind, with public structs and AtomExt encode/decode implementations, and extends the generated Any enum with Any::Cprt and Any::Kind variants. An iso639 module with language encode/decode helpers is introduced and re-exported crate‑privately. The Udta struct gains optional cprt and kind fields and its atom descriptor is updated. Tests and test fixtures were adjusted to use default/expanded Udta initializations where applicable.

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 61.11% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive No description was provided by the author. Add a pull request description explaining the motivation and technical details of adding cprt and kind child boxes.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adding cprt and kind child boxes to the udta atom.

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

✨ Finishing touches
  • 📝 Generate docstrings


📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 12ed54e and 6598907.

📒 Files selected for processing (13)
  • src/any.rs
  • src/iso639.rs
  • src/lib.rs
  • src/moov/trak/mdia/mdhd.rs
  • src/moov/udta/cprt.rs
  • src/moov/udta/kind.rs
  • src/moov/udta/mod.rs
  • src/test/av1.rs
  • src/test/bbb.rs
  • src/test/esds.rs
  • src/test/h264.rs
  • src/test/hevc.rs
  • src/test/uncompressed.rs
💤 Files with no reviewable changes (1)
  • src/moov/trak/mdia/mdhd.rs
🚧 Files skipped from review as they are similar to previous changes (8)
  • src/test/av1.rs
  • src/test/bbb.rs
  • src/test/uncompressed.rs
  • src/any.rs
  • src/moov/udta/kind.rs
  • src/iso639.rs
  • src/test/hevc.rs
  • src/test/h264.rs
🧰 Additional context used
📓 Path-based instructions (4)
**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.rs: Code must be clippy-clean (cargo clippy --all-targets --all-features -- -D warnings)
Code must be formatted with rustfmt (cargo fmt) and pass formatting checks (cargo fmt -- --check)
Write unit tests inline within modules using #[cfg(test)]

Files:

  • src/moov/udta/mod.rs
  • src/lib.rs
  • src/test/esds.rs
  • src/moov/udta/cprt.rs
src/**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

src/**/*.rs: Each MP4 atom type must implement the Atom trait with KIND, decode_body(), and encode_body()
Use Header::read_from() to handle large atoms without loading entire contents into memory
All operations should return Result using custom error types defined in error.rs
Use the custom Buf/BufMut traits for contiguous byte slices instead of ad-hoc buffer handling

Files:

  • src/moov/udta/mod.rs
  • src/lib.rs
  • src/test/esds.rs
  • src/moov/udta/cprt.rs
src/moov/**

📄 CodeRabbit inference engine (CLAUDE.md)

Place movie metadata atom implementations under src/moov/ (and its hierarchy: trak/mdia/minf/stbl/stsd)

Files:

  • src/moov/udta/mod.rs
  • src/moov/udta/cprt.rs
src/test/**

📄 CodeRabbit inference engine (CLAUDE.md)

Put integration tests under src/test/ with sample MP4 files for different codecs

Files:

  • src/test/esds.rs
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: kixelated/mp4-atom PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-31T04:06:57.364Z
Learning: Applies to src/any.rs : Register new atom types in the Any enum (src/any.rs) to enable flexible decoding
Learnt from: CR
Repo: kixelated/mp4-atom PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-31T04:06:57.364Z
Learning: Applies to src/**/*.rs : Each MP4 atom type must implement the Atom trait with KIND, decode_body(), and encode_body()
📚 Learning: 2025-08-31T04:06:57.364Z
Learnt from: CR
Repo: kixelated/mp4-atom PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-31T04:06:57.364Z
Learning: Applies to src/moov/** : Place movie metadata atom implementations under src/moov/ (and its hierarchy: trak/mdia/minf/stbl/stsd)

Applied to files:

  • src/moov/udta/mod.rs
📚 Learning: 2025-08-31T04:06:57.364Z
Learnt from: CR
Repo: kixelated/mp4-atom PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-31T04:06:57.364Z
Learning: Place top-level atoms (e.g., ftyp, mdat, free) at the top-level of src/

Applied to files:

  • src/lib.rs
📚 Learning: 2025-08-31T04:06:57.364Z
Learnt from: CR
Repo: kixelated/mp4-atom PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-31T04:06:57.364Z
Learning: Applies to src/**/*.rs : Use Header::read_from() to handle large atoms without loading entire contents into memory

Applied to files:

  • src/lib.rs
📚 Learning: 2025-08-31T04:06:57.364Z
Learnt from: CR
Repo: kixelated/mp4-atom PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-31T04:06:57.364Z
Learning: Applies to src/**/*.rs : Each MP4 atom type must implement the Atom trait with KIND, decode_body(), and encode_body()

Applied to files:

  • src/lib.rs
  • src/moov/udta/cprt.rs
📚 Learning: 2025-08-31T04:06:57.364Z
Learnt from: CR
Repo: kixelated/mp4-atom PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-31T04:06:57.364Z
Learning: Applies to src/any.rs : Register new atom types in the Any enum (src/any.rs) to enable flexible decoding

Applied to files:

  • src/moov/udta/cprt.rs
🧬 Code graph analysis (1)
src/moov/udta/cprt.rs (2)
src/moov/trak/mdia/mdhd.rs (2)
  • decode_body_ext (24-52)
  • encode_body_ext (54-65)
src/iso639.rs (2)
  • language_code (12-18)
  • language_string (1-10)
🔇 Additional comments (9)
src/lib.rs (1)

151-151: LGTM!

The iso639 module is appropriately added as an internal module with pub(crate) visibility, keeping language encoding/decoding helpers available within the crate without exposing them in the public API. The placement follows the existing alphabetical module organization.

Also applies to: 173-173

src/test/esds.rs (1)

205-205: LGTM!

Using Udta::default() is cleaner and more maintainable than explicitly listing all None fields. This adapts correctly to the expanded Udta struct with the new cprt and kind fields.

src/moov/udta/mod.rs (5)

1-7: LGTM!

The module structure follows the existing pattern with proper public re-exports. The new cprt and kind modules are correctly integrated alongside the existing skip module.


33-47: LGTM!

The test_udta_empty test correctly verifies round-trip encoding/decoding of an empty Udta with all fields set to None.


49-75: LGTM!

The test_udta test provides good coverage for a fully-populated Udta with all three optional fields (cprt, meta, kind) set, verifying round-trip encoding/decoding.


77-140: LGTM!

Excellent test coverage using real MPEG conformance test data. The test_udta_cprt and test_udta_kind tests verify:

  1. Decoding from known-good byte sequences
  2. Correct field values after decoding
  3. Round-trip re-encoding matches the original bytes

This provides strong confidence in the implementation's conformance to the specification.


11-27: Both Cprt and Kind atoms are properly registered in the Any enum.

The Udta struct and its Atom implementation are correct. The optional fields and the nested! macro configuration properly declare Cprt and Kind as optional child atoms. These atoms are registered in src/any.rs (lines 264-265) nested under Udta, enabling flexible decoding as required.

src/moov/udta/cprt.rs (2)

3-8: LGTM!

The Cprt struct is well-defined with appropriate derives. The Default implementation will initialize both fields to empty strings, which is reasonable for this use case.


10-31: LGTM!

The AtomExt implementation correctly follows the established pattern used in mdhd.rs for language code handling. The decode/encode logic properly uses the crate-internal language_string and language_code helpers from the iso639 module.

One note: the language_code function handles strings shorter than 3 characters gracefully (using unwrap_or(0)), so edge cases with empty or short language strings won't cause panics.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


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.

@bradh bradh force-pushed the udta_children_2026-01-15 branch from 12ed54e to 6598907 Compare January 15, 2026 06:45
@bradh bradh merged commit 994fc21 into kixelated:main Jan 16, 2026
1 check passed
@bradh bradh deleted the udta_children_2026-01-15 branch January 16, 2026 10:50
@github-actions github-actions bot mentioned this pull request Jan 16, 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