From e9d03c7c4899b4e40a86bc0b33cd53a2032ba11e Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Mon, 27 Jul 2026 07:40:07 +0000 Subject: [PATCH] fix(audio-transform): bound sample_rate, keep same-named uploads apart Four defects the whole-branch review found on the Go side, plus two comment corrections. sample_rate is a disk-exhaustion hazard. The branch added the `form:` tag that makes the field bind for the first time, so the resample path went from dead to live, and utils.AudioResample interpolates the int straight into ffmpeg's -ar with no bound. Measured with ffmpeg 7: -ar 999999999 on a 0.01 s clip writes 20 MB and exits 0, which scales linearly to the reported 3.9 GB for one second, into a GeneratedContentDir nothing sweeps, and convertStems repeats it once per separation stem. Clamped to 8000..192000 in the handler, before the temp dir and before the model is touched, and rejected with a 400 outside it. The low end was reported as "a 0-byte file". It is not: -ar 1 writes a 78-byte header with no audio behind it, whose declared data size still claims 70 bytes, so go-audio parses it as a 35 SECOND file and a size check does not see it. The guard therefore compares the declared data chunk against the bytes actually on disk, and AudioResample now fails rather than returning a WAV carrying nothing. Both parts of a transform request land in one temp dir, and the raw copy was named only after the client's basename, so `-F audio=@mic/clip.wav -F reference=@loopback/clip.wav` wrote "raw-clip.wav" twice. Since AudioToWavPreservingShape hardlinks an already-PCM16 WAV rather than copying it, the reference part's os.Create truncated the inode audio.wav pointed at: mic and reference came out identical, which makes an echo canceller null everything and return near-silence with a 200. The raw copy now carries the form field name. audio-cpp had no BackendCapabilities entry, so VoiceCloningForModel returned nil before it ever consulted the model's tts.voice_cloning override and every `voice: "profile:"` request was refused with a 400, on a backend that ships audio-cpp-chatterbox whose family serves cloning and not plain TTS. Registered with its RPCs, usecases and the reference-audio contract, and deliberately without the 16 kHz mono fold, which its separation families cannot survive. GetBackendCapability was exact-match only, so every pinned gallery variant read as an unknown backend: vulkan-localvqe lost the 16 kHz mono fold that used to be unconditional and started failing inside LocalVQE, and the usecase gate does not stand in for it because BuildFilteredFirstAvailableDefaultModel returns early once the client names a model. Lookup now falls back to the meta name by stripping the gallery's hardware prefix and release-channel suffix, exact match first so nothing can be shadowed. Same class as #10945. Also corrected: the AudioTransformRequest comment claimed echo's binder falls back to the field name, which it does not in either direction (bindData binds ONLY tagged fields and `continue`s otherwise; `model` arrives from setModelNameFromRequest's c.FormValue). And the stable_audio `src` heap corruption caveat now lives on ElevenLabsSoundGenerationRequest, where the Go developer who would add the field can see it, instead of only in C++. Assisted-by: Claude:claude-opus-5 [Claude Code] Signed-off-by: Ettore Di Giacinto --- core/config/backend_capabilities.go | 121 +++++++++-- core/config/backend_capabilities_test.go | 104 ++++++++- .../http/endpoints/localai/audio_transform.go | 57 ++++- .../localai/audio_transform_internal_test.go | 197 ++++++++++++++++++ core/schema/audio_transform.go | 26 ++- core/schema/elevenlabs.go | 18 ++ docs/content/features/audio-transform.md | 2 +- pkg/utils/ffmpeg.go | 75 +++++++ pkg/utils/ffmpeg_shape_test.go | 57 +++++ 9 files changed, 633 insertions(+), 24 deletions(-) create mode 100644 core/http/endpoints/localai/audio_transform_internal_test.go diff --git a/core/config/backend_capabilities.go b/core/config/backend_capabilities.go index e719d3a70..9a83c7455 100644 --- a/core/config/backend_capabilities.go +++ b/core/config/backend_capabilities.go @@ -448,6 +448,36 @@ var BackendCapabilities = map[string]BackendCapability{ DefaultUsecases: []string{UsecaseTranscript}, Description: "Sherpa-ONNX — multi-model speech toolkit (ASR, TTS, VAD)", }, + // audio-cpp is one gRPC server in front of ~30 audio.cpp model families, so + // PossibleUsecases is their UNION and no single model serves all of it: + // which RPCs a given model answers is decided by the family baked into its + // GGUF, and every audio-cpp gallery entry pins its own known_usecases. + // + // VoiceCloning is not decoration here. VoiceCloningForModel returns nil as + // soon as the backend has no capability entry, BEFORE it consults the + // model's own tts.voice_cloning override, so without this key a + // `voice: "profile:"` request is refused with 400 for every audio-cpp + // model and no model YAML can rescue it — on a backend that ships + // audio-cpp-chatterbox, whose family advertises cloning and not plain TTS, + // so a reference clip is the only way to use it at all. + // + // Deliberately NOT AudioTransformInputMono16k: the families this backend + // reaches through AudioTransform are separation and conversion (htdemucs, + // mel_band_roformer, seed_vc), which refuse any rate but their + // checkpoint's own and work from the stereo image. + "audio-cpp": { + GRPCMethods: []GRPCMethod{ + MethodTTS, MethodTTSStream, MethodAudioTranscription, + MethodVAD, MethodDiarize, MethodSoundGeneration, MethodAudioTransform, + }, + PossibleUsecases: []string{ + UsecaseTTS, UsecaseTranscript, UsecaseVAD, UsecaseDiarization, + UsecaseSoundGeneration, UsecaseAudioTransform, + }, + DefaultUsecases: []string{UsecaseTTS}, + VoiceCloning: referenceVoiceCloning(), + Description: "audio.cpp native engine — one server for TTS, voice cloning, ASR, forced alignment, VAD, diarization, source separation and music generation; the model's family decides which", + }, // --- TTS backends --- "piper": { @@ -667,11 +697,59 @@ func NormalizeBackendName(backend string) string { return strings.ReplaceAll(backend, ".", "-") } -// llamaCppChannelSuffixes are the release-channel suffixes appended to a -// llama.cpp backend name in the gallery ("llama-cpp" vs -// "llama-cpp-development"). They carry no engine information, so they are -// stripped before the family check below. -var llamaCppChannelSuffixes = []string{"-development", "-quantization"} +// galleryChannelSuffixes are the release-channel suffixes appended to a backend +// name in the gallery ("llama-cpp" vs "llama-cpp-development" vs +// "llama-cpp-quantization"). They carry no engine information, so they are +// stripped before any family or capability lookup falls back. +var galleryChannelSuffixes = []string{"-development", "-quantization"} + +// galleryHardwarePrefixes are the acceleration prefixes the gallery prepends +// when it publishes one concrete backend image per hardware capability behind a +// meta name: "cpu-localvqe", "vulkan-localvqe", "cuda12-audio-cpp", +// "metal-darwin-arm64-llama-cpp". They carry no engine information either, and +// an operator may pin any of them in a model config's `backend:`. +// +// This is the exhaustive set present in backend/index.yaml, longest first so +// "cuda13-nvidia-l4t-arm64-" is tried before "cuda13-" and +// "intel-sycl-f16-" before "intel-". Stripping is a FALLBACK only (see +// GetBackendCapability), so a backend whose real name happened to start with +// one of these would still be found by its exact name first. +var galleryHardwarePrefixes = []string{ + "cuda13-nvidia-l4t-arm64-", + "metal-darwin-arm64-", + "nvidia-l4t-arm64-", + "intel-sycl-f16-", + "intel-sycl-f32-", + "nvidia-l4t-", + "vulkan-", + "cuda12-", + "cuda13-", + "metal-", + "intel-", + "rocm-", + "cpu-", +} + +// stripBackendVariant reduces a concrete gallery backend name to the meta name +// its capabilities are registered under: "vulkan-localvqe-development" becomes +// "localvqe". Returns the input unchanged when nothing matches. +// +// Both halves are needed. A pinned variant carries a hardware prefix, a release +// channel carries a suffix, and backend/index.yaml ships names with both. +func stripBackendVariant(name string) string { + for _, suffix := range galleryChannelSuffixes { + if strings.HasSuffix(name, suffix) { + name = strings.TrimSuffix(name, suffix) + break + } + } + for _, prefix := range galleryHardwarePrefixes { + if strings.HasPrefix(name, prefix) { + return strings.TrimPrefix(name, prefix) + } + } + return name +} // IsLlamaCppBackend reports whether a backend name refers to a build of the // llama.cpp gRPC server. The gallery ships one concrete backend per hardware @@ -692,7 +770,7 @@ func IsLlamaCppBackend(backend string) bool { if name == "" { return true } - for _, suffix := range llamaCppChannelSuffixes { + for _, suffix := range galleryChannelSuffixes { name = strings.TrimSuffix(name, suffix) } if strings.HasSuffix(name, "ik-llama-cpp") { @@ -751,10 +829,29 @@ func UsesLlamaCppServingOptions(backend string) bool { // GetBackendCapability returns the capability info for a backend, or nil if unknown. // Handles backend name normalization. +// +// A PINNED GALLERY VARIANT RESOLVES TO ITS META NAME. The gallery ships one +// image per hardware capability ("cpu-localvqe", "vulkan-localvqe", +// "metal-localvqe") and an operator may put any of them in a model's +// `backend:`. They are the same engine, so an exact-match-only lookup silently +// downgraded every pinned model to "unknown backend": vulkan-localvqe lost the +// 16 kHz mono fold that /audio/transform used to apply unconditionally and +// started failing inside LocalVQE, and a pinned audio-cpp variant would lose +// its voice-cloning contract the same way. Same class of bug as #10945, same +// answer as IsLlamaCppBackend. +// +// Exact match FIRST, so a backend genuinely registered under a variant-looking +// name keeps its own entry and stripping can never shadow it. func GetBackendCapability(backend string) *BackendCapability { - if cap, ok := BackendCapabilities[NormalizeBackendName(backend)]; ok { + name := NormalizeBackendName(backend) + if cap, ok := BackendCapabilities[name]; ok { return &cap } + if base := stripBackendVariant(name); base != name { + if cap, ok := BackendCapabilities[base]; ok { + return &cap + } + } return nil } @@ -766,11 +863,11 @@ func GetBackendCapability(backend string) *BackendCapability { // (source separation, voice conversion at 44.1 kHz) works without an entry // here, and one that needs the fold cannot get it by accident. // -// Known limitation: the lookup is on the bare backend name, so a pinned variant -// (vulkan-localvqe, metal-localvqe) does not get the fold. Pinned variants are -// already second-class in the same way in the audio-transform gate in -// model_config.go, and the failure is loud rather than silent; IsLlamaCppBackend -// below is the suffix-tolerant precedent (#10945) if that ever has to change. +// Pinned gallery variants are covered: GetBackendCapability strips the hardware +// prefix and the release-channel suffix, so cpu-localvqe, vulkan-localvqe and +// metal-localvqe all fold exactly as "localvqe" does. They must, because the +// usecase gate does not stand in for this one: naming a model explicitly makes +// BuildFilteredFirstAvailableDefaultModel return before it filters. func AudioTransformRequiresMono16kInput(backend string) bool { capability := GetBackendCapability(backend) return capability != nil && capability.AudioTransformInputMono16k diff --git a/core/config/backend_capabilities_test.go b/core/config/backend_capabilities_test.go index 8f22ad44a..93729a2a6 100644 --- a/core/config/backend_capabilities_test.go +++ b/core/config/backend_capabilities_test.go @@ -70,6 +70,94 @@ var _ = Describe("GetBackendCapability", func() { It("returns nil for unknown backends", func() { Expect(GetBackendCapability("nonexistent")).To(BeNil()) }) + + // The gallery ships one concrete image per hardware capability behind a + // meta name, and an operator may pin any of them in a model's `backend:`. + // An exact-match-only lookup silently treated every one of them as an + // unknown backend, which cost vulkan-localvqe the 16 kHz mono fold its AEC + // needs and would cost a pinned audio-cpp variant its voice-cloning + // contract. Same class as #10945. + It("resolves a pinned hardware variant to its meta backend", func() { + for _, name := range []string{"cpu-localvqe", "vulkan-localvqe", "metal-localvqe"} { + capability := GetBackendCapability(name) + Expect(capability).NotTo(BeNil(), "pinned variant %q must resolve", name) + Expect(capability.PossibleUsecases).To(ContainElement(UsecaseAudioTransform), name) + } + }) + + It("resolves a pinned variant that also carries a release channel", func() { + for _, name := range []string{ + "cuda12-audio-cpp", "cuda13-audio-cpp-development", + "metal-audio-cpp", "cpu-audio-cpp-development", + "cuda13-nvidia-l4t-arm64-llama-cpp", "intel-sycl-f16-llama-cpp", + "metal-darwin-arm64-llama-cpp", "nvidia-l4t-arm64-llama-cpp", + "rocm-llama-cpp-development", "intel-llama-cpp", + } { + Expect(GetBackendCapability(name)).NotTo(BeNil(), "pinned variant %q must resolve", name) + } + }) + + It("does not invent a capability for a name that only looks like a variant", func() { + Expect(GetBackendCapability("cpu-nonexistent")).To(BeNil()) + Expect(GetBackendCapability("vulkan-")).To(BeNil()) + }) + + // Stripping is a fallback, never a rewrite: a backend registered under its + // own name keeps its own entry even if that name starts with a prefix. + It("prefers an exact match over the stripped one", func() { + BackendCapabilities["cpu-exact-match-probe"] = BackendCapability{ + PossibleUsecases: []string{UsecaseChat}, + Description: "test fixture", + } + BackendCapabilities["exact-match-probe"] = BackendCapability{ + PossibleUsecases: []string{UsecaseTTS}, + Description: "test fixture", + } + DeferCleanup(func() { + delete(BackendCapabilities, "cpu-exact-match-probe") + delete(BackendCapabilities, "exact-match-probe") + }) + + capability := GetBackendCapability("cpu-exact-match-probe") + Expect(capability).NotTo(BeNil()) + Expect(capability.PossibleUsecases).To(Equal([]string{UsecaseChat})) + }) +}) + +// audio-cpp advertises voice cloning from the backend itself and ships +// audio-cpp-chatterbox, whose family serves cloning and NOT plain TTS, so a +// reference clip is the only way to use it. Without a capability entry +// VoiceCloningForModel returns nil before it ever reads the model's own +// tts.voice_cloning override, so `voice: "profile:"` was refused with a 400 +// for every audio-cpp model and no model YAML could rescue it. +var _ = Describe("audio-cpp capabilities", func() { + It("is registered", func() { + Expect(GetBackendCapability("audio-cpp")).NotTo(BeNil()) + }) + + It("advertises the RPCs its families actually serve", func() { + capability := GetBackendCapability("audio-cpp") + Expect(capability.GRPCMethods).To(ContainElements( + MethodTTS, MethodTTSStream, MethodAudioTranscription, + MethodVAD, MethodDiarize, MethodSoundGeneration, MethodAudioTransform)) + Expect(capability.PossibleUsecases).To(ContainElements( + UsecaseTTS, UsecaseTranscript, UsecaseVAD, UsecaseDiarization, + UsecaseSoundGeneration, UsecaseAudioTransform)) + }) + + It("carries the reference-audio contract, for pinned variants too", func() { + for _, name := range []string{"audio-cpp", "cuda12-audio-cpp", "metal-audio-cpp"} { + cloning := VoiceCloningForModel(&ModelConfig{Backend: name}) + Expect(cloning).NotTo(BeNil(), "%q must reach the backend with a profile voice", name) + Expect(cloning.AcceptedAudioFormats).To(ContainElement("audio/wav")) + } + }) + + // The families audio-cpp reaches through AudioTransform are separation and + // conversion, which refuse any rate but their checkpoint's own. + It("does not ask for the 16 kHz mono fold", func() { + Expect(AudioTransformRequiresMono16kInput("audio-cpp")).To(BeFalse()) + }) }) // The fold to 16 kHz mono in /audio/transform is opt-IN. It used to be @@ -82,7 +170,21 @@ var _ = Describe("AudioTransformRequiresMono16kInput", func() { Expect(AudioTransformRequiresMono16kInput("localvqe")).To(BeTrue()) }) - It("does not fold for an unregistered backend", func() { + // Pinned gallery variants are the same engine and must fold identically. + // They did not: the lookup was exact-match only, so vulkan-localvqe was an + // unknown backend, lost the fold that used to be unconditional, and started + // failing inside LocalVQE. The usecase gate is no substitute, because + // BuildFilteredFirstAvailableDefaultModel returns early once the client + // names a model explicitly. + It("folds for every pinned localvqe variant the gallery ships", func() { + Expect(AudioTransformRequiresMono16kInput("cpu-localvqe")).To(BeTrue()) + Expect(AudioTransformRequiresMono16kInput("vulkan-localvqe")).To(BeTrue()) + Expect(AudioTransformRequiresMono16kInput("metal-localvqe")).To(BeTrue()) + }) + + It("does not fold for a backend that has not asked for it", func() { + // Registered, and deliberately NOT folding: its separation and + // conversion families refuse any rate but their checkpoint's own. Expect(AudioTransformRequiresMono16kInput("audio-cpp")).To(BeFalse()) Expect(AudioTransformRequiresMono16kInput("nonexistent")).To(BeFalse()) Expect(AudioTransformRequiresMono16kInput("")).To(BeFalse()) diff --git a/core/http/endpoints/localai/audio_transform.go b/core/http/endpoints/localai/audio_transform.go index ba1b2e349..5b3be15b5 100644 --- a/core/http/endpoints/localai/audio_transform.go +++ b/core/http/endpoints/localai/audio_transform.go @@ -60,8 +60,45 @@ const ( // default ceiling is generous; raised here to 1 MiB to allow larger // frame_samples for backends with longer hops. audioTransformWSReadLimit = 1 << 20 + + // minAudioTransformSampleRate / maxAudioTransformSampleRate bound the + // caller-supplied `sample_rate` form field. + // + // THIS IS A RESOURCE BOUND, not a taste judgement. The value is + // interpolated straight into ffmpeg's -ar by utils.AudioResample, and + // ffmpeg accepts absurd rates happily: -ar 999999999 on a one second clip + // writes a 3.9 GB WAV and exits 0, into a GeneratedContentDir that nothing + // sweeps, and a separation request repeats that for every stem + // (see convertStems). At the other end -ar 1 writes a ZERO byte file, also + // exit 0, which was then served to the caller as the transformed audio. + // + // 8000 is the lowest rate any telephony codec uses and the lowest anything + // here is trained on; 192000 is the highest rate consumer audio hardware + // and the WAV container are routinely used at, and is already 4x the + // 48 kHz every model in the gallery produces. `sample_rate` unset (0) still + // means "leave the backend's own rate alone" and skips the check. + minAudioTransformSampleRate = 8000 + maxAudioTransformSampleRate = 192000 ) +// validateAudioTransformSampleRate returns an HTTP 400 for a requested output +// rate outside [minAudioTransformSampleRate, maxAudioTransformSampleRate]. +// Zero means unset and is always accepted. +// +// Split out from the handler so the bound is testable without a model, a +// backend or a multipart body: the handler rejects before it touches disk. +func validateAudioTransformSampleRate(sampleRate int) error { + if sampleRate == 0 { + return nil + } + if sampleRate < minAudioTransformSampleRate || sampleRate > maxAudioTransformSampleRate { + return echo.NewHTTPError(http.StatusBadRequest, fmt.Sprintf( + "sample_rate must be between %d and %d Hz (got %d)", + minAudioTransformSampleRate, maxAudioTransformSampleRate, sampleRate)) + } + return nil +} + // AudioTransformEndpoint implements the batch audio-transform API. Accepts a // multipart/form-data request with `audio` (required) and an optional // `reference` file. Backend-specific tuning is forwarded via repeated @@ -77,7 +114,7 @@ const ( // @Param audio formData file true "primary input audio file" // @Param reference formData file false "auxiliary reference audio (loopback for AEC, target voice for conversion, etc.)" // @Param response_format formData string false "wav | mp3 | ogg | flac" -// @Param sample_rate formData integer false "desired output sample rate" +// @Param sample_rate formData integer false "desired output sample rate in Hz; omit for the backend's own rate, otherwise 8000-192000" // @Success 200 {string} binary "transformed audio file" // @Router /audio/transformations [post] // @Router /audio/transform [post] @@ -95,6 +132,12 @@ func AudioTransformEndpoint(cl *config.ModelConfigLoader, ml *model.ModelLoader, xlog.Debug("LocalAI Audio Transform Request received", "model", input.Model) + // Before the temp dir and before the model is touched: a rejected rate + // must not cost an upload, a decode or an inference. + if err := validateAudioTransformSampleRate(input.SampleRate); err != nil { + return err + } + audioFile, err := c.FormFile("audio") if err != nil { return echo.NewHTTPError(http.StatusBadRequest, "missing required 'audio' file field") @@ -450,7 +493,17 @@ func saveMultipartFileAsWAV(fh *multipart.FileHeader, dir, name string, foldToMo } defer func() { _ = f.Close() }() - raw := filepath.Join(dir, "raw-"+path.Base(fh.Filename)) + // `name` prefixes the raw copy too, and that is not cosmetic. Both parts of + // one request land in the SAME dir, and a client that uploads + // `-F audio=@mic/clip.wav -F reference=@loopback/clip.wav` sends the same + // BASENAME twice. Without the prefix both parts write "raw-clip.wav", and + // because utils.AudioToWavPreservingShape hardlinks a WAV that is already + // PCM16 rather than copying it, audio.wav and raw-clip.wav are one inode: + // the reference part's os.Create then truncates the audio the caller + // uploaded first and refills it with the reference. The request still + // returns 200, with mic == reference, which for AEC means the canceller + // nulls everything and hands back near-silence. + raw := filepath.Join(dir, name+"-raw-"+path.Base(fh.Filename)) out, err := os.Create(raw) if err != nil { return "", err diff --git a/core/http/endpoints/localai/audio_transform_internal_test.go b/core/http/endpoints/localai/audio_transform_internal_test.go new file mode 100644 index 000000000..64a6becb3 --- /dev/null +++ b/core/http/endpoints/localai/audio_transform_internal_test.go @@ -0,0 +1,197 @@ +package localai + +import ( + "bytes" + "encoding/binary" + "mime/multipart" + "net/http" + "os" + "path/filepath" + + "github.com/labstack/echo/v4" + laudio "github.com/mudler/LocalAI/pkg/audio" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +// pcm16WAV renders a 16-bit PCM WAV carrying a ramp seeded from `seed`, so two +// fixtures of the same shape still differ byte for byte. +func pcm16WAV(sampleRate uint32, channels uint16, frames int, seed int) []byte { + const bitsPerSample = uint16(16) + blockAlign := channels * (bitsPerSample / 8) + total := frames * int(channels) + dataSize := uint32(total) * uint32(bitsPerSample/8) + + hdr := laudio.WAVHeader{ + ChunkID: [4]byte{'R', 'I', 'F', 'F'}, + ChunkSize: 36 + dataSize, + Format: [4]byte{'W', 'A', 'V', 'E'}, + Subchunk1ID: [4]byte{'f', 'm', 't', ' '}, + Subchunk1Size: 16, + AudioFormat: 1, + NumChannels: channels, + SampleRate: sampleRate, + ByteRate: sampleRate * uint32(blockAlign), + BlockAlign: blockAlign, + BitsPerSample: bitsPerSample, + Subchunk2ID: [4]byte{'d', 'a', 't', 'a'}, + Subchunk2Size: dataSize, + } + buf := &bytes.Buffer{} + Expect(binary.Write(buf, binary.LittleEndian, &hdr)).To(Succeed()) + for i := 0; i < total; i++ { + Expect(binary.Write(buf, binary.LittleEndian, int16(seed+7*(i%100)))).To(Succeed()) + } + return buf.Bytes() +} + +// multipartFiles builds a real multipart form and parses it back, which is the +// only honest way to get *multipart.FileHeader values shaped exactly like the +// ones echo hands the endpoint. `parts` maps the form field name to the client +// side FILE NAME, which is the part of the request this fixture is about. +func multipartFiles(parts map[string]string, bodies map[string][]byte) map[string]*multipart.FileHeader { + body := &bytes.Buffer{} + writer := multipart.NewWriter(body) + for field, filename := range parts { + part, err := writer.CreateFormFile(field, filename) + Expect(err).ToNot(HaveOccurred()) + _, err = part.Write(bodies[field]) + Expect(err).ToNot(HaveOccurred()) + } + Expect(writer.Close()).To(Succeed()) + + reader := multipart.NewReader(bytes.NewReader(body.Bytes()), writer.Boundary()) + form, err := reader.ReadForm(1 << 20) + Expect(err).ToNot(HaveOccurred()) + + headers := map[string]*multipart.FileHeader{} + for field := range parts { + Expect(form.File[field]).To(HaveLen(1)) + headers[field] = form.File[field][0] + } + return headers +} + +// `sample_rate` is interpolated straight into ffmpeg's -ar by +// utils.AudioResample. Before this bound existed the field was accepted +// unchanged, and -ar 999999999 on a one second clip writes a 3.9 GB WAV and +// exits 0, into a GeneratedContentDir nothing sweeps; a separation repeats that +// per stem. At the other end -ar 1 writes a header with no audio in it, also +// exit 0, which was then served as the response body. +var _ = Describe("audio transform sample_rate bounds", func() { + status := func(err error) int { + Expect(err).To(HaveOccurred()) + httpErr, ok := err.(*echo.HTTPError) + Expect(ok).To(BeTrue(), "the rejection has to be an HTTP status, not a 500") + return httpErr.Code + } + + It("accepts an unset sample_rate, which means the backend's own rate", func() { + Expect(validateAudioTransformSampleRate(0)).To(Succeed()) + }) + + It("accepts both ends of the supported range", func() { + Expect(validateAudioTransformSampleRate(minAudioTransformSampleRate)).To(Succeed()) + Expect(validateAudioTransformSampleRate(maxAudioTransformSampleRate)).To(Succeed()) + Expect(validateAudioTransformSampleRate(48000)).To(Succeed()) + }) + + It("rejects a rate below the lower bound with a 400", func() { + Expect(status(validateAudioTransformSampleRate(minAudioTransformSampleRate - 1))). + To(Equal(http.StatusBadRequest)) + Expect(status(validateAudioTransformSampleRate(1))).To(Equal(http.StatusBadRequest)) + Expect(status(validateAudioTransformSampleRate(-1))).To(Equal(http.StatusBadRequest)) + }) + + It("rejects a rate above the upper bound with a 400", func() { + Expect(status(validateAudioTransformSampleRate(maxAudioTransformSampleRate + 1))). + To(Equal(http.StatusBadRequest)) + Expect(status(validateAudioTransformSampleRate(999999999))).To(Equal(http.StatusBadRequest)) + }) + + It("names both bounds and the offending value in the message", func() { + err := validateAudioTransformSampleRate(999999999) + Expect(err).To(HaveOccurred()) + Expect(err.(*echo.HTTPError).Message).To(ContainSubstring("8000")) + Expect(err.(*echo.HTTPError).Message).To(ContainSubstring("192000")) + Expect(err.(*echo.HTTPError).Message).To(ContainSubstring("999999999")) + }) +}) + +// Both uploads land in the SAME temp dir, and a client is free to send the same +// BASENAME on both parts (`-F audio=@mic/clip.wav -F reference=@loopback/clip.wav` +// is an ordinary request, not an attack). The raw copy is therefore named after +// the FORM FIELD as well as the file: without that both parts wrote +// "raw-clip.wav", and because AudioToWavPreservingShape hardlinks a WAV that is +// already PCM16 rather than copying it, the reference part's os.Create +// truncated the very inode audio.wav pointed at. The request still returned +// 200, with the primary input replaced by the reference. +var _ = Describe("saveMultipartFileAsWAV", func() { + var dir string + + BeforeEach(func() { + dir = GinkgoT().TempDir() + }) + + It("keeps the two parts apart when they share a filename", func() { + audioBody := pcm16WAV(16000, 1, 1600, 1000) + referenceBody := pcm16WAV(16000, 1, 1600, -1000) + Expect(audioBody).ToNot(Equal(referenceBody), "the fixtures must be distinguishable") + + headers := multipartFiles( + map[string]string{"audio": "clip.wav", "reference": "clip.wav"}, + map[string][]byte{"audio": audioBody, "reference": referenceBody}, + ) + + audioPath, err := saveMultipartFileAsWAV(headers["audio"], dir, "audio", false) + Expect(err).ToNot(HaveOccurred()) + referencePath, err := saveMultipartFileAsWAV(headers["reference"], dir, "reference", false) + Expect(err).ToNot(HaveOccurred()) + + onDiskAudio, err := os.ReadFile(audioPath) + Expect(err).ToNot(HaveOccurred()) + onDiskReference, err := os.ReadFile(referencePath) + Expect(err).ToNot(HaveOccurred()) + + Expect(onDiskAudio).To(Equal(audioBody), + "the primary input must still be the audio part after the reference is written") + Expect(onDiskReference).To(Equal(referenceBody)) + Expect(onDiskAudio).ToNot(Equal(onDiskReference), + "identical mic and reference makes an echo canceller null everything and return silence with a 200") + }) + + It("gives the two parts distinct raw copies", func() { + headers := multipartFiles( + map[string]string{"audio": "clip.wav", "reference": "clip.wav"}, + map[string][]byte{ + "audio": pcm16WAV(16000, 1, 800, 500), + "reference": pcm16WAV(16000, 1, 800, -500), + }, + ) + + _, err := saveMultipartFileAsWAV(headers["audio"], dir, "audio", false) + Expect(err).ToNot(HaveOccurred()) + _, err = saveMultipartFileAsWAV(headers["reference"], dir, "reference", false) + Expect(err).ToNot(HaveOccurred()) + + Expect(filepath.Join(dir, "audio-raw-clip.wav")).To(BeAnExistingFile()) + Expect(filepath.Join(dir, "reference-raw-clip.wav")).To(BeAnExistingFile()) + }) + + It("still works when the two parts have different filenames", func() { + headers := multipartFiles( + map[string]string{"audio": "mic.wav", "reference": "loopback.wav"}, + map[string][]byte{ + "audio": pcm16WAV(16000, 1, 400, 300), + "reference": pcm16WAV(16000, 1, 400, -300), + }, + ) + + audioPath, err := saveMultipartFileAsWAV(headers["audio"], dir, "audio", false) + Expect(err).ToNot(HaveOccurred()) + referencePath, err := saveMultipartFileAsWAV(headers["reference"], dir, "reference", false) + Expect(err).ToNot(HaveOccurred()) + Expect(audioPath).ToNot(Equal(referencePath)) + }) +}) diff --git a/core/schema/audio_transform.go b/core/schema/audio_transform.go index 73e9099f1..006389a07 100644 --- a/core/schema/audio_transform.go +++ b/core/schema/audio_transform.go @@ -7,17 +7,27 @@ package schema // `params[]=` form fields, collected into a generic map so // the schema doesn't bake in any one transform's vocabulary. // -// The `form` tags on the two snake_case fields are load bearing. This request -// arrives as multipart/form-data, and echo's binder falls back to the FIELD -// NAME when a form tag is missing, matching it only case-insensitively: -// "Format" never matches "response_format" and "SampleRate" never matches -// "sample_rate", so both documented form fields were silently ignored until -// these tags were added. `model` worked all along because its field name and -// its form key differ only in case. +// The `form` tags on the two snake_case fields are load bearing, and NOT for +// the reason first written here. echo's binder has no field-name fallback at +// all: bindData is documented as binding "ONLY fields in destination struct +// that have EXPLICIT tag", and a field whose tag is empty and whose kind is not +// a struct is skipped outright with a `continue` +// (labstack/echo/v4@v4.15.1 bind.go). So an untagged Format or SampleRate is +// not matched loosely, it is not looked at, which is why both documented form +// fields were silently ignored until these tags were added. +// +// `model` is not the counter-example it looks like. It is not bound by the +// binder either: it arrives because setModelNameFromRequest asks for it by +// name, c.FormValue("model") (core/http/middleware/request.go). +// +// SampleRate is validated in the handler, not here: it is interpolated into +// ffmpeg's -ar and an unbounded value is a disk-exhaustion hazard. See +// minAudioTransformSampleRate in +// core/http/endpoints/localai/audio_transform.go. type AudioTransformRequest struct { BasicModelRequest Format string `json:"response_format,omitempty" yaml:"response_format,omitempty" form:"response_format"` // wav | mp3 | ogg | flac - SampleRate int `json:"sample_rate,omitempty" yaml:"sample_rate,omitempty" form:"sample_rate"` // desired output sample rate; 0 = backend default + SampleRate int `json:"sample_rate,omitempty" yaml:"sample_rate,omitempty" form:"sample_rate"` // desired output sample rate; 0 = backend default, otherwise 8000..192000 Params map[string]string `json:"params,omitempty" yaml:"params,omitempty"` // backend-specific tuning } diff --git a/core/schema/elevenlabs.go b/core/schema/elevenlabs.go index 7818a877c..f8790a333 100644 --- a/core/schema/elevenlabs.go +++ b/core/schema/elevenlabs.go @@ -6,6 +6,24 @@ type ElevenLabsTTSRequest struct { LanguageCode string `json:"language_code" yaml:"language_code"` } +// DO NOT ADD A SOURCE-AUDIO FIELD TO THIS STRUCT. +// +// SoundGenerationRequest.src on the wire is the input clip for the editing +// routes, and setting it on a stable_audio model CORRUPTS THE HEAP AND ABORTS +// THE PROCESS in the audio-cpp backend's pinned audio.cpp: "free(): invalid +// size" / "munmap_chunk(): invalid pointer", SIGABRT, backend gone. It is +// upstream's bug, not LocalAI's, and it was attributed rather than assumed: +// upstream's own audiocpp_cli, built from the same checkout, aborts identically +// with --audio at exit 134 and completes cleanly without it. The long version +// is on the SoundGeneration handler in backend/cpp/audio-cpp/grpc-server.cpp. +// +// What keeps that off the network today is exactly this omission. The +// ElevenLabs endpoint (core/http/endpoints/elevenlabs/soundgeneration.go) +// passes nil for the source file because this struct has nowhere to put one, +// so the only caller that can set src is core/cli/soundgeneration.go, a local +// CLI. Adding the field here turns a local CLI abort into remotely reachable +// heap corruption with fully attacker-influenced input. Wait for the pin to +// move past the fix. type ElevenLabsSoundGenerationRequest struct { Text string `json:"text" yaml:"text"` ModelID string `json:"model_id" yaml:"model_id"` diff --git a/docs/content/features/audio-transform.md b/docs/content/features/audio-transform.md index 4f2a32dc4..6b32a2fe6 100644 --- a/docs/content/features/audio-transform.md +++ b/docs/content/features/audio-transform.md @@ -53,7 +53,7 @@ form-data, returns audio bytes. | `audio` | file | yes | Primary input audio | | `reference` | file | no | Optional auxiliary signal | | `response_format` | string | no | `wav` (default), `mp3`, `ogg`, `flac` | -| `sample_rate` | int | no | Desired output sample rate | +| `sample_rate` | int | no | Desired output sample rate in Hz. Omit it for the backend's own rate; otherwise it must be between 8000 and 192000, and a value outside that range is refused with a 400 | | `params[]` | string | no | Repeated; forwarded to backend | | `params[stem]` | string | no | Multi-output transforms only; picks which named output the body carries (see [stems](#multi-output-transforms-source-separation-stems)) | diff --git a/pkg/utils/ffmpeg.go b/pkg/utils/ffmpeg.go index 112f57cbe..aae4d0592 100644 --- a/pkg/utils/ffmpeg.go +++ b/pkg/utils/ffmpeg.go @@ -1,6 +1,7 @@ package utils import ( + "encoding/binary" "fmt" "io" "os" @@ -153,9 +154,83 @@ func AudioResample(src string, sampleRate int) (string, error) { if err != nil { return "", fmt.Errorf("error resampling audio: %w out: %s", err, out) } + // A successful exit is not enough. For a rate its resampler collapses to + // nothing (-ar 1 on a one second clip is the reproducer) ffmpeg writes a + // bare 78-byte header, no audio at all, and exits 0 - and every caller here + // hands that straight back to the client as the audio it asked for. A 200 + // carrying no audio is the silent wrong answer; callers already handle an + // error from this function. + info, statErr := os.Stat(dst) + if statErr != nil { + return "", fmt.Errorf("error resampling audio: no output at %s: %w out: %s", dst, statErr, out) + } + if info.Size() == 0 { + return "", fmt.Errorf("error resampling audio: ffmpeg produced an empty file at %d Hz out: %s", sampleRate, out) + } + if wavAudioBytes(dst) == 0 { + return "", fmt.Errorf("error resampling audio: ffmpeg produced a WAV with no audio at %d Hz out: %s", sampleRate, out) + } return dst, nil } +// wavAudioBytes reports how many bytes of audio a RIFF/WAVE file actually +// carries on disk. Returns -1 for anything it cannot walk as RIFF/WAVE, so a +// caller can tell "no audio" apart from "not a file I can judge". +// +// The declared data-chunk size is deliberately clamped to the bytes that are +// really present, and that clamp is the entire point. When ffmpeg's resampler +// collapses a signal to nothing it still writes a header CLAIMING 70 data +// bytes and then writes none of them, so the file is 78 bytes of header and +// go-audio's decoder reports a 35 second duration for it: both the declared +// size and the parsed duration say "audio", and only the file length says the +// truth. A plain size check does not work either, since the file is not empty. +func wavAudioBytes(path string) int64 { + f, err := os.Open(path) + if err != nil { + return -1 + } + defer func() { _ = f.Close() }() + + info, err := f.Stat() + if err != nil { + return -1 + } + var riffHeader [12]byte + if _, err := io.ReadFull(f, riffHeader[:]); err != nil { + return -1 + } + if string(riffHeader[0:4]) != "RIFF" || string(riffHeader[8:12]) != "WAVE" { + return -1 + } + + offset := int64(len(riffHeader)) + var chunkHeader [8]byte + for { + if _, err := f.ReadAt(chunkHeader[:], offset); err != nil { + // Ran off the end without meeting a data chunk: no audio. + return 0 + } + declared := int64(binary.LittleEndian.Uint32(chunkHeader[4:8])) + payload := offset + int64(len(chunkHeader)) + if string(chunkHeader[0:4]) == "data" { + available := info.Size() - payload + if available < 0 { + available = 0 + } + if declared < available { + return declared + } + return available + } + offset = payload + declared + // RIFF chunks are word aligned; an odd-sized one is followed by a pad + // byte that is not part of the next chunk's header. + if declared%2 == 1 { + offset++ + } + } +} + // AudioConvert converts generated wav file from tts to other output formats. // TODO: handle pcm to have 100% parity of supported format from OpenAI func AudioConvert(src string, format string) (string, error) { diff --git a/pkg/utils/ffmpeg_shape_test.go b/pkg/utils/ffmpeg_shape_test.go index ae4dc5afe..c21aa05dc 100644 --- a/pkg/utils/ffmpeg_shape_test.go +++ b/pkg/utils/ffmpeg_shape_test.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/binary" "os" + "os/exec" "path/filepath" laudio "github.com/mudler/LocalAI/pkg/audio" @@ -205,3 +206,59 @@ var _ = Describe("utils/ffmpeg shape-preserving conversion", func() { Expect(src).To(BeAnExistingFile(), "the upload must survive the conversion") }) }) + +// AudioResample interpolates the caller's sample rate straight into ffmpeg's +// -ar, and a successful exit does not mean a usable file came out. Measured +// against ffmpeg 7 on a one second 16 kHz clip: -ar 1 exits 0 and writes 78 +// bytes, a bare header with no audio behind it, whose declared data size still +// claims 70 bytes so go-audio parses it as a 35 SECOND file. Both the exit +// status and the parsed duration say "audio"; only the bytes on disk say +// otherwise. Before this guard that file was returned to the caller as the +// transformed audio, with a 200. +var _ = Describe("utils/ffmpeg resample output", func() { + var dir string + + BeforeEach(func() { + dir = GinkgoT().TempDir() + if _, err := exec.LookPath("ffmpeg"); err != nil { + Skip("ffmpeg is not installed") + } + }) + + It("refuses an output ffmpeg wrote with no audio in it", func() { + src := filepath.Join(dir, "speech.wav") + writeWAV(src, 16000, 1, 16000) + + dst, err := AudioResample(src, 1) + + Expect(err).To(HaveOccurred(), + "a header with no audio must not be handed back as the transformed audio") + Expect(err.Error()).To(ContainSubstring("no audio")) + Expect(dst).To(BeEmpty()) + }) + + It("resamples normally at a rate inside the endpoint's bounds", func() { + src := filepath.Join(dir, "speech.wav") + writeWAV(src, 16000, 1, 16000) + + dst, err := AudioResample(src, 8000) + + Expect(err).ToNot(HaveOccurred()) + rate, channels, _ := readShape(dst) + Expect(rate).To(Equal(uint32(8000))) + Expect(channels).To(Equal(uint16(1))) + info, err := os.Stat(dst) + Expect(err).ToNot(HaveOccurred()) + Expect(info.Size()).To(BeNumerically(">", 8000), "half a second of 8 kHz mono is 8 kB of PCM") + }) + + It("is a no-op for an unset rate", func() { + src := filepath.Join(dir, "speech.wav") + writeWAV(src, 16000, 1, 160) + + dst, err := AudioResample(src, 0) + + Expect(err).ToNot(HaveOccurred()) + Expect(dst).To(Equal(src)) + }) +})