From 7666a9773cf56627310cf295c5601271be014825 Mon Sep 17 00:00:00 2001 From: Cyril Fougeray Date: Mon, 28 Oct 2024 11:43:40 +0100 Subject: [PATCH] test(sound): check sound files in rust tests instead of invoking soxi check that all files are present in the assets check bits per sample (16 bits) sounds directory based on cargo manifest path for tests. tests are run with all other rust tests --- .github/workflows/check-sound-files.yaml | 23 -- .github/workflows/rust-ci.yaml | 1 + Cargo.lock | 1 + ui/Cargo.toml | 1 + ui/sound/examples/queue.rs | 17 +- ui/sound/examples/voice_connected.wav | 3 - ui/sound/examples/voice_overheating.wav | 3 - ui/sound/examples/wav.rs | 2 +- ui/sound/utils/README.md | 9 - ui/sound/utils/check_sounds.sh | 52 ---- ui/src/engine/diamond.rs | 19 +- ui/src/engine/pearl/mod.rs | 21 +- ui/src/sound/mod.rs | 332 +++++++++++++++++++---- 13 files changed, 308 insertions(+), 176 deletions(-) delete mode 100644 .github/workflows/check-sound-files.yaml delete mode 100644 ui/sound/examples/voice_connected.wav delete mode 100644 ui/sound/examples/voice_overheating.wav delete mode 100644 ui/sound/utils/README.md delete mode 100644 ui/sound/utils/check_sounds.sh diff --git a/.github/workflows/check-sound-files.yaml b/.github/workflows/check-sound-files.yaml deleted file mode 100644 index 3c74ab2b..00000000 --- a/.github/workflows/check-sound-files.yaml +++ /dev/null @@ -1,23 +0,0 @@ -name: Check Sound Files - -on: - push: - paths: - - 'ui/sound/assets/**' - - '.github/workflows/check-sound-files.yaml' - -jobs: - sound-files: - name: Check Sound Files - runs-on: ubuntu-22.04 - - steps: - - uses: actions/checkout@f43a0e5ff2bd294095638e18286ca9a3d1956744 # pin@v3 - with: - lfs: true - - run: sudo apt-get install -y sox - - name: Check sound files - run: | - bash ./ui/sound/utils/check_sounds.sh ui/sound/assets/ - # ensure error code is 0 - exit $? diff --git a/.github/workflows/rust-ci.yaml b/.github/workflows/rust-ci.yaml index 3ca05c0d..920c27fe 100644 --- a/.github/workflows/rust-ci.yaml +++ b/.github/workflows/rust-ci.yaml @@ -115,6 +115,7 @@ jobs: - uses: actions/checkout@f43a0e5ff2bd294095638e18286ca9a3d1956744 # pin@v3 with: token: ${{ secrets.GIT_HUB_TOKEN }} + lfs: true - uses: cachix/install-nix-action@ba0dd844c9180cbf77aa72a116d6fbc515d0e87b # pin@v27 with: github_access_token: ${{ secrets.GIT_HUB_TOKEN }} diff --git a/Cargo.lock b/Cargo.lock index fe1bba9c..965bb546 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4451,6 +4451,7 @@ dependencies = [ "derive_more", "eyre", "futures", + "hound", "orb-build-info 0.0.0", "orb-messages 0.0.0 (git+https://github.com/worldcoin/orb-messages?rev=787ab78581b705af0946bcfe3a0453b64af2193f)", "orb-rgb", diff --git a/ui/Cargo.toml b/ui/Cargo.toml index 6c25b5c8..537062f2 100644 --- a/ui/Cargo.toml +++ b/ui/Cargo.toml @@ -44,6 +44,7 @@ path = "examples/ui-replay.rs" # dependencies for the dbus-client example [dev-dependencies] chrono = "0.4.35" +hound = "3.5.1" [package.metadata.deb] maintainer-scripts = "debian/" diff --git a/ui/sound/examples/queue.rs b/ui/sound/examples/queue.rs index 0bb3219e..9d148f84 100644 --- a/ui/sound/examples/queue.rs +++ b/ui/sound/examples/queue.rs @@ -16,9 +16,8 @@ impl AsRef<[u8]> for Sound { async fn main() -> Result<()> { let queue = Queue::spawn("default")?; - let connected = Sound(Arc::new(fs::read("sound/examples/voice_connected.wav")?)); - let overheating = - Sound(Arc::new(fs::read("sound/examples/voice_overheating.wav")?)); + let connected = Sound(Arc::new(fs::read("sound/assets/voice_connected.wav")?)); + let timeout = Sound(Arc::new(fs::read("sound/assets/voice_timeout.wav")?)); // Said firstly because the queue starts playing immediately. queue @@ -40,19 +39,13 @@ async fn main() -> Result<()> { // Said secondly because it has a higher priority than all pending sounds in // the queue. queue - .sound( - Some(Cursor::new(overheating.clone())), - "overheating".to_string(), - ) + .sound(Some(Cursor::new(timeout.clone())), "timeout".to_string()) .priority(1) .push()?; // Not said because it doesn't meet the `max_delay`. assert!( !queue - .sound( - Some(Cursor::new(overheating.clone())), - "overheating".to_string() - ) + .sound(Some(Cursor::new(timeout.clone())), "timeout".to_string()) .priority(1) .max_delay(Duration::from_secs(1)) .push()? @@ -72,7 +65,7 @@ async fn main() -> Result<()> { ); // In result the queue should be played in the following order: connected, - // overheating, connected, connected. + // timeout, connected, connected. Ok(()) } diff --git a/ui/sound/examples/voice_connected.wav b/ui/sound/examples/voice_connected.wav deleted file mode 100644 index e37516ab..00000000 --- a/ui/sound/examples/voice_connected.wav +++ /dev/null @@ -1,3 +0,0 @@ -version https://git-lfs.github.com/spec/v1 -oid sha256:174676b7611a291ec5cb86c7ef4f29740333ca3e26c2913bddcad32f92b88dd4 -size 53036 diff --git a/ui/sound/examples/voice_overheating.wav b/ui/sound/examples/voice_overheating.wav deleted file mode 100644 index 8f5c4d8b..00000000 --- a/ui/sound/examples/voice_overheating.wav +++ /dev/null @@ -1,3 +0,0 @@ -version https://git-lfs.github.com/spec/v1 -oid sha256:a6b2758a49eea5ef0ca9e75a934133675d1b7034de79c3cce507812b34e7bbdf -size 57408 diff --git a/ui/sound/examples/wav.rs b/ui/sound/examples/wav.rs index d5fbd688..a405fcdb 100644 --- a/ui/sound/examples/wav.rs +++ b/ui/sound/examples/wav.rs @@ -6,7 +6,7 @@ fn main() -> Result<()> { let mut device = Device::open("default")?; let mut hw_params = HwParams::new()?; - let mut wav = File::open("sound/examples/voice_connected.wav")?; + let mut wav = File::open("sound/assets/voice_connected.wav")?; device.play_wav(&mut wav, &mut hw_params, 1.0)?; device.drain()?; diff --git a/ui/sound/utils/README.md b/ui/sound/utils/README.md deleted file mode 100644 index f09dd635..00000000 --- a/ui/sound/utils/README.md +++ /dev/null @@ -1,9 +0,0 @@ -# Sound utils - -## `check_sounds.sh` - -Ensure that all sounds are `.wav` files with 16-bit/sample. - -```shell -$ bash ./check_sounds.sh ../assets/ -``` diff --git a/ui/sound/utils/check_sounds.sh b/ui/sound/utils/check_sounds.sh deleted file mode 100644 index c0f30dc0..00000000 --- a/ui/sound/utils/check_sounds.sh +++ /dev/null @@ -1,52 +0,0 @@ -#!/usr/bin/env bash - -set -o errexit # abort on nonzero exitstatus -set -o nounset # abort on unbound variable -set -o pipefail # don't hide errors within pipes - -# Checks that the sounds directory only includes *.wav files -# Checks that the sounds are sampled on 16 bits -# Guards against bad archive preparation. - -validate_sounds::validate() { - local -r sounds_dir=${1} - - pushd "${sounds_dir}" >/dev/null - - local check_bit_sampling=true - if ! [[ -x "$(command -v soxi)" ]]; then - echo "Install sox (soxi) to check bits per sample" >>/dev/stderr - check_bit_sampling=false - fi - - for file in *; do - if ! [[ "${file}" =~ \.wav$ ]] ; then - echo "Invalid sounds directory: '${file}' is not a wav file" - exit 1 - fi - - if [[ "${check_bit_sampling}" = true ]]; then - local -i bits_per_sample - bits_per_sample="$(soxi -b "${file}")" - - if [[ "${bits_per_sample}" != 16 ]]; then - echo "${file} must be converted to 16 bits per sample" - exit 1 - fi - fi - done - - popd >/dev/null - - echo "Sounds directory successfully validated." -} - -main() { - trap trap_err ERR - - validate_sounds::validate "${1}" -} - -if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then - main "$@" -fi diff --git a/ui/src/engine/diamond.rs b/ui/src/engine/diamond.rs index 469eb288..2c9e8b8c 100644 --- a/ui/src/engine/diamond.rs +++ b/ui/src/engine/diamond.rs @@ -135,10 +135,7 @@ pub async fn event_loop( Runner::::new(sound) } Err(e) => { - return { - tracing::error!("Failed to initialize sound: {:?}", e); - Err(e) - }; + return Err(eyre::eyre!("Failed to initialize sound: {:?}", e)); } }; loop { @@ -935,9 +932,17 @@ impl EventHandler for Runner { let sound = self.sound.clone(); // spawn a new task because we need some async work here tokio::task::spawn(async move { - let language: Option<&str> = language.as_deref(); - if let Err(e) = sound.load_sound_files(language, true).await { - tracing::error!("Error loading sound files: {:?}", e); + match sound::SoundConfig::default() + .with_language(language.as_deref()) + { + Ok(config) => { + if let Err(e) = sound.load_sound_files(config).await { + tracing::error!("Error loading sound files: {:?}", e); + } + } + Err(e) => { + tracing::error!("Error creating sound config: {:?}", e); + } } }); } diff --git a/ui/src/engine/pearl/mod.rs b/ui/src/engine/pearl/mod.rs index 789a5293..5c4a77e9 100644 --- a/ui/src/engine/pearl/mod.rs +++ b/ui/src/engine/pearl/mod.rs @@ -109,10 +109,9 @@ pub async fn event_loop( interval.set_missed_tick_behavior(time::MissedTickBehavior::Delay); let mut interval = IntervalStream::new(interval); let mut rx = UnboundedReceiverStream::new(rx); - let mut runner = if let Ok(sound) = sound::Jetson::spawn().await { - Runner::::new(sound) - } else { - return Err(eyre::eyre!("Failed to initialize sound")); + let mut runner = match sound::Jetson::spawn().await { + Ok(sound) => Runner::::new(sound), + Err(e) => return Err(eyre::eyre!("Failed to initialize sound: {:?}", e)), }; loop { match future::select(rx.next(), interval.next()).await { @@ -274,9 +273,17 @@ impl EventHandler for Runner { let sound = self.sound.clone(); // spawn a new task because we need some async work here tokio::task::spawn(async move { - let language: Option<&str> = language.as_deref(); - if let Err(e) = sound.load_sound_files(language, true).await { - tracing::error!("Error loading sound files: {:?}", e); + match sound::SoundConfig::default() + .with_language(language.as_deref()) + { + Ok(config) => { + if let Err(e) = sound.load_sound_files(config).await { + tracing::error!("Error loading sound files: {:?}", e); + } + } + Err(e) => { + tracing::error!("Error creating sound config: {:?}", e); + } } }); } diff --git a/ui/src/sound/mod.rs b/ui/src/sound/mod.rs index 5ae4273c..0f5c8bb0 100644 --- a/ui/src/sound/mod.rs +++ b/ui/src/sound/mod.rs @@ -3,10 +3,10 @@ pub(crate) mod capture; use dashmap::DashMap; -use eyre::{Result, WrapErr}; use futures::prelude::*; use orb_sound::{Queue, SoundBuilder}; use serde::{Deserialize, Serialize}; +use std::fmt::{Display, Formatter}; use std::time::Duration; use std::{fmt, io::Cursor, path::Path, pin::Pin, sync::Arc}; use tokio::fs; @@ -15,6 +15,7 @@ use tokio::fs; const SOUND_CARD_NAME: &str = "default"; /// Path to the directory with the sound files. const SOUNDS_DIR: &str = "/home/worldcoin/data/sounds"; +const DEFAULT_LANGUAGE: Language = Language::En; /// Default master volume level. const DEFAULT_MASTER_VOLUME: f64 = 0.15; @@ -23,12 +24,11 @@ pub trait Player: fmt::Debug + Send { /// Loads sound files for the given language from the file system. fn load_sound_files( &self, - language: Option<&str>, - ignore_missing_sounds: bool, - ) -> Pin> + Send + '_>>; + config: SoundConfig, + ) -> Pin> + Send + '_>>; /// Creates a new sound builder object. - fn build(&mut self, sound_type: Type) -> Result; + fn build(&mut self, sound_type: Type) -> eyre::Result; /// Returns a new handler to the shared queue. fn clone(&self) -> Box; @@ -42,11 +42,11 @@ pub trait Player: fmt::Debug + Send { /// Queues a sound to be played. /// Helper method for `build` and `push`. /// Optionally delays the sound. - fn queue(&mut self, sound_type: Type, delay: Duration) -> Result<()>; + fn queue(&mut self, sound_type: Type, delay: Duration) -> eyre::Result<()>; /// Queues a sound to be played with a max delay. /// Helper method for `build` and `push`. - fn try_queue(&mut self, sound_type: Type) -> Result; + fn try_queue(&mut self, sound_type: Type) -> eyre::Result; } /// Sound queue for the Orb hardware. @@ -56,6 +56,106 @@ pub struct Jetson { volume: f64, } +#[derive(Debug)] +pub enum SoundError { + MissingFile(String), + NoSuchDirectory(String), + UnsupportedLanguage, + UnsupportedSoundFormat(String), + OsError, +} + +#[derive(Clone, Debug)] +pub enum Language { + /// English + En, + /// Spanish + Es, +} + +impl TryFrom for Language { + type Error = SoundError; + + fn try_from(value: String) -> Result { + if value.to_lowercase().as_str().contains("en") { + Ok(Language::En) + } else if value.to_lowercase().as_str().contains("es") { + Ok(Language::Es) + } else { + Err(SoundError::UnsupportedLanguage) + } + } +} + +impl Display for Language { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + match self { + Language::En => write!(f, "en-EN"), + Language::Es => write!(f, "es-ES"), + } + } +} + +impl Language { + /// Returns the file suffix for the language. + /// + pub fn as_suffix(&self) -> &str { + match self { + Language::En => "", + Language::Es => "__es-ES", + } + } +} + +#[derive(Debug, Clone)] +pub struct SoundConfig { + sound_path: String, + language: Language, + /// don't load sounds that are missing, nothing will be played + /// if false, an error will be raised, and default sounds will be used if it exists (EN-en) + ignore_missing_sounds: bool, +} + +impl SoundConfig { + /// Set language, use default if None + pub(crate) fn with_language( + mut self, + lang: Option<&str>, + ) -> Result { + if let Some(lang) = lang { + self.language = Language::try_from(lang.to_string())?; + } else { + self.language = DEFAULT_LANGUAGE; + } + Ok(self) + } + + #[cfg(test)] + pub(crate) fn with_path(mut self, path: &str) -> Result { + if !Path::new(path).exists() { + return Err(SoundError::NoSuchDirectory(path.to_string())); + } + self.sound_path = path.to_string(); + Ok(self) + } + + #[cfg(test)] + pub(crate) fn with_ignore_missing_sounds(mut self, ignore: bool) -> Self { + self.ignore_missing_sounds = ignore; + self + } +} + +impl Default for SoundConfig { + fn default() -> SoundConfig { + SoundConfig { + sound_path: SOUNDS_DIR.to_string(), + language: Language::En, + ignore_missing_sounds: true, + } + } +} + /// Available sound types #[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Hash)] #[serde(tag = "sound_type", content = "value")] @@ -89,13 +189,12 @@ macro_rules! sound_enum { impl $name { async fn load_sound_files( sound_files: &DashMap, - language: Option<&str>, - ignore_missing_sounds: bool, - ) -> Result<()> { + config: &SoundConfig, + ) -> Result<(), SoundError> { $( sound_files.insert( Type::$name(Self::$sound), - load_sound_file($file, language, ignore_missing_sounds).await?, + load_sound_file($file, config).await?, ); )* Ok(()) @@ -209,14 +308,16 @@ impl AsRef<[u8]> for SoundFile { impl Jetson { /// Spawns a new sound queue. - pub async fn spawn() -> Result { + pub async fn spawn() -> Result { let sound = Self { - queue: Arc::new(Queue::spawn(SOUND_CARD_NAME)?), + queue: Arc::new( + Queue::spawn(SOUND_CARD_NAME).map_err(|_| SoundError::OsError)?, + ), sound_files: Arc::new(DashMap::new()), volume: DEFAULT_MASTER_VOLUME, }; - let language = Some("EN-en"); - sound.load_sound_files(language, true).await?; + let config = SoundConfig::default(); + sound.load_sound_files(config).await?; Ok(sound) } } @@ -224,32 +325,23 @@ impl Jetson { impl Player for Jetson { fn load_sound_files( &self, - language: Option<&str>, - ignore_missing_sounds: bool, - ) -> Pin> + Send + '_>> { + config: SoundConfig, + ) -> Pin> + Send + '_>> { let sound_files = Arc::clone(&self.sound_files); - let language = language.map(ToOwned::to_owned); Box::pin(async move { - Voice::load_sound_files( - &sound_files, - language.as_deref(), - ignore_missing_sounds, - ) - .await?; - Melody::load_sound_files( - &sound_files, - language.as_deref(), - ignore_missing_sounds, - ) - .await?; + Voice::load_sound_files(&sound_files, &config.clone()).await?; + Melody::load_sound_files(&sound_files, &config.clone()).await?; let count = sound_files.len(); - tracing::debug!("Sound files for language {language:?} loaded successfully ({count:?} files)"); + tracing::debug!( + "Sound files for language {:?} loaded successfully ({count:?} files)", + config.language + ); Ok(()) }) } #[allow(clippy::missing_panics_doc)] - fn build(&mut self, sound_type: Type) -> Result { + fn build(&mut self, sound_type: Type) -> eyre::Result { let sound_file = self.sound_files.get(&sound_type).unwrap(); // It does Arc::clone under the hood, which is cheap. let reader = @@ -273,13 +365,13 @@ impl Player for Jetson { self.volume = level as f64 / 100.0; } - fn queue(&mut self, sound_type: Type, delay: Duration) -> Result<()> { + fn queue(&mut self, sound_type: Type, delay: Duration) -> eyre::Result<()> { let volume = self.volume; self.build(sound_type)?.volume(volume).delay(delay).push()?; Ok(()) } - fn try_queue(&mut self, sound_type: Type) -> Result { + fn try_queue(&mut self, sound_type: Type) -> eyre::Result { if self.queue.empty() { self.queue(sound_type, Duration::ZERO)?; Ok(true) @@ -291,35 +383,47 @@ impl Player for Jetson { /// Returns SoundFile if sound in filesystem entries. async fn load_sound_file( - sound: &str, - language: Option<&str>, - ignore_missing: bool, -) -> Result { - let sounds_dir = Path::new(SOUNDS_DIR); - if let Some(language) = language { - let file = sounds_dir.join(format!("{sound}__{language}.wav")); - if file.exists() { - let data = fs::read(&file) - .await - .wrap_err_with(|| format!("failed to read {}", file.display()))?; - return Ok(SoundFile(Arc::new(data))); - } - } - let file = sounds_dir.join(format!("{sound}.wav")); - let data = match fs::read(&file) - .await - .wrap_err_with(|| format!("failed to read {}", file.display())) - { - Ok(data) => data, - Err(err) => { - if ignore_missing { - tracing::error!("Ignoring missing sounds: {err}"); + filename: &str, + config: &SoundConfig, +) -> Result { + let sounds_dir = Path::new(config.sound_path.as_str()); + + // voices have language (other than english) appended to the file name + // sounds don't have any language + let voice_filename = format!("{filename}{}.wav", config.language.as_suffix()); + let file = match sounds_dir.join(voice_filename.clone()).exists() { + true => sounds_dir.join(voice_filename), + false => sounds_dir.join(format!("{filename}.wav")), + }; + + let data = match fs::read(&file).await { + Ok(d) => d, + Err(e) => { + if config.ignore_missing_sounds { + tracing::error!("ignoring missing sound: {e}"); Vec::new() } else { - return Err(err); + return Err(SoundError::MissingFile(e.to_string())); } } }; + + // we have had errors with reading files encoded over 24 bits, so + // this test ensure that wav files are sampled on 16 bits, for full Jetson compatibility. + // remove this test if different sampling are supported. + #[cfg(test)] + { + let reader = hound::WavReader::open(&file).map_err(|_| { + SoundError::MissingFile(String::from(file.to_str().unwrap())) + })?; + assert_eq!( + reader.spec().bits_per_sample, + 16, + "Only 16-bit sounds are supported: {:?}", + &file + ); + } + Ok(SoundFile(Arc::new(data))) } @@ -328,3 +432,113 @@ impl fmt::Debug for Jetson { f.debug_struct("Sound").finish() } } + +#[cfg(test)] +mod tests { + use super::{Melody, Player, SoundConfig, SoundError, SoundFile, Type, Voice}; + use dashmap::DashMap; + use orb_sound::SoundBuilder; + use std::fmt::{Debug, Formatter}; + use std::future::Future; + use std::pin::Pin; + use std::sync::Arc; + use std::time::Duration; + + struct MockJetson { + sound_files: Arc>, + } + + impl Debug for MockJetson { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + f.debug_struct("MockJetson").finish() + } + } + + impl Player for MockJetson { + fn load_sound_files( + &self, + config: SoundConfig, + ) -> Pin> + Send + '_>> + { + let sound_files = Arc::clone(&self.sound_files); + Box::pin(async move { + Voice::load_sound_files(&sound_files, &config).await?; + Melody::load_sound_files(&sound_files, &config).await?; + let count = sound_files.len(); + tracing::debug!("Sound files for language {:?} loaded successfully ({count:?} files)", config.language); + Ok(()) + }) + } + + fn build(&mut self, _sound_type: Type) -> eyre::Result { + unimplemented!() + } + + fn clone(&self) -> Box { + Box::new(MockJetson { + sound_files: self.sound_files.clone(), + }) + } + + fn volume(&self) -> u64 { + unimplemented!() + } + + fn set_master_volume(&mut self, _level: u64) { + unimplemented!() + } + + fn queue(&mut self, _sound_type: Type, _delay: Duration) -> eyre::Result<()> { + unimplemented!() + } + + fn try_queue(&mut self, _sound_type: Type) -> eyre::Result { + unimplemented!() + } + } + + /// This test allows us to check that all files that can be pulled by the UI + /// are present in the repository and are all encoded over 16 bits + #[tokio::test] + async fn test_load_sound_file() -> Result<(), SoundError> { + let sound = MockJetson { + sound_files: Arc::new(DashMap::new()), + }; + + let config = SoundConfig::default() + .with_language(None)? + .with_path(concat!(env!("CARGO_MANIFEST_DIR"), "/sound/assets"))? + .with_ignore_missing_sounds(false); + let res = sound.load_sound_files(config).await; + if let Err(e) = &res { + println!("{:?}", e); + } + assert!(res.is_ok(), "Default (None) failed"); + + let config = SoundConfig::default() + .with_language(Some("en-EN"))? + .with_path(concat!(env!("CARGO_MANIFEST_DIR"), "/sound/assets"))? + .with_ignore_missing_sounds(false); + let res = sound.load_sound_files(config).await; + assert!(res.is_ok(), "en-EN failed: {:?}", res); + + let config = SoundConfig::default() + .with_language(Some("es-ES"))? + .with_path(concat!(env!("CARGO_MANIFEST_DIR"), "/sound/assets"))? + .with_ignore_missing_sounds(false); + let res = sound.load_sound_files(config).await; + assert!(res.is_ok(), "es-ES failed: {:?}", res); + + // unsupported / missing voice files + let config = SoundConfig::default().with_language(Some("fr-FR")); + assert!(config.is_err(), "fr-FR should have failed"); + + let config = SoundConfig::default().with_path("doesnotexist"); + assert!( + config.is_err(), + "path that don't exist should throw an error" + ); + + Ok(()) + } +}