Fix metadata extraction for symphonia 0.6 and add Albums shuffle mode

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<dyn std::error::Error>> {
         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<AudioMetadata> {
     };
 
     {
-        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<usize>> = Vec::new();
+        let mut current_group: Vec<usize> = 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<usize> = 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<usize>, 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<usize>> = Vec::new();
+                let mut current_group: Vec<usize> = 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]