tx3g: implement ftab box

[bradh]

This comes up in a couple of the MPEG file format conformance files as the tx3g codec under decoding.

See 3GPP TS 26.245 or ETSI TS 126 245 Section 5.1

Its not clear that the box has to be there, so made it an Option.

[coderabbitai]

Walkthrough

Adds support for a font table box (ftab) in TX3G sample descriptions. Introduces a new Any::Ftab variant, a new ftab module with FontEntry and Ftab types and an Atom implementation that decodes/encodes UTF-8 and BOM-marked UTF-16 names, and re-exports the module from stsd. Extends Tx3g with an optional ftab field, updates decode/encode paths to handle the optional ftab, and adds tests and MPEG-09 reference data exercising ftab.

🚥 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 'tx3g: implement ftab box' is clear, concise, and directly describes the main change—implementing support for the ftab box within the tx3g codec module.
Description check	✅ Passed	The description explains the purpose (ftab appears in MPEG conformance files), provides relevant standards references (3GPP TS 26.245 / ETSI TS 126 245), and clarifies the design decision (optional implementation).

_{✏️ 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 bb20267 and 89221d0.

📒 Files selected for processing (1)

• src/moov/trak/mdia/minf/stbl/stsd/ftab.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/ftab.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/ftab.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/ftab.rs

🧠 Learnings (1)

📚 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/ftab.rs

🧬 Code graph analysis (1)

src/moov/trak/mdia/minf/stbl/stsd/ftab.rs (3)

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

• decode_body (54-93)
• encode_body (95-115)

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

• decode (119-150)

src/coding.rs (1)

• decode_exact (11-26)

🔇 Additional comments (7)

src/moov/trak/mdia/minf/stbl/stsd/ftab.rs (7)

8-19: LGTM!

The struct definitions are clean and follow Rust conventions. The derives are appropriate, and the conditional serde support aligns with the codebase pattern.

---

24-26: Approve the allocation safety pattern.

Good use of capacity capping to prevent excessive memory allocation from untrusted input. The min(8, entry_count) pattern prevents a malicious large entry_count from causing memory issues.

---

27-30: Well-implemented bounds checking.

The use of Vec::decode_exact ensures that exactly font_name_length bytes are available, providing the bounds check mentioned in the commit message. This will return Error::OutOfBounds if insufficient data is available.

---

31-44: Consider the minimum length requirement for UTF-16 detection.

The condition font_bytes.len() >= 4 requires at least 4 bytes (BOM + one character) to detect UTF-16. This means:

• A 2-byte BOM-only sequence would fall through to UTF-8 decoding
• A 3-byte sequence with BOM would also fall through

While this approach is safe and likely intentional, please verify whether the 3GPP TS 26.245 specification allows or requires support for empty UTF-16 strings (BOM-only) or other edge cases with lengths < 4.

Additionally, since font_bytes.len() == font_name_length after decode_exact, you could make the bounds check more explicit:

Alternative: More explicit bounds check

         let font_name_length = u8::decode(buf)?;
+        if font_name_length >= 2 && font_name_length < 4 {
+            return Err(Error::InvalidSize); // Or handle as UTF-8
+        }
         let font_bytes = Vec::decode_exact(buf, font_name_length as usize)?;
-        let font = if font_bytes.len() >= 4 && font_bytes[0] == 0xFF && font_bytes[1] == 0xFE {
+        let font = if font_name_length >= 4 && font_bytes[0] == 0xFF && font_bytes[1] == 0xFE {

---

59-62: LGTM!

The UTF-8 fallback path is clean and has appropriate error handling with a descriptive error message.

---

63-67: LGTM!

The completion of the decode logic correctly assembles the FontEntry and returns the Ftab structure.

---

69-89: LGTM!

The encoding logic is well-implemented with proper overflow checks for both the entry count (u16) and font name length (u8). The decision to always encode as UTF-8 is simpler and valid, though it means UTF-16-encoded inputs won't round-trip (appropriately documented on line 78).

Note that font names exceeding 255 bytes in UTF-8 will return Error::TooLarge, which is correct given the format's u8 length field limitation.

---

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: 2

🤖 Fix all issues with AI agents

In @src/moov/trak/mdia/minf/stbl/stsd/ftab.rs:
- Around line 31-62: The code reads font_bytes[0] and font_bytes[1] for BOM
detection without ensuring font_name_length is large enough; add an upfront
bounds check on font_name_length (e.g., ensure it's >= 1 or >= 2 as appropriate)
before decoding/inspecting font_bytes and return Error::InvalidSize for
too-small lengths (handle zero-length and <2 cases before the BOM branches that
reference font_bytes[0] and font_bytes[1]); update the logic around font_bytes,
font_name_length, and the Error::InvalidSize return so BOM checks occur only
after confirming sufficient length.

In @src/moov/trak/mdia/minf/stbl/stsd/tx3g.rs:
- Around line 75-81: The match arm incorrectly uses atom.into() to assign to
ftab; replace that with wrapping the value in Some(...) so ftab = Some(atom)
(i.e., set ftab to Some of the Ftab struct) in the Any::Ftab(branch), leaving
the unknown branch as-is and keeping the surrounding decoding loop intact.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between caa033a and bb20267.

📒 Files selected for processing (4)

• src/any.rs
• src/moov/trak/mdia/minf/stbl/stsd/ftab.rs
• src/moov/trak/mdia/minf/stbl/stsd/mod.rs
• src/moov/trak/mdia/minf/stbl/stsd/tx3g.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/trak/mdia/minf/stbl/stsd/mod.rs
• src/moov/trak/mdia/minf/stbl/stsd/ftab.rs
• src/any.rs
• src/moov/trak/mdia/minf/stbl/stsd/tx3g.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/moov/trak/mdia/minf/stbl/stsd/ftab.rs
• src/any.rs
• src/moov/trak/mdia/minf/stbl/stsd/tx3g.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/ftab.rs
• src/moov/trak/mdia/minf/stbl/stsd/tx3g.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 (2)

📚 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/ftab.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/any.rs : Register new atom types in the Any enum (src/any.rs) to enable flexible decoding

Applied to files:

• src/any.rs
• src/moov/trak/mdia/minf/stbl/stsd/tx3g.rs

🧬 Code graph analysis (2)

src/moov/trak/mdia/minf/stbl/stsd/ftab.rs (3)

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

• decode_body (54-93)
• encode_body (95-115)

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

• decode (119-150)

src/coding.rs (1)

• decode_exact (11-26)

src/moov/trak/mdia/minf/stbl/stsd/tx3g.rs (3)

src/atom.rs (3)

• decode_maybe (46-73)
• decode_unknown (13-15)
• decode (40-42)

src/error.rs (1)

• decode_unknown (73-78)

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

• decode (119-150)

🔇 Additional comments (8)

src/any.rs (1)

285-285: LGTM! Ftab atom properly registered.

The addition follows the established pattern and correctly places Ftab in the stsd hierarchy after Tx3g, enabling flexible decoding through the Any enum.

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

11-11: LGTM! Module properly integrated.

The ftab module declaration and public re-export follow the established pattern and maintain alphabetical ordering.

Also applies to: 34-34

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

3-19: LGTM! Well-documented data structures.

The FontEntry and Ftab structs are appropriately designed with proper serde support and clear documentation referencing the 3GPP TS 26.245 specification.

---

69-89: LGTM! Encoding implementation is clean.

The encode_body implementation correctly handles size conversions with proper error handling. The decision to always encode as UTF-8 (noted in the comment on line 78) simplifies the implementation while maintaining compatibility.

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

3-19: LGTM! Documentation and struct updated appropriately.

The documentation references the correct specification, and the optional ftab field aligns with the PR objective to support font table boxes that may not always be present.

---

31-48: LGTM! Default implementation updated correctly.

The Default trait implementation properly initializes ftab to None, which is appropriate for an optional field.

---

95-115: LGTM! Encoding correctly handles optional ftab.

The encode_body implementation properly encodes the optional ftab field, relying on the Encode trait implementation for Option<T>.

---

147-242: Excellent test coverage with MPEG-09 conformance data.

The addition of MPEG-09 conformance test cases provides valuable real-world validation of the ftab implementation. The round-trip encode/decode tests ensure correctness, and the included JSON documentation aids maintainability.