feat(skip): free and skip boxes are now automatically skipped during parsing

[b01o]

closes #66

• skips free space box during parsing
• adds a debug log when skipping
• removes redundant Option<Skip> from Udta and from related tests

Summary by CodeRabbit

• New Features

  • Expanded debug logging during decoding to report sizes for additional box variants.

• Refactor

  • Simplified the user-data container by removing an obsolete optional field; remaining metadata and behavior unchanged.

• Documentation

  • Added docs clarifying free padding/skip-like structures and their treatment during parsing.

• Tests

  • Updated fixtures and expectations to match the simplified user-data structure and removed reliance on default struct expansion.

[coderabbitai]

Walkthrough

Adds debug logging in the atom decoding path for Any::Skip and Any::Free to report box sizes. Removes the public skip: Option<Skip> field from Udta, leaving only meta: Option<Meta>, and updates Udta’s Atom nesting to optional: [ Meta ]. Tests across AV1, BBB, ESDS, H264, HEVC, and uncompressed were updated to remove or omit skip. Adds documentation comments for Free and Skip.

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title clearly and concisely states the primary change by indicating that both free and skip boxes will now be automatically skipped during parsing, directly reflecting the implementation updates and aligning with the PR’s main objective.
Linked Issues Check	✅ Passed	The changes implement the objectives of issue #66 by adding decode branches that skip free and skip boxes with debug logging to avoid UnexpectedBox errors, removing the redundant Option<Skip> from the Udta struct, and updating tests accordingly.
Out of Scope Changes Check	✅ Passed	All modifications directly support skipping free and skip boxes or updating the Udta struct and related tests, and the added documentation and test cleanups are relevant to the scope, so there are no unrelated changes.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e7f7510 and e1fe184.

📒 Files selected for processing (2)

• src/free.rs (1 hunks)
• src/moov/udta/skip.rs (1 hunks)

🧰 Additional context used

📓 Path-based instructions (3)

**/*.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/free.rs
• src/moov/udta/skip.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/free.rs
• src/moov/udta/skip.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/skip.rs

🔇 Additional comments (2)

src/moov/udta/skip.rs (1)

3-6: Documentation looks good and consistent with Free.

The documentation accurately describes the Skip box as free padding space and appropriately references the similar Free atom. The documentation pattern is consistent between both atom types.

src/free.rs (1)

3-6: Skipping behavior implemented
src/atom.rs matches Any::Free and Any::Skip and skips their contents as documented.

---

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

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

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

18-18: Retain Skip export; document box-indexing limitation

The Skip type in src/moov/udta/skip.rs is still referenced by Any::Skip in src/any.rs, so it must remain publicly exported. For issue #66, add a note in the crate-level docs (e.g. README or lib.rs), or open a follow-up issue to track that skipping boxes may affect the ordering when re-encoding.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 13667f0 and e7f7510.

📒 Files selected for processing (8)

• src/atom.rs (1 hunks)
• src/moov/udta/mod.rs (2 hunks)
• src/test/av1.rs (0 hunks)
• src/test/bbb.rs (0 hunks)
• src/test/esds.rs (1 hunks)
• src/test/h264.rs (1 hunks)
• src/test/hevc.rs (0 hunks)
• src/test/uncompressed.rs (0 hunks)

💤 Files with no reviewable changes (4)

• src/test/uncompressed.rs
• src/test/av1.rs
• src/test/hevc.rs
• src/test/bbb.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/atom.rs
• src/moov/udta/mod.rs
• src/test/esds.rs
• src/test/h264.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/atom.rs
• src/moov/udta/mod.rs
• src/test/esds.rs
• src/test/h264.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/test/**

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

• src/test/esds.rs
• src/test/h264.rs

🧠 Learnings (1)

📚 Learning: 2025-08-31T04:06:57.364Z

Learnt from: CR
PR: kixelated/mp4-atom#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/atom.rs

🔇 Additional comments (4)

src/test/h264.rs (1)

383-383: LGTM! Test data updated to reflect Udta structural change.

The Udta initialization correctly reflects the removal of the skip field from the struct. The change is consistent with the broader PR objective of automatically skipping free/skip boxes during parsing.

src/atom.rs (1)

198-199: LGTM! Skip and Free boxes now handled gracefully.

The new match branches correctly implement the automatic skipping behavior requested in issue #66. The debug logging provides visibility into skipped boxes without cluttering normal output.

Minor note: The log messages ("skipping skip box" and "skipping free box") are slightly redundant but remain clear and unambiguous.

src/test/esds.rs (1)

300-300: LGTM! Test data correctly updated.

The Udta initialization aligns with the structural change removing the skip field, consistent with other test updates in this PR.

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

29-29: Test correctly updated to reflect structural change.

The test initialization properly removes the skip field, consistent with the updated Udta structure.

[bradh]

LGTM. Sorry for the lag on getting this done.