From a5b3d0f2108b99d570c41299a3c94c9d43b1cc57 Mon Sep 17 00:00:00 2001 From: Chris Thompson Date: Tue, 15 Sep 2026 16:35:56 -0600 Subject: [PATCH] capi: list-valued request options MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The model spec has declared `string_list` / `float_list` / `path_list` / `audio_path_list` option types since schema v1, and the schema validates their declarations — but nothing carries one at run time. Options are string -> string from the ABI down to TaskRequest::options, so a family wanting a list had no transport, and no spec has ever used the types. This adds the missing half. audiocpp_request_set_option_array(request, key, values, count) Values are copied, a second call with the same key REPLACES the list (matching set_option's assignment semantics), count 0 sets an empty list — distinct from never setting the key — and a null element is rejected before anything is written, so a bad element cannot leave the option half-assigned. Lists live in their own map, TaskRequest::option_arrays, so a family reading single-valued options cannot see a half-formed list and vice versa; which one a family reads is declared by the type in its spec. validate_spec_backed_request_options gains an overload that checks array keys against the same contract, so an undeclared list key is rejected exactly as an undeclared scalar one is. Every existing call site is untouched. ⚠ build_preparation_request carries the new map too. Preparation decides how graphs are sized, and a family whose work depends on a list option would otherwise prepare itself against a different request than the one run() is handed — and worse, fall back to whatever it does when the option is absent. ABI minor 1 -> 2, and the minor now MEANS that. It has never moved, including across #544, which added four entry points; a three-field version where two fields never change cannot answer the question a caller has. Minor increments when entry points are added — nothing is removed or changed, so a caller built against a lower minor keeps working, and one that needs a newer entry point can require a minimum. Patch is behaviour only and must not be gated on. A C caller can of course resolve the symbol and test for NULL, which is exact and needs no version at all. The field is for the callers that cannot: a binding declaring its imports up front binds on first use and raises a missing-symbol error from inside the call, which is a poor way to find out a library is too old. ⚠ Recorded rather than papered over: 0.1 covers two different surfaces, because #544's entry points shipped without a bump. From 0.2 the minor is the answer. --- docs/c_api.md | 27 ++++++++++++++++++ include/audiocpp.h | 22 +++++++++++++-- include/engine/framework/runtime/session.h | 7 +++++ .../framework/runtime/spec_backed_model.h | 17 +++++++++++ src/capi/audiocpp.cpp | 28 +++++++++++++++++++ src/framework/runtime/session.cpp | 7 +++++ 6 files changed, 106 insertions(+), 2 deletions(-) diff --git a/docs/c_api.md b/docs/c_api.md index 677be9175..66063cbd3 100644 --- a/docs/c_api.md +++ b/docs/c_api.md @@ -71,6 +71,33 @@ if ((audiocpp_abi_version() >> 16) != AUDIOCPP_ABI_VERSION_MAJOR) { } ``` +**minor** increments when entry points are added. Nothing is removed or changed +by such a release, so a caller built against a lower minor keeps working +untouched — but a caller that needs a newer entry point can say so, which is +the only reason the field carries information: + +```c +/* audiocpp_request_set_option_array arrived in 0.2. */ +if ((audiocpp_abi_version() & 0xffff) < 0x0200) { + /* fall back, or refuse, rather than resolving a symbol that is not there */ +} +``` + +A C caller can also just resolve the symbol and test for NULL, which is exact +and needs no number at all. The version is for the callers that cannot: a +binding that declares its imports up front — C#, JNA, ctypes with prototypes — +binds on first use and raises a missing-symbol error from inside the call, which +is a poor way to discover that a library is too old. + +**patch** is for behaviour fixes that add and change no surface. Do not gate on +it; it tells a caller nothing about what it may call. + +⚠ `0.1` covers two different surfaces. The four entry points added in #544 +(`audiocpp_task_count`, `audiocpp_task_name`, `audiocpp_task_from_spec_name`, +`audiocpp_request_set_text_language`) shipped without a bump, before this rule +existed, so a library reporting `0.1` may or may not have them. From `0.2` +onward the minor is the answer. + ## Usage ```c diff --git a/include/audiocpp.h b/include/audiocpp.h index 3601da6eb..76cfe6b7d 100644 --- a/include/audiocpp.h +++ b/include/audiocpp.h @@ -51,11 +51,14 @@ extern "C" { /* ------------------------------------------------------------------ */ #define AUDIOCPP_ABI_VERSION_MAJOR 0 -#define AUDIOCPP_ABI_VERSION_MINOR 1 +#define AUDIOCPP_ABI_VERSION_MINOR 2 #define AUDIOCPP_ABI_VERSION_PATCH 0 /* Packed as (major << 16) | (minor << 8) | patch. A caller built against a - * different MAJOR must not use the library. */ + * different MAJOR must not use the library. MINOR increments when entry points + * are added -- nothing is removed or changed -- so a caller needing a newer one + * can require a minimum; PATCH is behaviour only and must not be gated on. See + * docs/c_api.md. */ AUDIOCPP_API uint32_t audiocpp_abi_version(void); /* audio.cpp's own build version, e.g. "0.2.1". Borrowed, static lifetime. */ @@ -370,6 +373,21 @@ AUDIOCPP_API audiocpp_status audiocpp_request_set_option(audiocpp_request * requ const char * key, const char * value); +/* Sets a list-valued request option -- the transport for the `*_list` option + * types the model spec already declares. `values` is `count` UTF-8 strings, + * copied into the request, so neither the array nor the strings need outlive + * the call. A second call with the same key REPLACES the list rather than + * appending, matching set_option's assignment semantics. + * + * List options live in their own map, so a key set here is not visible to a + * family reading single-valued options and vice versa; a family declares which + * one it wants by the type it puts in its spec. `count` may be 0, which sets an + * empty list -- distinct from never setting the key at all. */ +AUDIOCPP_API audiocpp_status audiocpp_request_set_option_array(audiocpp_request * request, + const char * key, + const char * const * values, + size_t count); + /* ------------------------------------------------------------------ */ /* Result */ /* ------------------------------------------------------------------ */ diff --git a/include/engine/framework/runtime/session.h b/include/engine/framework/runtime/session.h index 01612fac8..bdc03f2f9 100644 --- a/include/engine/framework/runtime/session.h +++ b/include/engine/framework/runtime/session.h @@ -163,6 +163,10 @@ struct TaskRequest { std::optional voice = std::nullopt; std::vector input_artifacts; std::unordered_map options; + /// List-valued options, kept apart from the single-valued ones so a family + /// reading either cannot silently see the other half-formed. The `*_list` + /// option types the spec schema already declares are carried here. + std::unordered_map> option_arrays; }; struct AudioPreparationContract { @@ -176,6 +180,9 @@ struct SessionPreparationRequest { std::optional text = std::nullopt; std::optional voice = std::nullopt; std::unordered_map options; + /// See TaskRequest::option_arrays. Carried through preparation so a session + /// can size its graphs for what run() will actually be handed. + std::unordered_map> option_arrays; }; struct VoiceActivityEvent { diff --git a/include/engine/framework/runtime/spec_backed_model.h b/include/engine/framework/runtime/spec_backed_model.h index 12185f3f5..b56de8541 100644 --- a/include/engine/framework/runtime/spec_backed_model.h +++ b/include/engine/framework/runtime/spec_backed_model.h @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -67,6 +68,22 @@ inline void validate_spec_backed_request_options( } } +/// The same check for list-valued options. An overload rather than a second +/// name: a family that gained a list option should not have to remember to call +/// something different, and every existing call site keeps working untouched. +inline void validate_spec_backed_request_options( + const std::unordered_map & options, + const std::unordered_map> & option_arrays, + const engine::model_spec::ModelContract & contract, + std::string_view model_name) { + validate_spec_backed_request_options(options, contract, model_name); + for (const auto & [key, _] : option_arrays) { + if (contract.request_option_keys.find(key) == contract.request_option_keys.end()) { + throw std::runtime_error("unknown " + std::string(model_name) + " request option: " + key); + } + } +} + inline std::unordered_map apply_option_v1_compatibility( std::unordered_map options, std::initializer_list aliases, diff --git a/src/capi/audiocpp.cpp b/src/capi/audiocpp.cpp index e62d49a2a..15820849d 100644 --- a/src/capi/audiocpp.cpp +++ b/src/capi/audiocpp.cpp @@ -830,6 +830,34 @@ audiocpp_status audiocpp_request_set_option(audiocpp_request * request, const ch }); } +audiocpp_status audiocpp_request_set_option_array(audiocpp_request * request, + const char * key, + const char * const * values, + size_t count) { + if (request == nullptr || key == nullptr) { + return fail(AUDIOCPP_ERR_INVALID_ARGUMENT, "request and key must be non-null"); + } + if (values == nullptr && count != 0) { + return fail(AUDIOCPP_ERR_INVALID_ARGUMENT, "values must be non-null when count is not 0"); + } + for (size_t i = 0; i < count; ++i) { + // Checked before anything is written, so a bad element cannot leave the + // option half-assigned. + if (values[i] == nullptr) { + return fail(AUDIOCPP_ERR_INVALID_ARGUMENT, "option array values must be non-null"); + } + } + return guard([&] { + std::vector copied; + copied.reserve(count); + for (size_t i = 0; i < count; ++i) { + copied.emplace_back(values[i]); + } + request->request.option_arrays[key] = std::move(copied); + return AUDIOCPP_OK; + }); +} + /* ------------------------------------------------------------------ */ /* Result */ /* ------------------------------------------------------------------ */ diff --git a/src/framework/runtime/session.cpp b/src/framework/runtime/session.cpp index 53113f921..035c2e53f 100644 --- a/src/framework/runtime/session.cpp +++ b/src/framework/runtime/session.cpp @@ -432,6 +432,13 @@ SessionPreparationRequest build_preparation_request(const AudioBuffer & audio) { SessionPreparationRequest build_preparation_request(const TaskRequest & request) { SessionPreparationRequest prep; prep.options = request.options; + // ⚠ CARRY THE LIST OPTIONS TOO. Preparation decides how the graphs are sized, and a family + // whose work depends on a list option would otherwise size itself for a different request + // than the one run() is handed. For kokoro_tts that was not merely a bad estimate: with the + // phonemes missing here, preparation fell back to the built-in G2P, and a Japanese request + // failed for want of UniDic even though the caller had supplied phonemes precisely so that + // the G2P would never be consulted. + prep.option_arrays = request.option_arrays; prep.text = request.text_input; prep.voice = request.voice; if (request.audio_input.has_value()) {