feat(server/coverart): embedded album art extraction

Adds an embedded-art layer to the album cover chain between the sidecar lookup and the external provider chain. Reads ID3v2 APIC, FLAC METADATA_BLOCK_PICTURE, and MP4 covr atoms via the dhowden/tag library (already a dependency for MBID extraction). Whatever Picard or Lidarr embedded into the file at tag time becomes available without any network call. Diagnostic context: file managers (GNOME Files, Dolphin, etc.) generate audio-file thumbnails by reading exactly this picture block. An album that "has no art" in Minstrel but renders a thumbnail in the file manager is the smoking-gun signal that embedded art is present and we were ignoring it. Adds 'embedded' to the chain so those albums get covered without any external provider hit. Chain after this change: sidecar → embedded → MBCAA → TheAudioDB → Deezer → Last.fm → none Embedded beats every external provider for accuracy because it's the canonical art the operator/Picard chose at tag time, not a guess from a remote catalog. Refactors the post-fetch write logic into Enricher.writeAlbumArt so the embedded layer and the provider chain share the same sidecar-then-managed-cache fallback semantics. Behavior identical to before for the provider path.
2026-05-07 08:05:56 -04:00
parent c53bcb6c99
commit e1d544f59f
3 changed files with 144 additions and 18 deletions
@@ -0,0 +1,45 @@
+package coverart
+
+import (
+	"errors"
+	"fmt"
+	"os"
+
+	"github.com/dhowden/tag"
+)
+
+// ErrNoEmbeddedArt indicates the audio file has no embedded picture.
+// Distinct from "tag read failed" so the enricher can quietly fall
+// through to the provider chain without logging at warn/error level.
+var ErrNoEmbeddedArt = errors.New("coverart: no embedded album art")
+
+// ExtractEmbeddedAlbumArt reads the embedded picture from an audio
+// file's tag block: ID3v2 APIC frames (MP3), FLAC
+// METADATA_BLOCK_PICTURE blocks, MP4 covr atoms (M4A/M4B/AAC), and OGG
+// METADATA_BLOCK_PICTURE base64 in vorbis comments. Whichever format
+// dhowden/tag recognises.
+//
+// Returns ErrNoEmbeddedArt when the tag block parses but contains no
+// usable picture; returns a wrapped error for I/O or tag-parse
+// failures so callers can distinguish "no art" from "couldn't read".
+//
+// Picard- and Lidarr-tagged libraries typically carry the canonical
+// art in this block already — extracting it beats every external API
+// guess for accuracy and avoids a network round-trip.
+func ExtractEmbeddedAlbumArt(path string) ([]byte, error) {
+	f, err := os.Open(path)
+	if err != nil {
+		return nil, fmt.Errorf("open: %w", err)
+	}
+	defer func() { _ = f.Close() }()
+
+	meta, err := tag.ReadFrom(f)
+	if err != nil {
+		return nil, fmt.Errorf("tag read: %w", err)
+	}
+	pic := meta.Picture()
+	if pic == nil || len(pic.Data) == 0 {
+		return nil, ErrNoEmbeddedArt
+	}
+	return pic.Data, nil
+}
@@ -0,0 +1,48 @@
+package coverart
+
+import (
+	"errors"
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func TestExtractEmbeddedAlbumArt_NonexistentFile(t *testing.T) {
+	_, err := ExtractEmbeddedAlbumArt("/nonexistent/path/track.mp3")
+	if err == nil {
+		t.Fatal("err = nil, want non-nil for nonexistent file")
+	}
+	if errors.Is(err, ErrNoEmbeddedArt) {
+		t.Errorf("err is ErrNoEmbeddedArt; want a wrapped open error instead")
+	}
+}
+
+func TestExtractEmbeddedAlbumArt_EmptyFile(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "empty.mp3")
+	if err := os.WriteFile(path, []byte{}, 0o644); err != nil {
+		t.Fatalf("seed: %v", err)
+	}
+	_, err := ExtractEmbeddedAlbumArt(path)
+	if err == nil {
+		t.Fatal("err = nil, want non-nil for empty file (tag.ReadFrom should fail)")
+	}
+	if errors.Is(err, ErrNoEmbeddedArt) {
+		t.Errorf("err is ErrNoEmbeddedArt; want a wrapped tag-read error instead")
+	}
+}
+
+func TestExtractEmbeddedAlbumArt_NonAudioFile(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "not-audio.mp3")
+	if err := os.WriteFile(path, []byte("this is not an audio file"), 0o644); err != nil {
+		t.Fatalf("seed: %v", err)
+	}
+	_, err := ExtractEmbeddedAlbumArt(path)
+	if err == nil {
+		t.Fatal("err = nil, want non-nil for non-audio bytes")
+	}
+	if errors.Is(err, ErrNoEmbeddedArt) {
+		t.Errorf("err is ErrNoEmbeddedArt; want a wrapped tag-read error instead")
+	}
+}
@@ -89,6 +89,29 @@ func (e *Enricher) EnrichAlbum(ctx context.Context, albumID pgtype.UUID) error {
 		}
 	}

+	// Embedded layer: read the picture stored inside the audio file's
+	// tag block (ID3v2 APIC, FLAC METADATA_BLOCK_PICTURE, MP4 covr).
+	// Picard/Lidarr-tagged libraries typically carry the canonical art
+	// here, so this beats every external provider for accuracy and
+	// avoids the network round-trip.
+	if row.TrackFilePath != nil && *row.TrackFilePath != "" {
+		body, eerr := ExtractEmbeddedAlbumArt(*row.TrackFilePath)
+		if eerr == nil {
+			dest, werr := e.writeAlbumArt(albumID, *row.TrackFilePath, body)
+			if werr != nil {
+				e.logger.Error("coverart: embedded write failed; leaving NULL",
+					"album_id", uuidString(albumID), "err", werr)
+				return nil
+			}
+			return e.recordAlbumCover(ctx, albumID, dest, "embedded", currentVersion)
+		}
+		if !errors.Is(eerr, ErrNoEmbeddedArt) {
+			e.logger.Debug("coverart: embedded extract failed; falling through",
+				"album_id", uuidString(albumID), "err", eerr)
+		}
+		// No embedded art (or extract failed): continue to provider chain.
+	}
+
 	// Build the ref. Every provider receives it and decides whether it
 	// can act on the fields available (MBID-only providers return
 	// ErrNotFound when MBID is empty; name-based providers fall back to
@@ -114,26 +137,10 @@ func (e *Enricher) EnrichAlbum(ctx context.Context, albumID pgtype.UUID) error {
 				// crash.
 				return e.recordAlbumCover(ctx, albumID, "", "none", currentVersion)
 			}
-			albumDir := filepath.Dir(*row.TrackFilePath)
-			dest := filepath.Join(albumDir, "cover.jpg")
-			werr := os.WriteFile(dest, body, 0o644)
-			if werr != nil && e.DataDir != "" {
-				// Sidecar location isn't writable (e.g. RO music mount).
-				// Fall back to the managed cache so enrichment isn't
-				// gated on the library being writable.
-				fallbackDir := filepath.Join(e.DataDir, "album-art", uuidString(albumID))
-				if mkErr := os.MkdirAll(fallbackDir, 0o755); mkErr != nil {
-					e.logger.Error("coverart: managed-cache mkdir failed; leaving NULL",
-						"album_id", uuidString(albumID), "fallback_dir", fallbackDir,
-						"sidecar_err", werr, "mkdir_err", mkErr)
-					return nil
-				}
-				dest = filepath.Join(fallbackDir, "cover.jpg")
-				werr = os.WriteFile(dest, body, 0o644)
-			}
+			dest, werr := e.writeAlbumArt(albumID, *row.TrackFilePath, body)
 			if werr != nil {
 				e.logger.Error("coverart: write file failed; leaving NULL",
-					"album_id", uuidString(albumID), "path", dest,
+					"album_id", uuidString(albumID),
 					"provider", provider.ID(), "err", werr)
 				return nil
 			}
@@ -154,6 +161,32 @@ func (e *Enricher) EnrichAlbum(ctx context.Context, albumID pgtype.UUID) error {
 	return nil // at least one transient failure; leave NULL for retry
 }

+// writeAlbumArt persists art bytes for an album, preferring the sidecar
+// location next to the track file. Falls back to the managed cache
+// (<DataDir>/album-art/<id>/cover.jpg) when the sidecar location
+// isn't writable (e.g. read-only music mount in a container) and
+// DataDir is set. Returns the path that was actually written.
+//
+// Used by both the embedded-art layer and the external provider
+// chain — both produce the same dest-and-fallback semantics.
+func (e *Enricher) writeAlbumArt(albumID pgtype.UUID, trackFilePath string, body []byte) (string, error) {
+	albumDir := filepath.Dir(trackFilePath)
+	dest := filepath.Join(albumDir, "cover.jpg")
+	werr := os.WriteFile(dest, body, 0o644)
+	if werr != nil && e.DataDir != "" {
+		fallbackDir := filepath.Join(e.DataDir, "album-art", uuidString(albumID))
+		if mkErr := os.MkdirAll(fallbackDir, 0o755); mkErr != nil {
+			return "", fmt.Errorf("managed-cache mkdir failed (sidecar: %v): %w", werr, mkErr)
+		}
+		dest = filepath.Join(fallbackDir, "cover.jpg")
+		werr = os.WriteFile(dest, body, 0o644)
+	}
+	if werr != nil {
+		return "", fmt.Errorf("write cover: %w", werr)
+	}
+	return dest, nil
+}
+
 // recordAlbumCover writes the new cover_art_path / cover_art_source /
 // cover_art_sources_version atomically.
 func (e *Enricher) recordAlbumCover(ctx context.Context, albumID pgtype.UUID, path, source string, version int32) error {