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}