"album playlist" function works with disc numbers

src/library.rs

@@ -1085,15 +1085,15 @@ impl<Config: AppConfigTrait, D: ?Sized + DecoderTrait> Library<Config, D> {
                 audio_file_path, id
                 from song where album = ? and analyzed = true and version = ?
                 order
-                by track_number;
+                by disc_number, track_number;
             ";
 
         // Get the song's analysis, and attach it to the existing song.
         let features_statement = "
             select
                 feature, song.id from feature join song on song.id = feature.song_id
                 where album=? and analyzed = true and version = ?
-                order by track_number;
+                order by disc_number, track_number;
             ";
         let songs = self._songs_from_statement(songs_statement, features_statement, params)?;
         if songs.is_empty() {
@@ -1565,10 +1565,10 @@ mod test {
             path: "/path/to/song2201".into(),
             artist: Some("Artist2001".into()),
             title: Some("Title2001".into()),
-            album: Some("Remixes of Album2001".into()),
+            album: Some("An Album2001".into()),
             album_artist: Some("An Album Artist2001".into()),
-            track_number: Some(2),
-            disc_number: Some(1),
+            track_number: Some(1),
+            disc_number: Some(2),
             genre: Some("Electronica2001".into()),
             analysis: Analysis {
                 internal_analysis: analysis_vector,
@@ -1760,8 +1760,8 @@ mod test {
                         \"/path/to/charlie2001\"}', null, null
                     ),
                     (
-                        2201, '/path/to/song2201', 'Artist2001', 'Title2001', 'Remixes of Album2001',
-                        'An Album Artist2001', 2, 1, 'Electronica2001', 410, true,
+                        2201, '/path/to/song2201', 'Artist2001', 'Title2001', 'An Album2001',
+                        'An Album Artist2001', 1, 2, 'Electronica2001', 410, true,
                         1, '{\"ignore\": false, \"metadata_bliss_does_not_have\":
                         \"/path/to/charlie2201\"}', null, null
                     ),
@@ -2287,10 +2287,10 @@ mod test {
                 // First album.
                 "/path/to/song5001".to_string(),
                 "/path/to/song1001".to_string(),
-                // Second album, well ordered.
+                // Second album, well ordered, disc 1
                 "/path/to/song6001".to_string(),
                 "/path/to/song2001".to_string(),
-                // Seecond album, remixes
+                // Seecond album disc 2
                 "/path/to/song2201".to_string(),
                 // Third album.
                 "/path/to/song7001".to_string(),
@@ -2320,6 +2320,7 @@ mod test {
                 // Second album, well ordered.
                 "/path/to/song6001".to_string(),
                 "/path/to/song2001".to_string(),
+                "/path/to/song2201".to_string(),
             ],
             album
                 .into_iter()

src/playlist.rs

@@ -324,12 +324,14 @@ pub fn closest_album_to_group<T: AsRef<Song> + Clone>(
         al.sort_by(|s1, s2| {
             let track_number1 = s1.as_ref().track_number.to_owned();
             let track_number2 = s2.as_ref().track_number.to_owned();
-            if let Some(x) = track_number1 {
-                if let Some(y) = track_number2 {
-                    return x.cmp(&y);
-                }
+            let disc_number1 = s1.as_ref().disc_number.to_owned();
+            let disc_number2 = s2.as_ref().disc_number.to_owned();
+            let track_comparison = track_number1.cmp(&track_number2);
+            if disc_number1.cmp(&disc_number2) == std::cmp::Ordering::Equal {
+                track_comparison
+            } else {
+                disc_number1.cmp(&disc_number2)
             }
-            std::cmp::Ordering::Equal
         });
         playlist.extend(al);
     }
@@ -813,36 +815,57 @@ mod test {
             album: Some(String::from("Album")),
             artist: Some(String::from("Artist")),
             track_number: Some(1),
+            disc_number: Some(1),
             ..Default::default()
         };
-
         let second_song = Song {
-            path: Path::new("path-to-second").to_path_buf(),
-            analysis: Analysis::new([0.1; 20]),
-            album: Some(String::from("Another Album")),
-            artist: Some(String::from("Artist")),
-            track_number: Some(10),
-            ..Default::default()
-        };
-
-        let third_song = Song {
             path: Path::new("path-to-third").to_path_buf(),
             analysis: Analysis::new([10.; 20]),
             album: Some(String::from("Album")),
             artist: Some(String::from("Another Artist")),
             track_number: Some(2),
+            disc_number: Some(1),
             ..Default::default()
         };
 
-        let fourth_song = Song {
+        let first_song_other_album_disc_1 = Song {
+            path: Path::new("path-to-second-2").to_path_buf(),
+            analysis: Analysis::new([0.15; 20]),
+            album: Some(String::from("Another Album")),
+            artist: Some(String::from("Artist")),
+            track_number: Some(1),
+            disc_number: Some(1),
+            ..Default::default()
+        };
+        let second_song_other_album_disc_1 = Song {
+            path: Path::new("path-to-second").to_path_buf(),
+            analysis: Analysis::new([0.1; 20]),
+            album: Some(String::from("Another Album")),
+            artist: Some(String::from("Artist")),
+            track_number: Some(2),
+            disc_number: Some(1),
+            ..Default::default()
+        };
+        let first_song_other_album_disc_2 = Song {
             path: Path::new("path-to-fourth").to_path_buf(),
             analysis: Analysis::new([20.; 20]),
             album: Some(String::from("Another Album")),
             artist: Some(String::from("Another Artist")),
             track_number: Some(1),
+            disc_number: Some(2),
             ..Default::default()
         };
-        let fifth_song = Song {
+        let second_song_other_album_disc_2 = Song {
+            path: Path::new("path-to-fourth").to_path_buf(),
+            analysis: Analysis::new([20.; 20]),
+            album: Some(String::from("Another Album")),
+            artist: Some(String::from("Another Artist")),
+            track_number: Some(4),
+            disc_number: Some(2),
+            ..Default::default()
+        };
+
+        let song_no_album = Song {
             path: Path::new("path-to-fifth").to_path_buf(),
             analysis: Analysis::new([40.; 20]),
             artist: Some(String::from("Third Artist")),
@@ -852,18 +875,22 @@ mod test {
 
         let pool = vec![
             first_song.to_owned(),
-            fourth_song.to_owned(),
-            third_song.to_owned(),
+            second_song_other_album_disc_1.to_owned(),
+            second_song_other_album_disc_2.to_owned(),
             second_song.to_owned(),
-            fifth_song.to_owned(),
+            first_song_other_album_disc_2.to_owned(),
+            first_song_other_album_disc_1.to_owned(),
+            song_no_album.to_owned(),
         ];
-        let group = vec![first_song.to_owned(), third_song.to_owned()];
+        let group = vec![first_song.to_owned(), second_song.to_owned()];
         assert_eq!(
             vec![
                 first_song.to_owned(),
-                third_song.to_owned(),
-                fourth_song.to_owned(),
-                second_song.to_owned()
+                second_song.to_owned(),
+                first_song_other_album_disc_1.to_owned(),
+                second_song_other_album_disc_1.to_owned(),
+                first_song_other_album_disc_2.to_owned(),
+                second_song_other_album_disc_2.to_owned(),
             ],
             closest_album_to_group(group, pool.to_owned()).unwrap(),
         );
@@ -876,34 +903,46 @@ mod test {
             bliss_song: second_song,
             something: true,
         };
-        let third_song = CustomSong {
-            bliss_song: third_song,
+
+        let first_song_other_album_disc_1 = CustomSong {
+            bliss_song: first_song_other_album_disc_1,
             something: true,
         };
-        let fourth_song = CustomSong {
-            bliss_song: fourth_song,
+        let second_song_other_album_disc_1 = CustomSong {
+            bliss_song: second_song_other_album_disc_1,
             something: true,
         };
-
-        let fifth_song = CustomSong {
-            bliss_song: fifth_song,
+        let first_song_other_album_disc_2 = CustomSong {
+            bliss_song: first_song_other_album_disc_2,
+            something: true,
+        };
+        let second_song_other_album_disc_2 = CustomSong {
+            bliss_song: second_song_other_album_disc_2,
+            something: true,
+        };
+        let song_no_album = CustomSong {
+            bliss_song: song_no_album,
             something: true,
         };
 
         let pool = vec![
             first_song.to_owned(),
-            fourth_song.to_owned(),
-            third_song.to_owned(),
+            second_song_other_album_disc_2.to_owned(),
+            second_song_other_album_disc_1.to_owned(),
             second_song.to_owned(),
-            fifth_song.to_owned(),
+            first_song_other_album_disc_2.to_owned(),
+            first_song_other_album_disc_1.to_owned(),
+            song_no_album.to_owned(),
         ];
-        let group = vec![first_song.to_owned(), third_song.to_owned()];
+        let group = vec![first_song.to_owned(), second_song.to_owned()];
         assert_eq!(
             vec![
                 first_song.to_owned(),
-                third_song.to_owned(),
-                fourth_song.to_owned(),
-                second_song.to_owned()
+                second_song.to_owned(),
+                first_song_other_album_disc_1.to_owned(),
+                second_song_other_album_disc_1.to_owned(),
+                first_song_other_album_disc_2.to_owned(),
+                second_song_other_album_disc_2.to_owned(),
             ],
             closest_album_to_group(group, pool.to_owned()).unwrap(),
         );

src/song/decoder.rs

@@ -512,13 +512,19 @@ pub mod ffmpeg {
             if let Some(track_number) = ictx.metadata().get("track") {
                 song.track_number = match track_number {
                     "" => None,
-                    t => t.parse::<i32>().ok(),
+                    t => t
+                        .parse::<i32>()
+                        .ok()
+                        .or_else(|| t.split_once('/').and_then(|(n, _)| n.parse::<i32>().ok())),
                 };
             };
             if let Some(disc_number) = ictx.metadata().get("disc") {
                 song.disc_number = match disc_number {
                     "" => None,
-                    t => t.parse::<i32>().ok(),
+                    t => t
+                        .parse::<i32>()
+                        .ok()
+                        .or_else(|| t.split_once('/').and_then(|(n, _)| n.parse::<i32>().ok())),
                 };
             };
             if let Some(album_artist) = ictx.metadata().get("album_artist") {
@@ -682,6 +688,19 @@ pub mod ffmpeg {
             assert!((song.duration.as_millis() as f32 - 11070.).abs() < 10.);
         }
 
+        #[test]
+        fn test_special_tags() {
+            let song = Decoder::decode(Path::new("data/special-tags.mp3")).unwrap();
+            assert_eq!(song.disc_number, Some(2));
+            assert_eq!(song.track_number, Some(6));
+        }
+
+        #[test]
+        fn test_unsupported_tags_format() {
+            let song = Decoder::decode(Path::new("data/unsupported-tags.mp3")).unwrap();
+            assert_eq!(song.track_number, None);
+        }
+
         #[test]
         fn test_empty_tags() {
             let song = Decoder::decode(Path::new("data/no_tags.flac")).unwrap();