feat(local): a multi-disc divider that names and illustrates its disc
Opening a box set FOLDER whose discs each have their own name and cover,
the divider said "Disc 2" and nothing else. Switching the header to
METADATA grouping showed them correctly, which is what made it look like
data the app already had.
IT WAS NOT. In folder grouping a box is deliberately ONE album, so
`TrackRow.album` is `album_group_title` — the name with the disc suffix
STRIPPED, which is exactly what makes the box hold together. "Box (Disc
1)" and "Box (Disc 2)" collapse to "Box", and the only thing separating
disc 1 from disc 2 in the published document was an integer. (Metadata
grouping works for an unrelated reason: there each disc is its own album
row, keyed album||artist, so it keeps its raw tag and its own artwork.)
So the document now carries a small `discs` array — one entry per disc,
not per row: the divider is 2-4 items where the track list can run to
hundreds, and widening TrackRow to label three of them would have been
the wrong trade on the surface this view is most careful about.
The title is resolved most-trustworthy-first: the disc FOLDER's own
titled tail ("Disc 1 - Rheingold"), else the raw album tag when it names
something the group title does not already say — compared AFTER stripping
the disc suffix, so a tag of "Box (Disc 2)" is rejected instead of being
echoed next to the "Disc 2" label — else nothing, and the divider stays
the bare "Disc N" it has always been.
THE ART NEEDED A SECOND FIX, and an adversarial pass is what caught it.
The plan was to read each disc's cover off its first track, on the
assumption that scanned artwork is per-folder. It is not:
`find_folder_artwork` gives the album ROOT a +5 bonus against a strict
`>`, so `Box/cover.jpg` beats `Box/Disc 01 - X/cover.jpg` on every track
of every disc, and the thumbnails would have been N copies of one image —
which the "only draw them when they differ" guard would then have
suppressed, leaving the feature silently inert. `folder_artwork_in_dir`
is the unbiased sibling, used only where the caller already knows it is
asking about one disc. Nothing on the scan path changes and no re-scan is
needed.
Additive only in the shared crate: `strip_disc_suffix` and
`album_root_dir` are untouched — they are what keep a box set one album,
there is a regression test pinning that, and a per-disc label must not be
bought by breaking it. qbz-library: 103 tests green.
cc
This commit is contained in:
2 files changed
+148
-3
No files matched your search
@@ -192,6 +192,47 @@ impl MetadataExtractor {
|
||||
/// whitespace on one side, so `CD-Rom Favourites` is not a disc folder
|
||||
/// either — an accidental merge of two albums is worse than the split this
|
||||
/// fixes, so the rule stays narrow.
|
||||
/// `strip_disc_suffix` for callers outside this crate — the disc-suffix
|
||||
/// rule is the one that decides whether a tag NAMES a disc or merely
|
||||
/// NUMBERS it, and a second copy of it elsewhere would drift.
|
||||
pub fn strip_disc_suffix_public(title: &str) -> String {
|
||||
Self::strip_disc_suffix(title)
|
||||
}
|
||||
|
||||
/// The TITLE half of a titled disc folder — "Disc 1 - Rheingold" -> "Rheingold".
|
||||
///
|
||||
/// `is_titled_disc_folder` already computes this split and throws the tail
|
||||
/// away; this returns it. PURELY ADDITIVE: nothing about grouping changes.
|
||||
/// In particular `strip_disc_suffix` and `album_root_dir` are untouched —
|
||||
/// they are what make a box set fold into ONE album, there is a regression
|
||||
/// test pinning that, and a per-disc LABEL must not be bought by breaking
|
||||
/// it.
|
||||
pub fn disc_title_from_name(name: &str) -> Option<String> {
|
||||
let trimmed = name.trim();
|
||||
let bytes = trimmed.as_bytes();
|
||||
|
||||
let mut split = None;
|
||||
for (idx, ch) in trimmed.char_indices() {
|
||||
if !matches!(ch, '-' | '\u{2013}' | '\u{2014}' | ':') {
|
||||
continue;
|
||||
}
|
||||
let space_before = idx > 0 && bytes[idx - 1].is_ascii_whitespace();
|
||||
let after = idx + ch.len_utf8();
|
||||
let space_after = trimmed[after..].starts_with(char::is_whitespace);
|
||||
if space_before || space_after {
|
||||
split = Some((idx, after));
|
||||
break;
|
||||
}
|
||||
}
|
||||
let (head_end, tail_start) = split?;
|
||||
let head = trimmed[..head_end].trim();
|
||||
let tail = trimmed[tail_start..].trim();
|
||||
if tail.is_empty() || !Self::is_bare_disc_folder(head) {
|
||||
return None;
|
||||
}
|
||||
Some(tail.to_string())
|
||||
}
|
||||
|
||||
fn is_titled_disc_folder(name: &str) -> bool {
|
||||
let trimmed = name.trim();
|
||||
let bytes = trimmed.as_bytes();
|
||||
@@ -896,6 +937,56 @@ impl MetadataExtractor {
|
||||
}
|
||||
|
||||
/// Look for folder artwork by file name heuristics
|
||||
/// A cover living in THIS directory, ignoring the album root.
|
||||
///
|
||||
/// `find_folder_artwork` deliberately PREFERS the album root — it gives
|
||||
/// index 0 a `dir_bonus` of 5 and compares with a strict `>`, so
|
||||
/// `Box/cover.jpg` (105) beats `Box/Disc 01 - Name/cover.jpg` (100) on
|
||||
/// every track of every disc. That is right for the album's own cover and
|
||||
/// exactly wrong for a per-DISC one: it is why a box set whose discs each
|
||||
/// ship their own art still resolved to one shared image on all of them.
|
||||
///
|
||||
/// So this is the un-biased sibling, used only where the caller already
|
||||
/// knows it is asking about one disc's folder. It does NOT touch the scan
|
||||
/// path and changes no stored value.
|
||||
pub fn folder_artwork_in_dir(dir: &Path) -> Option<String> {
|
||||
let mut best: Option<(u32, PathBuf)> = None;
|
||||
for entry in std::fs::read_dir(dir).ok()?.flatten() {
|
||||
let path = entry.path();
|
||||
if !path.is_file() {
|
||||
continue;
|
||||
}
|
||||
let ext = path
|
||||
.extension()
|
||||
.and_then(|e| e.to_str())
|
||||
.map(|e| e.to_lowercase())
|
||||
.unwrap_or_default();
|
||||
if !Self::is_supported_artwork_ext(&ext) {
|
||||
continue;
|
||||
}
|
||||
let stem = path
|
||||
.file_stem()
|
||||
.and_then(|s| s.to_str())
|
||||
.map(|s| s.to_lowercase())
|
||||
.unwrap_or_default();
|
||||
// Same vocabulary the scanner scores with, simplified: an exact
|
||||
// well-known name wins, a name that merely contains one is a
|
||||
// fallback, anything else is ignored.
|
||||
const EXACT: &[&str] = &["cover", "folder", "front", "album", "artwork", "art"];
|
||||
let score = if EXACT.contains(&stem.as_str()) {
|
||||
2
|
||||
} else if EXACT.iter().any(|k| stem.contains(k)) {
|
||||
1
|
||||
} else {
|
||||
continue;
|
||||
};
|
||||
if best.as_ref().map(|(s, _)| score > *s).unwrap_or(true) {
|
||||
best = Some((score, path));
|
||||
}
|
||||
}
|
||||
best.map(|(_, p)| p.to_string_lossy().to_string())
|
||||
}
|
||||
|
||||
pub fn find_folder_artwork(
|
||||
audio_file_path: &Path,
|
||||
album_title: Option<&str>,
|
||||
|
||||
@@ -245,6 +245,33 @@ Rectangle {
|
||||
root.artMap = Object.assign({}, m)
|
||||
}
|
||||
}
|
||||
/// The disc rows the document publishes for a MULTI-disc album
|
||||
/// (local_album_actions.rs::disc_rows). Empty on a single-disc album.
|
||||
readonly property var discRows: root.doc.discs || []
|
||||
function discInfo(n) {
|
||||
for (var i = 0; i < root.discRows.length; i++)
|
||||
if ((root.discRows[i].disc || 0) === n)
|
||||
return root.discRows[i]
|
||||
return null
|
||||
}
|
||||
/// TRUE when the discs genuinely have DIFFERENT covers.
|
||||
///
|
||||
/// `DiscRow.cover` is resolved in Rust from each disc's OWN folder, on
|
||||
/// purpose: the scan-time artwork path is biased to the album ROOT, so a
|
||||
/// box always reports one shared image and a thumbnail drawn from it would
|
||||
/// be N copies of the same picture. This still guards the honest case — a
|
||||
/// box that really does ship one cover for every disc — where drawing them
|
||||
/// is noise rather than information.
|
||||
readonly property bool discArtDistinct: {
|
||||
var seen = {}
|
||||
for (var i = 0; i < root.discRows.length; i++) {
|
||||
var p = root.discRows[i].cover || ""
|
||||
if (p === "") return false
|
||||
if (seen[p]) return false
|
||||
seen[p] = true
|
||||
}
|
||||
return root.discRows.length > 1
|
||||
}
|
||||
onAlbumChanged: {
|
||||
if (album && album.artKey) {
|
||||
QbzLocal.artworkWindow(JSON.stringify([album.artKey]))
|
||||
@@ -591,11 +618,38 @@ Rectangle {
|
||||
sourceComponent: Item {
|
||||
width: parent.width
|
||||
height: 40
|
||||
Text {
|
||||
// The disc's OWN cover, when the discs differ.
|
||||
// Silent on a box with one shared cover — see
|
||||
// `discArtDistinct`.
|
||||
RoundedImage {
|
||||
id: discThumb
|
||||
x: 12
|
||||
width: 28
|
||||
height: 28
|
||||
radius: theme.radiusSm
|
||||
anchors.verticalCenter: parent.verticalCenter
|
||||
text: QbzSession.tr("Disc", QbzSession.trRev) + " "
|
||||
+ root.discHeader(trackBlock.index)
|
||||
visible: root.discArtDistinct && source !== ""
|
||||
source: {
|
||||
var d = root.discInfo(root.discHeader(trackBlock.index))
|
||||
var c = d ? (d.cover || "") : ""
|
||||
// A bare absolute path — the local art
|
||||
// channel hands back file:// urls, this one
|
||||
// comes straight off the filesystem.
|
||||
return c === "" ? "" : (c.indexOf("file:") === 0 ? c : "file://" + c)
|
||||
}
|
||||
}
|
||||
Text {
|
||||
x: discThumb.visible ? 12 + 28 + 10 : 12
|
||||
anchors.verticalCenter: parent.verticalCenter
|
||||
// "Disc 2" alone when the box does not name its
|
||||
// discs — the behaviour this row has always had —
|
||||
// and "Disc 2 — Das Rheingold" when it does.
|
||||
text: {
|
||||
var n = root.discHeader(trackBlock.index)
|
||||
var base = QbzSession.tr("Disc", QbzSession.trRev) + " " + n
|
||||
var d = root.discInfo(n)
|
||||
return (d && d.title) ? base + " — " + d.title : base
|
||||
}
|
||||
color: theme.textMuted
|
||||
font.pixelSize: theme.fontLegal
|
||||
font.weight: theme.weightSemibold
|
||||
|
||||
Reference in new issue
Block a user