refactor(core): cpp23 read_json_model.c → .cpp (ADR-0846) by lusoris · Pull Request #215 · VMAFx/vmafx

lusoris · 2026-05-29T15:38:45Z

Summary

Converts core/src/read_json_model.c to read_json_model.cpp (C++23, ADR-0846 Wave 8)
goto fail: teardown replaced with scoped ModelParseGuard RAII class
malloc/free for cfg_name replaced with std::make_unique<char[]>
strdup/free for feature-option key replaced with std::string
[[nodiscard]] on capacity helpers; static_cast<> replaces C-style casts
No behaviour change; public C ABI unchanged; all 49 fast-suite tests pass

Deliverables checklist (ADR-0108)

Research digest — no digest needed: trivial Wave-pattern refactor
Decision matrix — docs/adr/0846-cpp23-wave8-read-json-model.md §Alternatives considered
AGENTS.md invariant note — core/AGENTS.md JSON loader TU note updated
Reproducer / smoke-test command — meson test -C build --suite=fast → 49/49 OK
CHANGELOG fragment — changelog.d/changed/0846-cpp23-read-json-model.md
Rebase note — docs/rebase-notes.md §C++23 Wave 8 entry

State.md

no state.md update needed: no bug opened, closed, or ruled not-affecting-fork

ffmpeg-patches

no ffmpeg-patches update needed: pure internal refactor, no public C API surface changed

Test plan

meson test -C build -Denable_cuda=false --suite=fast — 49/49 OK (verified locally)
CI Required Checks Aggregator green

🤖 Generated with Claude Code

Wave 8 of the C→C++23 migration series. `read_json_model.cpp` is compiled as an isolated static lib (`read_json_model_cpp23_lib`, `cpp_std=c++23`) following the ADR-0708 / ADR-0755 pattern. The four public entry points retain their original C signatures (`extern "C"`). C++23 improvements: - `goto fail:` teardown in `vmaf_read_json_model` replaced with RAII `ModelParseGuard` that calls `vmaf_model_destroy` + nulls `*model` on any early exit. - `malloc(cfg_name_sz)` + naked `free` in `model_collection_parse` replaced with `std::make_unique<char[]>`. - `strdup(key)` + `free(key)` in `parse_feature_opts_object` replaced with `std::string` (RAII). - `[[nodiscard]]` on `grow_count` and `ensure_*_capacity`. - `static_cast<>` replaces C-style casts; `nullptr` replaces `NULL`. - `uint64_t cfg->flags` → `enum VmafModelFlags` explicit cast. No behaviour change. All 49 fast-suite tests pass. Deliverables (ADR-0108): 1. research digest — no digest needed: trivial Wave-pattern refactor 2. ADR alternatives — docs/adr/0846-cpp23-wave8-read-json-model.md 3. AGENTS.md invariant — core/AGENTS.md (JSON loader TU note updated) 4. reproducer — `meson test -C build --suite=fast` 5. changelog — changelog.d/changed/0846-cpp23-read-json-model.md 6. rebase-notes — docs/rebase-notes.md §Wave 8 entry Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

lusoris · 2026-05-30T07:00:05Z

Cascading rebase conflicts. Recreated clean in next PR.

Two Open rows in docs/state.md cited PRs that were CLOSED-not-merged and flagged as follow-up by the PR #291 closing agent. Verified against master tip bbcaa8d and master state of the underlying issues: 1. T-CUDA-FILTER1D-RES-DISPATCH-CONFLICT-2026-05-29 — migrated from Open to Recently closed (superseded). The conflict markers only existed on the unmerged scaffold branch tip 35a1fb6. PR #91 (merged 2026-05-29T09:37:48Z) landed ADR-0753 resolution-aware dispatch via the adm_cm_device() consumer without extending dispatch into filter1d_8(), so master never carried the build-failing scaffold variant. PR #214 (the planned conflict-marker cleanup) was CLOSED-not-merged 2026-05-30 when its base scaffold branch was abandoned. core/src/feature/cuda/ integer_vif_cuda.c::filter1d_8() on master uses the clean unconditional cuLaunchKernel paths. 2. T-CPP23-READ-JSON-MODEL-PENDING-2026-05-29 — kept Open but the dead PR #215 citation removed. The C++23 Wave 8 conversion of core/src/read_json_model.c is still pending on master (still a .c source per core/src/meson.build:1578); PR #215 was CLOSED-not-merged 2026-05-30. Owner field rewritten to "Owner-driven; pending fresh PR per ADR-0846 Wave 8". Scope-coordinated with DRAFT PR #291 (docs/state-md-drift-sync — already migrates the 3 Vulkan rows + T-LEGACY-RUNNER-ANSNR-BROKEN + T-LEGACY-RUNNER-STUB-MISSING). Row 203 (T-LEGACY-RUNNER-STUB-MISSING-2026-05-29) cites closed PRs #213 and #181 as OPEN but is left untouched here since PR #291 already rewrites it. Net Open count -1; total T-row count unchanged (153). No code changes — documentation cleanup only. Deliverables (ADR-0108): - no digest needed: state.md hygiene - no alternatives: only-one-way reconciliation - no rebase-sensitive invariants - Reproducer: `gh pr view 214 -R VMAFx/vmafx --json state,mergedAt` returns `{"state":"CLOSED","mergedAt":null}`; `grep -nE '^(<{7}|={7}|>{7})( |$)' core/src/feature/cuda/integer_vif_cuda.c` on master returns empty; `ls core/src/read_json_model.*` returns only `.c` and `.h`. - Changelog: changelog.d/changed/state-md-closed-pr-row-sweep.md - no rebase impact: docs only Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

…overage + #336 state.md row sweep + #383 README badges + #337 ADR-0865 ANSNR) (#529) * docs(libvmaf): doxygen comments on 15 undocumented public C-API entry points Round-2 follow-on to PR #302 (which closed five targeted gap-findings in libvmaf.h / picture.h / dnn.h). This pass covers the public surfaces that PR #302 left untouched, focusing on the headers the ffmpeg patch stack, the upcoming Go/Rust bindings, and the embedded MCP server consume. Entry points documented: - feature.h (file was 100 % undocumented): - VmafFeatureDictionary (struct doc + ownership-transfer rules) - vmaf_feature_dictionary_set - vmaf_feature_dictionary_free - model.h: - VmafModelFlags (enum + per-flag semantics) - VmafModelConfig (struct + per-field doc) - vmaf_model_load - vmaf_model_load_from_path - vmaf_model_feature_overload (incl. opts_dict ownership transfer) - vmaf_model_destroy (incl. do-not-destroy-after-collection-handoff) - VmafModelCollection (struct doc) - VmafModelCollectionScoreType (enum doc) - VmafModelCollectionScore (struct + per-field doc) - vmaf_model_collection_load - vmaf_model_collection_load_from_path - vmaf_model_collection_feature_overload - vmaf_model_collection_destroy - dnn.h: - vmaf_dnn_session_close (pair-with-open contract) Each block documents the negative-errno return convention, NULL-safety, ownership-transfer semantics, and the destroy-pairing required to avoid double-free of collection-owned sub-models. No semantic / no ABI change. Cleanup pass (CLAUDE.md §12 r12 — touched-file lint-clean rule): the three Netflix-copyright include guards (__VMAF_FEATURE_H__ / __VMAF_MODEL_H__ / __VMAF_DNN_H__) trip clang-tidy's bugprone-reserved-identifier check. Renaming them would diverge from Netflix/vmaf master and break port-only upstream sync (CLAUDE.md §10), so each #ifndef / #define gets an inline NOLINT citing the upstream-mirror invariant — the exact pattern ADR-0278 endorses for load-bearing upstream-parity identifiers. Build + lint: - meson setup build-cpu-doc core -Denable_cuda=false -Denable_sycl=false - ninja -C build-cpu-doc (35 targets touched by header change; clean) - clang-tidy -p build-cpu-doc on all 3 touched headers: 0 fork-local warnings (the 3 reserved-identifier warnings present on master are now NOLINT-cited; remaining warnings are in system headers and suppressed). - pre-commit run --files <all 5 touched files>: green. ADR-0108 deliverables: - Research digest: no digest needed — trivial doc-only addition over Netflix-stable signatures already covered by the existing reference manual. - Decision matrix: no alternatives needed — only-one-way fix; the ownership-transfer text matches the implementation in core/src/model.c and core/src/dict.c verbatim. - AGENTS.md invariant note: no rebase-sensitive invariants — the doc text sits above unchanged upstream signatures; future merges from Netflix produce tractable 3-way merges. The NOLINT cites name the invariant they preserve (upstream-mirror include guards). - Reproducer / smoke test: see PR body. - Changelog fragment: changelog.d/added/libvmaf-public-header-doc-comments-round2.md. - Rebase notes: docs/rebase-notes.md updated with a dedicated section. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * test(go): expand cmd/vmafx-{controller,server,mcp} coverage Add unit tests for the lowest-coverage Go cmd/ subpackages identified by the master-tip workflow audit (Section F). No behavior change. Coverage deltas (against origin/master tip bbcaa8d): cmd/vmafx-controller 18.6% -> 32.4% (+13.8 pp) cmd/vmafx-controller/nodes 80.7% -> 82.5% (+1.8 pp) cmd/vmafx-server 27.5% -> 47.9% (+20.4 pp) cmd/vmafx-mcp 3.5% -> 24.6% (+21.1 pp) New test files: - cmd/vmafx-controller/main_extra_test.go 405 method-not-allowed on /healthz, /readyz, /v1/score; 400 invalid-JSON body; 500 scorer-error mapping via a stub vmaf binary; runHTTP graceful shutdown bounded by GracefulShutdownTimeout; envOr default+override; version(). - cmd/vmafx-controller/nodes/registry_edge_test.go Get(unknown), distinct-IDs-for-same-name contract pin, Heartbeat updates JobsRunning + advances LastHeartbeat (reaper eviction predicate), concurrent Register/Heartbeat under -race, defensive-copy assertion on All(). - cmd/vmafx-server/main_extra_test.go Same shape as the controller HTTP server tests; pins the PR #300 bounded-timeout shutdown invariant. - cmd/vmafx-mcp/impl_test.go All arg helpers (strArg, intArg, floatArg, boolArg, hasArg); every pure helper (classifySourceResolution, modelResolutionClass, resolutionMismatchWarning, inferBackendFromPayload, inferBackendFromSym, stripModelExt, toFFmpegPixfmt, pickWorstFrames, floatFromAny, roundF, truncate); representative handler error paths (handleProbeBackend missing/unknown backend, handleDescribeModel missing name, handleVmafScore invalid path / zero dimensions, handleCompareModels empty list). Pins the errorResult().IsError == true invariant from project memory (MCP isError must be True so clients branch correctly). Drive-by fix: .gitignore anchored the Go binary-ignore rules with a leading slash so they only match the repo-root binaries, not any path component sharing the name. Without this, untracked files like cmd/vmafx-server/main_extra_test.go were silently ignored by `git add`. Verification: go test -race -cover ./cmd/vmafx-controller/... \ ./cmd/vmafx-server/... \ ./cmd/vmafx-mcp/... go vet ./... All green; no behavior change. The pre-existing cmd/vmafx-operator/internal/controller failure (kubebuilder envtest needs etcd binaries on PATH) is unrelated to this change and reproduces on a clean origin/master checkout. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(state): reconcile 2 Open rows that cited CLOSED PRs (#214, #215) Two Open rows in docs/state.md cited PRs that were CLOSED-not-merged and flagged as follow-up by the PR #291 closing agent. Verified against master tip bbcaa8d and master state of the underlying issues: 1. T-CUDA-FILTER1D-RES-DISPATCH-CONFLICT-2026-05-29 — migrated from Open to Recently closed (superseded). The conflict markers only existed on the unmerged scaffold branch tip 35a1fb6. PR #91 (merged 2026-05-29T09:37:48Z) landed ADR-0753 resolution-aware dispatch via the adm_cm_device() consumer without extending dispatch into filter1d_8(), so master never carried the build-failing scaffold variant. PR #214 (the planned conflict-marker cleanup) was CLOSED-not-merged 2026-05-30 when its base scaffold branch was abandoned. core/src/feature/cuda/ integer_vif_cuda.c::filter1d_8() on master uses the clean unconditional cuLaunchKernel paths. 2. T-CPP23-READ-JSON-MODEL-PENDING-2026-05-29 — kept Open but the dead PR #215 citation removed. The C++23 Wave 8 conversion of core/src/read_json_model.c is still pending on master (still a .c source per core/src/meson.build:1578); PR #215 was CLOSED-not-merged 2026-05-30. Owner field rewritten to "Owner-driven; pending fresh PR per ADR-0846 Wave 8". Scope-coordinated with DRAFT PR #291 (docs/state-md-drift-sync — already migrates the 3 Vulkan rows + T-LEGACY-RUNNER-ANSNR-BROKEN + T-LEGACY-RUNNER-STUB-MISSING). Row 203 (T-LEGACY-RUNNER-STUB-MISSING-2026-05-29) cites closed PRs #213 and #181 as OPEN but is left untouched here since PR #291 already rewrites it. Net Open count -1; total T-row count unchanged (153). No code changes — documentation cleanup only. Deliverables (ADR-0108): - no digest needed: state.md hygiene - no alternatives: only-one-way reconciliation - no rebase-sensitive invariants - Reproducer: `gh pr view 214 -R VMAFx/vmafx --json state,mergedAt` returns `{"state":"CLOSED","mergedAt":null}`; `grep -nE '^(<{7}|={7}|>{7})( |$)' core/src/feature/cuda/integer_vif_cuda.c` on master returns empty; `ls core/src/read_json_model.*` returns only `.c` and `.h`. - Changelog: changelog.d/changed/state-md-closed-pr-row-sweep.md - no rebase impact: docs only Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore(meta): README badge audit + Cargo / pyproject repo-metadata sweep Audit + backfill the fork's repo-metadata surface so cargo / pip / GitHub all advertise the canonical VMAFx/vmafx URLs: - README.md: add Rust CI + Go CI workflow badges (both workflows ship on master but were not surfaced). All five pre-existing workflow badges already point at VMAFx/vmafx and reference real, active, master-green workflows; verified via the Actions API. License, Conventional Commits, OpenSSF Scorecard, ko-fi badges already present. - Cargo.toml: add [workspace.package] with repository / homepage / documentation / license / authors. Both workspace members (bindings/rust/vmafx-sys, core/src/feature/rust/tad) switched to workspace-inherited metadata so URL drift is impossible across the Rust workspace. cargo metadata confirms both crates now expose the VMAFx/vmafx URLs. - pyproject.toml (root, ai/, tools/vmaf-tune, tools/vmaf-roi-score, tools/ensemble-training-kit, dev-llm/, mcp-server/vmaf-mcp/): add [project.urls] with Homepage / Repository / Documentation / Issues / Changelog. All seven fork-authored projects now ship the same URL block; the package indexes (PyPI / internal) get a consistent repository link. - deploy/helm/vmafx/Chart.yaml: already correct (home + sources already point at VMAFx/vmafx). No change needed. - changelog.d/fixed/ + docs/rebase-notes.md: deliverables. Coordination with PR #331 (rebrand sweep): #331 only touches the line-1 copyright header of two of the seven pyproject files; this PR adds a new [project.urls] block below — no merge conflict. Deep-dive deliverables (ADR-0108): - Research digest: no digest needed: trivial repo-metadata sweep. - Decision matrix: no alternatives: only-one-way fix (URLs must match the rebrand target). - AGENTS.md invariant: no rebase-sensitive invariants — fork-only metadata files, none mirror upstream Netflix. - Reproducer: `cargo metadata --no-deps --format-version 1 | jq` + `python3 -c "import tomllib; tomllib.loads(open('pyproject.toml','rb').read().decode())"`. - CHANGELOG fragment: changelog.d/fixed/readme-badges-metadata-audit.md. - Rebase note: added to docs/rebase-notes.md (impact: none, fork-only). * docs(adr): author ADR-0865 for ANSNR sunset (closes PR #38 ADR-0108 gap) PR #38 (merged 2026-05-28) removed `float_ansnr` from the C backend but cited `Parent ADR-0709` in its body — ADR-0709 is the Phase 4b distributed-platform umbrella and contains zero ANSNR content. PR #295 + PR #324 inherited the bad cite. No dedicated ANSNR-sunset ADR existed in tree. This change: - Authors `docs/adr/0865-ansnr-sunset-pre-vmaf-metric-drop.md` as the missing parent ADR, back-dated to 2026-05-28 (PR #38 merge date) so the dependency chain (ADR-0865 -> PR #38 -> ADR-0749 Python sunset) is consistent. - Documents the historical mis-cite in the new ADR's `## Notes` section so future readers landing on PR #38 can recover the trail. PR bodies on the remote are immutable merge-history and cannot be rewritten. - Adds the index fragment + `_order.txt` row; regenerates `docs/adr/README.md` via `scripts/docs/concat-adr-index.sh`. - Adds `docs/state.md` row (Updated note + Recently-closed entry). - Adds `docs/rebase-notes.md` entry documenting the rebase invariant (upstream still ships `ansnr` extractors; fork must keep deleting). - Adds `changelog.d/changed/ansnr-sunset-adr-authoring.md` fragment. In-tree audit confirmed zero `ADR-0709` references mis-cite ANSNR — all remaining tree-side `ADR-0709` cites correctly point at Phase 4b distributed-platform content. No tree-side citation fix-up required. Docs-only PR. No code changes. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore(bundle): add changelog fragment for doc-sweep bundle batch-1 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>

…06 batch (#719) Refreshes bug-tracking and planning documents after the 18-PR June 5-6 batch (#691–#711). Changes in docs/state.md (tracked): - Add batch-sweep _Updated header summarising PRs #691–#711. - Add T-NEON-FMA-FLOAT-ADM-DWT2-2026-06-06 to Open bugs table (was only in the _Updated header, missing from the table body). - Remove duplicate T-CPP23-READ-JSON-MODEL-PENDING-2026-05-29 row (stale copy that cited closed PR #215 as OPEN; the de-cited version is kept). - Fix concatenated duplicate T-GPU-COVERAGE-STABLE-WEEKS rows in Recently closed (two entries were joined on a single line without a separator). - Add 8 new Recently closed rows for substantive June 5-6 fixes: T-NEON-FMA-FLOAT-ADM-DWT2-REVERT (PR #695 revert), T-METAL-FEATURE-COLLECTOR-EXTERN-C (PR #694), T-SYCL-DICT-INCLUDE-MISSING (PR #696), T-INTEGER-SSIM-I686-INCLUDE (PR #700), T-WAVE8-OBJ-TARGET-DEPS (PRs #699/#701), T-DNN-INT8-TEST-ADR1032-ALIGN (PR #705), T-MCP-SMOKE-11-FAILURES (PR #706). OPEN.md and BACKLOG.md (gitignored, updated in main tree only): - Header dates updated to 2026-06-06. - SIMD divergence cluster marked CLOSED (PR #681, 2026-06-04). - T-NEON-FMA-FLOAT-ADM-DWT2 added to Active right now. - June 5-6 PR table added to Recently completed sections. - Awaiting user decision updated: SIMD divergence closed, NEON FMA rewire new. no rebase impact: docs/state.md only; no C sources touched. Co-authored-by: Lusoris <lusoris@pm.me> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

Two Open rows in docs/state.md referred to bugs already in the Recently Closed section: - T-CUDA-FILTER1D-RES-DISPATCH-CONFLICT-2026-05-29 was closed by PR #680; the Open row citing "PR #214 (OPEN)" is removed. - T-CPP23-READ-JSON-MODEL-PENDING-2026-05-29 had a stale duplicate referencing "PR #215 (OPEN)"; the duplicate is removed. The correct owner-driven copy (PR #215 closed without merging, fresh PR required) is retained. T-DOC-LEGACY-RUNNER-MISSING-DEPRECATION-2026-05-29 updated: PR #216 was closed without merging 2026-05-30; the row now reflects that docs/development/deprecations.md exists (PR #605) but still lacks the VmafLegacyQualityRunner entry. Owner-driven. Also fixes a pre-existing data corruption: T-GPU-COVERAGE-STABLE-WEEKS appeared twice on one line in Recently Closed (no newline separator); corrected to a single properly-formatted row. Net Open change: -2 rows. no digest needed: trivial doc sweep no alternatives: only-one-way fix no rebase-sensitive invariants: docs/state.md only Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…#724) Two Open rows in docs/state.md referred to bugs already in the Recently Closed section: - T-CUDA-FILTER1D-RES-DISPATCH-CONFLICT-2026-05-29 was closed by PR #680; the Open row citing "PR #214 (OPEN)" is removed. - T-CPP23-READ-JSON-MODEL-PENDING-2026-05-29 had a stale duplicate referencing "PR #215 (OPEN)"; the duplicate is removed. The correct owner-driven copy (PR #215 closed without merging, fresh PR required) is retained. T-DOC-LEGACY-RUNNER-MISSING-DEPRECATION-2026-05-29 updated: PR #216 was closed without merging 2026-05-30; the row now reflects that docs/development/deprecations.md exists (PR #605) but still lacks the VmafLegacyQualityRunner entry. Owner-driven. Also fixes a pre-existing data corruption: T-GPU-COVERAGE-STABLE-WEEKS appeared twice on one line in Recently Closed (no newline separator); corrected to a single properly-formatted row. Net Open change: -2 rows. no digest needed: trivial doc sweep no alternatives: only-one-way fix no rebase-sensitive invariants: docs/state.md only Co-authored-by: Lusoris <lusoris@pm.me> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

lusoris mentioned this pull request May 29, 2026

docs(state): drift sync 2026-05-29 — remove 7 stale rows, add 7 in-flight PR rows #222

Closed

6 tasks done

lusoris closed this May 30, 2026

lusoris deleted the feat/cpp23-read-json-model-20260529 branch June 4, 2026 08:10

This was referenced Jun 6, 2026

chore(state): refresh state.md + OPEN.md + BACKLOG.md for 2026-06-05/06 batch #719

Merged

chore(state): remove 2 stale Open rows + fix T-GPU-COVERAGE duplicate #724

Merged

Navigation Menu

Repository navigation

refactor(core): cpp23 read_json_model.c → .cpp (ADR-0846) - #215

refactor(core): cpp23 read_json_model.c → .cpp (ADR-0846)#215
lusoris wants to merge 1 commit into
masterfrom
feat/cpp23-read-json-model-20260529

lusoris commented May 29, 2026 •

edited

Loading

Uh oh!

lusoris commented May 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

lusoris commented May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Deliverables checklist (ADR-0108)

State.md

ffmpeg-patches

Test plan

Uh oh!

lusoris commented May 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

lusoris commented May 29, 2026 •

edited

Loading