Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ All notable changes to Pelorus are documented here. The format is
- **`vf_pelorus_grain_estimate_vulkan` SIGSEGV on the first GPU frame**: the inline GLSL `% SLICES` modulo was passed unescaped through an `av_bprintf` format string, so `%S` was interpreted as a wide-string (`%ls`) conversion and dereferenced garbage when the shader was assembled. Escaped to `%%` so the literal `%` reaches the GLSL. ADR-0115.
- **Chroma-passthrough macro across five Vulkan filters (`dehalo`, `aa`, `deblock`, `deband`, `borderfix`)**: the per-plane passthrough branch was emitted as a two-line split `GLSLF(1, imageStore(output_images[%i], pos, ,i);` / `GLSLF(2, imageLoad(input_images[%i], pos)); ,i);` whose `imageStore(` parenthesis never closed inside the macro call. The C preprocessor swallowed the next line and emitted the raw `do { av_bprintf(...); } while (0)` macro body **into the shader source**, so shaderc aborted the SPIR-V compile at first frame with `syntax error, unexpected COMMA` and the filter failed with `-22 (Invalid argument)`. Active on the luma-only filters (`dehalo`, `aa`, `deblock` — the chroma planes always took the broken branch); latent on the all-planes filters (`deband`, `borderfix` — triggered by any non-default `planes` mask). Invisible to every static gate: the `.o` compiles (valid C, garbage only at `av_bprintf` runtime), `glslangValidator` only sees the `.comp` (the chroma loop is C-emitted), and the patch-stack CI never instantiates a Vulkan device. Surfaced by on-device execution on the RTX 4090. Fixed by collapsing to a single balanced `GLSLF(1, imageStore(output_images[%i], pos, imageLoad(input_images[%i], pos)); ,i, i);` — the idiom `denoise` already used; chroma is now passed through bit-identically (infinite PSNR on U/V vs a no-op round-trip). ADR-0129.
- **`vf_pelorus_mc_vulkan` out-of-bounds descriptor write**: the `cur_image` and `ref_image` bindings were declared `.elems = 1` (the shader reads only plane 0), but the host fill `ff_vk_shader_update_img_array()` writes one descriptor per frame plane (`dstArrayElement` 0..nb_planes−1). On any multi-plane input (yuv420p = 3 planes) the plane-1/2 writes landed past the one-element array — an out-of-bounds `VkWriteDescriptorSet` (VUID-…-dstArrayElement-00321) that a strict driver or the validation layers reject, and that only "worked" on the dev box's NVIDIA driver with validation absent. The per-plane image views already exist, so the binding size was the only defect. Fixed by sizing both bindings to `.elems = av_pix_fmt_count_planes(vkctx->input_format)`, matching the `deband`/`denoise` precedent. ADR-0129.
- **Bug-hunt remediation** ([ADR-0138](docs/adr/0138-bug-hunt-remediation.md)) — a verified sweep fixed 9 defects: **(must-fix)** `vf_pelorus_denoise_vulkan` left descriptor binding 5 (`stat_buffer`, statically used by the shader) **unbound on the default `meta=0` path** (invalid Vulkan / device-lost on strict drivers) — now always bound; and an uninitialized `exec` read on an error path. **(should-fix)** `vf_pelorus_mc_vulkan` `s_part[]` OOB on GPUs with `subgroupSize < 8`; `libpelorus` `pel_blob_pack` `uint32` size overflow → heap overflow (now rejected with `PEL_ERR_RANGE`) and `pel_blob_find_section` missing the R5 8-byte-alignment check (now `PEL_ERR_ABI`); `denoise` MC-consumer `grid_cols*grid_rows` signed-int overflow from untrusted side-data; `mc` growing a libc-allocated blob with `av_realloc` (cross-allocator UB); and `analyze` `coalesce_roi` truncating the ROI by raster position instead of by banding strength. Interop conformance fixture extended with malformed-blob rejection tests. No wire-ABI change.

<!-- END UNRELEASED -->

Expand Down
1 change: 1 addition & 0 deletions changelog.d/fixed/0138-bug-hunt.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- **Bug-hunt remediation** ([ADR-0138](docs/adr/0138-bug-hunt-remediation.md)) — a verified sweep fixed 9 defects: **(must-fix)** `vf_pelorus_denoise_vulkan` left descriptor binding 5 (`stat_buffer`, statically used by the shader) **unbound on the default `meta=0` path** (invalid Vulkan / device-lost on strict drivers) — now always bound; and an uninitialized `exec` read on an error path. **(should-fix)** `vf_pelorus_mc_vulkan` `s_part[]` OOB on GPUs with `subgroupSize < 8`; `libpelorus` `pel_blob_pack` `uint32` size overflow → heap overflow (now rejected with `PEL_ERR_RANGE`) and `pel_blob_find_section` missing the R5 8-byte-alignment check (now `PEL_ERR_ABI`); `denoise` MC-consumer `grid_cols*grid_rows` signed-int overflow from untrusted side-data; `mc` growing a libc-allocated blob with `av_realloc` (cross-allocator UB); and `analyze` `coalesce_roi` truncating the ROI by raster position instead of by banding strength. Interop conformance fixture extended with malformed-blob rejection tests. No wire-ABI change.
59 changes: 59 additions & 0 deletions docs/adr/0138-bug-hunt-remediation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
<!-- markdownlint-disable MD013 -->
# ADR-0138: Adversarial bug-hunt remediation — 9 confirmed defects fixed

- **Status**: Accepted
- **Date**: 2026-06-20
- **Deciders**: Lusoris

## Context

A systematic, adversarially-verified bug hunt swept the whole filter family, the
GPU dispatch lifecycle, the libpelorus interop ABI, and the patch stack: five
parallel finders (one per defect class) proposed 15 candidates; each was then
verified against the source by an independent agent that defaulted to
"not a bug". Nine were confirmed (two must-fix, six should-fix, one a duplicate).
Two are correctness defects that fire on **default** configurations.

## Decision

Fix all nine, with a conformance-fixture regression for the two parser hardenings.
Adopt the adversarially-verified hunt as a periodic practice (it complements the
on-device validation release gate of ADR-0129, which catches runtime GPU defects
the C-only patch CI cannot).

### The defects + fixes

| # | Sev | Defect | Fix |
|---|---|---|---|
| 1 | must-fix | `denoise` declares `stat_buffer` at binding 5; the SPIR-V statically references it but it is bound only inside `if(want_meta)` — so on the **default `meta=0`** path a used storage descriptor is left **unbound** (validation error / UB / device-lost on intolerant drivers). No `PARTIALLY_BOUND` escape exists in the FFmpeg helper. | Always allocate the (small) stat buffer and bind binding 5 every dispatch (mirroring the mv/conf 6/7 treatment) + add it as an exec dep on the async path; zero-fill + readback stay gated on `want_meta`. |
| 2 | must-fix | `denoise_dispatch` reads an uninitialized `FFVkExecContext *exec` on an early-`RET` fail path (the `if(exec)` cleanup guard sees garbage). | `FFVkExecContext *exec = NULL;`. |
| 3 | should-fix | `mc` block-SAD reduction OOB-writes `s_part[128]` on GPUs with `subgroupSize < 8` (8×8 subgroups would need up to 1024 slots). | Size `s_part` to the worst case `BLOCK_DIM*BLOCK_DIM` (=1024) in both the inline GLSL and the `.comp` (lockstep). |
| 4 | should-fix | `pel_blob_pack` computes `total_size` (a `uint32` wire field) with 32-bit arithmetic → a crafted oversized section wraps to an undersized allocation → heap overflow. | Accumulate in `uint64_t`, 8-align each section in 64-bit, `return PEL_ERR_RANGE` if `> UINT32_MAX` before allocating. No wire-format change. |
| 5 | should-fix | `denoise` MC consumer computes `cells = grid_cols * grid_rows` (signed `int`) from untrusted side-data → signed overflow → UB / bad size validation. | Compute in `uint64_t`, bound by a new `PEL_DENOISE_MAX_CELLS` cap, validate field sizes in 64-bit before any use. |
| 6 | should-fix | `pel_blob_find_section` returns a castable section pointer without checking the R5 8-byte-alignment invariant on the read side (the packer guarantees it on write). | Reject `(off & 7) != 0` with `PEL_ERR_ABI` before the bounds check. |
| 7 | should-fix | `mc` grows a libc-`calloc`'d Pelorus blob with `av_realloc` → cross-allocator UB (the blob is later `free`'d). | Use libc `realloc`; the error path already preserves the original for `pel_blob_free`. |
| 8 | should-fix | `analyze` `coalesce_roi` documents that it drops the lowest-**score** remainder when the `PEL_MAX_ROI` cap is hit, but actually truncates by **raster position** — so on a busy frame it can drop the strongest banding regions. | Collect all coalesced runs into a scratch buffer, sort by descending banding strength (most-negative qoffset), keep the top `max`. |

## Validation

`meson test --suite=fast` 11/11 (incl. the interop fixture, now with
`test_misaligned_offset` → `PEL_ERR_ABI` and `test_pack_size_overflow` →
`PEL_ERR_RANGE`, and the `pelorus_mc` SPIR-V compile validating the `s_part`
resize). Full n8.1.1 patch-stack apply + `make ffmpeg` clean; GPU smokes on
`vk:0` all pass — notably **denoise at the default `meta=0`** (the must-fix
binding-5 path), plus mc, analyze-roi, denoise meta=1 and denoise mc=1 (the
rewritten 64-bit cells path).

## Consequences

- The interop fixes (#4, #6) are defensive hardenings of the shared, vmafx-vendored
parser — append-only safe (no struct/offset/wire change); vmafx picks them up on
its next interop.c sync.
- No behaviour change on valid input; the must-fix descriptor binding makes the
default denoise path valid Vulkan on strict drivers (it happened to be tolerated
on the dev-box NVIDIA driver, masking the defect).

## References

- ADR-0129 (on-device validation release gate — the complementary runtime-defect
gate). The hunt + verification transcripts are this PR's provenance.
1 change: 1 addition & 0 deletions docs/adr/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,3 +50,4 @@ sibling keep vmafx's number (e.g. 0100, 0108) for an easy cross-walk.
| [0135](0135-analyze-perceptual-aq-map.md) | Source-side perceptual bit-allocation map (`aq` perceptual-AQ on the clean source) for analyze, aiming to beat the encoder's in-loop variance-AQ. Implemented (Chou-Li luma + NAMM masking + edge/banding floors, signed mean-centered qoffsets) + measured: strongly cuts banding (CAMBI −14–25%) but at a net fidelity cost (SSIMULACRA2 BD-rate +13–20%, PSNR +7–14%), and a pure loss on real non-banding content. Fails the kill-criterion; `roi=1` already does banding without the texture-starving harm. Code not merged | Rejected |
| [0136](0136-analyze-frame-metadata.md) | analyze emits its per-frame scalars (complexity/texture/motion/variance/edge/banding/scene_cut) as `lavfi.pelorus.*` frame metadata (the vf_scdet idiom), readable via ffprobe / `metadata=print`. The interop blob isn't muxed to file, so this is the host-side extraction path for per-shot CRF / autotune orchestration + shell debugging. Additive, host-side; no interop-ABI or shader change | Accepted |
| [0137](0137-denoise-forward-lookahead-cadence.md) | Forward-lookahead (bidirectional) temporal denoise — cadence-aware. The causal walk only denoises the trailing frame of each held animation drawing (2s/3s cadence); the leading frame sees a different drawing (tcut-breaks) → under-denoised. Premise validated: causal recovers +1.5 dB on a synthetic 2s-cadence clip vs ~+3 dB potential. Design: a 1-frame-delay buffered model (activate) + a next-frame shader tap, tcut-gated. First positive direction after the encoder-RC negatives. Build pending | Proposed |
| [0138](0138-bug-hunt-remediation.md) | Adversarial bug-hunt remediation: a verified sweep (15 candidates → 9 confirmed) fixed two must-fix denoise defects (the default `meta=0` unbound stat-buffer descriptor + an uninitialized `exec`) and six should-fix hardenings — mc `s_part` OOB on small subgroups, interop `pel_blob_pack` uint32 overflow → heap overflow + `find_section` R5 alignment, denoise `cells` int overflow, mc cross-allocator `av_realloc`, analyze `coalesce_roi` score-truncation. Conformance fixture extended. Complements the ADR-0129 on-device gate | Accepted |
87 changes: 77 additions & 10 deletions ffmpeg-patches/0002-add-vf_pelorus_analyze_vulkan.patch
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Lusoris <lusoris@pm.me>
Date: Sat, 20 Jun 2026 22:05:42 +0200
Date: Sat, 27 Jun 2026 16:51:25 +0200
Subject: [PATCH 02/18] feat(analyze): add vf_pelorus_analyze_vulkan (measured
banding/variance maps)
MIME-Version: 1.0
Expand Down Expand Up @@ -42,8 +42,8 @@ docs/adr/0133-analyze-coarse-banding-cambi.md
configure | 2 +
libavfilter/Makefile | 1 +
libavfilter/allfilters.c | 1 +
libavfilter/vf_pelorus_analyze_vulkan.c | 845 ++++++++++++++++++++++++
4 files changed, 849 insertions(+)
libavfilter/vf_pelorus_analyze_vulkan.c | 912 ++++++++++++++++++++++++
4 files changed, 916 insertions(+)
create mode 100644 libavfilter/vf_pelorus_analyze_vulkan.c

diff --git a/configure b/configure
Expand Down Expand Up @@ -92,10 +92,10 @@ index c02a049dbc..41761c6146 100644
extern const FFFilter ff_vf_perspective;
diff --git a/libavfilter/vf_pelorus_analyze_vulkan.c b/libavfilter/vf_pelorus_analyze_vulkan.c
new file mode 100644
index 0000000000..1d4b3ac145
index 0000000000..1cd20e3b9c
--- /dev/null
+++ b/libavfilter/vf_pelorus_analyze_vulkan.c
@@ -0,0 +1,845 @@
@@ -0,0 +1,912 @@
+/*
+ * Copyright 2026 Lusoris
+ *
Expand Down Expand Up @@ -149,6 +149,7 @@ index 0000000000..1d4b3ac145
+#include "filters.h"
+
+#include <math.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <pelorus/interop.h>
Expand Down Expand Up @@ -437,9 +438,24 @@ index 0000000000..1d4b3ac145
+ return av_make_q(milli, 1000);
+}
+
+/* qsort comparator: order coalesced runs by descending banding strength. A
+ * stronger banding tile gets a more-negative qoffset.num (down to -1000), so the
+ * strongest run is the most negative one — sort ascending by qoffset.num. */
+static int roi_strength_cmp(const void *pa, const void *pb)
+{
+ const AVRegionOfInterest *a = pa;
+ const AVRegionOfInterest *b = pb;
+ if (a->qoffset.num < b->qoffset.num)
+ return -1; /* a is stronger banding -> earlier */
+ if (a->qoffset.num > b->qoffset.num)
+ return 1;
+ return 0;
+}
+
+/* Greedy row-run coalescing: scan tiles row-major, merge horizontally adjacent
+ * banding tiles of the same qoffset bucket into one rectangle, cap at
+ * PEL_MAX_ROI rectangles (drop the lowest-score remainder once full). Per-tile
+ * banding tiles of the same qoffset bucket into one rectangle. ALL runs are
+ * collected first, then ranked by banding strength (|qoffset|) and the top
+ * `max` are kept (drop the lowest-score remainder once full). Per-tile
+ * rectangles fall out naturally when neighbours differ. Writes into out[] and
+ * returns the rectangle count. */
+static int coalesce_roi(const PelorusAnalyzeVulkanContext *s,
Expand All @@ -448,10 +464,53 @@ index 0000000000..1d4b3ac145
+ int cols = s->grid_cols, rows = s->grid_rows;
+ int n = 0;
+ int ty, tx;
+ AVRegionOfInterest *runs;
+ /* Worst case: every tile is a singleton run (no horizontal merge). */
+ size_t cap = (size_t)cols * (size_t)rows;
+
+ if (max <= 0)
+ return 0;
+
+ runs = av_calloc(cap > 0 ? cap : 1, sizeof(*runs));
+ if (!runs) {
+ /* Fallback: positional emission (the prior behaviour) so a transient
+ * alloc failure still yields a usable, bounded ROI set rather than none. */
+ for (ty = 0; ty < rows && n < max; ty++) {
+ tx = 0;
+ while (tx < cols && n < max) {
+ float sc = score[ty * cols + tx];
+ int q0, run, x;
+ if (sc <= 0.0f) {
+ tx++;
+ continue;
+ }
+ q0 = score_to_qoffset(s, sc).num;
+ run = 1;
+ for (x = tx + 1; x < cols; x++) {
+ float scn = score[ty * cols + x];
+ if (scn <= 0.0f || score_to_qoffset(s, scn).num != q0)
+ break;
+ run++;
+ }
+ out[n] = (AVRegionOfInterest) {
+ .self_size = sizeof(AVRegionOfInterest),
+ .top = ty * PEL_TILE,
+ .bottom = FFMIN((ty + 1) * PEL_TILE, s->vkctx.output_height),
+ .left = tx * PEL_TILE,
+ .right = FFMIN((tx + run) * PEL_TILE, s->vkctx.output_width),
+ .qoffset = av_make_q(q0, 1000),
+ };
+ n++;
+ tx += run;
+ }
+ }
+ return n;
+ }
+
+ for (ty = 0; ty < rows && n < max; ty++) {
+ /* Collect every coalesced run (uncapped). */
+ for (ty = 0; ty < rows; ty++) {
+ tx = 0;
+ while (tx < cols && n < max) {
+ while (tx < cols) {
+ float sc = score[ty * cols + tx];
+ int q0, run, x;
+ if (sc <= 0.0f) {
Expand All @@ -467,7 +526,7 @@ index 0000000000..1d4b3ac145
+ break;
+ run++;
+ }
+ out[n] = (AVRegionOfInterest) {
+ runs[n] = (AVRegionOfInterest) {
+ .self_size = sizeof(AVRegionOfInterest),
+ .top = ty * PEL_TILE,
+ .bottom = FFMIN((ty + 1) * PEL_TILE, s->vkctx.output_height),
Expand All @@ -479,6 +538,14 @@ index 0000000000..1d4b3ac145
+ tx += run;
+ }
+ }
+
+ /* Rank by banding strength and keep the strongest `max`. */
+ if (n > 1)
+ qsort(runs, (size_t)n, sizeof(*runs), roi_strength_cmp);
+ if (n > max)
+ n = max;
+ memcpy(out, runs, (size_t)n * sizeof(*out));
+ av_free(runs);
+ return n;
+}
+
Expand Down
Loading