Repository navigation
Conversation
…class) Follow-up to PR #840 (picture_pool_cpp23_lib ODR fix). Deep audit found three more static_library targets that consumed vmaf_cflags_common before the HAVE_CUDA / HAVE_NVTX / HAVE_SYCL lines were appended to it (~line 1856 in the pre-fix file), causing the same struct-layout ODR violation in: - cuda_static_lib (line ~1178): c_args uses vmaf_cflags_common captured early; cuda/common.h includes picture.h and gates VmafCudaState on #if HAVE_CUDA. - libvmaf_feature_static_lib (line ~1700): c_args uses vmaf_cflags_common captured early; feature_extractor.cpp includes picture.h and dereferences VmafPicturePrivate at line 545. - gpu_picture_pool_cpp23_lib (line ~1793): carried NO c_args/cpp_args at all; gpu_picture_pool.cpp includes picture.h (line 30) and gpu_picture_pool.h (line 31), which also pulls in picture.h. Fix strategy (option a from the audit): 1. Move the vmaf_cflags_common += '-DHAVE_CUDA' / '-DHAVE_NVTX' / '-DHAVE_SYCL' block from after the libvmaf_sources list to just before the `if is_cuda_enabled` backend block (~line 902). This fixes cuda_static_lib and libvmaf_feature_static_lib with no per-target change — any lib that already uses `c_args: vmaf_cflags_common` picks up the defines automatically. 2. Add explicit cpp_args to gpu_picture_pool_cpp23_lib (it never references vmaf_cflags_common), mirroring the picture_pool_cpp23_lib fix from PR #840. Verified: meson setup + ninja -C build-cuda-check (-Denable_cuda=true) builds cleanly; meson test --suite=fast: 101/109 OK (8 pre-existing CUDA hardware parity failures unrelated to this change); CPU-only build: 84/84 OK. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Author
|
Superseded — bundled into PR #844 for single-CI-cycle drain. |
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.
Summary
Follow-up to PR #840 which fixed the picture_pool_cpp23_lib ODR violation by adding explicit cpp_args propagating
-DHAVE_CUDA/-DHAVE_SYCL. The deep audit found 3 MORE static libraries with the same ODR bug class:Root cause: the three
vmaf_cflags_common += '-DHAVE_*'lines came AFTER the consuming static_library() declarations had already captured the list. Meson list appends are not retroactive.Fix: (a) moved the 3 append lines BEFORE all consuming libraries — fixes #2 and #3 automatically; (b) added explicit cpp_args to gpu_picture_pool_cpp23_lib (mirrors #840 pattern).
Test plan
meson setup build-cuda -Denable_cuda=true && ninja -C build-cudacleanmeson test -C build-cuda --suite=fastPASS (test_pic_preallocation incl.)meson setup build-cpu -Denable_cuda=false && ninja -C build-cpu && meson test -C build-cpu --suite=fast84/84 PASSDeep-dive deliverables (ADR-0108)
meson test -C build-cuda test_pic_preallocationpasses after the movestate.md touch