Repository navigation
fix(test): include internal picture.h for vmaf_picture_ref decl (macOS Clang + TSan) - #760
Merged
Merged
Conversation
21 of 27 tasks
lusoris
added a commit
that referenced
this pull request
Oct 2, 2026
…oes (ADR-1489) Two inherited routines compute the contrast-sensitivity weights of float ADM, and fork commits had widened both from float to double to quiet a static-analysis finding (cpp/integer-multiplication-cast-to-long), without a decision: - dwt_quant_step() in adm_tools.h (Watson): #552 cast an operand of the exponent k * temp * temp, #760 then kept r, temp and Q in double. 32 of 40 probed steps were not Netflix's bits. - barten_csf_tools.h (Barten, adm_csf_mode=1): #44 promoted one operand of six float products and quotients. 138 of 144 probed weight sets were not Netflix's bits. Both evaluate Netflix's arithmetic again. adm_tools.h has Netflix's three statements, with a suppression comment for the finding. The Barten header forms each product and quotient in float and writes the promotion of the result out: the SYCL and Metal twins of integer ADM compile it as C++, where pow(float, float) and exp(float) are the float functions, so Netflix's implicit form returns other weights there (135 of 144 probed sets) than in C. The Metal copy of the step (float_adm_metal.mm) follows; the CUDA, HIP and SYCL twins of float_adm call the CPU's routine. ADM_OPT_RECIP_DIVISION stays undefined (ADR-1442). Against Netflix/vmaf cea2b4d8 built with the plain quotient, float_adm is identical on every measured value (8225 of 8225 default-run values, 71280 of 71280 over 36 option variants, 1764 of 1764 frame scores of seven float models; 658 frames, scalar, AVX2 and AVX-512 dispatch): the division is the only difference left. Against Netflix as built on x86, adm2 is identical on 464 of 658 frames (30 before). Scores move: float_adm adm2 by at most 1.14e-7, the float models by at most 2.7e-5 on a frame, fixed-point adm with adm_csf_mode=1 by at most 1.6e-7. Fixed-point adm in its default mode, the default models and the testdata snapshots do not move. Twins on this host stay bit-identical to the CPU: float_adm and adm on CUDA (RTX 4090), HIP (gfx1036) and SYCL (Arc A380), parity tests and the gate's cells at tolerance 0, and integer adm in Barten mode on each twin. Metal: source changed, not run (no device). Snapshots: no CPU snapshot moves (they hold fixed-point features in Watson mode). testdata/scores_sycl_a380_{576,640,720,1080,4k}.json are re-recorded, because the files of 2026-09-26 no longer described the twin: they differ from the new ones on 101 to 139 of 576 shared values by up to 1.0e-4 and lack three metrics. Recorded with testdata/run_sycl_scores.py a380 inside the dev container (image sha256:43ef1e32cb32b148a076ed6dff73b72d7a6566ca3882bf90954b8a34a74761fc, icx 2026.1.1, glibc 2.43, /dev/dri passed through, Arc A380, compute runtime 26.35.39758), from this change as it stood at 539e99261 (no file under core/ differs from this commit). The image has no ocloc, so the binary was built with -Dsycl_icpx_aot_targets= (kernels compiled at run time). The recordings equal the CPU snapshots on 3599 of 3600 values; the one is vmaf of frame 26 at 1280x720 (88.435637 for 88.435634), an icx build's math library (T-ICX-LIBIMF-HOST-MATH-2026-10-01). The B580 and UHD 770 recordings stay stale: T-SYCL-SNAPSHOTS-STALE-2026-10-02. Closes T-ADM-CSF-EXPONENT-NOT-UPSTREAM-2026-10-01.
lusoris
added a commit
that referenced
this pull request
Oct 2, 2026
…oes (ADR-1489) Two inherited routines compute the contrast-sensitivity weights of float ADM, and fork commits had widened both from float to double to quiet a static-analysis finding (cpp/integer-multiplication-cast-to-long), without a decision: - dwt_quant_step() in adm_tools.h (Watson): #552 cast an operand of the exponent k * temp * temp, #760 then kept r, temp and Q in double. 32 of 40 probed steps were not Netflix's bits. - barten_csf_tools.h (Barten, adm_csf_mode=1): #44 promoted one operand of six float products and quotients. 138 of 144 probed weight sets were not Netflix's bits. Both evaluate Netflix's arithmetic again. adm_tools.h has Netflix's three statements, with a suppression comment for the finding. The Barten header forms each product and quotient in float and writes the promotion of the result out: the SYCL and Metal twins of integer ADM compile it as C++, where pow(float, float) and exp(float) are the float functions, so Netflix's implicit form returns other weights there (135 of 144 probed sets) than in C. The Metal copy of the step (float_adm_metal.mm) follows; the CUDA, HIP and SYCL twins of float_adm call the CPU's routine. ADM_OPT_RECIP_DIVISION stays undefined (ADR-1442). Against Netflix/vmaf cea2b4d8 built with the plain quotient, float_adm is identical on every measured value (8225 of 8225 default-run values, 71280 of 71280 over 36 option variants, 1764 of 1764 frame scores of seven float models; 658 frames, scalar, AVX2 and AVX-512 dispatch): the division is the only difference left. Against Netflix as built on x86, adm2 is identical on 464 of 658 frames (30 before). Scores move: float_adm adm2 by at most 1.14e-7, the float models by at most 2.7e-5 on a frame, fixed-point adm with adm_csf_mode=1 by at most 1.6e-7. Fixed-point adm in its default mode, the default models and the testdata snapshots do not move. Twins on this host stay bit-identical to the CPU: float_adm and adm on CUDA (RTX 4090), HIP (gfx1036) and SYCL (Arc A380), parity tests and the gate's cells at tolerance 0, and integer adm in Barten mode on each twin. Metal: source changed, not run (no device). Snapshots: no CPU snapshot moves (they hold fixed-point features in Watson mode). testdata/scores_sycl_a380_{576,640,720,1080,4k}.json are re-recorded, because the files of 2026-09-26 no longer described the twin: they differ from the new ones on 101 to 139 of 576 shared values by up to 1.0e-4 and lack three metrics. Recorded with testdata/run_sycl_scores.py a380 inside the dev container (image sha256:43ef1e32cb32b148a076ed6dff73b72d7a6566ca3882bf90954b8a34a74761fc, icx 2026.1.1, glibc 2.43, /dev/dri passed through, Arc A380, compute runtime 26.35.39758), from this change as it stood at 539e99261 (no file under core/ differs from this commit). The image has no ocloc, so the binary was built with -Dsycl_icpx_aot_targets= (kernels compiled at run time). The recordings equal the CPU snapshots on 3599 of 3600 values; the one is vmaf of frame 26 at 1280x720 (88.435637 for 88.435634), an icx build's math library (T-ICX-LIBIMF-HOST-MATH-2026-10-01). The B580 and UHD 770 recordings stay stale: T-SYCL-SNAPSHOTS-STALE-2026-10-02. Closes T-ADM-CSF-EXPONENT-NOT-UPSTREAM-2026-10-01.
lusoris
added a commit
that referenced
this pull request
Oct 2, 2026
…oes (ADR-1489) (#1894) * fix(adm): compute the CSF weights of float ADM in float, as Netflix does (ADR-1489) Two inherited routines compute the contrast-sensitivity weights of float ADM, and fork commits had widened both from float to double to quiet a static-analysis finding (cpp/integer-multiplication-cast-to-long), without a decision: - dwt_quant_step() in adm_tools.h (Watson): #552 cast an operand of the exponent k * temp * temp, #760 then kept r, temp and Q in double. 32 of 40 probed steps were not Netflix's bits. - barten_csf_tools.h (Barten, adm_csf_mode=1): #44 promoted one operand of six float products and quotients. 138 of 144 probed weight sets were not Netflix's bits. Both evaluate Netflix's arithmetic again. adm_tools.h has Netflix's three statements, with a suppression comment for the finding. The Barten header forms each product and quotient in float and writes the promotion of the result out: the SYCL and Metal twins of integer ADM compile it as C++, where pow(float, float) and exp(float) are the float functions, so Netflix's implicit form returns other weights there (135 of 144 probed sets) than in C. The Metal copy of the step (float_adm_metal.mm) follows; the CUDA, HIP and SYCL twins of float_adm call the CPU's routine. ADM_OPT_RECIP_DIVISION stays undefined (ADR-1442). Against Netflix/vmaf cea2b4d8 built with the plain quotient, float_adm is identical on every measured value (8225 of 8225 default-run values, 71280 of 71280 over 36 option variants, 1764 of 1764 frame scores of seven float models; 658 frames, scalar, AVX2 and AVX-512 dispatch): the division is the only difference left. Against Netflix as built on x86, adm2 is identical on 464 of 658 frames (30 before). Scores move: float_adm adm2 by at most 1.14e-7, the float models by at most 2.7e-5 on a frame, fixed-point adm with adm_csf_mode=1 by at most 1.6e-7. Fixed-point adm in its default mode, the default models and the testdata snapshots do not move. Twins on this host stay bit-identical to the CPU: float_adm and adm on CUDA (RTX 4090), HIP (gfx1036) and SYCL (Arc A380), parity tests and the gate's cells at tolerance 0, and integer adm in Barten mode on each twin. Metal: source changed, not run (no device). Snapshots: no CPU snapshot moves (they hold fixed-point features in Watson mode). testdata/scores_sycl_a380_{576,640,720,1080,4k}.json are re-recorded, because the files of 2026-09-26 no longer described the twin: they differ from the new ones on 101 to 139 of 576 shared values by up to 1.0e-4 and lack three metrics. Recorded with testdata/run_sycl_scores.py a380 inside the dev container (image sha256:43ef1e32cb32b148a076ed6dff73b72d7a6566ca3882bf90954b8a34a74761fc, icx 2026.1.1, glibc 2.43, /dev/dri passed through, Arc A380, compute runtime 26.35.39758), from this change as it stood at 539e99261 (no file under core/ differs from this commit). The image has no ocloc, so the binary was built with -Dsycl_icpx_aot_targets= (kernels compiled at run time). The recordings equal the CPU snapshots on 3599 of 3600 values; the one is vmaf of frame 26 at 1280x720 (88.435637 for 88.435634), an icx build's math library (T-ICX-LIBIMF-HOST-MATH-2026-10-01). The B580 and UHD 770 recordings stay stale: T-SYCL-SNAPSHOTS-STALE-2026-10-02. Closes T-ADM-CSF-EXPONENT-NOT-UPSTREAM-2026-10-01.
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
1 participant
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.
test_integer_motion_coverage.c calls vmaf_picture_ref() (introduced by PR #747) but only included the public libvmaf/picture.h — that header does not expose the symbol. The declaration lives in the internal core/src/picture.h.
macOS Clang and TSan Sanitizers builds fail with: error: call to undeclared function 'vmaf_picture_ref' at lines 151 and 215.
Fix: add #include "picture.h" (internal) right after the public include. The test executable already builds with -I../src/ so the path resolves.
ADR-0108: trivial build-fix following PR #747, no other deliverables.