fiel: implement box #85

bradh · 2025-12-30T09:29:59Z

implement field ordering box.

coderabbitai · 2025-12-30T09:33:57Z

Walkthrough

A new Fiel atom type was added with two u8 fields (field_count, field_order) and Atom trait implementation (KIND = "fiel", decode/encode). The Any enum gained an Any::Fiel variant. The stsd module now includes and re-exports fiel. Codec structs Avc1, Hev1, Hvc1 (and related Avcc/AvccExt uses in tests) were extended with fiel: Option<Fiel>, and their decode/encode paths were updated to handle it. Tests were adjusted to initialize or expect the new field.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 55.00% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fiel: implement box' is concise and directly reflects the main change: implementing a new 'fiel' (field ordering) box atom type throughout the codebase.
Description check	✅ Passed	The description 'implement field ordering box' is directly related to the changeset, which adds support for the fiel box type across multiple files and structs.

✨ 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 606e453 and 86ccf1b.

📒 Files selected for processing (12)

src/any.rs
src/moov/mod.rs
src/moov/trak/mdia/minf/stbl/stsd/fiel.rs
src/moov/trak/mdia/minf/stbl/stsd/h264/avc1.rs
src/moov/trak/mdia/minf/stbl/stsd/hevc/hev1.rs
src/moov/trak/mdia/minf/stbl/stsd/hevc/hvc1.rs
src/moov/trak/mdia/minf/stbl/stsd/hevc/hvcc.rs
src/moov/trak/mdia/minf/stbl/stsd/mod.rs
src/test/bbb.rs
src/test/esds.rs
src/test/h264.rs
src/test/hevc.rs

🚧 Files skipped from review as they are similar to previous changes (7)

src/moov/trak/mdia/minf/stbl/stsd/h264/avc1.rs
src/test/h264.rs
src/moov/trak/mdia/minf/stbl/stsd/fiel.rs
src/test/bbb.rs
src/moov/trak/mdia/minf/stbl/stsd/hevc/hvc1.rs
src/moov/trak/mdia/minf/stbl/stsd/hevc/hvcc.rs
src/test/esds.rs

🧰 Additional context used

📓 Path-based instructions (5)

**/*.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/hev1.rs
src/moov/trak/mdia/minf/stbl/stsd/mod.rs
src/any.rs
src/moov/mod.rs
src/test/hevc.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/hev1.rs
src/moov/trak/mdia/minf/stbl/stsd/mod.rs
src/any.rs
src/moov/mod.rs
src/test/hevc.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/hev1.rs
src/moov/trak/mdia/minf/stbl/stsd/mod.rs
src/moov/mod.rs

src/any.rs

📄 CodeRabbit inference engine (CLAUDE.md)

Register new atom types in the Any enum (src/any.rs) to enable flexible decoding

Files:

src/any.rs

src/test/**

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

src/test/hevc.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/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/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/hev1.rs
src/any.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/moov/trak/mdia/minf/stbl/stsd/hevc/hev1.rs
src/moov/trak/mdia/minf/stbl/stsd/mod.rs
src/any.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/hev1.rs

🧬 Code graph analysis (1)

src/moov/trak/mdia/minf/stbl/stsd/hevc/hev1.rs (2)

src/atom.rs (1)

decode_maybe (46-73)

src/coding.rs (1)

decode_maybe (31-31)

🔇 Additional comments (8)

src/test/hevc.rs (1)

569-572: LGTM! Test data correctly includes the fiel field.

The addition of the fiel field to the test data is correct and aligns with the encoded byte sequence in the test.

src/moov/trak/mdia/minf/stbl/stsd/mod.rs (1)

9-9: LGTM! Module registration follows the established pattern.

The fiel module is correctly declared and publicly re-exported, consistent with other atom modules in this file.

Based on learnings, registering new atom types in the module structure is the correct approach.

Also applies to: 31-31

src/any.rs (1)

278-278: LGTM! Fiel atom correctly registered in the Any enum.

The addition follows the established pattern and enables flexible decoding/encoding of the fiel atom across the codebase.

As per coding guidelines, registering new atom types in the Any enum is the correct integration approach.

src/moov/trak/mdia/minf/stbl/stsd/hevc/hev1.rs (4)

12-12: LGTM! Field addition follows the established pattern.

The fiel field is correctly added as an optional field, consistent with other optional atoms in this structure.

26-26: LGTM! Decode logic correctly handles the fiel atom.

The decode implementation follows the same pattern as other optional atoms, properly initializing the field and handling the Any::Fiel variant.

Also applies to: 34-34, 46-46

65-67: LGTM! Encode logic is consistent with existing optional atoms.

The encoding follows the same pattern used for other optional atoms in this file (btrt, colr, pasp, taic).

97-97: LGTM! Test correctly initializes the new field.

The test is properly updated to include the fiel field.

src/moov/mod.rs (1)

222-222: LGTM! Test data correctly updated.

The test is properly updated to initialize the new fiel field, maintaining consistency with the Avc1 struct changes.

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

Actionable comments posted: 0

🧹 Nitpick comments (4)

src/moov/trak/mdia/minf/stbl/stsd/hevc/hev1.rs (1)
65-67: Consider simplifying the encode pattern.

Since Option<T> implements Encode in this codebase, you can simplify this to:
-        if self.fiel.is_some() {
-            self.fiel.encode(buf)?
-        }
+        self.fiel.encode(buf)?;
This pattern is used elsewhere in the codebase and is more concise.
🔎 Proposed simplification
-        if self.fiel.is_some() {
-            self.fiel.encode(buf)?
-        }
+        self.fiel.encode(buf)?;
src/moov/trak/mdia/minf/stbl/stsd/hevc/hvc1.rs (1)
66-68: Consider simplifying the encode pattern.

Similar to the suggestion in hev1.rs, this can be simplified since Option<T> implements Encode:
-        if self.fiel.is_some() {
-            self.fiel.encode(buf)?;
-        }
+        self.fiel.encode(buf)?;
🔎 Proposed simplification
-        if self.fiel.is_some() {
-            self.fiel.encode(buf)?;
-        }
+        self.fiel.encode(buf)?;
src/moov/trak/mdia/minf/stbl/stsd/fiel.rs (2)
11-16: Constructor returns Result but never fails.

The new() method returns Result<Self> but has no error path. Either return Self directly for a simple constructor, or add validation for field_count and field_order if there are invalid values per the MP4/QuickTime specification.
🔎 Proposed fix to return Self directly
-    pub fn new(field_count: u8, field_order: u8) -> Result<Self> {
-        Ok(Fiel {
+    pub fn new(field_count: u8, field_order: u8) -> Self {
+        Fiel {
             field_count,
             field_order,
-        })
+        }
     }
22-30: Consider validation for field_count and field_order values.

The fiel atom is a QuickTime extension (not part of ISO/IEC 14496-12). According to QuickTime/FFmpeg specifications, field_count has valid values 0 (progressive), 1 or 2 (interlaced), and field_order uses specific bit-mapped values (e.g., 0x06 = BB field order). The current decode accepts any u8 without validation. While the codebase pattern (e.g., pasp.rs) doesn't validate in decode functions, adding validation here could improve robustness if invalid values should be rejected.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2f60395 and cc20b86.

📒 Files selected for processing (12)

src/any.rs
src/moov/mod.rs
src/moov/trak/mdia/minf/stbl/stsd/fiel.rs
src/moov/trak/mdia/minf/stbl/stsd/h264/avc1.rs
src/moov/trak/mdia/minf/stbl/stsd/hevc/hev1.rs
src/moov/trak/mdia/minf/stbl/stsd/hevc/hvc1.rs
src/moov/trak/mdia/minf/stbl/stsd/hevc/hvcc.rs
src/moov/trak/mdia/minf/stbl/stsd/mod.rs
src/test/bbb.rs
src/test/esds.rs
src/test/h264.rs
src/test/hevc.rs

🧰 Additional context used

📓 Path-based instructions (5)

**/*.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/mod.rs
src/test/bbb.rs
src/moov/trak/mdia/minf/stbl/stsd/fiel.rs
src/moov/trak/mdia/minf/stbl/stsd/hevc/hvcc.rs
src/moov/trak/mdia/minf/stbl/stsd/hevc/hvc1.rs
src/test/h264.rs
src/any.rs
src/test/esds.rs
src/moov/trak/mdia/minf/stbl/stsd/h264/avc1.rs
src/test/hevc.rs
src/moov/mod.rs
src/moov/trak/mdia/minf/stbl/stsd/hevc/hev1.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/mod.rs
src/test/bbb.rs
src/moov/trak/mdia/minf/stbl/stsd/fiel.rs
src/moov/trak/mdia/minf/stbl/stsd/hevc/hvcc.rs
src/moov/trak/mdia/minf/stbl/stsd/hevc/hvc1.rs
src/test/h264.rs
src/any.rs
src/test/esds.rs
src/moov/trak/mdia/minf/stbl/stsd/h264/avc1.rs
src/test/hevc.rs
src/moov/mod.rs
src/moov/trak/mdia/minf/stbl/stsd/hevc/hev1.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/mod.rs
src/moov/trak/mdia/minf/stbl/stsd/fiel.rs
src/moov/trak/mdia/minf/stbl/stsd/hevc/hvcc.rs
src/moov/trak/mdia/minf/stbl/stsd/hevc/hvc1.rs
src/moov/trak/mdia/minf/stbl/stsd/h264/avc1.rs
src/moov/mod.rs
src/moov/trak/mdia/minf/stbl/stsd/hevc/hev1.rs

src/test/**

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

src/test/bbb.rs
src/test/h264.rs
src/test/esds.rs
src/test/hevc.rs

src/any.rs

📄 CodeRabbit inference engine (CLAUDE.md)

Register new atom types in the Any enum (src/any.rs) to enable flexible decoding

Files:

src/any.rs

🧠 Learnings (5)

📓 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/**/*.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/mod.rs
src/moov/trak/mdia/minf/stbl/stsd/fiel.rs
src/moov/trak/mdia/minf/stbl/stsd/hevc/hvc1.rs
src/any.rs
src/moov/trak/mdia/minf/stbl/stsd/h264/avc1.rs
src/moov/trak/mdia/minf/stbl/stsd/hevc/hev1.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/fiel.rs
src/moov/trak/mdia/minf/stbl/stsd/hevc/hvc1.rs
src/any.rs
src/moov/trak/mdia/minf/stbl/stsd/h264/avc1.rs
src/moov/trak/mdia/minf/stbl/stsd/hevc/hev1.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/moov/** : Place movie metadata atom implementations under src/moov/ (and its hierarchy: trak/mdia/minf/stbl/stsd)

Applied to files:

src/moov/trak/mdia/minf/stbl/stsd/fiel.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
src/moov/trak/mdia/minf/stbl/stsd/h264/avc1.rs
src/moov/trak/mdia/minf/stbl/stsd/hevc/hev1.rs

🧬 Code graph analysis (4)

src/moov/trak/mdia/minf/stbl/stsd/fiel.rs (4)

src/moov/trak/mdia/minf/stbl/stsd/h264/avc1.rs (2)

decode_body (18-48)

encode_body (50-69)

src/moov/trak/mdia/minf/stbl/stsd/hevc/hev1.rs (2)

decode_body (18-48)

encode_body (50-69)

src/moov/trak/mdia/minf/stbl/stsd/hevc/hvc1.rs (2)

decode_body (19-49)

encode_body (51-70)

src/moov/trak/mdia/minf/stbl/stsd/mod.rs (1)

decode (117-146)

src/moov/trak/mdia/minf/stbl/stsd/hevc/hvc1.rs (2)

src/atom.rs (1)

decode_maybe (41-68)

src/coding.rs (1)

decode_maybe (31-31)

src/moov/trak/mdia/minf/stbl/stsd/h264/avc1.rs (2)

src/atom.rs (1)

decode_maybe (41-68)

src/coding.rs (1)

decode_maybe (31-31)

src/moov/trak/mdia/minf/stbl/stsd/hevc/hev1.rs (2)

src/atom.rs (1)

decode_maybe (41-68)

src/coding.rs (1)

decode_maybe (31-31)

🔇 Additional comments (18)

src/test/bbb.rs (1)

273-273: LGTM!

Test data correctly updated to include the new fiel field initialized to None, consistent with other optional fields in the structure.

src/moov/trak/mdia/minf/stbl/stsd/hevc/hvcc.rs (1)

176-176: LGTM!

Test correctly updated to include the new fiel field.

src/moov/trak/mdia/minf/stbl/stsd/mod.rs (2)

9-9: LGTM!

Module declaration follows the alphabetical ordering convention used in this file.

31-31: LGTM!

Public re-export correctly placed in alphabetical order, making the Fiel type available to consumers of this module.

src/moov/trak/mdia/minf/stbl/stsd/hevc/hev1.rs (2)

12-12: LGTM!

New fiel field correctly added as an optional field, consistent with other optional metadata fields in the structure.

26-34: LGTM!

Decode logic correctly handles the new Fiel atom variant, following the established pattern for optional child atoms.

src/test/h264.rs (1)

262-262: LGTM!

Test data correctly updated to include the new fiel field.

src/test/esds.rs (1)

202-202: LGTM!

Test correctly updated with the new fiel field.

src/any.rs (1)

278-278: LGTM - Fiel atom correctly registered.

The Fiel variant is properly added to the Any enum's basic atoms list. This macro invocation will automatically generate all necessary conversion traits (From, TryFrom, AnyAtom) and encode/decode logic for the new atom type, following the established pattern in this codebase.

Based on learnings: Each MP4 atom type must be registered in the Any enum to enable flexible decoding.

src/moov/trak/mdia/minf/stbl/stsd/hevc/hvc1.rs (2)

13-13: LGTM!

The fiel field is correctly added as optional metadata, maintaining consistency with other optional fields in the structure.

27-35: LGTM!

Decode logic properly handles the Fiel atom, following the established pattern for decoding optional child atoms.

src/moov/trak/mdia/minf/stbl/stsd/fiel.rs (1)

39-72: LGTM!

The test coverage is appropriate, with both decode and encode paths tested. The test data is well-structured with a constant for the encoded representation.

src/moov/trak/mdia/minf/stbl/stsd/h264/avc1.rs (4)

12-12: LGTM!

The new fiel field follows the established pattern for optional codec metadata atoms (btrt, colr, pasp, taic).

26-46: LGTM!

The decode logic correctly handles the new Any::Fiel variant and constructs the struct with the optional fiel field. This follows the same pattern as other optional atoms.

65-67: LGTM!

The encode logic conditionally writes the fiel atom when present, consistent with other optional fields.

106-106: LGTM!

Test coverage includes both cases: fiel: None in the basic test and fiel: Some(Fiel { ... }) in the extended test. This validates both encoding/decoding paths.

Also applies to: 158-161

src/moov/mod.rs (1)

222-222: LGTM!

The test expectation correctly includes fiel: None to match the decoded structure, as the encoded test data doesn't contain a fiel atom.

src/test/hevc.rs (1)

569-572: LGTM!

The test expectation correctly includes the fiel atom with field_count: 1 and field_order: 0, which matches the encoded test data in the byte array (lines 175-176 contain the fiel atom: size 10 bytes, type "fiel", followed by 0x01, 0x00).

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (2)

src/moov/trak/mdia/minf/stbl/stsd/fiel.rs (2)
5-8: Consider adding documentation for the Fiel struct and its fields.

The field_count and field_order fields lack documentation explaining their purpose and valid value ranges according to the MP4 specification.

10-17: Simplify the constructor to return Self directly.

The new method returns Result<Self> but never returns an error. Since no validation is performed, the constructor can simply return Self instead of wrapping it in Ok().
🔎 Proposed simplification
 impl Fiel {
-    pub fn new(field_count: u8, field_order: u8) -> Result<Self> {
-        Ok(Fiel {
+    pub fn new(field_count: u8, field_order: u8) -> Self {
+        Fiel {
             field_count,
             field_order,
-        })
+        }
     }
 }

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cc20b86 and 606e453.

📒 Files selected for processing (12)

src/any.rs
src/moov/mod.rs
src/moov/trak/mdia/minf/stbl/stsd/fiel.rs
src/moov/trak/mdia/minf/stbl/stsd/h264/avc1.rs
src/moov/trak/mdia/minf/stbl/stsd/hevc/hev1.rs
src/moov/trak/mdia/minf/stbl/stsd/hevc/hvc1.rs
src/moov/trak/mdia/minf/stbl/stsd/hevc/hvcc.rs
src/moov/trak/mdia/minf/stbl/stsd/mod.rs
src/test/bbb.rs
src/test/esds.rs
src/test/h264.rs
src/test/hevc.rs

🚧 Files skipped from review as they are similar to previous changes (7)

src/moov/trak/mdia/minf/stbl/stsd/mod.rs
src/any.rs
src/test/h264.rs
src/moov/trak/mdia/minf/stbl/stsd/hevc/hev1.rs
src/moov/mod.rs
src/moov/trak/mdia/minf/stbl/stsd/hevc/hvc1.rs
src/moov/trak/mdia/minf/stbl/stsd/hevc/hvcc.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/test/esds.rs
src/test/hevc.rs
src/moov/trak/mdia/minf/stbl/stsd/fiel.rs
src/moov/trak/mdia/minf/stbl/stsd/h264/avc1.rs
src/test/bbb.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/test/esds.rs
src/test/hevc.rs
src/moov/trak/mdia/minf/stbl/stsd/fiel.rs
src/moov/trak/mdia/minf/stbl/stsd/h264/avc1.rs
src/test/bbb.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/hevc.rs
src/test/bbb.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/fiel.rs
src/moov/trak/mdia/minf/stbl/stsd/h264/avc1.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/**/*.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/fiel.rs
src/moov/trak/mdia/minf/stbl/stsd/h264/avc1.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/fiel.rs
src/moov/trak/mdia/minf/stbl/stsd/h264/avc1.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/moov/trak/mdia/minf/stbl/stsd/fiel.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/moov/** : Place movie metadata atom implementations under src/moov/ (and its hierarchy: trak/mdia/minf/stbl/stsd)

Applied to files:

src/moov/trak/mdia/minf/stbl/stsd/fiel.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/h264/avc1.rs

🧬 Code graph analysis (2)

src/moov/trak/mdia/minf/stbl/stsd/fiel.rs (1)

src/moov/trak/mdia/minf/stbl/stsd/h264/avc1.rs (2)

decode_body (18-48)

encode_body (50-69)

src/moov/trak/mdia/minf/stbl/stsd/h264/avc1.rs (2)

src/atom.rs (1)

decode_maybe (41-68)

src/coding.rs (1)

decode_maybe (31-31)

🔇 Additional comments (8)

src/moov/trak/mdia/minf/stbl/stsd/fiel.rs (1)

19-37: LGTM! Atom trait implementation is correct.

The decode and encode logic correctly handles the two u8 fields in sequence.

src/test/esds.rs (1)

202-202: LGTM! Test fixture correctly updated.

The addition of fiel: None aligns with the new fiel field in the Avc1 struct.

src/test/hevc.rs (1)

569-572: LGTM! Test fixture correctly reflects encoded fiel atom.

The test expectation matches the encoded fiel atom in the ENCODED constant at line 175 (bytes: 0x00, 0x00, 0x00, 0x0A, 0x66, 0x69, 0x65, 0x6C, 0x01, 0x00), which decodes to field_count=1 and field_order=0.

src/test/bbb.rs (1)

273-273: LGTM! Test fixture correctly updated.

The addition of fiel: None aligns with the new fiel field in the Avc1 struct.

src/moov/trak/mdia/minf/stbl/stsd/h264/avc1.rs (4)

12-12: LGTM! Fiel field addition follows the pattern of other optional atoms.

The new fiel field is correctly declared as Option<Fiel> consistent with other optional atoms like btrt, colr, pasp, and taic.

26-34: LGTM! Decode logic correctly handles the Fiel atom.

The decode implementation properly initializes fiel to None, matches on Any::Fiel, and assigns the atom when present.

65-67: LGTM! Encode logic correctly handles the optional Fiel atom.

The encode implementation follows the same pattern as other optional atoms, checking for presence before encoding.

106-106: LGTM! Tests properly cover both None and Some cases for fiel.

The basic test (test_avc1) verifies fiel: None, while the extended test (test_avc1_with_extras) verifies fiel: Some(...) with field_count=2 and field_order=0.

Also applies to: 158-161

coderabbitai bot reviewed Dec 30, 2025

View reviewed changes

bradh force-pushed the fiel_box_2025-12-30 branch from cc20b86 to 606e453 Compare December 30, 2025 09:40

coderabbitai bot reviewed Dec 30, 2025

View reviewed changes

bradh mentioned this pull request Dec 31, 2025

Log unknown atoms instead of (sometimes) returning an error. #84

Merged

fiel: implement box

86ccf1b

bradh force-pushed the fiel_box_2025-12-30 branch from 606e453 to 86ccf1b Compare January 3, 2026 05:30

kixelated approved these changes Jan 3, 2026

View reviewed changes

bradh merged commit 6632462 into kixelated:main Jan 3, 2026
1 check passed

github-actions bot mentioned this pull request Jan 3, 2026

chore: release v0.10.0 #79

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fiel: implement box #85

fiel: implement box #85

Uh oh!

bradh commented Dec 30, 2025 •

edited

Loading

Uh oh!

coderabbitai bot commented Dec 30, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fiel: implement box #85

fiel: implement box #85

Uh oh!

Conversation

bradh commented Dec 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot commented Dec 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bradh commented Dec 30, 2025 •

edited

Loading

coderabbitai bot commented Dec 30, 2025 •

edited

Loading