refactor(core): pilot C++23 conversion (metadata_handler) + migration research digest

changelog.d/changed/0708-cpp23-internals-pilot.md

@@ -0,0 +1,12 @@
+- **refactor(core):** Pilot C++20 conversion of `core/src/metadata_handler.c`
+  (renamed to `.cpp` via `git mv`). `vmaf_metadata_destroy` now uses a
+  `std::unique_ptr<VmafCallbackList>` with a custom `CallbackListDeleter` that
+  walks and frees the linked-list nodes — replaces the manual traversal loop
+  and guarantees teardown on any future early-return path. Public C API in
+  `metadata_handler.h` is unchanged; `extern "C"` guards added so C callers
+  (`feature_collector.c`) continue to compile and link without modification.
+  Compiled as an isolated static lib (`metadata_handler_cpp20_lib`) with
+  `override_options : ['cpp_std=c++20']`, matching the Vulkan VMA precedent;
+  project-wide `cpp_std=c++11` default is not affected. Includes Research-0732
+  migration plan and ROI-ranked candidate table for Wave 1–3 follow-up PRs.
+  Netflix golden gate: `76.6678` (places=4 pass). (ADR-0708)

core/src/AGENTS.md

@@ -104,3 +104,22 @@ temporary C numeric locale lifetime. Path-based `vmaf_write_output()` uses
 `fdopen()` and may otherwise leave the final flush to `fclose()` after the
 locale has been restored/freed; that is the macOS-only SIGSEGV shape for
 `test_output` and `test_public_api_score`.
+
+### 9. `metadata_handler.cpp` — C++20 pilot; extern "C" guard must not be removed (ADR-0708)
+
+`core/src/metadata_handler.cpp` (previously `metadata_handler.c`) is the first
+C++20 internal implementation TU. `metadata_handler.h` carries `extern "C"`
+guards that allow `feature_collector.c` (a plain C file) to include the header
+and call the three functions without a link-name-mangling mismatch.
+
+Do not:
+- Remove the `extern "C"` guards from `metadata_handler.h`.
+- Rename the three public symbols (`vmaf_metadata_init`, `vmaf_metadata_append`,
+  `vmaf_metadata_destroy`).
+- Move the file back to `.c` — `unique_ptr` and `CallbackListDeleter` require
+  a C++ compiler.
+
+When porting an upstream Netflix/vmaf commit that modifies the original
+`libvmaf/src/metadata_handler.c`, apply the diff content to
+`core/src/metadata_handler.cpp` (C code is valid C++; the `extern "C"` block
+in the header stays). Run `make test-netflix-golden` post-port.

core/src/meson.build

@@ -1469,6 +1469,20 @@ libsvm_static_lib = static_library(
     cpp_args : libsvm_cpp_args,
 )
 
+# ADR-0708: metadata_handler.cpp is the first C++20 pilot TU. Compiled as a
+# separate static lib so its cpp_std override (c++20) does not propagate to
+# any other C++ TU in libvmaf — same isolation pattern used by vma_impl_lib
+# in core/src/vulkan/meson.build. Linked into the final libvmaf via
+# link_with below.
+metadata_handler_cpp20_lib = static_library(
+    'metadata_handler_cpp20',
+    src_dir + 'metadata_handler.cpp',
+    include_directories : [vmaf_base_include, libvmaf_include],
+    override_options : ['cpp_std=c++20'],
+    pic : true,
+    install : false,
+)
+
 libvmaf_sources = [
     src_dir + 'libvmaf.c',
     src_dir + 'predict.c',
@@ -1485,7 +1499,7 @@ libvmaf_sources = [
     src_dir + 'pdjson.c',
     src_dir + 'log.c',
     src_dir + 'framesync.c',
-    src_dir + 'metadata_handler.c',
+    # metadata_handler.cpp is compiled as metadata_handler_cpp20_lib (ADR-0708)
     src_dir + 'picture_pool.c',
     src_dir + 'thread_locale.c',
     src_dir + 'gpu_picture_pool.c',
@@ -1590,6 +1604,8 @@ libvmaf = library(
         libvmaf_feature_static_lib.extract_all_objects(recursive: true),
         libvmaf_cpu_static_lib.extract_all_objects(recursive: true),
         libsvm_static_lib.extract_all_objects(recursive: true),
+        # ADR-0708: C++20 pilot TU compiled as isolated static lib.
+        metadata_handler_cpp20_lib.extract_all_objects(recursive: true),
     ],
     version : vmaf_soname_version,
     soversion : vmaf_soversion,

core/src/metadata_handler.cpp

@@ -0,0 +1,132 @@
+/**
+ *
+ *  Copyright 2016-2026 Netflix, Inc.
+ *
+ *     Licensed under the BSD+Patent License (the "License");
+ *     you may not use this file except in compliance with the License.
+ *     You may obtain a copy of the License at
+ *
+ *         https://opensource.org/licenses/BSDplusPatent
+ *
+ *     Unless required by applicable law or agreed to in writing, software
+ *     distributed under the License is distributed on an "AS IS" BASIS,
+ *     WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *     See the License for the specific language governing permissions and
+ *     limitations under the License.
+ *
+ */
+
+/*
+ * C++20 implementation of the callback-list metadata handler.
+ *
+ * Migration note (ADR-0708 / Research-0732):
+ *   - `git mv metadata_handler.c metadata_handler.cpp` preserves blame.
+ *   - The public C API in metadata_handler.h is unchanged; all three
+ *     functions retain their original C signatures and are declared
+ *     `extern "C"` in the header, so every C caller (feature_collector.c)
+ *     links without modification.
+ *   - `std::unique_ptr<VmafCallbackList>` RAII-manages the list head,
+ *     replacing the manual `free(metadata)` tail-call in
+ *     `vmaf_metadata_destroy` with guaranteed cleanup on any exit path.
+ *   - The per-node teardown loop is encapsulated in `NodeDeleter`, which
+ *     means a future early-return inside `vmaf_metadata_destroy` cannot
+ *     leak nodes — the deleter always runs when the unique_ptr goes out
+ *     of scope.
+ *   - `nullptr` replaces `NULL` throughout this file (no change to the
+ *     struct fields, which are C-linkage and keep `NULL` in callers).
+ */
+
+#include <cerrno>
+#include <cstdlib>
+#include <cstring>
+#include <memory>
+
+#include "metadata_handler.h"
+
+/* RAII deleter for the VmafCallbackItem linked list.
+ * Walks and frees every node before the list head itself is released by
+ * the unique_ptr machinery.  This eliminates the manual traversal loop
+ * that was previously in vmaf_metadata_destroy and ensures correctness
+ * even if additional early-return paths are added in the future. */
+struct CallbackListDeleter {
+    void operator()(VmafCallbackList *list) const noexcept
+    {
+        if (!list)
+            return;
+        VmafCallbackItem *node = list->head;
+        while (node) {
+            VmafCallbackItem *next = node->next;
+            /* free() on a VmafCallbackItem that carries no heap-owned
+             * sub-fields (metadata_cfg, callback, data are all value or
+             * borrowed-pointer types — callers own those lifetimes). */
+            free(node); // NOLINT(cppcoreguidelines-no-malloc) — C ABI node
+            node = next;
+        }
+        free(list); // NOLINT(cppcoreguidelines-no-malloc) — C ABI struct
+    }
+};
+
+/* Convenience alias used only within this translation unit. */
+using CallbackListPtr = std::unique_ptr<VmafCallbackList, CallbackListDeleter>;
+
+int vmaf_metadata_init(VmafCallbackList **const metadata)
+{
+    if (!metadata)
+        return -EINVAL;
+
+    auto *list = static_cast<VmafCallbackList *>(malloc(sizeof(VmafCallbackList)));
+    if (!list)
+        return -ENOMEM;
+
+    list->head = nullptr;
+    *metadata = list;
+    return 0;
+}
+
+int vmaf_metadata_append(VmafCallbackList *metadata, const VmafMetadataConfiguration metadata_cfg)
+{
+    if (!metadata)
+        return -EINVAL;
+
+    auto *node = static_cast<VmafCallbackItem *>(malloc(sizeof(VmafCallbackItem)));
+    if (!node)
+        return -ENOMEM;
+
+    /* Zero-initialise before field assignment so that the `callback` and
+     * `data` fields (not set by this function) start at NULL/nullptr rather
+     * than carrying stack garbage.  memset is safe here: VmafCallbackItem
+     * is a C struct with no virtual functions or non-trivial members. */
+    memset(node, 0, sizeof(*node));
+    node->metadata_cfg = metadata_cfg;
+    node->next = nullptr;
+
+    /* Append at tail so callbacks fire in registration order. */
+    if (!metadata->head) {
+        metadata->head = node;
+    } else {
+        VmafCallbackItem *iter = metadata->head;
+        while (iter->next)
+            iter = iter->next;
+        iter->next = node;
+    }
+
+    return 0;
+}
+
+int vmaf_metadata_destroy(VmafCallbackList *metadata)
+{
+    if (!metadata)
+        return -EINVAL;
+
+    /* Adopt the raw C pointer into a unique_ptr with our custom deleter.
+     * When `guard` goes out of scope at the end of this function (including
+     * on any future early-return path), CallbackListDeleter::operator()
+     * walks and frees every node, then frees the list struct itself.
+     *
+     * No behaviour change vs the original C implementation on the happy
+     * path; the improvement is the guaranteed teardown on any new
+     * early-return that might be added during future maintenance. */
+    CallbackListPtr guard(metadata);
+    (void)guard; /* explicit acknowledgement that we want the side-effect */
+    return 0;
+}

core/src/metadata_handler.h

@@ -21,6 +21,10 @@
 
 #include "metadata.h"
 
+#ifdef __cplusplus
+extern "C" {
+#endif
+
 typedef struct VmafCallbackItem {
     VmafMetadataConfiguration metadata_cfg;
     void (*callback)(void *, VmafMetadata *);
@@ -38,4 +42,8 @@ int vmaf_metadata_append(VmafCallbackList *metadata, const VmafMetadataConfigura
 
 int vmaf_metadata_destroy(VmafCallbackList *metadata);
 
-#endif // !__VMAF_PROPAGATE_METADATA_H__
+#ifdef __cplusplus
+} /* extern "C" */
+#endif
+
+#endif /* __VMAF_SRC_PROPAGATE_METADATA_H__ */