mirror/LocalAI
1
0
Fork 0
mirror of https://github.com/mudler/LocalAI.git synced 2026-07-30 09:57:57 -04:00
Code Issues Packages Projects Releases Wiki Activity

backend(audio-cpp): refuse an impossible stem early, and stop blaming the caller for a failed write

Browse Source 
Four fixes from the first review of the AudioTransform RPC.

check_can_serve now returns the resolved route, so params[stem] on a route that
is not source separation is refused from the route instead of after a full
inference: 11 ms rather than the 4.5 s a miocodec conversion costs, and far
worse on seed_vc or vevo2. The post-run refusal stays as the backstop for a
separation-routed family that returns no stems anyway. The typo'd-stem-name
case still needs the run, since no framework header publishes the stem names
before one.

Stem names carrying control bytes are refused. GGUF strings are length prefixed
and demucs reads its sources from JSON, so an embedded NUL survives to here:
two names differing only after the NUL are distinct std::strings, so the
duplicate check passes them, and then path::c_str() truncates both and they
open the same file. That is exactly the silent overwrite the duplicate check
exists to prevent, with the .wav lost as well.

A failed write is now INTERNAL rather than INVALID_ARGUMENT. The destination is
LocalAI's own generated-content directory, not anything the caller named, so a
full disk or a permission fault there is a server fault and is worth retrying,
which is the opposite of what INVALID_ARGUMENT tells a client. An empty output
path stays INVALID_ARGUMENT.

Two comment corrections and one clarification: the separators' required rate is
their checkpoint's declared samplerate rather than a hardcoded 44100, seed_vc
resamples with soxr and falls back to sinc-hann, and the "no files left behind"
guarantee covers a refused request, not a write that fails partway through the
loop.

Assisted-by: Claude:claude-opus-5 [Claude Code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
This commit is contained in:
Ettore Di Giacinto
2026-07-26 18:15:13 +00:00
committed by localai-org-maint-bot
parent 6170e03f1b
commit b92eff90f1
8 changed files with 185 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
backend/cpp/audio-cpp/audio_io.cpp
View File

@@ -97,7 +97,18 @@ void write_audio_file(const std::string &path,
audio.channels > 0 ? audio.channels : 1,
audio.samples);
} catch (const std::exception &err) {
throw ConfigError("audio-cpp: cannot write " + path + ": " + err.what());
// NOT a ConfigError, and the distinction is not cosmetic. The
// destination is chosen by LocalAI rather than by the caller: it is a
// unique name inside GeneratedContentDir. A failure to write it is a
// full disk, a permission fault on the server's own directory, or a bad
// mount, none of which the caller can fix or is to blame for. As a
// ConfigError this surfaced as INVALID_ARGUMENT, which tells a client
// its request was wrong and not to retry; a plain runtime_error maps to
// INTERNAL, which is both true and retryable. The empty path above
// stays INVALID_ARGUMENT, because that one really is a malformed
// request.
throw std::runtime_error("audio-cpp: cannot write " + path + ": " +
err.what());
}
}

10
backend/cpp/audio-cpp/audio_io.h
View File

@@ -44,8 +44,14 @@ namespace audiocpp_backend {
engine::runtime::AudioBuffer read_audio_file(const std::string &path,
int target_sample_rate);
// Writes 16-bit PCM WAV, creating parent directories. Throws ConfigError when
// the destination cannot be written.
// Writes 16-bit PCM WAV, creating parent directories.
//
// Throws ConfigError, i.e. INVALID_ARGUMENT, ONLY for an empty path, which is a
// malformed request. Every other failure throws a plain runtime_error, i.e.
// INTERNAL: the destination is LocalAI's own generated-content directory and
// not anything the caller named, so a full disk or a permission fault there is
// a server fault and is worth retrying, which is the opposite of what
// INVALID_ARGUMENT tells a client.
void write_audio_file(const std::string &path,
const engine::runtime::AudioBuffer &audio);

49
backend/cpp/audio-cpp/audio_io_ctest.cpp
View File

@@ -152,6 +152,53 @@ static void test_unreadable_file_is_a_config_error() {
check(threw_config_error, "a non-WAV input is INVALID_ARGUMENT, not INTERNAL");
}
// The write side of the same distinction. The destination is LocalAI's own
// generated-content directory, not a caller-supplied path, so a failure to
// write it is a server fault: INTERNAL, which a client may retry, and not
// INVALID_ARGUMENT, which tells it the request itself was wrong.
static void test_write_failure_is_not_a_config_error() {
// A regular file where a directory has to be. ENOTDIR defeats root as well
// as an ordinary user, unlike a chmod, which CI running as root would walk
// straight through.
const auto blocker = scratch_dir() / "blocking-file";
{
FILE *file = fopen(blocker.string().c_str(), "wb");
if (file != nullptr) {
fputs("not a directory", file);
fclose(file);
}
}
const auto path = blocker / "nested" / "out.wav";
bool threw_config_error = false;
bool threw_something = false;
try {
write_audio_file(path.string(), tone(16000, 1, 0.05f));
} catch (const ConfigError &) {
threw_config_error = true;
threw_something = true;
} catch (const std::exception &) {
threw_something = true;
}
check(threw_something, "an unwritable destination is reported at all");
check(!threw_config_error,
"a failed write is INTERNAL, not INVALID_ARGUMENT: the caller did not "
"choose the destination and cannot fix it");
check(!std::filesystem::exists(path), "and nothing was written");
}
static void test_empty_output_path_is_a_config_error() {
// The one write failure that IS the caller's: no path at all.
bool threw_config_error = false;
try {
write_audio_file("", tone(16000, 1, 0.05f));
} catch (const ConfigError &) {
threw_config_error = true;
} catch (const std::exception &) {
}
check(threw_config_error, "an empty output path stays INVALID_ARGUMENT");
}
int main() {
test_441k_stereo_is_read_as_16k_mono();
test_48k_is_read_as_16k();
@@ -159,6 +206,8 @@ int main() {
test_zero_target_keeps_the_native_format();
test_missing_file_is_a_config_error();
test_unreadable_file_is_a_config_error();
test_write_failure_is_not_a_config_error();
test_empty_output_path_is_a_config_error();
if (failures) {
fprintf(stderr, "%d check(s) failed\n", failures);
return 1;

63
backend/cpp/audio-cpp/grpc-server.cpp
View File

@@ -132,8 +132,14 @@ constexpr int kSpeechSampleRate = 16000;
//
// Separation forces it. htdemucs and mel_band_roformer refuse anything but
// their own rate outright, from prepare(): "HTDemucs prepare() sample rate
// mismatch: expected 44100" (src/models/demucs/session.cpp) and the same shape
// in src/models/roformer/session.cpp. Passing kSpeechSampleRate here would
// mismatch: expected N" (src/models/demucs/session.cpp) and the same shape in
// src/models/roformer/session.cpp. That N is the CHECKPOINT'S declared rate,
// read from the packaged config ("samplerate" in demucs/assets.cpp,
// "sample_rate" in roformer/assets.cpp), not a constant: it is 44100 for every
// published checkpoint of both, which is why the messages below say 44100, but
// a checkpoint declaring something else would demand that instead, and only
// passing the file through unchanged can satisfy either. Passing
// kSpeechSampleRate here would
// therefore turn every separation request into an INTERNAL, which is a loud
// failure. The downmix half is the quiet one: both models declare two channels
// and ACCEPT mono, by duplicating it across both, so a mono read would be taken
@@ -147,27 +153,29 @@ constexpr int kSpeechSampleRate = 16000;
//
// seed_vc (vc, svc) seed_vc_prepare_audio_for_sample_rate takes the
// buffer's rate and channel count and resamples to its own mel
// rate with resample_mono_torchaudio_sinc_hann.
// rate: soxr when it is available, falling back to
// resample_mono_torchaudio_sinc_hann (seed_vc/audio_features.cpp).
// vevo2 (vc, s2s, svc) normalize_audio_to_24k_mono converts whatever
// it is given to 24 kHz mono before anything else runs.
// it is given to 24 kHz mono before anything else runs, linearly.
// miocodec (vc, s2s) prepare_miocodec_mono_audio mixes down, then
// resamples to the model's rate, again with sinc-hann.
// resamples to the model's rate with sinc-hann.
// chatterbox (vc) ChatterboxVcComponent::convert normalizes the source to
// 16 kHz and the reference to 24 kHz itself.
// 16 kHz and the reference to 24 kHz itself, linearly.
//
// So every one of them resamples internally, and passing the file through
// unchanged is not merely tolerated but strictly better: their outputs run at
// 22.05 to 44.1 kHz, and pre-folding the input to 16 kHz mono with
// read_audio_file's LINEAR resampler would band-limit at 8 kHz and alias, then
// hand the family a signal it would upsample again. Two of the four use a
// windowed-sinc resampler, which is exactly the quality this constant would
// throw away before they ever saw the samples.
// unchanged is better than folding it first. The reason is the DOUBLE
// conversion, not resampler quality: two of the four resample linearly, exactly
// as read_audio_file would. Their outputs run at 22.05 to 44.1 kHz, so
// pre-folding to 16 kHz mono with read_audio_file's LINEAR, unfiltered
// resampler would band-limit at 8 kHz and alias, and then the family would
// resample that damaged signal a second time on its way up. One conversion is
// the floor; this constant is what keeps it at one.
//
// The cost, stated plainly: a family that cannot handle the file's rate reports
// it from inside the engine as a plain runtime_error, which to_status maps to
// INTERNAL rather than INVALID_ARGUMENT. That is the accepted trade, since the
// only families that refuse are the separators and their requirement is
// 44.1 kHz stereo, i.e. an ordinary music file.
// only families that refuse are the separators and what they want is an
// ordinary music file at its own rate.
constexpr int kTransformSampleRate = 0;
// Parses ModelOptions.MainGPU into a device index.
@@ -801,7 +809,8 @@ public:
// behind somebody else's thirty second run, and it must not blame
// the caller's paths for a decision that had nothing to do with
// them. Same ordering as Diarize and AudioTranscription.
model->check_can_serve(audiocpp_backend::Rpc::AudioTransform, shape);
const audiocpp_backend::Route route =
model->check_can_serve(audiocpp_backend::Rpc::AudioTransform, shape);
if (request->audio_path().empty() || request->dst().empty()) {
throw audiocpp_backend::ConfigError(
@@ -823,6 +832,23 @@ public:
task.options[param.first] = param.second;
}
// Refused from the ROUTE, before the file reads and before the run.
// Only source separation produces named stems, and the route says
// whether this is separation without running anything: the identical
// refusal below, taken from the empty named_audio_outputs list,
// cannot fire until a full conversion has been paid for (measured at
// 1.6 s on miocodec, far worse on seed_vc or vevo2). The one below
// stays as the backstop for a separation-routed family that returns
// no stems anyway.
if (!requested_stem.empty() &&
route.task != audiocpp_backend::Task::SourceSeparation) {
throw audiocpp_backend::ConfigError(
"audio-cpp: family '" + model->family() + "' routes this " +
"request to " + audiocpp_backend::task_name(route.task) +
", which produces a single output with no named stems, so "
"params[stem]='" + requested_stem + "' cannot be honoured");
}
// Native rate, native channels, for both files. See
// kTransformSampleRate: separation is destroyed by a downmix and
// every conversion family resamples internally anyway.
@@ -856,6 +882,13 @@ public:
// unknown stem name is a refused request, and a refused request
// must not leave four files in the caller's output directory
// that it then reports nothing about.
//
// The guarantee is exactly that and no more: a request REFUSED
// ON ITS ARGUMENTS writes nothing. A write that FAILS partway
// through the loop below, on a full disk say, still leaves the
// siblings written before it, with no dst. Nothing is rolled
// back, because deleting files after a disk error is its own
// way to lose data, and the caller sees the failure.
const auto choice =
audiocpp_backend::select_named_output(names, requested_stem);
if (!choice.error.empty()) {

6
backend/cpp/audio-cpp/loaded_model.cpp
View File

@@ -337,11 +337,15 @@ LoadedModel::LoadedModel(const std::string &resolved_path,
capabilities_ = to_capabilities(family, engine_caps);
}
void LoadedModel::check_can_serve(Rpc rpc, const RequestShape &shape) const {
Route LoadedModel::check_can_serve(Rpc rpc, const RequestShape &shape) const {
const Route route = resolve_route(rpc, shape, capabilities_);
if (!route.ok) {
throw CapabilityError(route.error);
}
// Returned so a handler can act on the task before running it. It is the
// same route session_for will resolve, since both read the immutable
// capabilities_ from the same shape.
return route;
}
LoadedModel::Session LoadedModel::session_for(Rpc rpc, const RequestShape &shape,

12
backend/cpp/audio-cpp/loaded_model.h
View File

@@ -99,7 +99,15 @@ public:
const std::string &pinned_task() const noexcept { return pinned_task_; }
// Throws the same CapabilityError session_for would throw when this family
// cannot serve the RPC, and does nothing otherwise.
// cannot serve the RPC, and RETURNS THE RESOLVED ROUTE otherwise.
//
// The route is returned rather than computed and dropped because a handler
// often has to know which task it is about to run BEFORE running it.
// AudioTransform refuses params[stem] on any route but source separation,
// and reading that off the route costs microseconds where reading it off
// the result costs a whole inference first. A caller with no such need
// ignores the value, which is what the three transcription-shaped handlers
// do.
//
// It exists so a refusal does not have to buy a place in the queue first.
// resolve_route is a pure function of capabilities_, which is fixed at
@@ -115,7 +123,7 @@ public:
//
// Const and lane-free on purpose. If a future edit makes routing depend on
// mutable state, this must grow the lane parameter its siblings carry.
void check_can_serve(Rpc rpc, const RequestShape &shape) const;
Route check_can_serve(Rpc rpc, const RequestShape &shape) const;
// Routes the RPC and returns the cached session, creating it on first use.
// Throws CapabilityError when this family cannot serve the RPC, and a plain

16
backend/cpp/audio-cpp/stem_selection.cpp
View File

@@ -34,6 +34,22 @@ bool name_is_writable(const std::string &name) {
if (name.empty() || name == "." || name == "..") {
return false;
}
// Control bytes, NUL above all. GGUF strings are length prefixed and demucs
// reads its source names out of JSON, which can encode one, so a
// std::string holding an embedded NUL survives all the way here. Two such
// names differing only AFTER the NUL are distinct std::strings, so the
// duplicate check below waves them through, and then path::c_str()
// truncates both at the NUL and they open the same file: precisely the
// silent overwrite the duplicate check exists to prevent, with the ".wav"
// stripped off as well. The rest of the range goes with it, since a newline
// or an escape sequence in a file name is a terminal and log injection
// nuisance with no legitimate use.
for (const char byte : name) {
const auto value = static_cast<unsigned char>(byte);
if (value < 0x20 || value == 0x7f) {
return false;
}
}
// Both separators, not just the host's. A GGUF is a downloaded file and its
// strings are not this host's to trust, so a name written on Windows must
// not become a directory traversal wherever the check happens to run.

37
backend/cpp/audio-cpp/stem_selection_test.cpp
View File

@@ -127,6 +127,43 @@ static void test_unwritable_names_are_refused() {
check(late.index == -1 && !late.error.empty(),
"an unwritable name after the selected one still refuses the whole request");
// Control bytes, and the NUL case is why the whole range is refused. These
// two names are DIFFERENT std::strings, so the duplicate check does not
// fire, yet both truncate to "vocals" at path::c_str() and would open one
// file: the silent overwrite the duplicate check exists to prevent, with
// the ".wav" stripped off into the bargain.
const std::string nul_a("vocals\0drums", 12);
const std::string nul_b("vocals\0bass", 11);
check(nul_a != nul_b, "the two NUL names really are distinct std::strings");
check(std::string(nul_a.c_str()) == "vocals" &&
std::string(nul_b.c_str()) == "vocals",
"and both truncate to the same C string, which is the hazard");
const auto nul_pair = select_named_output({nul_a, nul_b}, "");
check(nul_pair.index == -1 && !nul_pair.error.empty(),
"two stem names differing only after an embedded NUL are refused");
check(!select_named_output({"drums", std::string("vo\0cals", 7)}, "").error.empty(),
"a single embedded NUL is refused on its own");
check(!select_named_output({"drums", "voc\nals"}, "").error.empty(),
"a newline in a stem name is refused");
check(!select_named_output({"drums", "voc\tals"}, "").error.empty(),
"a tab in a stem name is refused");
check(!select_named_output({"drums", "voc\033[31mals"}, "").error.empty(),
"an escape sequence in a stem name is refused");
check(!select_named_output({"drums", "voc\177als"}, "").error.empty(),
"DEL in a stem name is refused");
// The boundary below the refused range is the space, which is an ordinary
// file name character and must stay usable, or this check would be
// refusing real stem names.
const auto spaced = select_named_output({"lead vocals", "drums"}, "lead vocals");
check(spaced.index == 0 && spaced.error.empty(),
"a space is not a control character and stays usable");
// And every byte above DEL: a UTF-8 stem name is ordinary, and signed char
// would make those bytes compare as negative.
const auto utf8 = select_named_output({"vocals", "b\xc3\xa4sse"}, "b\xc3\xa4sse");
check(utf8.index == 1 && utf8.error.empty(),
"a UTF-8 stem name is not mistaken for a control character");
// A leading dot is not a traversal and must stay usable.
const auto dotted = select_named_output({".vocals", "drums"}, ".vocals");
check(dotted.index == 0 && dotted.error.empty(),