Repository navigation
Conversation
lusoris
marked this pull request as ready for review
May 29, 2026 10:12
lusoris
enabled auto-merge (squash)
May 29, 2026 10:12
lusoris
disabled auto-merge
May 29, 2026 11:49
lusoris
marked this pull request as draft
May 29, 2026 11:49
…pp (ADR-0761) Activate opt.cpp (completing Wave 1 / ADR-0721) and convert read_json_model.c → read_json_model.cpp (new, 639 LOC). Both compile under cpp_std=c++23 in isolated static libraries (opt_cpp23_lib / read_json_model_cpp23_lib) linked into libvmaf.so via extract_all_objects(). Public C ABI unchanged. Changes per file: - opt.h: extern "C" guards added; opt_cpp23_lib wired into meson - read_json_model.cpp: nullptr, static_cast<>, [[nodiscard]] on 4 entry points; goto-past-init replaced with if/else scoping; C++ <c*> headers - log.h, model.h, read_json_model.h: extern "C" guards added - core/src/meson.build: opt_cpp23_lib + read_json_model_cpp23_lib static libs; opt.c and read_json_model.c comments in libvmaf_sources - core/test/meson.build: wave8_cpp23_objects variable; all affected test executables updated to use objects instead of direct source compilation - scripts/ci/coverage-check.sh: updated opt.c -> opt.cpp, read_json_model.c -> read_json_model.cpp in critical-coverage exclusion list - core/src/AGENTS.md: invariant 10 documenting the active .cpp files predict.c skipped: depends on feature_extractor.h / feature_collector.h without extern "C" guards; deferred to a later wave. Build: 49/49 fast-suite pass (CPU-only). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
lusoris
force-pushed
the
feat/cpp23-wave8-bundle-20260529
branch
from
May 29, 2026 12:08
146d3c7 to
d80ce78
Compare
lusoris
marked this pull request as ready for review
May 31, 2026 13:47
Contributor
Author
|
Superseded by master merge marathon 2026-05-31. |
Contributor
Author
|
Superseded by #531 — bundled per 2026-06-01 triage. |
lusoris
added a commit
that referenced
this pull request
Jun 2, 2026
…136 opt+read_json_model + #154 feature_extractor + #198 cli_parse+vmaf) (#531) * refactor(core): C++23 pilot — log.c → log.cpp wired (ADR-0708 Wave 1) Converts core/src/log.c to core/src/log.cpp and wires it into the meson build as the second C++23 pilot under ADR-0708 (first was dict.c/dict.cpp per ADR-0727). The log.cpp file existed on master from the earlier PR #45 attempt but remained inert (meson still referenced log.c, see PR #205 "Wave 1 INERT" audit). This PR completes the conversion by: - Adding `extern "C"` guards to core/src/log.h so the header is includable from both C and C++ TUs. - Wrapping the function definitions in log.cpp in `extern "C"` so the emitted symbols carry C-mangling (verified via nm: `vmaf_log` and `vmaf_set_log_level` are C-mangled exports, same as the prior log.c build). - Replacing the C `static` file-scope log state with a C++ anonymous namespace (clang-tidy misc-use-anonymous-namespace clean). - Compiling log.cpp in an isolated `log_cpp23_lib` static_library with `override_options: ['cpp_std=' + libvmaf_cpu_cpp_std]`, mirroring the ADR-0708 `metadata_handler_cpp20_lib` pattern so the C++23 override does not leak to other TUs. - Removing log.c from libvmaf_sources; adding `log_cpp23_lib` objects to the libvmaf library() target. - Removing 21 inline `'../src/log.c'` source entries from test executables in core/test/meson.build; adding a shared `log_cpp23_test_objects` aggregate that those tests pick up via objects:. - Wiring the orphan `test_log` executable into the fast suite (mirrors the orphan-test sweep in draft PR #315) so the formatter / log-level filtering coverage is no longer silently dead. Behaviour is byte-identical to the prior C build: same fprintf format string, same va_list pass-through, same stderr destination, same ANSI colour codes. std::print / std::format were considered but rejected to preserve printf-style varargs at the libvmaf log surface. Build + test: meson setup build-cpu core -Denable_cuda=false -Denable_sycl=false \ -Denable_hip=false ninja -C build-cpu # 726 targets, all link clean meson test -C build-cpu --suite=fast # 50/50 pass, incl. new test_log ABI verification (nm libvmaf.so | grep vmaf_log): master: 0000000000000000 t vmaf_log (LTO-hidden) 0000000000000000 t vmaf_log.constprop.0 (LTO-hidden) this PR: 0000000000000000 T vmaf_log (externally visible) 0000000000000000 T vmaf_set_log_level (externally visible) The promotion from `t` (local) to `T` (global) is a consequence of moving the TU into an isolated static_library — the linker can no longer elide the symbol via LTO. This matches the established metadata_handler_cpp20_lib precedent (vmaf_metadata_* are also `T` in master). Neither symbol is marked VMAF_EXPORT, so the public libvmaf ABI is unchanged. **Research digest**: ADR-0708 (C++23 internals pilot policy) and the Wave 1 backlog rationale in ADR-0727 §Context. The log.c migration recipe is the same as metadata_handler (extern "C", isolated static_lib, cpp_std override) — no separate research digest needed. **Decision matrix**: log.c was chosen as the next pilot over mem.c, opt.c, and fex_ctx_vector.c because (1) its C surface is the smallest of the four (72 lines, two functions, no out-params), (2) it has zero callers in the public ABI (vmaf_log / vmaf_set_log_level are internal diagnostics, not VMAF_EXPORT-marked), and (3) the log.cpp body already existed in tree from a prior aborted Wave 1 attempt — adopting it is strictly cheaper than starting fresh from mem.c. See ADR-0708 `## Alternatives considered` for the original ROI ranking. **AGENTS.md invariant note**: no rebase-sensitive invariants — the log function semantics, output format, and stderr destination are unchanged. The rebase-mapping (upstream `libvmaf/src/log.c` → fork `core/src/log.cpp`) is recorded in docs/rebase-notes.md so future port-upstream-commit runs target the right file. **Reproducer / smoke-test command**: meson setup build-cpu core -Denable_cuda=false -Denable_sycl=false \ -Denable_hip=false ninja -C build-cpu meson test -C build-cpu test_log --print-errorlogs nm build-cpu/src/libvmaf.so | grep -E " T vmaf_log| T vmaf_set_log_level" **CHANGELOG fragment**: changelog.d/changed/log-c-to-cpp23.md **Rebase note**: docs/rebase-notes.md — section "log.c → log.cpp C++23 pilot (ADR-0708 Wave 1, 2026-05-30)". Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor(core): convert gpu_dispatch_env.c → C++23 (ADR-0858) Replace the platform-#ifdef pthread_mutex_t/CRITICAL_SECTION lock bootstrap with std::mutex + std::lock_guard RAII; replace strdup + nullable char * with std::optional<std::string>; use std::string_view for the fast-path comparisons; add [[nodiscard]] to the public entry point. The Windows-specific InitOnceExecuteOnce / CRITICAL_SECTION branch (~25 LOC) is eliminated entirely. Compiled as an isolated gpu_dispatch_env_cpp23_lib static library with override_options=['cpp_std=c++23'], following the ADR-0708 pattern for metadata_handler_cpp20_lib. Public extern "C" symbol unchanged; all GPU backends link without modification. Fast-test gate: 49/49 pass (CPU-only build). Six deliverables (ADR-0108): D1: no digest needed: trivial mechanical conversion D2: ADR-0858 §Alternatives considered D3: core/AGENTS.md — added invariant #7 for isolated lib pattern D4: reproducer: meson setup core/build -Denable_cuda=false && ninja D5: changelog.d/changed/adr-0858-cpp23-gpu-dispatch-env.md D6: docs/rebase-notes.md — feat/cpp23-gpu-dispatch-env-20260529 entry Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(core): cpp23 Wave 8 — opt.cpp activation + read_json_model.cpp (ADR-0761) Activate opt.cpp (completing Wave 1 / ADR-0721) and convert read_json_model.c → read_json_model.cpp (new, 639 LOC). Both compile under cpp_std=c++23 in isolated static libraries (opt_cpp23_lib / read_json_model_cpp23_lib) linked into libvmaf.so via extract_all_objects(). Public C ABI unchanged. Changes per file: - opt.h: extern "C" guards added; opt_cpp23_lib wired into meson - read_json_model.cpp: nullptr, static_cast<>, [[nodiscard]] on 4 entry points; goto-past-init replaced with if/else scoping; C++ <c*> headers - log.h, model.h, read_json_model.h: extern "C" guards added - core/src/meson.build: opt_cpp23_lib + read_json_model_cpp23_lib static libs; opt.c and read_json_model.c comments in libvmaf_sources - core/test/meson.build: wave8_cpp23_objects variable; all affected test executables updated to use objects instead of direct source compilation - scripts/ci/coverage-check.sh: updated opt.c -> opt.cpp, read_json_model.c -> read_json_model.cpp in critical-coverage exclusion list - core/src/AGENTS.md: invariant 10 documenting the active .cpp files predict.c skipped: depends on feature_extractor.h / feature_collector.h without extern "C" guards; deferred to a later wave. Build: 49/49 fast-suite pass (CPU-only). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(core): rename feature_extractor.c → .cpp (ADR-0772) Six void* cast fixups, one atomic_load arithmetic guard, placement-new pool slot init (avoids copying std::atomic), extern "C" guards on feature_extractor.h / log.h / opt.h, and extern "C" on opt.cpp definition. meson.build refs updated; old .c deleted via git rm. Fast-test gate: 49/49 PASS (CPU-only build-0772). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(tools)!: C++23 Wave 8 — cli_parse.c + vmaf.c → .cpp (ADR-0809) Convert core/tools/cli_parse.c and core/tools/vmaf.c to C++23 translation units. Conservative idioms only: nullptr, static_cast, [[nodiscard]], [[noreturn]], std::string_view for option-string dispatch, and a ModelArrays RAII struct that replaces the three manual vmaf_model_destroy/free loops in vmaf.cpp's goto-cleanup block. - cli_parse.h: extern "C" guards for C caller compatibility - spinner.h: static internal linkage on spinner[]/spinner_length - meson.build: cpp files + cpp_args + override_options cpp_std=c++23 - .pre-commit-config.yaml: add --disable-version-check to semgrep entry to fix spurious exit-2 from semgrep 1.159.0 version-check notification Build: CPU-only build in container — 724 targets, zero errors. Smoke: vmaf --help output identical; Netflix golden pair scores 76.66783 (ADR-0214 PASS; places=4 >= 76.668). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * chore(bundle): add CHANGELOG fragment for C++23 wave bundle PR Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(ci): remove pre-existing conflict-marker residuals from workflow files Conflicts in .github/workflows/libvmaf-build-matrix.yml (2 blocks) and .github/workflows/security-scans.yml (1 block) were committed into the PR branch prior to this rebase. They produced blank lines or garbled YAML steps that would fail CI. Remove all conflict markers and restore the correct content matching the master baseline. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: Lusoris <lusoris@pm.me>
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
opt.cpp(Wave 1 completion / ADR-0721):opt_cpp23_libisolated static lib wired into meson;opt.cremoved from build;extern "C"guards added toopt.hread_json_model.c→read_json_model.cpp(ADR-0761, 639 LOC):nullptr,static_cast<>,[[nodiscard]]on 4 entry points,goto-past-init removed;extern "C"guards added tolog.h,model.h,read_json_model.hwave8_cpp23_objectspattern incore/test/meson.buildreplaces direct.cppsource compilation in test executablesNote:
predict.cwas evaluated but skipped — it depends onfeature_extractor.h/feature_collector.hwhich lackextern "C"guards (deferred to a later wave).Test plan
meson test -C build --suite=fast— 49/49 pass (CPU-only, confirmed locally)test_opt,test_model,test_predictlink and pass with wave8_cpp23_objectsDeep-dive deliverables checklist
## Alternatives consideredcore/src/AGENTS.md§10 addedmeson test -C build --suite=fast(49/49 pass)changelog.d/changed/cpp23-wave8-opt-read-json-model.mddocs/rebase-notes.md— ADR-0761 entry addedState.md
no state.md update needed: pure internal refactor, no bug opened or closed.
ffmpeg-patches
no ffmpeg-patches update needed: no public C API surface changed.
🤖 Generated with Claude Code