Compare commits

..

10 Commits

Author SHA1 Message Date
33d83618dc Fix correctness and robustness issues found in a full code review
Correctness:

- MPD command reads no longer abandon a response half-way. The socket's
  500 ms read timeout exists so the idle loop can poll the shutdown flag,
  but it applied to every read, so a slow reply left unread bytes that the
  next command parsed as its own. Add MpdConn::read_line/read_exact, which
  retry across that timeout up to a 15 s deadline, and a desynced flag that
  forces a reconnect when a read is genuinely abandoned. read_response now
  returns Option so a truncated reply is a failure rather than an empty
  result -- a timed-out currentsong used to look like "MPD is stopped".
- resolve_cover returned a covers/<name> path even when the file was
  missing, so a deleted image produced a broken <img> instead of the
  placeholder. Gate the return value on the same existence check as the
  copy list.
- enrich --force no longer runs DELETE FROM album_cache. That discarded
  pin-album entries, MPD-extracted cover paths, and any genre the re-fetch
  then failed to find. Select every album via all_scrobbled_albums and
  overwrite rows in place instead.
- Album grouping, cover fallback and source attribution disagreed: the
  dominant_source subquery and artist_cover matched on album name alone, so
  two records sharing a title cross-contaminated. Extract one
  album_group_key_expr and use it everywhere.
- Escape quotes and backslashes in MusicBrainz Lucene queries; a title
  containing a quote closed the term early and matched nothing.
- Size the HTML cover grid from the period's own limit, so all-time no
  longer shows fewer covers than the table beneath it.
- Fix a panic in the terminal report: top_genres ranks broad single-word
  genres last, so first() is not the busiest row, and a later genre with
  more plays overflowed the bar width. Scale to the true maximum and clamp
  the bar arithmetic.
- Bound readpicture transfers by size and chunk count so a bogus size:
  header cannot drive an unbounded allocation or an endless loop.

Robustness:

- Kill the playerctl children before joining their reader threads. The
  threads block until stdout closes, which only happened because an
  interactive Ctrl+C signals the whole process group; under a supervisor
  that signals one PID, shutdown hung and left orphans.
- Reject --limit and --all-time-limit below 1. SQLite reads a negative
  LIMIT as unlimited, silently dumping the whole library.
- Set a 5 s busy_timeout, so running report against a live watch does not
  fail immediately with "database is locked".
- Replace panics in default_db_path, covers_dir and the enrich queries with
  reported errors, and check PRAGMA table_info for the source column
  migration instead of discarding every ALTER TABLE error.

Cleanups:

- Collapse eight reset_*_timestamps functions into reset_retry_timestamps
  with a RetryScope enum.
- Share one FNV-1a helper (enrich::cover_stem) between the MPD and iTunes
  filename generators.
- Use params_from_iter to remove six duplicated query_map branches.
- Extract source_dot_cell and reuse db::split_genre_list in the report.
- Skip the genre lookup when only covers were requested, saving a request
  and a rate-limit sleep per album.
- Keep the cached genre in pin-album when MusicBrainz returns nothing and a
  --cover-url was supplied.
2026-07-25 23:19:21 +01:00
23ef54555e Filter domain-like garbage out of genres
MusicBrainz community tags sometimes carry review-site domain names
(e.g. "musikexpress.de", "rollingstone.de") that leaked into the genre
field. Add a shared looks_like_url predicate and apply it at ingestion
(is_genre_tag, pick_genre_string) and at report read-time
(split_genre_list, top_genre_labels) so both new and already-cached
garbage are dropped.
2026-06-29 17:21:11 +01:00
f6e1a6d4ea Format long listen times in days and center the pin dot 2026-06-09 20:35:40 +01:00
8dd316f6ae Restore source dots in dedicated report columns 2026-03-24 08:24:22 +00:00
4ccd310667 Improve report source column and mobile table layout 2026-03-23 23:08:31 +00:00
275449d6d7 Refactor enrich CLI into pipeline, fetch, and retry modes 2026-03-23 22:52:01 +00:00
02ff0b4c97 Fix incomplete MPD cover transfers, add cover revalidation, and allow retrying missing MPD genres 2026-03-23 22:33:45 +00:00
6e0432490a Some checks just to be safe + link to the project 2026-03-23 21:08:36 +00:00
d199abdc68 Cover repair for MPD 2026-03-23 19:37:48 +00:00
ed5682c887 Various issues including cover issues with MPD (corrupted/incomplete files) 2026-03-23 19:34:11 +00:00
7 changed files with 2803 additions and 461 deletions

View File

@@ -268,13 +268,20 @@ scrbblr report [OPTIONS]
--db-path <PATH> Path to the SQLite database
scrbblr enrich [OPTIONS]
--online Fetch metadata and covers from MusicBrainz / iTunes / CAA
--force Re-fetch all albums from MusicBrainz (implies --online)
--retry-covers Reset the 7-day cooldown for albums missing covers
--no-itunes Skip iTunes; fall back directly to Cover Art Archive
--no-mpd-covers Skip MPD embedded cover extraction
--mpd-host <HOST> MPD host for cover extraction [default: localhost]
--mpd-port <PORT> MPD port for cover extraction [default: 6600]
--pipeline <MODE> Enrichment stage: mpd, online, or both [default: both]
--fetch <WHAT> Online payload: all, covers, or genres [default: all]
--retry <MODE> Retry selector: none, covers, genres, all, mpd-genres [default: none]
--cover-source <SRC> Cover source policy: itunes-caa or caa [default: itunes-caa]
--force Re-fetch all albums from MusicBrainz (online pipeline only)
--artist <ARTIST> Limit enrichment to one artist (case-insensitive)
--mpd-host <HOST> MPD host for MPD pipeline [default: localhost]
--mpd-port <PORT> MPD port for MPD pipeline [default: 6600]
--db-path <PATH> Path to the SQLite database
scrbblr repair-mpd-covers [OPTIONS]
--artist <ARTIST> Limit repair to one artist (case-insensitive)
--mpd-host <HOST> MPD host for cover validation [default: localhost]
--mpd-port <PORT> MPD port for cover validation [default: 6600]
--db-path <PATH> Path to the SQLite database
scrbblr last-scrobble [OPTIONS]
@@ -290,28 +297,36 @@ scrbblr pin-album [OPTIONS]
### Enrichment (covers + genres)
Enrichment runs in two stages:
`enrich` has a clean 3-part model:
- `--pipeline` decides *where* to run enrichment (`mpd`, `online`, `both`)
- `--fetch` decides *what* to fetch online (`covers`, `genres`, `all`)
- `--retry` decides *which missing rows* should bypass cooldown before online lookup
Defaults are: `--pipeline both --fetch all --retry none`.
The two pipelines are:
**Stage 1 -- MPD embedded covers (offline, no network)**
By default, `enrich` connects to MPD and extracts embedded cover art from your music files using MPD's `readpicture` command. This is fast, works entirely offline, and only processes albums scrobbled via MPD that don't yet have a cover:
By default (or with `--pipeline mpd`), `enrich` connects to MPD and extracts embedded cover art from your music files using MPD's `readpicture` command. This is fast, works entirely offline, and only processes albums scrobbled via MPD that don't yet have a cover:
```bash
scrbblr enrich
scrbblr enrich --pipeline mpd
```
**Stage 2 -- Online lookup (MusicBrainz + iTunes + Cover Art Archive)**
Pass `--online` to also query MusicBrainz for album metadata (MBID, genres) and download covers for albums that still have none after the MPD pass:
Use the online pipeline to query MusicBrainz for metadata (MBID, genres) and fetch covers:
```bash
scrbblr enrich --online
scrbblr enrich --pipeline online --fetch all
```
Cover art is sourced from **iTunes first** (fast, no rate limiting), then the **Cover Art Archive** as fallback. Pass `--no-itunes` to skip iTunes and use only CAA:
Cover art is sourced from **iTunes first** (fast, no rate limiting), then the **Cover Art Archive** as fallback. To use only CAA:
```bash
scrbblr enrich --online --no-itunes
scrbblr enrich --pipeline online --fetch covers --cover-source caa
```
When MusicBrainz matching is tricky, enrichment retries with normalised album variants (strips parenthetical suffixes, progressively shortens trailing words), tries artist aliases for symbol-heavy names, and falls back to recording search before giving up.
@@ -323,18 +338,33 @@ Genre extraction order:
3. Release-group `genres`
4. Release-group `tags`
Automatic enrichment (triggered by `report --html`) uses a 7-day retry cooldown for incomplete cache entries (missing cover or genre) so it doesn't hammer MusicBrainz on every report run. Use `--retry-covers` to reset that cooldown and retry albums missing covers immediately:
Automatic enrichment (triggered by `report --html`) uses a 7-day retry cooldown for incomplete cache entries (missing cover or genre) so it doesn't hammer MusicBrainz on every report run.
Retry missing covers immediately:
```bash
scrbblr enrich --online --retry-covers
scrbblr enrich --pipeline online --retry covers
```
Use `--force` to re-fetch all albums from scratch (ignores the cooldown entirely):
Retry missing genres only for MPD-sourced albums:
```bash
scrbblr enrich --online --force
scrbblr enrich --pipeline online --fetch genres --retry mpd-genres
```
You can combine this with `--artist` to target one artist.
Use `--force` to re-fetch all albums from scratch (online pipeline only):
```bash
scrbblr enrich --pipeline online --fetch all --force
```
Note: `--force` revisits every scrobbled album and overwrites each cache row in
place; it does not empty the cache first. An album whose re-fetch turns up
nothing keeps the data it already had, so covers pinned with `pin-album` and
covers extracted from MPD survive a forced run.
Genre normalisation notes:
- Genre labels are passed through from MusicBrainz with light cleanup only.
@@ -348,6 +378,26 @@ Downloaded covers are stored in:
`~/.local/share/scrbblr/covers/`
#### Repairing suspect MPD covers (`repair-mpd-covers`)
If you already have cached MPD-local covers (`mpd_*.jpg`) and suspect some are
incomplete/corrupted, run:
```bash
scrbblr repair-mpd-covers
```
This command re-extracts embedded art from MPD for albums with MPD-local cover
paths in `album_cache`, applies the same resize/re-encode pipeline, and
compares bytes with the existing file. Missing or mismatched files are
automatically overwritten.
Target one artist if needed:
```bash
scrbblr repair-mpd-covers --artist "Deftones"
```
#### Manually pinning an album (`pin-album`)
Sometimes automatic search fails -- most commonly for classical recordings where

1137
src/db.rs

File diff suppressed because it is too large Load Diff

View File

@@ -13,8 +13,8 @@
//! 2. For each, searches MusicBrainz for matching releases (to get MBID and
//! genre/tag data — always done regardless of cover source).
//! 3. Tries iTunes Search API first for cover art (fast, no rate limit,
//! good commercial coverage). Pass `--no-itunes` to skip.
//! 4. If iTunes has no cover (or `--no-itunes`), falls back to the Cover Art
//! good commercial coverage). Can be switched to CAA-only mode.
//! 4. If iTunes has no cover (or iTunes is disabled), falls back to the Cover Art
//! Archive using the MBID from step 2:
//! a. The specific release endpoint
//! b. The release-group endpoint (covers any edition of the album)
@@ -57,6 +57,7 @@ use image::codecs::jpeg::JpegEncoder;
use reqwest::blocking::Client;
use rusqlite::Connection;
use serde::Deserialize;
use std::fmt::Write as _;
use std::fs;
use std::io::Write;
use std::path::{Path, PathBuf};
@@ -97,6 +98,52 @@ const MAX_COVER_DIM: u32 = 500;
/// JPEG quality used when re-encoding resized cover art (0100).
const COVER_JPEG_QUALITY: u8 = 85;
/// What the online enrichment stage should fetch.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum OnlineFetchMode {
/// Fetch genres and covers.
All,
/// Fetch covers only.
Covers,
/// Fetch genres only.
Genres,
}
impl OnlineFetchMode {
fn wants_covers(self) -> bool {
matches!(self, Self::All | Self::Covers)
}
fn wants_genres(self) -> bool {
matches!(self, Self::All | Self::Genres)
}
}
/// Cover source policy for the online stage.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum CoverSourceMode {
/// Try iTunes first, then fall back to Cover Art Archive.
ItunesThenCaa,
/// Skip iTunes and use only Cover Art Archive.
CaaOnly,
}
/// Options controlling online enrichment behaviour.
#[derive(Debug, Clone, Copy)]
pub struct OnlineEnrichOptions {
pub fetch_mode: OnlineFetchMode,
pub cover_source: CoverSourceMode,
}
impl Default for OnlineEnrichOptions {
fn default() -> Self {
Self {
fetch_mode: OnlineFetchMode::All,
cover_source: CoverSourceMode::ItunesThenCaa,
}
}
}
// ---------------------------------------------------------------------------
// MusicBrainz API response types (only the fields we need)
// ---------------------------------------------------------------------------
@@ -199,14 +246,23 @@ struct MbTag {
///
/// Default: `~/.local/share/scrbblr/covers/`
/// (respects `$XDG_DATA_HOME`)
pub fn covers_dir() -> PathBuf {
let data_dir = std::env::var("XDG_DATA_HOME").unwrap_or_else(|_| {
let home = std::env::var("HOME").expect("HOME not set");
format!("{}/.local/share", home)
});
///
/// Returns an error rather than panicking when neither `$XDG_DATA_HOME` nor
/// `$HOME` is set, or the directory cannot be created — cover caching is an
/// optional convenience and should not take down a `watch` session that is
/// otherwise scrobbling fine.
pub fn covers_dir() -> Result<PathBuf, String> {
let data_dir = match std::env::var("XDG_DATA_HOME") {
Ok(d) if !d.is_empty() => d,
_ => {
let home = std::env::var("HOME")
.map_err(|_| "neither XDG_DATA_HOME nor HOME is set".to_string())?;
format!("{}/.local/share", home)
}
};
let dir = PathBuf::from(format!("{}/scrbblr/covers", data_dir));
fs::create_dir_all(&dir).expect("Failed to create covers directory");
dir
fs::create_dir_all(&dir).map_err(|e| format!("could not create {}: {}", dir.display(), e))?;
Ok(dir)
}
// ---------------------------------------------------------------------------
@@ -242,7 +298,8 @@ fn search_releases(client: &Client, artist: &str, album: &str) -> Vec<String> {
}
let query = format!(
"artist:\"{}\" AND release:\"{}\"",
artist_variants[0], variant
lucene_escape(&artist_variants[0]),
lucene_escape(variant)
);
let results = run_mb_search(client, &query);
if !results.is_empty() {
@@ -261,7 +318,11 @@ fn search_releases(client: &Client, artist: &str, album: &str) -> Vec<String> {
eprintln!(" Retrying with alternate album name: \"{}\"...", variant);
}
thread::sleep(RATE_LIMIT_DELAY);
let query = format!("artist:\"{}\" AND release:\"{}\"", alt_artist, variant);
let query = format!(
"artist:\"{}\" AND release:\"{}\"",
lucene_escape(alt_artist),
lucene_escape(variant)
);
let results = run_mb_search_with_min_score(client, &query, 60);
if !results.is_empty() {
return results;
@@ -284,7 +345,8 @@ fn search_releases(client: &Client, artist: &str, album: &str) -> Vec<String> {
client,
&format!(
"artist:\"{}\" AND recording:\"{}\"",
artist_variants[0], recording_query
lucene_escape(&artist_variants[0]),
lucene_escape(&recording_query)
),
60,
);
@@ -301,7 +363,8 @@ fn search_releases(client: &Client, artist: &str, album: &str) -> Vec<String> {
client,
&format!(
"artist:\"{}\" AND recording:\"{}\"",
artist_variants[0], shortest
lucene_escape(&artist_variants[0]),
lucene_escape(shortest)
),
50,
);
@@ -326,7 +389,7 @@ fn search_releases(client: &Client, artist: &str, album: &str) -> Vec<String> {
eprintln!(" Retrying with title-only search (no artist constraint)...");
}
thread::sleep(RATE_LIMIT_DELAY);
let query = format!("release:\"{}\"", variant);
let query = format!("release:\"{}\"", lucene_escape(variant));
let results = run_mb_search_with_min_score(client, &query, 85);
if !results.is_empty() {
return results;
@@ -434,6 +497,24 @@ fn normalise_spaces(s: &str) -> String {
s.split_whitespace().collect::<Vec<_>>().join(" ")
}
/// Escape a value for use inside a quoted Lucene term in a MusicBrainz query.
///
/// Searches are built as `artist:"X" AND release:"Y"`. Without escaping, a
/// title containing a double quote closes the term early and the rest is
/// parsed as query syntax, so MusicBrainz rejects the query or silently
/// matches nothing — precisely on the titles most likely to need the retry
/// ladder. Inside a quoted phrase Lucene only treats `\` and `"` as special.
fn lucene_escape(s: &str) -> String {
let mut out = String::with_capacity(s.len() + 4);
for c in s.chars() {
if c == '\\' || c == '"' {
out.push('\\');
}
out.push(c);
}
out
}
/// Trim common punctuation/noise from both title ends.
fn trim_title_edges(s: &str) -> String {
s.trim_matches(|c: char| {
@@ -601,6 +682,27 @@ fn fetch_release_details(client: &Client, mbid: &str) -> (Option<String>, Option
(genre, rgid)
}
/// Fetch just the release-group ID for a release.
///
/// Used when the caller wants covers but not genres: the release-group is the
/// Cover Art Archive's "any edition" fallback key, so it is still needed, but
/// requesting it alone skips the follow-up genre lookup that
/// [`fetch_release_details`] may perform.
fn fetch_release_group_id(client: &Client, mbid: &str) -> Option<String> {
let url = format!(
"https://musicbrainz.org/ws/2/release/{}?inc=release-groups&fmt=json",
mbid
);
let response = client.get(&url).send().ok()?;
if !response.status().is_success() {
return None;
}
let details: MbReleaseDetails = response.json().ok()?;
details.release_group.map(|rg| rg.id)
}
/// Fetch release-group genres/tags and return a genre string if available.
fn fetch_release_group_genre(client: &Client, release_group_id: &str) -> Option<String> {
let url = format!(
@@ -673,11 +775,15 @@ const TAG_BLOCKLIST: &[&str] = &[
///
/// Rejects:
/// - Tags containing digits (years like "2022", encodings like "iso-8859-1")
/// - Domain-like tags (review-site names such as "musikexpress.de")
/// - Known non-genre technical / language tags
fn is_genre_tag(tag: &str) -> bool {
if tag.chars().any(|c| c.is_ascii_digit()) {
return false;
}
if db::looks_like_url(tag) {
return false;
}
let lower = tag.to_ascii_lowercase();
!TAG_BLOCKLIST.contains(&lower.as_str())
}
@@ -689,7 +795,11 @@ fn is_genre_tag(tag: &str) -> bool {
/// 2. Community `tags` (filtered to entries with at least one vote and
/// passing the genre-tag heuristic to strip technical metadata)
fn pick_genre_string(genres: Vec<MbGenre>, tags: Vec<MbTag>) -> Option<String> {
let mut names: Vec<String> = genres.into_iter().map(|g| g.name).collect();
let mut names: Vec<String> = genres
.into_iter()
.map(|g| g.name)
.filter(|n| !db::looks_like_url(n))
.collect();
if names.is_empty() {
names = tags
.into_iter()
@@ -716,13 +826,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 +857,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`.
///
@@ -776,12 +892,20 @@ pub fn resize_cover_bytes(bytes: &[u8]) -> Option<Vec<u8>> {
// iTunes Search API — cover art fallback
// ---------------------------------------------------------------------------
/// Generate a stable filename stem for a cover sourced from iTunes.
/// Generate a stable filename stem for a locally-cached cover.
///
/// Uses the same FNV-1a 64-bit hash approach as the MPD cover extractor,
/// keyed on `"artist\talbum"`, so the filename is deterministic across runs.
/// Prefixed with `"itunes_"` to distinguish from MBID-based filenames.
fn itunes_cover_stem(artist: &str, album: &str) -> String {
/// Uses a 64-bit FNV-1a hash of `"artist\talbum"`, formatted as a 16-char hex
/// string behind `prefix`. This gives a compact, deterministic filename that:
///
/// - Is stable across runs (same artist+album always maps to the same name).
/// - Is visually distinct from MBID-based filenames (MBIDs contain hyphens).
/// - Has negligible collision probability across a typical music library.
///
/// `prefix` names the source the cover came from, so the origin of a cached
/// file is obvious on disk and MPD-local covers can be found again for
/// revalidation. Example: `cover_stem("mpd", ..)` -> `mpd_a3f7c2b91e045d68`.
pub fn cover_stem(prefix: &str, artist: &str, album: &str) -> String {
// FNV-1a 64-bit hash constants.
const FNV_OFFSET: u64 = 14695981039346656037;
const FNV_PRIME: u64 = 1099511628211;
@@ -791,7 +915,77 @@ fn itunes_cover_stem(artist: &str, album: &str) -> String {
hash ^= byte as u64;
hash = hash.wrapping_mul(FNV_PRIME);
}
format!("itunes_{:016x}", hash)
format!("{}_{:016x}", prefix, 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.
@@ -818,13 +1012,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`.
@@ -861,14 +1051,17 @@ fn try_download(client: &Client, url: &str, dest: &Path) -> Option<String> {
/// Simple URL encoding for query parameters.
/// Encodes characters that are not unreserved per RFC 3986.
fn urlencoded(s: &str) -> String {
let mut result = String::with_capacity(s.len() * 2);
for c in s.chars() {
match c {
'A'..='Z' | 'a'..='z' | '0'..='9' | '-' | '_' | '.' | '~' => result.push(c),
let mut result = String::with_capacity(s.len() * 3);
// Encode straight from the UTF-8 bytes: a multi-byte character is encoded
// one byte at a time anyway, and going through `char` would allocate a
// fresh `String` per character.
for &byte in s.as_bytes() {
match byte {
b'A'..=b'Z' | b'a'..=b'z' | b'0'..=b'9' | b'-' | b'_' | b'.' | b'~' => {
result.push(byte as char)
}
_ => {
for byte in c.to_string().as_bytes() {
result.push_str(&format!("%{:02X}", byte));
}
let _ = write!(result, "%{:02X}", byte);
}
}
}
@@ -887,8 +1080,43 @@ pub fn run_enrich_targeted(
conn: &Connection,
needed: &std::collections::HashSet<(String, String)>,
quiet: bool,
no_itunes: bool,
) {
run_enrich_targeted_with_options(conn, needed, quiet, false, OnlineEnrichOptions::default());
}
/// 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,
force: bool,
options: OnlineEnrichOptions,
) {
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, true, options);
return;
}
let all_uncached = match db::uncached_albums(conn) {
Ok(v) => v,
Err(e) => {
@@ -901,7 +1129,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, false, options);
}
/// Run the enrichment process: find all uncached albums and fetch their
@@ -915,15 +1143,35 @@ pub fn run_enrich_targeted(
/// - `force` — if true, re-fetch all albums even if already cached
/// - `quiet` — if true, suppress the "nothing to do" hint (used when called
/// from `report --html` where the user didn't explicitly ask for enrichment)
/// - `no_itunes` — if true, skip the iTunes cover lookup and go straight to CAA
pub fn run_enrich(conn: &Connection, force: bool, quiet: bool, no_itunes: bool) {
if force {
conn.execute("DELETE FROM album_cache", [])
.expect("Failed to clear album_cache");
eprintln!("Cleared album cache (force mode).");
}
let albums = db::uncached_albums(conn).expect("Failed to query uncached albums");
run_enrich_albums(conn, albums, quiet, no_itunes);
/// - `options` — which metadata to fetch and which cover source to prefer
pub fn run_enrich_with_options(
conn: &Connection,
force: bool,
quiet: bool,
options: OnlineEnrichOptions,
) {
// Force mode selects every scrobbled album instead of clearing the cache
// first. `DELETE FROM album_cache` would also discard albums pinned by
// hand with `pin-album` (which exist precisely because automatic search
// cannot find them) and the paths of covers extracted from MPD, orphaning
// those files on disk — and anything the re-fetch then failed to find
// would be lost for good. Each row is instead overwritten in place by
// `upsert_album_cache`, so a failed lookup leaves the old value standing.
let albums = if force {
eprintln!("Force mode enabled: refreshing existing cover files when re-fetching.");
db::all_scrobbled_albums(conn)
} else {
db::uncached_albums(conn)
};
let albums = match albums {
Ok(v) => v,
Err(e) => {
eprintln!("[error] Failed to query albums to enrich: {}", e);
return;
}
};
run_enrich_albums(conn, albums, quiet, force, options);
}
/// Inner enrichment loop shared by `run_enrich` and `run_enrich_targeted`.
@@ -931,7 +1179,8 @@ fn run_enrich_albums(
conn: &Connection,
albums: Vec<db::UncachedAlbum>,
quiet: bool,
no_itunes: bool,
force_redownload: bool,
options: OnlineEnrichOptions,
) {
if albums.is_empty() {
if !quiet {
@@ -942,12 +1191,22 @@ fn run_enrich_albums(
}
let client = build_client();
let covers = covers_dir();
let covers = match covers_dir() {
Ok(d) => d,
Err(e) => {
eprintln!("[error] Cover cache unavailable: {}", e);
return;
}
};
let fetch_covers = options.fetch_mode.wants_covers();
let fetch_genres = options.fetch_mode.wants_genres();
let use_itunes = fetch_covers && matches!(options.cover_source, CoverSourceMode::ItunesThenCaa);
eprintln!("Found {} album(s) to enrich.", albums.len());
let mut success_count = 0;
let mut cover_count = 0;
let mut genre_count = 0;
for (i, album) in albums.iter().enumerate() {
eprintln!(
@@ -958,6 +1217,12 @@ fn run_enrich_albums(
album.album
);
let existing = db::album_cache_meta(conn, &album.artist, &album.album)
.ok()
.flatten();
let existing_genre = existing.as_ref().and_then(|m| m.genre.clone());
let existing_mbid = existing.as_ref().and_then(|m| m.mbid.clone());
// Step 1: Search MusicBrainz for matching releases.
// Always done — we need the MBID for genre data and as the CAA
// lookup key, even if we end up getting the cover from iTunes.
@@ -969,13 +1234,15 @@ fn run_enrich_albums(
// Without an MBID we can't query CAA, so iTunes is the only
// cover option. Use a hash-based filename since there's no MBID.
let cover = if no_itunes {
let cover = if !fetch_covers {
None
} else if !use_itunes {
eprintln!(" iTunes disabled, no cover available.");
None
} else {
eprintln!(" Trying iTunes...");
fetch_itunes_cover_url(&client, &album.artist, &album.album).and_then(|art_url| {
let stem = itunes_cover_stem(&album.artist, &album.album);
let stem = cover_stem("itunes", &album.artist, &album.album);
let dest = covers.join(format!("{}.jpg", stem));
try_download(&client, &art_url, &dest)
})
@@ -984,7 +1251,7 @@ fn run_enrich_albums(
if cover.is_some() {
eprintln!(" Cover downloaded (from iTunes).");
cover_count += 1;
} else if !no_itunes {
} else if fetch_covers && use_itunes {
eprintln!(" No cover art available from any source.");
}
@@ -992,9 +1259,9 @@ fn run_enrich_albums(
let entry = AlbumCacheEntry {
artist: album.artist.clone(),
album: album.album.clone(),
musicbrainz_id: None,
musicbrainz_id: existing_mbid,
cover_url: cover,
genre: None,
genre: existing_genre,
fetched_at: now_str(),
};
match db::upsert_album_cache(conn, &entry) {
@@ -1008,19 +1275,32 @@ fn run_enrich_albums(
let primary_mbid = &candidates[0];
eprintln!(" Found MBID: {}", primary_mbid);
// Step 2: Fetch genres/tags and release-group ID from the primary release.
thread::sleep(RATE_LIMIT_DELAY);
let (genre, release_group_id) = fetch_release_details(&client, primary_mbid);
if let Some(ref g) = genre {
// Step 2: Fetch genres/tags and release-group ID from the primary
// release. Skip the genre half of the work when only covers were asked
// for: the release-group ID is still needed as a CAA fallback key, but
// chasing genres would cost an extra request and rate-limit sleep per
// album for a value that is then thrown away.
let (fetched_genre, release_group_id) = if fetch_genres {
thread::sleep(RATE_LIMIT_DELAY);
fetch_release_details(&client, primary_mbid)
} else if fetch_covers {
thread::sleep(RATE_LIMIT_DELAY);
(None, fetch_release_group_id(&client, primary_mbid))
} else {
(None, None)
};
if let Some(ref g) = fetched_genre {
if fetch_genres {
genre_count += 1;
}
eprintln!(" Genres: {}", g);
}
// Step 3: Try iTunes first — it's fast, has no rate limit, and has
// good commercial coverage. Reuse the MBID-based filename so the file
// is still associated with the correct release regardless of source.
let mut cover = if no_itunes {
None
} else {
let mut cover = if fetch_covers && use_itunes {
let itunes_cover = fetch_itunes_cover_url(&client, &album.artist, &album.album)
.and_then(|art_url| {
let dest = covers.join(format!("{}.jpg", primary_mbid));
@@ -1030,13 +1310,15 @@ fn run_enrich_albums(
eprintln!(" Cover downloaded (from iTunes).");
}
itunes_cover
} else {
None
};
// Step 4: If iTunes had no cover, fall back to the Cover Art Archive.
// CAA is better for obscure or non-commercial releases, and has the
// highest possible resolution for releases it does have.
if cover.is_none() {
if !no_itunes {
if fetch_covers && cover.is_none() {
if use_itunes {
eprintln!(" No iTunes cover. Trying Cover Art Archive...");
}
@@ -1047,6 +1329,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.
@@ -1070,17 +1353,23 @@ fn run_enrich_albums(
if cover.is_some() {
cover_count += 1;
} else {
} else if fetch_covers {
eprintln!(" No cover art available from any source.");
}
let genre_to_store = if fetch_genres {
fetched_genre.or(existing_genre)
} else {
existing_genre
};
// Step 5: Store the result in album_cache.
let entry = AlbumCacheEntry {
artist: album.artist.clone(),
album: album.album.clone(),
musicbrainz_id: Some(primary_mbid.clone()),
cover_url: cover,
genre,
genre: genre_to_store,
fetched_at: now_str(),
};
@@ -1094,7 +1383,12 @@ fn run_enrich_albums(
eprintln!("Enrichment complete:");
eprintln!(" Albums processed: {}", albums.len());
eprintln!(" Successfully cached: {}", success_count);
eprintln!(" Covers downloaded: {}", cover_count);
if fetch_covers {
eprintln!(" Covers downloaded: {}", cover_count);
}
if fetch_genres {
eprintln!(" Albums with genres fetched: {}", genre_count);
}
eprintln!(" Covers directory: {}", covers.display());
}
@@ -1109,8 +1403,24 @@ pub fn enrich_by_mbid(
mbid: &str,
cover_url_override: Option<&str>,
) {
// Reject MBIDs that contain path-traversal sequences or directory
// separators — a valid UUID contains only hex digits and hyphens.
if mbid.contains("..") || mbid.contains('/') || mbid.contains('\\') {
eprintln!(
"[error] Invalid MBID '{}': must not contain path components.",
mbid
);
return;
}
let client = build_client();
let covers = covers_dir();
let covers = match covers_dir() {
Ok(d) => d,
Err(e) => {
eprintln!("[error] Cover cache unavailable: {}", e);
return;
}
};
eprintln!("Fetching release details for MBID {}...", mbid);
let (genre, release_group_id) = fetch_release_details(&client, mbid);
@@ -1151,8 +1461,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 {
@@ -1161,6 +1476,17 @@ pub fn enrich_by_mbid(
result
};
// `upsert_album_cache` overwrites `genre` unconditionally. When the caller
// supplied `--cover-url` and MusicBrainz returned nothing, writing None
// would wipe a perfectly good genre that a previous run had found, so keep
// whatever is already cached in that case.
let genre = genre.or_else(|| {
db::album_cache_meta(conn, artist, album)
.ok()
.flatten()
.and_then(|m| m.genre)
});
let entry = AlbumCacheEntry {
artist: artist.to_string(),
album: album.to_string(),
@@ -1212,6 +1538,37 @@ mod tests {
assert_eq!(urlencoded("A-Z_a-z.0~9"), "A-Z_a-z.0~9");
}
#[test]
fn test_lucene_escape_quotes_and_backslashes() {
assert_eq!(lucene_escape("Purple Rain"), "Purple Rain");
// A quote inside a title would otherwise close the quoted term early
// and turn the remainder into query syntax.
assert_eq!(lucene_escape("Say \"Hello\""), "Say \\\"Hello\\\"");
assert_eq!(lucene_escape("AC\\DC"), "AC\\\\DC");
// Colons are meaningful outside a quoted phrase but not inside one,
// so they are left alone.
assert_eq!(lucene_escape("Vivaldi: Gloria"), "Vivaldi: Gloria");
}
#[test]
fn test_cover_stem_prefix_and_stability() {
let a = cover_stem("mpd", "Bonobo", "Black Sands");
assert_eq!(a, cover_stem("mpd", "Bonobo", "Black Sands"));
assert!(a.starts_with("mpd_"), "stem = {}", a);
assert_eq!(a.len(), "mpd_".len() + 16);
// The prefix distinguishes sources without changing the hash.
let b = cover_stem("itunes", "Bonobo", "Black Sands");
assert_eq!(
a.trim_start_matches("mpd_"),
b.trim_start_matches("itunes_")
);
// Different albums must not collide.
assert_ne!(a, cover_stem("mpd", "Bonobo", "The North Borders"));
assert_ne!(a, cover_stem("mpd", "Deftones", "Black Sands"));
}
#[test]
fn test_strip_bracketed_segments() {
assert_eq!(strip_bracketed_segments("If (Killing Eve)"), "If");
@@ -1254,6 +1611,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(
@@ -1283,6 +1701,43 @@ mod tests {
assert_eq!(from_tags.as_deref(), Some("electronic"));
}
#[test]
fn test_pick_genre_string_rejects_domain_tags() {
// Review-site tags such as "musikexpress.de" must never be picked.
let from_tags = pick_genre_string(
vec![],
vec![
MbTag {
name: "musikexpress.de".to_string(),
count: 3,
},
MbTag {
name: "rollingstone.de".to_string(),
count: 2,
},
MbTag {
name: "shoegaze".to_string(),
count: 1,
},
],
);
assert_eq!(from_tags.as_deref(), Some("shoegaze"));
// Domains must also be stripped from the curated-genres path.
let from_genres = pick_genre_string(
vec![
MbGenre {
name: "pitchfork.com".to_string(),
},
MbGenre {
name: "dream pop".to_string(),
},
],
vec![],
);
assert_eq!(from_genres.as_deref(), Some("dream pop"));
}
#[test]
fn test_uncached_albums_query() {
// Verify the uncached_albums query works correctly.

View File

@@ -1,12 +1,14 @@
//! scrbblr — a local music scrobbler for MPRIS and MPD on Linux.
//!
//! This is the CLI entry point. It provides five subcommands:
//! This is the CLI entry point. It provides six subcommands:
//!
//! - `watch` — monitors a player via `playerctl` and/or MPD, recording
//! scrobbles to SQLite. Both sources can run simultaneously.
//! - `report` — generates listening statistics from the stored data
//! - `enrich` — fetches album art and genre info from MusicBrainz,
//! and/or extracts embedded covers from MPD
//! - `repair-mpd-covers` — revalidate MPD-local cached covers and repair
//! any mismatches by re-fetching from MPD
//! - `last-scrobble`— prints the newest scrobble timestamp
//! - `pin-album` — manually assign a MusicBrainz ID to an album
//!
@@ -53,7 +55,7 @@ mod mpd;
mod report;
mod watcher;
use clap::{Parser, Subcommand};
use clap::{Parser, Subcommand, ValueEnum};
use std::io::BufRead;
use std::process::{Command, Stdio};
use std::sync::atomic::{AtomicBool, Ordering};
@@ -75,6 +77,53 @@ struct Cli {
command: Commands,
}
#[derive(Clone, Copy, Debug, Eq, PartialEq, ValueEnum)]
#[value(rename_all = "kebab-case")]
enum EnrichPipeline {
Mpd,
Online,
Both,
}
#[derive(Clone, Copy, Debug, Eq, PartialEq, ValueEnum)]
#[value(rename_all = "kebab-case")]
enum EnrichFetch {
All,
Covers,
Genres,
}
#[derive(Clone, Copy, Debug, Eq, PartialEq, ValueEnum)]
#[value(rename_all = "kebab-case")]
enum EnrichRetry {
None,
Covers,
Genres,
All,
MpdGenres,
}
impl EnrichRetry {
/// The database-level scope this retry mode selects, or `None` when the
/// normal cooldown should be left alone.
fn scope(self) -> Option<db::RetryScope> {
match self {
Self::None => None,
Self::Covers => Some(db::RetryScope::Covers),
Self::Genres => Some(db::RetryScope::Genres),
Self::All => Some(db::RetryScope::All),
Self::MpdGenres => Some(db::RetryScope::MpdGenres),
}
}
}
#[derive(Clone, Copy, Debug, Eq, PartialEq, ValueEnum)]
#[value(rename_all = "kebab-case")]
enum CoverSource {
ItunesCaa,
Caa,
}
#[derive(Subcommand)]
enum Commands {
/// Watch playerctl metadata and MPD, scrobbling tracks to the local database.
@@ -162,52 +211,76 @@ enum Commands {
},
/// Fetch album art and genre info for all scrobbled albums.
///
/// By default, connects to MPD and extracts embedded cover art from music
/// files via `readpicture` — fully offline, no network access required.
/// Pass `--no-mpd-covers` to skip this step.
/// By default, runs both stages:
///
/// Pass `--online` to also query MusicBrainz for album metadata (MBID,
/// genre) and download cover art. Cover art is sourced from iTunes first
/// (fast, no rate limit), then the Cover Art Archive as fallback. Pass
/// `--no-itunes` to skip iTunes and use only the Cover Art Archive.
/// 1. MPD embedded-cover extraction (`readpicture`, offline)
/// 2. Online metadata/cover enrichment (MusicBrainz + iTunes/CAA)
///
/// Use `--pipeline`, `--fetch`, and `--retry` to narrow the behaviour.
Enrich {
/// Query MusicBrainz for album metadata (MBID, genre) and download
/// cover art from the Cover Art Archive for albums that still have
/// no cover after local extraction.
#[arg(long)]
online: bool,
/// Which enrichment stage(s) to run.
#[arg(long, value_enum, default_value = "both")]
pipeline: EnrichPipeline,
/// Which metadata to fetch during the online stage.
#[arg(long, value_enum, default_value = "all")]
fetch: EnrichFetch,
/// Reset cooldown before online enrichment.
///
/// - `none`: keep normal 7-day cooldown
/// - `covers`: retry cover-missing rows
/// - `genres`: retry genre-missing rows
/// - `all`: retry rows missing either cover or genre
/// - `mpd-genres`: retry genre-missing rows only for MPD-sourced albums
#[arg(long, value_enum, default_value = "none")]
retry: EnrichRetry,
/// Cover source policy for the online stage.
#[arg(long, value_enum, default_value = "itunes-caa")]
cover_source: CoverSource,
/// Re-fetch metadata for all albums from MusicBrainz, even those
/// already cached. Only meaningful together with `--online`.
/// already cached. Only meaningful when `--pipeline` includes `online`.
#[arg(long)]
force: bool,
/// Re-attempt cover downloads for albums that have metadata cached
/// but no cover art yet. Resets the 7-day cooldown for those albums
/// only, so the next `--online` run retries them. Does not affect
/// albums that already have a cover.
/// 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)]
retry_covers: bool,
/// 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.
#[arg(long)]
no_itunes: bool,
/// Disable the MPD embedded cover extraction step. By default,
/// `enrich` connects to MPD and extracts embedded cover art from
/// music files via `readpicture` before doing anything else.
#[arg(long)]
no_mpd_covers: bool,
artist: Option<String>,
/// MPD server hostname, IP address, or Unix socket path.
/// Used for embedded cover extraction (unless `--no-mpd-covers`).
/// Used for MPD extraction when `--pipeline` includes `mpd`.
#[arg(long, default_value = "localhost")]
mpd_host: String,
/// MPD server TCP port. Used for embedded cover extraction when
/// connecting via TCP (ignored for Unix socket paths).
/// MPD server TCP port. Ignored for Unix socket paths.
#[arg(long, default_value = "6600")]
mpd_port: u16,
/// Path to the SQLite database file. Same default as `watch`.
#[arg(long)]
db_path: Option<String>,
},
/// Revalidate cached MPD-local covers and automatically repair mismatches.
///
/// This checks albums whose `cover_url` points to an MPD-generated local
/// file (`mpd_*.jpg`), re-extracts embedded art from MPD, and compares the
/// processed bytes. Missing or mismatched files are overwritten.
RepairMpdCovers {
/// Limit revalidation to albums by this artist (case-insensitive match).
#[arg(long)]
artist: Option<String>,
/// MPD server hostname, IP address, or Unix socket path.
#[arg(long, default_value = "localhost")]
mpd_host: String,
/// MPD server TCP port. Ignored for Unix socket paths.
#[arg(long, default_value = "6600")]
mpd_port: u16,
@@ -259,13 +332,28 @@ enum Commands {
/// Falls back to: ~/.local/share/scrbblr/scrobbles.db
///
/// Creates the parent directory if it doesn't exist.
/// Exits with a diagnostic instead of panicking when the location cannot be
/// determined or created — an unset `$HOME` is a misconfiguration to report,
/// not a bug to dump a backtrace for. Pass `--db-path` to bypass this.
fn default_db_path() -> String {
let data_dir = std::env::var("XDG_DATA_HOME").unwrap_or_else(|_| {
let home = std::env::var("HOME").expect("HOME not set");
format!("{}/.local/share", home)
});
let data_dir = match std::env::var("XDG_DATA_HOME") {
Ok(d) if !d.is_empty() => d,
_ => match std::env::var("HOME") {
Ok(home) => format!("{}/.local/share", home),
Err(_) => {
eprintln!(
"Cannot determine the database location: neither XDG_DATA_HOME \
nor HOME is set. Pass --db-path explicitly."
);
std::process::exit(1);
}
},
};
let dir = format!("{}/scrbblr", data_dir);
std::fs::create_dir_all(&dir).expect("Failed to create data directory");
if let Err(e) = std::fs::create_dir_all(&dir) {
eprintln!("Failed to create data directory {}: {}", dir, e);
std::process::exit(1);
}
format!("{}/scrobbles.db", dir)
}
@@ -324,13 +412,21 @@ fn run_watch(player: &str, use_mpris: bool, mpd_config: Option<mpd::MpdConfig>,
let (tx, rx) = mpsc::channel::<watcher::Event>();
// Track how many reader threads have sent Eof so we know when both
// playerctl processes have ended. Set to 2 when MPRIS is disabled so the
// main loop exits immediately on Ctrl+C (no processes to wait for).
// playerctl processes have ended. Set to 0 when MPRIS is disabled so the
// first Eof (from the Ctrl+C handler) ends the loop immediately — there
// are no reader threads to wait for in that case.
let expected_eofs: usize = if use_mpris { 2 } else { 0 };
let mut meta_handle = None;
let mut status_handle = None;
// Live playerctl children. They are killed explicitly on shutdown: their
// reader threads block in `read_line` until stdout closes, so joining them
// without terminating the child first would hang. An interactive Ctrl+C
// happens to signal the whole process group, but a supervisor that signals
// only our PID (systemd, a wrapper script) would not.
let mut children: Vec<std::process::Child> = Vec::new();
if use_mpris {
// --- Spawn playerctl metadata follower ---
// Outputs one tab-separated line per track change:
@@ -357,15 +453,27 @@ fn run_watch(player: &str, use_mpris: bool, mpd_config: Option<mpd::MpdConfig>,
.spawn();
match (metadata_proc, status_proc) {
(Ok(meta_proc), Ok(stat_proc)) => {
(Ok(mut meta_proc), Ok(mut stat_proc)) => {
// Take the pipes out of the children so the reader threads own
// the streams while we retain the handles needed to kill them.
let meta_stdout = meta_proc
.stdout
.take()
.expect("No stdout for metadata process");
let stat_stdout = stat_proc
.stdout
.take()
.expect("No stdout for status process");
children.push(meta_proc);
children.push(stat_proc);
// --- Metadata reader thread ---
// Reads lines from the metadata process stdout, parses them into
// `Event::Metadata` values, and sends them through the channel.
// Sends `Event::Eof` when the process exits (stdout closes).
let tx_meta = tx.clone();
meta_handle = Some(thread::spawn(move || {
let stdout = meta_proc.stdout.expect("No stdout for metadata process");
let reader = std::io::BufReader::new(stdout);
let reader = std::io::BufReader::new(meta_stdout);
for line in reader.lines() {
match line {
Ok(l) => {
@@ -385,8 +493,7 @@ fn run_watch(player: &str, use_mpris: bool, mpd_config: Option<mpd::MpdConfig>,
// Same pattern as the metadata reader, but parses status lines.
let tx_status = tx.clone();
status_handle = Some(thread::spawn(move || {
let stdout = stat_proc.stdout.expect("No stdout for status process");
let reader = std::io::BufReader::new(stdout);
let reader = std::io::BufReader::new(stat_stdout);
for line in reader.lines() {
match line {
Ok(l) => {
@@ -402,12 +509,19 @@ fn run_watch(player: &str, use_mpris: bool, mpd_config: Option<mpd::MpdConfig>,
let _ = tx_status.send(watcher::Event::Eof);
}));
}
(meta_result, _) => {
(meta_result, status_result) => {
// At least one spawn failed — MPRIS watcher cannot run.
// Print the error and send synthetic Eofs so the main loop
// Print the error, reap whichever process did start so it is
// not left orphaned, and send synthetic Eofs so the main loop
// below exits cleanly; the MPD watcher (if any) continues.
if let Err(e) = meta_result {
eprintln!("[warn] Failed to spawn playerctl: {}", e);
for result in [meta_result, status_result] {
match result {
Ok(mut child) => {
let _ = child.kill();
let _ = child.wait();
}
Err(e) => eprintln!("[warn] Failed to spawn playerctl: {}", e),
}
}
eprintln!("[warn] MPRIS watcher will be inactive.");
let _ = tx.send(watcher::Event::Eof);
@@ -465,26 +579,53 @@ fn run_watch(player: &str, use_mpris: bool, mpd_config: Option<mpd::MpdConfig>,
tracker.handle_event(watcher::Event::Eof);
}
// Terminate the playerctl children. `playerctl --follow` never exits on
// its own, so its stdout would stay open and the reader threads below
// would block forever. Do this before any join.
for mut child in children {
let _ = child.kill();
let _ = child.wait();
}
// Wait for the MPD watcher thread to finish its graceful shutdown.
// It will notice `running == false` on the next idle timeout (≤ 500 ms).
if let Some(handle) = mpd_handle {
let _ = handle.join();
}
eprintln!("Goodbye.");
// Wait for MPRIS reader threads to finish.
// Wait for MPRIS reader threads to finish. Their pipes are now closed.
if let Some(h) = meta_handle {
let _ = h.join();
}
if let Some(h) = status_handle {
let _ = h.join();
}
eprintln!("Goodbye.");
}
/// Round a value to the nearest multiple of 5.
/// Round a non-negative value to the nearest multiple of 5.
///
/// Only ever called with a validated positive `--limit`, so integer division
/// truncating toward zero is not a concern here.
fn round_to_5(n: i64) -> i64 {
((n + 2) / 5) * 5
((n.max(0) + 2) / 5) * 5
}
/// Everything [`run_report`] needs, bundled so the call stays readable.
struct ReportOptions<'a> {
period: &'a str,
json: bool,
html: bool,
/// Output directory for `--html`. `None` prints to stdout.
output: Option<&'a str>,
/// Top-N size for the per-period sections.
limit: i64,
/// Top-N size for the all-time sections.
all_time_limit: i64,
no_enrich: bool,
mpd_cfg: &'a mpd::MpdConfig,
db_path: &'a str,
}
// ---------------------------------------------------------------------------
@@ -501,17 +642,26 @@ fn round_to_5(n: i64) -> i64 {
/// - **HTML** (`--html`): generates a multi-period report (Today / Week / Month /
/// All Time) with bar charts and album cover art.
/// With `--output <dir>`, writes `index.html` + `covers/` to a directory.
fn run_report(
period: &str,
json: bool,
html: bool,
output: Option<&str>,
limit: i64,
all_time_limit: i64,
no_enrich: bool,
mpd_cfg: &mpd::MpdConfig,
db_path: &str,
) {
fn run_report(opts: &ReportOptions<'_>) {
let ReportOptions {
period,
json,
html,
output,
limit,
all_time_limit,
no_enrich,
mpd_cfg,
db_path,
} = *opts;
// Validate output-format flags before doing any work, so an obvious
// mistake fails immediately rather than after opening the database.
if json && html {
eprintln!("Please choose one output format: either --json or --html.");
std::process::exit(1);
}
let conn = match db::open_db(db_path) {
Ok(c) => c,
Err(e) => {
@@ -531,11 +681,6 @@ fn run_report(
std::process::exit(1);
}
if json && html {
eprintln!("Please choose one output format: either --json or --html.");
std::process::exit(1);
}
// Cover enrichment is only meaningful for HTML reports — terminal and JSON
// output never displays cover art, so there is nothing to gain from
// extracting or downloading covers for those modes.
@@ -556,7 +701,7 @@ fn run_report(
// iTunes is tried first (fast, no rate limit), then CAA as fallback.
// Respects the 7-day cooldown so we don't hammer MusicBrainz on every
// report run.
enrich::run_enrich_targeted(&conn, &needed, true, false);
enrich::run_enrich_targeted(&conn, &needed, true);
}
if html {
@@ -696,42 +841,74 @@ fn main() {
mpd_port,
db_path,
} => {
// Both limits are interpolated directly into SQL `LIMIT` clauses.
// They are plain integers so there is no injection risk, but SQLite
// treats a negative LIMIT as "no limit", which would silently dump
// the entire library instead of the requested top-N.
if limit < 1 {
eprintln!("--limit must be at least 1 (got {}).", limit);
std::process::exit(1);
}
if let Some(atl) = all_time_limit
&& atl < 1
{
eprintln!("--all-time-limit must be at least 1 (got {}).", atl);
std::process::exit(1);
}
let path = db_path.unwrap_or_else(default_db_path);
let atl =
all_time_limit.unwrap_or_else(|| round_to_5((limit as f64 * 2.5).round() as i64));
let atl = all_time_limit
.unwrap_or_else(|| round_to_5((limit as f64 * 2.5).round() as i64))
.max(1);
let mpd_cfg = mpd::MpdConfig {
host: mpd_host,
port: mpd_port,
};
run_report(
&period,
run_report(&ReportOptions {
period: &period,
json,
html,
output.as_deref(),
output: output.as_deref(),
limit,
atl,
all_time_limit: atl,
no_enrich,
&mpd_cfg,
&path,
);
mpd_cfg: &mpd_cfg,
db_path: &path,
});
}
Commands::Enrich {
online,
pipeline,
fetch,
retry,
cover_source,
force,
retry_covers,
no_itunes,
no_mpd_covers,
artist,
mpd_host,
mpd_port,
db_path,
} => {
if no_mpd_covers && !online {
eprintln!("Nothing to do. Pass --online and/or omit --no-mpd-covers.");
let run_mpd = matches!(pipeline, EnrichPipeline::Mpd | EnrichPipeline::Both);
let run_online = matches!(pipeline, EnrichPipeline::Online | EnrichPipeline::Both);
let wants_covers = matches!(fetch, EnrichFetch::All | EnrichFetch::Covers);
if force && !run_online {
eprintln!("--force requires --pipeline online or --pipeline both.");
std::process::exit(1);
}
if !matches!(retry, EnrichRetry::None) && !run_online {
eprintln!("--retry requires --pipeline online or --pipeline both.");
std::process::exit(1);
}
if run_mpd && !wants_covers && !run_online {
eprintln!(
" (default) Extract embedded covers from music files via MPD (offline)"
);
eprintln!(
" --online Fetch metadata and covers from MusicBrainz / iTunes / CAA"
"Nothing to do: --pipeline mpd only handles covers, but --fetch {} excludes covers.",
match fetch {
EnrichFetch::Genres => "genres",
EnrichFetch::All => "all",
EnrichFetch::Covers => "covers",
}
);
std::process::exit(0);
}
@@ -745,32 +922,102 @@ fn main() {
}
};
// 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.
if !no_mpd_covers {
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)
// when the selected pipeline includes MPD and the fetch mode asks
// for covers.
if run_mpd && wants_covers {
let mpd_cfg = mpd::MpdConfig {
host: mpd_host,
port: mpd_port,
};
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) {
Ok(n) => eprintln!("Reset cooldown for {} album(s) missing covers.", n),
Err(e) => eprintln!("[warn] Failed to reset cover timestamps: {}", e),
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);
}
}
if online {
enrich::run_enrich(&conn, force, false, no_itunes);
if run_online {
// Optional retry selector: reset cooldown timestamps so the
// online stage revisits selected missing fields immediately.
if let Some(scope) = retry.scope() {
match db::reset_retry_timestamps(&conn, scope, artist.as_deref()) {
Ok(n) => eprintln!(
"Reset cooldown for {} album(s) via --retry {}.",
n,
scope.as_str()
),
Err(e) => eprintln!("[warn] Failed to reset retry timestamps: {}", e),
}
}
let cover_source_mode = match cover_source {
CoverSource::ItunesCaa => enrich::CoverSourceMode::ItunesThenCaa,
CoverSource::Caa => enrich::CoverSourceMode::CaaOnly,
};
let fetch_mode = match fetch {
EnrichFetch::All => enrich::OnlineFetchMode::All,
EnrichFetch::Covers => enrich::OnlineFetchMode::Covers,
EnrichFetch::Genres => enrich::OnlineFetchMode::Genres,
};
let options = enrich::OnlineEnrichOptions {
fetch_mode,
cover_source: cover_source_mode,
};
if let Some(ref needed) = artist_needed {
enrich::run_enrich_targeted_with_options(&conn, needed, false, force, options);
} else {
enrich::run_enrich_with_options(&conn, force, false, options);
}
}
}
Commands::RepairMpdCovers {
artist,
mpd_host,
mpd_port,
db_path,
} => {
let path = db_path.unwrap_or_else(default_db_path);
let conn = match db::open_db(&path) {
Ok(c) => c,
Err(e) => {
eprintln!("Failed to open database at {}: {}", path, e);
std::process::exit(1);
}
};
let mpd_cfg = mpd::MpdConfig {
host: mpd_host,
port: mpd_port,
};
mpd::run_mpd_cover_revalidate(&mpd_cfg, &conn, artist.as_deref());
}
Commands::LastScrobble { db_path } => {
let path = db_path.unwrap_or_else(default_db_path);
let conn = match db::open_db(&path) {

View File

@@ -73,9 +73,10 @@ use std::io::{self, BufRead, BufReader, Read, Write};
use std::net::TcpStream;
#[cfg(unix)]
use std::os::unix::net::UnixStream;
use std::path::Path;
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::{Arc, Mutex};
use std::time::Duration;
use std::time::{Duration, Instant};
use rusqlite::Connection;
@@ -128,6 +129,27 @@ impl MpdConfig {
// Low-level connection
// ---------------------------------------------------------------------------
/// How long to keep waiting for a command's response before giving up.
///
/// The socket's own read timeout is deliberately short (see [`READ_TIMEOUT`])
/// so the idle loop can poll the shutdown flag. Every other exchange must wait
/// for the real reply instead: abandoning a response half-read leaves unread
/// bytes in the socket that the next command would parse as its own reply.
const RESPONSE_TIMEOUT: Duration = Duration::from_secs(15);
/// Socket read timeout. Short enough that the idle loop notices shutdown
/// promptly; [`MpdConn`] transparently retries across it for command replies.
const READ_TIMEOUT: Duration = Duration::from_millis(500);
/// Return true when an I/O error is just the socket's read timeout firing,
/// rather than a genuine failure.
fn is_timeout(e: &io::Error) -> bool {
matches!(
e.kind(),
io::ErrorKind::WouldBlock | io::ErrorKind::TimedOut | io::ErrorKind::Interrupted
)
}
/// An open, authenticated connection to MPD.
///
/// Holds a buffered reader for receiving response lines (and binary chunks)
@@ -135,12 +157,96 @@ impl MpdConfig {
/// the same underlying socket.
///
/// The read half has a short timeout (500 ms) so that the idle loop can
/// periodically check the shutdown flag without blocking indefinitely.
/// periodically check the shutdown flag without blocking indefinitely. All
/// command/response reads go through [`MpdConn::read_line`] and
/// [`MpdConn::read_exact`], which retry across that timeout so a slow reply is
/// never mistaken for a finished one.
struct MpdConn {
/// Buffered reader wrapping the socket's read half.
reader: BufReader<Box<dyn Read + Send>>,
/// Write handle for sending commands.
writer: Box<dyn Write + Send>,
/// Set once we abandon a read part-way through a response. The stream can
/// no longer be trusted to be positioned at a message boundary, so the
/// connection must be torn down and re-established rather than reused.
desynced: bool,
}
impl MpdConn {
/// Read one `\n`-terminated line, retrying while the socket's short read
/// timeout fires. Returns `Ok(None)` at EOF.
///
/// Partial data accumulated before a timeout is retained and the read
/// resumes, so a reply split across timeouts is reassembled correctly.
/// Giving up marks the connection desynced.
fn read_line(&mut self) -> io::Result<Option<String>> {
let deadline = Instant::now() + RESPONSE_TIMEOUT;
let mut buf: Vec<u8> = Vec::new();
loop {
match self.reader.read_until(b'\n', &mut buf) {
Ok(_) => {
if buf.last() == Some(&b'\n') {
return Ok(Some(String::from_utf8_lossy(&buf).into_owned()));
}
// No delimiter and no error means EOF.
if buf.is_empty() {
return Ok(None);
}
return Ok(Some(String::from_utf8_lossy(&buf).into_owned()));
}
Err(ref e) if is_timeout(e) => {
if Instant::now() >= deadline {
self.desynced = true;
return Err(io::Error::new(
io::ErrorKind::TimedOut,
"timed out waiting for MPD response line",
));
}
}
Err(e) => {
self.desynced = true;
return Err(e);
}
}
}
}
/// Fill `buf` completely, retrying while the socket's short read timeout
/// fires.
///
/// `Read::read_exact` cannot be used here: on error it does not report how
/// many bytes it consumed, so a timeout mid-chunk would silently swallow
/// part of the binary payload. This loop tracks progress itself.
fn read_exact(&mut self, buf: &mut [u8]) -> io::Result<()> {
let deadline = Instant::now() + RESPONSE_TIMEOUT;
let mut filled = 0;
while filled < buf.len() {
match self.reader.read(&mut buf[filled..]) {
Ok(0) => {
self.desynced = true;
return Err(io::Error::new(
io::ErrorKind::UnexpectedEof,
"MPD closed the connection mid-chunk",
));
}
Ok(n) => filled += n,
Err(ref e) if is_timeout(e) => {
if Instant::now() >= deadline {
self.desynced = true;
return Err(io::Error::new(
io::ErrorKind::TimedOut,
"timed out reading MPD binary chunk",
));
}
}
Err(e) => {
self.desynced = true;
return Err(e);
}
}
}
Ok(())
}
}
/// Open a TCP connection to MPD, set a read timeout, and consume the
@@ -153,7 +259,7 @@ fn connect_tcp(config: &MpdConfig) -> Result<MpdConn, String> {
// A short read timeout lets the idle loop notice shutdown requests without
// blocking for potentially minutes waiting for the next player event.
stream
.set_read_timeout(Some(Duration::from_millis(500)))
.set_read_timeout(Some(READ_TIMEOUT))
.map_err(|e| format!("Failed to set MPD read timeout: {}", e))?;
// Clone the stream so we have independent read and write handles.
@@ -164,6 +270,7 @@ fn connect_tcp(config: &MpdConfig) -> Result<MpdConn, String> {
let mut conn = MpdConn {
reader: BufReader::new(Box::new(stream)),
writer: Box::new(write_stream),
desynced: false,
};
// Consume the "OK MPD x.y.z" greeting before any command can be sent.
@@ -178,7 +285,7 @@ fn connect_unix(config: &MpdConfig) -> Result<MpdConn, String> {
.map_err(|e| format!("Could not connect to MPD socket {}: {}", config.host, e))?;
stream
.set_read_timeout(Some(Duration::from_millis(500)))
.set_read_timeout(Some(READ_TIMEOUT))
.map_err(|e| format!("Failed to set MPD read timeout: {}", e))?;
let write_stream = stream
@@ -188,6 +295,7 @@ fn connect_unix(config: &MpdConfig) -> Result<MpdConn, String> {
let mut conn = MpdConn {
reader: BufReader::new(Box::new(stream)),
writer: Box::new(write_stream),
desynced: false,
};
consume_welcome(&mut conn)?;
@@ -213,10 +321,10 @@ fn connect(config: &MpdConfig) -> Result<MpdConn, String> {
/// Returns `Err` if the connection is closed immediately or the greeting
/// doesn't start with `"OK MPD"`, which would indicate a wrong host/port.
fn consume_welcome(conn: &mut MpdConn) -> Result<(), String> {
let mut line = String::new();
conn.reader
.read_line(&mut line)
.map_err(|e| format!("Failed to read MPD welcome: {}", e))?;
let line = conn
.read_line()
.map_err(|e| format!("Failed to read MPD welcome: {}", e))?
.ok_or_else(|| "MPD closed the connection before sending a greeting".to_string())?;
if !line.starts_with("OK MPD") {
return Err(format!("Unexpected MPD greeting: {:?}", line));
@@ -238,26 +346,37 @@ fn send_command(conn: &mut MpdConn, cmd: &str) -> io::Result<()> {
/// Read a `key: value` response from MPD until `OK\n` is seen.
///
/// Returns a `HashMap<key, value>` of all lines parsed before `OK`. An `ACK`
/// line is treated as an error and returns an empty map (the caller ignores
/// the result for most queries).
/// line is treated as an error and returns what was parsed so far (usually
/// nothing) — the `ACK` itself terminates the response, so the stream stays in
/// sync.
///
/// Returns `None` when the response could not be read to completion (EOF or a
/// read failure). In that case the connection is left desynced and the caller
/// must reconnect rather than issue another command: the unread remainder of
/// this reply would otherwise be parsed as the next command's response.
///
/// Lines not matching `"key: value"` are silently skipped — MPD sometimes
/// includes continuation data that doesn't follow this pattern.
fn read_response(conn: &mut MpdConn) -> HashMap<String, String> {
fn read_response(conn: &mut MpdConn) -> Option<HashMap<String, String>> {
let mut map = HashMap::new();
loop {
let mut line = String::new();
match conn.reader.read_line(&mut line) {
Ok(0) => break, // EOF — connection closed
Ok(_) => {}
Err(_) => break,
}
let line = line.trim_end_matches('\n').trim_end_matches('\r');
let raw = match conn.read_line() {
Ok(Some(l)) => l,
// EOF part-way through a response is a lost connection, not an
// empty result.
Ok(None) => {
conn.desynced = true;
return None;
}
Err(_) => return None,
};
let line = raw.trim_end_matches('\n').trim_end_matches('\r');
if line == "OK" {
break;
}
if line.starts_with("ACK") {
// Protocol error — log it and return what we have (usually nothing).
// Protocol-level error. The ACK line ends the response, so the
// stream remains usable.
break;
}
// Split on the first ": " only, in case a value contains ": ".
@@ -267,7 +386,7 @@ fn read_response(conn: &mut MpdConn) -> HashMap<String, String> {
map.insert(key, val);
}
}
map
Some(map)
}
/// Read the `readpicture` response header lines until `binary: N` is seen.
@@ -278,9 +397,8 @@ fn read_response(conn: &mut MpdConn) -> HashMap<String, String> {
fn read_picture_header(conn: &mut MpdConn) -> Option<(u64, u64)> {
let mut total_size: u64 = 0;
loop {
let mut line = String::new();
conn.reader.read_line(&mut line).ok()?;
let line = line.trim_end_matches('\n').trim_end_matches('\r');
let raw = conn.read_line().ok()??;
let line = raw.trim_end_matches('\n').trim_end_matches('\r');
if line == "OK" {
// Reached OK without a binary field — unexpected, treat as no data.
@@ -302,19 +420,47 @@ fn read_picture_header(conn: &mut MpdConn) -> Option<(u64, u64)> {
/// Read exactly `n` raw bytes from MPD's response stream.
///
/// Used after parsing the `binary: N` header to extract the image chunk.
/// `BufReader::read_exact` correctly drains buffered data first and then
/// reads the remainder from the socket, so mixing text line reads and binary
/// chunk reads on the same `BufReader` is safe.
/// [`MpdConn::read_exact`] drains buffered data first and then reads the
/// remainder from the socket, so mixing text line reads and binary chunk reads
/// on the same `BufReader` is safe, and it retries across the socket's short
/// read timeout so a chunk that arrives slowly is not abandoned half-read.
fn read_exact_bytes(conn: &mut MpdConn, n: u64) -> Option<Vec<u8>> {
let mut buf = vec![0u8; n as usize];
conn.reader.read_exact(&mut buf).ok()?;
conn.read_exact(&mut buf).ok()?;
Some(buf)
}
/// Read the single `OK\n` line that terminates a binary chunk response.
fn read_ok_line(conn: &mut MpdConn) {
let mut line = String::new();
let _ = conn.reader.read_line(&mut line);
/// 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 raw = conn.read_line().ok()??;
let line = raw.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") {
// An ACK terminates the response, so the stream is still aligned.
return None;
}
// Any other non-empty line here means the stream got out of sync:
// we cannot tell how much of the binary payload we mis-parsed, so the
// connection must not be reused.
conn.desynced = true;
return None;
}
}
// ---------------------------------------------------------------------------
@@ -350,7 +496,7 @@ struct MpdStatus {
/// Returns `None` if MPD is stopped (no current song) or if the query fails.
fn query_currentsong(conn: &mut MpdConn) -> Option<MpdSong> {
send_command(conn, "currentsong").ok()?;
let map = read_response(conn);
let map = read_response(conn)?;
// If MPD is stopped, currentsong returns an empty response.
if map.is_empty() {
@@ -383,9 +529,13 @@ fn query_status(conn: &mut MpdConn) -> MpdStatus {
if send_command(conn, "status").is_err() {
return MpdStatus::default();
}
let map = read_response(conn);
MpdStatus {
state: map.get("state").cloned().unwrap_or_default(),
match read_response(conn) {
Some(map) => MpdStatus {
state: map.get("state").cloned().unwrap_or_default(),
},
// Unreadable response: the connection is already marked desynced and
// the event loop will tear it down. Report an empty state meanwhile.
None => MpdStatus::default(),
}
}
@@ -411,11 +561,17 @@ fn search_song_for_album(conn: &mut MpdConn, artist: &str, album: &str) -> Optio
escape_mpd_string(artist),
escape_mpd_string(album)
);
if send_command(conn, &cmd).is_ok() {
let map = read_response(conn);
if let Some(file) = map.get("file") {
return Some(file.clone());
}
if send_command(conn, &cmd).is_ok()
&& let Some(map) = read_response(conn)
&& let Some(file) = map.get("file")
{
return Some(file.clone());
}
// A failed read leaves the stream mid-response; issuing the fallback
// search on the same connection would misparse it.
if conn.desynced {
return None;
}
// Fall back to the Artist tag (handles single-artist albums and tracks
@@ -425,11 +581,11 @@ fn search_song_for_album(conn: &mut MpdConn, artist: &str, album: &str) -> Optio
escape_mpd_string(artist),
escape_mpd_string(album)
);
if send_command(conn, &cmd).is_ok() {
let map = read_response(conn);
if let Some(file) = map.get("file") {
return Some(file.clone());
}
if send_command(conn, &cmd).is_ok()
&& let Some(map) = read_response(conn)
&& let Some(file) = map.get("file")
{
return Some(file.clone());
}
None
@@ -455,11 +611,17 @@ fn escape_mpd_string(s: &str) -> String {
///
/// Returns `None` if the file has no embedded picture, if the file is not
/// found in MPD's database, or if a protocol error occurs.
///
/// The transfer is bounded on both size and iteration count. MPD is normally
/// trustworthy, but the loop is driven entirely by numbers MPD supplies, so a
/// buggy or hostile server could otherwise keep it allocating forever — see
/// [`MAX_PICTURE_BYTES`] and [`MAX_PICTURE_CHUNKS`].
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 {
for _ in 0..MAX_PICTURE_CHUNKS {
// Request the next chunk starting at `offset`.
let cmd = format!("readpicture \"{}\" {}", escape_mpd_string(file_uri), offset);
send_command(conn, &cmd).ok()?;
@@ -467,11 +629,24 @@ 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 > MAX_PICTURE_BYTES || chunk_size > MAX_PICTURE_BYTES {
eprintln!(" [warn] Embedded cover art is implausibly large; skipping.");
return None;
}
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
// was no picture at all.
break;
return finish_picture(all_bytes, expected_total);
}
// Read the raw binary chunk from the stream.
@@ -479,14 +654,49 @@ 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;
// Stop when we've read the complete image.
if total_size > 0 && offset >= total_size {
break;
if all_bytes.len() as u64 > MAX_PICTURE_BYTES {
eprintln!(" [warn] Embedded cover art exceeded the size limit; skipping.");
return None;
}
// Stop when we've read the complete image. A server that never
// declares a total size falls through to the chunk-count guard rather
// than looping forever.
if total_size > 0 && offset >= total_size {
return finish_picture(all_bytes, expected_total);
}
}
// Ran out of iterations without MPD ever signalling completion.
None
}
/// Largest embedded picture we are willing to buffer, in bytes.
///
/// Cover art is a JPEG or PNG of a few megabytes at most; the cap only exists
/// so a bogus `size:`/`binary:` header cannot drive an unbounded allocation.
const MAX_PICTURE_BYTES: u64 = 64 * 1024 * 1024;
/// Upper bound on `readpicture` round-trips for a single image.
///
/// MPD's default chunk size is 8 KiB, so this allows well over
/// [`MAX_PICTURE_BYTES`] worth of chunks while still terminating if a server
/// reports no total size and keeps returning data.
const MAX_PICTURE_CHUNKS: usize = 16_384;
/// Validate a completed picture transfer and hand back the bytes.
///
/// Returns `None` when MPD declared a total size we did not fully receive, or
/// when the file simply had no embedded art.
fn finish_picture(all_bytes: Vec<u8>, expected_total: Option<u64>) -> Option<Vec<u8>> {
if let Some(total) = expected_total
&& all_bytes.len() as u64 != total
{
return None;
}
if all_bytes.is_empty() {
@@ -502,27 +712,12 @@ fn read_picture(conn: &mut MpdConn, file_uri: &str) -> Option<Vec<u8>> {
/// Generate a stable filename stem for a locally-extracted MPD cover.
///
/// Uses a 64-bit FNV-1a hash of `"artist\talbum"`, formatted as a 16-char
/// hex string prefixed with `"mpd_"`. This gives a compact, deterministic
/// filename that:
///
/// - Is stable across runs (same artist+album always maps to the same name).
/// - Is visually distinct from MBID-based filenames (MBIDs contain hyphens).
/// - Has negligible collision probability across a typical music library.
/// Thin wrapper over [`enrich::cover_stem`] that pins the `mpd_` prefix, which
/// `repair-mpd-covers` relies on to find these files again.
///
/// Example: `mpd_a3f7c2b91e045d68.jpg`
fn album_cover_stem(artist: &str, album: &str) -> String {
// FNV-1a 64-bit hash constants.
const FNV_OFFSET: u64 = 14695981039346656037;
const FNV_PRIME: u64 = 1099511628211;
let input = format!("{}\t{}", artist, album);
let mut hash = FNV_OFFSET;
for byte in input.bytes() {
hash ^= byte as u64;
hash = hash.wrapping_mul(FNV_PRIME);
}
format!("mpd_{:016x}", hash)
enrich::cover_stem("mpd", artist, album)
}
// ---------------------------------------------------------------------------
@@ -707,6 +902,15 @@ fn run_event_loop(
// Query MPD's current state now that we know something changed.
let song = query_currentsong(mpd_conn);
let status = query_status(mpd_conn);
// If either query failed part-way, the stream is no longer positioned
// at a message boundary. Reconnect instead of issuing more commands
// against a connection whose replies we can no longer line up.
if mpd_conn.desynced {
eprintln!("[mpd] Lost sync with MPD protocol stream.");
return false;
}
let new = MpdPlayerState::from_queries(song, status);
// Determine what changed and dispatch the appropriate events.
@@ -716,6 +920,11 @@ fn run_event_loop(
// cover for the new album if we don't already have one cached.
if new.file != prev.file && !new.file.is_empty() && !new.album.is_empty() {
try_extract_cover(mpd_conn, db_conn, &new);
if mpd_conn.desynced {
eprintln!("[mpd] Lost sync while reading embedded cover art.");
*prev = new;
return false;
}
}
*prev = new;
@@ -744,12 +953,24 @@ enum IdleResult {
/// of whether a `changed: player` line appeared. In both cases the caller
/// re-queries MPD state, so the distinction doesn't matter.
fn wait_for_idle_response(conn: &mut MpdConn, running: &Arc<AtomicBool>) -> IdleResult {
// This is the one place that reads the socket directly rather than going
// through `MpdConn::read_line`: here a timeout is the *point*, since it is
// what lets us poll the shutdown flag while MPD is silent.
//
// `line` lives outside the loop so that a line split across two timeouts
// is reassembled rather than truncated. `read_until` appends what it got
// before erroring and leaves it in the buffer, so resuming is correct.
let mut line: Vec<u8> = Vec::new();
loop {
let mut line = String::new();
match conn.reader.read_line(&mut line) {
match conn.reader.read_until(b'\n', &mut line) {
Ok(0) => return IdleResult::ConnectionLost, // EOF
Ok(_) => {
let trimmed = line.trim();
if line.last() != Some(&b'\n') {
// Delimiter missing without an error means EOF mid-line.
return IdleResult::ConnectionLost;
}
let text = String::from_utf8_lossy(&line);
let trimmed = text.trim();
if trimmed == "OK" {
// End of idle response — re-query state regardless of whether
// "changed: player" appeared. With `idle player` the line
@@ -759,10 +980,9 @@ fn wait_for_idle_response(conn: &mut MpdConn, running: &Arc<AtomicBool>) -> Idle
}
// "changed: player" and any other key/value lines are consumed
// and discarded here — we query currentsong/status ourselves.
line.clear();
}
Err(ref e)
if e.kind() == io::ErrorKind::WouldBlock || e.kind() == io::ErrorKind::TimedOut =>
{
Err(ref e) if is_timeout(e) => {
// Read timeout — check if we should shut down, then keep waiting.
if !running.load(Ordering::SeqCst) {
return IdleResult::Shutdown;
@@ -897,8 +1117,13 @@ fn try_extract_cover(
None => return,
};
// Save the processed image to the covers cache directory.
let covers = enrich::covers_dir();
// Save the processed image to the covers cache directory. Caching a cover
// is opportunistic, so an unusable cache directory just means no cover —
// it must not interrupt the watch loop.
let covers = match enrich::covers_dir() {
Ok(d) => d,
Err(_) => return,
};
let stem = album_cover_stem(&state.artist, &state.album);
let dest = covers.join(format!("{}.jpg", stem));
if std::fs::write(&dest, &processed).is_err() {
@@ -994,6 +1219,169 @@ pub fn run_mpd_cover_enrich_targeted(
run_mpd_cover_enrich_albums(config, conn, albums);
}
/// Revalidate already-cached local MPD covers and re-fetch mismatches.
///
/// This command targets albums that already have `cover_url` pointing to an
/// MPD-local path (`mpd_*.jpg`). For each candidate album it re-reads embedded
/// art from MPD, applies the same resize/re-encode pipeline, then compares the
/// resulting bytes to the existing file.
///
/// If bytes differ (or the file is missing), the local file is overwritten and
/// `album_cache.fetched_at` is refreshed via [`db::set_local_cover`].
pub fn run_mpd_cover_revalidate(config: &MpdConfig, conn: &Connection, artist: Option<&str>) {
let mut albums = match db::albums_with_local_mpd_cover(conn) {
Ok(a) => a,
Err(e) => {
eprintln!("[error] Failed to query MPD-local covers: {}", e);
return;
}
};
if let Some(artist_name) = artist {
albums.retain(|a| a.artist.eq_ignore_ascii_case(artist_name));
}
if albums.is_empty() {
if let Some(artist_name) = artist {
eprintln!(
"No MPD-local covers found for artist \"{}\". Nothing to revalidate.",
artist_name
);
} else {
eprintln!("No MPD-local covers found. Nothing to revalidate.");
}
return;
}
// Resolve the covers directory once, up front: it is the boundary the
// per-album path check below is measured against, so failing to resolve it
// must stop the run rather than weaken the check.
let covers_root = match enrich::covers_dir() {
Ok(d) => d,
Err(e) => {
eprintln!("[error] Cover cache unavailable: {}", e);
return;
}
};
let mut mpd_conn = match connect(config) {
Ok(c) => c,
Err(e) => {
eprintln!("[warn] MPD unreachable, skipping cover revalidation: {}", e);
return;
}
};
eprintln!("Revalidating {} MPD-local cover(s)...", albums.len());
let mut unchanged = 0;
let mut repaired = 0;
let mut skipped = 0;
for (i, album) in albums.iter().enumerate() {
eprintln!(
"[{}/{}] {} - {}",
i + 1,
albums.len(),
album.artist,
album.album
);
if mpd_conn.desynced {
eprintln!(" [warn] Lost sync with MPD; aborting revalidation.");
skipped += albums.len() - i;
break;
}
let file_uri = match search_song_for_album(&mut mpd_conn, &album.artist, &album.album) {
Some(f) => f,
None => {
eprintln!(" Not found in MPD database, skipping.");
skipped += 1;
continue;
}
};
let picture_bytes = match read_picture(&mut mpd_conn, &file_uri) {
Some(b) => b,
None => {
eprintln!(" No embedded cover art found, skipping.");
skipped += 1;
continue;
}
};
let processed = match enrich::resize_cover_bytes(&picture_bytes) {
Some(b) => b,
None => {
eprintln!(" [warn] Could not decode embedded cover, skipping.");
skipped += 1;
continue;
}
};
let dest = Path::new(&album.cover_url);
// Sanity-check: only write to paths inside the scrbblr covers
// directory. Rejects any DB row whose cover_url was tampered with to
// point outside our own data directory.
if !dest.starts_with(&covers_root) {
eprintln!(
" [warn] cover_url '{}' is outside the covers directory; skipping.",
album.cover_url
);
skipped += 1;
continue;
}
let needs_repair = match std::fs::read(dest) {
Ok(existing) => existing != processed,
Err(_) => true,
};
if !needs_repair {
eprintln!(" OK: existing file matches MPD extraction.");
unchanged += 1;
continue;
}
if let Some(parent) = dest.parent()
&& let Err(e) = std::fs::create_dir_all(parent)
{
eprintln!(" [warn] Failed to create parent directory: {}", e);
skipped += 1;
continue;
}
if let Err(e) = std::fs::write(dest, &processed) {
eprintln!(" [warn] Failed to write repaired cover: {}", e);
skipped += 1;
continue;
}
match db::set_local_cover(conn, &album.artist, &album.album, &album.cover_url) {
Ok(_) => {
eprintln!(" Repaired: {}", album.cover_url);
repaired += 1;
}
Err(e) => {
eprintln!(
" [error] Repaired file but failed to update album_cache: {}",
e
);
skipped += 1;
}
}
}
eprintln!();
eprintln!("MPD cover revalidation complete:");
eprintln!(" Albums checked: {}", albums.len());
eprintln!(" Covers unchanged: {}", unchanged);
eprintln!(" Covers repaired: {}", repaired);
eprintln!(" Skipped: {}", skipped);
}
/// Inner loop shared by `run_mpd_cover_enrich` and
/// `run_mpd_cover_enrich_targeted`. Connects to MPD and processes the given
/// album list.
@@ -1011,7 +1399,13 @@ fn run_mpd_cover_enrich_albums(
}
};
let covers = enrich::covers_dir();
let covers = match enrich::covers_dir() {
Ok(d) => d,
Err(e) => {
eprintln!("[error] Cover cache unavailable: {}", e);
return;
}
};
eprintln!(
"Extracting covers from MPD for {} album(s)...",
albums.len()
@@ -1029,6 +1423,12 @@ fn run_mpd_cover_enrich_albums(
album.album
);
if mpd_conn.desynced {
eprintln!(" [warn] Lost sync with MPD; aborting cover extraction.");
skipped += albums.len() - i;
break;
}
// Step 1: Ask MPD for any file from this album so we have a path to
// hand to readpicture.
let file_uri = match search_song_for_album(&mut mpd_conn, &album.artist, &album.album) {
@@ -1101,8 +1501,29 @@ 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),
desynced: false,
}
}
fn state(
file: &str,
artist: &str,
@@ -1215,6 +1636,88 @@ 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());
}
#[test]
fn test_read_picture_terminates_when_total_size_never_declared() {
// A server that keeps returning chunks but never declares a total size
// must not loop forever. Two chunks then EOF: the header read fails and
// the transfer is abandoned rather than spinning.
let mut conn = fake_conn(b"size: 0\nbinary: 5\nhello\nOK\nsize: 0\nbinary: 5\nworld\nOK\n");
assert!(read_picture(&mut conn, "music/test.flac").is_none());
}
#[test]
fn test_read_picture_rejects_implausible_size() {
// A bogus `size:` header must not drive a multi-gigabyte allocation.
let mut conn = fake_conn(b"size: 999999999999\nbinary: 4\ndata\nOK\n");
assert!(read_picture(&mut conn, "music/test.flac").is_none());
}
#[test]
fn test_read_chunk_terminator_flags_desync_on_unexpected_line() {
// A line that is neither blank nor OK means we mis-parsed the binary
// payload boundary. The connection must be marked unusable so the
// caller reconnects rather than reading the rest as a fresh response.
let mut conn = fake_conn(b"size: 99\n");
assert!(read_chunk_terminator(&mut conn).is_none());
assert!(
conn.desynced,
"unexpected line must mark the stream desynced"
);
}
#[test]
fn test_read_chunk_terminator_ack_keeps_stream_usable() {
// An ACK terminates the response cleanly, so the connection is fine.
let mut conn = fake_conn(b"ACK [50@0] {readpicture} No file exists\n");
assert!(read_chunk_terminator(&mut conn).is_none());
assert!(!conn.desynced, "an ACK is a normal response terminator");
}
#[test]
fn test_read_response_reports_eof_as_failure() {
// A response that ends without its terminating OK is a lost
// connection, not an empty result set. Returning an empty map here
// would make `currentsong` look like "MPD is stopped".
let mut conn = fake_conn(b"file: a.flac\nTitle: Song\n");
assert!(read_response(&mut conn).is_none());
assert!(conn.desynced);
}
#[test]
fn test_read_response_parses_complete_reply() {
let mut conn = fake_conn(b"file: a.flac\nTitle: Song\nOK\n");
let map = read_response(&mut conn).expect("complete response");
assert_eq!(map.get("file").map(String::as_str), Some("a.flac"));
assert_eq!(map.get("Title").map(String::as_str), Some("Song"));
assert!(!conn.desynced);
}
// -----------------------------------------------------------------------
// dispatch_events
// -----------------------------------------------------------------------

View File

@@ -51,24 +51,44 @@ const MIN_BAR_WIDTH: usize = 8;
/// Format a duration in seconds into a human-readable string.
///
/// Once the duration reaches a full day it is reported in days and hours
/// (minutes are dropped, with the leftover hour rounded to the nearest hour).
///
/// Examples:
/// - 45 seconds → "45s"
/// - 120 seconds → "2m"
/// - 45 seconds → "45s"
/// - 120 seconds → "2m"
/// - 3660 seconds → "1h 01m"
/// - 0 seconds → "0s"
/// - 0 seconds → "0s"
/// - 90000 secs → "1d 1h"
/// - 86400 secs → "1d"
pub fn format_duration(secs: i64) -> String {
const HOUR: i64 = 3600;
const DAY: i64 = 86400;
if secs < 60 {
format!("{}s", secs)
} else if secs < 3600 {
} else if secs < HOUR {
format!("{}m", secs / 60)
} else {
let hours = secs / 3600;
let mins = (secs % 3600) / 60;
} else if secs < DAY {
let hours = secs / HOUR;
let mins = (secs % HOUR) / 60;
if mins == 0 {
format!("{}h", hours)
} else {
format!("{}h {:02}m", hours, mins)
}
} else {
let mut days = secs / DAY;
// Round the leftover hours to the nearest hour, carrying into days.
let mut hours = (secs % DAY + HOUR / 2) / HOUR;
if hours == 24 {
days += 1;
hours = 0;
}
if hours == 0 {
format!("{}d", days)
} else {
format!("{}d {}h", days, hours)
}
}
}
@@ -350,10 +370,14 @@ fn print_box_table(
let shown = truncate_cell(cell, widths[i]);
print!(" {:<width$} ", shown, width = widths[i]);
if bar_max.is_some() && i == ncols - 1 {
// Render bar chart in the extra column.
// Render bar chart in the extra column. `filled` is
// clamped rather than trusted: a caller that passes a
// `bar_max` below the largest row would otherwise make
// `bar_width - filled` underflow and panic.
let val: i64 = cell.parse().unwrap_or(0);
let max = bar_max.unwrap_or(1).max(1);
let filled = ((val as f64 / max as f64) * bar_width as f64).round() as usize;
let ratio = (val as f64 / max as f64).clamp(0.0, 1.0);
let filled = ((ratio * bar_width as f64).round() as usize).min(bar_width);
let empty = bar_width - filled;
print!("{}{} ", "".repeat(filled), "".repeat(empty));
}
@@ -607,7 +631,7 @@ pub fn print_terminal_report(data: &ReportData) {
// --- Top Artists ---
if !data.top_artists.is_empty() {
println!("\n Top Artists");
let max_plays = data.top_artists.first().map(|a| a.plays).unwrap_or(1);
let max_plays = data.top_artists.iter().map(|a| a.plays).max().unwrap_or(1);
let rows: Vec<Vec<String>> = data
.top_artists
.iter()
@@ -632,7 +656,7 @@ pub fn print_terminal_report(data: &ReportData) {
// --- Top Albums ---
if !data.top_albums.is_empty() {
println!("\n Top Albums");
let max_plays = data.top_albums.first().map(|a| a.plays).unwrap_or(1);
let max_plays = data.top_albums.iter().map(|a| a.plays).max().unwrap_or(1);
let rows: Vec<Vec<String>> = data
.top_albums
.iter()
@@ -658,7 +682,9 @@ pub fn print_terminal_report(data: &ReportData) {
// --- Top Genres ---
if !data.top_genres.is_empty() {
println!("\n Top Genres");
let max_plays = data.top_genres.first().map(|g| g.plays).unwrap_or(1);
// Not `first()`: `top_genres` ranks multi-word genres above broad
// single-word ones, so the top row is not necessarily the busiest.
let max_plays = data.top_genres.iter().map(|g| g.plays).max().unwrap_or(1);
let rows: Vec<Vec<String>> = data
.top_genres
.iter()
@@ -683,7 +709,7 @@ pub fn print_terminal_report(data: &ReportData) {
// --- Top Tracks ---
if !data.top_tracks.is_empty() {
println!("\n Top Tracks");
let max_plays = data.top_tracks.first().map(|t| t.plays).unwrap_or(1);
let max_plays = data.top_tracks.iter().map(|t| t.plays).max().unwrap_or(1);
let rows: Vec<Vec<String>> = data
.top_tracks
.iter()
@@ -813,7 +839,7 @@ pub fn render_html_report(conn: &Connection, limit: i64, all_time_limit: i64) ->
h.open("<div class=\"hero\">");
h.line("<h1>Listening Report</h1>");
h.linef(format_args!(
"<div class=\"sub\">Generated by scrbblr on {}</div>",
"<div class=\"sub\">Generated by <a href=\"https://github.com/arturmeski/scrbblr\">scrbblr</a> on {}</div>",
html_escape(&generated_at)
));
h.close("</div>");
@@ -956,7 +982,14 @@ pub fn render_html_report(conn: &Connection, limit: i64, all_time_limit: i64) ->
// We intentionally round the cover count up to a full desktop row so
// the grid doesn't end with a partially filled row when the user picks
// a limit like 20 and desktop layout has 6 columns.
let cover_limit = album_cover_grid_limit(limit);
//
// Use this period's own limit, not the base one: all-time sections
// render a longer top-albums table, and a grid sized from `limit`
// would show fewer covers than the table directly beneath it. It also
// has to match what `albums_needed_for_report` pre-enriched, which is
// likewise computed per period.
let period_limit = if is_all_time { all_time_limit } else { limit };
let cover_limit = album_cover_grid_limit(period_limit);
let cover_albums = db::top_albums(conn, &data.period.name, cover_limit)
.unwrap_or_else(|_| data.top_albums.clone());
if !cover_albums.is_empty() {
@@ -1053,25 +1086,14 @@ pub fn render_html_report(conn: &Connection, limit: i64, all_time_limit: i64) ->
.flatten()
.and_then(|m| m.cover_url);
let cover = resolve_cover(cover_url, &mut cover_files);
// Prepend a subtle source dot to the album name if we know
// the dominant source for this album.
let album_cell = if let Some(src) = a.dominant_source.as_deref() {
if let Some((bg, _)) = source_colours(src, &ordered_sources) {
format!(
"<span style=\"display:inline-block;width:9px;height:9px;\
border-radius:50%;background:{};margin-right:5px;\
vertical-align:middle\"></span>{}",
bg,
html_escape(&a.album)
)
} else {
html_escape(&a.album)
}
} else {
html_escape(&a.album)
};
let source_cell = source_dot_cell(a.dominant_source.as_deref(), &ordered_sources);
BarRow {
cells: vec![(i + 1).to_string(), html_escape(&a.artist), album_cell],
cells: vec![
(i + 1).to_string(),
html_escape(&a.artist),
html_escape(&a.album),
source_cell,
],
value: a.plays,
suffix: format_duration(a.listen_time_secs),
cover,
@@ -1082,7 +1104,7 @@ pub fn render_html_report(conn: &Connection, limit: i64, all_time_limit: i64) ->
write_bar_table(
&mut h,
"Top Albums",
&["#", "Artist", "Album", "Plays", ""],
&["#", "Artist", "Album", "Src", "Plays", ""],
&album_rows,
);
@@ -1146,23 +1168,14 @@ pub fn render_html_report(conn: &Connection, limit: i64, all_time_limit: i64) ->
.flatten()
.and_then(|m| m.cover_url);
let cover = resolve_cover(cover_url, &mut cover_files);
let title_cell = if let Some(src) = t.dominant_source.as_deref() {
if let Some((bg, _)) = source_colours(src, &ordered_sources) {
format!(
"<span style=\"display:inline-block;width:9px;height:9px;\
border-radius:50%;background:{};margin-right:5px;\
vertical-align:middle\"></span>{}",
bg,
html_escape(&t.title)
)
} else {
html_escape(&t.title)
}
} else {
html_escape(&t.title)
};
let source_cell = source_dot_cell(t.dominant_source.as_deref(), &ordered_sources);
BarRow {
cells: vec![(i + 1).to_string(), html_escape(&t.artist), title_cell],
cells: vec![
(i + 1).to_string(),
html_escape(&t.artist),
html_escape(&t.title),
source_cell,
],
value: t.plays,
suffix: format_duration(t.listen_time_secs),
cover,
@@ -1173,7 +1186,7 @@ pub fn render_html_report(conn: &Connection, limit: i64, all_time_limit: i64) ->
write_bar_table(
&mut h,
"Top Tracks",
&["#", "Artist", "Title", "Plays", ""],
&["#", "Artist", "Title", "Src", "Plays", ""],
&track_rows,
);
@@ -1243,6 +1256,12 @@ h1, h2, h3 { margin: 0; }
border-radius: 14px; box-shadow: 0 14px 40px rgba(0,0,0,.25);
}
.sub { margin-top: 6px; color: var(--muted); font-size: 14px; }
.sub a {
color: var(--bar-fill);
text-decoration-color: #a87937;
text-underline-offset: 2px;
}
.sub a:hover { color: #ffd38e; }
.jump-nav {
position: sticky; top: 0; z-index: 20;
margin-top: 12px; padding: 8px;
@@ -1288,7 +1307,7 @@ h1, h2, h3 { margin: 0; }
.kpi .v { margin-top: 4px; font-size: 22px; font-weight: 700; }
section { margin-top: 18px; }
.section-title { font-size: 16px; margin-bottom: 8px; }
.card { background: var(--panel2); border: 1px solid var(--line); border-radius: 10px; overflow: hidden; }
.card { background: var(--panel2); border: 1px solid var(--line); border-radius: 10px; overflow-x: auto; overflow-y: hidden; }
table { width: 100%; border-collapse: collapse; }
th, td { text-align: left; padding: 8px 12px; border-bottom: 1px solid var(--line); }
th { font-size: 11px; color: var(--muted); letter-spacing: .02em; text-transform: uppercase; }
@@ -1301,6 +1320,15 @@ tr:last-child td { border-bottom: none; }
.bar-track { flex: 1; }
.bar { height: 18px; border-radius: 4px; background: var(--bar-fill); min-width: 2px; }
.bar-label { font-size: 12px; color: var(--muted); white-space: nowrap; }
.source-col { width: 38px; min-width: 38px; text-align: center; }
.source-dot {
display: inline-block;
width: 10px;
height: 10px;
border-radius: 50%;
vertical-align: middle;
}
.source-dot-fallback { background: #7a6a55; }
.grid { display: grid; grid-template-columns: repeat(6, minmax(0, 1fr)); gap: 10px; }
.album { position: relative; background: var(--panel2); border: 1px solid var(--line); border-radius: 10px; overflow: hidden; }
.cover { width: 100%; aspect-ratio: 1/1; object-fit: cover; background: #0d1116; }
@@ -1308,8 +1336,9 @@ tr:last-child td { border-bottom: none; }
color: #a99984;
background: linear-gradient(135deg, #17130d, #2c2116); font-size: 12px; }
.pin-dot { position: absolute; top: 5px; right: 5px; width: 22px; height: 22px;
border-radius: 50%; background: rgba(0,0,0,0.45); border: none;
cursor: pointer; font-size: 12px; line-height: 22px; text-align: center;
border-radius: 50%; background: rgba(0,0,0,0.45); border: none; padding: 0;
cursor: pointer; font-size: 12px; line-height: 1;
display: flex; align-items: center; justify-content: center;
opacity: 0; transition: opacity 0.15s; z-index: 1; }
.album:hover .pin-dot { opacity: 1; }
.pin-dot:hover { background: rgba(0,0,0,0.75); }
@@ -1338,6 +1367,11 @@ tr:last-child td { border-bottom: none; }
.wrap { padding: 16px 12px 36px; }
.grid { grid-template-columns: repeat(2, minmax(0, 1fr)); }
.jump-nav { margin-top: 10px; padding: 7px; }
th, td { padding: 7px 8px; }
.source-col { width: 28px; min-width: 28px; }
.bar-cell { width: 32%; }
.bar-wrap { gap: 4px; }
.bar-label { font-size: 11px; }
/* On mobile show rank + duration only; drop the raw play-count and the bar graphic.
The .bar-label (duration text) remains visible inside .bar-cell. */
.play-count { display: none; }
@@ -1358,6 +1392,29 @@ const SOURCE_PALETTE: &[(&str, &str)] = &[
("rgba(220,130,130,0.22)", "rgba(220,130,130,0.60)"), // red
];
/// Render the "Src" table cell for a row: a coloured dot carrying the source
/// name as its tooltip, or `-` when the row predates the `source` column.
///
/// Sources beyond the palette fall back to a neutral dot so the column never
/// silently loses the tooltip. The returned string is HTML and must be placed
/// in a `BarRow` with `raw_cells: true`.
fn source_dot_cell(source: Option<&str>, ordered_sources: &[String]) -> String {
let Some(src) = source else {
return "-".to_string();
};
match source_colours(src, ordered_sources) {
Some((bg, _)) => format!(
"<span class=\"source-dot\" style=\"background:{}\" title=\"{}\"></span>",
bg,
html_attr_escape(src)
),
None => format!(
"<span class=\"source-dot source-dot-fallback\" title=\"{}\"></span>",
html_attr_escape(src)
),
}
}
/// Look up the card background and border colours for a source name, given
/// the ordered list of all-time sources (most played first).
fn source_colours<'a>(source: &str, ordered_sources: &[String]) -> Option<(&'a str, &'a str)> {
@@ -1466,6 +1523,11 @@ impl HtmlWriter {
/// for use in HTML (`covers/<filename>`), and track the source file for
/// copying into the output directory. Returns `None` if the path is missing
/// or the file doesn't exist on disk.
///
/// The existence check gates the returned path as well as the copy list: a
/// cache row whose image has since been deleted must fall through to the
/// "No cover" placeholder rather than emit an `<img>` pointing at a file that
/// was never copied into the output directory.
fn resolve_cover(
abs_path: Option<String>,
cover_files: &mut Vec<std::path::PathBuf>,
@@ -1473,9 +1535,10 @@ fn resolve_cover(
let src = abs_path?;
let src_path = std::path::Path::new(&src);
let filename = src_path.file_name()?;
if src_path.exists() {
cover_files.push(src_path.to_path_buf());
if !src_path.exists() {
return None;
}
cover_files.push(src_path.to_path_buf());
Some(format!("covers/{}", filename.to_string_lossy()))
}
@@ -1532,7 +1595,12 @@ fn write_bar_table(h: &mut HtmlWriter, title: &str, headers: &[&str], rows: &[Ba
// it can be hidden together with the td.play-count cells on mobile.
let play_count_idx = headers.len().saturating_sub(2);
for (i, hdr) in headers.iter().enumerate() {
if i == play_count_idx {
if *hdr == "Src" {
h.linef(format_args!(
"<th class=\"source-col\">{}</th>",
html_escape(hdr)
));
} else if i == play_count_idx {
h.linef(format_args!(
"<th class=\"play-count\">{}</th>",
html_escape(hdr)
@@ -1558,8 +1626,17 @@ fn write_bar_table(h: &mut HtmlWriter, title: &str, headers: &[&str], rows: &[Ba
h.line("<td><span class=\"mini-ph\"></span></td>");
}
}
for cell in &row.cells {
if row.raw_cells {
for (i, cell) in row.cells.iter().enumerate() {
if headers.get(i).is_some_and(|h| *h == "Src") {
if row.raw_cells {
h.linef(format_args!("<td class=\"source-col\">{}</td>", cell));
} else {
h.linef(format_args!(
"<td class=\"source-col\">{}</td>",
html_escape(cell)
));
}
} else if row.raw_cells {
h.linef(format_args!("<td>{}</td>", cell));
} else {
h.linef(format_args!("<td>{}</td>", html_escape(cell)));
@@ -1658,12 +1735,9 @@ fn html_attr_escape(s: &str) -> String {
/// applying the same deprioritisation rule as `top_genres`: multi-word genres
/// rank before single-word broad descriptors ("rock", "electronic", etc.).
fn top_genre_labels(genre_csv: &str, max_labels: usize) -> Vec<String> {
let mut labels: Vec<String> = genre_csv
.split(',')
.map(str::trim)
.filter(|s| !s.is_empty())
.map(|s| s.to_string())
.collect();
// Reuse the parser the genre ranking uses, so a label rejected there (a
// review-site domain, say) can never reappear on an album card.
let mut labels = db::split_genre_list(genre_csv);
// Stable sort so that multi-word genres always precede single-word ones,
// preserving the original MusicBrainz order within each group.
@@ -1708,6 +1782,16 @@ mod tests {
assert_eq!(format_duration(0), "0s");
}
#[test]
fn test_format_duration_days() {
assert_eq!(format_duration(86400), "1d");
assert_eq!(format_duration(90000), "1d 1h");
// 1d 5h 40m rounds up to 1d 6h.
assert_eq!(format_duration(106800), "1d 6h");
// Leftover hours rounding up to 24 carries into the next day.
assert_eq!(format_duration(172799), "2d");
}
#[test]
fn test_format_play_count() {
assert_eq!(format_play_count(0), "0 plays");
@@ -1722,6 +1806,57 @@ mod tests {
assert_eq!(truncate_cell("abcdef", 3), "abc");
}
#[test]
fn test_print_box_table_survives_row_exceeding_bar_max() {
// `top_genres` ranks deprioritised single-word genres last, so the
// first row is not necessarily the busiest. Passing an under-stated
// bar_max used to underflow `bar_width - filled` and panic mid-report.
let rows = vec![
vec![
"1".to_string(),
"alternative metal".to_string(),
"1".to_string(),
],
vec!["2".to_string(), "pop".to_string(), "9".to_string()],
];
// bar_max deliberately smaller than the largest row value.
print_box_table(&["#", "Genre", "Plays"], &rows, Some(1), &[1]);
}
#[test]
fn test_resolve_cover_skips_missing_file() {
let mut files: Vec<std::path::PathBuf> = Vec::new();
// No cover recorded at all.
assert_eq!(resolve_cover(None, &mut files), None);
assert!(files.is_empty());
// A cache row pointing at a file that is no longer on disk must not
// produce an <img> — nothing would be copied next to the HTML, so the
// page would show a broken image instead of the placeholder.
let missing = "/nonexistent/scrbblr/covers/gone.jpg".to_string();
assert_eq!(resolve_cover(Some(missing), &mut files), None);
assert!(
files.is_empty(),
"a missing file must not be queued to copy"
);
}
#[test]
fn test_resolve_cover_accepts_existing_file() {
let dir = std::env::temp_dir().join("scrbblr-test-covers");
std::fs::create_dir_all(&dir).unwrap();
let path = dir.join("present.jpg");
std::fs::write(&path, b"not really a jpeg").unwrap();
let mut files: Vec<std::path::PathBuf> = Vec::new();
let rel = resolve_cover(Some(path.to_string_lossy().to_string()), &mut files);
assert_eq!(rel.as_deref(), Some("covers/present.jpg"));
assert_eq!(files, vec![path.clone()]);
let _ = std::fs::remove_file(&path);
}
#[test]
fn test_album_cover_grid_limit_rounds_to_full_rows() {
assert_eq!(album_cover_grid_limit(20), 24);
@@ -1732,9 +1867,10 @@ mod tests {
#[test]
fn test_top_genre_labels_caps_to_three() {
// "alternative rock" and "industrial" are multi-word rank first.
// "electronic" and "darkwave" are single-word → deprioritised.
// With limit 3 we get both multi-word genres + one single-word.
// "alternative rock" is the only multi-word genre → it ranks first.
// "electronic", "industrial" and "darkwave" are single-word and so are
// deprioritised, keeping their original order among themselves.
// With limit 3 we get the multi-word genre + the first two of those.
let labels = top_genre_labels("alternative rock, electronic, industrial, darkwave", 3);
assert_eq!(
labels,

View File

@@ -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,