Conversation
There was a problem hiding this comment.
Pull request overview
Adds end-to-end support for YUVA 4:2:0 planar sources (8-bit and selected high-bit depths) across the frame validators, YUV walkers, row conversion dispatchers, and MixedSinker output wiring, including source-derived alpha in RGBA outputs.
Changes:
- Introduces new YUVA420 frame types (
Yuva420pFrame,Yuva420pFrame16) and per-format YUV walker modules for 8/9/10/16-bit. - Adds scalar row-kernel plumbing and public row dispatchers to convert YUVA420 → RGBA with alpha sourced from the input A plane (u8 and native-depth u16 outputs where applicable).
- Wires
MixedSinkerimplementations for YUVA420 formats, plus adds extensive test coverage for alpha passthrough/masking and buffer sizing errors.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/yuv/yuva420p.rs | New 8-bit YUVA420 walker + row type and sink trait. |
| src/yuv/yuva420p9.rs | New 9-bit YUVA420 walker + row type and sink trait. |
| src/yuv/yuva420p10.rs | New 10-bit YUVA420 walker + row type and sink trait. |
| src/yuv/yuva420p16.rs | New 16-bit YUVA420 walker + row type and sink trait. |
| src/yuv/mod.rs | Registers and re-exports the new YUVA420 modules/types. |
| src/frame.rs | Adds validated YUVA420 frame structs + error types (+ checked constructors for Frame16). |
| src/row/scalar.rs | Extends shared 4:2:0 scalar kernels to optionally source per-pixel alpha from an input plane; adds wrappers. |
| src/row/mod.rs | Adds public YUVA420 → RGBA row dispatchers (scalar-only for now). |
| src/row/arch/* | Updates scalar-tail calls for 8-bit 4:2:0 SIMD implementations to new scalar signature. |
| src/sinker/mixed/mod.rs | Adds RowSlice variants for alpha rows and registers the new YUVA420 sinker module. |
| src/sinker/mixed/yuva_4_2_0.rs | New MixedSinker impls for YUVA420 (8/9/10/16) including RGBA with source alpha and alpha-drop paths. |
| src/sinker/mixed/tests.rs | Adds tests for YUVA420 alpha passthrough/masking + buffer-too-short errors and RGB alpha-drop equivalence. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- yuva_4_2_0.rs: replace `impl SourceFormat` APIT with explicit
`F: SourceFormat` generic on `yuva420p_high_bit_process` for
consistency with the surrounding generic-heavy template (compiles
identically; idiomatic style fix).
- frame.rs: correct `Yuva420pFrameError::{U,V}PlaneTooShort` doc
comments to use `height.div_ceil(2)` so the docs match the
implementation (`chroma_height = height.div_ceil(2)`), matching the
already-correct `Yuva420pFrame16Error` siblings.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
First sub-PR of Ship 8b-2 (4:2:0 source-side YUVA mass-apply). Adds scalar reference +
Frametypes + walker scaffolding + sinker integration + 7 dispatchers for the 4 Yuva420p formats —Yuva420p(8-bit),Yuva420p9,Yuva420p10,Yuva420p16.Mirrors PR #32 (Ship 8b-1a) structurally, but covers 4 formats instead of 1. Two sub-PRs follow:
Changes
Strategy B refactor — extend 5 4:2:0 scalar templates with
ALPHA_SRCEach existing
yuv_420*_to_rgb_or_rgba_*_row<…, ALPHA>scalar template grows a third const-generic,ALPHA_SRC: bool, plus ana_src: Option<&[…]>parameter. The five templates extended:yuv_420_to_rgb_or_rgba_row<ALPHA><ALPHA, ALPHA_SRC>Option<&[u8]>yuv_420p_n_to_rgb_or_rgba_row<BITS, ALPHA><BITS, ALPHA, ALPHA_SRC>Option<&[u16]>yuv_420p_n_to_rgb_or_rgba_u16_row<BITS, ALPHA><BITS, ALPHA, ALPHA_SRC>Option<&[u16]>yuv_420p16_to_rgb_or_rgba_row<ALPHA><ALPHA, ALPHA_SRC>Option<&[u16]>yuv_420p16_to_rgb_or_rgba_u16_row<ALPHA><ALPHA, ALPHA_SRC>Option<&[u16]>Per-pixel store branched on three
(ALPHA, ALPHA_SRC)combinations, identical to Ship 8b-1's pattern. Const-asserted!ALPHA_SRC || ALPHA. Existing_to_rgb_*/_to_rgba_*public wrappers backward-compat (passALPHA_SRC = false, None).5 new public scalar wrappers
yuv_420*_to_rgba*_with_alpha_src_row<…>— Strategy B Yuva path consumed by the new dispatchers.Both PR #32 review fixes pre-applied upfront
The Codex review on Ship 8b-1a (PR #32) surfaced two must-fix issues that are now baked into the canonical Strategy B template:
bits_mask::<BITS>()before depth conversion —try_newadmits unchecked u16 samples; without masking an overrange1024at BITS=10 would shift to256and cast to u8 zero, silently turning over-range alpha into transparent output. Applied to all 5 scalar template extensions in this PR. Pinned by 2yuva420pN_rgba_overrange_alpha_maskedregression tests. (8-bitYuva420pdoesn't need the mask —u8output already fits the sourceu8alpha.)process()must wire alpha-drop paths forwith_rgb/with_rgb_u16/with_luma/with_hsv(declared on the genericMixedSinker<F>impl) — initial implementations only wrote RGBA, leaving the others as silent stale-buffer bugs. Applied to all 4 new sinker impls in this PR. Pinned by 4yuva420pN_with_rgb_alpha_drop_matches_yuv420pNregression tests.New
Frametypes (src/frame.rs+785)Yuva420pFrame<'a>(8-bit) — extraaslice +a_stride. Alpha is full-width × full-height (4:2:0 only subsamples chroma).Yuva420pFrame16<'a, const BITS: u32>— const-assertedBITS == 9 || 10 || 16(FFmpeg has noyuva420p12/yuva420p14).Yuva420p9Frame,Yuva420p10Frame,Yuva420p16Frame.try_newvalidates dimensions + plane lengths;try_new_checked(high-bit) additionally validates every active sample range.Yuva420pFrameError+Yuva420pFrame16ErrorwithYuva420pFrame16Plane::Adiscriminator.4 new walker modules (
src/yuv/yuva420p{,9,10,16}.rs)Each with: marker type,
Rowtype carrying the alpha slice,Sinktrait, walker fn. Wired intosrc/yuv/mod.rs.7 new public dispatchers in
src/row/mod.rsyuva420p_to_rgba_row(u8 only — 8-bit input has no native-u16 RGBA path);yuva420p9/10/16_to_rgba_row+_u16_row(each high-bit variant has both u8 and u16 dispatchers). All stublet _ = use_simd;for SIMD wiring in 8b-2b/2c. Doc strings include# ⚠ Scalar-only as of Ship 8b-2aheadings naming the follow-up sub-PRs.Sinker integration (
src/sinker/mixed/yuva_4_2_0.rs+752)4
MixedSinker<F>impls with full Strategy A combine paths + alpha-drop wiring. The 9/10/16-bitprocessbodies share a genericyuva420p_high_bit_process<BITS, ...>helper to avoid 3× duplication.Tests (
src/sinker/mixed/tests.rs+701)29 new sinker tests — gray-to-gray pass-through, opaque/zero alpha, depth conversion, alpha-drop equivalence vs
Yuv420pN, Strategy A combine, overrange-alpha masking, buffer-too-short errors. 617 tests pass on host (was 588).Test plan
cargo test --lib: 617 pass on aarch64-darwin, 0 failcargo check --tests --libclean across host, x86_64-unknown-freebsd, wasm32-unknown-unknownRUSTFLAGS=\"-Dwarnings\" cargo clippy --lib --testscleandead_codewarnings — every new_with_alpha_src_rowwrapper is consumed by a dispatcherCodex adversarial review
Verdict: approve. No material findings. Quote: "SIMD backend changes are limited to scalar-tail call-site adaptation, while new YUVA420p RGBA paths are explicitly scalar-only."
Out of scope (deferred to follow-up sub-PRs)
🤖 Generated with Claude Code