feat: make STT binaries (whisper/ffmpeg) platform-configurable#7
Merged
Conversation
Add WHISPER_BIN and FFMPEG_BIN env overrides so the speech-to-text engine can be swapped without editing source. Previously `whisper` and `ffmpeg` were invoked by literal binary name, forcing anyone with a differently-named binary (whisper.cpp, faster-whisper, a venv path, Homebrew vs pip) to patch the code. - TranscribeConfig gains `whisper_bin` / `ffmpeg_bin`, read in from_env() (default `whisper` / `ffmpeg`; empty/whitespace falls back to default). - check_transcribe_support() takes the config so the startup availability check resolves the same binaries that will actually be invoked. - find_executable() now resolves an explicit path to an existing executable directly, falling back to which/where for bare names. - transcribe_inner() invokes the configured binaries. - README: document WHISPER_BIN / FFMPEG_BIN. Values are trusted operator config (like WHISPER_MODEL) and invoked exec-direct (no shell), so no escaping is needed. Defaults preserve current behaviour — fully backward compatible. Co-authored-by: Agis (agent) <agis@hyperdev.local>
…w nit) Pin the actual voice-transcription invocation to the absolute paths that check_transcribe_support() resolved at startup, instead of letting transcribe_inner() re-resolve the bare WHISPER_BIN/FFMPEG_BIN name via PATH at exec time. This closes a TOCTOU / PATH-drift gap where the startup availability check and the real invocation could resolve to different binaries. - main.rs: after a successful support check, write the resolved whisper_path/ffmpeg_path back into transcribe_config (kept as the configured name when a path didn't resolve, preserving fallback). transcribe_inner() is unchanged — it now spawns the resolved path. - Add a test asserting check_transcribe_support resolves a real executable path for an existing binary. Backward compatible: with no env set, the binaries still resolve to whisper/ffmpeg via PATH exactly as before. Addresses cross-model review nit (b) on PR #7. Co-authored-by: Agis (agent) <agis@hyperdev.local>
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
1 participant
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.
Motivation
The speech-to-text path hardcodes
whisperandffmpegby literal binary name. Anyone whose platform has a differently-named binary — whisper.cpp, faster-whisper, a venv path, Homebrew vs pip installs, a NAS with a non-standard location — has to edit source and recompile to swap the engine. (This is the exact pain hit running hdcd-telegram across Mac + NAS.)This makes the STT backend a config change, not a code change.
What changed
TranscribeConfiggainswhisper_bin/ffmpeg_bin, read infrom_env()from new env varsWHISPER_BIN(defaultwhisper) andFFMPEG_BIN(defaultffmpeg). Empty/whitespace-only values fall back to the default with awarn!(mirrors the existingWHISPER_MODELstyle).check_transcribe_support()now takes the config, so the startup availability check resolves the same binaries thattranscribewill actually invoke (and logs the resolved binaries).find_executable()resolves an explicit absolute/relative path to an existing executable directly (existence + Unix exec-bit check), falling back towhich/wherefor bare names inPATH.transcribe_inner()invokesconfig.ffmpeg_bin/config.whisper_bininstead of literals.WHISPER_BIN/FFMPEG_BINrows in the env-var table.Security note
These values are trusted operator config (same trust level as
WHISPER_MODEL) and invocation is exec-direct viaCommand::new— no shell, so no escaping/sanitisation is needed.Backward compatibility
Defaults are
whisper/ffmpeg— identical to current behaviour. No action needed for existing users.Verification (observed locally)
cargo build— Finished, clean.cargo test— 51 + 6 + 1 tests pass (incl. 4 new transcribe-config tests). One integration test (router_ipc_test, unrelated IPC timing) flaked once then passed consistently on re-run.cargo fmt --check— clean.cargo clippy --all-targets— no warnings.Co-authored-by: Agis (agent) agis@hyperdev.local
🤖 Generated with Claude Code