From 956e3be3808f918923aef3259ea5b28bfc055b03 Mon Sep 17 00:00:00 2001 From: salmonik Date: Fri, 22 May 2026 17:57:08 +0200 Subject: Fix metadata extraction for symphonia 0.6 and add Albums shuffle mode --- Cargo.toml | 2 +- completions/rsp.bash | 3 +- completions/rsp.fish | 1 + completions/rsp.zsh | 2 +- src/config.rs | 2 +- src/daemon.rs | 7 ++- src/playback.rs | 1 + src/queue.rs | 130 ++++++++++++++++++++++++++++++++++++++++++--------- tests/playback.rs | 2 + tests/queue.rs | 105 +++++++++++++++++++++++++++++++++++++++++ 10 files changed, 227 insertions(+), 28 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 86a9b54..62acef4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,7 +23,7 @@ directories = "6.0" serde = { version = "1.0.228", features = ["derive"] } toml = "1.1.2" reqwest = { version = "0.13.3", features = ["json", "form", "blocking"] } -symphonia = { version = "0.6", default-features = false, features = ["mp3", "flac", "vorbis", "wav", "ogg", "opt-simd"] } +symphonia = { version = "0.6", default-features = false, features = ["mp3", "flac", "vorbis", "wav", "ogg", "opt-simd", "id3v1", "id3v2", "ape"] } cpal = "0.17.3" chrono = "0.4.44" serde_json = "1.0.149" diff --git a/completions/rsp.bash b/completions/rsp.bash index c4e295f..f5c87dc 100644 --- a/completions/rsp.bash +++ b/completions/rsp.bash @@ -766,7 +766,8 @@ _rsp() { fi case "${prev}" in *) - COMPREPLY=() + COMPREPLY=( $(compgen -W "on off all none album albums" -- "${cur}") ) + return 0 ;; esac COMPREPLY=( $(compgen -W "${opts}" -- "${cur}") ) diff --git a/completions/rsp.fish b/completions/rsp.fish index 8764bb9..56f3766 100644 --- a/completions/rsp.fish +++ b/completions/rsp.fish @@ -56,6 +56,7 @@ complete -c rsp -n "__fish_rsp_using_subcommand next" -s h -l help -d 'Print hel complete -c rsp -n "__fish_rsp_using_subcommand previous" -s h -l help -d 'Print help' complete -c rsp -n "__fish_rsp_using_subcommand volume" -s h -l help -d 'Print help' complete -c rsp -n "__fish_rsp_using_subcommand shuffle" -s h -l help -d 'Print help' +complete -c rsp -n "__fish_rsp_using_subcommand shuffle" -f -a "on off all none album albums" -d 'Shuffle mode' complete -c rsp -n "__fish_rsp_using_subcommand queue" -s h -l help -d 'Print help' complete -c rsp -n "__fish_rsp_using_subcommand status" -s h -l help -d 'Print help' complete -c rsp -n "__fish_rsp_using_subcommand scan" -l paths -r -F diff --git a/completions/rsp.zsh b/completions/rsp.zsh index dd24572..87fbfcd 100644 --- a/completions/rsp.zsh +++ b/completions/rsp.zsh @@ -102,7 +102,7 @@ _arguments "${_arguments_options[@]}" : \ _arguments "${_arguments_options[@]}" : \ '-h[Print help]' \ '--help[Print help]' \ -'::mode:_default' \ +'::mode:(on off all none album albums)' \ && ret=0 ;; (queue) diff --git a/src/config.rs b/src/config.rs index 5fd2f1b..63b0164 100644 --- a/src/config.rs +++ b/src/config.rs @@ -200,7 +200,7 @@ impl Config { self.volume = Some(value.parse()?); } "shuffle" => { - if value == "on" || value == "all" || value == "album" { + if value == "on" || value == "all" || value == "album" || value == "albums" { self.shuffle = Some(value.to_string()); } else if value == "off" || value == "none" { self.shuffle = None; diff --git a/src/daemon.rs b/src/daemon.rs index 8b74357..b18ecc9 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -248,6 +248,7 @@ pub fn start_daemon() -> Result<(), Box> { match shuffle_str.as_str() { "on" | "all" => q.set_shuffle(ShuffleMode::All), "album" => q.set_shuffle(ShuffleMode::Album), + "albums" => q.set_shuffle(ShuffleMode::Albums), _ => {} } } @@ -633,6 +634,7 @@ fn process_command( ShuffleMode::None => "Shuffle: off\n".to_string(), ShuffleMode::All => "Shuffle: on (all)\n".to_string(), ShuffleMode::Album => "Shuffle: on (album)\n".to_string(), + ShuffleMode::Albums => "Shuffle: on (albums)\n".to_string(), } } else { "Error accessing queue\n".to_string() @@ -647,11 +649,14 @@ fn process_command( } else if mode_str == "album" { q.set_shuffle(ShuffleMode::Album); "Shuffle: on (album)\n".to_string() + } else if mode_str == "albums" { + q.set_shuffle(ShuffleMode::Albums); + "Shuffle: on (albums)\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|album]\n".to_string() + "Usage: shuffle [on|off|album|albums]\n".to_string() }; // Save to config if let Ok(mut config) = Config::load() { diff --git a/src/playback.rs b/src/playback.rs index d9dd1f9..f16bf8f 100644 --- a/src/playback.rs +++ b/src/playback.rs @@ -24,6 +24,7 @@ pub enum ShuffleMode { None, All, Album, + Albums, } #[allow(dead_code)] diff --git a/src/queue.rs b/src/queue.rs index b0acf5f..9d09fec 100644 --- a/src/queue.rs +++ b/src/queue.rs @@ -6,6 +6,7 @@ pub enum ShuffleMode { None, All, Album, + Albums, } #[derive(Debug, Default, Clone)] @@ -46,28 +47,8 @@ pub fn extract_metadata(path: &Path) -> Option { }; { - let meta = probed.metadata(); - if let Some(rev) = meta.current() { - for tag in &rev.media.tags { - match &tag.std { - Some(StandardTag::TrackTitle(value)) => { - metadata.title = Some(value.to_string()); - } - Some(StandardTag::Artist(value)) => { - metadata.artist = Some(value.to_string()); - } - Some(StandardTag::Album(value)) => { - metadata.album = Some(value.to_string()); - } - _ => {} - } - } - } - } - - if metadata.title.is_none() && metadata.artist.is_none() && metadata.album.is_none() { - let meta = probed.metadata(); - if let Some(rev) = meta.current() { + let mut meta = probed.metadata(); + if let Some(rev) = meta.skip_to_latest() { for tag in &rev.media.tags { match &tag.std { Some(StandardTag::TrackTitle(value)) => { @@ -183,6 +164,12 @@ impl Queue { self.update_shuffle_indices(); } + #[allow(dead_code)] + pub fn push_track(&mut self, track: Track) { + self.tracks.push(track); + 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)); @@ -213,6 +200,11 @@ impl Queue { self.shuffle = ShuffleMode::Album; self.update_shuffle_indices_album(); } + ShuffleMode::Albums => { + self.original_tracks = self.tracks.clone(); + self.shuffle = ShuffleMode::Albums; + self.update_shuffle_indices_albums(); + } } } @@ -268,6 +260,9 @@ impl Queue { ShuffleMode::Album => { self.update_shuffle_indices_album(); } + ShuffleMode::Albums => { + self.update_shuffle_indices_albums(); + } ShuffleMode::None => {} } } @@ -285,6 +280,62 @@ impl Queue { } } + fn update_shuffle_indices_albums(&mut self) { + let len = self.tracks.len(); + self.shuffled_indices = (0..len).collect(); + + if len <= 1 { + return; + } + + let pos = self.position.min(len.saturating_sub(1)); + if pos + 1 >= len { + return; + } + + let range_start = pos + 1; + let mut rng = rand::rng(); + + // Build album groups within the range, shuffling tracks within each group + let mut groups: Vec> = Vec::new(); + let mut current_group: Vec = Vec::new(); + let mut current_album: Option<&str> = None; + + for i in range_start..len { + let album = self.tracks.get(i).and_then(|t| t.album.as_deref()); + + if current_group.is_empty() { + current_group.push(i); + current_album = album; + } else if album != current_album { + // Shuffle tracks within this album group before adding it + if current_group.len() > 1 { + current_group.shuffle(&mut rng); + } + groups.push(std::mem::take(&mut current_group)); + current_group.push(i); + current_album = album; + } else { + current_group.push(i); + } + } + if !current_group.is_empty() { + if current_group.len() > 1 { + current_group.shuffle(&mut rng); + } + groups.push(current_group); + } + + // Shuffle the album group order + groups.shuffle(&mut rng); + + let mut new_indices: Vec = self.shuffled_indices[..=pos].to_vec(); + for group in &groups { + new_indices.extend(group); + } + self.shuffled_indices = new_indices; + } + fn shuffle_album_range(&mut self, range: std::ops::Range, rng: &mut impl rand::Rng) { if range.start >= range.end { return; @@ -327,7 +378,7 @@ impl Queue { self.shuffled_indices .get(self.position) .and_then(|&idx| self.tracks.get(idx)) - } else if self.shuffle == ShuffleMode::Album { + } else if self.shuffle == ShuffleMode::Album || self.shuffle == ShuffleMode::Albums { self.shuffled_indices .get(self.position) .and_then(|&idx| self.tracks.get(idx)) @@ -397,6 +448,39 @@ impl Queue { ShuffleMode::Album => { self.shuffle_album_range(0..self.tracks.len(), &mut rng); } + ShuffleMode::Albums => { + let mut groups: Vec> = Vec::new(); + let mut current_group: Vec = Vec::new(); + let mut current_album: Option<&str> = None; + + for i in 0..self.tracks.len() { + let album = self.tracks.get(i).and_then(|t| t.album.as_deref()); + + if current_group.is_empty() { + current_group.push(i); + current_album = album; + } else if album != current_album { + if current_group.len() > 1 { + current_group.shuffle(&mut rng); + } + groups.push(std::mem::take(&mut current_group)); + current_group.push(i); + current_album = album; + } else { + current_group.push(i); + } + } + if !current_group.is_empty() { + if current_group.len() > 1 { + current_group.shuffle(&mut rng); + } + groups.push(current_group); + } + + groups.shuffle(&mut rng); + + self.shuffled_indices = groups.concat(); + } ShuffleMode::None => {} } } diff --git a/tests/playback.rs b/tests/playback.rs index 80ecf58..1513a4d 100644 --- a/tests/playback.rs +++ b/tests/playback.rs @@ -127,6 +127,8 @@ fn test_player_set_shuffle() { let mut player = Player::new(); player.set_shuffle(ShuffleMode::All); assert_eq!(player.shuffle, ShuffleMode::All); + player.set_shuffle(ShuffleMode::Albums); + assert_eq!(player.shuffle, ShuffleMode::Albums); } #[test] diff --git a/tests/queue.rs b/tests/queue.rs index ccc6e54..2dd9f8c 100644 --- a/tests/queue.rs +++ b/tests/queue.rs @@ -286,6 +286,8 @@ fn test_shuffle_modes() { assert_eq!(*q.shuffle_mode(), ShuffleMode::All); q.set_shuffle(ShuffleMode::Album); assert_eq!(*q.shuffle_mode(), ShuffleMode::Album); + q.set_shuffle(ShuffleMode::Albums); + assert_eq!(*q.shuffle_mode(), ShuffleMode::Albums); q.set_shuffle(ShuffleMode::None); assert_eq!(*q.shuffle_mode(), ShuffleMode::None); } @@ -486,6 +488,109 @@ fn test_track_no_metadata_fields() { assert!(t.album.is_none()); } +// ---------- Shuffle albums ---------- + +#[test] +fn test_shuffle_albums_groups_tracks_by_album() { + let mut q = Queue::new(); + q.push_track(track( + "/music/a1.mp3", + Some("A1"), + Some("Artist"), + Some("Album A"), + )); + q.push_track(track( + "/music/a2.mp3", + Some("A2"), + Some("Artist"), + Some("Album A"), + )); + q.push_track(track( + "/music/b1.mp3", + Some("B1"), + Some("Artist"), + Some("Album B"), + )); + q.push_track(track( + "/music/b2.mp3", + Some("B2"), + Some("Artist"), + Some("Album B"), + )); + q.push_track(track( + "/music/c1.mp3", + Some("C1"), + Some("Artist"), + Some("Album C"), + )); + // Queue: Album A (a1, a2), Album B (b1, b2), Album C (c1) + + q.set_shuffle(ShuffleMode::Albums); + q.shuffle_initial(); // full shuffle of everything (as happens at play start) + + let shuffled = q.shuffled_indices().to_vec(); + assert_eq!(shuffled.len(), 5); + + // Tracks from the same album must appear as a contiguous block + let shuffled_tracks: Vec<_> = shuffled.iter().map(|&i| q.track_at(i).unwrap()).collect(); + + // Find positions of all tracks + let a_positions: Vec<_> = shuffled_tracks + .iter() + .enumerate() + .filter(|(_, t)| t.album.as_deref() == Some("Album A")) + .map(|(i, _)| i) + .collect(); + let b_positions: Vec<_> = shuffled_tracks + .iter() + .enumerate() + .filter(|(_, t)| t.album.as_deref() == Some("Album B")) + .map(|(i, _)| i) + .collect(); + + assert_eq!(a_positions.len(), 2, "Album A should have 2 tracks"); + assert_eq!(b_positions.len(), 2, "Album B should have 2 tracks"); + assert_eq!( + a_positions[1] - a_positions[0], + 1, + "Album A tracks should be contiguous" + ); + assert_eq!( + b_positions[1] - b_positions[0], + 1, + "Album B tracks should be contiguous" + ); +} + +#[test] +fn test_shuffle_albums_solo_track_forms_own_group() { + let mut q = Queue::new(); + q.push_track(track("/music/x.mp3", Some("X"), None, None)); + q.push_track(track("/music/y.mp3", Some("Y"), None, Some("Album Y"))); + q.push_track(track("/music/z.mp3", Some("Z"), None, Some("Album Y"))); + + q.set_shuffle(ShuffleMode::Albums); + let shuffled = q.shuffled_indices().to_vec(); + assert_eq!(shuffled.len(), 3); + + // Track without album should form its own group of size 1 + // Tracks y and z share Album Y and should be adjacent + let shuffled_tracks: Vec<_> = shuffled.iter().map(|&i| q.track_at(i).unwrap()).collect(); + let y_pos = shuffled_tracks + .iter() + .position(|t| t.title.as_deref() == Some("Y")) + .unwrap(); + let z_pos = shuffled_tracks + .iter() + .position(|t| t.title.as_deref() == Some("Z")) + .unwrap(); + assert_eq!( + (y_pos as isize - z_pos as isize).abs(), + 1, + "tracks from same album must be adjacent" + ); +} + // ---------- Shuffle restore ---------- #[test] -- cgit v1.2.3