Skip to content

libvmaf: add median and percentile C API pooling - #1589

Open
lusoris wants to merge 1 commit into
Netflix:masterfrom
VMAFx:feat/percentile-pooling
Open

lusoris wants to merge 1 commit into
Netflix:masterfrom
VMAFx:feat/percentile-pooling

Conversation

@lusoris

@lusoris lusoris commented Sep 8, 2026 •

Copy link
Copy Markdown

The C pooling API cannot currently select the median or lower percentiles requested in #818. This adapts VMAFx #1340 to current upstream C11: append MEDIAN, PERC5, PERC10, and PERC20, gather the scores selected by the inclusive interval and n_subsample, then sort and linearly interpolate at percentile * (n - 1) / 100. Existing pooling values, accumulation order, and allocation behavior remain unchanged. The README and public header document the semantics and memory cost.

The implementation explicitly rejects non-finite samples because current upstream permits importing them (unlike the fork's collector). It propagates missing-score errors, rejects an empty selected interval, checks allocation size, and uses 64-bit interval arithmetic to avoid wrapping at UINT_MAX. The feature-pooling output is unchanged on failure.

This adds the C API functionality. FFmpeg's filter option mapping needs a separate FFmpeg-side change, so this does not claim that its pool option immediately accepts the new values. No CLI option, model, bootstrap-confidence-interval formula, or golden assertion changes.

Correction, 2026-09-19: as submitted until today (fe73c3fd, rebased as e2c17610) this PR broke the XML and JSON reports. Their writers in output.c loop over every method below VMAF_POOL_METHOD_NB and index a name table with four entries, so with the four new enum values they read past the table and printed garbage keys in pooled_metrics; ASan reports a global-buffer-overflow at output.c:131. The writers now stop at the end of the name table. Reports keep listing min, max, mean and harmonic_mean only, the percentile methods are reachable through the API, and XML, JSON, CSV and subtitle output of the vmaf tool is identical to upstream's apart from the version string and fps. A sixth regression case writes a JSON report through vmaf_write_output() and requires exactly those four pooled entries; it fails against the earlier state of this branch with “pooled psnr_y does not have exactly four entries”.

Validation on upstream 9e48141bd1eb8d2329e09d3744e7c24af53017ca, x86-64 Linux, GCC 16.2.1 / C11, Meson 1.12.1. Rebased on master 9e48141 (2026-10-02); the patch is unchanged and the results below were re-run (the negative controls were run on 6ec23e8; the code involved is unchanged in 9e48141):

meson setup build libvmaf -Denable_cuda=false -Denable_docs=false -Denable_float=true --buildtype=release
ninja -C build
meson test -C build --print-errorlogs
# 25/25 passed

meson setup build-sanitize libvmaf -Denable_cuda=false -Denable_docs=false -Denable_float=true --buildtype=debug -Db_sanitize=address,undefined \
  -Db_lto=false
ninja -C build-sanitize
meson test -C build-sanitize test_pool_percentile --print-errorlogs
# 6 regression cases passed with ASan/UBSan/LSan

Cases cover interpolation on unsorted scores without mutating their stored order, singleton/tied/negative values, subsampling and partial intervals, missing/non-finite scores, invalid arguments, UINT_MAX endpoints, and delegation from cached model scores. The new interpolation test fails when only libvmaf.c is replaced by the original upstream file, and the report test fails when only output.c is (UBSan reports index 5 of the 5-entry name table, and the vmaf tool reports the same at output.c:131 when writing an XML report). Sanitizer coverage claimed here is the targeted test; the full sanitizer suite passes on this branch except test_predict and test_pic_preallocation, which abort the same way on unpatched master. The three Netflix reference pairs score identically with and without this change (default dispatch and SIMD masked off), and the XML, JSON, CSV and subtitle reports of a 48-frame run are identical to master's apart from the version string and fps.

@lusoris
lusoris force-pushed the feat/percentile-pooling branch 2 times, most recently from e2c1761 to 095b4c2 Compare September 19, 2026 10:07
@lusoris

lusoris commented Sep 19, 2026

Copy link
Copy Markdown
Author

Correction to this PR: as submitted it corrupted the pooled_metrics section of XML and JSON reports. The report writers in output.c loop over every method below VMAF_POOL_METHOD_NB and index a name table with four entries, so the four new enum values made them read past the table. The vmaf tool printed garbage keys after harmonic_mean, and ASan reports a global-buffer-overflow at output.c:131. The earlier validation ran the unit tests but never wrote a report, which is why it was not caught.

Fixed in 095b4c20: the writers stop at the end of the name table, so reports list min, max, mean and harmonic_mean as before and the percentile methods stay API-only. XML, JSON, CSV and subtitle output is identical to upstream 86da14d0 apart from the version string and fps, and a sixth test case writes a JSON report and requires exactly four pooled entries (it fails against the earlier state of the branch). The branch was also rebased onto 86da14d0; the description has the details.

@lusoris
lusoris force-pushed the feat/percentile-pooling branch 3 times, most recently from f3d3c98 to 3077782 Compare October 2, 2026 05:22
@lusoris
lusoris force-pushed the feat/percentile-pooling branch from 3077782 to 1436dba Compare October 2, 2026 18:38
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