fix(feature-cpu): CIEDE 4:2:2 chroma swap (OOB) + cambi init leak + integer_ssim 16bpc overflow (bug-hunt r1+r2)

changelog.d/fixed/bughunt-feature-cpu-ciede-422-cambi-leak.md

@@ -0,0 +1,40 @@
+### Fixed
+
+- **CIEDE2000 4:2:2 chroma-upsample flag swap (heap OOB read + wrong scores)**
+  (`core/src/feature/ciede.c`): `scale_chroma_planes` and
+  `scale_chroma_planes_hbd` used `ss_ver` for the horizontal sample index and
+  `ss_hor` for the vertical row advance — the two subsampling flags were
+  transposed. On YUV422P (half-width, full-height chroma) the horizontal index
+  read past the half-width input row (heap OOB) and the vertical advance
+  skipped half the input rows, producing wrong `ciede2000` scores. YUV420P
+  (both flags set) and YUV444P (early return) were unaffected, so the Netflix
+  golden CIEDE2000 test (420P content) never caught it. Fix: horizontal index
+  now uses `ss_hor`, vertical advance uses `ss_ver`, matching the CPU
+  subsample semantics. Adds regression tests
+  `test_ciede_scale_chroma_422_8b` / `_16b` to `core/test/test_ciede.c` with
+  non-identical chroma so the upsample pattern is actually validated.
+- **cambi init() partial-failure allocation leak**
+  (`core/src/feature/cambi.c`): every error return in `init()` (dimension
+  validation, picture-pool alloc, contrast/tvi/c-values/histogram/mask buffers,
+  heatmap file opening) returned without freeing the buffers, picture pool, or
+  feature-name dictionary acquired earlier in the same call — the framework
+  never invokes `close()` after a failed `init()`. Fix: route every error path
+  through a `fail:` label that calls the null-tolerant `close_cambi()`
+  (`fex->priv` is zero-initialised, so unallocated pointers are NULL).
+  `close_cambi()`'s heatmap `fclose` loop is now NULL-guarded for the
+  partial-open case. No scoring path changed.
+- **integer SSIM 16-bpc `samplemax²` signed-integer overflow (corrupt c1/c2;
+  CPU↔GPU divergence)** (`core/src/feature/integer_ssim.c`):
+  `ssim_reduce_row_range` computed `c1/c2 = samplemax * samplemax * K * w_d²`
+  with `int samplemax = (1<<depth)-1`. For 16-bpc input, `samplemax = 65535`
+  and `65535 * 65535 = 4 294 836 225 > INT_MAX`, so the multiply overflowed
+  `int` (signed-overflow UB, wrapping to −131071) and the SSIM stability
+  constants went negative — corrupting every 16-bpc SSIM term and diverging
+  from the CUDA / HIP / SYCL twins, which already widen to `int64_t` / `double`.
+  8/10/12-bpc (`samplemax² ≤ 16 769 025`) fit in `int` and are bit-unchanged.
+  Fix: hoist `const double sm = (double)samplemax;` and compute
+  `c1/c2 = sm * sm * K * w_d²`, matching the GPU twins. Adds load-bearing
+  regression test `test_ssim_16bit_distorted_in_range` to
+  `core/test/test_ssim_coverage.c` (anti-correlated 16-bpc content scores
+  0.999992 with the fix vs 1.185 — out of `[0,1]` — with the overflow).
+  (round-2 bug-hunt finding R2-1.)

core/src/feature/AGENTS.md

@@ -87,6 +87,29 @@ feature/
   If libjxl changes any of these upstream, update the scalar extractor
   in the same PR (same for the SIMD follow-ups, which will mirror the
   same coefficient path).
+- **CIEDE chroma-upsample subsample flags (FORK DIVERGES FROM
+  UPSTREAM, 2026-06-27)**: `ciede.c` `scale_chroma_planes` /
+  `scale_chroma_planes_hbd` must key the *horizontal* sample-index
+  divisor off `ss_hor` and the *vertical* row advance off `ss_ver`.
+  **Upstream Netflix carries these two flags transposed** (horizontal
+  off `ss_ver`, vertical off `ss_hor`) — that bug heap-OOB-reads and
+  mis-scores YUV422P (half-width / full-height chroma). YUV420P is a
+  no-op (both flags set) and YUV444P never calls the function, so the
+  Netflix golden CIEDE2000 pair (420P) cannot detect a regression
+  here. On rebase, do NOT let an upstream sync revert the flags back to
+  the transposed form. Guarded by `test_ciede_scale_chroma_422_8b` and
+  `test_ciede_scale_chroma_422_16b` in `core/test/test_ciede.c` (both
+  fail against the upstream form).
+- **integer SSIM `samplemax²` must be widened to `double` (16-bpc
+  parity, 2026-06-27)**: `integer_ssim.c` `ssim_reduce_row_range`
+  computes `c1/c2 = sm * sm * K * w_d²` via a hoisted
+  `const double sm = (double)samplemax;`. It must NOT regress to the
+  `int samplemax * samplemax` form: for 16-bpc `samplemax = 65535` and
+  `65535² > INT_MAX` overflows `int` (UB, wraps negative), corrupting
+  the stability constants and diverging from the CUDA/HIP/SYCL twins
+  (which already use `int64_t`/`double`). 8/10/12-bpc are bit-unchanged
+  by the cast. Guarded by `test_ssim_16bit_distorted_in_range` in
+  `core/test/test_ssim_coverage.c` (fails against the `int` form).
 - **MS-SSIM decimate LPF coefficients**: the 9-tap 9/7 biorthogonal
   filter table (`ms_ssim_lpf_h` / `ms_ssim_lpf_v`) appears verbatim in
   four TUs that must stay byte-identical for the bit-exactness

core/src/feature/cambi.c

@@ -571,9 +571,13 @@ static void get_derivative_data_for_row(const uint16_t *image_data, uint16_t *de
  *
  * Returns `0` on success, `-EINVAL` for invalid encode/source
  * dimensions or heatmap-path failures, `-ENOMEM` on allocation
- * failure. Allocations are NOT freed here on partial failure — the
- * matching `close()` walks the same buffer set and is null-tolerant.
+ * failure. On any partial failure the `fail:` unwind calls
+ * `close_cambi()`, which walks the same buffer set and is
+ * null-tolerant, so no allocation leaks (the framework does not call
+ * `close()` after a failed `init()`).
  */
+static int close_cambi(VmafFeatureExtractor *fex);
+
 static int init(VmafFeatureExtractor *fex, enum VmafPixelFormat pix_fmt, unsigned bpc, unsigned w,
                 unsigned h)
 {
@@ -588,6 +592,8 @@ static int init(VmafFeatureExtractor *fex, enum VmafPixelFormat pix_fmt, unsigne
     s->window_size = (uint16_t)s->window_size_opt;
     s->max_log_contrast = (uint16_t)s->max_log_contrast_opt;
 
+    int err = 0;
+
     s->feature_name_dict =
         vmaf_feature_name_dict_from_provided_features(fex->provided_features, fex->options, s);
     if (!s->feature_name_dict)
@@ -612,16 +618,20 @@ static int init(VmafFeatureExtractor *fex, enum VmafPixelFormat pix_fmt, unsigne
     }
 
     if (s->enc_width < CAMBI_MIN_WIDTH_HEIGHT && s->enc_height < CAMBI_MIN_WIDTH_HEIGHT) {
-        return -EINVAL;
+        err = -EINVAL;
+        goto fail;
     }
     if (s->src_width < CAMBI_MIN_WIDTH_HEIGHT && s->src_height < CAMBI_MIN_WIDTH_HEIGHT) {
-        return -EINVAL;
+        err = -EINVAL;
+        goto fail;
     }
     if (s->src_width > s->enc_width && s->src_height < s->enc_height) {
-        return -EINVAL;
+        err = -EINVAL;
+        goto fail;
     }
     if (s->src_width < s->enc_width && s->src_height > s->enc_height) {
-        return -EINVAL;
+        err = -EINVAL;
+        goto fail;
     }
 
     int enc_pix = s->enc_width * s->enc_height;
@@ -648,24 +658,23 @@ static int init(VmafFeatureExtractor *fex, enum VmafPixelFormat pix_fmt, unsigne
     int alloc_w = s->full_ref ? MAX(s->src_width, s->enc_width) : s->enc_width;
     int alloc_h = s->full_ref ? MAX(s->src_height, s->enc_height) : s->enc_height;
 
-    int err = 0;
     for (unsigned i = 0; i < PICS_BUFFER_SIZE; i++) {
         err |= vmaf_picture_alloc(&s->pics[i], VMAF_PIX_FMT_YUV400P, 10, alloc_w, alloc_h);
     }
     if (err)
-        return err;
+        goto fail;
 
     const int num_diffs = 1 << s->max_log_contrast;
 
     err = set_contrast_arrays(num_diffs, &s->buffers.diffs_to_consider, &s->buffers.diff_weights,
                               &s->buffers.all_diffs);
     if (err)
-        return err;
+        goto fail;
 
     VmafLumaRange luma_range;
     err = vmaf_luminance_init_luma_range(&luma_range, 10, VMAF_PIXEL_RANGE_LIMITED);
     if (err)
-        return err;
+        goto fail;
 
     /* use cambi_eotf if it has a non-default value, else use eotf */
     const char *effective_eotf;
@@ -678,11 +687,13 @@ static int init(VmafFeatureExtractor *fex, enum VmafPixelFormat pix_fmt, unsigne
     VmafEOTF eotf;
     err = vmaf_luminance_init_eotf(&eotf, effective_eotf);
     if (err)
-        return err;
+        goto fail;
 
     s->buffers.tvi_for_diff = aligned_malloc(ALIGN_CEIL(sizeof(uint16_t)) * num_diffs, 16);
-    if (!s->buffers.tvi_for_diff)
-        return -ENOMEM;
+    if (!s->buffers.tvi_for_diff) {
+        err = -ENOMEM;
+        goto fail;
+    }
     for (int d = 0; d < num_diffs; d++) {
         s->buffers.tvi_for_diff[d] = get_tvi_for_diff(s->buffers.diffs_to_consider[d],
                                                       s->tvi_threshold, 10, luma_range, eotf);
@@ -702,12 +713,15 @@ static int init(VmafFeatureExtractor *fex, enum VmafPixelFormat pix_fmt, unsigne
     if (max_window * max_window >= CAMBI_RECIPROCAL_LUT_SIZE) {
         vmaf_log(VMAF_LOG_LEVEL_ERROR, "cambi: window_size %d too large for reciprocal LUT\n",
                  max_window);
-        return -EINVAL;
+        err = -EINVAL;
+        goto fail;
     }
 
     s->buffers.c_values = aligned_malloc(ALIGN_CEIL(alloc_w * sizeof(float)) * alloc_h, 32);
-    if (!s->buffers.c_values)
-        return -ENOMEM;
+    if (!s->buffers.c_values) {
+        err = -ENOMEM;
+        goto fail;
+    }
 
     {
         int v_lo_signed = (int)s->vlt_luma - 3 * (int)num_diffs + 1;
@@ -724,34 +738,45 @@ static int init(VmafFeatureExtractor *fex, enum VmafPixelFormat pix_fmt, unsigne
                      "cambi_vis_lum_threshold may be too low\n",
                      (unsigned)s->buffers.tvi_for_diff[num_diffs - 1],
                      (unsigned)s->buffers.v_band_base);
-            return -EINVAL;
+            err = -EINVAL;
+            goto fail;
         }
         s->buffers.v_band_size = (uint16_t)v_band_size_signed;
     }
     s->buffers.c_values_histograms =
         aligned_malloc(ALIGN_CEIL((size_t)alloc_w * s->buffers.v_band_size * sizeof(uint16_t)), 32);
-    if (!s->buffers.c_values_histograms)
-        return -ENOMEM;
+    if (!s->buffers.c_values_histograms) {
+        err = -ENOMEM;
+        goto fail;
+    }
 
     int pad_size = MASK_FILTER_SIZE >> 1;
     int dp_width = alloc_w + 2 * pad_size + 1;
     int dp_height = 2 * pad_size + 2;
 
     s->buffers.mask_dp =
         aligned_malloc(ALIGN_CEIL((size_t)dp_height * dp_width * sizeof(uint32_t)), 32);
-    if (!s->buffers.mask_dp)
-        return -ENOMEM;
+    if (!s->buffers.mask_dp) {
+        err = -ENOMEM;
+        goto fail;
+    }
     s->buffers.filter_mode_buffer = aligned_malloc(ALIGN_CEIL(3 * alloc_w * sizeof(uint16_t)), 32);
-    if (!s->buffers.filter_mode_buffer)
-        return -ENOMEM;
+    if (!s->buffers.filter_mode_buffer) {
+        err = -ENOMEM;
+        goto fail;
+    }
     s->buffers.derivative_buffer = aligned_malloc(ALIGN_CEIL(alloc_w * sizeof(uint16_t)), 32);
-    if (!s->buffers.derivative_buffer)
-        return -ENOMEM;
+    if (!s->buffers.derivative_buffer) {
+        err = -ENOMEM;
+        goto fail;
+    }
 
     if (s->heatmaps_path) {
         int mkdir_err = mkdirp(s->heatmaps_path, 0770);
-        if (mkdir_err)
-            return -EINVAL;
+        if (mkdir_err) {
+            err = -EINVAL;
+            goto fail;
+        }
         char path[1024] = {0};
         int scaled_w = s->enc_width;
         int scaled_h = s->enc_height;
@@ -760,8 +785,10 @@ static int init(VmafFeatureExtractor *fex, enum VmafPixelFormat pix_fmt, unsigne
                 int snp_ret =
                     snprintf(path, sizeof(path), "%s%ccambi_heatmap_scale_%d_%dx%d_16b.gray",
                              s->heatmaps_path, PATH_SEPARATOR, scale, scaled_w, scaled_h);
-                if (snp_ret < 0 || (size_t)snp_ret >= sizeof(path))
-                    return -ENAMETOOLONG;
+                if (snp_ret < 0 || (size_t)snp_ret >= sizeof(path)) {
+                    err = -ENAMETOOLONG;
+                    goto fail;
+                }
             }
             /* Mode 0644: owner-rw, group-r, other-r. Pinning the mode at
              * open(2) avoids the world-writable surface fopen(3) inherits
@@ -773,7 +800,8 @@ static int init(VmafFeatureExtractor *fex, enum VmafPixelFormat pix_fmt, unsigne
 #endif
             if (hfd < 0) {
                 vmaf_log(VMAF_LOG_LEVEL_ERROR, "cambi: could not open heatmaps_path: %s\n", path);
-                return -EINVAL;
+                err = -EINVAL;
+                goto fail;
             }
 #ifdef _WIN32
             s->heatmaps_files[scale] = _fdopen(hfd, "wb");
@@ -787,7 +815,8 @@ static int init(VmafFeatureExtractor *fex, enum VmafPixelFormat pix_fmt, unsigne
 #else
                 (void)close(hfd);
 #endif
-                return -EINVAL;
+                err = -EINVAL;
+                goto fail;
             }
             scaled_w = (scaled_w + 1) >> 1;
             scaled_h = (scaled_h + 1) >> 1;
@@ -810,6 +839,14 @@ static int init(VmafFeatureExtractor *fex, enum VmafPixelFormat pix_fmt, unsigne
 #endif
 
     return err;
+
+fail:
+    /* Partial init: free every buffer/picture/dict acquired so far.
+     * close_cambi tolerates a partially-populated state because fex->priv
+     * is zero-initialised before init() runs, so unallocated pointers are
+     * NULL (aligned_free(NULL) and unref of a zeroed picture are no-ops). */
+    (void)close_cambi(fex);
+    return err;
 }
 
 /* Preprocessing functions */
@@ -1633,7 +1670,10 @@ static int close_cambi(VmafFeatureExtractor *fex)
 
     if (s->heatmaps_path) {
         for (int scale = 0; scale < NUM_SCALES; scale++) {
-            (void)fclose(s->heatmaps_files[scale]);
+            /* NULL-tolerant: a partial init() failure may close before every
+             * scale's file was opened; fclose(NULL) is undefined. */
+            if (s->heatmaps_files[scale])
+                (void)fclose(s->heatmaps_files[scale]);
         }
     }
 

core/src/feature/ciede.c

@@ -94,9 +94,16 @@ static void scale_chroma_planes_hbd(VmafPicture *in, VmafPicture *out)
         uint16_t *out_buf = out->data[p];
         for (unsigned i = 0; i < out->h[p]; i++) {
             for (unsigned j = 0; j < out->w[p]; j++) {
-                out_buf[j] = in_buf[(j / ((p && ss_ver) ? 2 : 1))];
+                /* Horizontal upsample is governed by ss_hor: chroma is
+                 * half-width whenever the format is not 4:4:4. Using ss_ver
+                 * here read past the half-width input row on 4:2:2 (OOB). */
+                out_buf[j] = in_buf[(j / ((p && ss_hor) ? 2 : 1))];
             }
-            in_buf += (((p && ss_hor) ? i % 2 : 1) * in->stride[p]) / 2;
+            /* Vertical upsample is governed by ss_ver: only 4:2:0 is
+             * half-height, so the input row advances every other output row
+             * for 4:2:0 and every row otherwise. Using ss_hor here skipped
+             * half the input rows on 4:2:2. */
+            in_buf += (((p && ss_ver) ? i % 2 : 1) * in->stride[p]) / 2;
             out_buf += out->stride[p] / 2;
         }
     }
@@ -112,9 +119,16 @@ static void scale_chroma_planes(VmafPicture *in, VmafPicture *out)
         uint8_t *out_buf = out->data[p];
         for (unsigned i = 0; i < out->h[p]; i++) {
             for (unsigned j = 0; j < out->w[p]; j++) {
-                out_buf[j] = in_buf[(j / ((p && ss_ver) ? 2 : 1))];
+                /* Horizontal upsample is governed by ss_hor: chroma is
+                 * half-width whenever the format is not 4:4:4. Using ss_ver
+                 * here read past the half-width input row on 4:2:2 (OOB). */
+                out_buf[j] = in_buf[(j / ((p && ss_hor) ? 2 : 1))];
             }
-            in_buf += ((p && ss_hor) ? i % 2 : 1) * in->stride[p];
+            /* Vertical upsample is governed by ss_ver: only 4:2:0 is
+             * half-height, so the input row advances every other output row
+             * for 4:2:0 and every row otherwise. Using ss_hor here skipped
+             * half the input rows on 4:2:2. */
+            in_buf += ((p && ss_ver) ? i % 2 : 1) * in->stride[p];
             out_buf += out->stride[p];
         }
     }

core/src/feature/integer_ssim.c

@@ -207,6 +207,14 @@ static void ssim_reduce_row_range(ssim_moments *const *lines, int line_mask, int
                                   int vkernel_sz, const unsigned *vkernel, int samplemax, int k_min,
                                   int k_max, double *ssim, double *ssimw)
 {
+    // samplemax² must be computed in double, not int. For 16-bpc input
+    // samplemax = 65535 and 65535*65535 = 4,294,836,225 > INT_MAX, which
+    // overflows a plain int multiply (signed-overflow UB, wraps to −131071)
+    // and corrupts the SSIM C1/C2 stability constants. The CUDA/HIP/SYCL
+    // twins already widen to int64_t/double; this keeps the CPU path in
+    // parity. 8/10/12-bpc (≤ 4095² = 16,769,025) fit in int and are
+    // bit-unchanged by the cast. (round-2 bug-hunt R2-1)
+    const double sm = (double)samplemax;
     for (int x = 0; x < w; x++) {
         ssim_moments m;
         const ssim_moments *buf;
@@ -234,8 +242,8 @@ static void ssim_reduce_row_range(ssim_moments *const *lines, int line_mask, int
         }
         // NOLINTEND(clang-analyzer-security.ArrayBound)
         w_d = m.w;
-        c1 = samplemax * samplemax * SSIM_K1 * w_d * w_d;
-        c2 = samplemax * samplemax * SSIM_K2 * w_d * w_d;
+        c1 = sm * sm * SSIM_K1 * w_d * w_d;
+        c2 = sm * sm * SSIM_K2 * w_d * w_d;
         mx2 = m.mux * (double)m.mux;
         mxy = m.mux * (double)m.muy;
         my2 = m.muy * (double)m.muy;