Repository navigation
fix: sync bug fixes from the WasmEdge MLX plugin - #7
Merged
Merged
Conversation
Port fixes made to plugins/wasi_nn/MLX in WasmEdge back to this repo: - whisper: stop running ffmpeg through popen()/sh. loadAudio built a command string from the audio path, so shell metacharacters in the path could run arbitrary commands and paths with spaces broke. ffmpeg now runs via posix_spawnp with an argv array (cf. WasmEdge 4742d02fba). File-path input is kept for library users. - vlm: stop generation on the model's EOS tokens instead of always running to MaxTokens. Stop ids are a per-model virtual (eosTokenIds); Gemma3 returns <eos> (1) and <end_of_turn> (106). Unlike WasmEdge's hard-coded check, mllama is not affected. - loadBytesFromFile: log and return an empty string on open failure instead of calling exit(1) (cf. WasmEdge 584437baa5). - Rename parameters and locals that shadowed members or outer names in Module, Embedding/Linear, pooling, RotatingKVCache, whisper decoding and tokenizer (cf. WasmEdge e271503856). - Pass values as spdlog format arguments instead of concatenating them into the format string (cf. WasmEdge 2b8429d303). Co-Authored-By: Claude Opus 5.5 <[email protected]>
GitHub no longer provides the macos-14 runner, so the macOS job was cancelled before it started. Co-Authored-By: Claude Opus 5.5 <[email protected]>
fmt was fetched and compiled but never linked or included; spdlog uses its bundled copy. The pinned fmt 10.2.1 fails to compile with newer clang (consteval FMT_STRING errors), which broke the macOS build on macos-latest. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Ports bug fixes made to
plugins/wasi_nn/MLXin WasmEdge back to this repo. WasmEdge-specific changes (namespaces, license headers, log prefixes,svsuffixes, WASI VFS) are left out.Changes
whisper: remove shell command injection in
loadAudio(cf. WasmEdge 4742d02fba)loadAudiojoined the audio path into a command string and passed it topopen(), which runs it through/bin/sh. Shell metacharacters in the path (e.g.a.wav; rm -rf ~) ran arbitrary commands, and paths containing spaces broke.ffmpeg now runs via
posix_spawnpwith an argv array, so no shell is involved. Unlike WasmEdge, file-path input is kept becauseexample/whisper.cppand library users rely on it.vlm: stop generation at EOS
generate()always ran toMaxTokens. This adds a per-model virtualeosTokenIds(), and Gemma3 returns<eos>(1) and<end_of_turn>(106).WasmEdge hard-codes
Token == 1 || Token == 106in the shared loop. That would break mllama, where token 1 is"in the Llama 3 vocab, so the stop IDs are per-model here.loadBytesFromFile: don't kill the process on failure (cf. WasmEdge 584437baa5)It now logs an error and returns an empty string instead of calling
exit(1).Rename shadowed identifiers (cf. WasmEdge e271503856)
Renames parameters and locals that shadowed members or outer names in
Module,Embedding/Linear, pooling,RotatingKVCache, and the whisper decoding and tokenizer code. No behavior change.spdlog formatting (cf. WasmEdge 2b8429d303)
Values are passed as
{}format arguments instead of being concatenated into the format string.ci: run macOS build on
macos-latestGitHub no longer provides the
macos-14runner (The configuration 'macos-14-arm64-us-default' is not supported), so the macOS job was cancelled before it started.build: drop unused fmt dependency
CMakeLists.txtfetched and compiled fmt 10.2.1 but never linked or included it; spdlog uses its bundled copy. That fmt version fails to compile with the newer clang onmacos-latest(consteval/FMT_STRINGerrors), so it is removed.Not ported
const T&back to by-value parameters. This repo intentionally moved to const references in f1fffaf.flattenadded toQuantizedEmbedding::forwardin WasmEdge. It has no effect.take(...)difference. Both versions look inconsistent with the Python referencemask[..., -K:], so this needs a separate look.Testing
example/vlm.cpp, which includes the changedgemma3.h.model/gemma3/gemma3.cppfails locally with the same error onmain. It's a local clang / GCC 15 libstdc++ issue and unrelated to this PR.🤖 Generated with Claude Code