Repository navigation
Conversation
kylophone
changed the title
lusoris
added a commit
to VMAFx/vmafx
that referenced
this pull request
Sep 5, 2026
…ge cannot represent The integer ADM pipeline stores each DWT scale's contrast-sensitivity weight as a fixed-point integer -- `uint16_t` at scale 0 (h/v bands scaled by 2^21, diagonal by 2^23) and `uint32_t` at scales 1-3 (scaled by 2^32). Those budgets were sized for the Watson97 weights, which sit around 1e-2. Two reachable configurations exceed them and were converted anyway: - `adm_csf_mode=1` (Barten) at the default `adm_csf_scale=1.0` yields 1.2105 at scale 0 and 26.98 at scale 3. The scale-0 conversions are 2538595 and 10154382 -- 38x and 155x past 65535 -- and every scale-1..3 conversion is past 2^32. Scoring the 576x324 Netflix pair with `--feature adm=adm_csf_mode=1` emitted `integer_adm2_csf_1: null` and `integer_adm_scale0_csf_1: 0.030096` against a float reference of 0.9396. - `adm_csf_mode=2` / `=3` at a viewing geometry the blended-CSF tables do not carry (e.g. `adm_ref_display_height=1200`) return `-EINVAL` *as a float*, which then reached an undefined negative-to-unsigned conversion. Add `core/src/feature/adm_csf_fixed_point.h`, which owns the fixed-point exponents, the storage bounds, the tabulated-fast-path predicate and the scale-0 narrowing conversion. `integer_adm.c` evaluates the verdict once in `init()`, caches it in `AdmState::csf_config_err`, and returns it from `extract()` beside the pre-existing viewing-geometry guard -- the same place and the same status `test_adm_coverage.c` already pins for an unsupported ADM configuration. The CUDA, HIP and SYCL twins apply the identical bounds from the same header so their accept/reject set matches the CPU reference, which the ADR-1183 option / feature-name parity contract depends on. The conversion is bit-exact with the code it replaces: the products stay `double`-valued, exactly as `float * double` promotion already made them. Configurations that fit are untouched -- `adm_csf_mode=1` with `adm_csf_scale=0.002893` / `adm_csf_diag_scale=0.001586`, and `adm_csf_mode=2` as requested by the default model `vmaf_v1.0.16_3d0h`, both still score. Netflix golden gate: 271 passed, 12 skipped, 0 failed. libvmaf fast suite: 115/115 OK. Refs ADR-1191, Netflix/vmaf#1494. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lusoris
added a commit
to VMAFx/vmafx
that referenced
this pull request
Sep 6, 2026
…ge cannot represent The integer ADM pipeline stores each DWT scale's contrast-sensitivity weight as a fixed-point integer -- `uint16_t` at scale 0 (h/v bands scaled by 2^21, diagonal by 2^23) and `uint32_t` at scales 1-3 (scaled by 2^32). Those budgets were sized for the Watson97 weights, which sit around 1e-2. Two reachable configurations exceed them and were converted anyway: - `adm_csf_mode=1` (Barten) at the default `adm_csf_scale=1.0` yields 1.2105 at scale 0 and 26.98 at scale 3. The scale-0 conversions are 2538595 and 10154382 -- 38x and 155x past 65535 -- and every scale-1..3 conversion is past 2^32. Scoring the 576x324 Netflix pair with `--feature adm=adm_csf_mode=1` emitted `integer_adm2_csf_1: null` and `integer_adm_scale0_csf_1: 0.030096` against a float reference of 0.9396. - `adm_csf_mode=2` / `=3` at a viewing geometry the blended-CSF tables do not carry (e.g. `adm_ref_display_height=1200`) return `-EINVAL` *as a float*, which then reached an undefined negative-to-unsigned conversion. Add `core/src/feature/adm_csf_fixed_point.h`, which owns the fixed-point exponents, the storage bounds, the tabulated-fast-path predicate and the scale-0 narrowing conversion. `integer_adm.c` evaluates the verdict once in `init()`, caches it in `AdmState::csf_config_err`, and returns it from `extract()` beside the pre-existing viewing-geometry guard -- the same place and the same status `test_adm_coverage.c` already pins for an unsupported ADM configuration. The CUDA, HIP and SYCL twins apply the identical bounds from the same header so their accept/reject set matches the CPU reference, which the ADR-1183 option / feature-name parity contract depends on. The conversion is bit-exact with the code it replaces: the products stay `double`-valued, exactly as `float * double` promotion already made them. Configurations that fit are untouched -- `adm_csf_mode=1` with `adm_csf_scale=0.002893` / `adm_csf_diag_scale=0.001586`, and `adm_csf_mode=2` as requested by the default model `vmaf_v1.0.16_3d0h`, both still score. Netflix golden gate: 271 passed, 12 skipped, 0 failed. libvmaf fast suite: 115/115 OK. Refs ADR-1191, Netflix/vmaf#1494. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lusoris
added a commit
to VMAFx/vmafx
that referenced
this pull request
Sep 6, 2026
…ge cannot represent The integer ADM pipeline stores each DWT scale's contrast-sensitivity weight as a fixed-point integer -- `uint16_t` at scale 0 (h/v bands scaled by 2^21, diagonal by 2^23) and `uint32_t` at scales 1-3 (scaled by 2^32). Those budgets were sized for the Watson97 weights, which sit around 1e-2. Two reachable configurations exceed them and were converted anyway: - `adm_csf_mode=1` (Barten) at the default `adm_csf_scale=1.0` yields 1.2105 at scale 0 and 26.98 at scale 3. The scale-0 conversions are 2538595 and 10154382 -- 38x and 155x past 65535 -- and every scale-1..3 conversion is past 2^32. Scoring the 576x324 Netflix pair with `--feature adm=adm_csf_mode=1` emitted `integer_adm2_csf_1: null` and `integer_adm_scale0_csf_1: 0.030096` against a float reference of 0.9396. - `adm_csf_mode=2` / `=3` at a viewing geometry the blended-CSF tables do not carry (e.g. `adm_ref_display_height=1200`) return `-EINVAL` *as a float*, which then reached an undefined negative-to-unsigned conversion. Add `core/src/feature/adm_csf_fixed_point.h`, which owns the fixed-point exponents, the storage bounds, the tabulated-fast-path predicate and the scale-0 narrowing conversion. `integer_adm.c` evaluates the verdict once in `init()`, caches it in `AdmState::csf_config_err`, and returns it from `extract()` beside the pre-existing viewing-geometry guard -- the same place and the same status `test_adm_coverage.c` already pins for an unsupported ADM configuration. The CUDA, HIP and SYCL twins apply the identical bounds from the same header so their accept/reject set matches the CPU reference, which the ADR-1183 option / feature-name parity contract depends on. The conversion is bit-exact with the code it replaces: the products stay `double`-valued, exactly as `float * double` promotion already made them. Configurations that fit are untouched -- `adm_csf_mode=1` with `adm_csf_scale=0.002893` / `adm_csf_diag_scale=0.001586`, and `adm_csf_mode=2` as requested by the default model `vmaf_v1.0.16_3d0h`, both still score. Netflix golden gate: 271 passed, 12 skipped, 0 failed. libvmaf fast suite: 115/115 OK. Refs ADR-1191, Netflix/vmaf#1494. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lusoris
added a commit
to VMAFx/vmafx
that referenced
this pull request
Sep 6, 2026
…ge cannot represent The integer ADM pipeline stores each DWT scale's contrast-sensitivity weight as a fixed-point integer -- `uint16_t` at scale 0 (h/v bands scaled by 2^21, diagonal by 2^23) and `uint32_t` at scales 1-3 (scaled by 2^32). Those budgets were sized for the Watson97 weights, which sit around 1e-2. Two reachable configurations exceed them and were converted anyway: - `adm_csf_mode=1` (Barten) at the default `adm_csf_scale=1.0` yields 1.2105 at scale 0 and 26.98 at scale 3. The scale-0 conversions are 2538595 and 10154382 -- 38x and 155x past 65535 -- and every scale-1..3 conversion is past 2^32. Scoring the 576x324 Netflix pair with `--feature adm=adm_csf_mode=1` emitted `integer_adm2_csf_1: null` and `integer_adm_scale0_csf_1: 0.030096` against a float reference of 0.9396. - `adm_csf_mode=2` / `=3` at a viewing geometry the blended-CSF tables do not carry (e.g. `adm_ref_display_height=1200`) return `-EINVAL` *as a float*, which then reached an undefined negative-to-unsigned conversion. Add `core/src/feature/adm_csf_fixed_point.h`, which owns the fixed-point exponents, the storage bounds, the tabulated-fast-path predicate and the scale-0 narrowing conversion. `integer_adm.c` evaluates the verdict once in `init()`, caches it in `AdmState::csf_config_err`, and returns it from `extract()` beside the pre-existing viewing-geometry guard -- the same place and the same status `test_adm_coverage.c` already pins for an unsupported ADM configuration. The CUDA, HIP and SYCL twins apply the identical bounds from the same header so their accept/reject set matches the CPU reference, which the ADR-1183 option / feature-name parity contract depends on. The conversion is bit-exact with the code it replaces: the products stay `double`-valued, exactly as `float * double` promotion already made them. Configurations that fit are untouched -- `adm_csf_mode=1` with `adm_csf_scale=0.002893` / `adm_csf_diag_scale=0.001586`, and `adm_csf_mode=2` as requested by the default model `vmaf_v1.0.16_3d0h`, both still score. Netflix golden gate: 271 passed, 12 skipped, 0 failed. libvmaf fast suite: 115/115 OK. Refs ADR-1191, Netflix/vmaf#1494. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lusoris
added a commit
to VMAFx/vmafx
that referenced
this pull request
Sep 6, 2026
…ge cannot represent The integer ADM pipeline stores each DWT scale's contrast-sensitivity weight as a fixed-point integer -- `uint16_t` at scale 0 (h/v bands scaled by 2^21, diagonal by 2^23) and `uint32_t` at scales 1-3 (scaled by 2^32). Those budgets were sized for the Watson97 weights, which sit around 1e-2. Two reachable configurations exceed them and were converted anyway: - `adm_csf_mode=1` (Barten) at the default `adm_csf_scale=1.0` yields 1.2105 at scale 0 and 26.98 at scale 3. The scale-0 conversions are 2538595 and 10154382 -- 38x and 155x past 65535 -- and every scale-1..3 conversion is past 2^32. Scoring the 576x324 Netflix pair with `--feature adm=adm_csf_mode=1` emitted `integer_adm2_csf_1: null` and `integer_adm_scale0_csf_1: 0.030096` against a float reference of 0.9396. - `adm_csf_mode=2` / `=3` at a viewing geometry the blended-CSF tables do not carry (e.g. `adm_ref_display_height=1200`) return `-EINVAL` *as a float*, which then reached an undefined negative-to-unsigned conversion. Add `core/src/feature/adm_csf_fixed_point.h`, which owns the fixed-point exponents, the storage bounds, the tabulated-fast-path predicate and the scale-0 narrowing conversion. `integer_adm.c` evaluates the verdict once in `init()`, caches it in `AdmState::csf_config_err`, and returns it from `extract()` beside the pre-existing viewing-geometry guard -- the same place and the same status `test_adm_coverage.c` already pins for an unsupported ADM configuration. The CUDA, HIP and SYCL twins apply the identical bounds from the same header so their accept/reject set matches the CPU reference, which the ADR-1183 option / feature-name parity contract depends on. The conversion is bit-exact with the code it replaces: the products stay `double`-valued, exactly as `float * double` promotion already made them. Configurations that fit are untouched -- `adm_csf_mode=1` with `adm_csf_scale=0.002893` / `adm_csf_diag_scale=0.001586`, and `adm_csf_mode=2` as requested by the default model `vmaf_v1.0.16_3d0h`, both still score. Netflix golden gate: 271 passed, 12 skipped, 0 failed. libvmaf fast suite: 115/115 OK. Refs ADR-1191, Netflix/vmaf#1494. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lusoris
added a commit
to VMAFx/vmafx
that referenced
this pull request
Sep 6, 2026
…ge cannot represent The integer ADM pipeline stores each DWT scale's contrast-sensitivity weight as a fixed-point integer -- `uint16_t` at scale 0 (h/v bands scaled by 2^21, diagonal by 2^23) and `uint32_t` at scales 1-3 (scaled by 2^32). Those budgets were sized for the Watson97 weights, which sit around 1e-2. Two reachable configurations exceed them and were converted anyway: - `adm_csf_mode=1` (Barten) at the default `adm_csf_scale=1.0` yields 1.2105 at scale 0 and 26.98 at scale 3. The scale-0 conversions are 2538595 and 10154382 -- 38x and 155x past 65535 -- and every scale-1..3 conversion is past 2^32. Scoring the 576x324 Netflix pair with `--feature adm=adm_csf_mode=1` emitted `integer_adm2_csf_1: null` and `integer_adm_scale0_csf_1: 0.030096` against a float reference of 0.9396. - `adm_csf_mode=2` / `=3` at a viewing geometry the blended-CSF tables do not carry (e.g. `adm_ref_display_height=1200`) return `-EINVAL` *as a float*, which then reached an undefined negative-to-unsigned conversion. Add `core/src/feature/adm_csf_fixed_point.h`, which owns the fixed-point exponents, the storage bounds, the tabulated-fast-path predicate and the scale-0 narrowing conversion. `integer_adm.c` evaluates the verdict once in `init()`, caches it in `AdmState::csf_config_err`, and returns it from `extract()` beside the pre-existing viewing-geometry guard -- the same place and the same status `test_adm_coverage.c` already pins for an unsupported ADM configuration. The CUDA, HIP and SYCL twins apply the identical bounds from the same header so their accept/reject set matches the CPU reference, which the ADR-1183 option / feature-name parity contract depends on. The conversion is bit-exact with the code it replaces: the products stay `double`-valued, exactly as `float * double` promotion already made them. Configurations that fit are untouched -- `adm_csf_mode=1` with `adm_csf_scale=0.002893` / `adm_csf_diag_scale=0.001586`, and `adm_csf_mode=2` as requested by the default model `vmaf_v1.0.16_3d0h`, both still score. Netflix golden gate: 271 passed, 12 skipped, 0 failed. libvmaf fast suite: 115/115 OK. Refs ADR-1191, Netflix/vmaf#1494. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lusoris
added a commit
to VMAFx/vmafx
that referenced
this pull request
Sep 6, 2026
…ge cannot represent, and fix the x86 DWT2 tail bound (#1339) * fix(adm): reject integer-ADM CSF configurations the fixed-point storage cannot represent The integer ADM pipeline stores each DWT scale's contrast-sensitivity weight as a fixed-point integer -- `uint16_t` at scale 0 (h/v bands scaled by 2^21, diagonal by 2^23) and `uint32_t` at scales 1-3 (scaled by 2^32). Those budgets were sized for the Watson97 weights, which sit around 1e-2. Two reachable configurations exceed them and were converted anyway: - `adm_csf_mode=1` (Barten) at the default `adm_csf_scale=1.0` yields 1.2105 at scale 0 and 26.98 at scale 3. The scale-0 conversions are 2538595 and 10154382 -- 38x and 155x past 65535 -- and every scale-1..3 conversion is past 2^32. Scoring the 576x324 Netflix pair with `--feature adm=adm_csf_mode=1` emitted `integer_adm2_csf_1: null` and `integer_adm_scale0_csf_1: 0.030096` against a float reference of 0.9396. - `adm_csf_mode=2` / `=3` at a viewing geometry the blended-CSF tables do not carry (e.g. `adm_ref_display_height=1200`) return `-EINVAL` *as a float*, which then reached an undefined negative-to-unsigned conversion. Add `core/src/feature/adm_csf_fixed_point.h`, which owns the fixed-point exponents, the storage bounds, the tabulated-fast-path predicate and the scale-0 narrowing conversion. `integer_adm.c` evaluates the verdict once in `init()`, caches it in `AdmState::csf_config_err`, and returns it from `extract()` beside the pre-existing viewing-geometry guard -- the same place and the same status `test_adm_coverage.c` already pins for an unsupported ADM configuration. The CUDA, HIP and SYCL twins apply the identical bounds from the same header so their accept/reject set matches the CPU reference, which the ADR-1183 option / feature-name parity contract depends on. The conversion is bit-exact with the code it replaces: the products stay `double`-valued, exactly as `float * double` promotion already made them. Configurations that fit are untouched -- `adm_csf_mode=1` with `adm_csf_scale=0.002893` / `adm_csf_diag_scale=0.001586`, and `adm_csf_mode=2` as requested by the default model `vmaf_v1.0.16_3d0h`, both still score. Netflix golden gate: 271 passed, 12 skipped, 0 failed. libvmaf fast suite: 115/115 OK. Refs ADR-1191, Netflix/vmaf#1494. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(adr): route the ADR-1191 index row through _index_fragments (ADR-0221) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(state): close T-UPSTREAM-1494-ADM-CSF-MODE-IRFACTOR-OVERFLOW stage 1 (PR #1339) Moves the row from '## Open bugs' to '## Recently closed', citing ADR-1191 and PR #1339. Records what stage 1 fixed (the unrepresentable CSF configurations now return -EINVAL on the CPU and on all three GPU twins) and what it did not (the widening half, still tracked under T-ADM-CSF-MODE-1-BARTEN-DEGENERATE-2026-09-05). Corrects the row's stale 'no cross-backend hole' claim, which #1324 invalidated by mirroring the CPU option table onto the CUDA, SYCL and HIP twins. Updates the two cross-referencing rows to match. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(feature/adm): fix x86 DWT2 tail bounds for AVX2 and AVX-512 (cherry picked from commit dd6b018) * docs(state): record the x86 DWT2 tail-bound port and close T-UPSTREAM-1494 stage 1 Absorbs the unique half of PR #1303 (x86 DWT2 tail bound, cherry-picked in the preceding commit) so that PR can be closed without losing work: - docs/rebase-notes.md: adds the AVX2 / AVX-512 `half_w - 1 - ((half_w - 2) % N)` tail-bound invariant and the pinning test to this branch's entry. Upstream Netflix still carries the unguarded bound, so the guard must survive syncs. - docs/state.md: the T-UPSTREAM-1564 Recently-closed row now records residual (3) (the x86 tail bound) as closed here; T-UPSTREAM-1494 stage 1 moves from Open to Recently closed with a marker comment pointing at the move, so the id appears exactly once as a row. Stage 2 stays open under T-ADM-CSF-MODE-1-BARTEN-DEGENERATE-2026-09-05. - CHANGELOG.md re-rendered from changelog.d/ by scripts/release/concat-changelog-fragments.sh --write. Verified: scripts/ci/check-state-md-rows.sh docs/state.md -> OK (361 rows, no duplicate ids); meson test -C build-cpu test_adm_dwt2_x86 test_adm_csf_representable test_adm_coverage test_integer_adm_simd -> Ok 4 / Fail 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(test): make the x86 ADM DWT2 parity test clang-tidy clean The ported test carried 29 clang-tidy findings (function-size, isolate-declaration, modernize-use-nullptr, unix.Malloc leak paths on the early-return branch, cert-err33-c on fprintf), which ADR-0141 and the ADR-1142 whole-tree ratchet do not allow a newly added file to introduce. Restructure instead of suppressing: - ref_src_indices() splits into a per-axis ref_mirror_indices() helper, which also removes the duplicated y/x mirror loops and brings both functions under the 60-line budget. - The two near-identical test bodies collapse into one dwt2_kernel_matches_scalar(label, kernel) driver behind an adm_dwt2_8_fn function pointer, with a Dwt2Fixture that owns every buffer and a single fixture_free() cleanup path, so the analyzer no longer sees a leak on the mismatch branch. - fprintf return values are (void)-discarded; declarations are isolated. - modernize-use-nullptr is suppressed with the same NOLINTBEGIN block and the same ADR-1138 citation core/test/test_adm_csf_representable.c already uses for this C translation unit. Behaviour is unchanged: the test still fails against the origin/master kernels (AVX2 34x32 band_a[0][16] scalar 5589 != simd 6706) and passes with the fixed tail bound. clang-tidy -p build-cpu reports 0 findings. * docs(state): drop the duplicate rows a keep-both rebase created Each dropped row restates one origin/master already carries; master is the authoritative record. Verified with scripts/ci/check-state-md-rows.sh. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Lusoris <lusoris@pm.me> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
lusoris
added a commit
to VMAFx/vmafx
that referenced
this pull request
Sep 16, 2026
`scripts/ci/check-state-md-rows.sh` enforces ADR-0165's "every bug id appears exactly once" rule, and reported a file carrying 33 duplicated rows as clean. It matched only `| **T-ID**`: - non-bold ids hid 95 of 576 id-bearing rows (17%), among them a byte-identical duplicate of `T-CUDA-MUL24-AUDIT-2026-05-28`; - `Netflix#NNN` rows were never matched at all — 34 rows carrying 13 duplicate pairs; - `**T6-1**` / `**T7-16**` tranche ids added four more; - ~143 rows open with prose and carry no id, so no id-based check can reach them; 13 of those were duplicated too. The reporting path had a second, independent bug: it located hits with a bold-only pattern, so every non-bold duplicate printed "appears on lines:" followed by nothing. Detection and reporting now share one awk extraction rule, which is what made that divergence possible. Added a shape-independent verbatim-row check for the prose-led rows. It normalises away the `_(verified YYYY-MM-DD: ...)_` annotation a later verification sweep appends to one copy — without that, a row and its own annotated duplicate compare unequal and the check reports clean. Column headers repeat once per section by design and are excluded; counting them was a false positive that this gate's own test fixture caught. All 33 duplicates are resolved. **The resolution direction is not uniform.** Most pairs keep the later copy, which carries the verification annotation and in two cases resolves "this PR" to the real PR number. But `T6-2` keeps the *earlier* copy — it says PR #469 is merged where the later says it is still in flight — and `Netflix/vmaf#1494` keeps the earlier copy, which records ADR-1191 as closed where the later does not. A blanket "delete the first occurrence" would have silently reverted two bugs to an older state. Every pair was read before either copy was deleted, and each deletion was verified to leave its twin byte-identical in the file. No ADR: the gate's contract is unchanged, ADR-0165 already states it. This is the implementation catching up with it. Six new cases cover the four id shapes, the prose-led-plus-suffix shape and the header false positive; all twelve pass, shellcheck is clean, and `pre-commit run --all-files` is green. Closes ledger L-05. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Sep 19, 2026
|
Tested at f601b8e, built on its own base (eb88c00) and merged against master 6ec23e8. Against its own base: builds (gcc 16.2.1). Scores on the three Netflix pairs (frames and pooled values; psnr, float_ssim, motion, float_motion, vmaf_v0.6.1) are identical to the base with the default CPU mask, with AVX-512 masked off (
Against master: it does not merge. UBSan at rdh=480: the PR head reports Our open PRs (
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.