add support for btrt box in uncompressed audio

[bradh]

Builds on recent work on uncompressed audio (ISO/IEC 23000-5).

@SanchayanMaity does this work for you?

[coderabbitai]

Walkthrough

An optional btrt: Option<Btrt> field was added to PCM-related types and propagated through decoding and encoding paths. Pcm and macro-generated PCM sample entry structs now include pub btrt: Option<Btrt>. encode_fields and related encode/decode functions were updated to accept and propagate btrt. Tests and test data were added/updated to construct and verify IPCM and other PCM entries with btrt.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% 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 clearly and concisely describes the main change: adding support for the btrt box in uncompressed audio, which matches the changeset's primary focus.
Description check	✅ Passed	The description is related to the changeset, explaining that it builds on recent uncompressed audio work and adds btrt box support, though brief and seeking validation.

✨ 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: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between af64347 and 309af8f.

📒 Files selected for processing (1)

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

🧠 Learnings (3)

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

🧬 Code graph analysis (1)

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

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

• decode (115-144)

🔇 Additional comments (6)

src/moov/trak/mdia/minf/stbl/stsd/pcm.rs (6)

48-54: LGTM!

The btrt field addition follows the established pattern for optional boxes in this struct, consistent with how chnl is handled.

---

57-76: LGTM!

The encode_fields signature extension and encoding logic correctly mirror the existing chnl pattern. The encoding order (audio → pcmc → chnl → btrt) is appropriate.

---

78-124: LGTM!

The decode path correctly handles the optional btrt box, following the same pattern as pcmc. The decoded atom is captured when encountered and included in the returned struct.

---

126-134: LGTM!

The encode_with_fourcc correctly passes self.btrt.as_ref() to encode_fields.

---

137-172: LGTM!

The macro correctly propagates the btrt field to all generated PCM sample entry types, maintaining consistency with the Pcm struct and following the Atom trait pattern.

---

429-482: Well-structured tests with known-good encoded data.

The new ENCODED_IPCM constant and corresponding decode/encode tests provide excellent coverage for the btrt integration. The test validates round-trip correctness against real encoded data, which helps ensure compatibility with other implementations.

---

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 (2)

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

126-139: Consider reusing encode_fields to eliminate duplication.

This method duplicates the encoding logic already present in encode_fields. Both encode the same fields in the same order. Consider delegating to avoid divergence if encoding logic changes.

🔎 Proposed refactor

     pub fn encode_with_fourcc<B: BufMut>(&self, buf: &mut B) -> Result<()> {
-        self.audio.encode(buf)?;
-        self.pcmc.encode(buf)?;
-
-        if let Some(ref chnl) = self.chnl {
-            chnl.encode(buf)?;
-        }
-
-        if let Some(ref btrt) = self.btrt {
-            btrt.encode(buf)?;
-        }
-
-        Ok(())
+        Self::encode_fields(&self.audio, &self.pcmc, self.chnl.as_ref(), self.btrt.as_ref(), buf)
     }

---

474-479: Simplify the type annotation.

The explicit type annotation is verbose. Consider simplifying for clarity.

🔎 Proposed simplification

     #[test]
     fn test_ipcm_decode() {
-        let buf: &mut std::io::Cursor<&&[u8]> = &mut std::io::Cursor::new(&ENCODED_IPCM);
+        let mut buf = std::io::Cursor::new(ENCODED_IPCM);
         let ipcm = Ipcm::decode(buf).expect("failed to decode ipcm");
         assert_eq!(ipcm, decoded_ipcm());
     }

Cursor::new can take &[u8] directly since ENCODED_IPCM is already &'static [u8].

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a912528 and af64347.

📒 Files selected for processing (1)

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

🧠 Learnings (4)

📚 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/pcm.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/pcm.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/pcm.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/buf.rs : Define and maintain contiguous byte-slice buffer behavior via custom Buf and BufMut traits in buf.rs

Applied to files:

• src/moov/trak/mdia/minf/stbl/stsd/pcm.rs

🧬 Code graph analysis (1)

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

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

• decode (115-144)

🔇 Additional comments (5)

src/moov/trak/mdia/minf/stbl/stsd/pcm.rs (5)

46-54: LGTM!

The btrt field addition follows the same pattern as the existing chnl field, appropriately using Option<Btrt> to represent the optional nature of the btrt box.

---

57-76: LGTM!

The encode_fields function correctly handles the optional btrt parameter using the same pattern as chnl, maintaining consistency in the encoding order.

---

78-124: LGTM!

The decoding logic correctly integrates btrt handling via Any::decode_atom, following the same pattern as pcmc. Unlike chnl, which requires channel_count context, btrt can be decoded through the standard path.

---

142-177: LGTM!

The macro correctly propagates the btrt field through both decode and encode paths, ensuring all PCM sample entry types (Sowt, Twos, Lpcm, Ipcm, Fpcm, In24, In32, Fl32, Fl64, S16l) gain btrt support.

---

434-472: Good test coverage for btrt support.

The test data and helper function provide comprehensive verification of IPCM encoding/decoding with the btrt box. The raw bytes in ENCODED_IPCM correctly include the btrt atom (identifiable by the 0x62, 0x74, 0x72, 0x74 / "btrt" fourcc at offset 75-78).

[SanchayanMaity]

Yes, LGTM.