Skip to content

Conversation

@bradh
Copy link
Collaborator

@bradh bradh commented Jan 15, 2026

This is required in HEIF image sequences (i.e. a track with pict as the handler), per ISO/IEC 23008-12.

@coderabbitai
Copy link

coderabbitai bot commented Jan 15, 2026

Walkthrough

Adds an optional ccst: Option<Ccst> field to the Hvc1 struct in src/moov/trak/mdia/minf/stbl/stsd/hevc/hvc1.rs. Updates decode_body to parse and populate a Ccst atom when present, updates encode_body and the Hvc1 constructor/serialization to include the ccst atom when set, and adds tests decoding an ENCODED_HVC1_HEIF containing a ccst atom to assert correct population.

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: adding ccst child support to hvc1 atom.
Description check ✅ Passed The description is directly related to the changeset, explaining the purpose and standards compliance for the ccst support addition.

✏️ 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 a9d86f5 and b9f2397.

📒 Files selected for processing (1)
  • src/moov/trak/mdia/minf/stbl/stsd/hevc/hvc1.rs
🧰 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/moov/trak/mdia/minf/stbl/stsd/hevc/hvc1.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/trak/mdia/minf/stbl/stsd/hevc/hvc1.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/trak/mdia/minf/stbl/stsd/hevc/hvc1.rs
🧠 Learnings (4)
📓 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/**/*.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/**/*.rs : Each MP4 atom type must implement the Atom trait with KIND, decode_body(), and encode_body()

Applied to files:

  • src/moov/trak/mdia/minf/stbl/stsd/hevc/hvc1.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/trak/mdia/minf/stbl/stsd/hevc/hvc1.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/moov/trak/mdia/minf/stbl/stsd/hevc/hvc1.rs
🧬 Code graph analysis (1)
src/moov/trak/mdia/minf/stbl/stsd/hevc/hvc1.rs (5)
src/moov/trak/mdia/minf/stbl/stsd/fiel.rs (1)
  • new (11-16)
src/moov/trak/mdia/minf/stbl/stsd/hevc/hvcc.rs (1)
  • new (27-32)
src/moov/trak/mdia/minf/stbl/stsd/pasp.rs (1)
  • new (11-16)
src/moov/trak/mdia/minf/stbl/stsd/mod.rs (1)
  • decode (119-150)
src/atom.rs (1)
  • decode (40-42)
🔇 Additional comments (4)
src/moov/trak/mdia/minf/stbl/stsd/hevc/hvc1.rs (4)

14-14: LGTM!

The new ccst field follows the established pattern for optional child atoms in this struct.


63-63: LGTM!

Encoding follows the same pattern as other optional child atoms.


68-167: Excellent test coverage with authoritative conformance data.

The test uses real data from MPEG File Format Conformance (heif/C001.heif), performs round-trip verification (decode then re-encode), and asserts all expected field values including the new ccst atom. This follows the coding guidelines for inline unit tests with #[cfg(test)].


29-51: Decoding logic correctly follows the established pattern.

The implementation properly initializes, matches, and constructs the ccst field using the same pattern as other optional atoms. The Ccst variant is registered in the Any enum, confirming the match arm will work correctly.

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

This is required in HEIF image sequences (i.e. a track with `pict`
as the handler), per ISO/IEC 23008-12.
@bradh bradh force-pushed the hvc1_ccst_2026-01-15 branch from a9d86f5 to b9f2397 Compare January 15, 2026 02:41
@bradh bradh merged commit 998e5a5 into kixelated:main Jan 16, 2026
1 check passed
@bradh bradh deleted the hvc1_ccst_2026-01-15 branch January 16, 2026 10:49
@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