mirror of
https://github.com/mudler/LocalAI.git
synced 2026-07-31 02:18:50 -04:00
fix(audio-transform): reject extensible WAV from the passthrough, escape stem URLs, convert stems with dst
Four fixes from the second review, plus one bug they made visible. isPCM16Wav tested only the bit depth, and go-audio's IsValidFile never looks at the format tag, so a 16-bit WAVE_FORMAT_EXTENSIBLE (0xFFFE) upload was passed through untouched where the old fold would have transcoded it. audio.cpp's WAV reader accepts 16-bit only when the tag is 1, so such a file died with "unsupported WAV encoding". Extensible is what many DAWs and Windows tools write and music files are this endpoint's new headline input, so it is a first-contact failure rather than a corner. The check now requires tag 1, with a spec that fails against the old implementation. Stem URLs are percent-escaped. A stem name is the model's own string and legally contains a space, a '#', a '?' or a '%'; an unescaped '#' truncates the URL before the request is even sent. The name field keeps the raw name. sample_rate and response_format are applied to the stems as well as to dst. Applying beat documenting: dst IS one of those stems, so leaving them alone broke the "dst duplicates the selected stem" invariant the whole design rests on, and both conversions are no-ops when unset. A stem whose conversion fails is dropped from the header rather than advertised in the wrong shape. Verifying that turned up why it had never been noticed: the two fields were never bound at all. The request arrives as multipart/form-data and echo's binder falls back to the FIELD NAME without a form tag, matching only case-insensitively, so "SampleRate" never matched "sample_rate" and "Format" never matched "response_format". Both were documented in the endpoint table and silently ignored. Two form tags fix it, and with them the conversion is observable end to end. Docs: audio-transform.md now documents what LocalAI does to an upload before the backend sees it, which backend gets the 16 kHz mono fold and why, params[stem], and the X-Audio-Stems header with a worked example. Also records the known limitation that the fold lookup is on the bare backend name, so pinned variants (vulkan-localvqe) do not match, and points at IsLlamaCppBackend as the suffix-tolerant precedent. Assisted-by: Claude:claude-opus-5 [Claude Code] Signed-off-by: Ettore Di Giacinto <mudler@localai.io>
This commit is contained in:
@@ -743,6 +743,12 @@ func GetBackendCapability(backend string) *BackendCapability {
|
||||
// backend gets its upload unchanged, so a model that needs the file intact
|
||||
// (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.
|
||||
func AudioTransformRequiresMono16kInput(backend string) bool {
|
||||
capability := GetBackendCapability(backend)
|
||||
return capability != nil && capability.AudioTransformInputMono16k
|
||||
|
||||
@@ -8,6 +8,7 @@ import (
|
||||
"io"
|
||||
"mime/multipart"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"os"
|
||||
"path"
|
||||
"path/filepath"
|
||||
@@ -177,7 +178,13 @@ func AudioTransformEndpoint(cl *config.ModelConfigLoader, ml *model.ModelLoader,
|
||||
// stems it did not ask for without paying for another separation.
|
||||
// Header rather than body for the same reason the input URLs are
|
||||
// headers: the body is the audio itself.
|
||||
if header := stemsHeader(out.Stems); header != "" {
|
||||
//
|
||||
// Converted on the same terms as dst, because dst IS one of these stems
|
||||
// and the caller is entitled to expect the set to match. Skipping this
|
||||
// left a caller who asked for 48 kHz mp3 with an mp3 body and four
|
||||
// 44.1 kHz WAV siblings, one of which was supposed to be the same
|
||||
// recording as the body.
|
||||
if header := stemsHeader(convertStems(out.Stems, input.SampleRate, input.Format)); header != "" {
|
||||
c.Response().Header().Set(echo.HeaderAccessControlExposeHeaders, exposedAudioTransformHeaders)
|
||||
c.Response().Header().Set("X-Audio-Stems", header)
|
||||
}
|
||||
@@ -341,6 +348,44 @@ func buildConfigRequest(fmt_ proto.AudioTransformStreamConfig_SampleFormat, ctrl
|
||||
// this endpoint sets. A browser cannot read any of them without it.
|
||||
const exposedAudioTransformHeaders = "X-Audio-Input-Url, X-Audio-Reference-Url, X-Audio-Stems"
|
||||
|
||||
// convertStems applies the request's sample_rate and response_format to every
|
||||
// stem, so the whole set stays in the shape the caller asked for and dst keeps
|
||||
// duplicating the stem it was selected from. Both conversions are no-ops when
|
||||
// unset, so an ordinary request pays nothing.
|
||||
//
|
||||
// A stem whose conversion fails is DROPPED from the list rather than reported
|
||||
// at its original rate or format: advertising a URL whose file is not in the
|
||||
// shape the caller asked for is the silent wrong answer this whole design is
|
||||
// trying to avoid, and the caller still receives the audio it actually asked
|
||||
// for in the body. The failure is logged, since the file is on disk either way.
|
||||
func convertStems(stems []backend.AudioTransformStem, sampleRate int, format string) []backend.AudioTransformStem {
|
||||
if len(stems) == 0 {
|
||||
return nil
|
||||
}
|
||||
converted := make([]backend.AudioTransformStem, 0, len(stems))
|
||||
for _, stem := range stems {
|
||||
path := stem.Dst
|
||||
var err error
|
||||
if sampleRate > 0 {
|
||||
path, err = utils.AudioResample(path, sampleRate)
|
||||
if err != nil {
|
||||
xlog.Warn("audio_transform: cannot resample stem", "stem", stem.Name, "error", err)
|
||||
continue
|
||||
}
|
||||
}
|
||||
path, err = utils.AudioConvert(path, format)
|
||||
if err != nil {
|
||||
xlog.Warn("audio_transform: cannot convert stem", "stem", stem.Name, "error", err)
|
||||
continue
|
||||
}
|
||||
converted = append(converted, backend.AudioTransformStem{Name: stem.Name, Dst: path})
|
||||
}
|
||||
if len(converted) == 0 {
|
||||
return nil
|
||||
}
|
||||
return converted
|
||||
}
|
||||
|
||||
// stemsHeader renders the extra named outputs as a compact JSON array for the
|
||||
// X-Audio-Stems response header:
|
||||
//
|
||||
@@ -365,7 +410,15 @@ func stemsHeader(stems []backend.AudioTransformStem) string {
|
||||
if name == "" || name == "." || name == string(filepath.Separator) {
|
||||
continue
|
||||
}
|
||||
entries = append(entries, stemEntry{Name: stem.Name, URL: "/generated-audio/" + name})
|
||||
// PathEscape, because the file name carries the MODEL'S stem name and a
|
||||
// stem name legally contains a space (there is a spec for "lead
|
||||
// vocals"), and '#', '?' and '%' are legal too. An unescaped '#' would
|
||||
// truncate the URL in the client before it ever reached the server.
|
||||
// The `name` field keeps the raw stem name; only the URL is escaped.
|
||||
entries = append(entries, stemEntry{
|
||||
Name: stem.Name,
|
||||
URL: "/generated-audio/" + url.PathEscape(name),
|
||||
})
|
||||
}
|
||||
if len(entries) == 0 {
|
||||
return ""
|
||||
|
||||
@@ -6,11 +6,19 @@ package schema
|
||||
// conversion, etc.) is optional. Backend-specific tuning lives in the
|
||||
// `params[<key>]=<value>` 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.
|
||||
type AudioTransformRequest struct {
|
||||
BasicModelRequest
|
||||
Format string `json:"response_format,omitempty" yaml:"response_format,omitempty"` // wav | mp3 | ogg | flac
|
||||
SampleRate int `json:"sample_rate,omitempty" yaml:"sample_rate,omitempty"` // desired output sample rate; 0 = backend default
|
||||
Params map[string]string `json:"params,omitempty" yaml:"params,omitempty"` // backend-specific tuning
|
||||
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
|
||||
Params map[string]string `json:"params,omitempty" yaml:"params,omitempty"` // backend-specific tuning
|
||||
}
|
||||
|
||||
// AudioTransformStreamControl is the JSON envelope used on the
|
||||
|
||||
@@ -49,6 +49,7 @@ form-data, returns audio bytes.
|
||||
| `response_format` | string | no | `wav` (default), `mp3`, `ogg`, `flac` |
|
||||
| `sample_rate` | int | no | Desired output sample rate |
|
||||
| `params[<key>]` | 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)) |
|
||||
|
||||
First install an audio-transform model from the gallery (the examples below use `localvqe-v1.3-4.8m`):
|
||||
|
||||
@@ -71,6 +72,71 @@ curl -X POST http://localhost:8080/audio/transformations \
|
||||
When `reference` is omitted, LocalVQE zero-fills the reference channel and
|
||||
the operation reduces to noise suppression + dereverberation.
|
||||
|
||||
### What LocalAI does to your upload before the backend sees it
|
||||
|
||||
By default, **nothing**: the file reaches the backend at its own sample rate and
|
||||
its own channel count. A WAV already carrying plain 16-bit PCM is passed through
|
||||
byte for byte; any other container or encoding is transcoded to 16-bit PCM WAV
|
||||
with the rate and the channel layout kept.
|
||||
|
||||
The exception is a backend that declares it needs a fixed input shape.
|
||||
**LocalVQE** does: its echo cancellation is trained on 16 kHz mono and needs the
|
||||
primary input and the reference in the same shape, so uploads for it are folded
|
||||
to 16 kHz mono s16 with ffmpeg. The declaration is
|
||||
`BackendCapability.AudioTransformInputMono16k` in `core/config`, and `localvqe`
|
||||
is currently the only backend that sets it.
|
||||
|
||||
This matters for anything that is not speech enhancement. Source separation
|
||||
models refuse any sample rate but their checkpoint's own (44.1 kHz for every
|
||||
published htdemucs and mel_band_roformer checkpoint) and rely on the stereo
|
||||
image to tell a centred vocal from a wide mix, so a 16 kHz mono downmix would
|
||||
remove both the format they accept and the cue they work from.
|
||||
|
||||
### Multi-output transforms (source separation stems)
|
||||
|
||||
Some transforms produce **several** named outputs from one run: htdemucs yields
|
||||
`drums`, `bass`, `other` and `vocals` in a single pass. The response body can
|
||||
carry only one file, so:
|
||||
|
||||
- The backend runs **once** and writes every stem beside the main output.
|
||||
- `params[stem]=<name>` chooses which one the body carries. Without it the
|
||||
default is `vocals` when the model has one, and the model's first output
|
||||
otherwise. An unknown stem name is refused with an error listing the real
|
||||
ones, never silently substituted.
|
||||
- Every stem, including the one in the body, is named in the **`X-Audio-Stems`**
|
||||
response header, a compact JSON array:
|
||||
|
||||
```
|
||||
X-Audio-Stems: [{"name":"drums","url":"/generated-audio/transform.drums.wav"},
|
||||
{"name":"bass","url":"/generated-audio/transform.bass.wav"},
|
||||
{"name":"other","url":"/generated-audio/transform.other.wav"},
|
||||
{"name":"vocals","url":"/generated-audio/transform.vocals.wav"}]
|
||||
```
|
||||
|
||||
Fetch any of those URLs to get the other stems without paying for a second
|
||||
separation. `sample_rate` and `response_format` are applied to the stems as well
|
||||
as to the body, so the whole set stays in the shape you asked for. The header is
|
||||
listed in `Access-Control-Expose-Headers`, so browser clients can read it.
|
||||
|
||||
Single-output transforms (echo cancellation, voice conversion) do not set the
|
||||
header at all, and `params[stem]` against such a model is refused rather than
|
||||
ignored.
|
||||
|
||||
```bash
|
||||
# isolate the vocals (the default), then see where the other stems went
|
||||
curl -sS -D headers.txt -X POST http://localhost:8080/audio/transformations \
|
||||
-F model=htdemucs -F audio=@song.wav -o vocals.wav
|
||||
grep -i '^x-audio-stems' headers.txt
|
||||
|
||||
# or ask for a specific stem in the body
|
||||
curl -sS -X POST http://localhost:8080/audio/transformations \
|
||||
-F model=htdemucs -F audio=@song.wav -F 'params[stem]=drums' -o drums.wav
|
||||
```
|
||||
|
||||
The stems live in the generated-content directory beside the main output and are
|
||||
served from `/generated-audio/`. Like every other generated artifact, they are
|
||||
not swept automatically.
|
||||
|
||||
## Streaming endpoint
|
||||
|
||||
`GET /audio/transformations/stream` - bidirectional WebSocket. The first
|
||||
|
||||
@@ -103,9 +103,18 @@ func convertWithFFmpeg(src, dst string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// isPCM16Wav returns true when src is a valid 16-bit PCM WAV at ANY sample rate
|
||||
// and ANY channel count. Deliberately weaker than isTargetWav: it is the "no
|
||||
// conversion needed" test for callers that want the file's own shape kept.
|
||||
// isPCM16Wav returns true when src is a valid WAV carrying PLAIN 16-bit PCM, at
|
||||
// ANY sample rate and ANY channel count. Weaker than isTargetWav on rate and
|
||||
// channels; exactly as strict on the encoding, and deliberately so.
|
||||
//
|
||||
// The format tag is checked and that is not a formality. go-audio's
|
||||
// IsValidFile() never inspects it, so a BitDepth == 16 test on its own also
|
||||
// accepts WAVE_FORMAT_EXTENSIBLE (0xFFFE), which is what many DAWs and Windows
|
||||
// tools write. Consumers are not as relaxed: audio.cpp's WAV reader accepts
|
||||
// 16-bit only when the tag is 1 and otherwise refuses with "unsupported WAV
|
||||
// encoding". Passing such a file through unchanged would trade a transcode for
|
||||
// a failed request, and music files, which are the input this passthrough
|
||||
// exists for, are exactly where extensible turns up.
|
||||
func isPCM16Wav(src string) bool {
|
||||
f, err := os.Open(src)
|
||||
if err != nil {
|
||||
@@ -117,7 +126,7 @@ func isPCM16Wav(src string) bool {
|
||||
if !dec.IsValidFile() {
|
||||
return false
|
||||
}
|
||||
return dec.BitDepth == 16
|
||||
return dec.BitDepth == 16 && dec.WavAudioFormat == 1
|
||||
}
|
||||
|
||||
// convertWithFFmpegPreservingShape transcodes to 16-bit PCM WAV with no -ar and
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package utils_test
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"encoding/binary"
|
||||
"os"
|
||||
"path/filepath"
|
||||
@@ -56,6 +57,54 @@ func readShape(path string) (uint32, uint16, uint16) {
|
||||
return hdr.SampleRate, hdr.NumChannels, hdr.BitsPerSample
|
||||
}
|
||||
|
||||
// readFormatTag reports wFormatTag, which sits at the fixed offset 20 of a
|
||||
// RIFF/WAVE header. 1 is plain PCM; 0xFFFE is WAVE_FORMAT_EXTENSIBLE.
|
||||
func readFormatTag(path string) uint16 {
|
||||
raw, err := os.ReadFile(path)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(len(raw)).To(BeNumerically(">=", 22))
|
||||
return binary.LittleEndian.Uint16(raw[20:22])
|
||||
}
|
||||
|
||||
// writeExtensibleWAV writes a real WAVE_FORMAT_EXTENSIBLE file: a 40-byte fmt
|
||||
// chunk with tag 0xFFFE, cbSize 22 and the KSDATAFORMAT_SUBTYPE_PCM GUID. Many
|
||||
// DAWs and Windows tools emit exactly this, and the samples inside are ordinary
|
||||
// 16-bit PCM, which is why a bit-depth-only check waves it through.
|
||||
func writeExtensibleWAV(path string, sampleRate uint32, channels uint16, frames int) {
|
||||
const bitsPerSample = uint16(16)
|
||||
blockAlign := channels * (bitsPerSample / 8)
|
||||
dataSize := uint32(frames) * uint32(channels) * uint32(bitsPerSample/8)
|
||||
|
||||
buf := &bytes.Buffer{}
|
||||
write := func(values ...any) {
|
||||
for _, value := range values {
|
||||
Expect(binary.Write(buf, binary.LittleEndian, value)).To(Succeed())
|
||||
}
|
||||
}
|
||||
buf.WriteString("RIFF")
|
||||
write(uint32(4 + 8 + 40 + 8 + dataSize))
|
||||
buf.WriteString("WAVE")
|
||||
buf.WriteString("fmt ")
|
||||
write(uint32(40)) // fmt chunk size for extensible
|
||||
write(uint16(0xFFFE)) // wFormatTag = WAVE_FORMAT_EXTENSIBLE
|
||||
write(channels, sampleRate) // nChannels, nSamplesPerSec
|
||||
write(sampleRate * uint32(blockAlign)) // nAvgBytesPerSec
|
||||
write(blockAlign, bitsPerSample) // nBlockAlign, wBitsPerSample
|
||||
write(uint16(22), bitsPerSample) // cbSize, wValidBitsPerSample
|
||||
write(uint32(0x3)) // dwChannelMask: front left + right
|
||||
// KSDATAFORMAT_SUBTYPE_PCM {00000001-0000-0010-8000-00AA00389B71}
|
||||
buf.Write([]byte{
|
||||
0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00,
|
||||
0x80, 0x00, 0x00, 0xAA, 0x00, 0x38, 0x9B, 0x71,
|
||||
})
|
||||
buf.WriteString("data")
|
||||
write(dataSize)
|
||||
for i := 0; i < frames*int(channels); i++ {
|
||||
write(int16(200 * (i % 100)))
|
||||
}
|
||||
Expect(os.WriteFile(path, buf.Bytes(), 0o600)).To(Succeed())
|
||||
}
|
||||
|
||||
// The distinction these specs defend: /audio/transform used to fold EVERY
|
||||
// upload to 16 kHz mono, which made source separation impossible to reach
|
||||
// through the HTTP API (htdemucs refuses any rate but its checkpoint's own and
|
||||
@@ -121,6 +170,32 @@ var _ = Describe("utils/ffmpeg shape-preserving conversion", func() {
|
||||
Expect(channels).To(Equal(uint16(1)))
|
||||
})
|
||||
|
||||
// The passthrough is about the SHAPE, never about the encoding. An
|
||||
// extensible WAV carries plain 16-bit samples behind a 0xFFFE tag, and
|
||||
// audio.cpp's reader accepts 16-bit only when the tag is 1, so passing one
|
||||
// through unchanged would turn a transcode into "unsupported WAV encoding".
|
||||
It("transcodes a WAVE_FORMAT_EXTENSIBLE upload instead of passing it through", func() {
|
||||
src := filepath.Join(dir, "extensible.wav")
|
||||
dst := filepath.Join(dir, "extensible-out.wav")
|
||||
writeExtensibleWAV(src, 44100, 2, 4410)
|
||||
Expect(readFormatTag(src)).To(Equal(uint16(0xFFFE)), "the fixture really is extensible")
|
||||
|
||||
Expect(AudioToWavPreservingShape(src, dst)).To(Succeed())
|
||||
|
||||
Expect(readFormatTag(dst)).To(Equal(uint16(1)),
|
||||
"the output must be plain PCM, which is all a backend's WAV reader accepts")
|
||||
rate, channels, depth := readShape(dst)
|
||||
Expect(rate).To(Equal(uint32(44100)), "and the transcode still keeps the rate")
|
||||
Expect(channels).To(Equal(uint16(2)), "and the channels")
|
||||
Expect(depth).To(Equal(uint16(16)))
|
||||
|
||||
before, err := os.ReadFile(src)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
after, err := os.ReadFile(dst)
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(after).ToNot(Equal(before), "it was converted, not hardlinked")
|
||||
})
|
||||
|
||||
It("leaves the source file in place", func() {
|
||||
src := filepath.Join(dir, "keep.wav")
|
||||
dst := filepath.Join(dir, "keep-out.wav")
|
||||
|
||||
Reference in New Issue
Block a user