Various issues including cover issues with MPD (corrupted/incomplete files)
This commit is contained in:
146
src/db.rs
146
src/db.rs
@@ -747,6 +747,21 @@ pub fn reset_missing_cover_timestamps(conn: &Connection) -> Result<usize> {
|
||||
Ok(count)
|
||||
}
|
||||
|
||||
/// Reset `fetched_at` to NULL for cover-missing albums for one artist only.
|
||||
///
|
||||
/// Matching is case-insensitive (`LOWER(artist) = LOWER(?1)`), so callers can
|
||||
/// pass user input without worrying about exact casing.
|
||||
pub fn reset_missing_cover_timestamps_for_artist(conn: &Connection, artist: &str) -> Result<usize> {
|
||||
let count = conn.execute(
|
||||
"UPDATE album_cache
|
||||
SET fetched_at = NULL
|
||||
WHERE cover_url IS NULL
|
||||
AND LOWER(artist) = LOWER(?1)",
|
||||
params![artist],
|
||||
)?;
|
||||
Ok(count)
|
||||
}
|
||||
|
||||
pub fn uncached_albums(conn: &Connection) -> Result<Vec<UncachedAlbum>> {
|
||||
// Retry incomplete cache entries at most once per 7 days.
|
||||
let retry_before = (chrono::Local::now().naive_local() - chrono::Duration::days(7))
|
||||
@@ -776,6 +791,27 @@ pub fn uncached_albums(conn: &Connection) -> Result<Vec<UncachedAlbum>> {
|
||||
rows.collect()
|
||||
}
|
||||
|
||||
/// Return all distinct albums with scrobbles for a specific artist.
|
||||
///
|
||||
/// Matching is case-insensitive. Empty album names are excluded.
|
||||
pub fn albums_for_artist(conn: &Connection, artist: &str) -> Result<Vec<UncachedAlbum>> {
|
||||
let mut stmt = conn.prepare(
|
||||
"SELECT DISTINCT s.artist, s.album
|
||||
FROM scrobbles s
|
||||
WHERE s.album != ''
|
||||
AND LOWER(s.artist) = LOWER(?1)
|
||||
ORDER BY s.artist, s.album",
|
||||
)?;
|
||||
|
||||
let rows = stmt.query_map(params![artist], |row| {
|
||||
Ok(UncachedAlbum {
|
||||
artist: row.get(0)?,
|
||||
album: row.get(1)?,
|
||||
})
|
||||
})?;
|
||||
rows.collect()
|
||||
}
|
||||
|
||||
/// Returns true if `genre` should rank below more specific genres.
|
||||
///
|
||||
/// Single-word genres ("rock", "electronic", "ambient") are broad descriptors
|
||||
@@ -1847,4 +1883,114 @@ mod tests {
|
||||
"CAA cover should replace the local cover"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_albums_for_artist_case_insensitive() {
|
||||
let conn = open_memory_db().unwrap();
|
||||
|
||||
insert_scrobble(
|
||||
&conn,
|
||||
&NewScrobble {
|
||||
artist: "Pink Floyd".into(),
|
||||
album: "The Division Bell".into(),
|
||||
title: "Marooned".into(),
|
||||
track_duration_secs: Some(329),
|
||||
played_duration_secs: 200,
|
||||
scrobbled_at: today_at("10:00:00"),
|
||||
source: "MPRIS".into(),
|
||||
},
|
||||
)
|
||||
.unwrap();
|
||||
insert_scrobble(
|
||||
&conn,
|
||||
&NewScrobble {
|
||||
artist: "Pink Floyd".into(),
|
||||
album: "The Wall".into(),
|
||||
title: "Mother".into(),
|
||||
track_duration_secs: Some(345),
|
||||
played_duration_secs: 210,
|
||||
scrobbled_at: today_at("10:05:00"),
|
||||
source: "MPRIS".into(),
|
||||
},
|
||||
)
|
||||
.unwrap();
|
||||
insert_scrobble(
|
||||
&conn,
|
||||
&NewScrobble {
|
||||
artist: "Deftones".into(),
|
||||
album: "White Pony".into(),
|
||||
title: "Digital Bath".into(),
|
||||
track_duration_secs: Some(291),
|
||||
played_duration_secs: 200,
|
||||
scrobbled_at: today_at("10:10:00"),
|
||||
source: "MPRIS".into(),
|
||||
},
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let albums = albums_for_artist(&conn, "pInK fLoYd").unwrap();
|
||||
assert_eq!(albums.len(), 2);
|
||||
assert!(
|
||||
albums
|
||||
.iter()
|
||||
.any(|a| a.artist == "Pink Floyd" && a.album == "The Division Bell")
|
||||
);
|
||||
assert!(
|
||||
albums
|
||||
.iter()
|
||||
.any(|a| a.artist == "Pink Floyd" && a.album == "The Wall")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_reset_missing_cover_timestamps_for_artist_only() {
|
||||
let conn = open_memory_db().unwrap();
|
||||
|
||||
upsert_album_cache(
|
||||
&conn,
|
||||
&AlbumCacheEntry {
|
||||
artist: "Pink Floyd".into(),
|
||||
album: "The Division Bell".into(),
|
||||
musicbrainz_id: Some("mbid-1".into()),
|
||||
cover_url: None,
|
||||
genre: None,
|
||||
fetched_at: today_at("12:00:00"),
|
||||
},
|
||||
)
|
||||
.unwrap();
|
||||
upsert_album_cache(
|
||||
&conn,
|
||||
&AlbumCacheEntry {
|
||||
artist: "Deftones".into(),
|
||||
album: "White Pony".into(),
|
||||
musicbrainz_id: Some("mbid-2".into()),
|
||||
cover_url: None,
|
||||
genre: None,
|
||||
fetched_at: today_at("12:01:00"),
|
||||
},
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
let updated = reset_missing_cover_timestamps_for_artist(&conn, "pink floyd").unwrap();
|
||||
assert_eq!(updated, 1);
|
||||
|
||||
let pink_fetched: Option<String> = conn
|
||||
.query_row(
|
||||
"SELECT fetched_at FROM album_cache WHERE artist = 'Pink Floyd' AND album = 'The Division Bell'",
|
||||
[],
|
||||
|r| r.get(0),
|
||||
)
|
||||
.unwrap();
|
||||
assert!(pink_fetched.is_none());
|
||||
|
||||
let deftones_fetched: Option<String> = conn
|
||||
.query_row(
|
||||
"SELECT fetched_at FROM album_cache WHERE artist = 'Deftones' AND album = 'White Pony'",
|
||||
[],
|
||||
|r| r.get(0),
|
||||
)
|
||||
.unwrap();
|
||||
let expected = today_at("12:01:00");
|
||||
assert_eq!(deftones_fetched.as_deref(), Some(expected.as_str()));
|
||||
}
|
||||
}
|
||||
|
||||
203
src/enrich.rs
203
src/enrich.rs
@@ -716,13 +716,14 @@ fn download_cover_with_fallback(
|
||||
mbid: &str,
|
||||
release_group_id: Option<&str>,
|
||||
covers_dir: &Path,
|
||||
force_redownload: bool,
|
||||
) -> Option<String> {
|
||||
// Use the MBID as the filename regardless of which endpoint succeeds,
|
||||
// so we have a consistent cache key.
|
||||
let dest = covers_dir.join(format!("{}.jpg", mbid));
|
||||
|
||||
// Skip download if we already have the file from a previous run.
|
||||
if dest.exists() {
|
||||
// Reuse cached file unless force mode explicitly asks for a refresh.
|
||||
if should_reuse_existing_cover(dest.exists(), force_redownload) {
|
||||
return Some(dest.to_string_lossy().to_string());
|
||||
}
|
||||
|
||||
@@ -746,6 +747,11 @@ fn download_cover_with_fallback(
|
||||
None
|
||||
}
|
||||
|
||||
/// Decide whether an existing cover file should be reused as-is.
|
||||
fn should_reuse_existing_cover(dest_exists: bool, force_redownload: bool) -> bool {
|
||||
dest_exists && !force_redownload
|
||||
}
|
||||
|
||||
/// Decode image bytes, downscale if either dimension exceeds `MAX_COVER_DIM`,
|
||||
/// and re-encode as JPEG at `COVER_JPEG_QUALITY`.
|
||||
///
|
||||
@@ -794,6 +800,76 @@ fn itunes_cover_stem(artist: &str, album: &str) -> String {
|
||||
format!("itunes_{:016x}", hash)
|
||||
}
|
||||
|
||||
/// Build a relaxed normalised key for fuzzy text matching.
|
||||
///
|
||||
/// Lowercases and keeps only alphanumeric characters and spaces, then
|
||||
/// collapses whitespace. This makes matching resilient to punctuation,
|
||||
/// symbols, and repeated spacing differences.
|
||||
fn normalise_match_key(s: &str) -> String {
|
||||
let mapped: String = s
|
||||
.chars()
|
||||
.map(|c| {
|
||||
if c.is_alphanumeric() || c.is_whitespace() {
|
||||
c
|
||||
} else {
|
||||
' '
|
||||
}
|
||||
})
|
||||
.collect();
|
||||
normalise_spaces(&mapped.to_ascii_lowercase())
|
||||
}
|
||||
|
||||
/// Return true when two strings match exactly or one contains the other,
|
||||
/// after relaxed normalisation.
|
||||
fn loosely_matches(query: &str, candidate: &str) -> bool {
|
||||
let q = normalise_match_key(query);
|
||||
let c = normalise_match_key(candidate);
|
||||
if q.is_empty() || c.is_empty() {
|
||||
return false;
|
||||
}
|
||||
q == c || q.contains(&c) || c.contains(&q)
|
||||
}
|
||||
|
||||
/// Score an iTunes result for the requested artist/album.
|
||||
///
|
||||
/// The score prefers exact artist/album matches, but still accepts useful
|
||||
/// near-matches (for example "The Division Bell - Single" for album field).
|
||||
fn score_itunes_result(result: &serde_json::Value, artist: &str, album: &str) -> Option<i32> {
|
||||
let result_artist = result.get("artistName")?.as_str()?;
|
||||
let result_album = result.get("collectionName")?.as_str()?;
|
||||
let _ = result.get("artworkUrl100")?.as_str()?;
|
||||
|
||||
if !loosely_matches(artist, result_artist) || !loosely_matches(album, result_album) {
|
||||
return None;
|
||||
}
|
||||
|
||||
let artist_q = normalise_match_key(artist);
|
||||
let artist_r = normalise_match_key(result_artist);
|
||||
let album_q = normalise_match_key(album);
|
||||
let album_r = normalise_match_key(result_album);
|
||||
|
||||
let artist_score = if artist_q == artist_r { 4 } else { 2 };
|
||||
let album_score = if album_q == album_r { 4 } else { 2 };
|
||||
Some(artist_score + album_score)
|
||||
}
|
||||
|
||||
/// Pick the best-matching iTunes artwork URL for the requested artist/album.
|
||||
fn pick_best_itunes_cover_url(
|
||||
results: &[serde_json::Value],
|
||||
artist: &str,
|
||||
album: &str,
|
||||
) -> Option<String> {
|
||||
let best = results
|
||||
.iter()
|
||||
.filter_map(|r| score_itunes_result(r, artist, album).map(|score| (score, r)))
|
||||
.max_by_key(|(score, _)| *score)?;
|
||||
|
||||
best.1
|
||||
.get("artworkUrl100")?
|
||||
.as_str()
|
||||
.map(|s| s.replace("100x100bb", "600x600bb"))
|
||||
}
|
||||
|
||||
/// Search the iTunes Store for the front cover of an album.
|
||||
///
|
||||
/// Uses Apple's public search API — no authentication required. Searches
|
||||
@@ -818,13 +894,9 @@ fn fetch_itunes_cover_url(client: &Client, artist: &str, album: &str) -> Option<
|
||||
let json: serde_json::Value = response.json().ok()?;
|
||||
let results = json.get("results")?.as_array()?;
|
||||
|
||||
// Take the first result's artwork URL and bump the resolution.
|
||||
// Apple's URLs end in "100x100bb.jpg" — replace with "600x600bb".
|
||||
results.iter().find_map(|r| {
|
||||
r.get("artworkUrl100")?
|
||||
.as_str()
|
||||
.map(|s| s.replace("100x100bb", "600x600bb"))
|
||||
})
|
||||
// Prefer a result whose artist and album actually match the query, so
|
||||
// unrelated top hits don't produce obviously wrong covers.
|
||||
pick_best_itunes_cover_url(results, artist, album)
|
||||
}
|
||||
|
||||
/// Attempt to download an image from the given URL and save it to `dest`.
|
||||
@@ -889,6 +961,42 @@ pub fn run_enrich_targeted(
|
||||
quiet: bool,
|
||||
no_itunes: bool,
|
||||
) {
|
||||
run_enrich_targeted_with_options(conn, needed, quiet, no_itunes, false);
|
||||
}
|
||||
|
||||
/// Like [`run_enrich_targeted`], but supports force refresh for the provided
|
||||
/// subset.
|
||||
pub fn run_enrich_targeted_with_options(
|
||||
conn: &Connection,
|
||||
needed: &std::collections::HashSet<(String, String)>,
|
||||
quiet: bool,
|
||||
no_itunes: bool,
|
||||
force: bool,
|
||||
) {
|
||||
if needed.is_empty() {
|
||||
if !quiet {
|
||||
eprintln!("No matching albums to enrich.");
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
if force {
|
||||
let mut albums: Vec<db::UncachedAlbum> = needed
|
||||
.iter()
|
||||
.map(|(artist, album)| db::UncachedAlbum {
|
||||
artist: artist.clone(),
|
||||
album: album.clone(),
|
||||
})
|
||||
.collect();
|
||||
albums.sort_by(|a, b| a.artist.cmp(&b.artist).then_with(|| a.album.cmp(&b.album)));
|
||||
eprintln!(
|
||||
"Force mode enabled: refreshing {} selected album(s).",
|
||||
albums.len()
|
||||
);
|
||||
run_enrich_albums(conn, albums, quiet, no_itunes, true);
|
||||
return;
|
||||
}
|
||||
|
||||
let all_uncached = match db::uncached_albums(conn) {
|
||||
Ok(v) => v,
|
||||
Err(e) => {
|
||||
@@ -901,7 +1009,7 @@ pub fn run_enrich_targeted(
|
||||
.into_iter()
|
||||
.filter(|a| needed.contains(&(a.artist.clone(), a.album.clone())))
|
||||
.collect();
|
||||
run_enrich_albums(conn, albums, quiet, no_itunes);
|
||||
run_enrich_albums(conn, albums, quiet, no_itunes, false);
|
||||
}
|
||||
|
||||
/// Run the enrichment process: find all uncached albums and fetch their
|
||||
@@ -921,9 +1029,10 @@ pub fn run_enrich(conn: &Connection, force: bool, quiet: bool, no_itunes: bool)
|
||||
conn.execute("DELETE FROM album_cache", [])
|
||||
.expect("Failed to clear album_cache");
|
||||
eprintln!("Cleared album cache (force mode).");
|
||||
eprintln!("Force mode enabled: refreshing existing cover files when re-fetching.");
|
||||
}
|
||||
let albums = db::uncached_albums(conn).expect("Failed to query uncached albums");
|
||||
run_enrich_albums(conn, albums, quiet, no_itunes);
|
||||
run_enrich_albums(conn, albums, quiet, no_itunes, force);
|
||||
}
|
||||
|
||||
/// Inner enrichment loop shared by `run_enrich` and `run_enrich_targeted`.
|
||||
@@ -932,6 +1041,7 @@ fn run_enrich_albums(
|
||||
albums: Vec<db::UncachedAlbum>,
|
||||
quiet: bool,
|
||||
no_itunes: bool,
|
||||
force_redownload: bool,
|
||||
) {
|
||||
if albums.is_empty() {
|
||||
if !quiet {
|
||||
@@ -1047,6 +1157,7 @@ fn run_enrich_albums(
|
||||
primary_mbid,
|
||||
release_group_id.as_deref(),
|
||||
&covers,
|
||||
force_redownload,
|
||||
);
|
||||
|
||||
// If still nothing, try alternate candidate releases from the search.
|
||||
@@ -1151,8 +1262,13 @@ pub fn enrich_by_mbid(
|
||||
);
|
||||
return;
|
||||
}
|
||||
let result =
|
||||
download_cover_with_fallback(&client, mbid, release_group_id.as_deref(), &covers);
|
||||
let result = download_cover_with_fallback(
|
||||
&client,
|
||||
mbid,
|
||||
release_group_id.as_deref(),
|
||||
&covers,
|
||||
false,
|
||||
);
|
||||
if result.is_some() {
|
||||
eprintln!(" Cover downloaded.");
|
||||
} else {
|
||||
@@ -1254,6 +1370,67 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_pick_best_itunes_cover_url_prefers_matching_artist_album() {
|
||||
let results = vec![
|
||||
serde_json::json!({
|
||||
"artistName": "Amiteve",
|
||||
"collectionName": "The Division Bell - Single",
|
||||
"artworkUrl100": "https://example.com/amiteve/100x100bb.jpg"
|
||||
}),
|
||||
serde_json::json!({
|
||||
"artistName": "Pink Floyd",
|
||||
"collectionName": "The Division Bell",
|
||||
"artworkUrl100": "https://example.com/pinkfloyd/100x100bb.jpg"
|
||||
}),
|
||||
];
|
||||
|
||||
let chosen = pick_best_itunes_cover_url(&results, "Pink Floyd", "The Division Bell");
|
||||
assert_eq!(
|
||||
chosen.as_deref(),
|
||||
Some("https://example.com/pinkfloyd/600x600bb.jpg")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_pick_best_itunes_cover_url_rejects_wrong_artist() {
|
||||
let results = vec![serde_json::json!({
|
||||
"artistName": "Amiteve",
|
||||
"collectionName": "The Division Bell - Single",
|
||||
"artworkUrl100": "https://example.com/amiteve/100x100bb.jpg"
|
||||
})];
|
||||
|
||||
let chosen = pick_best_itunes_cover_url(&results, "Pink Floyd", "The Division Bell");
|
||||
assert!(chosen.is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_pick_best_itunes_cover_url_accepts_album_suffix_variant() {
|
||||
let results = vec![serde_json::json!({
|
||||
"artistName": "Pink Floyd",
|
||||
"collectionName": "The Division Bell - Single",
|
||||
"artworkUrl100": "https://example.com/pinkfloyd-single/100x100bb.jpg"
|
||||
})];
|
||||
|
||||
let chosen = pick_best_itunes_cover_url(&results, "Pink Floyd", "The Division Bell");
|
||||
assert_eq!(
|
||||
chosen.as_deref(),
|
||||
Some("https://example.com/pinkfloyd-single/600x600bb.jpg")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_should_reuse_existing_cover_default_mode() {
|
||||
assert!(should_reuse_existing_cover(true, false));
|
||||
assert!(!should_reuse_existing_cover(false, false));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_should_reuse_existing_cover_force_mode() {
|
||||
assert!(!should_reuse_existing_cover(true, true));
|
||||
assert!(!should_reuse_existing_cover(false, true));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_pick_genre_string_prefers_genres_then_tags() {
|
||||
let genre = pick_genre_string(
|
||||
|
||||
52
src/main.rs
52
src/main.rs
@@ -189,6 +189,14 @@ enum Commands {
|
||||
#[arg(long)]
|
||||
retry_covers: bool,
|
||||
|
||||
/// Limit enrichment to albums by this artist (case-insensitive match).
|
||||
///
|
||||
/// Useful when fixing one artist's covers/metadata without touching
|
||||
/// the rest of the library. With `--force`, only this artist's albums
|
||||
/// are re-fetched.
|
||||
#[arg(long)]
|
||||
artist: Option<String>,
|
||||
|
||||
/// Skip the iTunes Search API when fetching cover art. Falls back
|
||||
/// directly to the Cover Art Archive (CAA). Useful if you prefer
|
||||
/// open-data sources or iTunes results are poor for your library.
|
||||
@@ -719,6 +727,7 @@ fn main() {
|
||||
online,
|
||||
force,
|
||||
retry_covers,
|
||||
artist,
|
||||
no_itunes,
|
||||
no_mpd_covers,
|
||||
mpd_host,
|
||||
@@ -745,6 +754,31 @@ fn main() {
|
||||
}
|
||||
};
|
||||
|
||||
let artist_needed = if let Some(ref artist_name) = artist {
|
||||
match db::albums_for_artist(&conn, artist_name) {
|
||||
Ok(v) => {
|
||||
if v.is_empty() {
|
||||
eprintln!("No scrobbled albums found for artist \"{}\".", artist_name);
|
||||
return;
|
||||
}
|
||||
Some(
|
||||
v.into_iter()
|
||||
.map(|a| (a.artist, a.album))
|
||||
.collect::<std::collections::HashSet<_>>(),
|
||||
)
|
||||
}
|
||||
Err(e) => {
|
||||
eprintln!(
|
||||
"[error] Failed to query albums for artist \"{}\": {}",
|
||||
artist_name, e
|
||||
);
|
||||
std::process::exit(1);
|
||||
}
|
||||
}
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
// Run MPD cover extraction first (fast, local, no rate limits).
|
||||
// Pre-populating cover_url means the online enrichment that follows
|
||||
// will skip fetching covers for albums that already have one.
|
||||
@@ -753,24 +787,40 @@ fn main() {
|
||||
host: mpd_host,
|
||||
port: mpd_port,
|
||||
};
|
||||
if let Some(ref needed) = artist_needed {
|
||||
mpd::run_mpd_cover_enrich_targeted(&mpd_cfg, &conn, needed);
|
||||
} else {
|
||||
mpd::run_mpd_cover_enrich(&mpd_cfg, &conn);
|
||||
}
|
||||
}
|
||||
|
||||
// Online enrichment: MusicBrainz lookup for MBID + genre, Cover Art
|
||||
// Archive for any albums still missing a cover after the MPD pass.
|
||||
// Reset the cooldown for cover-missing albums before the online
|
||||
// pass so they are picked up by uncached_albums.
|
||||
if retry_covers {
|
||||
match db::reset_missing_cover_timestamps(&conn) {
|
||||
let reset = if let Some(ref artist_name) = artist {
|
||||
db::reset_missing_cover_timestamps_for_artist(&conn, artist_name)
|
||||
} else {
|
||||
db::reset_missing_cover_timestamps(&conn)
|
||||
};
|
||||
|
||||
match reset {
|
||||
Ok(n) => eprintln!("Reset cooldown for {} album(s) missing covers.", n),
|
||||
Err(e) => eprintln!("[warn] Failed to reset cover timestamps: {}", e),
|
||||
}
|
||||
}
|
||||
|
||||
if online {
|
||||
if let Some(ref needed) = artist_needed {
|
||||
enrich::run_enrich_targeted_with_options(
|
||||
&conn, needed, false, no_itunes, force,
|
||||
);
|
||||
} else {
|
||||
enrich::run_enrich(&conn, force, false, no_itunes);
|
||||
}
|
||||
}
|
||||
}
|
||||
Commands::LastScrobble { db_path } => {
|
||||
let path = db_path.unwrap_or_else(default_db_path);
|
||||
let conn = match db::open_db(&path) {
|
||||
|
||||
94
src/mpd.rs
94
src/mpd.rs
@@ -311,10 +311,34 @@ fn read_exact_bytes(conn: &mut MpdConn, n: u64) -> Option<Vec<u8>> {
|
||||
Some(buf)
|
||||
}
|
||||
|
||||
/// Read the single `OK\n` line that terminates a binary chunk response.
|
||||
fn read_ok_line(conn: &mut MpdConn) {
|
||||
/// Consume the terminator after a binary chunk (`\nOK\n`).
|
||||
///
|
||||
/// MPD's binary responses place a newline byte immediately after the raw
|
||||
/// chunk payload, then an `OK` line. That means the first `read_line` after
|
||||
/// `read_exact(binary_len)` often returns an empty line, not `OK`.
|
||||
///
|
||||
/// Returns `Some(())` only when an `OK` line is observed; returns `None` on
|
||||
/// EOF, protocol errors, or I/O errors.
|
||||
fn read_chunk_terminator(conn: &mut MpdConn) -> Option<()> {
|
||||
loop {
|
||||
let mut line = String::new();
|
||||
let _ = conn.reader.read_line(&mut line);
|
||||
conn.reader.read_line(&mut line).ok()?;
|
||||
|
||||
let line = line.trim_end_matches('\n').trim_end_matches('\r');
|
||||
if line.is_empty() {
|
||||
// Expected: this is the newline that follows the raw binary bytes.
|
||||
continue;
|
||||
}
|
||||
if line == "OK" {
|
||||
return Some(());
|
||||
}
|
||||
if line.starts_with("ACK") {
|
||||
return None;
|
||||
}
|
||||
|
||||
// Any other non-empty line here means the stream got out of sync.
|
||||
return None;
|
||||
}
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -458,6 +482,7 @@ fn escape_mpd_string(s: &str) -> String {
|
||||
fn read_picture(conn: &mut MpdConn, file_uri: &str) -> Option<Vec<u8>> {
|
||||
let mut all_bytes: Vec<u8> = Vec::new();
|
||||
let mut offset: u64 = 0;
|
||||
let mut expected_total: Option<u64> = None;
|
||||
|
||||
loop {
|
||||
// Request the next chunk starting at `offset`.
|
||||
@@ -467,6 +492,14 @@ fn read_picture(conn: &mut MpdConn, file_uri: &str) -> Option<Vec<u8>> {
|
||||
// Parse the `size`, `type`, and `binary` header fields.
|
||||
let (total_size, chunk_size) = read_picture_header(conn)?;
|
||||
|
||||
if total_size > 0 {
|
||||
match expected_total {
|
||||
None => expected_total = Some(total_size),
|
||||
Some(prev) if prev != total_size => return None,
|
||||
Some(_) => {}
|
||||
}
|
||||
}
|
||||
|
||||
// `binary: 0` means no embedded picture (or we've read everything).
|
||||
if chunk_size == 0 {
|
||||
// If we have already accumulated bytes, we're done; otherwise there
|
||||
@@ -479,7 +512,7 @@ fn read_picture(conn: &mut MpdConn, file_uri: &str) -> Option<Vec<u8>> {
|
||||
all_bytes.extend_from_slice(&chunk);
|
||||
|
||||
// Each chunk response is terminated by a bare `OK` line.
|
||||
read_ok_line(conn);
|
||||
read_chunk_terminator(conn)?;
|
||||
|
||||
offset += chunk_size;
|
||||
|
||||
@@ -489,6 +522,13 @@ fn read_picture(conn: &mut MpdConn, file_uri: &str) -> Option<Vec<u8>> {
|
||||
}
|
||||
}
|
||||
|
||||
// If MPD declared a total byte size, ensure we received all of it.
|
||||
if let Some(total) = expected_total
|
||||
&& all_bytes.len() as u64 != total
|
||||
{
|
||||
return None;
|
||||
}
|
||||
|
||||
if all_bytes.is_empty() {
|
||||
None
|
||||
} else {
|
||||
@@ -1101,8 +1141,28 @@ fn run_mpd_cover_enrich_albums(
|
||||
mod tests {
|
||||
use super::*;
|
||||
use std::cell::RefCell;
|
||||
use std::io::Cursor;
|
||||
use std::rc::Rc;
|
||||
|
||||
struct NullWriter;
|
||||
|
||||
impl Write for NullWriter {
|
||||
fn write(&mut self, buf: &[u8]) -> io::Result<usize> {
|
||||
Ok(buf.len())
|
||||
}
|
||||
|
||||
fn flush(&mut self) -> io::Result<()> {
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
fn fake_conn(input: &[u8]) -> MpdConn {
|
||||
MpdConn {
|
||||
reader: BufReader::new(Box::new(Cursor::new(input.to_vec()))),
|
||||
writer: Box::new(NullWriter),
|
||||
}
|
||||
}
|
||||
|
||||
fn state(
|
||||
file: &str,
|
||||
artist: &str,
|
||||
@@ -1215,6 +1275,32 @@ mod tests {
|
||||
assert_eq!(parse_mpd_state("unknown"), PlayerStatus::Stopped);
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// readpicture protocol handling
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
#[test]
|
||||
fn test_read_chunk_terminator_consumes_blank_then_ok() {
|
||||
let mut conn = fake_conn(b"\nOK\n");
|
||||
assert!(read_chunk_terminator(&mut conn).is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_read_picture_reads_multiple_chunks_fully() {
|
||||
// Two chunks: "hello" + " world!" = 12 bytes total.
|
||||
let mut conn =
|
||||
fake_conn(b"size: 12\nbinary: 5\nhello\nOK\nsize: 12\nbinary: 7\n world!\nOK\n");
|
||||
let bytes = read_picture(&mut conn, "music/test.flac").unwrap();
|
||||
assert_eq!(bytes, b"hello world!");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_read_picture_rejects_truncated_transfer() {
|
||||
// MPD says total is 12, but only 5 bytes arrive before binary: 0.
|
||||
let mut conn = fake_conn(b"size: 12\nbinary: 5\nhello\nOK\nsize: 12\nbinary: 0\nOK\n");
|
||||
assert!(read_picture(&mut conn, "music/test.flac").is_none());
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// dispatch_events
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
@@ -127,6 +127,9 @@ impl CurrentTrack {
|
||||
/// track, accumulated across multiple play/pause cycles
|
||||
pub struct ScrobbleTracker<F: FnMut(NewScrobble)> {
|
||||
current_track: Option<CurrentTrack>,
|
||||
/// Local timestamp captured when tracking for the current track starts.
|
||||
/// Used for report period attribution instead of evaluation time.
|
||||
track_started_at: Option<chrono::NaiveDateTime>,
|
||||
is_playing: bool,
|
||||
has_seen_status: bool,
|
||||
playing_since: Option<Instant>,
|
||||
@@ -140,6 +143,7 @@ impl<F: FnMut(NewScrobble)> ScrobbleTracker<F> {
|
||||
pub fn new(scrobble_fn: F, source: String) -> Self {
|
||||
Self {
|
||||
current_track: None,
|
||||
track_started_at: None,
|
||||
is_playing: false,
|
||||
has_seen_status: false,
|
||||
playing_since: None,
|
||||
@@ -205,6 +209,7 @@ impl<F: FnMut(NewScrobble)> ScrobbleTracker<F> {
|
||||
title,
|
||||
duration_us,
|
||||
});
|
||||
self.track_started_at = Some(chrono::Local::now().naive_local());
|
||||
self.accumulated_secs = 0.0;
|
||||
|
||||
// Do not blindly assume metadata implies Playing. If we've
|
||||
@@ -284,8 +289,10 @@ impl<F: FnMut(NewScrobble)> ScrobbleTracker<F> {
|
||||
|
||||
// Only scrobble if the user listened for at least the threshold duration.
|
||||
if self.accumulated_secs >= threshold {
|
||||
let now = chrono::Local::now()
|
||||
.naive_local()
|
||||
let attribution_time = self
|
||||
.track_started_at
|
||||
.take()
|
||||
.unwrap_or_else(|| chrono::Local::now().naive_local())
|
||||
.format("%Y-%m-%dT%H:%M:%S")
|
||||
.to_string();
|
||||
|
||||
@@ -298,11 +305,13 @@ impl<F: FnMut(NewScrobble)> ScrobbleTracker<F> {
|
||||
title: track.title,
|
||||
track_duration_secs: track_dur,
|
||||
played_duration_secs: self.accumulated_secs.round() as i64,
|
||||
scrobbled_at: now,
|
||||
scrobbled_at: attribution_time,
|
||||
source: self.source.clone(),
|
||||
};
|
||||
(self.scrobble_fn)(scrobble);
|
||||
}
|
||||
|
||||
self.track_started_at = None;
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -446,6 +455,8 @@ pub fn create_db_tracker(
|
||||
#[cfg(test)]
|
||||
pub struct TestableTracker {
|
||||
current_track: Option<CurrentTrack>,
|
||||
/// Simulated timestamp for when current track tracking started.
|
||||
track_started_at_secs: Option<f64>,
|
||||
is_playing: bool,
|
||||
has_seen_status: bool,
|
||||
/// Simulated timestamp (in seconds) when the current Playing stretch began.
|
||||
@@ -465,6 +476,7 @@ impl TestableTracker {
|
||||
pub fn new() -> Self {
|
||||
Self {
|
||||
current_track: None,
|
||||
track_started_at_secs: None,
|
||||
is_playing: false,
|
||||
has_seen_status: false,
|
||||
playing_since_secs: None,
|
||||
@@ -499,17 +511,20 @@ impl TestableTracker {
|
||||
let threshold = track.threshold_secs();
|
||||
if self.accumulated_secs >= threshold {
|
||||
let track_dur = track.duration_secs();
|
||||
let attribution_secs = self.track_started_at_secs.take().unwrap_or(self.clock_secs);
|
||||
let scrobble = NewScrobble {
|
||||
artist: track.artist,
|
||||
album: track.album,
|
||||
title: track.title,
|
||||
track_duration_secs: track_dur,
|
||||
played_duration_secs: self.accumulated_secs.round() as i64,
|
||||
scrobbled_at: format!("test-time-{}", self.clock_secs),
|
||||
scrobbled_at: format!("test-time-{}", attribution_secs),
|
||||
source: self.source.clone(),
|
||||
};
|
||||
self.scrobbled.push(scrobble);
|
||||
}
|
||||
|
||||
self.track_started_at_secs = None;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -541,6 +556,7 @@ impl TestableTracker {
|
||||
title,
|
||||
duration_us,
|
||||
});
|
||||
self.track_started_at_secs = Some(self.clock_secs);
|
||||
self.accumulated_secs = 0.0;
|
||||
if self.has_seen_status {
|
||||
self.playing_since_secs = if self.is_playing {
|
||||
@@ -1014,6 +1030,32 @@ mod tests {
|
||||
assert_eq!(tracker.scrobbled.len(), 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_scrobble_timestamp_uses_track_start_time() {
|
||||
// Attribute the scrobble to when tracking started, not when the
|
||||
// outgoing track was evaluated.
|
||||
let mut tracker = TestableTracker::new();
|
||||
|
||||
tracker.advance_time(1_000.0);
|
||||
tracker.handle_event(Event::Metadata {
|
||||
artist: "Burial".into(),
|
||||
album: "Untrue".into(),
|
||||
title: "Untrue".into(),
|
||||
duration_us: Some(60_000_000), // threshold = 30s
|
||||
});
|
||||
|
||||
tracker.advance_time(35.0);
|
||||
tracker.handle_event(Event::Metadata {
|
||||
artist: "Deftones".into(),
|
||||
album: "White Pony".into(),
|
||||
title: "Digital Bath".into(),
|
||||
duration_us: Some(291_000_000),
|
||||
});
|
||||
|
||||
assert_eq!(tracker.scrobbled.len(), 1);
|
||||
assert_eq!(tracker.scrobbled[0].scrobbled_at, "test-time-1000");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_stop_below_threshold_no_scrobble() {
|
||||
// Play a track for 3 seconds then stop — should NOT be scrobbled,
|
||||
|
||||
Reference in New Issue
Block a user