From cfd0781ebae76447cc8072fd60883aabf8158d32 Mon Sep 17 00:00:00 2001 From: salmonik Date: Fri, 8 May 2026 00:40:40 +0200 Subject: Extract queue module, add comprehensive tests, fix queue/play bugs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit • Moved Queue, Track, ShuffleMode, AudioMetadata from daemon.rs into new src/queue.rs module with public API\n• Fixed rsp play interrupting active playback (now checks is_playing)\n• Fixed directory play -n/-i inserting files in reverse order (now uses .iter().rev())\n• Removed dead duplicate shuffle handler in process_command\n• Fixed queue display showing (playing) when playback is stopped\n• Added 34 tests covering navigation, shuffle, insert ordering, edge cases, display --- src/daemon.rs | 380 ++++++++++++---------------------------------------------- src/lib.rs | 1 + src/main.rs | 1 + src/queue.rs | 250 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 328 insertions(+), 304 deletions(-) create mode 100644 src/queue.rs (limited to 'src') diff --git a/src/daemon.rs b/src/daemon.rs index 9e02e3f..0921e11 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -1,5 +1,6 @@ use crate::audio::AudioEngine; use crate::config::Config; +use crate::queue::{Queue, ShuffleMode, Track}; use std::io::{Read, Write}; use std::os::unix::net::{UnixListener, UnixStream}; use std::path::{Path, PathBuf}; @@ -8,217 +9,6 @@ use std::sync::{Arc, Mutex}; use std::thread; use std::time::Duration; -#[derive(Debug, Clone)] -enum ShuffleMode { - None, - All, - Album, -} - -#[derive(Debug, Clone)] -struct Track { - path: PathBuf, - title: Option, - artist: Option, - album: Option, -} - -impl Track { - fn from_path(path: PathBuf) -> Self { - let metadata = extract_metadata(&path); - let metadata = metadata.as_ref(); - Track { - path, - title: metadata.and_then(|m| m.title.clone()), - artist: metadata.and_then(|m| m.artist.clone()), - album: metadata.and_then(|m| m.album.clone()), - } - } - - fn display_string(&self, index: usize) -> String { - let album_artist = self - .artist - .clone() - .or_else(|| self.album.clone()) - .unwrap_or_default(); - let title = self.title.clone().unwrap_or_else(|| { - self.path - .file_name() - .and_then(|n| n.to_str()) - .unwrap_or("Unknown") - .to_string() - }); - - if album_artist.is_empty() { - format!("{}. {}", index + 1, title) - } else { - format!("{}. {} - {}", index + 1, album_artist, title) - } - } -} - -#[derive(Debug, Default, Clone)] -struct AudioMetadata { - title: Option, - artist: Option, - album: Option, -} - -fn extract_metadata(path: &Path) -> Option { - use symphonia::core::io::MediaSourceStream; - use symphonia::core::meta::StandardTagKey; - use symphonia::core::probe::Hint; - - let file = std::fs::File::open(path).ok()?; - let mss = MediaSourceStream::new(Box::new(file), Default::default()); - - let hint = Hint::new(); - let mut probed = symphonia::default::get_probe() - .format(&hint, mss, &Default::default(), &Default::default()) - .ok()?; - - let mut metadata = AudioMetadata::default(); - - // First check probed metadata (e.g., ID3v2 headers before container) - if let Some(meta) = probed.metadata.get() { - if let Some(rev) = meta.current() { - for tag in rev.tags() { - match tag.std_key { - Some(StandardTagKey::TrackTitle) => { - metadata.title = Some(tag.value.to_string()); - } - Some(StandardTagKey::Artist) => { - metadata.artist = Some(tag.value.to_string()); - } - Some(StandardTagKey::Album) => { - metadata.album = Some(tag.value.to_string()); - } - _ => {} - } - } - } - } - - // If nothing found, try container metadata - if metadata.title.is_none() && metadata.artist.is_none() && metadata.album.is_none() { - let mut format = probed.format; - if let Some(rev) = format.metadata().current() { - for tag in rev.tags() { - match tag.std_key { - Some(StandardTagKey::TrackTitle) => { - metadata.title = Some(tag.value.to_string()); - } - Some(StandardTagKey::Artist) => { - metadata.artist = Some(tag.value.to_string()); - } - Some(StandardTagKey::Album) => { - metadata.album = Some(tag.value.to_string()); - } - _ => {} - } - } - } - } - - if metadata.title.is_some() || metadata.artist.is_some() || metadata.album.is_some() { - Some(metadata) - } else { - None - } -} - -struct Queue { - tracks: Vec, - position: usize, - shuffle: ShuffleMode, - shuffled_indices: Vec, -} - -impl Queue { - fn new() -> Self { - Queue { - tracks: Vec::new(), - position: 0, - shuffle: ShuffleMode::None, - shuffled_indices: Vec::new(), - } - } - - fn add_track(&mut self, path: PathBuf) { - self.tracks.push(Track::from_path(path)); - self.update_shuffle_indices(); - } - - fn insert_after_current(&mut self, path: PathBuf) { - if self.tracks.is_empty() { - self.tracks.push(Track::from_path(path)); - } else { - let insert_idx = self.position + 1; - self.tracks.insert(insert_idx, Track::from_path(path)); - } - self.update_shuffle_indices(); - } - - fn track_at(&self, idx: usize) -> Option<&Track> { - self.tracks.get(idx) - } - - fn set_shuffle(&mut self, mode: ShuffleMode) { - self.shuffle = mode.clone(); - self.update_shuffle_indices(); - } - - fn update_shuffle_indices(&mut self) { - let len = self.tracks.len(); - self.shuffled_indices = (0..len).collect(); - if let ShuffleMode::All = self.shuffle { - use rand::seq::SliceRandom; - let mut rng = rand::rng(); - let pos = self.position.min(len.saturating_sub(1)); - if pos + 1 < len { - self.shuffled_indices[pos + 1..].shuffle(&mut rng); - } - } - } - - fn current(&self) -> Option<&Track> { - if self.tracks.is_empty() { - None - } else if let ShuffleMode::All = self.shuffle { - self.shuffled_indices - .get(self.position) - .and_then(|&idx| self.tracks.get(idx)) - } else if let ShuffleMode::Album = self.shuffle { - // TODO: When database is ready, only shuffle tracks in same album/folder - // For now, behave like All - self.shuffled_indices - .get(self.position) - .and_then(|&idx| self.tracks.get(idx)) - } else { - self.tracks.get(self.position) - } - } - - fn next(&mut self) -> Option { - let max_pos = self.tracks.len().saturating_sub(1); - if self.position < max_pos { - self.position += 1; - self.current().map(|t| t.path.clone()) - } else { - None - } - } - - fn previous(&mut self) -> Option { - if self.position > 0 { - self.position -= 1; - self.current().map(|t| t.path.clone()) - } else { - None - } - } -} - pub fn start_daemon() -> Result<(), Box> { let config = Config::load()?; let socket_path = config.get_socket_path(); @@ -268,13 +58,13 @@ pub fn start_daemon() -> Result<(), Box> { if let Some(vol) = config.volume { audio_engine.set_volume(vol as f32 / 100.0); } - if let Some(ref shuffle_str) = config.shuffle { - if let Ok(mut q) = queue.lock() { - match shuffle_str.as_str() { - "on" | "all" => q.set_shuffle(ShuffleMode::All), - "album" => q.set_shuffle(ShuffleMode::Album), - _ => {} - } + if let Some(ref shuffle_str) = config.shuffle + && let Ok(mut q) = queue.lock() + { + match shuffle_str.as_str() { + "on" | "all" => q.set_shuffle(ShuffleMode::All), + "album" => q.set_shuffle(ShuffleMode::Album), + _ => {} } } @@ -285,13 +75,11 @@ pub fn start_daemon() -> Result<(), Box> { use std::time::{Duration, Instant}; let start = Instant::now(); while start.elapsed() < Duration::from_millis(timeout_ms) { - // If playing and the current track matches, assume started - if audio.is_playing() { - if let Some(cur) = audio.current_track() { - if &cur == target { - return true; - } - } + if audio.is_playing() + && let Some(cur) = audio.current_track() + && &cur == target + { + return true; } std::thread::sleep(Duration::from_millis(10)); } @@ -304,36 +92,27 @@ pub fn start_daemon() -> Result<(), Box> { } // Check if track ended and play next (robust with startup feedback) - if audio_engine.poll_track_ended() { - if let Ok(mut q) = queue.lock() { - // Save the current position in case we need to revert - let original_position = q.position; - // Try a handful of next candidates until one starts - let mut started = false; - // We'll attempt up to 4 candidates ahead - let max_trials = 4usize; - for _ in 0..max_trials { - if let Some(next_track) = q.next() { - audio_engine.play(next_track.clone()); - // Wait a short time to see if playback actually starts - if wait_for_start(&audio_engine, &next_track, 500) { - started = true; - break; - } else { - // Try next candidate - q.next() already advanced position - continue; - } - } else { + if audio_engine.poll_track_ended() + && let Ok(mut q) = queue.lock() + { + let original_position = q.position(); + let mut started = false; + let max_trials = 4usize; + for _ in 0..max_trials { + if let Some(next_track) = q.next() { + audio_engine.play(next_track.clone()); + if wait_for_start(&audio_engine, &next_track, 500) { + started = true; break; } - } - if !started { - // Revert position since we couldn't start any track - q.position = original_position; - // No candidate started; stop playback safely - audio_engine.stop(); + } else { + break; } } + if !started { + q.set_position(original_position); + audio_engine.stop(); + } } match listener.accept() { @@ -494,10 +273,8 @@ fn process_command( } "stop" => { audio.stop(); - // Clear queue on stop if let Ok(mut q) = queue.lock() { - q.tracks.clear(); - q.position = 0; + q.clear(); } "Stopped\n".to_string() } @@ -533,7 +310,7 @@ fn process_command( } "shuffle" => { if let Ok(q) = queue.lock() { - match q.shuffle { + match q.shuffle_mode() { ShuffleMode::None => "Shuffle: off\n".to_string(), ShuffleMode::All => "Shuffle: on (all)\n".to_string(), ShuffleMode::Album => "Shuffle: on (album)\n".to_string(), @@ -591,14 +368,15 @@ fn process_command( let path = std::path::PathBuf::from(path_str.clone()); if path.is_file() { if let Ok(mut q) = queue.lock() { - let was_empty = q.tracks.is_empty(); + let was_empty = q.is_empty(); match position { "now" => { q.insert_after_current(path.clone()); if !was_empty { - q.position += 1; + let new_pos = q.position() + 1; + q.set_position(new_pos); } - if let Some(track) = q.track_at(q.position) { + if let Some(track) = q.track_at(q.position()) { audio.play(track.path.clone()); } format!("Playing now: {}\n", path_str) @@ -608,9 +386,14 @@ fn process_command( format!("Queued next: {}\n", path_str) } _ => { + let was_playing = audio.is_playing(); q.add_track(path.clone()); - audio.play(path); - format!("Playing: {}\n", path_str) + if !was_playing { + audio.play(path); + format!("Playing: {}\n", path_str) + } else { + format!("Queued: {}\n", path_str) + } } } } else { @@ -620,16 +403,17 @@ fn process_command( match find_audio_in_dir(&path) { Some(files) if !files.is_empty() => { if let Ok(mut q) = queue.lock() { - let was_empty = q.tracks.is_empty(); + let was_empty = q.is_empty(); match position { "now" => { - for file in &files { + for file in files.iter().rev() { q.insert_after_current(file.clone()); } if !was_empty { - q.position += 1; + let new_pos = q.position() + 1; + q.set_position(new_pos); } - if let Some(track) = q.track_at(q.position) { + if let Some(track) = q.track_at(q.position()) { audio.play(track.path.clone()); } format!( @@ -639,7 +423,7 @@ fn process_command( ) } "next" => { - for file in &files { + for file in files.iter().rev() { q.insert_after_current(file.clone()); } format!( @@ -652,10 +436,10 @@ fn process_command( for file in &files { q.add_track(file.clone()); } - if !audio.is_playing() { - if let Some(track) = files.first() { - audio.play(track.clone()); - } + if !audio.is_playing() + && let Some(track) = files.first() + { + audio.play(track.clone()); } format!("Queued {} tracks from: {}\n", files.len(), path_str) } @@ -687,43 +471,31 @@ fn process_command( "Usage: volume <0-100>\n".to_string() } } - _ if cmd.starts_with("shuffle ") => { - let mode_str = cmd[8..].trim(); - if let Ok(mut q) = queue.lock() { - if mode_str == "on" || mode_str == "all" { - q.set_shuffle(ShuffleMode::All); - "Shuffle: on\n".to_string() - } else if mode_str == "off" || mode_str == "none" { - q.set_shuffle(ShuffleMode::None); - "Shuffle: off\n".to_string() - } else { - "Usage: shuffle [on|off]\n".to_string() - } - } else { - "Error accessing queue\n".to_string() - } - } _ if cmd.starts_with("queue") => { if let Ok(q) = queue.lock() { - if q.tracks.is_empty() { + if q.is_empty() { "Queue is empty\n".to_string() } else { let mut output = String::new(); - let current_position = q.position; - let playing_track_idx = match q.shuffle { + let current_position = q.position(); + let playing_track_idx = match q.shuffle_mode() { ShuffleMode::None => current_position, - _ => *q.shuffled_indices.get(current_position).unwrap_or(&0), + _ => q + .shuffled_indices() + .get(current_position) + .copied() + .unwrap_or(0), }; - // Use shuffled_indices for display order if shuffle is on - let indices: Vec = if let ShuffleMode::None = q.shuffle { - (0..q.tracks.len()).collect() + let indices: Vec = if *q.shuffle_mode() == ShuffleMode::None { + (0..q.len()).collect() } else { - q.shuffled_indices.clone() + q.shuffled_indices().to_vec() }; + let is_playing = audio.is_playing(); for (display_idx, &track_idx) in indices.iter().enumerate() { - if let Some(track) = q.tracks.get(track_idx) { + if let Some(track) = q.track_at(track_idx) { let track_str = track.display_string(display_idx); - if track_idx == playing_track_idx { + if track_idx == playing_track_idx && is_playing { output.push_str(&format!("> {} (playing)\n", track_str)); } else { output.push_str(&format!(" {}\n", track_str)); @@ -766,18 +538,18 @@ fn find_audio_in_dir(dir: &Path) -> Option> { for entry in entries { let path = entry.path(); if path.is_file() { - if let Some(ext) = path.extension().and_then(OsStr::to_str) { - if matches!( + if let Some(ext) = path.extension().and_then(OsStr::to_str) + && matches!( ext.to_lowercase().as_str(), "mp3" | "flac" | "ogg" | "m4a" | "wav" | "opus" - ) { - results.push(path); - } - } - } else if path.is_dir() { - if let Some(mut found) = find_audio_in_dir(&path) { - results.append(&mut found); + ) + { + results.push(path); } + } else if path.is_dir() + && let Some(mut found) = find_audio_in_dir(&path) + { + results.append(&mut found); } } } diff --git a/src/lib.rs b/src/lib.rs index 9317bcd..cc5d7bf 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,4 +4,5 @@ pub mod config; pub mod daemon; pub mod database; pub mod playback; +pub mod queue; pub mod scrobbler; diff --git a/src/main.rs b/src/main.rs index 2b88c44..aac80f0 100644 --- a/src/main.rs +++ b/src/main.rs @@ -4,6 +4,7 @@ mod config; mod daemon; mod database; mod playback; +mod queue; mod scrobbler; use clap::Parser; diff --git a/src/queue.rs b/src/queue.rs new file mode 100644 index 0000000..53b104f --- /dev/null +++ b/src/queue.rs @@ -0,0 +1,250 @@ +use rand::seq::SliceRandom; +use std::path::{Path, PathBuf}; + +#[derive(Debug, Clone, PartialEq)] +pub enum ShuffleMode { + None, + All, + Album, +} + +#[derive(Debug, Default, Clone)] +pub struct AudioMetadata { + pub title: Option, + pub artist: Option, + pub album: Option, +} + +pub fn extract_metadata(path: &Path) -> Option { + use symphonia::core::io::MediaSourceStream; + use symphonia::core::meta::StandardTagKey; + use symphonia::core::probe::Hint; + + let file = std::fs::File::open(path).ok()?; + let mss = MediaSourceStream::new(Box::new(file), Default::default()); + + let hint = Hint::new(); + let mut probed = symphonia::default::get_probe() + .format(&hint, mss, &Default::default(), &Default::default()) + .ok()?; + + let mut metadata = AudioMetadata::default(); + + if let Some(meta) = probed.metadata.get() + && let Some(rev) = meta.current() + { + for tag in rev.tags() { + match tag.std_key { + Some(StandardTagKey::TrackTitle) => { + metadata.title = Some(tag.value.to_string()); + } + Some(StandardTagKey::Artist) => { + metadata.artist = Some(tag.value.to_string()); + } + Some(StandardTagKey::Album) => { + metadata.album = Some(tag.value.to_string()); + } + _ => {} + } + } + } + + if metadata.title.is_none() && metadata.artist.is_none() && metadata.album.is_none() { + let mut format = probed.format; + if let Some(rev) = format.metadata().current() { + for tag in rev.tags() { + match tag.std_key { + Some(StandardTagKey::TrackTitle) => { + metadata.title = Some(tag.value.to_string()); + } + Some(StandardTagKey::Artist) => { + metadata.artist = Some(tag.value.to_string()); + } + Some(StandardTagKey::Album) => { + metadata.album = Some(tag.value.to_string()); + } + _ => {} + } + } + } + } + + if metadata.title.is_some() || metadata.artist.is_some() || metadata.album.is_some() { + Some(metadata) + } else { + None + } +} + +#[derive(Debug, Clone)] +pub struct Track { + pub path: PathBuf, + pub title: Option, + pub artist: Option, + pub album: Option, +} + +impl Track { + pub fn from_path(path: PathBuf) -> Self { + let metadata = extract_metadata(&path); + let metadata = metadata.as_ref(); + Track { + path, + title: metadata.and_then(|m| m.title.clone()), + artist: metadata.and_then(|m| m.artist.clone()), + album: metadata.and_then(|m| m.album.clone()), + } + } + + pub fn display_string(&self, index: usize) -> String { + let album_artist = self + .artist + .clone() + .or_else(|| self.album.clone()) + .unwrap_or_default(); + let title = self.title.clone().unwrap_or_else(|| { + self.path + .file_name() + .and_then(|n| n.to_str()) + .unwrap_or("Unknown") + .to_string() + }); + + if album_artist.is_empty() { + format!("{}. {}", index + 1, title) + } else { + format!("{}. {} - {}", index + 1, album_artist, title) + } + } +} + +pub struct Queue { + tracks: Vec, + position: usize, + shuffle: ShuffleMode, + shuffled_indices: Vec, +} + +impl Default for Queue { + fn default() -> Self { + Self::new() + } +} + +impl Queue { + pub fn new() -> Self { + Queue { + tracks: Vec::new(), + position: 0, + shuffle: ShuffleMode::None, + shuffled_indices: Vec::new(), + } + } + + pub fn add_track(&mut self, path: PathBuf) { + self.tracks.push(Track::from_path(path)); + self.update_shuffle_indices(); + } + + pub fn insert_after_current(&mut self, path: PathBuf) { + if self.tracks.is_empty() { + self.tracks.push(Track::from_path(path)); + } else { + let insert_idx = self.position + 1; + self.tracks.insert(insert_idx, Track::from_path(path)); + } + self.update_shuffle_indices(); + } + + pub fn track_at(&self, idx: usize) -> Option<&Track> { + self.tracks.get(idx) + } + + pub fn set_shuffle(&mut self, mode: ShuffleMode) { + self.shuffle = mode; + self.update_shuffle_indices(); + } + + fn update_shuffle_indices(&mut self) { + let len = self.tracks.len(); + self.shuffled_indices = (0..len).collect(); + if let ShuffleMode::All = self.shuffle { + let mut rng = rand::rng(); + let pos = self.position.min(len.saturating_sub(1)); + if pos + 1 < len { + self.shuffled_indices[pos + 1..].shuffle(&mut rng); + } + } + } + + pub fn current(&self) -> Option<&Track> { + if self.tracks.is_empty() { + None + } else if self.shuffle == ShuffleMode::All { + self.shuffled_indices + .get(self.position) + .and_then(|&idx| self.tracks.get(idx)) + } else if self.shuffle == ShuffleMode::Album { + self.shuffled_indices + .get(self.position) + .and_then(|&idx| self.tracks.get(idx)) + } else { + self.tracks.get(self.position) + } + } + + #[allow(clippy::should_implement_trait)] + pub fn next(&mut self) -> Option { + let max_pos = self.tracks.len().saturating_sub(1); + if self.position < max_pos { + self.position += 1; + self.current().map(|t| t.path.clone()) + } else { + None + } + } + + pub fn previous(&mut self) -> Option { + if self.position > 0 { + self.position -= 1; + self.current().map(|t| t.path.clone()) + } else { + None + } + } + + pub fn is_empty(&self) -> bool { + self.tracks.is_empty() + } + + pub fn len(&self) -> usize { + self.tracks.len() + } + + pub fn position(&self) -> usize { + self.position + } + + pub fn set_position(&mut self, pos: usize) { + self.position = pos; + } + + #[allow(dead_code)] + pub fn tracks(&self) -> &[Track] { + &self.tracks + } + + pub fn shuffle_mode(&self) -> &ShuffleMode { + &self.shuffle + } + + pub fn shuffled_indices(&self) -> &[usize] { + &self.shuffled_indices + } + + pub fn clear(&mut self) { + self.tracks.clear(); + self.position = 0; + self.shuffled_indices.clear(); + } +} -- cgit v1.2.3