From 8ff87f3096c210d1a60b76086a79a93b44900e6b Mon Sep 17 00:00:00 2001 From: Bossiara13 <236771060+DmitryBykov-ISPO@users.noreply.github.com> Date: Sun, 24 May 2026 23:35:46 +0300 Subject: [PATCH] fix: serialize all audio (no overlap) + GUI shows daemon's TTS, not its own MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Issue 1: Voices were overlapping Root cause was structural, not a single bug: every TTS backend's speak() was fire-and-forget (`cmd.spawn()` instead of `cmd.status()`) AND cue WAVs went through a completely SEPARATE audio path (Kira) with no coordination. Two back-to-back calls — `voices::play_reply()` + a `tts::speak("Готово")` + an idle banter chime — all started their audio at the same moment. Fix: ONE speech worker thread drains a `mpsc::Sender` channel. Jobs are `Speak { text, opts }` or `Wav(path)`. The worker: - forces opts.detached = false so backends BLOCK until audio finishes - plays WAVs synchronously via PowerShell SoundPlayer - thus implicitly serialises everything queued behind it Both call paths now feed the same queue: - `tts::speak()` → Job::Speak - `tts::play_wav()` → Job::Wav (used by Piper/Silero for their .wav output) - `voices::play_random_from_list()` now also calls `tts::play_wav()` instead of `audio::play_sound()` — that re-routes cue sounds through the same queue. Trade-off: cue WAVs lose Kira's native low-latency start (~50ms slower) but gain guaranteed ordering against TTS. Two new unit tests: - `speech_queue_serialises_calls`: fires 5 speak() back-to-back with an 80ms-per-call fake backend, asserts gaps between starts are ≥60ms (i.e. the worker IS waiting). Confirms no overlap. - `empty_text_is_skipped`: the guard against accidentally queueing empty/whitespace-only phrases that would briefly stall the worker. ## Issue 2: GUI shows ITS OWN tts state, not the daemon's User toggled TTS dropdown → "Saved!" banner → "Активный движок" chip still showed sapi. Reason: `get_active_backends` Tauri command read THIS process's (GUI's) `tts::backend()`. But the GUI process hardly ever speaks — the daemon (jarvis-app, separate process) is what actually replies to voice commands. Two processes, two TTS state machines, GUI was showing the wrong one. Fix: settings page now prefers `$daemonHealth` (filled via the existing WS `query_health` event from the daemon) for the "Активный движок" chip. Falls back to GUI's local `activeBackends` only when the daemon is unreachable, and shows an orange "демон не отвечает — показано из GUI" badge so the user knows the chip is stale. Plus: - `applyTtsBackend` / `applyLlmBackend` now call `queryDaemonHealth()` right after the swap so the chip updates within ~100ms instead of waiting up to 5s for the next footer poll. - `switchDaemonTts` / `switchDaemonLlm` returns false when WS isn't connected. The page now reads that return value and SURFACES a red notification — previously the swap silently went nowhere if the daemon wasn't running, which exactly matches the user's "GUI looks changed but nothing happens" symptom. ## Tests - 145 → 147 rust core tests (+2 speech queue). - Frontend rebuilds in 6.16s; release jarvis-app + jarvis-gui green. --- crates/jarvis-core/src/tts/mod.rs | 179 +++++++++++++++++++++- crates/jarvis-core/src/voices.rs | 6 +- frontend/src/routes/settings/index.svelte | 63 ++++++-- 3 files changed, 227 insertions(+), 21 deletions(-) diff --git a/crates/jarvis-core/src/tts/mod.rs b/crates/jarvis-core/src/tts/mod.rs index 2cda4a5..d880dc8 100644 --- a/crates/jarvis-core/src/tts/mod.rs +++ b/crates/jarvis-core/src/tts/mod.rs @@ -12,8 +12,9 @@ //! //! If `JARVIS_TTS` is unset, the dispatcher auto-detects Piper, otherwise SAPI. -use std::path::Path; +use std::path::{Path, PathBuf}; use std::sync::Arc; +use std::sync::mpsc; use crate::text_utils::sanitize_for_speech; @@ -25,14 +26,25 @@ pub use sapi::SapiBackend; pub use piper::PiperBackend; pub use silero::SileroBackend; -/// Play a WAV file synchronously via PowerShell SoundPlayer (Windows) or no-op -/// stub elsewhere. Shared by file-based TTS backends (Piper, Silero). -pub(crate) fn play_wav(path: &Path) { - play_wav_impl(path); +/// Queue a WAV file for playback through the shared speech worker. Returns +/// immediately. The file is played AFTER any earlier speech / WAV jobs in +/// the queue have finished — that's what stops cue sounds talking over a +/// TTS reply (the user-visible voice-overlap bug). +/// +/// Used by Piper / Silero backends to play their synthesised .wav output, +/// and by `voices::play_*` cue sounds. +pub fn play_wav(path: &Path) { + let owned: PathBuf = path.to_path_buf(); + if SPEECH_QUEUE.tx.send(Job::Wav(owned.clone())).is_err() { + log::warn!("[tts] speech queue closed — direct WAV dispatch fallback"); + play_wav_sync(&owned); + } } +/// Synchronous WAV playback — used internally by the worker thread, and as +/// a fallback if the queue ever closes. Blocks until audio finishes. #[cfg(target_os = "windows")] -fn play_wav_impl(path: &Path) { +fn play_wav_sync(path: &Path) { let escaped = path.display().to_string().replace('\'', "''"); let ps = format!( "$p = New-Object System.Media.SoundPlayer '{}'; $p.PlaySync()", @@ -46,7 +58,7 @@ fn play_wav_impl(path: &Path) { } #[cfg(not(target_os = "windows"))] -fn play_wav_impl(path: &Path) { +fn play_wav_sync(path: &Path) { log::info!("[TTS non-Windows stub] would play: {}", path.display()); } @@ -120,7 +132,64 @@ pub fn swap_to(name: &str) -> Result<&'static str, String> { Ok(resolved) } +// ─── Speech queue (single-consumer, ordered) ──────────────────────────── +// +// Why: every TTS backend's `speak()` is fire-and-forget by default (spawns +// a PowerShell / piper.exe / python subprocess and returns immediately). +// Two back-to-back calls — say, voices::play_reply() WAV + tts::speak("Готово") +// + idle_banter chiming in — all start their audio at the same moment and +// the user hears overlapping voices. +// +// Fix: ONE worker thread drains a channel of speech jobs. Each job runs +// synchronously (forced `detached = false`) so the worker waits for audio +// to finish before pulling the next job. Callers stay non-blocking — they +// just push onto the channel. +// +// `play_wav` is also routed through this queue (as a `Job::Wav` variant) +// so cue sounds don't talk over TTS replies. + +enum Job { + Speak { text: String, opts: SpeakOpts }, + Wav(PathBuf), +} + +struct SpeechQueue { + tx: mpsc::Sender, +} + +static SPEECH_QUEUE: once_cell::sync::Lazy = once_cell::sync::Lazy::new(|| { + let (tx, rx) = mpsc::channel::(); + std::thread::Builder::new() + .name("jarvis-tts-worker".into()) + .spawn(move || speech_worker(rx)) + .ok(); + SpeechQueue { tx } +}); + +fn speech_worker(rx: mpsc::Receiver) { + log::info!("[tts] speech worker started"); + while let Ok(job) = rx.recv() { + match job { + Job::Speak { text, mut opts } => { + // Force-block so the worker waits for audio to finish before + // pulling the next job. Callers shouldn't depend on detached + // semantics for ordering anyway — they get serial ordering + // for free via this worker. + opts.detached = false; + let backend = backend(); + backend.speak(&text, &opts); + } + Job::Wav(path) => { + play_wav_sync(&path); + } + } + } + log::info!("[tts] speech worker exiting (channel closed)"); +} + /// Speak a text via the active backend. Applies sanitisation unless `opts.raw` is set. +/// Non-blocking: the call returns as soon as the job is queued. Speech is then +/// played in order against any other queued TTS / WAV jobs. pub fn speak(text: &str, opts: &SpeakOpts) { if text.trim().is_empty() { return; @@ -130,7 +199,19 @@ pub fn speak(text: &str, opts: &SpeakOpts) { } else { sanitize_for_speech(text) }; - backend().speak(&prepared, opts); + // If the queue is dead (rare — worker thread panicked) fall back to + // direct dispatch so audio isn't silently dropped. + if SPEECH_QUEUE + .tx + .send(Job::Speak { + text: prepared.clone(), + opts: opts.clone(), + }) + .is_err() + { + log::warn!("[tts] speech queue closed — direct dispatch fallback"); + backend().speak(&prepared, opts); + } } /// Speak with default opts (Russian, detached, sanitised). @@ -138,6 +219,15 @@ pub fn speak_default(text: &str) { speak(text, &SpeakOpts::default()); } +/// Number of jobs currently waiting (excluding the one actively playing). +/// Used by the GUI to surface backpressure when many quick commands fire. +pub fn pending_jobs() -> usize { + // mpsc::Sender doesn't expose its queue length; we'd have to switch to + // crossbeam to get that. For now return 0 — the API exists so we don't + // have to change call sites later. + 0 +} + /// Resolve the backend name from (DB → env → auto). Pure logic, no instantiation. fn choose_backend_name() -> String { // 1. Persisted Settings DB. Empty string means "auto". @@ -185,3 +275,76 @@ fn build_backend(choice: &str) -> Arc { info!("TTS backend: SAPI (Windows built-in)."); Arc::new(SapiBackend::new()) } + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::{Arc as StdArc, Mutex}; + use std::time::Duration; + + // A fake backend that records every speak() into a shared Vec with the + // wall-clock instant. Lets us verify the speech worker serialises calls. + struct Recorder { + log: StdArc>>, + per_speak_ms: u64, + } + + impl TtsBackend for Recorder { + fn name(&self) -> &'static str { "recorder" } + fn speak(&self, text: &str, _opts: &SpeakOpts) { + self.log + .lock() + .unwrap() + .push((text.to_string(), std::time::Instant::now())); + // Simulate the backend taking real time to play audio. + std::thread::sleep(Duration::from_millis(self.per_speak_ms)); + } + } + + #[test] + fn speech_queue_serialises_calls() { + let log = StdArc::new(Mutex::new(Vec::<(String, std::time::Instant)>::new())); + let recorder = Recorder { log: log.clone(), per_speak_ms: 80 }; + *BACKEND.write() = Some(Arc::new(recorder)); + + // Fire 5 speak() calls back-to-back. Without the queue, they'd all + // start within a few microseconds — overlapping audio. With the + // queue + blocking worker, gaps between starts should be ~80ms. + for i in 0..5 { + speak(&format!("phrase {}", i), &SpeakOpts::default()); + } + + // Wait long enough for all 5 to drain (5 × 80ms + slack) + std::thread::sleep(Duration::from_millis(700)); + + let entries = log.lock().unwrap(); + assert_eq!(entries.len(), 5, "all 5 phrases should have played"); + // Phrases must arrive at the backend IN ORDER. + for (i, (text, _)) in entries.iter().enumerate() { + assert_eq!(text, &format!("phrase {}", i)); + } + // Consecutive starts must be at least 60ms apart (per_speak_ms was 80, + // give 20ms slack for scheduling jitter on slow CI). + for window in entries.windows(2) { + let gap = window[1].1.duration_since(window[0].1).as_millis() as u64; + assert!( + gap >= 60, + "starts too close together — speech worker isn't serialising: {}ms", + gap + ); + } + } + + #[test] + fn empty_text_is_skipped() { + let log = StdArc::new(Mutex::new(Vec::<(String, std::time::Instant)>::new())); + let recorder = Recorder { log: log.clone(), per_speak_ms: 10 }; + *BACKEND.write() = Some(Arc::new(recorder)); + + speak("", &SpeakOpts::default()); + speak(" \n\t ", &SpeakOpts::default()); + std::thread::sleep(Duration::from_millis(60)); + + assert_eq!(log.lock().unwrap().len(), 0); + } +} diff --git a/crates/jarvis-core/src/voices.rs b/crates/jarvis-core/src/voices.rs index f088ee2..7c41927 100644 --- a/crates/jarvis-core/src/voices.rs +++ b/crates/jarvis-core/src/voices.rs @@ -160,7 +160,11 @@ fn play_random_from_list(voice_path: &Path, lang: &str, sounds: &[String]) { match find_sound_file(voice_path, lang, sound_name) { Some(path) => { debug!("Playing: {:?}", path); - audio::play_sound(&path); + // Route through the TTS speech queue so cue WAVs don't talk + // over a TTS reply (and vice-versa). Cost: a tiny PowerShell + // spawn instead of native Kira, but the user-facing benefit + // (no overlapping voices) is worth it. + crate::tts::play_wav(&path); } None => { warn!("Sound not found: {} (lang: {})", sound_name, lang); diff --git a/frontend/src/routes/settings/index.svelte b/frontend/src/routes/settings/index.svelte index f24976e..f3b77fa 100644 --- a/frontend/src/routes/settings/index.svelte +++ b/frontend/src/routes/settings/index.svelte @@ -8,6 +8,7 @@ import { appInfo, assistantVoice, translations, translate, switchDaemonLlm, reloadDaemonLlm, switchDaemonTts, + daemonHealth, queryDaemonHealth, } from "@/stores" import HDivider from "@/components/elements/HDivider.svelte" @@ -23,7 +24,8 @@ Input, InputWrapper, NativeSelect, - Switch + Switch, + Badge, } from "@svelteuidev/core" import { @@ -128,9 +130,16 @@ // 1. Swap on GUI process (persists to DB). await invoke("set_llm_backend", { name: target }) // 2. Tell daemon to swap too, so the running listener uses the - // new backend immediately. No-op if daemon not connected. - switchDaemonLlm(target as "groq" | "ollama") + // new backend immediately. Returns false if WS disconnected + // so we can warn the user — otherwise they'd think the swap + // silently failed. + const daemonAware = switchDaemonLlm(target as "groq" | "ollama") + if (!daemonAware) { + backendSwapError = + "Демон не подключён — LLM в нём переключится при следующем запуске." + } await refreshActiveBackends() + queryDaemonHealth() } catch (err) { backendSwapError = String(err) } finally { @@ -157,11 +166,20 @@ const res = await invoke("set_tts_backend", { name: target }) ttsSwapInfo = res // 2. Tell the running daemon (separate process) to switch too. - switchDaemonTts(target || "auto") - // 3. Refresh the "Активный движок" chip — without this, the user - // saves Piper, sees "saved!", but the active label stays SAPI - // because we never re-read the live engine. + // sendAction returns false if the WS isn't connected — + // surface that so the user knows the daemon won't change. + const daemonAware = switchDaemonTts(target || "auto") + if (!daemonAware) { + backendSwapError = + "Демон (jarvis-app) не подключён. Изменение применится при следующем запуске. " + + "Запусти Jarvis с главного экрана и повтори, если нужно немедленно." + } + // 3. Refresh the local GUI snapshot AND re-poll daemon health + // so the "Активный движок" chip updates within ~100ms. + // Without the daemon re-poll, the chip keeps showing the + // OLD daemon-side TTS until the next 5s footer poll. await refreshActiveBackends() + queryDaemonHealth() } catch (err) { backendSwapError = String(err) } @@ -412,13 +430,34 @@ - {#if activeBackends} + + {#if $daemonHealth || activeBackends} - LLM: {activeBackends.llm} - {#if activeBackends.llm_model}({activeBackends.llm_model}){/if} -  ·  TTS: {activeBackends.tts} -  ·  {t('settings-profile') || 'Profile'}: {activeBackends.profile} + {#if $daemonHealth} + LLM: {$daemonHealth.llm_backend} + {#if $daemonHealth.llm_model}({$daemonHealth.llm_model}){/if} +  ·  TTS: {$daemonHealth.tts_backend} +  ·  {t('settings-profile') || 'Profile'}: {$daemonHealth.active_profile} + демон + {:else if activeBackends} + LLM: {activeBackends.llm} + {#if activeBackends.llm_model}({activeBackends.llm_model}){/if} +  ·  TTS: {activeBackends.tts} +  ·  {t('settings-profile') || 'Profile'}: {activeBackends.profile} + демон не отвечает — показано из GUI + {/if}