implement LHVCDecoderConfigurationBox (lhvC)

[bradh]

See ISO/IEC 14496-15:2024 Section 9

Resolves #126

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

---

Walkthrough

Adds full support for the lhvC atom: a new Lhvc struct and Atom implementation in src/.../hevc/lhvc.rs, a mod lhvc; declaration and pub use lhvc::*; in the HEVC module, and a new Any::Lhvc(Lhvc) variant with generated conversions and AnyAtom implementations. Hev1 and Hvc1 gain pub lhvc: Option<Lhvc> with updated encode/decode paths. Tests include a new LHVC round-trip test and existing HEVC tests were simplified to use ..Default::default() for unspecified optional fields.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 53.85% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'implement LHVCDecoderConfigurationBox (lhvC)' accurately and specifically describes the main change: adding support for the lhvC atom with its abbreviation clarified.
Description check	✅ Passed	The description references ISO/IEC 14496-15:2024 Section 9 and issue #126, which is related to implementing the lhvC box as confirmed in the objectives.
Linked Issues check	✅ Passed	The PR successfully implements the lhvC (LHVCDecoderConfigurationBox) with full decode/encode support per ISO/IEC 14496-15:2024, including the new Lhvc struct, integration into Hev1/Hvc1, and comprehensive tests.
Out of Scope Changes check	✅ Passed	All changes directly support lhvC implementation: new Lhvc module, integration into HEVC structures, Any enum variant, and test updates for consistency. No unrelated changes detected.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

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

---

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

🧹 Nitpick comments (1)

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

32-32: Cap nalus pre-allocation for untrusted input.

num_nalus is read directly from the buffer and used as Vec::with_capacity(num_nalus as usize). While bounded to u16::MAX, consider capping it for consistency with the arrays allocation on line 28 (which uses .min(8)). The same pattern is used in hvcc.rs line 75 (also uncapped there), but the learning recommends capping.

Suggested fix

-            let mut nalus = Vec::with_capacity(num_nalus as usize);
+            let mut nalus = Vec::with_capacity(num_nalus.min(16) as usize);

Based on learnings: "when decoding arrays from untrusted input, use Vec::with_capacity(std::cmp::min(count, N)) to cap initial memory allocation to a reasonable value."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/moov/trak/mdia/minf/stbl/stsd/hevc/lhvc.rs` at line 32, The allocation
for nalus uses Vec::with_capacity(num_nalus as usize) which can grow from
untrusted input; change the allocation to cap the initial capacity (mirror the
.min(8) behavior used above) by using std::cmp::min on num_nalus (e.g.,
min(num_nalus as usize, 8)) so nalus is pre-allocated with a bounded size;
update the allocation site where nalus is created in lhvc.rs accordingly.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/moov/trak/mdia/minf/stbl/stsd/hevc/lhvc.rs`:
- Around line 77-81: The encode path (encode_body) writes temporal_id_nested
into bit 3, colliding with num_temporal_layers; change the shift for
temporal_id_nested from 1 << 3 to 1 << 2 so it matches how decode_body reads
`(temp & 0b0000_0100) != 0` and mirrors hvcc.rs's `<< 2` behavior; update the
bit composition in the encode expression that currently builds ((0b11 << 6) |
(self.num_temporal_layers << 3) | (if self.temporal_id_nested { 1 << 3 } else {
0 }) | self.length_size_minus_one).encode(buf)? to use `1 << 2` for
temporal_id_nested, and add a round-trip unit test exercising encode/decode with
temporal_id_nested = true and a non-zero num_temporal_layers to prevent
regressions.

---

Nitpick comments:
In `@src/moov/trak/mdia/minf/stbl/stsd/hevc/lhvc.rs`:
- Line 32: The allocation for nalus uses Vec::with_capacity(num_nalus as usize)
which can grow from untrusted input; change the allocation to cap the initial
capacity (mirror the .min(8) behavior used above) by using std::cmp::min on
num_nalus (e.g., min(num_nalus as usize, 8)) so nalus is pre-allocated with a
bounded size; update the allocation site where nalus is created in lhvc.rs
accordingly.