Skip to content

feat(core): add percentile pooling methods (median, perc5, perc10, perc20) (ADR-1181) - #1392

Merged
lusoris merged 2 commits into
masterfrom
feat/percentile-pooling-methods
Sep 7, 2026
Merged

lusoris merged 2 commits into
masterfrom
feat/percentile-pooling-methods

Conversation

@lusoris

@lusoris lusoris commented Sep 7, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Implements upstream Netflix/vmaf#818: VMAF_POOL_METHOD_MEDIAN, PERC5, PERC10
and PERC20, evaluated by sort plus linear rank interpolation (matching NumPy's
default percentile).

This is the pooling half of #1311, split out. That PR bundled ADR-1181
(percentile pooling) with ADR-1182 (Windows UTF-8 path contract). The two are
independent, and bundling them dragged model_loader.c, vmaf_roi.c and
vmaf_per_shot.c into the touched-file set purely to route their fopen calls
through the shim — three files carrying ~71 pre-existing clang-tidy findings that
rule 12 then made this PR's problem. The UTF-8 shim follows separately.

⚠️ Breaking: the emitted output schema widens

Both writers loop for (j = 1; j < VMAF_POOL_METHOD_NB; j++), so extending the
enum adds an attribute per method to every <metric> element and every JSON
pool object:

<!-- before -->
<metric name="psnr_y" min="29.640688" max="34.760779" mean="30.755064" harmonic_mean="30.727905" />
<!-- after -->
<metric name="psnr_y" min="29.640688" max="34.760779" mean="30.755064" harmonic_mean="30.727905"
        median="30.526752" perc5="29.843079" perc10="29.903105" perc20="30.025991" />

There is no flag to select a subset. The four pre-existing values do not move —
verified byte-identical on the 576x324 pair with and without frame skipping — so a
reader that looks an attribute up by name is unaffected; one that matches a
<metric> line exactly, or rejects unknown attribute names, must be updated.

python/test/command_line_test.py carried three such exact-match assertions and is
updated here. The scores in them are unchanged — only the four new attributes
are appended. That file is not one of the five Netflix golden-assertion files in
CLAUDE.md §8, and no assertAlmostEqual value is touched anywhere in this PR.

Also in here

  • output.cpp is now clang-tidy clean (29 findings → 0). Most were style, but
    8 were bugprone-suspicious-stringview-data-usage: fmt_or_default() returned a
    std::string_view whose .data() went straight to std::fprintf as a format
    string
    . string_view::data() carries no null-termination guarantee, so that is
    UB the moment the view is built from a non-terminated buffer. It returns
    const char * now — latent rather than live, since today's callers all pass
    terminated data, but worth removing while the file is open.
  • test_pooling_percentile skips instead of failing without its fixtures. The
    Netflix golden YUVs are untracked (scripts/test/fetch-test-yuvs.sh fetches
    them), and only the golden-harness job restores them — so the sanitizer, coverage
    and ARM legs had no file to open. It now probes and sets mu_skipped (exit 77).
    It also no longer leaks its model and context on early returns, which under
    LeakSanitizer had been turning one failed assertion into 96912 bytes of unrelated-
    looking LSan output.

Reproducer / smoke-test command

meson setup core/build core -Denable_cuda=false -Denable_sycl=false -Denable_float=true
ninja -C core/build && meson test -C core/build --suite=fast

# The widened schema, and that the original four values are unmoved:
Y=python/test/resource/yuv
core/build/tools/vmaf --reference $Y/src01_hrc00_576x324.yuv \
  --distorted $Y/src01_hrc01_576x324.yuv --width 576 --height 324 \
  --pixel_format 420 --bitdepth 8 --xml --feature psnr \
  --model path=model/other_models/vmaf_v0.6.0.json --quiet --output /tmp/o.xml
grep -o '<metric name="psnr_y"[^/]*/>' /tmp/o.xml

# output.cpp is clean:
clang-tidy -p core/build --quiet core/src/output.cpp | grep -c warning:   # 0

Measured: meson test --suite=fast 117 Ok / 0 Fail; clang-tidy on output.cpp
29 → 0.

Deep-dive deliverables (ADR-0108)

  • Research digest — no digest needed: the algorithm is specified in ADR-1181 and the upstream issue; this PR implements it.
  • Decision matrix — ADR-1181 ## Alternatives considered.
  • AGENTS.md invariant note — no rebase-sensitive invariants beyond the rebase note below, which carries all four.
  • Reproducer / smoke-test command — above.
  • Changelog fragment — changelog.d/added/pooling-percentile-methods.md, with the breaking schema change called out; CHANGELOG.md regenerated.
  • Rebase note — no rebase impact: already in master via feat(core): add percentile temporal pooling to the public C API #1340.

Docs (rule 10)

docs/usage/cli.md documents the new pooled attributes and
tells readers to tolerate new attribute names;
docs/api/index.md covers the enum additions.

State (rule 13)

docs/state.md: T-UPSTREAM-818-POOLING-ENUM-NO-PERCENTILES-2026-09-03 moves from
Open bugs to Recently closed, citing this PR.

🤖 Generated with Claude Code

@lusoris
lusoris force-pushed the feat/percentile-pooling-methods branch from a3f3b00 to 10d8914 Compare September 7, 2026 13:28
@lusoris
lusoris marked this pull request as ready for review September 7, 2026 14:25
@lusoris
lusoris enabled auto-merge (squash) September 7, 2026 14:25
@lusoris

lusoris commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Blocked on Tidy Ratchet, and the two tidy gates disagree with each other on the same file in the same CI run.

tidy-ratchet[cpu]: 289 TUs, 2922 warnings (baseline 2950), 58 uncited NOLINTs (baseline 58)
::error core/src/output.cpp: warnings 29 -> 0 (-29) — tighten the baseline
::error core/test/test_pooling_percentile.c: warnings 0 -> 1 (+1) — fix the code, never raise the baseline

The output.cpp half is expected and good — this PR took that file to zero. The +1 is the problem, and I cannot see the diagnostic:

Gate Same runner, same commit 036005ca0 Result for test_pooling_percentile.c
Tidy Changed clang-tidy-22 -p build --quiet <changed files>, fails on any warning SUCCESS — 0 warnings
Tidy Ratchet clang-tidy -p build <source>, counts per file 1 warning

The file is not in Tidy Changed's exclusion list, so that pass is not vacuous — it genuinely linted it and found nothing.

What I ruled out locally, replicating the ratchet's exact invocation (cwd = build dir, source path as it appears in compile_commands.json, -Denable_cuda=false -Denable_sycl=false -Db_lto=false, clang-tidy 22.1.8 — the same version the report records):

  • gcc 16.2.1 build → 0 warnings attributed to the file
  • gcc-15 build → 0 warnings attributed to the file
  • both: stdout empty, stderr only 1 warning generated. Suppressed 29 warnings (1 in non-user code, 28 NOLINT). — i.e. the one warning is a system-header diagnostic (glibc stdlib.h, bugprone-casting-through-void), which parse_diagnostics correctly drops via relpath returning None for out-of-repo paths

CI builds the ratchet lane with CC=gcc-14 CXX=g++-14; I have 15 and 16 only. The most likely explanation is a diagnostic that lands inside the test TU under gcc-14's headers and inside a system header under 15/16 — but that is a hypothesis, not something I have evidence for.

Why I am not just baselining it: the ratchet explicitly refuses ("fix the code, never raise the baseline"), and it is right to.

What would settle it: the ratchet's report artifact records per-file counts only, not the diagnostics. Having tidy-ratchet.py emit the matched diagnostic lines into the report — it already parses them in parse_diagnostics — would turn this from an unreproducible disagreement into a one-line answer. That looks worth doing on its own merits, since any future +1 on a machine that cannot reproduce CI's toolchain hits exactly this wall.

Holding in the merge train meanwhile; it has held the window ~25 minutes.

@lusoris
lusoris marked this pull request as draft September 7, 2026 14:56
auto-merge was automatically disabled September 7, 2026 14:56

Pull request was converted to draft

lusoris added a commit that referenced this pull request Sep 7, 2026
The clang-tidy lanes install three toolchain components and treated them
inconsistently. clang comes from apt.llvm.org at 22 because ubuntu-24.04 ships
18, which cannot parse the tree's C++26. meson comes from PyPI because the
distro's 1.3.2 predates `c23` in c_std. gcc was left at the image's gcc-14,
with no rationale in the workflow and no ADR pinning it -- the only mention of
gcc-14 in docs/adr/ is an incidental line in a VVenC changelog summary.

That stopped being cosmetic on PR #1392. The ratchet reported
test_pooling_percentile.c going 0 -> 1 while Tidy Changed -- which fails on ANY
warning and does not exclude that file -- called the same file clean, on the
same runner and commit. It could not be reproduced on gcc-15 or gcc-16.

The cause is structural: clang-tidy parses each TU against the system headers
the C compiler provides, so a ratchet count depends on gcc's version as much as
on clang-tidy's, and the baseline recorded only clang_tidy_version. Pinning gcc
a major behind every developer's box while recording nothing about it makes a
disagreement unexplainable.

Three changes:

  - the lanes install gcc-15/g++-15 from ppa:ubuntu-toolchain-r/test;
  - the ratchet records cc_version, read from the build directory's
    meson-info/intro-compilers.json so it is the compiler that actually
    produced compile_commands.json, and annotates a mismatch like it already
    does for clang-tidy;
  - the --report artifact keeps every diagnostic as `path:line:col: [check]`.
    parse_diagnostics() already produced them and the script discarded all but
    the count. The baseline stays counts-only so it remains reviewable and does
    not churn on line-number shifts.

Verified locally: the report now carries `cc (GCC) 16.2.1 20260810` and 29
diagnostics for output.cpp, naming check and location.

This requires a CI-side regeneration of tidy-baseline-cpu.json -- counts under
gcc-15's headers will not equal gcc-14's, and a local --write would resolve
enable_dnn=auto differently and rebaseline unrelated files against the wrong
build.

libvmaf-build-matrix.yml still pins gcc-14; moving it surfaces its own crop of
warnings across ten-plus legs and belongs in its own PR.

Refs: ADR-1230

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@lusoris
lusoris force-pushed the feat/percentile-pooling-methods branch 4 times, most recently from fa92170 to 46f4c84 Compare September 7, 2026 20:40
@lusoris
lusoris marked this pull request as ready for review September 7, 2026 21:25
@lusoris
lusoris enabled auto-merge (squash) September 7, 2026 21:25
…GPU filters

#1340 landed the percentile pooling C API (ADR-1188). This branch had
implemented the same feature independently under ADR-1181 -- byte-identical
enum names, order and semantics -- so its core half is now dead code that can
never merge: it collides with master on libvmaf.h, libvmaf.c, output.cpp,
predict.c and core/test/meson.build.

Reduced to the consumer surfaces #1340 did not ship, which are genuinely
additive:

  - pkg/libvmaf: a typed `PoolMethod` with a String() mapping to the
    on-the-wire names, so Go callers pick a pooling method without touching
    cgo enums.
  - ffmpeg-patches 0005 / 0006 / 0013: the libvmaf_sycl, libvmaf_vulkan and
    libvmaf_metal filters accept the four new `pool` values. The CPU
    libvmaf filter already did.
  - docs/usage/cli.md: documents them on the CLI.
  - python/test/command_line_test.py: end-to-end coverage through the CLI.

Dropped: the duplicate core implementation, core/src/pooling_percentile.h,
core/test/test_pooling_percentile.c, ADR-1181 and its index rows, and the
docs/api, docs/state and rebase-notes edits master already carries from #1340.
ADR-1181 is not superseded because it never landed -- ADR-1188 is the record.

The changelog fragment now describes only the consumer wiring; master already
carries `percentile-pooling-methods.md` for the API itself.

Verified against master's implementation rather than its own: `go build` and
`go vet ./pkg/libvmaf/` are clean and `gofmt -l` is empty, which also confirms
the enum values agree. `go test ./pkg/libvmaf/` cannot link here for an
unrelated reason -- the workstation has no DNN-enabled libvmaf installed, so
cgo fails on vmaf_dnn_session_run; plain master fails identically, and CI's Go
lane builds the library first.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@lusoris
lusoris force-pushed the feat/percentile-pooling-methods branch from 46f4c84 to dac17be Compare September 7, 2026 21:51
…DR-1188 output

ADR-1188 intentionally keeps pool_report_order limited to the historical four
methods (min, max, mean, harmonic_mean) so the default XML output does not
include median/perc5/perc10/perc20. Revert the speculative additions in
command_line_test.py so vmafexec tests pass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@lusoris
lusoris merged commit 102b825 into master Sep 7, 2026
74 of 76 checks passed
@lusoris
lusoris deleted the feat/percentile-pooling-methods branch September 7, 2026 22:45
lusoris added a commit that referenced this pull request Sep 8, 2026
The clang-tidy lanes install three toolchain components and treated them
inconsistently. clang comes from apt.llvm.org at 22 because ubuntu-24.04 ships
18, which cannot parse the tree's C++26. meson comes from PyPI because the
distro's 1.3.2 predates `c23` in c_std. gcc was left at the image's gcc-14,
with no rationale in the workflow and no ADR pinning it -- the only mention of
gcc-14 in docs/adr/ is an incidental line in a VVenC changelog summary.

That stopped being cosmetic on PR #1392. The ratchet reported
test_pooling_percentile.c going 0 -> 1 while Tidy Changed -- which fails on ANY
warning and does not exclude that file -- called the same file clean, on the
same runner and commit. It could not be reproduced on gcc-15 or gcc-16.

The cause is structural: clang-tidy parses each TU against the system headers
the C compiler provides, so a ratchet count depends on gcc's version as much as
on clang-tidy's, and the baseline recorded only clang_tidy_version. Pinning gcc
a major behind every developer's box while recording nothing about it makes a
disagreement unexplainable.

Three changes:

  - the lanes install gcc-15/g++-15 from ppa:ubuntu-toolchain-r/test;
  - the ratchet records cc_version, read from the build directory's
    meson-info/intro-compilers.json so it is the compiler that actually
    produced compile_commands.json, and annotates a mismatch like it already
    does for clang-tidy;
  - the --report artifact keeps every diagnostic as `path:line:col: [check]`.
    parse_diagnostics() already produced them and the script discarded all but
    the count. The baseline stays counts-only so it remains reviewable and does
    not churn on line-number shifts.

Verified locally: the report now carries `cc (GCC) 16.2.1 20260810` and 29
diagnostics for output.cpp, naming check and location.

This requires a CI-side regeneration of tidy-baseline-cpu.json -- counts under
gcc-15's headers will not equal gcc-14's, and a local --write would resolve
enable_dnn=auto differently and rebaseline unrelated files against the wrong
build.

libvmaf-build-matrix.yml still pins gcc-14; moving it surfaces its own crop of
warnings across ten-plus legs and belongs in its own PR.

Refs: ADR-1230

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lusoris added a commit that referenced this pull request Sep 8, 2026
The clang-tidy lanes install three toolchain components and treated them
inconsistently. clang comes from apt.llvm.org at 22 because ubuntu-24.04 ships
18, which cannot parse the tree's C++26. meson comes from PyPI because the
distro's 1.3.2 predates `c23` in c_std. gcc was left at the image's gcc-14,
with no rationale in the workflow and no ADR pinning it -- the only mention of
gcc-14 in docs/adr/ is an incidental line in a VVenC changelog summary.

That stopped being cosmetic on PR #1392. The ratchet reported
test_pooling_percentile.c going 0 -> 1 while Tidy Changed -- which fails on ANY
warning and does not exclude that file -- called the same file clean, on the
same runner and commit. It could not be reproduced on gcc-15 or gcc-16.

The cause is structural: clang-tidy parses each TU against the system headers
the C compiler provides, so a ratchet count depends on gcc's version as much as
on clang-tidy's, and the baseline recorded only clang_tidy_version. Pinning gcc
a major behind every developer's box while recording nothing about it makes a
disagreement unexplainable.

Three changes:

  - the lanes install gcc-15/g++-15 from ppa:ubuntu-toolchain-r/test;
  - the ratchet records cc_version, read from the build directory's
    meson-info/intro-compilers.json so it is the compiler that actually
    produced compile_commands.json, and annotates a mismatch like it already
    does for clang-tidy;
  - the --report artifact keeps every diagnostic as `path:line:col: [check]`.
    parse_diagnostics() already produced them and the script discarded all but
    the count. The baseline stays counts-only so it remains reviewable and does
    not churn on line-number shifts.

Verified locally: the report now carries `cc (GCC) 16.2.1 20260810` and 29
diagnostics for output.cpp, naming check and location.

This requires a CI-side regeneration of tidy-baseline-cpu.json -- counts under
gcc-15's headers will not equal gcc-14's, and a local --write would resolve
enable_dnn=auto differently and rebaseline unrelated files against the wrong
build.

libvmaf-build-matrix.yml still pins gcc-14; moving it surfaces its own crop of
warnings across ten-plus legs and belongs in its own PR.

Refs: ADR-1230

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lusoris added a commit that referenced this pull request Sep 8, 2026
The clang-tidy lanes install three toolchain components and treated them
inconsistently. clang comes from apt.llvm.org at 22 because ubuntu-24.04 ships
18, which cannot parse the tree's C++26. meson comes from PyPI because the
distro's 1.3.2 predates `c23` in c_std. gcc was left at the image's gcc-14,
with no rationale in the workflow and no ADR pinning it -- the only mention of
gcc-14 in docs/adr/ is an incidental line in a VVenC changelog summary.

That stopped being cosmetic on PR #1392. The ratchet reported
test_pooling_percentile.c going 0 -> 1 while Tidy Changed -- which fails on ANY
warning and does not exclude that file -- called the same file clean, on the
same runner and commit. It could not be reproduced on gcc-15 or gcc-16.

The cause is structural: clang-tidy parses each TU against the system headers
the C compiler provides, so a ratchet count depends on gcc's version as much as
on clang-tidy's, and the baseline recorded only clang_tidy_version. Pinning gcc
a major behind every developer's box while recording nothing about it makes a
disagreement unexplainable.

Three changes:

  - the lanes install gcc-15/g++-15 from ppa:ubuntu-toolchain-r/test;
  - the ratchet records cc_version, read from the build directory's
    meson-info/intro-compilers.json so it is the compiler that actually
    produced compile_commands.json, and annotates a mismatch like it already
    does for clang-tidy;
  - the --report artifact keeps every diagnostic as `path:line:col: [check]`.
    parse_diagnostics() already produced them and the script discarded all but
    the count. The baseline stays counts-only so it remains reviewable and does
    not churn on line-number shifts.

Verified locally: the report now carries `cc (GCC) 16.2.1 20260810` and 29
diagnostics for output.cpp, naming check and location.

This requires a CI-side regeneration of tidy-baseline-cpu.json -- counts under
gcc-15's headers will not equal gcc-14's, and a local --write would resolve
enable_dnn=auto differently and rebaseline unrelated files against the wrong
build.

libvmaf-build-matrix.yml still pins gcc-14; moving it surfaces its own crop of
warnings across ten-plus legs and belongs in its own PR.

Refs: ADR-1230

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lusoris added a commit that referenced this pull request Sep 15, 2026
The clang-tidy lanes install three toolchain components and treated them
inconsistently. clang comes from apt.llvm.org at 22 because ubuntu-24.04 ships
18, which cannot parse the tree's C++26. meson comes from PyPI because the
distro's 1.3.2 predates `c23` in c_std. gcc was left at the image's gcc-14,
with no rationale in the workflow and no ADR pinning it -- the only mention of
gcc-14 in docs/adr/ is an incidental line in a VVenC changelog summary.

That stopped being cosmetic on PR #1392. The ratchet reported
test_pooling_percentile.c going 0 -> 1 while Tidy Changed -- which fails on ANY
warning and does not exclude that file -- called the same file clean, on the
same runner and commit. It could not be reproduced on gcc-15 or gcc-16.

The cause is structural: clang-tidy parses each TU against the system headers
the C compiler provides, so a ratchet count depends on gcc's version as much as
on clang-tidy's, and the baseline recorded only clang_tidy_version. Pinning gcc
a major behind every developer's box while recording nothing about it makes a
disagreement unexplainable.

Three changes:

  - the lanes install gcc-15/g++-15 from ppa:ubuntu-toolchain-r/test;
  - the ratchet records cc_version, read from the build directory's
    meson-info/intro-compilers.json so it is the compiler that actually
    produced compile_commands.json, and annotates a mismatch like it already
    does for clang-tidy;
  - the --report artifact keeps every diagnostic as `path:line:col: [check]`.
    parse_diagnostics() already produced them and the script discarded all but
    the count. The baseline stays counts-only so it remains reviewable and does
    not churn on line-number shifts.

Verified locally: the report now carries `cc (GCC) 16.2.1 20260810` and 29
diagnostics for output.cpp, naming check and location.

This requires a CI-side regeneration of tidy-baseline-cpu.json -- counts under
gcc-15's headers will not equal gcc-14's, and a local --write would resolve
enable_dnn=auto differently and rebaseline unrelated files against the wrong
build.

libvmaf-build-matrix.yml still pins gcc-14; moving it surfaces its own crop of
warnings across ten-plus legs and belongs in its own PR.

Refs: ADR-1230

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lusoris added a commit that referenced this pull request Sep 15, 2026
The clang-tidy lanes install three toolchain components and treated them
inconsistently. clang comes from apt.llvm.org at 22 because ubuntu-24.04 ships
18, which cannot parse the tree's C++26. meson comes from PyPI because the
distro's 1.3.2 predates `c23` in c_std. gcc was left at the image's gcc-14,
with no rationale in the workflow and no ADR pinning it -- the only mention of
gcc-14 in docs/adr/ is an incidental line in a VVenC changelog summary.

That stopped being cosmetic on PR #1392. The ratchet reported
test_pooling_percentile.c going 0 -> 1 while Tidy Changed -- which fails on ANY
warning and does not exclude that file -- called the same file clean, on the
same runner and commit. It could not be reproduced on gcc-15 or gcc-16.

The cause is structural: clang-tidy parses each TU against the system headers
the C compiler provides, so a ratchet count depends on gcc's version as much as
on clang-tidy's, and the baseline recorded only clang_tidy_version. Pinning gcc
a major behind every developer's box while recording nothing about it makes a
disagreement unexplainable.

Three changes:

  - the lanes install gcc-15/g++-15 from ppa:ubuntu-toolchain-r/test;
  - the ratchet records cc_version, read from the build directory's
    meson-info/intro-compilers.json so it is the compiler that actually
    produced compile_commands.json, and annotates a mismatch like it already
    does for clang-tidy;
  - the --report artifact keeps every diagnostic as `path:line:col: [check]`.
    parse_diagnostics() already produced them and the script discarded all but
    the count. The baseline stays counts-only so it remains reviewable and does
    not churn on line-number shifts.

Verified locally: the report now carries `cc (GCC) 16.2.1 20260810` and 29
diagnostics for output.cpp, naming check and location.

This requires a CI-side regeneration of tidy-baseline-cpu.json -- counts under
gcc-15's headers will not equal gcc-14's, and a local --write would resolve
enable_dnn=auto differently and rebaseline unrelated files against the wrong
build.

libvmaf-build-matrix.yml still pins gcc-14; moving it surfaces its own crop of
warnings across ten-plus legs and belongs in its own PR.

Refs: ADR-1230

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lusoris added a commit that referenced this pull request Sep 15, 2026
The clang-tidy lanes install three toolchain components and treated them
inconsistently. clang comes from apt.llvm.org at 22 because ubuntu-24.04 ships
18, which cannot parse the tree's C++26. meson comes from PyPI because the
distro's 1.3.2 predates `c23` in c_std. gcc was left at the image's gcc-14,
with no rationale in the workflow and no ADR pinning it -- the only mention of
gcc-14 in docs/adr/ is an incidental line in a VVenC changelog summary.

That stopped being cosmetic on PR #1392. The ratchet reported
test_pooling_percentile.c going 0 -> 1 while Tidy Changed -- which fails on ANY
warning and does not exclude that file -- called the same file clean, on the
same runner and commit. It could not be reproduced on gcc-15 or gcc-16.

The cause is structural: clang-tidy parses each TU against the system headers
the C compiler provides, so a ratchet count depends on gcc's version as much as
on clang-tidy's, and the baseline recorded only clang_tidy_version. Pinning gcc
a major behind every developer's box while recording nothing about it makes a
disagreement unexplainable.

Three changes:

  - the lanes install gcc-15/g++-15 from ppa:ubuntu-toolchain-r/test;
  - the ratchet records cc_version, read from the build directory's
    meson-info/intro-compilers.json so it is the compiler that actually
    produced compile_commands.json, and annotates a mismatch like it already
    does for clang-tidy;
  - the --report artifact keeps every diagnostic as `path:line:col: [check]`.
    parse_diagnostics() already produced them and the script discarded all but
    the count. The baseline stays counts-only so it remains reviewable and does
    not churn on line-number shifts.

Verified locally: the report now carries `cc (GCC) 16.2.1 20260810` and 29
diagnostics for output.cpp, naming check and location.

This requires a CI-side regeneration of tidy-baseline-cpu.json -- counts under
gcc-15's headers will not equal gcc-14's, and a local --write would resolve
enable_dnn=auto differently and rebaseline unrelated files against the wrong
build.

libvmaf-build-matrix.yml still pins gcc-14; moving it surfaces its own crop of
warnings across ten-plus legs and belongs in its own PR.

Refs: ADR-1230

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lusoris added a commit that referenced this pull request Sep 15, 2026
The clang-tidy lanes install three toolchain components and treated them
inconsistently. clang comes from apt.llvm.org at 22 because ubuntu-24.04 ships
18, which cannot parse the tree's C++26. meson comes from PyPI because the
distro's 1.3.2 predates `c23` in c_std. gcc was left at the image's gcc-14,
with no rationale in the workflow and no ADR pinning it -- the only mention of
gcc-14 in docs/adr/ is an incidental line in a VVenC changelog summary.

That stopped being cosmetic on PR #1392. The ratchet reported
test_pooling_percentile.c going 0 -> 1 while Tidy Changed -- which fails on ANY
warning and does not exclude that file -- called the same file clean, on the
same runner and commit. It could not be reproduced on gcc-15 or gcc-16.

The cause is structural: clang-tidy parses each TU against the system headers
the C compiler provides, so a ratchet count depends on gcc's version as much as
on clang-tidy's, and the baseline recorded only clang_tidy_version. Pinning gcc
a major behind every developer's box while recording nothing about it makes a
disagreement unexplainable.

Three changes:

  - the lanes install gcc-15/g++-15 from ppa:ubuntu-toolchain-r/test;
  - the ratchet records cc_version, read from the build directory's
    meson-info/intro-compilers.json so it is the compiler that actually
    produced compile_commands.json, and annotates a mismatch like it already
    does for clang-tidy;
  - the --report artifact keeps every diagnostic as `path:line:col: [check]`.
    parse_diagnostics() already produced them and the script discarded all but
    the count. The baseline stays counts-only so it remains reviewable and does
    not churn on line-number shifts.

Verified locally: the report now carries `cc (GCC) 16.2.1 20260810` and 29
diagnostics for output.cpp, naming check and location.

This requires a CI-side regeneration of tidy-baseline-cpu.json -- counts under
gcc-15's headers will not equal gcc-14's, and a local --write would resolve
enable_dnn=auto differently and rebaseline unrelated files against the wrong
build.

libvmaf-build-matrix.yml still pins gcc-14; moving it surfaces its own crop of
warnings across ten-plus legs and belongs in its own PR.

Refs: ADR-1230

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant