Skip to content

ADR-1213: ciede_hip sizes its chroma staging with the picture's ceil dimensions

  • Status: Accepted
  • Date: 2026-09-06
  • Deciders: Lusoris
  • Tags: hip, correctness, feature-extractor, memory-safety

Context

core/src/picture.c allocates subsampled chroma planes as (w + ss_hor) >> ss_hor — a ceil — with a comment naming the exact hazard: floor under-allocates by one row/column on odd dimensions and causes one-past-end reads in ciede's chroma upsampling. CPU, CUDA, SYCL and Metal all consume the picture's real w[1] / h[1].

ciede_hip was the one implementation re-deriving the geometry itself, with a floor:

s->chroma_w = ss_hor ? (w >> 1) : w;
s->chroma_h = ss_ver ? (h >> 1) : h;

For a 577-wide 4:2:0 picture the real chroma plane is 289 wide; HIP staged 288. The last chroma column was never uploaded, and the kernel's cx = x >> 1 for the last luma column read one element past the staged row — the first sample of the next chroma row, and past the end of the allocation on the last row. Every ciede parity fixture was even-sized, which is how this survived.

Decision

We will size the HIP chroma staging buffers with the same ceil formula picture.c uses, so the staged planes match the picture's w[1] / h[1] and the upload copies every column. test_hip_ciede_parity is additionally registered at 577x325.

Alternatives considered

Option Pros Cons Why not chosen
Use picture.c's ceil formula in init (chosen) Two-line change; matches every other implementation; the upload code needs no change Init does not have the picture, so the formula is duplicated rather than read from pic->w[1] —
Read pic->w[1] / pic->h[1] at first extract and allocate lazily No duplicated formula Moves allocation out of init into the frame path and complicates error handling for a geometry the formula already defines Rejected
Clamp cx / cy in the kernel to the staged width Prevents the out-of-bounds read Still drops the real last chroma column, so odd widths would score differently from every other backend Rejected — hides the bug instead of fixing it

Consequences

  • Positive: odd-dimension 4:2:0 / 4:2:2 input no longer reads past the staged chroma row on HIP. test_hip_ciede_parity_oddw (577x325) passes on a gfx1030 and the even-sized test is unchanged.
  • Negative: none measured; the extra column/row of staging is one sample wide.
  • Neutral / follow-ups: none.

References

  • core/src/picture.c (chroma allocation and its hazard comment).
  • ADR-1154 — the HIP backend gap inventory.
  • Source: req — user direction to fix bugs found by the twin-drift sweep.

Status update 2026-10-05: Accepted

Per ADR-0106 the body above is frozen; this note records why the status line changed from Proposed, as found by the 2026-10-05 ADR status sweep. HIP ciede sizes its chroma staging planes with the ceil formula in core/src/feature/hip/ciede_hip.c and the parity test has an odd-width 577x325 variant. Evidence on master: PR #1425, 8276f2b0d, 5b523b24e.

Verification command:

grep -n 'odd luma' core/src/feature/hip/ciede_hip.c