Sub-sampling-aware analytic chroma weighting for the Qfactor path (+ MCT guard fix)#461
Merged
Merged
Conversation
…n either axis
The Cycc guard only disabled the color transform when components differed
in BOTH XRsiz and YRsiz, so 4:2:2-shaped input (horizontal-only mismatch)
kept the ICT enabled across differently-sized component buffers and
corrupted the heap ("double free or corruption" on a plain 3-component
PGX encode with the default Cycc=yes). Any single-axis mismatch already
rules out the built-in MCT, so the guard now ORs the per-axis checks.
Applied to both the buffered and streaming entry points; regression
tests (enc_qf_422_legacy) land with the sub-sampled-weighting commit.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01M9cKpzqghsLxJUu7kkyRAw
…nent-type hints The analytic (Qcsf=mannos/daly) chroma path already derived 4:2:0/4:2:2 weights from one chroma CSF via a per-axis (sx, sy) frequency shift, but it was unreachable for genuinely sub-sampled input: sub-sampling forces the MCT off, and the no-MCT branch routed every component to the luminance CSF because the codestream cannot label channels. This makes the mapping usable end to end: - Qctype=Y,Cb,Cr (encoder) / --ctype (estimate_qfactor): per-component role hints for no-MCT input; hinted Cb/Cr take the chroma CSF with the component's SIZ XRsiz/YRsiz folded into the per-axis frequency mapping. Unhinted components stay generic (luminance CSF), now also with the (sx, sy) shift. New API: set_component_types(). - (sx, sy) now come from the per-component SIZ fields threaded through QCC_marker, with the 444/420/422 chroma_format mapping as fallback; luma_visual_weights() gains the same per-axis form (LH from the vertical axis, HL from the horizontal), bit-identical at (1,1). - Qchromacsf=luma / --chroma-csf luma: A/B switch that keeps the sub-sampling mapping but evaluates the luminance CSF for Cb/Cr, separating the CSF shape from the frequency mapping. New API: set_chroma_csf_reuse_luma(). - visual_weight_check (tests/tools): dumps the derived weight table for any (ctype, sx, sy, ppd, zoom, levels) and asserts the mapping invariants: (1,1) bit-identity with the pre-generalization output, the exact one-level shift at (2,2) (the "chroma one level down" rule falls out of the mapping; no manual offset), 4:2:2 HL > LH anisotropy (impossible for a scalar table), and the generic default role. - Tests: qf_vw_invariants plus 4:2:0 and 4:2:2 encode<->estimate round-trips (residual 0), a negative control proving the ctype hints actually reach the emitted QCC bytes, and a legacy 4:2:2 encode that regression-guards the preceding MCT guard fix. Tiny synthetic 4:2:2 PGX fixtures added (QCD/QCC bytes never depend on sample data). Defaults are unchanged: legacy remains byte-identical (existing golden test), and all analytic 4:4:4 outputs were verified byte-identical against pre-change encodes across Qfactor x model. Encoder-only; no bitstream syntax or decoder change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M9cKpzqghsLxJUu7kkyRAw
1681ad1 to
20047cb
Compare
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
Makes the analytic CSF visual weighting (
Qcsf=mannos|daly) produce correct per-subband weights for chroma components sub-sampled to 4:2:0 / 4:2:2, and fixes a heap-corruptingCyccguard bug found along the way. Encoder-only: no bitstream syntax change, no decoder change, no conformance impact — only the emitted QCD/QCC step sizes differ, and only when the new options are used.Background
The analytic chroma path already derived the 4:2:0/4:2:2 weight tables from a single chroma CSF by shifting each axis's frequency by the sub-sampling factor
(sx, sy). But for genuinely sub-sampled input that code was unreachable: sub-sampled components rule out the built-in MCT, the codestream therefore cannot label channels, and the no-MCT branch routed every component to the luminance CSF. Sub-sampled YCbCr input silently got full-resolution luma weights on Cb/Cr.Changes
Qctype=Y,Cb,Cr(encoder) /--ctype(estimate_qfactor): per-component role hints for no-MCT input, mirroring the convention of other encoders' component-type hints. HintedCb/Crtake the low-pass chroma CSF with the component's SIZXRsiz/YRsizfolded into the per-axis frequency mapping. Unhinted components staygeneric(luminance CSF) — now also with the(sx, sy)shift. New API:set_component_types().(sx, sy)now come from the per-component SIZ fields (threaded throughQCC_marker), with the 4:4:4/4:2:0/4:2:2chroma_formatmapping kept as a fallback.luma_visual_weights()gains the same per-axis form (LH from the vertical axis, HL from the horizontal); at(1, 1)it is bit-identical to the previous single-radial form by construction.Qchromacsf=chroma|luma/--chroma-csf: A/B switch that keeps the sub-sampling frequency mapping but evaluates the luminance CSF forCb/Cr— separates the effect of the CSF shape from the mapping. New API:set_chroma_csf_reuse_luma().visual_weight_check(new test tool): dumps the derived weight table for any(ctype, sx, sy, ppd, zoom, levels)configuration (one command per ITU-T T.802 Annex B comparison) and asserts the mapping invariants incheckmode.docs/qfactor.mdanddocs/cli_encoder.mdcover the new options, why a hint is needed, and the two properties of the mapping (below).Fix:
Cyccguard heap corruption (first commit)The guard that disables the color transform for non-identically-sized components required a mismatch on both axes. 4:2:2-shaped input (horizontal-only mismatch) therefore kept the ICT enabled across differently-sized component buffers, corrupting the heap — reproducible with a plain 3-component PGX encode at default
Cycc=yes, no special options:The guard now ORs the per-axis checks (either-axis mismatch rules out the MCT), at both the buffered and streaming entry points.
enc_qf_422_legacyregression-guards this. Split into its own commit so it can be cherry-picked independently.Correctness properties (asserted by
qf_vw_invariants)(1,1)weight vectors are additionally pinned in-tree to 17 significant digits.(2, 2)sub-sampling equals a one-level shift exactly (bit-for-bit): the classic "weight chroma like luma one level down" rule falls out of the frequency mapping, so no manual level offset exists (applying one would double-count).(2, 1), HL (horizontal detail) strictly outweighs LH at every level — the property a scalar per-subband table cannot represent, and the signature that the per-axis mapping is live.Tests
qf_vw_invariants— the invariant suite above.enc/qfest_qf_420_ctype,enc/qfest_qf_422_ctype— encode ↔estimate_qfactorround-trips at residual 0 through the real SIZ-driven plumbing.qfest_420_ctype_required— negative control: estimating without the hints must fail, proving the chroma CSF actually reached the emitted QCC bytes.enc/qfest_qf_420_reuseluma— the A/B switch round-trip.enc/qfest_qf_422_legacy— legacy 4:2:2 encode (crash regression for the guard fix).Full suite: 473/473 pass.
🤖 Generated with Claude Code
https://claude.ai/code/session_01M9cKpzqghsLxJUu7kkyRAw