Serve Subsonic cover art from a local cache

getCoverArt now always returns an image. Covers come from a stored path
or URL, art embedded in the album's tracks, or the Cover Art Archive, and
are saved under <data_dir>/covers. Remote images are downloaded once
instead of redirecting the client. Albums with no art get a placeholder
image and a marker that skips the lookup for 7 days. Responses carry
Cache-Control so clients stop re-requesting the same covers.

Cargo.toml

@@ -44,7 +44,9 @@ tracing-actix-web = "0.7"
 chrono = { version = "0.4", features = ["serde"] }
 dirs = "6"
 uuid = { version = "1", features = ["v4"] }
+lofty = "0.22"
 
 [dev-dependencies]
 actix-rt = "2"
+tempfile = "3"
 tokio = { version = "1", features = ["full", "test-util"] }

src/cover_art.rs

@@ -0,0 +1,278 @@
+//! Album and artist images for Subsonic clients, cached on disk.
+//!
+//! Each image is resolved once, from a local file, art embedded in the album's
+//! tracks, or a remote URL, and saved under `<data_dir>/covers`. Later requests
+//! are served from that copy. Images that can't be found get a marker file so
+//! the lookup isn't repeated on every request; the marker expires after
+//! [`MISSING_RETRY`].
+
+use std::path::{Path, PathBuf};
+use std::time::{Duration, SystemTime};
+
+use lofty::file::TaggedFileExt;
+use lofty::picture::PictureType;
+
+/// How long a "no image" result is trusted before the sources are tried again.
+pub const MISSING_RETRY: Duration = Duration::from_secs(7 * 24 * 3600);
+
+/// Largest remote image accepted.
+const MAX_REMOTE_BYTES: usize = 10 * 1024 * 1024;
+
+/// Embedded art is read from at most this many tracks of an album.
+const MAX_TRACKS_SCANNED: usize = 5;
+
+/// Where an image can come from, in the order they should be tried.
+pub enum Source {
+    /// An image file on disk, served as-is.
+    File(PathBuf),
+    /// Audio files whose embedded pictures are read.
+    Embedded(Vec<PathBuf>),
+    /// A remote image, downloaded once.
+    Remote(String),
+}
+
+/// Result of resolving an image.
+pub enum Resolved {
+    /// An image file to serve.
+    Found(PathBuf),
+    /// No source had an image.
+    Missing,
+    /// A source failed in a way that may be temporary (network error).
+    Unavailable,
+}
+
+/// Directory holding cached images.
+pub fn cache_dir() -> PathBuf {
+    shanty_config::data_dir().join("covers")
+}
+
+/// Resolve the image for `key` (e.g. `al-12`), using the cache in `dir` first.
+pub async fn resolve(dir: &Path, key: &str, sources: Vec<Source>) -> Resolved {
+    if let Some(path) = cached(dir, key) {
+        return Resolved::Found(path);
+    }
+    let marker = dir.join(format!("{key}.missing"));
+    if marker_is_fresh(&marker) {
+        return Resolved::Missing;
+    }
+
+    let mut transient = false;
+    for source in sources {
+        let bytes = match source {
+            Source::File(path) => {
+                if tokio::fs::metadata(&path).await.is_ok() {
+                    return Resolved::Found(path);
+                }
+                tracing::warn!(path = %path.display(), "cover art file not found");
+                continue;
+            }
+            Source::Embedded(paths) => {
+                match tokio::task::spawn_blocking(move || embedded_picture(&paths)).await {
+                    Ok(Some(bytes)) => bytes,
+                    _ => continue,
+                }
+            }
+            Source::Remote(url) => match fetch(&url).await {
+                Ok(Some(bytes)) => bytes,
+                Ok(None) => continue,
+                Err(e) => {
+                    tracing::warn!(url = %url, error = %e, "cover art download failed");
+                    transient = true;
+                    continue;
+                }
+            },
+        };
+        match store(dir, key, &bytes).await {
+            Ok(path) => return Resolved::Found(path),
+            Err(e) => {
+                tracing::error!(key, error = %e, "failed to cache cover art");
+                return Resolved::Unavailable;
+            }
+        }
+    }
+
+    if transient {
+        return Resolved::Unavailable;
+    }
+    if let Err(e) = write_marker(dir, &marker).await {
+        tracing::warn!(key, error = %e, "failed to write missing-cover marker");
+    }
+    Resolved::Missing
+}
+
+/// Path of a cached image for `key`, if one exists.
+fn cached(dir: &Path, key: &str) -> Option<PathBuf> {
+    ["jpg", "png", "webp", "gif"]
+        .iter()
+        .map(|ext| dir.join(format!("{key}.{ext}")))
+        .find(|p| p.is_file())
+}
+
+fn marker_is_fresh(marker: &Path) -> bool {
+    std::fs::metadata(marker)
+        .and_then(|m| m.modified())
+        .ok()
+        .and_then(|t| SystemTime::now().duration_since(t).ok())
+        .is_some_and(|age| age < MISSING_RETRY)
+}
+
+async fn write_marker(dir: &Path, marker: &Path) -> std::io::Result<()> {
+    tokio::fs::create_dir_all(dir).await?;
+    tokio::fs::write(marker, b"").await
+}
+
+/// Write `bytes` to the cache under `key`, named by image type.
+async fn store(dir: &Path, key: &str, bytes: &[u8]) -> std::io::Result<PathBuf> {
+    tokio::fs::create_dir_all(dir).await?;
+    let ext = image_extension(bytes).unwrap_or("jpg");
+    let path = dir.join(format!("{key}.{ext}"));
+    // Write then rename, so a concurrent request never serves a partial file.
+    let tmp = dir.join(format!(".{key}.{}.tmp", uuid::Uuid::new_v4()));
+    tokio::fs::write(&tmp, bytes).await?;
+    tokio::fs::rename(&tmp, &path).await?;
+    let _ = tokio::fs::remove_file(dir.join(format!("{key}.missing"))).await;
+    Ok(path)
+}
+
+/// File extension for an image, from its leading bytes.
+pub fn image_extension(bytes: &[u8]) -> Option<&'static str> {
+    if bytes.starts_with(&[0xFF, 0xD8, 0xFF]) {
+        Some("jpg")
+    } else if bytes.starts_with(b"\x89PNG") {
+        Some("png")
+    } else if bytes.starts_with(b"GIF8") {
+        Some("gif")
+    } else if bytes.starts_with(b"RIFF") && bytes.get(8..12) == Some(b"WEBP") {
+        Some("webp")
+    } else {
+        None
+    }
+}
+
+/// The front cover embedded in the first of `paths` that has a picture, or any
+/// picture if none is marked as the front cover.
+pub fn embedded_picture(paths: &[PathBuf]) -> Option<Vec<u8>> {
+    paths.iter().take(MAX_TRACKS_SCANNED).find_map(|path| {
+        let file = lofty::read_from_path(path).ok()?;
+        let pictures: Vec<_> = file.tags().iter().flat_map(|t| t.pictures()).collect();
+        let picture = pictures
+            .iter()
+            .find(|p| p.pic_type() == PictureType::CoverFront)
+            .or_else(|| pictures.first())?;
+        let data = picture.data();
+        image_extension(data).map(|_| data.to_vec())
+    })
+}
+
+/// Download an image. `Ok(None)` means the server has no image (a 4xx or a
+/// non-image body); `Err` means the request failed and may work later.
+async fn fetch(url: &str) -> Result<Option<Vec<u8>>, reqwest::Error> {
+    let client = reqwest::Client::builder()
+        .timeout(Duration::from_secs(20))
+        .user_agent(concat!("shanty/", env!("CARGO_PKG_VERSION")))
+        .build()?;
+    let resp = client.get(url).send().await?;
+    if resp.status().is_client_error() {
+        return Ok(None);
+    }
+    let resp = resp.error_for_status()?;
+    let bytes = resp.bytes().await?;
+    if bytes.len() > MAX_REMOTE_BYTES || image_extension(&bytes).is_none() {
+        return Ok(None);
+    }
+    Ok(Some(bytes.to_vec()))
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_image_extension() {
+        assert_eq!(image_extension(&[0xFF, 0xD8, 0xFF, 0xE0]), Some("jpg"));
+        assert_eq!(image_extension(b"\x89PNG\r\n\x1a\n"), Some("png"));
+        assert_eq!(image_extension(b"GIF89a"), Some("gif"));
+        assert_eq!(image_extension(b"RIFF\0\0\0\0WEBPVP8 "), Some("webp"));
+        assert_eq!(image_extension(b"<html>"), None);
+        assert_eq!(image_extension(&[]), None);
+    }
+
+    /// Write a minimal MP3 with an ID3v2 tag holding `pictures`.
+    fn mp3_with_pictures(path: &Path, pictures: Vec<lofty::picture::Picture>) {
+        use lofty::config::WriteOptions;
+        use lofty::tag::{Tag, TagExt, TagType};
+
+        let mut frame = vec![0u8; 417];
+        frame[..4].copy_from_slice(&[0xFF, 0xFB, 0x90, 0x00]);
+        std::fs::write(path, frame.repeat(10)).unwrap();
+
+        let mut tag = Tag::new(TagType::Id3v2);
+        for picture in pictures {
+            tag.push_picture(picture);
+        }
+        tag.save_to_path(path, WriteOptions::default()).unwrap();
+    }
+
+    #[test]
+    fn test_embedded_picture_prefers_front_cover() {
+        use lofty::picture::{MimeType, Picture};
+
+        let dir = tempfile::tempdir().unwrap();
+        let back = b"\x89PNG\r\n\x1a\nback".to_vec();
+        let front = vec![0xFF, 0xD8, 0xFF, 0xE0, 1, 2, 3];
+
+        let bare = dir.path().join("bare.mp3");
+        mp3_with_pictures(&bare, vec![]);
+        let tagged = dir.path().join("tagged.mp3");
+        mp3_with_pictures(
+            &tagged,
+            vec![
+                Picture::new_unchecked(PictureType::CoverBack, Some(MimeType::Png), None, back),
+                Picture::new_unchecked(
+                    PictureType::CoverFront,
+                    Some(MimeType::Jpeg),
+                    None,
+                    front.clone(),
+                ),
+            ],
+        );
+
+        // Tracks without art are skipped.
+        assert_eq!(embedded_picture(&[bare.clone(), tagged]), Some(front));
+        assert_eq!(embedded_picture(&[bare]), None);
+    }
+
+    #[tokio::test]
+    async fn test_resolve_caches_and_marks_missing() {
+        let dir = tempfile::tempdir().unwrap();
+        let png = b"\x89PNG\r\n\x1a\nrest".to_vec();
+        let src = dir.path().join("cover.png");
+        std::fs::write(&src, &png).unwrap();
+
+        // A local file is served from where it is.
+        match resolve(dir.path(), "al-1", vec![Source::File(src.clone())]).await {
+            Resolved::Found(p) => assert_eq!(p, src),
+            _ => panic!("expected local file"),
+        }
+
+        // Nothing available: a marker is written and later lookups skip the sources.
+        let gone = dir.path().join("gone.jpg");
+        assert!(matches!(
+            resolve(dir.path(), "al-2", vec![Source::File(gone)]).await,
+            Resolved::Missing
+        ));
+        assert!(dir.path().join("al-2.missing").is_file());
+        assert!(matches!(
+            resolve(dir.path(), "al-2", vec![Source::File(src)]).await,
+            Resolved::Missing
+        ));
+
+        // A stored image is found again without any sources.
+        let stored = store(dir.path(), "al-3", &png).await.unwrap();
+        assert_eq!(stored, dir.path().join("al-3.png"));
+        match resolve(dir.path(), "al-3", vec![]).await {
+            Resolved::Found(p) => assert_eq!(p, stored),
+            _ => panic!("expected cached file"),
+        }
+    }
+}

src/lib.rs

@@ -7,6 +7,7 @@
 pub mod auth;
 pub mod config;
 pub mod cookie_refresh;
+pub mod cover_art;
 pub mod error;
 pub mod mb_update;
 pub mod monitor;

src/routes/subsonic/media.rs

@@ -1,9 +1,11 @@
 use actix_files::NamedFile;
+use actix_web::http::header;
 use actix_web::{HttpRequest, HttpResponse, web};
 use tokio::process::Command;
 
 use shanty_db::queries;
 
+use crate::cover_art;
 use crate::state::AppState;
 
 use super::SubsonicArgs;
@@ -220,6 +222,9 @@ pub async fn download(
 }
 
 /// /rest/getCoverArt[.view]
+///
+/// Always answers with an image: the cached cover, or a placeholder when there
+/// is none, so clients cache the result instead of asking again.
 pub async fn get_cover_art(
     args: SubsonicArgs,
     req: HttpRequest,
@@ -241,83 +246,98 @@ pub async fn get_cover_art(
         }
     };
 
-    let (prefix, entity_id) = match parse_subsonic_id(id_str) {
-        Some(v) => v,
-        None => {
-            return response::error(&params, response::ERROR_NOT_FOUND, "invalid cover art id");
-        }
+    let Some((prefix, entity_id)) = parse_subsonic_id(id_str) else {
+        return placeholder(PLACEHOLDER_MAX_AGE);
     };
 
-    match prefix {
-        "al" => {
-            let album = match queries::albums::get_by_id(state.db.conn(), entity_id).await {
-                Ok(a) => a,
-                Err(_) => {
-                    return response::error(&params, response::ERROR_NOT_FOUND, "album not found");
-                }
-            };
-
-            if let Some(ref cover_art_path) = album.cover_art_path {
-                if cover_art_path.starts_with("http://") || cover_art_path.starts_with("https://") {
-                    return HttpResponse::TemporaryRedirect()
-                        .append_header(("Location", cover_art_path.as_str()))
-                        .finish();
-                }
-
-                match NamedFile::open_async(cover_art_path).await {
-                    Ok(file) => return file.into_response(&req),
-                    Err(e) => {
-                        tracing::warn!(path = %cover_art_path, error = %e, "cover art file not found");
-                    }
-                }
+    let sources = match prefix {
+        "al" => album_cover_sources(&state, entity_id).await,
+        "ar" => artist_image_sources(&state, entity_id).await,
+        _ => Vec::new(),
+    };
+    let key = format!("{prefix}-{entity_id}");
+    match cover_art::resolve(&cover_art::cache_dir(), &key, sources).await {
+        cover_art::Resolved::Found(path) => match NamedFile::open_async(&path).await {
+            Ok(file) => {
+                let mut resp = file.into_response(&req);
+                resp.headers_mut().insert(
+                    header::CACHE_CONTROL,
+                    header::HeaderValue::from_static("public, max-age=604800"),
+                );
+                resp
             }
-
-            if let Some(ref mbid) = album.musicbrainz_id {
-                let url = format!("https://coverartarchive.org/release/{mbid}/front-250");
-                return HttpResponse::TemporaryRedirect()
-                    .append_header(("Location", url.as_str()))
-                    .finish();
+            Err(e) => {
+                tracing::warn!(path = %path.display(), error = %e, "failed to open cover art");
+                placeholder(TRANSIENT_MAX_AGE)
             }
+        },
+        cover_art::Resolved::Missing => placeholder(PLACEHOLDER_MAX_AGE),
+        cover_art::Resolved::Unavailable => placeholder(TRANSIENT_MAX_AGE),
+    }
+}
 
-            HttpResponse::NotFound().finish()
+/// Client cache lifetime for the placeholder when an item has no image.
+const PLACEHOLDER_MAX_AGE: u32 = 86400;
+/// Client cache lifetime for the placeholder after a failure that may be temporary.
+const TRANSIENT_MAX_AGE: u32 = 600;
+
+fn placeholder(max_age: u32) -> HttpResponse {
+    HttpResponse::Ok()
+        .content_type("image/png")
+        .insert_header((header::CACHE_CONTROL, format!("public, max-age={max_age}")))
+        .body(&include_bytes!("../../../assets/cover-placeholder.png")[..])
+}
+
+/// Where an album's cover can come from: a stored path or URL, art embedded in
+/// its tracks, then the Cover Art Archive.
+async fn album_cover_sources(state: &AppState, album_id: i32) -> Vec<cover_art::Source> {
+    let conn = state.db.conn();
+    let Ok(album) = queries::albums::get_by_id(conn, album_id).await else {
+        return Vec::new();
+    };
+    let mut sources = Vec::new();
+    match album.cover_art_path {
+        Some(url) if url.starts_with("http://") || url.starts_with("https://") => {
+            sources.push(cover_art::Source::Remote(url));
         }
-        "ar" => {
-            let artist = match queries::artists::get_by_id(state.db.conn(), entity_id).await {
-                Ok(a) => a,
-                Err(_) => return HttpResponse::NotFound().finish(),
-            };
-
-            // Look up the cached artist image — same lookup pattern that
-            // browsing::get_artist_info2 uses.
-            let mbid = artist.musicbrainz_id.as_deref().unwrap_or("");
-            if mbid.is_empty() {
-                return HttpResponse::NotFound().finish();
-            }
-            let config = state.config.read().await;
-            let image_source = config.metadata.artist_image_source.clone();
-            drop(config);
-            let key = format!("artist_image:{image_source}:{mbid}");
-            let cached = queries::cache::get(state.db.conn(), &key)
-                .await
-                .ok()
-                .flatten()
-                .filter(|s| !s.is_empty());
+        Some(path) => sources.push(cover_art::Source::File(path.into())),
+        None => {}
+    }
+    if let Ok(tracks) = queries::tracks::get_by_album(conn, album_id).await {
+        let paths = tracks.into_iter().map(|t| t.file_path.into()).collect();
+        sources.push(cover_art::Source::Embedded(paths));
+    }
+    if let Some(mbid) = album.musicbrainz_id {
+        sources.push(cover_art::Source::Remote(format!(
+            "https://coverartarchive.org/release/{mbid}/front-500"
+        )));
+    }
+    sources
+}
 
-            if let Some(url) = cached {
-                if url.starts_with("http://") || url.starts_with("https://") {
-                    HttpResponse::TemporaryRedirect()
-                        .append_header(("Location", url))
-                        .finish()
-                } else {
-                    match NamedFile::open_async(&url).await {
-                        Ok(file) => file.into_response(&req),
-                        Err(_) => HttpResponse::NotFound().finish(),
-                    }
-                }
-            } else {
-                HttpResponse::NotFound().finish()
-            }
+/// Where an artist's image can come from: the URL or path cached by artist
+/// enrichment, the same lookup `browsing::get_artist_info2` uses.
+async fn artist_image_sources(state: &AppState, artist_id: i32) -> Vec<cover_art::Source> {
+    let conn = state.db.conn();
+    let Ok(artist) = queries::artists::get_by_id(conn, artist_id).await else {
+        return Vec::new();
+    };
+    let Some(mbid) = artist.musicbrainz_id.filter(|m| !m.is_empty()) else {
+        return Vec::new();
+    };
+    let image_source = state
+        .config
+        .read()
+        .await
+        .metadata
+        .artist_image_source
+        .clone();
+    let key = format!("artist_image:{image_source}:{mbid}");
+    match queries::cache::get(conn, &key).await.ok().flatten() {
+        Some(url) if url.starts_with("http://") || url.starts_with("https://") => {
+            vec![cover_art::Source::Remote(url)]
         }
-        _ => HttpResponse::NotFound().finish(),
+        Some(path) if !path.is_empty() => vec![cover_art::Source::File(path.into())],
+        _ => Vec::new(),
     }
 }