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); }