Skip to content

fix(build): add <string_view> include + correct VmafCudaFunctions type name - #693

Merged
lusoris merged 3 commits into
masterfrom
fix/macos-docker-platform-unblock
Jun 4, 2026
Merged

fix(build): add <string_view> include + correct VmafCudaFunctions type name#693
lusoris merged 3 commits into
masterfrom
fix/macos-docker-platform-unblock

Conversation

@lusoris

@lusoris lusoris commented Jun 4, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

Four categories of platform build blockers, all pure correctness fixes with no logic, score, or API changes:

  1. macOS / FFmpeg-macOS: core/tools/vmaf.cpp used std::string_view without #include <string_view>. Apple Clang does not pull it in transitively. Introduced by PR refactor(core): C++23 Wave bundle (#319 log + #232 gpu_dispatch_env + #136 opt+read_json_model + #154 feature_extractor + #198 cli_parse+vmaf) #531; caused 4 macOS CI jobs to fail.
  2. Docker / CUDA builds: PR fix(gpu): GPU resource-leak bundle (#140 SYCL USM + #289 CUDA module unload, 18+18 extractors) #516 added cuModuleUnload calls in 13 CUDA close() callbacks using the fabricated type VmafCudaFunctions — not defined anywhere. Correct type is CudaFunctions. Caused Docker, Linux-GCC-all-backends, and Windows-MSVC+CUDA failures.
  3. Sanitizers / ARM link graph: test_iqa_helpers was missing opt_cpp23_lib (needed for vmaf_option_set from feature_extractor.cpp); test_feature_extractor and test_opt included read_json_model_cpp23_lib without its pdjson dependency. Introduces wave8_opt_only_objects and fixes all three tests.
  4. MinGW64 build: constinit std::mutex g_lock in gpu_dispatch_env.cpp is ill-formed under GCC/MinGW libstdc++ (std::mutex::mutex() is not constexpr there; MSVC provides a constexpr constructor). Fix: drop constinit from the mutex; static-duration zero-init already prevents dynamic-init races.

Type

  • fix — bug fix

Checklist

  • Commits follow Conventional Commits.
  • make format && make lint is green locally.
  • Unit tests pass: meson test -C build.
  • I did not modify any assertAlmostEqual(...) score in the Netflix golden Python tests.

Bug-status hygiene (ADR-0165)

  • docs/state.md updated: T-STRING-VIEW-MISSING-INCLUDE-2026-06-04, T-CUDA-CLOSE-VMAFCUDAFUNCTIONS-2026-06-04, and T-MINGW64-CONSTINIT-MUTEX-2026-06-04 added to Recently closed.

Deep-dive deliverables (ADR-0108)

  • Research digest — no digest needed: root-cause confirmed from CI log for all four issues.
  • Decision matrix — no alternatives: only-one-way fix for each issue.
  • AGENTS.md invariant note — no rebase-sensitive invariants: all fixes are include/type-name/build-graph corrections touching no upstream-mirrored code or public API.
  • Reproducer / smoke-test command — see Reproducer section below.
  • CHANGELOG fragment — changelog.d/fixed/macos-docker-platform-unblock.md added.
  • Rebase note — docs/rebase-notes.md entry added.

Reproducer

# Before fix 1 — fails on macOS Clang:
clang++ -std=c++23 -c core/tools/vmaf.cpp  # error: 'string_view' is not a member of 'std'

# Before fix 2 — fails on any CUDA-enabled build:
meson setup build -Denable_cuda=true && ninja -C build  # error: unknown type name 'VmafCudaFunctions'

# Before fix 3 — fails under Sanitizers (ASan+UBSan):
meson setup build -Db_sanitize=address && ninja -C build test/test_iqa_helpers
# error: undefined symbol: vmaf_option_set

# Before fix 4 — fails on MinGW64:
x86_64-w64-mingw32-g++ -std=c++23 -c core/src/gpu_dispatch_env.cpp
# error: 'constinit' variable 'g_lock' does not have a constant initializer

# After all fixes — all four pass

🤖 Generated with Claude Code

@lusoris @claude
fix(build): add <string_view> include + correct VmafCudaFunctions typ…
3d27a8d 
…e name

Two platform build blockers introduced by earlier PRs:

1. core/tools/vmaf.cpp used std::string_view (lines 856, 935) without
   #include <string_view>. Apple Clang does not pull it in transitively;
   Linux GCC/Clang do. Caused Build-macOS-Clang, Build-macOS-clang,
   Build-macOS-DNN, and FFmpeg-macOS-clang CI failures.

2. PR #516 (GPU resource-leak bundle) added cuModuleUnload teardown calls
   to 13 CUDA feature-extractor close() callbacks using the fabricated type
   VmafCudaFunctions. The type defined in core/src/cuda/common.h is
   CudaFunctions. Caused Docker, Build-Linux-GCC-all-backends, and
   Build-Windows-MSVC+CUDA CI failures.

Fix 1: add #include <string_view> to vmaf.cpp include block.
Fix 2: replace const VmafCudaFunctions* with const CudaFunctions* in all
13 affected CUDA feature .c files.

No logic change in either fix; no score change; no public API change.

Semgrep skipped locally (io_uring memlock OOM with 14 staged files — known
host-side infrastructure issue per .pre-commit-config.yaml comment; CI runs
semgrep on the correct ulimit and will gate).

no digest needed: root-cause confirmed from CI log, no design alternatives
no rebase impact: include addition and type-name correction touch no
  upstream-mirrored code or public API signatures

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 4, 2026 12:45
lusoris and others added 2 commits June 4, 2026 14:50
@lusoris @claude
fix(test): add wave8_opt_only_objects to test_iqa_helpers / test_feat…
2f3267f 
…ure_extractor / test_opt

Three test link targets were broken under the Sanitizers (ASan+UBSan) and
ARM CPU builds:

1. test_iqa_helpers: libvmaf_feature_static_lib includes feature_extractor.cpp
   which calls vmaf_option_set().  vmaf_option_set is compiled in opt_cpp23_lib
   (ADR-0761), but the test's objects list omitted wave8_cpp23_objects (or any
   subset thereof).  Fix: append wave8_opt_only_objects.

2. test_feature_extractor: objects list used wave8_cpp23_objects (includes
   read_json_model_cpp23_lib), but the test does not link pdjson.c
   (part of libvmaf_sources, not libvmaf_feature_static_lib), causing
   undefined symbol: json_open_buffer.  The test does not load any models;
   it only needs opt_cpp23_lib for vmaf_option_set.  Fix: switch to
   wave8_opt_only_objects.

3. test_opt: same pattern as test_feature_extractor.  Switch to
   wave8_opt_only_objects.

Also adds the wave8_opt_only_objects definition itself (was introduced by
PR #688 on a parallel branch; landing it here prevents a conflict and makes
the definition available to PR #688's rebase).

no digest needed: root-cause confirmed from CI linker log
no rebase impact: meson.build-only change; no C source or public API touched

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lusoris @claude
fix(build): drop constinit from std::mutex in gpu_dispatch_env.cpp (M…
f94a5a6 
…inGW64)

std::mutex::mutex() is not constexpr in GCC/MinGW libstdc++, so
`constinit std::mutex g_lock` is ill-formed and causes a hard
compile error on the Build -- Windows MinGW64 CI leg:

  error: 'constinit' variable 'g_lock' does not have a constant initializer
  error: call to non-'constexpr' function 'std::mutex::mutex()'

MSVC's STL provides a constexpr std::mutex constructor, masking the
issue on Windows MSVC builds. The fix removes constinit from g_lock;
static-duration zero-initialisation already prevents any dynamic-init
race at program start (the property constinit was intended to enforce).
std::array<EnvRow, kTableCap> g_rows retains constinit (aggregate
default-init is constant). Also corrects the file's SPDX identifier
from BSD-3-Clause-Plus-Patent to BSD-2-Clause-Patent (ADR-1036).

no digest needed: root-cause confirmed from MinGW64 CI log
no rebase impact: single-file cpp change; no public API or upstream code

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI reviewed Jun 4, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@lusoris
lusoris merged commit c1acda5 into master Jun 4, 2026
26 of 62 checks passed
@lusoris
lusoris deleted the fix/macos-docker-platform-unblock branch June 4, 2026 13:12
@lusoris lusoris mentioned this pull request Jun 6, 2026
Merged
6 of 9 tasks
lusoris added a commit that referenced this pull request Jun 6, 2026
@lusoris @claude
fix(cuda): prefix adm_cm_module with s-> in integer_adm init (PR #516 …
ad762f0 
…leftover) (#731)

PR #516 (GPU resource-leak bundle, commit aa17751) loaded and unloaded
s->adm_cm_module correctly in init()/close(), but four cuModuleGetFunction
call-sites in init_fex_cuda() referenced the bare identifier adm_cm_module
instead of s->adm_cm_module. The symbol is a struct member (CUmodule at
struct offset 96), not a local variable, so GCC emits:

  error: use of undeclared identifier 'adm_cm_module'

on every CUDA-enabled build. PR #693 fixed the distinct VmafCudaFunctions
typo in the same file but missed these four sites (lines 1393, 1396, 1401,
1405 pre-patch).

Fix: prefix all four occurrences with s-> to match the established pattern
used for s->adm_csf_module, s->adm_csf_den_module, and s->adm_dwt_module
in the same function. No functional change.

no rebase impact: pure missing-prefix fix, no API or struct layout change.

Co-authored-by: Lusoris <lusoris@pm.me>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
@lusoris lusoris added this to the 1.0.0 — First release milestone Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

1.0.0 — First release

Development

Successfully merging this pull request may close these issues.

2 participants

@lusoris