From c4ff6269a36df39511656728b447a37c18fe50aa Mon Sep 17 00:00:00 2001 From: KeelrLeek <156356974+KeelrLeek@users.noreply.github.com> Date: Mon, 10 Aug 2026 18:08:58 -0500 Subject: [PATCH] import: records named by their own tags, and covers resampled where the pixels are MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two threads from one session, both of them the same shape: something was being decided from the wrong source. THE PROVIDERS WERE ASKED ABOUT A FOLDER. The picker sent the directory name to Deezer and MusicBrainz — the ripper's naming, carrying catalogue numbers, years and initials. "[WARP CD 156] Battles - Mirrored - 2007" matches nothing anywhere; "Mirrored" matches instantly. Across the reference library 501 of 515 records carry a tag album and 315 disagree with their folder, and no amount of cleaning turns "ACPP" into "Prospect Park, Brooklyn NY". The record's own claim now leads and the folder is the fallback, in one place (src/lib/import-title.js) that apply and the picker both read — apply already resolved it this way, and only the picker did not. The verdict judge had the same split: it compared candidates against the folder too, so an exact answer scored as mere containment and a clean match wore a warning. Identification also stops searching when searching cannot work. A folder named "1989 Pauls Boutique" is a hopeless query and an easy comparison, so after Deezer and MusicBrainz come back empty the artist's own catalogue is ranked by title similarity. Scoring rather than stripping is the point: 1989, 1984 and 2001 are real albums, and any cleaner aggressive enough to rescue Paul's Boutique would have destroyed them. Words split on whitespace with punctuation folded inside them, so "Paul's" stays one token; pressing vocabulary is ignored for scoring only; coverage is measured from both sides. 15 assertions pin the cases that would break it. Cached verdicts now carry the engine that produced them. A cached match is good forever, a cached "nothing found" only as good as the thing that looked — and nothing ever re-asked. Negatives from an older engine, or from a query that has since changed, are dropped and asked again. Soulseek was asked the same wrong question, with the artist said twice: "battles battles - mirrored - 2007". The server matches on ALL terms, so every leftover token narrows the search — that copy was only found because the peer happened to have 2007 in its path. The ladder now leads with the record's name and keeps the folder as the narrower rung behind it. AVAILABILITY OUTRANKS ENCODING. Candidate copies were ranked with avgBitrate above hasFreeSlot, and queue depth — captured on every candidate — was never read at all. A peer with no free slot and 277 people ahead beat a free peer with a queue of two, because its files reported a bitrate and the others reported none, and `?? 0` scores "didn't say" as the worst possible answer. Bitrate is now ignored between two fully-lossless copies, where it describes the material rather than the quality. And nothing ever gave up. Track 1 is requested alone so it is not queued behind eleven of our own files, the rest follow once the peer proves it will serve us, and a copy that makes no progress for 45s is abandoned for the next ranked one — cancelling the transfers it leaves, which used to trickle on forever. A refusal falls through to the next peer instead of ending the record. "Try another peer" is reachable while one is loading, which it never was. RE-IMPORTING BUILT A DUPLICATE FACTORY. Apply decided a track was already filed by comparing the destination's size to its source — but apply itself rewrites that file, embedding the chosen cover, so it is 582 KB larger from the moment it is written and the sizes can never match again. Every run laid down another " (n)" copy with its own library row: three runs left three of everything, 2.16 GB. A file at the destination that this run did not write is now simply an earlier import; within one run a repeated name is still a collision, which is what `kept` is for. ARTWORK FOLLOWS THE RECORD, ONLY THE AUDIO FOLLOWS THE CHOICE. Apply read art from whichever folder the audio came from, which for a kept found copy is the pen — and a downloaded album often carries no picture, so three records imported with no cover while their own files held a 1500px one. It also only ever looked at tags, never at the Folder.jpg the review had just offered as "yours". COVERS ARE RESAMPLED BY RUST, NOT BY THE COMPOSITOR. The webview was handed 2000px masters to paint 132px tiles — a 15x single-step reduction that reads as aliasing on photographic covers, and carries megabytes of base64 to draw a thumbnail. Artist portraits already had this exact fix; album covers never got it. getArt is now bounded by default and getArtMaster is the deliberate act, because ~60 places paint a cover and only two want real pixels: with the polarity the other way round, every surface anyone forgets is silently wrong. The fullscreen viewer also framed non-square art inside a square element, so its rounded corners and shadow wrapped 96px of nothing; it takes the artwork's own ratio now, bounded on both axes. Smaller, from the same passes: the preview play button is a real transport rather than a restart, and leaving a record silences its own preview without touching music started elsewhere; bucket chips navigate with per-lens memory instead of overwriting the saved resume position; Next counts from the record's real place in a queue that re-sorts under it, which was skipping one record for every one reviewed under a filter; a cover URL with nothing behind it is shown as the empty answer it is rather than a broken image in a pickable frame; write_tags returns early when there is nothing to change, instead of rewriting the whole file to write values it already had; and the import window has a Start over, which says what it costs before it costs it. --- crates/metadata/src/lib.rs | 279 +++++++++++++++++++ crates/tagger/src/lib.rs | 34 +++ src-tauri/src/commands.rs | 29 +- src-tauri/src/import.rs | 274 +++++++++++++++++-- src-tauri/src/pipeline.rs | 38 ++- src/components/ArtworkViewer.svelte | 39 ++- src/components/ImportPicker.svelte | 207 +++++++++++--- src/components/ImportWindow.svelte | 405 ++++++++++++++++++++++++---- src/components/Sidebar.svelte | 7 +- src/lib/api.js | 12 +- src/lib/import-audio.js | Bin 6227 -> 9641 bytes src/lib/import-evidence.js | 8 +- src/lib/import-title.js | 64 +++++ src/lib/state.svelte.js | 68 ++++- 14 files changed, 1326 insertions(+), 138 deletions(-) create mode 100644 src/lib/import-title.js diff --git a/crates/metadata/src/lib.rs b/crates/metadata/src/lib.rs index 6693d54..4861fe5 100644 --- a/crates/metadata/src/lib.rs +++ b/crates/metadata/src/lib.rs @@ -8187,6 +8187,23 @@ impl MetadataClient { // PURPOSE: the judge shows it as "Wrong record" honestly, which // beats an empty card (Jenny Death stays wrong, deliberately). } + // Still nothing: stop SEARCHING for the title and start COMPARING it. + // Both passes above ask a provider to find a phrase, so a folder whose + // name no release carries — "1989 Pauls Boutique" — cannot be found by + // either, however it is cleaned. Ranking the artist's own catalogue + // asks a different question, and one that tolerates a stray year, a + // missing apostrophe and a rip descriptor at once. + // + // Only when there is NOTHING: a wrong Deezer answer is left standing + // for the judge to call wrong, exactly as above. + if candidate.is_none() { + if let Some((c, matched, _score)) = + self.deezer_artist_catalogue(artist, raw_title, &tries).await + { + candidate = Some(c); + used = Some(matched); + } + } IdentifyResult { tries, used, @@ -8205,8 +8222,23 @@ impl MetadataClient { .await?; let hit = found.get("data")?.as_array()?.first()?.clone(); let id = hit.get("id")?.as_u64()?; + self.deezer_album_by_id(id, Some(&hit)).await + } + /// One album, fully hydrated: detail (label/date/genre) plus the full + /// paginated disc-aware tracklist — `/album/{id}` embeds only the first + /// ~25. `hit` is the search result when there was one; the album detail + /// carries the same fields, so the catalogue path passes None. + async fn deezer_album_by_id( + &self, + id: u64, + hit: Option<&serde_json::Value>, + ) -> Option { let full = self.deezer(&format!("https://api.deezer.com/album/{id}")).await; + let hit = match hit { + Some(h) => h.clone(), + None => full.clone()?, + }; let mut tracklist: Vec = Vec::new(); let mut next = Some(format!("https://api.deezer.com/album/{id}/tracks?limit=200")); while let Some(url) = next { @@ -8254,6 +8286,94 @@ impl MetadataClient { }) } + /// The artist's own catalogue, ranked against what we call this record. + /// + /// The last resort, and the one that answers "the search found nothing but + /// I can see the album exists". A folder named `1989 Pauls Boutique` is a + /// hopeless SEARCH — Deezer is asked for a phrase no release carries — but + /// it is an easy COMPARISON once you have the fifty-odd titles Deezer files + /// under Beastie Boys and can score each one. + /// + /// Anchored on an exactly-matching artist name, never a name-alike: the + /// same guard `select_audiodb_artist` applies, because the whole value of + /// this pass is that the candidate set is already known to be the right + /// artist's. Every name we have for the record is scored, and the best + /// title across all of them wins if it clears `MIN`. + async fn deezer_artist_catalogue( + &self, + artist: &str, + raw_title: &str, + tries: &[String], + ) -> Option<(IdentifyCandidate, String, f32)> { + /// Below this, "best match" is just "least bad" — better to say + /// nothing and let the record read as unmatched. + const MIN: f32 = 0.72; + + let want = normalize(artist); + if want.is_empty() { + return None; + } + let q = urlencode(artist); + let found = self + .deezer(&format!("https://api.deezer.com/search/artist?q={q}&limit=5")) + .await?; + let artist_id = found + .get("data")? + .as_array()? + .iter() + .find(|a| { + a.get("name") + .and_then(|v| v.as_str()) + .is_some_and(|n| normalize(n) == want) + }) + .and_then(|a| a.get("id")) + .and_then(|v| v.as_u64())?; + + // The catalogue, bounded: a prolific artist with hundreds of singles + // must not turn one unmatched record into an unbounded walk. + let mut albums: Vec<(u64, String)> = Vec::new(); + let mut next = Some(format!( + "https://api.deezer.com/artist/{artist_id}/albums?limit=100" + )); + while let Some(url) = next { + let Some(page) = self.deezer(&url).await else { break }; + for al in page.get("data").and_then(|d| d.as_array()).into_iter().flatten() { + if let (Some(id), Some(t)) = ( + al.get("id").and_then(|v| v.as_u64()), + al.get("title").and_then(|v| v.as_str()), + ) { + albums.push((id, t.to_string())); + } + } + if albums.len() >= 300 { + break; + } + next = page.get("next").and_then(|v| v.as_str()).map(String::from); + } + + // Score every name we have for this record against every album. + let mut names: Vec<&str> = vec![raw_title]; + for t in tries { + names.push(t.as_str()); + } + let mut best: Option<(u64, String, f32)> = None; + for (id, title) in &albums { + let score = names + .iter() + .map(|n| title_match_score(n, title)) + .fold(0.0_f32, f32::max); + if best.as_ref().is_none_or(|(_, _, b)| score > *b) { + best = Some((*id, title.clone(), score)); + } + } + let (id, title, score) = best?; + if score < MIN { + return None; + } + let candidate = self.deezer_album_by_id(id, None).await?; + Some((candidate, title, score)) + } + /// MusicBrainz release-group as the album RECORD, not just as cover art /// (the mock's mbCandidate). Exact-normalized title first, containment /// second; one official release with recordings gives the tracklist. @@ -11417,6 +11537,115 @@ fn normx(s: &str) -> String { .collect() } +/// A title's words, punctuation folded INSIDE each word rather than split on. +/// "Paul's Boutique" is two words, not three — splitting on the apostrophe +/// would leave "paul" and "s" and stop it matching a folder spelled "Pauls". +fn title_words(s: &str) -> Vec { + s.split_whitespace() + .map(|w| { + w.chars() + .filter(|c| c.is_alphanumeric()) + .flat_map(char::to_lowercase) + .collect::() + }) + .filter(|w| !w.is_empty()) + .collect() +} + +/// How strongly two album titles name the same record, 0.0 to 1.0. +/// +/// This is a COMPARISON, never a rewrite. Nothing is stripped from either +/// side, which is the whole point: an album genuinely called "1989" still +/// scores 1.0 against "1989", while a folder called "1989 Pauls Boutique" +/// scores high against "Paul's Boutique" and zero against "Licensed to Ill". +/// Stripping a leading year would have destroyed the first case to fix the +/// second. +/// +/// Scored on the EDITION-FOLDED form, so a provider selling "Paul's Boutique +/// (20th Anniversary Edition / Remastered)" is not penalised for the four +/// words that say which pressing it is. Coverage is measured from BOTH sides +/// and averaged: covering all of their title matters as much as covering all +/// of ours, so a short title contained in a long one cannot score full marks. +fn title_match_score(ours: &str, theirs: &str) -> f32 { + if ours.trim().is_empty() || theirs.trim().is_empty() { + return 0.0; + } + if normx(ours) == normx(theirs) || album_title_identity(ours) == album_title_identity(theirs) { + return 1.0; + } + let a = comparable_words(ours); + let b = comparable_words(theirs); + if a.is_empty() || b.is_empty() { + return 0.0; + } + // Multiset intersection: a word repeated in one title only counts as many + // times as the other title actually repeats it. + let mut pool: Vec<&String> = b.iter().collect(); + let mut shared = 0usize; + for w in &a { + if let Some(at) = pool.iter().position(|x| *x == w) { + pool.remove(at); + shared += 1; + } + } + if shared == 0 { + return 0.0; + } + // Half for covering the shorter title, half for covering the longer. The + // first half is what lets a folder carry an extra year and still match; + // the second is what stops a two-word title matching everything that + // contains it. + let (small, large) = if a.len() <= b.len() { (a.len(), b.len()) } else { (b.len(), a.len()) }; + let shared = shared as f32; + 0.5 * (shared / small as f32) + 0.5 * (shared / large as f32) +} + +/// Words that say which PRESSING this is rather than which record. Mirrors the +/// frontend's `EDITION_WORD` vocabulary (`import-evidence.js`), so a provider +/// selling "(20th Anniversary Edition / Remastered)" is compared on the four +/// words that name the album and not the four that name the pressing. +/// +/// Deliberately narrower than that list: "original" and "special" are dropped +/// from it because they are ordinary title words ("Original Pirate Material"). +const EDITION_NOISE: [&str; 14] = [ + "deluxe", + "expanded", + "remaster", + "remastered", + "anniversary", + "edition", + "editions", + "reissue", + "bonus", + "version", + "explicit", + "clean", + "mono", + "stereo", +]; + +/// A bare ordinal — the "20th" of "20th Anniversary". Alone it identifies +/// nothing, and left in it drags a real match below the threshold. +fn is_ordinal(w: &str) -> bool { + let (digits, suffix) = w.split_at(w.len().saturating_sub(2)); + !digits.is_empty() + && digits.chars().all(|c| c.is_ascii_digit()) + && matches!(suffix, "st" | "nd" | "rd" | "th") +} + +/// The words worth comparing. Falls back to the full set when stripping the +/// pressing vocabulary would leave nothing — a record really called "Deluxe" +/// still has to be comparable to itself. +fn comparable_words(s: &str) -> Vec { + let all = title_words(s); + let kept: Vec = all + .iter() + .filter(|w| !EDITION_NOISE.contains(&w.as_str()) && !is_ordinal(w)) + .cloned() + .collect(); + if kept.is_empty() { all } else { kept } +} + /// The mock's match rule: normalized equality or containment either way. /// Containment is what keeps an edition ("... 10th Anniversary Edition") /// from reading as a different record. @@ -14371,6 +14600,56 @@ mod tests { ); } + /// The ranking rule that replaced "strip the leading year". + /// + /// The point of scoring rather than stripping: a title that IS a year has + /// to survive. `1989`, `1984`, `2001` and `1999` are all real albums, and + /// any cleaner aggressive enough to rescue "1989 Pauls Boutique" would + /// have reduced every one of them to nothing. + #[test] + fn title_match_score_ranks_without_stripping() { + use super::title_match_score as score; + const MIN: f32 = 0.72; // the threshold deezer_artist_catalogue uses + + // The case this exists for: a year the folder added, an apostrophe the + // folder dropped, and a provider selling the anniversary pressing. + assert!(score("1989 Pauls Boutique", "Paul's Boutique") >= MIN); + assert!( + score( + "1989 Pauls Boutique", + "Paul's Boutique (20th Anniversary Edition / Remastered)" + ) >= MIN + ); + // …and it must not drag in the rest of the same artist's catalogue. + assert!(score("1989 Pauls Boutique", "Licensed to Ill") < MIN); + assert!(score("1989 Pauls Boutique", "Check Your Head") < MIN); + assert!(score("1989 Pauls Boutique", "Ill Communication") < MIN); + + // A title that is a year keeps its own identity, and does not become a + // free match for every record that happens to carry one. + assert_eq!(score("1989", "1989"), 1.0); + assert!(score("1989", "Paul's Boutique") < MIN); + assert!(score("1984", "1989") < MIN); + assert!(score("2001", "2001") >= MIN); + // The re-recording is a different work and must not beat the original. + assert!(score("1989", "1989") > score("1989", "1989 (Taylor's Version)")); + + // Rip descriptors. A short title buried in a long descriptor is the + // try-list's job, not the scorer's — `search_titles` hands its cleaned + // forms in as extra names to score, so "Heartleap - WEB V0" is matched + // through the try "Heartleap" rather than by loosening this. + assert!(score("Sung Tongs [FLAC]", "Sung Tongs") >= MIN); + + // The canonical wrong record stays wrong. + assert_eq!(score("Bastard", "Cherry Bomb"), 0.0); + // A shared word is not a match. + assert!(score("Greatest Hits", "Greatest Hits II") < 1.0); + assert!(score("Kid A", "Kid Amnesiae") < MIN); + // Nothing to compare is not a match with everything. + assert_eq!(score("", "Paul's Boutique"), 0.0); + assert_eq!(score("Paul's Boutique", ""), 0.0); + } + /// The edition rule's foundation: containment matches, disjoint titles /// don't (Bastard → Cherry Bomb is the canonical wrong record). #[test] diff --git a/crates/tagger/src/lib.rs b/crates/tagger/src/lib.rs index 7c95a63..480af4f 100644 --- a/crates/tagger/src/lib.rs +++ b/crates/tagger/src/lib.rs @@ -93,8 +93,42 @@ pub fn write_tags( art_jpeg: Option<&[u8]>, replace_art: bool, ) -> Result<()> { + // Saving REWRITES the file, so a call with nothing to say costs the whole + // length of it twice — read and write — on top of the copy that just put + // it there. The import calls this for every track of every record that + // carries any choice at all, so a record whose review changed nothing was + // paying a full second pass over the album to write exactly the values it + // already had. + // + // Art is the subtle half: with `replace_art` false a picture is only added + // when the file HAS none, so a file that already carries art has nothing to + // do either. Checking that needs the file open, so the cheap fields are + // tested first and the probe only happens when something might change. + let writes_field = write.artist.is_some() + || write.album_artist.is_some() + || write.album.is_some() + || write.title.is_some() + || write.track_no.is_some() + || write.year.is_some() + || write.label.is_some() + || write.genre.is_some(); + if !writes_field && art_jpeg.is_none() { + return Ok(()); + } + let mut tagged = Probe::open(path)?.read()?; + // Nothing to write, and the art it would add is already there. + if !writes_field && !replace_art { + let has_art = tagged + .primary_tag() + .or_else(|| tagged.first_tag()) + .is_some_and(|t| !t.pictures().is_empty()); + if has_art { + return Ok(()); + } + } + if tagged.primary_tag().is_none() && tagged.first_tag().is_none() { let tag_type = tagged.primary_tag_type(); tagged.insert_tag(Tag::new(tag_type)); diff --git a/src-tauri/src/commands.rs b/src-tauri/src/commands.rs index e8ac0db..a79ab51 100644 --- a/src-tauri/src/commands.rs +++ b/src-tauri/src/commands.rs @@ -4114,12 +4114,31 @@ pub async fn refresh_all_album_art(app: tauri::AppHandle) -> Result Result { - let bytes = tauri::async_runtime::spawn_blocking(move || std::fs::read(path)) - .await - .map_err(|e| e.to_string())? - .map_err(|e| e.to_string())?; +pub async fn album_art(path: String, max_edge: Option) -> Result { + let bytes = tauri::async_runtime::spawn_blocking(move || -> Result, String> { + let raw = std::fs::read(&path).map_err(|e| e.to_string())?; + let Some(edge) = max_edge else { return Ok(raw) }; + // An undecodable or already-small source falls through to the original + // bytes: a thumbnail is an optimisation, never a reason to show nothing. + match pipeline::thumbnail_jpeg(&raw, edge) { + Ok(Some(thumb)) => Ok(thumb), + _ => Ok(raw), + } + }) + .await + .map_err(|e| e.to_string())??; use base64::Engine as _; Ok(format!( "data:image/jpeg;base64,{}", diff --git a/src-tauri/src/import.rs b/src-tauri/src/import.rs index 9f236df..d183309 100644 --- a/src-tauri/src/import.rs +++ b/src-tauri/src/import.rs @@ -571,6 +571,58 @@ fn local_art_of(app: &tauri::AppHandle, source_root: &str, rel: &str) -> Option< }) } +/// A record's own artwork, in the order the review means it: embedded in its +/// files first, then the best loose image sitting beside them. +/// +/// Mirrors [`local_art_of`], which is what the picker shows as "yours" — the +/// two must agree, or the review offers a cover apply cannot file. +fn art_of(dir: &Path, files: &[PathBuf]) -> Option> { + for f in files.iter().take(5) { + if let Ok(Some(bytes)) = tagger::extract_art(f) { + return Some(bytes); + } + } + folder_image_of(dir).and_then(|p| std::fs::read(&p).ok()) +} + +/// The best loose image in a record's own folder, largest first, WMP's derived +/// thumbnails only when nothing else is there. +/// +/// Shared with [`local_art_of`]'s rules on purpose: the picker offers this +/// picture as "yours", so apply has to be able to file the same one. Apply +/// used to look only for EMBEDDED art, which meant a record whose artwork +/// lives in a `Folder.jpg` was shown a cover in the review, accepted with +/// "yours", and then imported with no cover at all. +fn folder_image_of(dir: &Path) -> Option { + let mut real: Vec = Vec::new(); + let mut derived: Vec = Vec::new(); + for e in std::fs::read_dir(dir).ok()?.flatten() { + let p = e.path(); + let is_img = p.is_file() + && p.extension() + .and_then(|x| x.to_str()) + .map(|x| IMAGE_EXTS.contains(&x.to_ascii_lowercase().as_str())) + .unwrap_or(false); + if !is_img { + continue; + } + let name = p.file_name().unwrap_or_default().to_string_lossy().to_lowercase(); + if name.starts_with("albumart") { + derived.push(p); + } else { + real.push(p); + } + } + let pool = if real.is_empty() { derived } else { real }; + pool.into_iter() + .filter_map(|p| { + let (w, h) = image::image_dimensions(&p).ok()?; + Some((p, u64::from(w) * u64::from(h))) + }) + .max_by_key(|(_, px)| *px) + .map(|(p, _)| p) +} + /* -------------------------------------------------------------- audition -- */ /// Transfer ids belonging to audition downloads, each mapped to the record it @@ -623,9 +675,16 @@ pub(crate) fn audition_dir(download_root: &Path, rel: &str) -> PathBuf { /// Download a found album copy into `(Cache)\_audition\\` for the /// version comparison — the WHOLE album (his call: keeping it later is a -/// move, not a re-download, and one search's worth of requests). The caller -/// orders the list with track 1 first so the A/B can start while the rest -/// fills in. Returns transfer ids aligned with the input files. +/// move, not a re-download, and one search's worth of requests). +/// +/// Called TWICE per copy: track 1 alone, then the rest once it lands. Asking +/// for all twelve at once put eleven of our own requests in front of the one +/// file the comparison needs — the peer serves its queue in its own order, so +/// "track 1 first" was only ever true of the list we sent. `start_index` keeps +/// the second batch's indices continuing from the first, because the UI reads +/// index 0 as "this is track 1". +/// +/// Returns transfer ids aligned with the input files. #[tauri::command] pub async fn import_audition( app: tauri::AppHandle, @@ -633,6 +692,7 @@ pub async fn import_audition( username: String, rel: String, files: Vec, + start_index: Option, ) -> Result, String> { let guard = state.client.lock().await; let client = guard.as_ref().ok_or("not connected")?; @@ -647,8 +707,9 @@ pub async fn import_audition( // immediately, and an event whose id is not yet registered is one the // UI throws away — and worse, one commands.rs routes down the ordinary // download path instead of the pen's. + let idx = start_index.unwrap_or(0) + i; let id = client.download_to_claimed(&username, &f.path, f.size, target, |id| { - AUDITION_IDS.lock().unwrap().insert(id, (rel.clone(), i)); + AUDITION_IDS.lock().unwrap().insert(id, (rel.clone(), idx)); }); ids.push(id); } @@ -1029,11 +1090,25 @@ fn apply_one( if let Some(bytes) = chosen_art { art_sources.push(bytes.to_vec()); } else { - for f in files.iter().take(5) { - if let Ok(Some(bytes)) = tagger::extract_art(f) { - art_sources.push(bytes); - break; - } + // No provider cover chosen means "keep the LOCAL artwork" — and local + // is the user's OWN folder, which is where the review found the + // picture it put on screen. + // + // This used to read whatever folder the AUDIO came from, which is a + // different folder entirely once a record keeps the found copy: the + // pen. A downloaded album frequently carries no embedded picture, so + // keeping someone else's copy threw away the artwork the review had + // just shown — three records imported with no cover at all while + // their own files held a 1500px one. + // + // Art follows the RECORD; only the audio follows the choice. + let own_dir = src_root.join(&a.rel); + let mut own_files = audio_files_in(&own_dir); + own_files.sort(); + if let Some(bytes) = + art_of(&own_dir, &own_files).or_else(|| art_of(&src_dir, &files)) + { + art_sources.push(bytes); } } // The master cover is the album folder's cover.jpg — filed into the @@ -1089,17 +1164,24 @@ fn apply_one( // from an earlier apply: reuse it, so re-applying is idempotent // rather than a " (2)" duplicate factory. let mut dest = dest_dir.join(pipeline::safe_component(&name)); - let src_len = std::fs::metadata(src).map(|m| m.len()).unwrap_or(0); - // "Same name, same size" means an EARLIER RUN already filed this very - // track — but only if this run has not just written that path itself. - // A folded release flattens its parts into one folder, and parts number - // from 01 each: `Disc 1/01.mp3` and `Disc 2/01.mp3` collide, and often - // at the same length, so the idempotency rule read the second one as - // already-imported and dropped it while still counting it. Two - // different files in one run are a collision, never a reuse. - let already = dest.exists() - && !kept.contains(&dest.to_string_lossy().to_lowercase()) - && std::fs::metadata(&dest).map(|m| m.len()).unwrap_or(u64::MAX) == src_len; + // A file already at this path, that THIS RUN did not write, is this + // very track from an earlier apply. + // + // Decided by name alone. It used to also require the destination to be + // the same SIZE as the source, which was self-defeating: apply writes + // the review's tags and embeds the chosen cover INTO the copy, so an + // imported file is bigger than its source from the moment it is made + // (582 KB bigger, measured on a Tribe Called Quest track). The sizes + // could therefore never match again, `already` was false on every + // re-run, and each import laid down another " (n)" copy — three runs + // left three copies of every track on disk and three rows in the + // library. The rule meant to stop a duplicate factory was driving one. + // + // Within ONE run a repeated name is still a genuine collision: a folded + // release flattens parts that each number from 01, so `Disc 1/01.mp3` + // and `Disc 2/01.mp3` land in the same folder. `kept` — what this run + // has written — is what tells a collision from a reuse. + let already = dest.exists() && !kept.contains(&dest.to_string_lossy().to_lowercase()); if dest.exists() && !already { let stem = dest.file_stem().unwrap_or_default().to_string_lossy().into_owned(); let ext = dest @@ -1340,6 +1422,158 @@ mod tests { out } + /// Re-importing the same record must not copy it again — even though the + /// imported files no longer match their sources byte for byte. + /// + /// They never can: apply writes the review's tags and embeds the chosen + /// cover into the copy, so every imported file is larger than the source + /// it came from. The old rule asked "same name AND same size?" to decide a + /// track was already filed, so after the first run the answer was always + /// no, and each import laid down another " (n)" copy with its own library + /// row. Three runs over one album left three of everything. + #[test] + fn re_importing_does_not_duplicate_a_track_apply_has_since_rewritten() { + let b = bed(0); + let a = album(&b, Some(choices("yours", None))); + run(&b, &a).unwrap(); + assert_eq!(landed(&b).len(), 2, "first import files the album"); + + // What tagging does: the copy grows. Nothing else about it changed. + let dir = b.dest_root.join("Weezer").join("Pinkerton"); + for n in 1..=2 { + let f = dir.join(format!("{n:02} track.mp3")); + let mut body = std::fs::read(&f).unwrap(); + body.extend_from_slice(b"...embedded cover bytes..."); + std::fs::write(&f, body).unwrap(); + } + + let (tracks, _, _) = run(&b, &a).unwrap(); + let after = landed(&b); + assert_eq!(tracks, 2, "the record still reports its two tracks"); + assert_eq!( + after.len(), + 2, + "a second import must add no files, got {:?}", + after.iter().map(|(n, _)| n).collect::>() + ); + assert!( + !after.iter().any(|(n, _)| n.contains('(')), + "no \" (n)\" duplicate was made: {:?}", + after.iter().map(|(n, _)| n).collect::>() + ); + // …and the rewritten bytes are left alone, not clobbered by a re-copy. + assert!( + after.iter().all(|(_, body)| body.ends_with("...embedded cover bytes...")), + "the imported file keeps the tags apply wrote into it" + ); + } + + /// A record whose artwork is a loose file rather than a tag still gets a + /// cover. The review offers that picture as "yours", so accepting it has + /// to file it — apply used to look only at embedded art, and imported a + /// record it had just shown a cover for with no cover at all. + #[test] + fn a_folder_image_is_filed_when_nothing_is_embedded() { + let b = bed(0); + let src = b.src_root.join(&b.rel); + // A real JPEG, because the cover writer decodes what it is given. + let img = image::RgbImage::from_pixel(8, 8, image::Rgb([9, 9, 9])); + let mut jpeg = std::io::Cursor::new(Vec::new()); + image::DynamicImage::ImageRgb8(img) + .write_with_encoder(image::codecs::jpeg::JpegEncoder::new(&mut jpeg)) + .unwrap(); + std::fs::write(src.join("Folder.jpg"), jpeg.into_inner()).unwrap(); + // The tracks carry no tags at all, so there is nothing to extract. + + run(&b, &album(&b, None)).unwrap(); + + let dir = b.dest_root.join("Weezer").join("Pinkerton"); + assert!(dir.join("cover.jpg").is_file(), "the folder image became the album cover"); + assert!(dir.join("folder.jpg").is_file(), "and the second canonical name"); + } + + /// Keeping the FOUND COPY replaces the audio, not the artwork. + /// + /// The review shows the picture from YOUR folder and calls it "yours"; a + /// downloaded copy often carries none at all. Apply used to read art from + /// whichever folder the audio came from, so choosing "theirs" imported the + /// record with no cover while a 1500px one sat in the source files. + #[test] + fn keeping_theirs_still_takes_the_artwork_from_your_own_folder() { + let b = bed(2); + // The pen's copy has no picture anywhere. The user's folder does. + let src = b.src_root.join(&b.rel); + let img = image::RgbImage::from_pixel(8, 8, image::Rgb([7, 7, 7])); + let mut jpeg = std::io::Cursor::new(Vec::new()); + image::DynamicImage::ImageRgb8(img) + .write_with_encoder(image::codecs::jpeg::JpegEncoder::new(&mut jpeg)) + .unwrap(); + let bytes = jpeg.into_inner(); + std::fs::write(src.join("Folder.jpg"), &bytes).unwrap(); + + run(&b, &album(&b, Some(choices("theirs", Some(2))))).unwrap(); + + let dir = b.dest_root.join("Weezer").join("Pinkerton"); + assert!( + dir.join("cover.jpg").is_file(), + "the record's own artwork is filed even though the audio came from the pen" + ); + // And the audio really is the found copy, not a silent fallback. + let landed_names: Vec = landed(&b).into_iter().map(|(n, _)| n).collect(); + assert!( + landed_names.iter().any(|n| n.ends_with(".flac")), + "the pen's tracks are what landed: {landed_names:?}" + ); + } + + /// Windows Media Player's derived thumbnails are a last resort, never a + /// preference — a real picture beside them wins. + #[test] + fn a_real_folder_image_beats_a_media_player_thumbnail() { + let b = bed(0); + let src = b.src_root.join(&b.rel); + let write = |name: &str, size: u32| { + let img = image::RgbImage::from_pixel(size, size, image::Rgb([1, 2, 3])); + let mut out = std::io::Cursor::new(Vec::new()); + image::DynamicImage::ImageRgb8(img) + .write_with_encoder(image::codecs::jpeg::JpegEncoder::new(&mut out)) + .unwrap(); + std::fs::write(src.join(name), out.into_inner()).unwrap(); + }; + write("AlbumArt_Large.jpg", 64); // WMP's, and the bigger of the two + write("Folder.jpg", 16); + + let picked = super::folder_image_of(&src).unwrap(); + assert_eq!( + picked.file_name().unwrap(), + "Folder.jpg", + "a real image wins even when the derived one is larger" + ); + } + + /// The other half of the same rule: two DIFFERENT files that land on one + /// name inside a single run are a collision, and both must survive. + #[test] + fn one_run_still_disambiguates_two_files_with_the_same_name() { + let b = bed(0); + // A folded release: parts that each number from 01. + let part_a = b.src_root.join(&b.rel).join("Disc 1"); + let part_b = b.src_root.join(&b.rel).join("Disc 2"); + std::fs::create_dir_all(&part_a).unwrap(); + std::fs::create_dir_all(&part_b).unwrap(); + std::fs::write(part_a.join("01 track.mp3"), "disc one").unwrap(); + std::fs::write(part_b.join("01 track.mp3"), "disc two").unwrap(); + let mut a = album(&b, None); + a.parts = vec!["Disc 1".into(), "Disc 2".into()]; + + let (tracks, _, _) = run(&b, &a).unwrap(); + let after = landed(&b); + assert_eq!(tracks, 2, "both discs' track 1 is imported"); + let bodies: Vec<&str> = after.iter().map(|(_, body)| body.as_str()).collect(); + assert!(bodies.contains(&"disc one") && bodies.contains(&"disc two"), + "neither disc was dropped as a false reuse: {after:?}"); + } + #[test] fn keeping_theirs_moves_the_pen_and_leaves_the_source_alone() { let b = bed(2); diff --git a/src-tauri/src/pipeline.rs b/src-tauri/src/pipeline.rs index b943bb7..9b47248 100644 --- a/src-tauri/src/pipeline.rs +++ b/src-tauri/src/pipeline.rs @@ -768,16 +768,42 @@ async fn fetch_artist_art_cached( Some(target.to_string_lossy().into_owned()) } +/// One definition of "a thumbnail" for every surface that shows art in a tile: +/// Lanczos3 down to `max_edge`, re-encoded JPEG q92. +/// +/// The webview is a poor resampler for big reductions. It scales a decoded +/// source into the tile in ONE bilinear step, so a 2000px cover in a 132px +/// card samples roughly every fifteenth pixel — which reads as aliasing, and +/// is worst on photographic covers with fine detail or overlaid text. Lanczos3 +/// on the Rust side averages the pixels that are actually being dropped. +/// +/// `Ok(None)` means the source is already within `max_edge` and should be +/// handed through untouched; `Err(())` means it could not be decoded at all. +/// Aspect ratio is preserved (`resize` fits inside the box), so a non-square +/// cover stays non-square and the tile's `object-fit: cover` still does the +/// cropping — the thumbnail never decides the composition. +pub fn thumbnail_jpeg(bytes: &[u8], max_edge: u32) -> Result>, ()> { + let decoded = image::load_from_memory(bytes).map_err(|_| ())?; + if decoded.width().max(decoded.height()) <= max_edge { + return Ok(None); + } + let thumb = decoded.resize(max_edge, max_edge, image::imageops::FilterType::Lanczos3); + let mut out = std::io::Cursor::new(Vec::new()); + let encoder = image::codecs::jpeg::JpegEncoder::new_with_quality(&mut out, 92); + thumb.write_with_encoder(encoder).map_err(|_| ())?; + Ok(Some(out.into_inner())) +} + /// Preserve the original portrait, but hand the webview a clean 512px /// thumbnail generated with Lanczos3. This is large enough for a 2x card and /// avoids browser scaling a huge compressed source directly into the tile. fn write_artist_thumbnail(bytes: &[u8], target: &Path) -> Option<()> { - let decoded = image::load_from_memory(bytes).ok()?; - let thumb = decoded.resize(512, 512, image::imageops::FilterType::Lanczos3); - let mut out = std::io::Cursor::new(Vec::new()); - let encoder = image::codecs::jpeg::JpegEncoder::new_with_quality(&mut out, 92); - thumb.write_with_encoder(encoder).ok()?; - std::fs::write(target, out.into_inner()).ok() + let out = match thumbnail_jpeg(bytes, 512) { + Ok(Some(thumb)) => thumb, + Ok(None) => bytes.to_vec(), + Err(()) => return None, + }; + std::fs::write(target, out).ok() } /// Refresh portraits for every distinct artist already represented in the diff --git a/src/components/ArtworkViewer.svelte b/src/components/ArtworkViewer.svelte index d6ddbcf..c41a45b 100644 --- a/src/components/ArtworkViewer.svelte +++ b/src/components/ArtworkViewer.svelte @@ -6,7 +6,7 @@ // at the full resolution of the canonical cover (3000px-class for // library albums). Click anywhere, Escape, or the corner button closes. import { fade } from "svelte/transition"; - import { ui, getArt, getCatalogArt } from "../lib/state.svelte.js"; + import { ui, getArtMaster, getCatalogArt } from "../lib/state.svelte.js"; import Icon from "./Icon.svelte"; // { src, local, title, subtitle } — the caller says which cover and how to @@ -22,6 +22,14 @@ const AMBIENT_BLUR = 18; let artUrl = $state(null); let ambientUrl = $state(null); + /** The artwork's own width/height, once it has decoded. + * + * The frame is the IMG element itself — it carries the rounded corners and + * the drop shadow — so a fixed square box around a 4:3 cover framed empty + * letterbox bands instead of the picture. Sizing the element to the real + * ratio makes the frame hug the artwork whatever shape it is. 1 until the + * image reports, which is also what the placeholder wants. */ + let ratio = $state(1); async function makeAmbient(src, isActive) { const img = new Image(); @@ -61,9 +69,13 @@ const source = art; artUrl = null; ambientUrl = null; + ratio = 1; if (!source?.src) return; let active = true; - const pending = source.local ? getArt(source.src) : getCatalogArt(source.src); + // The master, deliberately: this window's whole point is the artwork at + // its real resolution, so it is one of the two places that opts out of + // the tile-sized default. + const pending = source.local ? getArtMaster(source.src) : getCatalogArt(source.src); pending .then(async (url) => { if (!active || !url) return; @@ -121,7 +133,17 @@ {#if artUrl} - {art.title + {art.title { + const { naturalWidth: w, naturalHeight: h } = e.currentTarget; + if (w > 0 && h > 0) ratio = w / h; + }} + /> {:else}
{/if} @@ -181,8 +203,15 @@ .art { position: relative; z-index: 2; - width: min(94vh, 94vw); - aspect-ratio: 1; + /* Sized to the artwork's own ratio (--ar), bounded by BOTH axes: a wide + cover is capped by the viewport width, a tall one by its height. The + old `width: min(94vh, 94vw); aspect-ratio: 1` forced a square, so + anything non-square wore its frame around empty letterbox space. + Still fills the viewport for small masters, as it did before. */ + --ar: 1; + aspect-ratio: var(--ar); + width: min(94vw, 94vh * var(--ar)); + height: auto; object-fit: contain; border-radius: 10px; box-shadow: 0 48px 110px -34px rgba(0, 0, 0, 0.9); diff --git a/src/components/ImportPicker.svelte b/src/components/ImportPicker.svelte index bf29a81..2643229 100644 --- a/src/components/ImportPicker.svelte +++ b/src/components/ImportPicker.svelte @@ -13,12 +13,20 @@ import { fade } from "svelte/transition"; import * as api from "../lib/api.js"; import Icon from "./Icon.svelte"; - import { getArt, playLocalFile } from "../lib/state.svelte.js"; + import { onDestroy } from "svelte"; + import { getArtMaster, playLocalFile, player, togglePlayPause } from "../lib/state.svelte.js"; import { evidence as matchEvidence, normx } from "../lib/import-evidence.js"; + import { IDENTIFY_ENGINE } from "../lib/import-title.js"; let { album: a, // derived album from ImportWindow (rel, title, …) artist, // the resolved artist (plan logic, not folder-blind) + albumName, // the resolved album NAME — tags first, folder as + // fallback. This is what the record IS: the + // heading, the verdict, what the providers are + // asked, and what the player announces. `a.title` + // is the folder on disk and survives as a hover. + sourceRoot, index, total, // where the queue cursor is found, // rel -> { state, result } identify cache ($state, shared) @@ -31,6 +39,8 @@ audition, // the found-copy state for this record (window-owned) live, // in-flight transfer facts + stall verdict, or null onFindCopy, // () => void — search + start the audition download + onTryAnother, // () => void — abandon this peer for the next + // ranked copy, without a fresh search accentStyle, // this record's --accent pair, cast by the window } = $props(); @@ -90,11 +100,15 @@ found[rel] = { state: "loading", result: null }; (async () => { try { - const r = await api.importIdentify(artist, a.title); - found[rel] = { state: "done", result: r }; + const ask = albumName ?? a.title; + const r = await api.importIdentify(artist, ask); + // Stamped with the engine that produced it: a cached "nothing + // found" is only trustworthy while the thing that looked has + // not learned anything new. See IDENTIFY_ENGINE. + found[rel] = { state: "done", result: r, engine: IDENTIFY_ENGINE }; if (r.candidate?.cover) { const rep = await api.importCoverReport( - artist, r.used ?? a.title, r.candidate.cover); + artist, r.used ?? ask, r.candidate.cover); reports[rel] = rep; } } catch (e) { @@ -124,12 +138,14 @@ () => {}); }); - // Local art renders through the same data-URL path every cover uses. + // The MASTER, not the tile-sized default: this card exists to judge one + // cover against another, and the loupe magnifies it 3x. A resampled copy + // would hide exactly the difference being judged. let yoursSrc = $state(null); $effect(() => { const p = yours?.path; yoursSrc = null; - if (p) getArt(p).then((u) => { if (yours?.path === p) yoursSrc = u; }); + if (p) getArtMaster(p).then((u) => { if (yours?.path === p) yoursSrc = u; }); }); // ---- provider covers, as the mock's tiles --------------------------- @@ -182,21 +198,40 @@ // for which of two very different situations the panel is in: the report // is still coming, or it came back with nothing. Neither is "no service // has this record" — the identify already proved one does. + /** Candidate cover URLs the webview could not load, by url. + * + * A provider can name a cover it does not have: MusicBrainz identifies a + * release group and the cover URL is built from its MBID, but the Cover + * Art Archive answers 404 when nothing has been uploaded for it. Only the + * knows — the report says "size unknown", which is also what a + * measurement that merely timed out says. */ + let deadArt = $state({}); + + /** Every candidate turned out to be a URL with nothing behind it — which + * is a different sentence from "the sizes could not be measured": the + * service was reached and simply has no picture. */ + const allDead = $derived(covers.length > 0 && covers.every((c) => deadArt[c.url])); + const unmeasured = $derived(covers.length === 1 && covers[0].px == null); const measuring = $derived(unmeasured && !report); - const measureFailed = $derived(unmeasured && !!report); + const measureFailed = $derived(unmeasured && !!report && !allDead); /** A match with no cover at all — a real answer, and not the same as * nobody having heard of the record. */ const matchHasNoArt = $derived( idn?.state === "done" && !!candidate && !candidate.cover && !covers.length); // ---- evidence (shared with the window's outcome buckets) ------------- - const ev = $derived(matchEvidence(candidate, artist, a.title, a.tracks.length)); + // Judged against the record's own NAME, the same string the provider was + // asked about — not the folder. Comparing a candidate to the folder made + // an exact hit look doubtful: "Mirrored" against + // "[WARP CD 156] Battles - Mirrored - 2007" is a containment, so a clean + // match wore a "!" on its title row. + const ev = $derived(matchEvidence(candidate, artist, albumName ?? a.title, a.tracks.length)); const verdictLine = $derived.by(() => { const c = candidate; const v = { - none: ["warn", "Nothing on the provider matches this folder. It may be a bootleg, a session, or named differently."], + none: ["warn", "Nothing on the provider matches this record. It may be a bootleg, a session, or named differently."], bad: ["bad", "This does not look like the same record. Applying it would replace your files with a different album."], warn: ["warn", "Close, but not identical. Check the tracklist and listen before applying — it may be a different edition."], ok: ["ok", "Artist, title and track count all agree. Listen to be certain."], @@ -403,25 +438,53 @@ const yoursPath = $derived( a.tracks[0] ? `${sourceRoot}\\${a.rel}\\${a.tracks[0].name}` : null); const resolvedAudio = $derived(choice?.audio ?? "yours"); + + // ---- preview transport ---------------------------------------------- + // A preview plays through the MAIN player (playLocalFile replaces the + // queue), so "is this one playing?" is a question about the player's + // current local path — not about anything this card owns. + const nowPath = $derived(player.track?.source === "local" ? player.track.path : null); + const isLive = (p) => !!p && nowPath === p; + /** Sounding right now (buffering counts — the button must offer the stop + * before the audio arrives, or a slow start is unstoppable). */ + const isSounding = (p) => isLive(p) && (player.status === "playing" || player.status === "buffering"); + + /** One button, both jobs: start this preview, or pause/resume it if it is + * already the thing playing. Previously every press re-issued + * playLocalFile, which restarted the track — there was no way to stop it. */ + function togglePreview(path, start) { + if (!path) return; + if (isLive(path)) togglePlayPause(); + else start(); + } function playYours() { - if (!yoursPath) return; playLocalFile( yoursPath, a.tracks[0].name.replace(/\.[a-z0-9]+$/i, ""), - `${artist} — ${a.title} · yours`, + `${artist} — ${albumName ?? a.title} · yours`, yours?.path ?? null, ); } function playTheirs() { - if (!audition?.track1Path) return; playLocalFile( audition.track1Path, (audition.probe?.title ?? "Track 1"), - `${artist} — ${a.title} · found copy`, + `${artist} — ${albumName ?? a.title} · found copy`, yours?.path ?? null, ); } + // Leaving the record silences ITS preview. The card is re-keyed per record, + // so this fires on every move as well as on closing the window. + // + // Deliberately narrow: only when what is sounding is one of THIS record's + // two files. Music the user started before opening the importer is theirs, + // and walking through records must not silence it. + onDestroy(() => { + const mine = [yoursPath, audition?.track1Path].filter(Boolean); + if (mine.includes(nowPath) && player.status === "playing") togglePlayPause(); + }); + // Real peaks for both lanes, drawn against the JOINT maximum so a // quieter master honestly looks smaller — the shapes are comparable // because the scale is shared (his ask: match the waveforms). @@ -499,7 +562,11 @@ skipped for now {/if} -

{a.title}

+ +

{albumName ?? a.title}

{artist} · {a.tracks.length} tracks · {fmtMB(a.bytes)} · {ext} {a.avgBitrate ?? "—"}k
@@ -517,6 +584,7 @@ {#if !idn || idn.state === "loading"}looking the record up… {:else if idn.state === "error"}the lookup failed {:else if measuring}found it — measuring what each service has… + {:else if allDead}found it, but no service has a cover for it {:else if measureFailed}found it, but the sizes could not be measured {:else if matchHasNoArt}the matching release carries no artwork {:else if !covers.length}no service has this record @@ -547,23 +615,42 @@
{resolvedArt === "yours" ? "✓ using this" : "use this"}
{#each covers as c, n (c.url)} + {@const dead = !!deadArt[c.url]} {@const smaller = yours && c.px && c.px < yours.w} {@const wrongTitle = c.title && norm2(c.title) !== norm2(a.title)} +
onChoose({ art: c.provider })} - onkeydown={(e) => e.key === "Enter" && onChoose({ art: c.provider })}> -
{ e.stopPropagation(); openFs(n + 1); }} + class:dead + role={dead ? "presentation" : "button"} tabindex={dead ? -1 : 0} + title={dead + ? `${c.provider} has no cover on file for this release` + : `Use ${c.provider}'s artwork`} + onclick={() => !dead && onChoose({ art: c.provider })} + onkeydown={(e) => !dead && e.key === "Enter" && onChoose({ art: c.provider })}> +
{ if (!dead) { e.stopPropagation(); openFs(n + 1); } }} role="presentation"> -
-
{c.px - ? `${c.px} × ${c.realH ?? c.px}` - : measuring ? "measuring…" : "size unknown"} - {c.provider}{smaller ? " · smaller than yours" : ""}
+ {#if dead} +
nothing on file here —
{c.provider} has no cover for this release
+ {:else} + (deadArt[c.url] = true)} /> + {/if}
+
{dead + ? "no cover" + : c.px + ? `${c.px} × ${c.realH ?? c.px}` + : measuring ? "measuring…" : "size unknown"} + {c.provider}{smaller && !dead ? " · smaller than yours" : ""}
{#if wrongTitle}
“{c.title}”
{/if} -
{resolvedArt === c.provider ? "✓ using this" : "use this"}
+
{dead + ? "nothing to use" + : resolvedArt === c.provider ? "✓ using this" : "use this"}
{/each} +
+ Found copy {au.probe ? `${(au.candidate.files[0]?.name ?? "").split(".").pop().toUpperCase()} ${au.probe.bitrate ?? "—"}k${au.probe.bitDepth ? ` · ${au.probe.bitDepth}-bit` : ""} · measured` : theirsReady @@ -707,16 +804,18 @@ nothing used to say so — the pill changed and that was all. --> {#if !theirsReady && live?.stalled} Nothing has arrived for {Math.round(live.quietMs / 1000)}s. - The peer may have gone offline or dropped the queue. You can wait, search - again, or keep your own copy — nothing is lost either way. + The peer may have gone offline or dropped the queue. You can wait, try + another peer, or keep your own copy — nothing is lost either way. {:else if !theirsReady} - Track 1 comes first on purpose: the comparison can start before the rest of - the album is down. + Track 1 is asked for on its own, so it is not queued behind the other + {au.candidate.trackCount - 1} files of its own album.{#if au.switchedFrom} +  · Moved on from {au.switchedFrom}{au.reason ? ` — ${au.reason}` : ""}. + {/if} {:else if au.probe?.seconds && a.tracks[0]?.seconds} {@const d = Math.abs(a.tracks[0].seconds - au.probe.seconds)} {#if d <= 2} Ready to compare. - Track 1 is the same length in both ({mmss(au.probe.seconds)}) — the strongest signal it is the same recording. The waveforms share one scale, so shape and loudness compare honestly.{#if au.status !== "complete"} The other {Math.max(0, (au.ids?.length ?? 0) - (au.arrived ?? 0))} tracks are still coming; keeping this version waits for them.{/if} + Track 1 is the same length in both ({mmss(au.probe.seconds)}) — the strongest signal it is the same recording. The waveforms share one scale, so shape and loudness compare honestly.{#if au.status !== "complete"} The other {Math.max(0, (au.expected ?? au.ids?.length ?? 0) - (au.arrived ?? 0))} tracks are still coming; keeping this version waits for them.{/if} {:else} Track 1 differs by {d}s — different recording, different edit, or a different record. Listen before keeping it. {/if} @@ -726,6 +825,22 @@ The whole album downloads into the cache — if you keep it, nothing is fetched twice; if you keep yours, the sweep reclaims it in a week. {/if}
+ + {#if !theirsReady} + {@const alts = au.alts?.length ?? 0} +
+ + {alts + ? `${alts} more ${alts === 1 ? "copy" : "copies"} ranked behind this one${au.candidate.queue ? ` — this peer has ${au.candidate.queue} ahead of you` : ""}. Switching cancels this download; nothing already cached is lost.` + : "No ranked alternates left for this copy — this asks the network again."} +
+ {/if} {/if} @@ -916,7 +1031,7 @@  vs  {right.provider} {right.px} × {right.realH ?? right.px}
- {#each covers as c, n (c.url)} + {#each covers.filter((c) => !deadArt[c.url]) as c, n (c.url)}
(fsPick = n + 1)} @@ -1028,6 +1143,12 @@ .im-art-res { font-variant-numeric: tabular-nums; } .im-art.better .im-art-res { color: var(--accent-hover); } .im-art.worse .im-art-res { color: #e9b949; } + /* Not an option, so it does not look like one: no pointer, no hover lift, + and the size line stays muted rather than reading as a measurement. */ + .im-art.dead { cursor: default; } + .im-art.dead:hover { background: none; } + .im-art.dead .im-art-res, + .im-art.dead .im-art-pick { color: var(--text-muted); } .im-art-none { display: grid; place-items: center; height: 100%; color: var(--text-muted); font-size: 12px; text-align: center; padding: 12px; } .im-ev { display: flex; flex-wrap: wrap; gap: 7px; } diff --git a/src/components/ImportWindow.svelte b/src/components/ImportWindow.svelte index 90f0ce7..3e666cc 100644 --- a/src/components/ImportWindow.svelte +++ b/src/components/ImportWindow.svelte @@ -16,6 +16,7 @@ import ImportPicker from "./ImportPicker.svelte"; import { evidence as matchEvidence } from "../lib/import-evidence.js"; import { foldReleases } from "../lib/import-fold.js"; + import { majority, albumNameOf, artistNameOf, IDENTIFY_ENGINE } from "../lib/import-title.js"; import { findAlbumCopies, startAudition } from "../lib/import-audio.js"; const STEPS = [ @@ -83,14 +84,17 @@ // would dress a transfer that is not running as one that is — // reading either "53% of track 1" or, once the stall clock caught // up, "the peer went quiet" about a peer nobody is talking to. + // `switching` goes too: it is a re-entry guard for one in-flight + // cancel-and-retry. Persisted true, it would come back as a record + // that can never switch peers again. const auditionsToSave = Object.fromEntries( Object.entries(auditions).map(([rel, au]) => { - const { xfer, ...rest } = au ?? {}; + const { xfer, switching, ...rest } = au ?? {}; return [rel, rest]; })); await api.importSessionSet(JSON.stringify({ step, root, scan, layout: layoutChoice, choices, found: done, pick, pickRel, - bucket, auditions: auditionsToSave, keepSeparate, + bucket, bucketRel, auditions: auditionsToSave, keepSeparate, })); } catch (e) { console.warn("import: session save failed", e); @@ -112,6 +116,7 @@ pick = v.pick ?? 0; pickRel = v.pickRel ?? null; bucket = v.bucket ?? "all"; + bucketRel = v.bucketRel ?? {}; // Auditions survive by their files: ids are stale after a // restart, but a cached track 1 (or full album) on disk is // still every bit as comparable and keepable. @@ -405,22 +410,19 @@ albums .filter((a) => !a.junk) .map((a) => { - const tagArtist = majority(a.tracks.map((t) => t.artist)); const tagYear = majority(a.tracks.map((t) => t.year)); // The album NAME comes from the tags when they agree on one — // the folder is the ripper's naming ("Heartleap - WEB V0"), // the tags are the record's own claim ("Heartleap"), and the // row title is what the catalog joins on. Folder as fallback. + // One definition, in import-title.js, so the picker asks the + // provider the same question apply writes (they disagreed: the + // picker searched the folder and found nothing for records + // whose tags name them perfectly well). return { rel: a.rel, - artist: a.depth >= 2 ? a.artistFolder : (tagArtist ?? a.artistFolder), - // A folded release takes its FOLDER's name. The tags on a - // folded record describe the parts ("Analord 01"…"Analord - // 11"), so a tag majority would title the whole set after - // whichever volume happened to carry the most tracks. - album: a.parts - ? a.title - : majority(a.tracks.map((t) => t.album)) ?? a.title, + artist: artistNameOf(a), + album: albumNameOf(a), year: a.year ?? tagYear ?? null, // Where the audio actually is, relative to `rel`. Empty for // an ordinary record; apply reads its own folder then. @@ -428,13 +430,6 @@ }; })); - function majority(values) { - const tally = new Map(); - for (const v of values) if (v != null && v !== "") tally.set(v, (tally.get(v) ?? 0) + 1); - let best = null, n = 0; - for (const [v, c] of tally) if (c > n) { best = v; n = c; } - return best; - } // ---- the upgrades picker's queue ------------------------------------- // Artist A–Z, then chronological within the artist — the shelf order, @@ -465,6 +460,10 @@ let bucket = $state("all"); const bucketByRel = $derived.by(() => { const artistOf = new Map(plan.map((p) => [p.rel, p.artist])); + // Judged against the name the provider was ASKED about, so the bucket + // and the picker's verdict panel agree — and so a record whose folder + // is a rip descriptor is not filed as merely "close" to its own match. + const albumOf = new Map(plan.map((p) => [p.rel, p.album])); const m = new Map(); for (const x of pickerBase) { const f = found[x.rel]; @@ -475,7 +474,7 @@ k = matchEvidence( f.result.candidate, artistOf.get(x.rel) ?? x.artistFolder, - x.title, + albumOf.get(x.rel) ?? x.title, x.tracks.length, ).verdict; m.set(x.rel, k); @@ -568,9 +567,58 @@ schedulePersist(); } + // A cached verdict is an answer to the question that was ASKED. The picker + // used to ask about the folder, so every "nothing found" in a resumed + // session answers a question we no longer ask — and because the cache is + // never revisited (`if (found[rel]) return`), those records would stay + // unmatched forever behind a stale no. + // + // Drop exactly the ones whose question changed, and only when the answer + // was NOTHING: a real match is left alone, so this cannot cost a good + // answer, and re-asking is deferred until the record is actually opened. + let sweptStale = false; + $effect(() => { + if (sweptStale || !albums.length) return; + const rels = Object.keys(found); + if (!rels.length) return; + sweptStale = true; + const byRel = new Map(albums.map((a) => [a.rel, a])); + let dropped = 0; + for (const rel of rels) { + const f = found[rel]; + if (f?.result?.candidate) continue; // a real answer stands + // Looked with an older engine: the question may be unchanged but + // the answer is not, so it gets asked again. + if ((f?.engine ?? 1) < IDENTIFY_ENGINE) { + delete found[rel]; + dropped++; + continue; + } + const rec = byRel.get(rel); + if (!rec) continue; + const asked = f?.result?.tries?.[0] ?? null; + const asksNow = albumNameOf(rec); + if (asked != null && asksNow != null && asked !== asksNow) { + delete found[rel]; + dropped++; + } + } + if (dropped) { + console.info(`import: re-asking ${dropped} record(s) — the query changed`); + schedulePersist(); + } + }); + function pickerArtist(x) { return plan.find((p) => p.rel === x.rel)?.artist ?? x.artistFolder; } + /** The name the picker asks the PROVIDERS about. The artist already came + * from the plan rather than the folder; the album did not, which is why a + * record whose tags say "Mirrored" was searched for as + * "[WARP CD 156] Battles - Mirrored - 2007" and matched nothing. */ + function pickerAlbum(x) { + return plan.find((p) => p.rel === x.rel)?.album ?? x.title; + } let persistTimer = 0; function schedulePersist() { clearTimeout(persistTimer); @@ -593,46 +641,142 @@ * audition download (whole album into the cache pen, track 1 first). * Identification never gates this — deep cuts get their find too; a * TRUSTED provider match just strengthens the query ladder. */ + /** How many ranked runners-up to keep, so a bad peer costs a retry rather + * than a whole new search. */ + const AUDITION_ALTS = 4; + /** How long track 1 may make NO progress before we stop waiting on this + * peer. A queue place that never moves reads exactly like a live transfer + * otherwise — there was nothing that ever gave up, so a peer with 277 + * people ahead held the card indefinitely. */ + const AUDITION_WAIT_MS = 45000; + + /** Only what the card and a retry need — the raw search result carries the + * whole response object. */ + const slimCandidate = (c) => ({ + username: c.username, + folderLeaf: c.folderLeaf, + trackCount: c.trackCount, + avgBitrate: c.avgBitrate, + lossless: c.lossless, + bytes: c.bytes, + countMatch: c.countMatch, + queue: c.queue ?? null, + hasFreeSlot: !!c.hasFreeSlot, + files: (c.files ?? []).map((f) => ({ path: f.path, name: f.name, size: f.size ?? 0 })), + }); + + /** Begin (or re-begin) an audition on `cand`, keeping `alts` as fallbacks. + * TRACK 1 ONLY — the rest is requested once it lands. */ + async function beginAudition(rel, cand, alts, { attempt = 1, dead = [], switchedFrom = null, reason = null } = {}) { + const { ids } = await startAudition(rel, cand, { from: 0, count: 1 }); + auditions[rel] = { + status: "downloading", + candidate: cand, + alts, + attempt, + switchedFrom, + reason, + // The album's real length. `ids` only holds track 1 until the rest + // are asked for, so it cannot be what "all of it arrived" counts. + expected: cand.trackCount, + ids, + dead, + arrived: 0, + track1Path: null, + probe: null, + startedAt: Date.now(), + }; + schedulePersist(); + } + + /** The remaining tracks, once track 1 has proved the peer will actually + * serve us. */ + async function startRest(rel) { + const au = auditions[rel]; + if (!au || au.restStarted || !au.candidate?.files?.length) return; + au.restStarted = true; + if (au.candidate.files.length <= 1) return; + try { + const { ids } = await startAudition(rel, au.candidate, { from: 1 }); + const cur = auditions[rel]; + if (cur && cur.candidate?.username === au.candidate.username) { + cur.ids = [...(cur.ids ?? []), ...ids]; + schedulePersist(); + } + } catch (e) { + console.warn("import: rest of album failed to start", e); + } + } + + /** Stop waiting on this peer and drop to the next ranked copy. + * + * Cancels the transfers first — without that, a switch left the old peer + * trickling in the background forever. Their ids go on a `dead` list so a + * late event from a cancelled transfer cannot be credited to the new one. */ + async function switchPeer(rel, reason) { + const au = auditions[rel]; + if (!au || au.switching || au.status === "searching") return; + au.switching = true; + const dead = [...(au.dead ?? []), ...(au.ids ?? [])]; + for (const id of au.ids ?? []) { + try { await api.cancelDownload(id); } catch {} + } + const [next, ...rest] = au.alts ?? []; + if (!next) { + auditions[rel] = { status: "none", dead, exhausted: true, reason }; + schedulePersist(); + return; + } + try { + await beginAudition(rel, next, rest, { + attempt: (au.attempt ?? 1) + 1, + dead, + switchedFrom: au.candidate?.username ?? null, + reason, + }); + } catch (e) { + console.warn("import: switching peer failed", e); + auditions[rel] = { status: "failed", error: String(e), dead }; + schedulePersist(); + } + } + async function findBetterCopy(rel) { const x = pickerBase.find((r) => r.rel === rel); if (!x || auditions[rel]?.status === "searching") return; - auditions[rel] = { status: "searching" }; + // A re-search from a live audition stops the old peer first; otherwise + // its transfers keep running with nothing watching them. + const prev = auditions[rel]; + const dead = [...(prev?.dead ?? []), ...(prev?.ids ?? [])]; + for (const id of prev?.ids ?? []) { + try { await api.cancelDownload(id); } catch {} + } + auditions[rel] = { status: "searching", dead }; try { const trusted = ["ok", "warn"].includes(bucketByRel.get(rel)); const list = await findAlbumCopies({ artist: pickerArtist(x), - title: x.title, + // The record's own name leads; its folder is the narrower + // fallback for when the tags are wrong or absent. + album: pickerAlbum(x), + folder: x.title, candidate: trusted ? found[rel]?.result?.candidate : null, wantTracks: x.tracks.length, }); if (!list.length) { - auditions[rel] = { status: "none" }; + auditions[rel] = { status: "none", dead }; schedulePersist(); return; } - const best = list[0]; - const { ids } = await startAudition(rel, best); - auditions[rel] = { - status: "downloading", - candidate: { - username: best.username, - folderLeaf: best.folderLeaf, - trackCount: best.trackCount, - avgBitrate: best.avgBitrate, - lossless: best.lossless, - bytes: best.bytes, - countMatch: best.countMatch, - files: best.files.map((f) => ({ path: f.path, name: f.name, size: f.size ?? 0 })), - }, - ids, - arrived: 0, - track1Path: null, - probe: null, - }; - schedulePersist(); + await beginAudition( + rel, + slimCandidate(list[0]), + list.slice(1, 1 + AUDITION_ALTS).map(slimCandidate), + { dead }, + ); } catch (e) { console.warn("import: find copy failed", e); - auditions[rel] = { status: "failed", error: String(e) }; + auditions[rel] = { status: "failed", error: String(e), dead }; } } @@ -662,19 +806,32 @@ const au = rel ? auditions[rel] : null; const idx = typeof p.idx === "number" ? p.idx : (au?.ids?.indexOf(p.id) ?? -1); if (!au || idx < 0) return; + // A peer we walked away from can still emit — the cancel and its last + // event race. Crediting those to the copy that replaced it would + // inflate its count and, for a Complete, fake a track it never sent. + if (p.id != null && (au.dead ?? []).includes(p.id)) return; if (p.done) { au.arrived = (au.arrived ?? 0) + 1; if (idx === 0) { au.track1Path = p.localPath; au.status = "ready"; probeAudition(rel, p.localPath); + // The peer has proved it will serve us; ask for the rest now. + startRest(rel); } - if (au.arrived >= (au.ids?.length ?? 0)) au.status = "complete"; + if (au.arrived >= (au.expected ?? au.ids?.length ?? 0)) au.status = "complete"; au.xfer = { ...(au.xfer ?? {}), phase: "transferring", at: Date.now(), bytesAt: Date.now() }; schedulePersist(); } else if (p.failed && idx === 0 && !au.track1Path) { - au.status = "failed"; + // A refusal is an answer, and the answer is "not this peer". Giving + // up here wasted the four ranked copies already in hand — the same + // dead end as waiting forever, reached faster. au.error = p.failed; + if ((au.alts?.length ?? 0) && !au.switching) { + switchPeer(rel, `the peer could not send it — ${p.failed}`); + } else { + au.status = "failed"; + } } else if (!p.failed) { // Progress. Before track 1 lands the row speaks for track 1 // specifically — that is the file the comparison needs — and @@ -711,6 +868,24 @@ const t = setInterval(() => (now = Date.now()), 1000); return () => clearInterval(t); }); + + // Nothing used to give up. A peer that accepts the request and then parks + // you at #277 looks identical to one about to send — the row said "queued + // at the peer · nothing to do but wait", and meant it literally. + // + // The clock is on PROGRESS, not on the attempt: a copy that is actually + // transferring keeps its slot however long it takes, and only silence is + // punished. `bytesAt` already tracks the last byte that really landed. + $effect(() => { + const t = now; + for (const [rel, au] of Object.entries(auditions)) { + if (!au || au.status !== "downloading" || au.track1Path || au.switching) continue; + const since = au.xfer?.bytesAt ?? au.startedAt ?? t; + if (t - since > AUDITION_WAIT_MS) { + switchPeer(rel, `no progress for ${Math.round(AUDITION_WAIT_MS / 1000)}s`); + } + } + }); /** No bytes for this long, while actually transferring, is a stall. A * QUEUED transfer is not stalled — it is behaving exactly as told. */ const STALL_MS = 30_000; @@ -775,10 +950,24 @@ * record would otherwise seek to where it already is and look like a * dead button. Only with nothing left undecided does Review get it. */ function advanceCursor() { - if (pick + 1 < pickerQueue.length) seekIndex(pick + 1); + // Where "next" is depends on whether the record you just dealt with is + // still IN the lens. A filtered queue re-sorts itself under you: in + // "Not looked yet", merely opening a record identifies it, so it leaves + // the bucket while you are looking at it and every later index shifts + // down by one. `pick` does not follow (the cursor effect only tracks a + // record the queue still contains), so `pick + 1` stepped over the + // record that had just slid into the vacated slot — one skipped for + // every record reviewed under that filter. + const at = pickerQueue.findIndex((x) => x.rel === curPick?.rel); + const next = at >= 0 ? at + 1 : pick; + if (next < pickerQueue.length) seekIndex(next); else if (!widenPastBucket()) { + // Exclude the record just dealt with by its position in THIS queue. + // `pick` is not it once the record has left the lens, and excluding + // the wrong index is how a wrap could hand back the very record it + // meant to skip past. const back = pickerQueue.findIndex( - (x, i) => i !== pick && !choices[x.rel]?.seen); + (x, i) => i !== at && !choices[x.rel]?.seen); if (back >= 0) seekIndex(back); else step = "review"; } @@ -799,14 +988,36 @@ if (pick <= 0) step = "review"; else seekIndex(pick - 1); } - /** Changing the lens keeps the record under the cursor when the new lens - * still shows it — the old `pick = 0` threw the position away on every - * bucket click, which is half of why the position was never worth - * saving. */ + /** Where each lens was last looking. A chip is a place you go, so it has to + * be a place you can come BACK to — otherwise browsing "Not looked yet" + * from the middle of a review costs you your position, which is exactly + * what the old `seekIndex` fallback did (it writes `pickRel`, the saved + * resume point, so the place was not merely lost on screen but persisted + * over). Remembering each lens separately means the chips can navigate — + * visibly do something — without any of them being able to lose a place. */ + let bucketRel = $state({}); + + /** Changing the lens takes you into it, and remembers where you were. + * + * Where you land, in order: + * 1. Where you last were IN THIS LENS. Tabs that keep their own place + * are the whole point — "All" has to be able to hand you back the + * record you were reviewing, and it can only do that from memory: + * "keep the current record if the lens shows it" can never lose for + * All, which contains everything, so it would win every time and the + * memory would be dead code. + * 2. The record already under the cursor, when this lens shows it and + * has no memory of its own — switching lenses on the record you are + * reading should not move you for no reason. + * 3. The lens's first undecided record. */ function setBucket(k) { + if (k === bucket) return; const keep = curPick?.rel; + if (keep) bucketRel[bucket] = keep; bucket = k; - if (!(keep && seekTo(keep))) seekIndex(Math.max(0, nextUndecided(pickerQueue))); + if (bucketRel[k] && seekTo(bucketRel[k])) { schedulePersist(); return; } + if (keep && seekTo(keep)) { schedulePersist(); return; } + seekIndex(Math.max(0, nextUndecided(pickerQueue))); schedulePersist(); } @@ -1060,6 +1271,50 @@ } const stepIndex = $derived(STEPS.findIndex(([k]) => k === step)); + + // ---- start over ------------------------------------------------------ + // A session is sticky by design — it survives closing the window, and the + // resume machinery exists to put you back exactly where you were. That + // left no way to say "forget all this and scan something else": the only + // route was deleting import-session.json by hand. + let confirmReset = $state(false); + /** What the reset actually costs, counted rather than guessed at. */ + const resetCost = $derived.by(() => ({ + records: pickerBase.length, + decided: Object.values(choices).filter((c) => c?.seen).length, + cached: Object.values(auditions).filter( + (a) => a?.track1Path || a?.status === "complete" || a?.status === "ready").length, + })); + async function startOver() { + // Clears the session file AND the audition pen — every penned copy is + // one this run downloaded and did not keep, so the run ending is what + // makes it collectable. + try { + await api.importSessionClear(); + } catch (e) { + console.warn("import: could not clear the session", e); + error = String(e); + } + step = "source"; + root = null; + scan = null; + scanProg = null; + scanEta = null; + ledger = []; + choices = {}; + found = {}; + auditions = {}; + keepSeparate = []; + localArts = {}; + reports = {}; + pick = 0; + pickRel = null; + bucket = "all"; + bucketRel = {}; + report = null; + confirmReset = false; + updateResume(); + } @@ -1076,11 +1331,34 @@ {i < stepIndex ? "✓" : i + 1}{label}
{/each} + + {#if root || scan} + + {/if}
+ + {#if confirmReset} +
+ Start over? + This forgets the scan of {nfmt(resetCost.records)} record{resetCost.records === 1 ? "" : "s"}{#if resetCost.decided}, the + {nfmt(resetCost.decided)} decision{resetCost.decided === 1 ? "" : "s"} made so far{/if}{#if resetCost.cached}, and + {nfmt(resetCost.cached)} downloaded cop{resetCost.cached === 1 ? "y" : "ies"} waiting in the cache{/if}. + Your music is not touched — not the source folder, and not what is already in the library. +
+ + +
+
+ {/if}