From 0ebcab6d3e36118bc9c843b17f9552b82662512e Mon Sep 17 00:00:00 2001 From: vicrodh <103399283+vicrodh@users.noreply.github.com> Date: Tue, 8 Sep 2026 15:26:03 -0600 Subject: [PATCH] fix(library): support SACD artwork fallback and image sidecars Refresh missing artwork for unchanged SACD images when a folder cover is added after scanning. Enable image-specific metadata and cover sidecars in the Qt editor while preserving physical track numbers and never writing the ISO. Validated with SACD regressions, the real Heathen image, 13 Qt editor tests, QML audits and the combined release build. cx --- .../translations/de/LC_MESSAGES/qbz-ui.po | 6 + .../translations/en/LC_MESSAGES/qbz-ui.po | 6 + .../translations/es/LC_MESSAGES/qbz-ui.po | 6 + .../translations/fr/LC_MESSAGES/qbz-ui.po | 6 + .../translations/ja/LC_MESSAGES/qbz-ui.po | 6 + .../translations/nl/LC_MESSAGES/qbz-ui.po | 6 + .../translations/pt/LC_MESSAGES/qbz-ui.po | 6 + .../translations/ru/LC_MESSAGES/qbz-ui.po | 6 + crates/qbz-library/src/lib.rs | 6 +- crates/qbz-library/src/sacd.rs | 82 +++++++++++ crates/qbz-library/src/sacd_scan.rs | 129 ++++++++++++++++- crates/qbz-library/src/tag_sidecar.rs | 60 +++++++- crates/qbz-library/src/tag_writer.rs | 50 ++++++- crates/qbz-qt/qml/controls/TagEditorModal.qml | 5 +- .../qml/controls/TagEditorWorkspace.qml | 1 + .../qml/views/local/LocalEphemeralPane.qml | 4 +- crates/qbz-qt/src/local_ephemeral.rs | 14 +- crates/qbz-qt/src/tag_editor_qt.rs | 137 +++++++++++++++--- 18 files changed, 496 insertions(+), 40 deletions(-) diff --git a/crates/qbz-i18n/translations/de/LC_MESSAGES/qbz-ui.po b/crates/qbz-i18n/translations/de/LC_MESSAGES/qbz-ui.po index c1e048edb..fb54afd3d 100644 --- a/crates/qbz-i18n/translations/de/LC_MESSAGES/qbz-ui.po +++ b/crates/qbz-i18n/translations/de/LC_MESSAGES/qbz-ui.po @@ -6843,3 +6843,9 @@ msgstr "Baum" msgid "Forward" msgstr "Vorwärts" + +msgid "SACD images use sidecar metadata." +msgstr "SACD-Abbilder verwenden Metadaten in einer Sidecar-Datei." + +msgid "SACD track numbers cannot be changed." +msgstr "SACD-Titelnummern können nicht geändert werden." diff --git a/crates/qbz-i18n/translations/en/LC_MESSAGES/qbz-ui.po b/crates/qbz-i18n/translations/en/LC_MESSAGES/qbz-ui.po index e7d10f264..9ad1e95e7 100644 --- a/crates/qbz-i18n/translations/en/LC_MESSAGES/qbz-ui.po +++ b/crates/qbz-i18n/translations/en/LC_MESSAGES/qbz-ui.po @@ -2427,3 +2427,9 @@ msgstr "List" msgid "Tree" msgstr "Tree" + +msgid "SACD images use sidecar metadata." +msgstr "SACD images use sidecar metadata." + +msgid "SACD track numbers cannot be changed." +msgstr "SACD track numbers cannot be changed." diff --git a/crates/qbz-i18n/translations/es/LC_MESSAGES/qbz-ui.po b/crates/qbz-i18n/translations/es/LC_MESSAGES/qbz-ui.po index 6c23f5acb..690d6adcd 100644 --- a/crates/qbz-i18n/translations/es/LC_MESSAGES/qbz-ui.po +++ b/crates/qbz-i18n/translations/es/LC_MESSAGES/qbz-ui.po @@ -6849,3 +6849,9 @@ msgstr "Árbol" msgid "Forward" msgstr "Adelante" + +msgid "SACD images use sidecar metadata." +msgstr "Las imágenes SACD usan metadatos en un archivo sidecar." + +msgid "SACD track numbers cannot be changed." +msgstr "No se pueden cambiar los números de pista de un SACD." diff --git a/crates/qbz-i18n/translations/fr/LC_MESSAGES/qbz-ui.po b/crates/qbz-i18n/translations/fr/LC_MESSAGES/qbz-ui.po index 27a377fe0..a213bb9e4 100644 --- a/crates/qbz-i18n/translations/fr/LC_MESSAGES/qbz-ui.po +++ b/crates/qbz-i18n/translations/fr/LC_MESSAGES/qbz-ui.po @@ -6841,3 +6841,9 @@ msgstr "Arborescence" msgid "Forward" msgstr "Suivant" + +msgid "SACD images use sidecar metadata." +msgstr "Les images SACD utilisent des métadonnées dans un fichier annexe." + +msgid "SACD track numbers cannot be changed." +msgstr "Les numéros de piste SACD ne peuvent pas être modifiés." diff --git a/crates/qbz-i18n/translations/ja/LC_MESSAGES/qbz-ui.po b/crates/qbz-i18n/translations/ja/LC_MESSAGES/qbz-ui.po index 870a178d2..268dd192d 100644 --- a/crates/qbz-i18n/translations/ja/LC_MESSAGES/qbz-ui.po +++ b/crates/qbz-i18n/translations/ja/LC_MESSAGES/qbz-ui.po @@ -6824,3 +6824,9 @@ msgstr "ツリー" msgid "Forward" msgstr "進む" + +msgid "SACD images use sidecar metadata." +msgstr "SACDイメージはサイドカーファイルにメタデータを保存します。" + +msgid "SACD track numbers cannot be changed." +msgstr "SACDのトラック番号は変更できません。" diff --git a/crates/qbz-i18n/translations/nl/LC_MESSAGES/qbz-ui.po b/crates/qbz-i18n/translations/nl/LC_MESSAGES/qbz-ui.po index 72114d52e..2bd55b789 100644 --- a/crates/qbz-i18n/translations/nl/LC_MESSAGES/qbz-ui.po +++ b/crates/qbz-i18n/translations/nl/LC_MESSAGES/qbz-ui.po @@ -6831,3 +6831,9 @@ msgstr "Boomstructuur" msgid "Forward" msgstr "Vooruit" + +msgid "SACD images use sidecar metadata." +msgstr "SACD-images gebruiken metadata in een sidecarbestand." + +msgid "SACD track numbers cannot be changed." +msgstr "SACD-tracknummers kunnen niet worden gewijzigd." diff --git a/crates/qbz-i18n/translations/pt/LC_MESSAGES/qbz-ui.po b/crates/qbz-i18n/translations/pt/LC_MESSAGES/qbz-ui.po index 4155900c4..20b9de41c 100644 --- a/crates/qbz-i18n/translations/pt/LC_MESSAGES/qbz-ui.po +++ b/crates/qbz-i18n/translations/pt/LC_MESSAGES/qbz-ui.po @@ -6841,3 +6841,9 @@ msgstr "Árvore" msgid "Forward" msgstr "Avançar" + +msgid "SACD images use sidecar metadata." +msgstr "As imagens SACD usam metadados em um arquivo sidecar." + +msgid "SACD track numbers cannot be changed." +msgstr "Os números das faixas SACD não podem ser alterados." diff --git a/crates/qbz-i18n/translations/ru/LC_MESSAGES/qbz-ui.po b/crates/qbz-i18n/translations/ru/LC_MESSAGES/qbz-ui.po index 5a14112cf..d139aaa3e 100644 --- a/crates/qbz-i18n/translations/ru/LC_MESSAGES/qbz-ui.po +++ b/crates/qbz-i18n/translations/ru/LC_MESSAGES/qbz-ui.po @@ -6862,3 +6862,9 @@ msgstr "Дерево" msgid "Forward" msgstr "Вперёд" + +msgid "SACD images use sidecar metadata." +msgstr "Образы SACD используют метаданные в отдельном файле." + +msgid "SACD track numbers cannot be changed." +msgstr "Номера дорожек SACD нельзя изменить." diff --git a/crates/qbz-library/src/lib.rs b/crates/qbz-library/src/lib.rs index 3351f1a29..07de6c0b1 100644 --- a/crates/qbz-library/src/lib.rs +++ b/crates/qbz-library/src/lib.rs @@ -82,9 +82,9 @@ pub use tag_writer::{ compute_track_artist_match, inspect_album_tag_layers, read_editor_tag_snapshots, write_album_tags_to_files, write_album_tags_to_files_extended, write_album_tags_to_files_with_options, write_folder_front_cover, write_purchase_tags, - AlbumTagInspection, AlbumTagWrite, DirectTagWriteOptions, EditorTrackTagSnapshot, - ExtendedAlbumTagWrite, ExtendedTrackTagWrite, FrontCoverWrite, Id3v2WriteVersion, - PurchaseTagWrite, TagLayerInspection, TrackTagWrite, + write_sacd_front_cover, AlbumTagInspection, AlbumTagWrite, DirectTagWriteOptions, + EditorTrackTagSnapshot, ExtendedAlbumTagWrite, ExtendedTrackTagWrite, FrontCoverWrite, + Id3v2WriteVersion, PurchaseTagWrite, TagLayerInspection, TrackTagWrite, }; pub use thumbnails::{ clear_thumbnails, generate_thumbnail, generate_thumbnail_from_bytes, get_cache_size, diff --git a/crates/qbz-library/src/sacd.rs b/crates/qbz-library/src/sacd.rs index d1db6d426..ede7588ed 100644 --- a/crates/qbz-library/src/sacd.rs +++ b/crates/qbz-library/src/sacd.rs @@ -42,6 +42,44 @@ pub struct SacdImportResult { } impl LibraryDatabase { + pub fn sacd_image_needs_artwork(&self, image_path: &str) -> Result { + self.with_connection(|connection| { + connection + .query_row( + "SELECT EXISTS(SELECT 1 FROM local_tracks l + JOIN local_sacd_tracks t ON t.local_track_id=l.id + JOIN local_sacd_images i ON i.fingerprint=t.fingerprint + WHERE i.image_path=?1 AND COALESCE(l.artwork_path,'')='')", + params![image_path], + |row| row.get(0), + ) + .map_err(database_error) + }) + } + + /// Fill only missing image artwork. This never reads or modifies the ISO, + /// and existing disc artwork always wins over a newly discovered folder image. + pub fn fill_missing_sacd_artwork( + &self, + image_path: &str, + artwork: &str, + ) -> Result { + self.with_connection(|connection| { + connection + .execute( + "UPDATE local_tracks SET artwork_path=?2, + indexed_at=MAX(COALESCE((SELECT MAX(indexed_at) FROM local_tracks),0)+1, + CAST(strftime('%s','now') AS INTEGER)) + WHERE COALESCE(artwork_path,'')='' AND id IN ( + SELECT t.local_track_id FROM local_sacd_tracks t + JOIN local_sacd_images i ON i.fingerprint=t.fingerprint + WHERE i.image_path=?1)", + params![image_path, artwork], + ) + .map_err(database_error) + }) + } + /// Mark a known image as seen by one registered library root, and report /// whether its file facts and parser revision still match. Unknown paths /// remain unowned until a complete parse is imported. @@ -1121,6 +1159,50 @@ mod tests { assert_eq!(old_images, 0); } + #[test] + fn artwork_backfill_keeps_existing_covers_and_other_images_unchanged() { + let (_temp, db, root_id) = fresh_db(); + let mut first = image("first", "/music/first.iso", 2); + first.tracks[0].artwork_path = Some("original-disc-cover".into()); + let ids = db.import_sacd_image(root_id, 1, &first).unwrap().track_ids; + let other = db + .import_sacd_image(root_id, 1, &image("other", "/music/other.iso", 1)) + .unwrap() + .track_ids; + assert_eq!( + db.fill_missing_sacd_artwork("/music/first.iso", "folder-cover") + .unwrap(), + 1 + ); + assert_eq!( + db.get_track(ids[0]) + .unwrap() + .unwrap() + .artwork_path + .as_deref(), + Some("original-disc-cover") + ); + assert_eq!( + db.get_track(ids[1]) + .unwrap() + .unwrap() + .artwork_path + .as_deref(), + Some("folder-cover") + ); + assert!(db + .get_track(other[0]) + .unwrap() + .unwrap() + .artwork_path + .is_none()); + assert_eq!( + db.fill_missing_sacd_artwork("/music/first.iso", "replacement") + .unwrap(), + 0 + ); + } + #[test] fn clear_library_removes_tracks_and_sacd_identity_together() { let (_temp, db, root_id) = fresh_db(); diff --git a/crates/qbz-library/src/sacd_scan.rs b/crates/qbz-library/src/sacd_scan.rs index 64e0be45c..309771e9f 100644 --- a/crates/qbz-library/src/sacd_scan.rs +++ b/crates/qbz-library/src/sacd_scan.rs @@ -146,9 +146,7 @@ pub fn build_image_rows(path: &Path, labels: &SacdLabels) -> Result area.artist.clone().filter(|artist| !artist.is_empty()), }; - let artwork = MetadataExtractor::find_folder_artwork(path, Some(&album)).and_then(|found| { - MetadataExtractor::cache_artwork_file(Path::new(&found), &crate::get_artwork_cache_dir()) - }); + let artwork = image_sidecar_or_folder_artwork(path, Some(&album)); match artwork.as_deref() { Some(_) => log::info!("[sacd] cover cached"), None => log::info!("[sacd] no cover beside the image"), @@ -156,7 +154,7 @@ pub fn build_image_rows(path: &Path, labels: &SacdLabels) -> Result = area + let mut tracks: Vec = area .tracks .iter() .enumerate() @@ -233,6 +231,25 @@ pub fn build_image_rows(path: &Path, labels: &SacdLabels) -> Result Result) -> Option { + crate::read_sacd_sidecar(path) + .ok() + .flatten() + .and_then(|sidecar| sidecar.extended_album) + .and_then(|album| album.artwork_path) + .filter(|art| Path::new(art).is_file()) + .or_else(|| { + MetadataExtractor::find_folder_artwork(path, album).and_then(|found| { + MetadataExtractor::cache_artwork_file( + Path::new(&found), + &crate::get_artwork_cache_dir(), + ) + }) + }) +} + /// What one root's SACD pass did. #[derive(Debug, Clone, Default, PartialEq, Eq)] pub struct SacdScanSummary { @@ -314,7 +351,7 @@ pub fn scan_root_for_sacd( let path = entry.path(); let path_string = path.to_string_lossy().into_owned(); - // Known and unchanged: no file I/O at all. + // Unchanged audio avoids parsing; auxiliary artwork can still change. let facts = image_facts(path); match db.observe_sacd_image( root_id, @@ -325,6 +362,22 @@ pub fn scan_root_for_sacd( SACD_PARSER_REVISION, ) { Ok(true) => { + // Covers can arrive after the initial scan without changing + // the ISO's size or mtime. Refresh that separate input while + // keeping the fast path free of SACD parsing/audio reads. + if db.sacd_image_needs_artwork(&path_string).unwrap_or(false) { + if let Some(artwork) = image_sidecar_or_folder_artwork(path, None) { + match db.fill_missing_sacd_artwork(&path_string, &artwork) { + Ok(0) => {} + Ok(count) => { + log::info!("[sacd] filled missing artwork for {count} tracks") + } + Err(error) => summary + .failed + .push((path_string.clone(), error.to_string())), + } + } + } summary.unchanged = summary.unchanged.saturating_add(1); continue; } @@ -544,6 +597,47 @@ mod tests { assert_eq!(second.imported, 0); assert_eq!(db.get_all_track_paths().unwrap(), tracks); + // A cover added after the audio scan must be discovered without + // reimporting or changing the virtual track ids / ISO bytes. + let iso_before = std::fs::read(&image_path).unwrap(); + image::DynamicImage::new_rgb8(4, 4) + .save(root.join("cover.jpg")) + .unwrap(); + let artwork_refresh = scan(3); + assert_eq!(artwork_refresh.unchanged, 1); + assert_eq!(artwork_refresh.imported, 0); + assert!(artwork_refresh.failed.is_empty()); + assert!(!db + .sacd_image_needs_artwork(&image_path.to_string_lossy()) + .unwrap()); + assert_eq!(std::fs::read(&image_path).unwrap(), iso_before); + assert_eq!(db.get_all_track_paths().unwrap(), tracks); + + let mut sidecar = crate::AlbumTagSidecar::new( + crate::AlbumMetadataOverride { + album_title: Some("Sidecar title".into()), + ..Default::default() + }, + vec![crate::TrackMetadataOverride { + file_path: format!("sacd:{}#1", image_path.display()), + cue_start_secs: None, + title: Some("Sidecar track".into()), + disc_number: Some(1), + track_number: Some(99), + }], + ); + sidecar.extended_album = Some(crate::AlbumExtendedMetadataOverride { + artwork_path: Some(root.join("cover.jpg").to_string_lossy().into_owned()), + ..Default::default() + }); + crate::write_sacd_sidecar(&image_path, &sidecar).unwrap(); + let edited = super::build_image_rows(&image_path, &SacdLabels::default()).unwrap(); + assert_eq!(edited.album, "Sidecar title"); + assert_eq!(edited.tracks[0].title, "Sidecar track"); + assert_eq!(edited.tracks[0].track_number, Some(1)); + assert!(edited.tracks[0].artwork_path.is_some()); + assert_eq!(std::fs::read(&image_path).unwrap(), iso_before); + // Even a missing physical trailer is a failed SACD read, not a file // that may be silently ignored and have its known tracks pruned. std::fs::OpenOptions::new() @@ -552,13 +646,36 @@ mod tests { .unwrap() .set_len(640 * 2064 - 4) .unwrap(); - let third = scan(3); + let third = scan(4); assert_eq!(third.failed.len(), 1); assert_eq!(third.ignored, 0); assert_eq!(third.removed, 0); assert_eq!(db.get_all_track_paths().unwrap(), tracks); } + #[test] + #[ignore = "requires QBZ_TEST_SACD_IMAGE pointing to a real image with external artwork"] + fn real_sacd_image_artwork_probe() { + let path = + std::path::PathBuf::from(std::env::var_os("QBZ_TEST_SACD_IMAGE").expect("image path")); + let before = std::fs::metadata(&path).unwrap(); + let rows = super::build_image_rows(&path, &SacdLabels::default()).unwrap(); + assert!(!rows.tracks.is_empty()); + let cover = rows.tracks[0] + .artwork_path + .as_ref() + .expect("external cover resolved"); + assert!(std::path::Path::new(cover).is_file()); + let after = std::fs::metadata(&path).unwrap(); + assert_eq!(before.len(), after.len()); + assert_eq!(before.modified().unwrap(), after.modified().unwrap()); + println!( + "SACD probe: album={} tracks={} cover={cover}", + rows.album, + rows.tracks.len() + ); + } + #[test] fn cancel_stops_before_the_first_candidate() { let temp = TempDir::new().unwrap(); diff --git a/crates/qbz-library/src/tag_sidecar.rs b/crates/qbz-library/src/tag_sidecar.rs index 39f1844dc..77e70f6bd 100644 --- a/crates/qbz-library/src/tag_sidecar.rs +++ b/crates/qbz-library/src/tag_sidecar.rs @@ -114,8 +114,26 @@ pub fn sidecar_path(album_dir: &Path) -> PathBuf { album_dir.join(SIDECAR_FILE_NAME) } +/// One image owns its sidecar, even when several ISOs share a directory. +pub fn sacd_sidecar_path(image: &Path) -> PathBuf { + let mut name = image.as_os_str().to_os_string(); + name.push(".qbz.json"); + PathBuf::from(name) +} + +pub fn read_sacd_sidecar(image: &Path) -> Result, LibraryError> { + read_sidecar_file(&sacd_sidecar_path(image)) +} + +pub fn write_sacd_sidecar(image: &Path, sidecar: &AlbumTagSidecar) -> Result<(), LibraryError> { + write_sidecar_file(&sacd_sidecar_path(image), sidecar) +} + pub fn read_album_sidecar(album_dir: &Path) -> Result, LibraryError> { - let path = sidecar_path(album_dir); + read_sidecar_file(&sidecar_path(album_dir)) +} + +fn read_sidecar_file(path: &Path) -> Result, LibraryError> { if !path.exists() { return Ok(None); } @@ -130,15 +148,20 @@ pub fn write_album_sidecar( album_dir: &Path, sidecar: &AlbumTagSidecar, ) -> Result<(), LibraryError> { - fs::create_dir_all(album_dir).map_err(LibraryError::Io)?; + write_sidecar_file(&sidecar_path(album_dir), sidecar) +} - let target = sidecar_path(album_dir); - let tmp = album_dir.join(format!("{}.tmp", SIDECAR_FILE_NAME)); +fn write_sidecar_file(target: &Path, sidecar: &AlbumTagSidecar) -> Result<(), LibraryError> { + if let Some(parent) = target.parent() { + fs::create_dir_all(parent).map_err(LibraryError::Io)?; + } + let mut temporary = target.as_os_str().to_os_string(); + temporary.push(".tmp"); + let tmp = PathBuf::from(temporary); let content = serde_json::to_vec_pretty(sidecar).map_err(|e| LibraryError::Metadata(e.to_string()))?; - fs::write(&tmp, content).map_err(LibraryError::Io)?; - fs::rename(&tmp, &target).map_err(LibraryError::Io)?; + fs::rename(&tmp, target).map_err(LibraryError::Io)?; Ok(()) } @@ -235,6 +258,31 @@ fn normalize(value: &str) -> Option { mod tests { use super::*; + #[test] + fn sacd_sidecars_are_per_image_and_leave_audio_untouched() { + let temp = tempfile::tempdir().unwrap(); + let first = temp.path().join("Disc 1.iso"); + let second = temp.path().join("Disc 2.iso"); + fs::write(&first, b"first image").unwrap(); + fs::write(&second, b"second image").unwrap(); + let mut sidecar = AlbumTagSidecar::new(AlbumMetadataOverride::default(), Vec::new()); + sidecar.album.album_title = Some("Edited title".into()); + write_sacd_sidecar(&first, &sidecar).unwrap(); + assert_eq!( + read_sacd_sidecar(&first) + .unwrap() + .unwrap() + .album + .album_title + .as_deref(), + Some("Edited title") + ); + assert!(read_sacd_sidecar(&second).unwrap().is_none()); + assert!(read_album_sidecar(temp.path()).unwrap().is_none()); + assert_eq!(fs::read(first).unwrap(), b"first image"); + assert_eq!(fs::read(second).unwrap(), b"second image"); + } + fn track() -> LocalTrack { LocalTrack { id: 42, diff --git a/crates/qbz-library/src/tag_writer.rs b/crates/qbz-library/src/tag_writer.rs index b6016beea..ad68d4a2f 100644 --- a/crates/qbz-library/src/tag_writer.rs +++ b/crates/qbz-library/src/tag_writer.rs @@ -271,6 +271,17 @@ pub struct FrontCoverWrite { /// Decode and atomically install a conventional folder front cover. /// The old file is restored if the final rename fails. pub fn write_folder_front_cover(album_dir: &Path, bytes: &[u8]) -> Result { + write_front_cover_at(&album_dir.join("cover.jpg"), bytes) +} + +/// Persist artwork next to one SACD image without overwriting a shared folder cover. +pub fn write_sacd_front_cover(image: &Path, bytes: &[u8]) -> Result { + let mut path = image.as_os_str().to_os_string(); + path.push(".cover.jpg"); + write_front_cover_at(&std::path::PathBuf::from(path), bytes) +} + +fn write_front_cover_at(target: &Path, bytes: &[u8]) -> Result { if bytes.is_empty() || bytes.len() > 25 * 1024 * 1024 { return Err(LibraryError::Metadata( "The selected cover is empty or larger than 25 MiB.".to_string(), @@ -288,9 +299,17 @@ pub fn write_folder_front_cover(album_dir: &Path, bytes: &[u8]) -> Result Vec { pub(crate) fn editor_snapshot(group_key: &str) -> Option<(String, String, Vec)> { let selected = album_tracks(group_key); let first = selected.first()?; - let directory = Path::new(&first.file_path).parent()?.to_path_buf(); + let sacd = qbz_disc::SacdRef::parse(&first.file_path); + let physical = sacd + .as_ref() + .map(|reference| reference.image.as_path()) + .unwrap_or_else(|| Path::new(&first.file_path)); + let directory = physical.parent()?.to_path_buf(); if !directory.is_dir() { return None; } let tracks = STATE .tracks_snapshot() .into_iter() - .filter(|track| Path::new(&track.file_path).parent() == Some(directory.as_path())) + .filter(|track| match sacd.as_ref() { + Some(reference) => qbz_disc::SacdRef::parse(&track.file_path) + .map(|other| other.image == reference.image) + .unwrap_or(false), + None => Path::new(&track.file_path).parent() == Some(directory.as_path()), + }) .collect::>(); if tracks.is_empty() { return None; diff --git a/crates/qbz-qt/src/tag_editor_qt.rs b/crates/qbz-qt/src/tag_editor_qt.rs index 4e1611b14..3e10027f7 100644 --- a/crates/qbz-qt/src/tag_editor_qt.rs +++ b/crates/qbz-qt/src/tag_editor_qt.rs @@ -102,6 +102,13 @@ fn editor_remote_source(track: &LocalTrack) -> String { /// was unnecessarily strict (a renamed/missing row can still be corrected in /// a valid album sidecar) and broke mounted network libraries in particular. fn editor_directory(track: &LocalTrack) -> Option { + if let Some(reference) = qbz_disc::SacdRef::parse(&track.file_path) { + return reference + .image + .parent() + .filter(|path| path.is_dir()) + .map(|path| path.to_string_lossy().into_owned()); + } let group = Path::new(&track.album_group_key); if group.is_dir() { return Some(group.to_string_lossy().into_owned()); @@ -228,6 +235,7 @@ struct EditorSeed { artwork: ArtworkSeed, sidecar_exists: bool, remote_sidecar_only: bool, + fixed_track_numbers: bool, can_direct_write: bool, direct_write_reason: String, inspection: InspectionDoc, @@ -236,6 +244,10 @@ struct EditorSeed { fn build_seed(open: &EditorSession) -> EditorSeed { let tracks = &open.tracks; + let sacd_image = tracks + .first() + .and_then(|track| qbz_disc::SacdRef::parse(&track.file_path)) + .map(|reference| reference.image); let remote_target = match &open.target { EditorTarget::Remote { target } => Some(target), _ => None, @@ -243,7 +255,7 @@ fn build_seed(open: &EditorSession) -> EditorSeed { let cue_based = tracks .iter() .any(|track| track.cue_file_path.is_some() || track.cue_start_secs.is_some()); - let paths = if remote_target.is_some() { + let paths = if remote_target.is_some() || sacd_image.is_some() { Vec::new() } else { tracks @@ -251,7 +263,7 @@ fn build_seed(open: &EditorSession) -> EditorSeed { .map(|track| track.file_path.clone()) .collect::>() }; - let inspection = if remote_target.is_some() { + let inspection = if remote_target.is_some() || sacd_image.is_some() { InspectionDoc { file_count: 0, canonical_layers: Vec::new(), @@ -264,7 +276,7 @@ fn build_seed(open: &EditorSession) -> EditorSeed { } else { InspectionDoc::from_result(qbz_library::inspect_album_tag_layers(&paths)) }; - let snapshots = if remote_target.is_some() { + let snapshots = if remote_target.is_some() || sacd_image.is_some() { Vec::new() } else { qbz_library::read_editor_tag_snapshots(&paths) @@ -277,7 +289,10 @@ fn build_seed(open: &EditorSession) -> EditorSeed { .and_then(crate::remote_metadata_qt::sidecar) .or_else(|| { (remote_target.is_none()) - .then(|| qbz_library::read_album_sidecar(Path::new(&open.directory))) + .then(|| match sacd_image.as_ref() { + Some(image) => qbz_library::read_sacd_sidecar(image), + None => qbz_library::read_album_sidecar(Path::new(&open.directory)), + }) .and_then(Result::ok) .flatten() }); @@ -288,10 +303,15 @@ fn build_seed(open: &EditorSession) -> EditorSeed { && tracks .iter() .all(|track| Path::new(&track.file_path).is_file()); - let can_direct_write = - remote_target.is_none() && !cue_based && local_files && inspection.direct_write_supported; + let can_direct_write = remote_target.is_none() + && sacd_image.is_none() + && !cue_based + && local_files + && inspection.direct_write_supported; let direct_write_reason = if remote_target.is_some() { qbz_i18n::t("Media-server metadata is stored in a local sidecar.") + } else if sacd_image.is_some() { + qbz_i18n::t("SACD images use sidecar metadata.") } else if cue_based { qbz_i18n::t("CUE-based albums use sidecar metadata.") } else if !local_files { @@ -443,9 +463,14 @@ fn build_seed(open: &EditorSession) -> EditorSeed { sidecar_exists: if remote_target.is_some() { sidecar.is_some() } else { - qbz_library::sidecar_path(Path::new(&open.directory)).exists() + sacd_image + .as_ref() + .map(|image| qbz_library::sacd_sidecar_path(image)) + .unwrap_or_else(|| qbz_library::sidecar_path(Path::new(&open.directory))) + .exists() }, remote_sidecar_only: remote_target.is_some(), + fixed_track_numbers: sacd_image.is_some(), can_direct_write, direct_write_reason, inspection, @@ -628,7 +653,10 @@ pub fn open(album_id: String) { open_session( album_id, group_key.clone(), - group_key, + tracks + .first() + .and_then(editor_directory) + .unwrap_or(group_key), tracks, EditorTarget::Library, None, @@ -926,6 +954,13 @@ fn validate_draft(draft: SaveDraft, open: EditorSession) -> Result Result SaveOutcome::Library { @@ -1959,6 +2028,40 @@ mod tests { assert_eq!(editor_remote_source(&track), "plex"); } + #[test] + fn sacd_editor_resolves_image_directory_and_refuses_direct_writes() { + let mut open = session_fixture(); + let directory = std::env::temp_dir(); + let image = directory.join("Disc #1.iso"); + for (index, track) in open.tracks.iter_mut().enumerate() { + track.file_path = qbz_disc::SacdRef { + image: image.clone(), + track: (index + 1) as u8, + } + .to_path_string(); + track.track_number = Some(index as u32 + 1); + track.album_group_key = "sacd|||fingerprint".into(); + } + open.directory = directory.to_string_lossy().into_owned(); + assert_eq!( + editor_directory(&open.tracks[0]), + Some(open.directory.clone()) + ); + let seed = build_seed(&open); + assert!(!seed.can_direct_write); + assert!(seed.fixed_track_numbers); + assert!(seed.inspection.error.is_empty()); + let rows = r#"[{"id":"10","title":"One","trackNumber":"1","discNumber":"1"},{"id":"20","title":"Two","trackNumber":"2","discNumber":"1"}]"#; + let draft: SaveDraft = serde_json::from_str(&draft_json(rows)).unwrap(); + assert!(validate_draft(draft, open.clone()).is_ok()); + let mut direct: SaveDraft = serde_json::from_str(&draft_json(rows)).unwrap(); + direct.persistence = "direct".into(); + assert!(validate_draft(direct, open.clone()).is_err()); + let mut renumbered: SaveDraft = serde_json::from_str(&draft_json(rows)).unwrap(); + renumbered.tracks[0].track_number = "2".into(); + assert!(validate_draft(renumbered, open).is_err()); + } + #[test] fn editor_directory_uses_real_parent_for_non_path_group_identity() { let directory = std::env::temp_dir();