Initial avc3 support
This PR aims to add avc3 support.
It's an initial implementation, reusing all the avc1 codebase. Tested it with a bbc test stream.
Tested with this live stream: https://vs-dash-ww-rd-live.akamaized.net/pl/testcard2020/avc-full.m3u8
Walkthrough
Adds an Avc3 sample-entry variant and wires it throughout the codebase. Introduces Avc3 as a public type alias (AvcSampleEntry<{ AVC3_CODE }>), the private AVC3_CODE, and unit tests exercising base and extended forms. Exposes avc3 in the H.264 module and adds Any::Avc3 handling. Extends the Codec enum with Avc3(Avc3) and updates decode/encode paths to convert between Any::Avc3 and Codec::Avc3. Also generalizes AVC sample entries via AvcSampleEntry<const KIND_CODE: u32> and adds FourCC::from_u32.
Pre-merge checks and finishing touches
❌ 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%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title 'Initial avc3 support' directly summarizes the main change: adding avc3 atom support to the MP4 parser. |
| Description check | ✅ Passed | The description explains the PR's purpose (adding avc3 support), implementation approach (reusing avc1 codebase), and testing validation with live streams. |
✨ Finishing touches
- [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
- [ ] Create PR with unit tests
- [ ] Post copyable unit tests in a comment
[!TIP]
📝 Customizable high-level summaries are now available in beta!
You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
- Provide your own instructions using the
high_level_summary_instructionssetting.- Format the summary however you like (bullet lists, tables, multi-section layouts, contributor stats, etc.).
- Use
high_level_summary_in_walkthroughto move the summary from the description to the walkthrough section.Example instruction:
"Divide the high-level summary into five sections:
- 📝 Description — Summarize the main change in 50–60 words, explaining what was done.
- 📓 References — List relevant issues, discussions, documentation, or related PRs.
- 📦 Dependencies & Requirements — Mention any new/updated dependencies, environment variable changes, or configuration updates.
- 📊 Contributor Summary — Include a Markdown table showing contributions:
| Contributor | Lines Added | Lines Removed | Files Changed |- ✔️ Additional Notes — Add any extra reviewer context. Keep each section concise (under 200 words) and use bullet or numbered lists for clarity."
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later.
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.
Comment @coderabbitai help to get the list of available commands and usage tips.
Looks generally OK. Didn't check the specs yet.
Its a bit annoying that we have to duplicate code like this, but not clear there is a significantly better way.
Can you fix the formatting issue?
I said they should start by copy-pasting AVC1 and sure enough, it decodes properly. But it's definitely not implementing the full spec and yeah the code reuse is frustrating.
Maybe until there's interest in full Avc3 support, a type alias might just work? pub type Avc3 = Avc1?
Maybe until there's interest in full Avc3 support, a type alias might just work?
pub type Avc3 = Avc1?
Not an expert in Rust, so after a quick Cursor investigation, I got the following answer:
- Atom::KIND must be b"avc3" for Avc3 while Avc1 stays b"avc1". With a type alias there is only a single impl block, so we can’t publish two different constants or decode_body registrations. The dispatcher (Any, Codec) would see both boxes as avc1 and treat avc3 bytes as unknown.
- Traits implemented for an alias are the same as for the underlying type, so we couldn’t even add a second impl Atom for Avc3. Rust forbids duplicate impls on aliases.
My latest commit looks to minimize code reuse and plan for future additions to other avc types by:
- Adding one data structure
AvcSampleEntryand one implementation for AVC sample entries, eliminating the previous wrapper struct and duplicate logic. - Adding future FourCCs (e.g., avc2/avc4) is a single-line alias instead of a new module or macro.
- Public API stays the same (Avc1 { … } literals still work), so there’s no user-facing churn.
Happy to defer the decision to you guys! Or if we want to leave this open for now...
Not an expert in Rust, so after a quick Cursor investigation, I got the following answer:
* Atom::KIND must be b"avc3" for Avc3 while Avc1 stays b"avc1". With a type alias there is only a single impl block, so we can’t publish two different constants or decode_body registrations. The dispatcher (Any, Codec) would see both boxes as avc1 and treat avc3 bytes as unknown. * Traits implemented for an alias are the same as for the underlying type, so we couldn’t even add a second impl Atom for Avc3. Rust forbids duplicate impls on aliases.
I'm going to guess you have some background in an object-oriented language. Trying to force everything to be an object is making it hard to see alternatives. (Maybe I'm just projecting here, because I sometimes still have this problem).
I haven't actually tried it, but we can probably just turn the Visual parsing into a fn outside the impl block and call that.
You guessed correctly 😂
I'll take a look at those suggestions during the weekend.
Thanks for the suggestions!
I've fixed the formatting nit. The other parts are messier than I'd hoped, and won't get looked at until the weekend (UTC+11 time).