1 files changed, 100 insertions, 8 deletions

diff --git a/src/sound.rs b/src/sound.rs
index 12b127bb..ab6c11c5 100644
--- a/src/sound.rs
+++ b/src/sound.rs
@@ -1,16 +1,18 @@
 //! Sound notifications for agent state changes.
 //!
 //! Embeds mp3 files in the binary and plays them via system audio tools.
-//! Uses afplay (macOS) or paplay/aplay (Linux) — no Rust audio dependencies.
+//! Uses afplay (macOS) or decoder-capable Linux audio players — no Rust audio dependencies.
 
 use std::io::Write;
-use std::path::Path;
+use std::path::{Path, PathBuf};
 use std::process::{Command, Output};
+use std::sync::atomic::{AtomicU64, Ordering};
 
 use tracing::warn;
 
 const DISABLE_SOUND_ENV: &str = "HERDR_DISABLE_SOUND";
 
+static SOUND_TMP_COUNTER: AtomicU64 = AtomicU64::new(0);
 static SOUND_DONE: &[u8] = include_bytes!("../assets/sounds/done.mp3");
 static SOUND_REQUEST: &[u8] = include_bytes!("../assets/sounds/request.mp3");
 
@@ -65,8 +67,8 @@ fn play_file(path: &Path) -> Result<(), String> {
 }
 
 fn play_bytes(data: &[u8]) -> Result<(), String> {
-    // Write to a temp file (audio players need a file path)
-    let tmp = std::env::temp_dir().join(format!("herdr-sound-{}.mp3", std::process::id()));
+    // Write to a temp file because the supported audio players need a file path.
+    let tmp = temp_sound_path();
     let mut file = std::fs::File::create(&tmp).map_err(|e| e.to_string())?;
     file.write_all(data).map_err(|e| e.to_string())?;
     drop(file);
@@ -82,6 +84,11 @@ fn play_bytes(data: &[u8]) -> Result<(), String> {
     }
 }
 
+fn temp_sound_path() -> PathBuf {
+    let id = SOUND_TMP_COUNTER.fetch_add(1, Ordering::Relaxed);
+    std::env::temp_dir().join(format!("herdr-sound-{}-{id}.mp3", std::process::id()))
+}
+
 fn run_player(path: &Path) -> Result<Output, String> {
     if cfg!(target_os = "macos") {
         Command::new("afplay")
@@ -89,11 +96,96 @@ fn run_player(path: &Path) -> Result<Output, String> {
             .output()
             .map_err(|e| format!("no audio player available: {e}"))
     } else {
-        // Try paplay (PulseAudio) first, fall back to aplay (ALSA)
-        Command::new("paplay")
+        run_linux_player(path)
+    }
+}
+
+#[derive(Debug, Clone, Copy)]
+struct AudioPlayer {
+    program: &'static str,
+    args: &'static [&'static str],
+}
+
+impl AudioPlayer {
+    fn output(self, path: &Path) -> std::io::Result<Output> {
+        Command::new(self.program)
+            .args(self.args)
             .arg(path)
             .output()
-            .or_else(|_| Command::new("aplay").arg(path).output())
-            .map_err(|e| format!("no audio player available: {e}"))
+    }
+}
+
+fn linux_audio_players() -> &'static [AudioPlayer] {
+    // Do not add bare aplay here. It does not decode MP3 and plays MP3 bytes as raw PCM.
+    &[
+        AudioPlayer {
+            program: "paplay",
+            args: &[],
+        },
+        AudioPlayer {
+            program: "pw-play",
+            args: &[],
+        },
+        AudioPlayer {
+            program: "ffplay",
+            args: &["-nodisp", "-autoexit", "-loglevel", "quiet"],
+        },
+        AudioPlayer {
+            program: "mpg123",
+            args: &["-q"],
+        },
+        AudioPlayer {
+            program: "mpv",
+            args: &["--no-video", "--really-quiet"],
+        },
+    ]
+}
+
+fn run_linux_player(path: &Path) -> Result<Output, String> {
+    let mut errors = Vec::new();
+
+    for player in linux_audio_players() {
+        match player.output(path) {
+            Ok(output) if output.status.success() => return Ok(output),
+            Ok(output) => errors.push(player_error(*player, &output)),
+            Err(err) => errors.push(format!("{} failed: {err}", player.program)),
+        }
+    }
+
+    Err(format!(
+        "no mp3-capable audio player available: {}",
+        errors.join("; ")
+    ))
+}
+
+fn player_error(player: AudioPlayer, output: &Output) -> String {
+    let stderr = String::from_utf8_lossy(&output.stderr);
+    let stderr = stderr.trim();
+
+    if stderr.is_empty() {
+        format!("{} exited with {}", player.program, output.status)
+    } else {
+        format!("{} exited with {}: {stderr}", player.program, output.status)
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn temp_sound_paths_are_unique() {
+        assert_ne!(temp_sound_path(), temp_sound_path());
+    }
+
+    #[test]
+    fn linux_audio_players_are_mp3_capable() {
+        let programs: Vec<&str> = linux_audio_players()
+            .iter()
+            .map(|player| player.program)
+            .collect();
+
+        assert_eq!(programs, ["paplay", "pw-play", "ffplay", "mpg123", "mpv"]);
+        assert!(!programs.contains(&"aplay"));
     }
 }