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
This commit is contained in:
18 files changed
+496
-40
No files matched your search
@@ -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."
|
||||
@@ -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."
|
||||
@@ -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."
|
||||
@@ -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."
|
||||
@@ -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のトラック番号は変更できません。"
|
||||
@@ -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."
|
||||
@@ -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."
|
||||
@@ -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 нельзя изменить."
|
||||
@@ -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,
|
||||
|
||||
@@ -42,6 +42,44 @@ pub struct SacdImportResult {
|
||||
}
|
||||
|
||||
impl LibraryDatabase {
|
||||
pub fn sacd_image_needs_artwork(&self, image_path: &str) -> Result<bool, LibraryError> {
|
||||
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<usize, LibraryError> {
|
||||
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();
|
||||
|
||||
@@ -146,9 +146,7 @@ pub fn build_image_rows(path: &Path, labels: &SacdLabels) -> Result<SacdImageRow
|
||||
None => 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<SacdImageRow
|
||||
|
||||
let facts = image_facts(path);
|
||||
let indexed_at = unix_now_secs();
|
||||
let tracks: Vec<LocalTrack> = area
|
||||
let mut tracks: Vec<LocalTrack> = area
|
||||
.tracks
|
||||
.iter()
|
||||
.enumerate()
|
||||
@@ -233,6 +231,25 @@ pub fn build_image_rows(path: &Path, labels: &SacdLabels) -> Result<SacdImageRow
|
||||
},
|
||||
);
|
||||
|
||||
if let Ok(Some(sidecar)) = crate::read_sacd_sidecar(path) {
|
||||
for track in &mut tracks {
|
||||
// The physical track number is part of the SACD reference, never
|
||||
// mutable metadata. The editor enforces the same restriction.
|
||||
let number = track.track_number;
|
||||
crate::apply_sidecar_to_track(track, &sidecar);
|
||||
track.track_number = number;
|
||||
track.artwork_path = artwork.clone();
|
||||
}
|
||||
}
|
||||
|
||||
let album = tracks
|
||||
.first()
|
||||
.map(|track| track.album.clone())
|
||||
.unwrap_or(album);
|
||||
let artist = tracks
|
||||
.first()
|
||||
.and_then(|track| track.album_artist.clone())
|
||||
.or(artist);
|
||||
let import = SacdImageImport {
|
||||
fingerprint: fingerprint.clone(),
|
||||
image_path: path.to_string_lossy().into_owned(),
|
||||
@@ -252,6 +269,26 @@ pub fn build_image_rows(path: &Path, labels: &SacdLabels) -> Result<SacdImageRow
|
||||
})
|
||||
}
|
||||
|
||||
/// Artwork is external to the audio image. Resolve an explicit image sidecar
|
||||
/// before the normal folder heuristics; a missing cached sidecar image can
|
||||
/// still fall back to a cover placed beside the ISO.
|
||||
fn image_sidecar_or_folder_artwork(path: &Path, album: Option<&str>) -> Option<String> {
|
||||
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();
|
||||
|
||||
@@ -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<Option<AlbumTagSidecar>, 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<Option<AlbumTagSidecar>, LibraryError> {
|
||||
let path = sidecar_path(album_dir);
|
||||
read_sidecar_file(&sidecar_path(album_dir))
|
||||
}
|
||||
|
||||
fn read_sidecar_file(path: &Path) -> Result<Option<AlbumTagSidecar>, 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<String> {
|
||||
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,
|
||||
|
||||
@@ -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<String, LibraryError> {
|
||||
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<String, LibraryError> {
|
||||
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<String, LibraryError> {
|
||||
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<String
|
||||
"The selected cover has unsupported dimensions.".to_string(),
|
||||
));
|
||||
}
|
||||
let target = album_dir.join("cover.jpg");
|
||||
let temporary = album_dir.join(".cover.jpg.qbz-tmp");
|
||||
let backup = album_dir.join(".cover.jpg.qbz-backup");
|
||||
let name = target
|
||||
.file_name()
|
||||
.ok_or_else(|| LibraryError::InvalidPath(target.display().to_string()))?;
|
||||
let sibling = |suffix: &str| {
|
||||
let mut file = std::ffi::OsString::from(".");
|
||||
file.push(name);
|
||||
file.push(suffix);
|
||||
target.with_file_name(file)
|
||||
};
|
||||
let temporary = sibling(".qbz-tmp");
|
||||
let backup = sibling(".qbz-backup");
|
||||
image
|
||||
.save_with_format(&temporary, image::ImageFormat::Jpeg)
|
||||
.map_err(|error| LibraryError::Metadata(format!("Cover encode failed: {error}")))?;
|
||||
@@ -1470,6 +1489,31 @@ mod editor_write_target_tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sacd_front_cover_is_durable_and_does_not_touch_iso_or_folder_art() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let iso = tmp.path().join("Disc.iso");
|
||||
std::fs::write(&iso, b"immutable ISO").unwrap();
|
||||
std::fs::write(tmp.path().join("cover.jpg"), b"shared folder art").unwrap();
|
||||
let mut bytes = Vec::new();
|
||||
image::DynamicImage::new_rgb8(4, 4)
|
||||
.write_to(
|
||||
&mut std::io::Cursor::new(&mut bytes),
|
||||
image::ImageFormat::Png,
|
||||
)
|
||||
.unwrap();
|
||||
let path = write_sacd_front_cover(&iso, &bytes).unwrap();
|
||||
assert_eq!(Path::new(&path), tmp.path().join("Disc.iso.cover.jpg"));
|
||||
assert_eq!(image::open(&path).unwrap().width(), 4);
|
||||
assert_eq!(std::fs::read(&iso).unwrap(), b"immutable ISO");
|
||||
assert_eq!(
|
||||
std::fs::read(tmp.path().join("cover.jpg")).unwrap(),
|
||||
b"shared folder art"
|
||||
);
|
||||
assert!(write_sacd_front_cover(&iso, b"bad image").is_err());
|
||||
assert_eq!(image::open(&path).unwrap().width(), 4);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn folder_cover_rejects_invalid_bytes_without_replacing_existing_art() {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
|
||||
@@ -29,6 +29,7 @@ Item {
|
||||
property bool syncSecondary: false
|
||||
property var tracks: []
|
||||
property var inspection: ({})
|
||||
property bool fixedTrackNumbers: false
|
||||
property bool canDirectWrite: false
|
||||
property string directReason: ""
|
||||
property bool remoteSidecarOnly: false
|
||||
@@ -107,6 +108,7 @@ Item {
|
||||
artworkResults = []
|
||||
tracks = cloneRows(doc.tracks)
|
||||
inspection = doc.inspection || ({})
|
||||
fixedTrackNumbers = doc.fixedTrackNumbers === true
|
||||
canDirectWrite = doc.canDirectWrite === true
|
||||
directReason = doc.directWriteReason || ""
|
||||
remoteSidecarOnly = doc.remoteSidecarOnly === true
|
||||
@@ -202,7 +204,8 @@ Item {
|
||||
if (match) {
|
||||
next[i].title = match.title || next[i].title
|
||||
next[i].discNumber = String(match.disc_number || next[i].discNumber)
|
||||
next[i].trackNumber = String(match.track_number || next[i].trackNumber)
|
||||
if (!fixedTrackNumbers)
|
||||
next[i].trackNumber = String(match.track_number || next[i].trackNumber)
|
||||
next[i].artistCredit = match.artist_credit || next[i].artistCredit
|
||||
if (match.artist_credits && match.artist_credits.length) {
|
||||
next[i].artists = match.artist_credits.map(function(credit) { return credit.name })
|
||||
|
||||
@@ -252,6 +252,7 @@ Item {
|
||||
}
|
||||
EditCell {
|
||||
width: trackPane.trackW
|
||||
enabled: !workspace.editor.fixedTrackNumbers
|
||||
numeric: true
|
||||
text: trackDelegate.modelData.trackNumber
|
||||
onEdited: function(value) { trackDelegate.modelData.trackNumber = value }
|
||||
|
||||
@@ -246,7 +246,7 @@ Item {
|
||||
}
|
||||
QbzCircleAction {
|
||||
id: editFolderAlbumBtn
|
||||
visible: !QbzLocal.localSessionIsDisc
|
||||
visible: !QbzLocal.localEphemeralIsCd
|
||||
&& root.allAlbums.length === 1
|
||||
&& !QbzLocal.localEphemeralLoading
|
||||
name: "pen-line"
|
||||
@@ -535,7 +535,7 @@ Item {
|
||||
spacing: 8
|
||||
QbzCircleAction {
|
||||
id: editBlockBtn
|
||||
visible: !QbzLocal.localSessionIsDisc
|
||||
visible: !QbzLocal.localEphemeralIsCd
|
||||
name: "pen-line"
|
||||
onClicked: QbzLocal.ephemeralEditTags(
|
||||
block.modelData.groupKey)
|
||||
|
||||
@@ -187,14 +187,24 @@ pub(crate) fn album_tracks_for(group_key: &str) -> Vec<LocalTrack> {
|
||||
pub(crate) fn editor_snapshot(group_key: &str) -> Option<(String, String, Vec<LocalTrack>)> {
|
||||
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::<Vec<_>>();
|
||||
if tracks.is_empty() {
|
||||
return None;
|
||||
|
||||
@@ -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<String> {
|
||||
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::<Vec<_>>()
|
||||
};
|
||||
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<SavePayload,
|
||||
}
|
||||
let year = parse_year(&draft.year)?;
|
||||
let direct = draft.persistence == "direct";
|
||||
let sacd = open
|
||||
.tracks
|
||||
.iter()
|
||||
.any(|track| qbz_disc::SacdRef::is_sacd_path(&track.file_path));
|
||||
if direct && sacd {
|
||||
return Err(qbz_i18n::t("SACD images use sidecar metadata."));
|
||||
}
|
||||
if !matches!(draft.persistence.as_str(), "sidecar" | "direct") {
|
||||
return Err(qbz_i18n::t("Unknown metadata persistence mode."));
|
||||
}
|
||||
@@ -1009,6 +1044,9 @@ fn validate_draft(draft: SaveDraft, open: EditorSession) -> Result<SavePayload,
|
||||
return Err(qbz_i18n::t("Every track needs a title."));
|
||||
}
|
||||
let track_number = parse_optional_number(&row.track_number, &qbz_i18n::t("Track number"))?;
|
||||
if sacd && track_number != track.track_number {
|
||||
return Err(qbz_i18n::t("SACD track numbers cannot be changed."));
|
||||
}
|
||||
let disc_number = parse_optional_number(&row.disc_number, &qbz_i18n::t("Disc number"))?;
|
||||
let artist_credit = if row.artist_credit.trim().is_empty() {
|
||||
track.artist.clone()
|
||||
@@ -1269,11 +1307,27 @@ pub fn save(draft_json: &str) {
|
||||
// A sidecar is removed only after every file was verified.
|
||||
qbz_library::delete_album_sidecar(Path::new(&payload.session.directory))?;
|
||||
} else {
|
||||
let sacd_image = payload
|
||||
.session
|
||||
.tracks
|
||||
.first()
|
||||
.and_then(|track| qbz_disc::SacdRef::parse(&track.file_path))
|
||||
.map(|reference| reference.image);
|
||||
let mut persistent_cover = sacd_image
|
||||
.as_ref()
|
||||
.and_then(|image| qbz_library::read_sacd_sidecar(image).ok().flatten())
|
||||
.and_then(|sidecar| sidecar.extended_album)
|
||||
.and_then(|album| album.artwork_path);
|
||||
if let Some(cover) = payload.front_cover.as_ref() {
|
||||
qbz_library::write_folder_front_cover(
|
||||
Path::new(&payload.session.directory),
|
||||
&cover.bytes,
|
||||
)?;
|
||||
if let Some(image) = sacd_image.as_ref() {
|
||||
persistent_cover =
|
||||
Some(qbz_library::write_sacd_front_cover(image, &cover.bytes)?);
|
||||
} else {
|
||||
qbz_library::write_folder_front_cover(
|
||||
Path::new(&payload.session.directory),
|
||||
&cover.bytes,
|
||||
)?;
|
||||
}
|
||||
artwork_path =
|
||||
qbz_library::MetadataExtractor::cache_artwork_bytes(&cover.bytes);
|
||||
if artwork_path.is_none() {
|
||||
@@ -1283,13 +1337,28 @@ pub fn save(draft_json: &str) {
|
||||
));
|
||||
}
|
||||
if let Some(extended) = payload.sidecar.extended_album.as_mut() {
|
||||
extended.artwork_path = artwork_path.clone();
|
||||
extended.artwork_path =
|
||||
persistent_cover.clone().or_else(|| artwork_path.clone());
|
||||
}
|
||||
}
|
||||
qbz_library::write_album_sidecar(
|
||||
Path::new(&payload.session.directory),
|
||||
&payload.sidecar,
|
||||
)?;
|
||||
if sacd_image.is_some() {
|
||||
if let Some(extended) = payload.sidecar.extended_album.as_mut() {
|
||||
extended.artwork_path = persistent_cover;
|
||||
}
|
||||
}
|
||||
if let Some(reference) = payload
|
||||
.session
|
||||
.tracks
|
||||
.first()
|
||||
.and_then(|track| qbz_disc::SacdRef::parse(&track.file_path))
|
||||
{
|
||||
qbz_library::write_sacd_sidecar(&reference.image, &payload.sidecar)?;
|
||||
} else {
|
||||
qbz_library::write_album_sidecar(
|
||||
Path::new(&payload.session.directory),
|
||||
&payload.sidecar,
|
||||
)?;
|
||||
}
|
||||
}
|
||||
let outcome = match &payload.session.target {
|
||||
EditorTarget::Library => 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();
|
||||
|
||||
Reference in new issue
Block a user