From ca6303171a4350db24f12291b720c2eb0f25670d Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Sun, 26 Jul 2026 21:53:23 +0000 Subject: [PATCH] backend(audio-cpp): stop emitting an empty style language, and name the missing clip StyleCondition::language was set whenever has_language() was true, with no !empty() guard, while the language option twelve lines below had one. core/backend/tts.go sets Language unconditionally, so has_language() is true on every request LocalAI sends and carries "" when the caller named none. An engaged-but-empty style language is worse than an absent one: supertonic reads text_input->language behind its own !empty() guard and then overrides it from style->language with no guard at all, so "" replaced its "en" default and its tokenizer threw "invalid Supertonic language: ". Every /v1/audio/speech request that set instructions and no language would have been an INTERNAL against a supertonic model. A plain request never saw it, because the style condition only exists when instructions are non-empty, which is why the chatterbox end to end run did not catch it. TTS also stops discarding the Route that check_can_serve already returns. A family routed to voice cloning without a reference clip used to be refused from inside its own prepare(), which meant an INTERNAL naming neither the RPC nor the field to set; chatterbox advertises clon and no tts, so that was every preset-only request to it. It is now an INVALID_ARGUMENT naming TTSRequest.voice, answered in about 4 ms, and it cannot misfire because has_voice_reference is what selected cloning in the first place. Reading CapabilitySet::supports_speaker_reference to generalise this stays a follow-up. The src read carries a written caveat rather than a family blocklist, because ace_step's editing routes legitimately need src: setting src on a stable_audio model corrupts the heap and aborts the process in the pinned upstream, and the only thing keeping that off the network is that schema.ElevenLabsSoundGenerationRequest has no field for it. Nobody reading that Go schema would know why, so the reason is recorded where the field is read. build_tts_shape is extracted so TTSStream cannot describe the same request differently, and it arrived untested: two mutations of it survived until a test_tts_shape case was added. Assisted-by: Claude:claude-opus-5 [Claude Code] Signed-off-by: Ettore Di Giacinto --- backend/cpp/audio-cpp/generation_request.cpp | 25 ++- backend/cpp/audio-cpp/generation_request.h | 12 ++ .../audio-cpp/generation_request_ctest.cpp | 145 ++++++++++++++++++ backend/cpp/audio-cpp/grpc-server.cpp | 89 +++++++++-- 4 files changed, 256 insertions(+), 15 deletions(-) diff --git a/backend/cpp/audio-cpp/generation_request.cpp b/backend/cpp/audio-cpp/generation_request.cpp index 6dbc7489e..2e63e1661 100644 --- a/backend/cpp/audio-cpp/generation_request.cpp +++ b/backend/cpp/audio-cpp/generation_request.cpp @@ -20,6 +20,18 @@ bool voice_is_reference_file(const std::string &voice) { return std::filesystem::is_regular_file(std::filesystem::path(voice), ec); } +RequestShape build_tts_shape(const backend::TTSRequest &request) { + RequestShape shape; + shape.has_voice_reference = voice_is_reference_file(request.voice()); + // !empty() as well as has_instructions(), and it must match the guard in + // build_tts_request: a request whose instructions are an empty string + // carries no style condition, so telling routing to prefer VoiceDesign for + // it would route to a task with nothing to design from. + shape.has_instructions = + request.has_instructions() && !request.instructions().empty(); + return shape; +} + engine::runtime::TaskRequest build_tts_request(const backend::TTSRequest &request, std::optional reference_audio) { @@ -90,7 +102,18 @@ build_tts_request(const backend::TTSRequest &request, // Spelling it "instructions" here would have made the whole // StyleCondition dead weight. style.tags["instruct"] = request.instructions(); - if (request.has_language()) { + // !empty(), matching the option emission below, and load-bearing rather + // than tidiness. core/backend/tts.go's newTTSRequest sets + // `Language: &language` UNCONDITIONALLY, so has_language() is true on + // every request LocalAI sends and carries "" whenever the caller named + // no language. An engaged-but-empty style language is WORSE than an + // absent one: supertonic reads text_input->language behind its own + // !empty() guard and then OVERRIDES it from style->language with no + // guard at all (supertonic/session.cpp), so "" would replace its "en" + // default and tokenizer_text.cpp would throw + // "invalid Supertonic language: " on every request that set + // instructions and no language. + if (request.has_language() && !request.language().empty()) { style.language = request.language(); } condition.style = std::move(style); diff --git a/backend/cpp/audio-cpp/generation_request.h b/backend/cpp/audio-cpp/generation_request.h index b98891994..e703b11c3 100644 --- a/backend/cpp/audio-cpp/generation_request.h +++ b/backend/cpp/audio-cpp/generation_request.h @@ -16,6 +16,7 @@ // result_map does. #include "backend.pb.h" +#include "capability_routing.h" #include "engine/framework/runtime/session.h" @@ -45,6 +46,17 @@ namespace audiocpp_backend { // filesystem exception thrown while classifying a string. bool voice_is_reference_file(const std::string &voice); +// Everything routing needs to know about a TTSRequest, in one place, so that TTS +// and TTSStream cannot describe the same request differently. +// +// `pinned_task` is deliberately NOT filled here: it comes off the LoadedModel, +// not off the request, and this unit links no engine. The caller must still +// write `shape.pinned_task = model->pinned_task();` or the model's `task:` +// option is dead. That is the one field a new handler can forget, so it is the +// one field left visible at the call site rather than hidden behind this +// helper. +RequestShape build_tts_shape(const backend::TTSRequest &request); + // `reference_audio` is the already-read speaker clip, present exactly when // voice_is_reference_file(request.voice()) was true. Passing it in rather than a // path keeps this function pure and lets the caller do the read where the diff --git a/backend/cpp/audio-cpp/generation_request_ctest.cpp b/backend/cpp/audio-cpp/generation_request_ctest.cpp index 86c35525d..d6f8aa8fd 100644 --- a/backend/cpp/audio-cpp/generation_request_ctest.cpp +++ b/backend/cpp/audio-cpp/generation_request_ctest.cpp @@ -89,6 +89,83 @@ static void test_voice_is_reference_file() { std::filesystem::remove_all(dir); } +// build_tts_shape is what TTS and TTSStream both hand to routing, so a wrong +// answer here silently changes which task a request runs as, with a 200 and no +// diagnostic. Every field is asserted in both directions. +static void test_tts_shape() { + const auto dir = std::filesystem::temp_directory_path() / + "audiocpp-generation-request-ctest-shape"; + std::filesystem::remove_all(dir); + std::filesystem::create_directories(dir); + const auto file = dir / "reference.wav"; + { + std::ofstream out(file, std::ios::binary); + out << "a regular file"; + } + + { + backend::TTSRequest request; + request.set_text("hello"); + const auto shape = build_tts_shape(request); + check(!shape.has_voice_reference && !shape.has_instructions, + "shape: bare request has neither signal"); + // Never filled here: it comes off the LoadedModel, and leaving it empty + // is what makes the caller's assignment visible at the call site. + check(shape.pinned_task.empty(), "shape: pinned_task is left to the caller"); + } + { + backend::TTSRequest request; + request.set_voice(file.string()); + const auto shape = build_tts_shape(request); + check(shape.has_voice_reference, + "shape: an existing file is a voice reference"); + check(!shape.has_instructions, "shape: a clip is not an instruction"); + } + { + backend::TTSRequest request; + request.set_voice("alloy"); + const auto shape = build_tts_shape(request); + check(!shape.has_voice_reference, + "shape: a preset name is not a voice reference"); + } + { + backend::TTSRequest request; + request.set_voice(dir.string()); + const auto shape = build_tts_shape(request); + check(!shape.has_voice_reference, + "shape: a directory is not a voice reference"); + } + { + backend::TTSRequest request; + request.set_instructions("a calm older man"); + const auto shape = build_tts_shape(request); + check(shape.has_instructions, "shape: instructions are seen"); + check(!shape.has_voice_reference, + "shape: instructions do not imply a reference"); + } + { + // The guard that has to match build_tts_request's. An empty + // instructions string builds no style condition, so telling routing to + // prefer VoiceDesign for it would route to a task with nothing to + // design from. + backend::TTSRequest request; + request.set_instructions(""); + const auto shape = build_tts_shape(request); + check(!shape.has_instructions, + "shape: an empty instructions string is not an instruction"); + } + { + backend::TTSRequest request; + request.set_voice(file.string()); + request.set_instructions("a calm older man"); + const auto shape = build_tts_shape(request); + check(shape.has_voice_reference && shape.has_instructions, + "shape: both signals are reported when both are set"); + } + + std::filesystem::remove_all(dir); +} + static void test_tts_plain() { backend::TTSRequest request; request.set_text("hello there"); @@ -200,6 +277,71 @@ static void test_tts_empty_instructions_are_not_instructions() { "tts: an empty instructions string sets no voice condition"); } +// THE EXACT SHAPE LocalAI PUTS ON THE WIRE. core/backend/tts.go's newTTSRequest +// sets Language: &language UNCONDITIONALLY, so has_language() is true on every +// request that ever reaches this backend, carrying an empty string whenever the +// caller named no language. +// +// An empty StyleCondition::language is not a harmless default. supertonic reads +// text_input->language behind a !empty() guard and then OVERRIDES it from +// style->language whenever that optional is engaged, with no guard at all +// (supertonic/session.cpp generation_options_from_request), so an empty style +// language replaces its "en" default (session.h) with "" and +// tokenizer_text.cpp's preprocess throws "invalid Supertonic language: ". +// Every /v1/audio/speech request carrying instructions and no language would be +// an INTERNAL. A plain request never sees it, because the style condition only +// exists when instructions are non-empty. +static void test_tts_empty_language_is_not_a_language() { + backend::TTSRequest request; + request.set_text("hello"); + request.set_instructions("a calm older man"); + request.set_language(""); + + const auto task = build_tts_request(request, std::nullopt); + + check(task.voice.has_value() && task.voice->style.has_value(), + "tts empty language: the style condition still exists"); + check(!task.voice->style->language.has_value(), + "tts empty language: style language is left unset, not set to empty"); + // The option emission has always guarded on !empty(); this pins the two to + // the same rule so they cannot drift apart again. + check(!has_key(task.options, "language"), + "tts empty language: no language option"); + check(task.text_input.has_value() && task.text_input->language.empty(), + "tts empty language: transcript language stays empty"); +} + +// The one shape routing treats specially: a clip outranks instructions, and the +// VoiceCondition then has to carry BOTH, because the family that wins is chosen +// on the clip but may still read the style tag. +static void test_tts_clip_and_instructions() { + backend::TTSRequest request; + request.set_text("hello"); + request.set_voice("/tmp/reference.wav"); + request.set_instructions("bright and fast"); + request.set_language("en"); + + const auto task = build_tts_request(request, clip(22050, 1)); + + check(task.voice.has_value(), "tts clip+instructions: voice condition present"); + check(task.voice->speaker.has_value() && + task.voice->speaker->audio.has_value() && + task.voice->speaker->audio->sample_rate == 22050, + "tts clip+instructions: speaker carries the clip"); + check(!task.voice->speaker->cached_voice_id.has_value(), + "tts clip+instructions: the clip path is not also a preset id"); + check(task.voice->style.has_value() && + option_or(task.voice->style->tags, "instruct", "") == "bright and fast", + "tts clip+instructions: style carries the instruct tag"); + check(task.voice->style->language.has_value() && + *task.voice->style->language == "en", + "tts clip+instructions: a real language does reach the style condition"); + check(option_or(task.options, "instruct", "") == "bright and fast", + "tts clip+instructions: instruct option still emitted"); + check(!has_key(task.options, "voice"), + "tts clip+instructions: still no voice option for a clip"); +} + static void test_tts_language_and_params() { backend::TTSRequest request; request.set_text("ciao"); @@ -304,11 +446,14 @@ static void test_sound_generation_full() { int main() { test_voice_is_reference_file(); + test_tts_shape(); test_tts_plain(); test_tts_named_preset(); test_tts_reference_clip(); test_tts_instructions(); test_tts_empty_instructions_are_not_instructions(); + test_tts_empty_language_is_not_a_language(); + test_tts_clip_and_instructions(); test_tts_language_and_params(); test_sound_generation_minimal(); test_sound_generation_full(); diff --git a/backend/cpp/audio-cpp/grpc-server.cpp b/backend/cpp/audio-cpp/grpc-server.cpp index 027492222..360f4d0e4 100644 --- a/backend/cpp/audio-cpp/grpc-server.cpp +++ b/backend/cpp/audio-cpp/grpc-server.cpp @@ -1027,12 +1027,13 @@ public: return refusal; } - const bool voice_is_file = - audiocpp_backend::voice_is_reference_file(request->voice()); - audiocpp_backend::RequestShape shape; - shape.has_voice_reference = voice_is_file; - shape.has_instructions = - request->has_instructions() && !request->instructions().empty(); + // One description of the request, shared with TTSStream, so the two + // handlers cannot describe the same request differently. + // pinned_task stays here on purpose: it comes off the model rather + // than the request, and it is the field a new handler forgets. + audiocpp_backend::RequestShape shape = + audiocpp_backend::build_tts_shape(*request); + const bool voice_is_file = shape.has_voice_reference; shape.pinned_task = model->pinned_task(); // FIRST, before the lane and before any file read. A family that @@ -1041,7 +1042,40 @@ public: // must not blame the caller's reference clip for a decision that had // nothing to do with it. Same ordering as Diarize and // AudioTransform. - model->check_can_serve(audiocpp_backend::Rpc::Tts, shape); + const audiocpp_backend::Route route = + model->check_can_serve(audiocpp_backend::Rpc::Tts, shape); + + // Refused FROM THE ROUTE, the same trick AudioTransform uses for + // params[stem]. Voice cloning without a clip is a request the family + // cannot answer, and every cloning family says so only from inside + // its own prepare(): chatterbox throws "Chatterbox prepare requires + // speaker reference audio", which to_status maps to INTERNAL and + // which names neither the RPC nor the field the caller has to set. + // chatterbox advertises clon and no tts at all, so before this every + // preset-only or voice-less request to it got that engine-internal + // message. + // + // It cannot misfire on the legitimate case: has_voice_reference is + // what made routing choose VoiceCloning in the first place, so + // reaching here with a clip present is impossible unless the model + // pinned task:clon, and a clon pin with no clip is exactly the + // misconfiguration worth naming. + // + // Deliberately NOT generalised to "every task whose family declares + // supports_speaker_reference". That flag lives on the engine's + // CapabilitySet, is not carried through this backend's Capabilities + // mirror, and would need its own answer to whether it is advisory or + // binding before routing could act on it. Tracked as a follow-up. + if (route.task == audiocpp_backend::Task::VoiceCloning && + !voice_is_file) { + throw audiocpp_backend::ConfigError( + "audio-cpp: family '" + model->family() + + "' routes this request to " + + audiocpp_backend::task_name(route.task) + + ", which needs a speaker reference clip; set TTSRequest.voice" + " to the path of a WAV file (a voice that is not a file on " + "disk is treated as a named preset)"); + } if (request->dst().empty()) { throw audiocpp_backend::ConfigError( @@ -1089,10 +1123,13 @@ public: result->set_message(request->dst()); return GStatus::OK; } catch (const std::exception &err) { - // Both channels, unlike the transcription-shaped handlers: Result - // has a success flag that the Go side checks in addition to the - // status, so leaving it default-false with no message would report a - // failure with no reason attached. + // DEFENSIVE, not load-bearing, and worth saying so plainly. gRPC + // discards the response message entirely when the status is not OK, + // so a client that checks the status, which core/backend/tts.go + // does before it ever looks at res.Success, receives a nil Result + // and never sees these two fields. They are set for a client that + // ignores the status, and because a half-filled Result is a worse + // thing to leave behind than a filled one. result->set_success(false); result->set_message(err.what()); return to_status(err); @@ -1124,11 +1161,35 @@ public: "audio-cpp: SoundGeneration needs a dst output path"); } + // DO NOT WIRE src INTO THE HTTP LAYER UNTIL THE PIN IS BUMPED PAST + // THE FIX. Setting src on a stable_audio model CORRUPTS THE HEAP AND + // ABORTS THE PROCESS in the pinned upstream: "free(): invalid size" + // / "munmap_chunk(): invalid pointer", SIGABRT, backend gone. It is + // upstream's, not this handler's, and it was attributed rather than + // assumed: upstream's own audiocpp_cli, built from this same + // checkout, aborts identically with --audio at exit 134, at both + // 44.1 kHz stereo and 24 kHz mono, and completes cleanly with no + // --audio at all. It is therefore neither caused nor worsened by + // reading the clip at its native rate below. + // + // The only thing keeping that off the network today is an OMISSION: + // core/http/endpoints/elevenlabs/soundgeneration.go passes nil for + // sourceFile, and schema.ElevenLabsSoundGenerationRequest has no + // field for it, so the sole caller that can set src is + // core/cli/soundgeneration.go. Adding the field to that schema turns + // a local CLI crash into a remotely reachable heap corruption with + // fully attacker-influenced input. Nobody reading that Go schema + // would know why the field is missing, which is why this is written + // here as well as in the task report. + // + // No family blocklist is applied: ace_step's editing routes + // legitimately need src, and a family-specific guard here would rot. std::optional source; if (request->has_src() && !request->src().empty()) { - // Native rate and channels. ace_step and stable_audio both - // resample per channel, so a downmix here would delete the - // stereo image of the very clip they are editing. + // Native rate and channels. ace_step resamples left and right + // separately and stable_audio resamples per channel, so a + // downmix here would delete the stereo image of the very clip + // they are editing. source = audiocpp_backend::read_audio_file( request->src(), kVoiceReferenceSampleRate); }