fix(artwork): run disc resolution for single-disc albums too

ed4178a6 gated serveDisc on len(album.Discs) > 1, claiming parity with the legacy
reader. The legacy reader has no such gate: artwork.go dispatches every dc- id to
newDiscArtworkReader, whose Reader() walks DiscArtPriority unconditionally.

The gate also lost art. For a single-disc album whose only image is disc1.jpg,
the disc request skipped the chain and fell through to album art, which does not
match CoverArtPriority — so tracks tagged disc 1 (whose CoverArtID is a dc- id)
served nothing at all, where before they served disc1.jpg.

A single disc can legitimately have its own cover, distinct from the album's, and
DiscArtPriority is what expresses that preference. Drop the gate and restore the
single-disc e2e scenarios that covered it.
This commit is contained in:
Deluan 2026-07-24 21:40:51 -04:00
parent 0064de7bd5
commit eb283b1646
9 changed files with 463 additions and 195 deletions

View File

@ -5,6 +5,7 @@ import (
"errors"
"io"
"os"
"time"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/conf/configtest"
@ -211,6 +212,73 @@ var _ = Describe("Acquisition → serve loop", func() {
Expect(readAll(resolved)).To(Equal(provisionalBytes))
})
It("stores dimensions, mime and a real blurhash alongside the acquired bytes", func() {
seedFolderAlbum("al1")
worker.Bump("al", "al1")
runWorkerUntil(ctx, worker, itemFound(model.KindAlbumArtwork, "al1"))
ia, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "al1", model.ImageTypePrimary)
Expect(err).ToNot(HaveOccurred())
art, err := artRepo.GetImage(ia.Hash)
Expect(err).ToNot(HaveOccurred())
Expect(art.Mime).To(Equal("image/jpeg"))
Expect(art.Width).To(BeNumerically(">", 0))
Expect(art.Height).To(BeNumerically(">", 0))
Expect(art.SizeBytes).To(BeNumerically("==", len(coverBytes)))
// Never a synthesized value: the blurhash is encoded from the real pixels.
Expect(art.BlurHash).ToNot(BeEmpty())
})
It("deduplicates byte-identical art across entities onto one image row", func() {
folderRepo.result = []model.Folder{{Path: albumFolderPath, ImageFiles: []string{"cover.jpg"}}}
albumRepo.SetData(model.Albums{
{ID: "al1", Name: "Album", FolderIDs: []string{"f1"}, LibraryID: 0},
{ID: "al2", Name: "Same Cover", FolderIDs: []string{"f1"}, LibraryID: 0},
})
worker.Bump("al", "al1")
worker.Bump("al", "al2")
runWorkerUntil(ctx, worker, func() bool {
return itemFound(model.KindAlbumArtwork, "al1")() && itemFound(model.KindAlbumArtwork, "al2")()
})
ia1, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "al1", model.ImageTypePrimary)
Expect(err).ToNot(HaveOccurred())
ia2, err := artRepo.GetItemArtwork(model.KindAlbumArtwork, "al2", model.ImageTypePrimary)
Expect(err).ToNot(HaveOccurred())
Expect(ia1.Hash).To(Equal(ia2.Hash), "identical bytes must share one content hash")
Expect(readAll(mustGet(svc.Get(ctx, model.MustParseArtworkID("al-al2"), 0, false)))).To(Equal(coverBytes))
})
It("stops serving a file-backed image once its source file changes underneath", func() {
name := writeUpload(consts.EntityRadio, "radio-stale.jpg", coverFixture)
radioRepo.Data["ra1"] = &model.Radio{ID: "ra1", Name: "Station", UploadedImage: name}
worker.Bump("ra", "ra1")
runWorkerUntil(ctx, worker, itemFound(model.KindRadioArtwork, "ra1"))
ia, err := artRepo.GetItemArtwork(model.KindRadioArtwork, "ra1", model.ImageTypePrimary)
Expect(err).ToNot(HaveOccurred())
staleHash := ia.Hash
// Replace the backing file with different bytes and a newer mtime.
path := model.UploadedImagePath(consts.EntityRadio, name)
Expect(os.WriteFile(path, readFixture(artistPngFixture), 0o600)).To(Succeed())
newer := time.Now().Add(2 * time.Second)
Expect(os.Chtimes(path, newer, newer)).To(Succeed())
// The mtime no longer matches the state row, so the old hash's bytes are never served.
_, err = svc.Get(ctx, model.MustParseArtworkID("ra-ra1"), 0, false)
Expect(err).To(MatchError(artwork.ErrUnavailable))
// That read enqueued a re-resolution; draining it republishes the new bytes.
runWorkerUntil(ctx, worker, func() bool {
cur, gerr := artRepo.GetItemArtwork(model.KindRadioArtwork, "ra1", model.ImageTypePrimary)
return gerr == nil && cur.Hash != "" && cur.Hash != staleHash
})
img, err := svc.Get(ctx, model.MustParseArtworkID("ra-ra1"), 0, false)
Expect(err).ToNot(HaveOccurred())
Expect(readAll(img)).To(Equal(readFixture(artistPngFixture)))
})
It("records an absent state for an entity with no art and reports it unavailable", func() {
albumRepo.SetData(model.Albums{{ID: "alx", Name: "Artless", LibraryID: 0}})
worker.Bump("al", "alx")
@ -224,3 +292,10 @@ var _ = Describe("Acquisition → serve loop", func() {
Expect(img.Placeholder).To(BeTrue())
})
})
// mustGet unwraps a Service.Get result for inline byte assertions.
func mustGet(img *artwork.Image, err error) *artwork.Image {
GinkgoHelper()
Expect(err).ToNot(HaveOccurred())
return img
}

View File

@ -354,8 +354,10 @@ var _ = Describe("Album artwork resolution", func() {
})
scan()
expectAbsent(albumByName("Album A"))
// Album B first: a drain settles every ready item, and folder art is only
// byte-servable while the album still has no state row.
expectFolderCover(albumByName("Album B"), "Artist/Album B/cover.jpg")
expectAbsent(albumByName("Album A"))
})
})
@ -387,6 +389,48 @@ var _ = Describe("Album artwork resolution", func() {
})
})
// albumRootParent refuses the library root as an album root (parent.ParentID == ""), so a
// stray image at the top of the library never becomes some album's cover.
When("a multi-disc album sits directly at the library root with a cover.jpg beside it", func() {
// (library root)
// ├── cover.jpg ← must NOT be adopted: the root is never an album root
// ├── CD1/
// │ └── 01 - Track.mp3
// └── CD2/
// └── 01 - Track.mp3
It("does not adopt the library-root image as album art", func() {
conf.Server.CoverArtPriority = defaultCoverPriority
setLayout(fstest.MapFS{
"cover.jpg": smallPNG("library-root"),
"CD1/01 - Track.mp3": trackFile(1, "T1", map[string]any{"album": "Rootless", "disc": "1"}),
"CD2/01 - Track.mp3": trackFile(1, "T2", map[string]any{"album": "Rootless", "disc": "2"}),
})
scan()
expectAbsent(firstAlbum())
})
})
// compareImageFiles prefers shallower paths on a basename tie, so an artist-folder cover.jpg
// would outrank the album's own if the parent folder were ever considered here. It is not:
// albumRootParent skips the parent for a single-folder album that has images of its own.
When("a single-folder album has its own cover.jpg and the artist folder has one too", func() {
// Artist/
// ├── cover.jpg ← shallower, but must NOT win
// └── Album/
// ├── 01 - Track.mp3
// └── cover.jpg ← should win (the album has images of its own)
It("prefers the album's own cover over the shallower artist-folder cover", func() {
conf.Server.CoverArtPriority = defaultCoverPriority
setLayout(fstest.MapFS{
"Artist/cover.jpg": smallPNG("artist-image"),
"Artist/Album/01 - Track.mp3": trackFile(1, "Track", map[string]any{"albumartist": "Artist"}),
"Artist/Album/cover.jpg": smallPNG("album-own"),
})
scan()
expectFolderCover(firstAlbum(), "Artist/Album/cover.jpg")
})
})
When("a spread album has its own front.jpg but the artist folder has cover.jpg", func() {
// Artist/
// ├── cover.jpg ← artist image; matches cover.* (first pattern),

View File

@ -8,6 +8,7 @@ import (
"github.com/Masterminds/squirrel"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/consts"
"github.com/navidrome/navidrome/core/artwork"
"github.com/navidrome/navidrome/model"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
@ -73,22 +74,6 @@ var _ = Describe("Artist artwork resolution", func() {
})
})
When("ArtistArtPriority uses album/<arbitrary pattern> (not just album/artist.*)", func() {
// Artist/
// └── Album/
// ├── 01 - Track.mp3
// └── artist.jpg ← matched by album/artist.*
It("resolves the pattern against the artist's album image files", func() {
conf.Server.ArtistArtPriority = "album/artist.*, external"
setLayout(fstest.MapFS{
"Artist/Album/01 - Track.mp3": trackFile(1, "Track", map[string]any{"albumartist": "Artist"}),
"Artist/Album/artist.jpg": smallPNG("album-artist"),
})
scan()
expectArtistFolder(soleArtist(), "Artist/Album/artist.jpg")
})
})
When("an artist has an uploaded image and a matching artist.* file", func() {
// <DataFolder>/
// └── artwork/
@ -119,6 +104,48 @@ var _ = Describe("Artist artwork resolution", func() {
})
})
When("ArtistArtPriority uses album/<arbitrary pattern> (not just album/artist.*)", func() {
// Artist/
// └── Album/
// ├── 01 - Track.mp3
// └── artist.jpg ← matched by album/artist.*
It("resolves the pattern against the artist's album image files", func() {
conf.Server.ArtistArtPriority = "album/artist.*, external"
setLayout(fstest.MapFS{
"Artist/Album/01 - Track.mp3": trackFile(1, "Track", map[string]any{"albumartist": "Artist"}),
"Artist/Album/artist.jpg": smallPNG("album-artist"),
})
scan()
expectArtistFolder(soleArtist(), "Artist/Album/artist.jpg")
})
})
// resolveArtist only samples albums where this artist is the SOLE album artist, so a
// collaboration or compilation never donates its images as the artist's own.
When("the artist's only album is credited to two album artists", func() {
// Artist/
// └── Collab Album/ (album artists: "Artist" + a collaborator)
// ├── 01 - Track.mp3
// └── artist.jpg ← must NOT become the artist image
It("ignores the album's images and settles absent", func() {
conf.Server.ArtistArtPriority = "album/artist.*"
// " / " is a default artists split separator, so this single tag yields two album artists.
setLayout(fstest.MapFS{
"Artist/Collab Album/01 - Track.mp3": trackFile(1, "Track", map[string]any{"albumartist": "Artist / Collaborator"}),
"Artist/Collab Album/artist.jpg": smallPNG("collab-artist"),
})
scan()
Expect(firstAlbum().Participants[model.RoleAlbumArtist]).To(HaveLen(2),
"sanity check: the album must be credited to two album artists")
ar := soleArtist()
ia := acquire(model.KindArtistArtwork, ar.ID)
Expect(ia.Hash).To(BeEmpty())
Expect(serveErr(model.NewArtworkID(model.KindArtistArtwork, ar.ID, nil))).
To(MatchError(artwork.ErrUnavailable))
})
})
When("ArtistArtPriority starts with image-folder and ArtistImageFolder has a name-matching image", func() {
// <ArtistImageFolder>/
// └── Artist.jpg ← matched by artist name (image-folder source)

View File

@ -11,14 +11,79 @@ import (
)
// Disc art is a serve-time read through the library FS (no worker state row), so per-disc images
// are asserted byte-for-byte. Only multi-disc albums use the disc chain; a single-disc album serves
// its album art directly (a deliberate change from the legacy reader), so those cases are covered by
// the album suite. Album-root covers here are folder-backed and asserted on the album state row.
// are asserted byte-for-byte. Single-disc albums run the disc chain too — a disc can carry art
// distinct from the album cover. Album-root covers are folder-backed and asserted on the state row.
var _ = Describe("Disc artwork resolution", func() {
BeforeEach(func() {
setupResolutionHarness()
})
When("the album is single-disc with a disc1.jpg in the only folder", func() {
// Artist/
// └── Album/
// ├── 01 - Track.mp3
// └── disc1.jpg ← matched by disc*.*
It("returns the disc1.jpg image (matched as disc*.*)", func() {
conf.Server.DiscArtPriority = "disc*.*, cd*.*, cover.*, folder.*, front.*, embedded"
setLayout(fstest.MapFS{
"Artist/Album/01 - Track.mp3": trackFile(1, "Track"),
"Artist/Album/disc1.jpg": smallPNG("disc1-image"),
})
scan()
expectDiscImage(firstAlbum(), 1, "disc1-image")
})
})
When("the album has no per-disc image and no album cover", func() {
// Artist/
// └── Album/
// └── 01 - Track.mp3 (no disc or album art — nothing to serve)
It("reports the disc lookup as unavailable", func() {
conf.Server.DiscArtPriority = "disc*.*, cd*.*"
conf.Server.CoverArtPriority = "cover.*, folder.*"
setLayout(fstest.MapFS{
"Artist/Album/01 - Track.mp3": trackFile(1, "Track"),
})
scan()
Expect(serveErr(discArtID(firstAlbum(), 1))).To(MatchError(artwork.ErrUnavailable))
})
})
When("the album has no per-disc image but has an album cover", func() {
// Artist/
// └── Album/
// ├── 01 - Track.mp3
// └── cover.jpg ← album-level fallback (no disc art present)
It("falls back to the album cover", func() {
conf.Server.DiscArtPriority = "disc*.*, cd*.*"
conf.Server.CoverArtPriority = defaultCoverPriority
setLayout(fstest.MapFS{
"Artist/Album/01 - Track.mp3": trackFile(1, "Track"),
"Artist/Album/cover.jpg": smallPNG("album-cover"),
})
scan()
expectDiscImage(firstAlbum(), 1, "album-cover")
})
})
When("multiple disc images exist in the same folder (disc1 vs disc10)", func() {
// Artist/
// └── Album/
// ├── 01 - Track.mp3
// ├── disc1.jpg ← matches request for disc 1
// └── disc10.jpg
It("matches the requested disc number, not a higher-numbered one", func() {
conf.Server.DiscArtPriority = "disc*.*"
setLayout(fstest.MapFS{
"Artist/Album/01 - Track.mp3": trackFile(1, "Track"),
"Artist/Album/disc1.jpg": smallPNG("disc-one"),
"Artist/Album/disc10.jpg": smallPNG("disc-ten"),
})
scan()
expectDiscImage(firstAlbum(), 1, "disc-one")
})
})
When("a multi-disc album has per-disc covers", func() {
// Artist/
// └── Album/
@ -41,95 +106,6 @@ var _ = Describe("Disc artwork resolution", func() {
})
})
When("multiple disc images exist in the same folder (disc1 vs disc10)", func() {
// Artist/
// └── Album/
// ├── 01 - Track.mp3 (disc 1)
// ├── 02 - Track.mp3 (disc 10)
// ├── disc1.jpg ← matches request for disc 1
// └── disc10.jpg
It("matches the requested disc number, not a higher-numbered one", func() {
conf.Server.DiscArtPriority = "disc*.*"
setLayout(fstest.MapFS{
"Artist/Album/01 - Track.mp3": trackFile(1, "T1", map[string]any{"disc": "1"}),
"Artist/Album/02 - Track.mp3": trackFile(2, "T10", map[string]any{"disc": "10"}),
"Artist/Album/disc1.jpg": smallPNG("disc-one"),
"Artist/Album/disc10.jpg": smallPNG("disc-ten"),
})
scan()
expectDiscImage(firstAlbum(), 1, "disc-one")
})
})
When("a multi-disc album has no per-disc image but has an album cover", func() {
// Artist/
// └── Album/
// ├── CD1/
// │ └── 01 - Track.mp3
// ├── CD2/
// │ └── 01 - Track.mp3
// └── cover.jpg ← album-level fallback (no disc art present)
It("falls back to the album cover", func() {
conf.Server.DiscArtPriority = "disc*.*, cd*.*"
conf.Server.CoverArtPriority = defaultCoverPriority
setLayout(fstest.MapFS{
"Artist/Album/CD1/01 - Track.mp3": trackFile(1, "T1", map[string]any{"disc": "1"}),
"Artist/Album/CD2/01 - Track.mp3": trackFile(1, "T2", map[string]any{"disc": "2"}),
"Artist/Album/cover.jpg": smallPNG("album-cover"),
})
scan()
expectDiscImage(firstAlbum(), 1, "album-cover")
})
})
When("a multi-disc album has no per-disc image and no album cover", func() {
// Artist/
// └── Album/
// ├── CD1/
// │ └── 01 - Track.mp3
// └── CD2/
// └── 01 - Track.mp3 (no images anywhere — nothing to serve)
It("reports the disc lookup as unavailable", func() {
conf.Server.DiscArtPriority = "disc*.*, cd*.*"
conf.Server.CoverArtPriority = "cover.*, folder.*"
setLayout(fstest.MapFS{
"Artist/Album/CD1/01 - Track.mp3": trackFile(1, "T1", map[string]any{"disc": "1"}),
"Artist/Album/CD2/01 - Track.mp3": trackFile(1, "T2", map[string]any{"disc": "2"}),
})
scan()
Expect(serveErr(discArtID(firstAlbum(), 1))).To(MatchError(artwork.ErrUnavailable))
})
})
When("DiscArtPriority is the empty string", func() {
// Artist/
// └── Album/
// ├── CD1/
// │ ├── 01 - Track.mp3
// │ └── disc1.jpg (ignored — DiscArtPriority is empty)
// ├── CD2/
// │ ├── 01 - Track.mp3
// │ └── cd2.png (ignored — DiscArtPriority is empty)
// └── cover.jpg ← used for every disc (album-level fallback)
It("skips every disc-level source and returns the album cover", func() {
conf.Server.DiscArtPriority = ""
conf.Server.CoverArtPriority = defaultCoverPriority
setLayout(fstest.MapFS{
"Artist/Album/CD1/01 - Track.mp3": trackFile(1, "T1", map[string]any{"disc": "1"}),
"Artist/Album/CD2/01 - Track.mp3": trackFile(1, "T2", map[string]any{"disc": "2"}),
"Artist/Album/CD1/disc1.jpg": smallPNG("disc-1"),
"Artist/Album/CD2/cd2.png": smallPNG("cd-2"),
"Artist/Album/cover.jpg": smallPNG("album-cover"),
})
scan()
al := firstAlbum()
for _, n := range []int{1, 2} {
expectDiscImage(al, n, "album-cover")
}
})
})
// Doc scenarios from:
// https://www.navidrome.org/docs/usage/library/artwork/#disc-cover-art
// Default DiscArtPriority is "disc*.*, cd*.*, cover.*, folder.*, front.*, discsubtitle, embedded".
@ -177,22 +153,55 @@ var _ = Describe("Disc artwork resolution", func() {
})
})
When("DiscArtPriority is the empty string", func() {
// Artist/
// └── Album/
// ├── CD1/
// │ ├── 01 - Track.mp3
// │ └── disc1.jpg (ignored — DiscArtPriority is empty)
// ├── CD2/
// │ ├── 01 - Track.mp3
// │ └── cd2.png (ignored — DiscArtPriority is empty)
// └── cover.jpg ← used for every disc (album-level fallback)
It("skips every disc-level source and returns the album cover", func() {
conf.Server.DiscArtPriority = ""
conf.Server.CoverArtPriority = defaultCoverPriority
setLayout(fstest.MapFS{
"Artist/Album/CD1/01 - Track.mp3": trackFile(1, "T1", map[string]any{"disc": "1"}),
"Artist/Album/CD2/01 - Track.mp3": trackFile(1, "T2", map[string]any{"disc": "2"}),
"Artist/Album/CD1/disc1.jpg": smallPNG("disc-1"),
"Artist/Album/CD2/cd2.png": smallPNG("cd-2"),
"Artist/Album/cover.jpg": smallPNG("album-cover"),
})
scan()
al := firstAlbum()
for _, n := range []int{1, 2} {
expectDiscImage(al, n, "album-cover")
}
})
})
When("the documented multi-disc layout is used (disc1.jpg + cd2.png + album-root cover.jpg)", func() {
// Artist/
// └── Album/
// ├── disc1/
// │ ├── disc1.jpg ← matched by disc*.* for disc 1
// │ └── 01 - Track.mp3
// │ ├── 01 - Track.mp3
// │ └── 02 - Track.mp3
// ├── disc2/
// │ ├── cd2.png ← matched by cd*.* for disc 2
// │ └── 01 - Track.mp3
// └── cover.jpg ← album-level cover
It("matches the per-disc image for each disc and the album-root cover for the album", func() {
// │ ├── 01 - Track.mp3
// │ └── 02 - Track.mp3
// └── cover.jpg (album-level fallback, unused here)
It("matches the per-disc image for each disc", func() {
conf.Server.DiscArtPriority = defaultDiscPriority
conf.Server.CoverArtPriority = defaultCoverPriority
setLayout(fstest.MapFS{
"Artist/Album/disc1/01 - Track.mp3": trackFile(1, "T1", map[string]any{"disc": "1"}),
"Artist/Album/disc1/02 - Track.mp3": trackFile(2, "T2", map[string]any{"disc": "1"}),
"Artist/Album/disc2/01 - Track.mp3": trackFile(1, "T3", map[string]any{"disc": "2"}),
"Artist/Album/disc2/02 - Track.mp3": trackFile(2, "T4", map[string]any{"disc": "2"}),
"Artist/Album/disc1/disc1.jpg": smallPNG("disc-1"),
"Artist/Album/disc2/cd2.png": smallPNG("cd-2"),
"Artist/Album/cover.jpg": smallPNG("album-root"),
@ -202,47 +211,72 @@ var _ = Describe("Disc artwork resolution", func() {
al := firstAlbum()
expectDiscImage(al, 1, "disc-1")
expectDiscImage(al, 2, "cd-2")
expectAlbumFolderCover(al, "Artist/Album/cover.jpg")
})
})
When("discsubtitle keyword matches an image whose stem equals the disc's subtitle", func() {
// Artist/
// └── Album/
// ├── CD1/
// │ └── 01 - Track.mp3 (discsubtitle="Bonus Tracks")
// ├── CD2/
// │ └── 01 - Track.mp3
// └── Bonus Tracks.jpg ← matched by "discsubtitle" keyword for disc 1
// ├── 01 - Track.mp3 (discsubtitle="Bonus Tracks")
// └── Bonus Tracks.jpg ← matched by "discsubtitle" keyword
It("selects the subtitle-named image", func() {
conf.Server.DiscArtPriority = "discsubtitle"
setLayout(fstest.MapFS{
"Artist/Album/CD1/01 - Track.mp3": trackFile(1, "T1", map[string]any{"disc": "1", "discsubtitle": "Bonus Tracks"}),
"Artist/Album/CD2/01 - Track.mp3": trackFile(1, "T2", map[string]any{"disc": "2"}),
"Artist/Album/Bonus Tracks.jpg": smallPNG("bonus-tracks"),
"Artist/Album/01 - Track.mp3": trackFile(1, "T1", map[string]any{"disc": "1", "discsubtitle": "Bonus Tracks"}),
"Artist/Album/Bonus Tracks.jpg": smallPNG("bonus-tracks"),
})
scan()
expectDiscImage(firstAlbum(), 1, "bonus-tracks")
})
})
When("discsubtitle is set but no image filename matches the subtitle", func() {
// Artist/
// └── Album/
// ├── CD1/
// │ ├── 01 - Track.mp3 (discsubtitle="Bonus Tracks")
// │ └── cover.jpg ← wins (discsubtitle has no match, falls through)
// └── CD2/
// └── 01 - Track.mp3
It("falls through to the next priority entry", func() {
conf.Server.DiscArtPriority = "discsubtitle, cover.*"
setLayout(fstest.MapFS{
"Artist/Album/CD1/01 - Track.mp3": trackFile(1, "T1", map[string]any{"disc": "1", "discsubtitle": "Bonus Tracks"}),
"Artist/Album/CD2/01 - Track.mp3": trackFile(1, "T2", map[string]any{"disc": "2"}),
"Artist/Album/CD1/cover.jpg": smallPNG("disc1-cover"),
})
// Reproduces https://github.com/navidrome/navidrome/issues/5456
// Deeply nested layout matching the reporter's actual structure.
When("a deeply nested multi-disc album has cover.jpg and per-disc folder.jpg", func() {
// Genre/Artist/Album/ ← album root with cover.jpg
// ├── cover.jpg ← album-level cover
// ├── Disc 01 (Subtitle)/
// │ ├── 01 - Track.mp3
// │ └── folder.jpg ← disc 1 art
// ├── Disc 02 (Subtitle)/
// │ ├── 01 - Track.mp3
// │ └── folder.jpg
// └── ... (12 discs)
It("uses album-root cover.jpg for album art and per-disc folder.jpg for each disc", func() {
conf.Server.DiscArtPriority = defaultDiscPriority
conf.Server.CoverArtPriority = defaultCoverPriority
discNames := []string{
"Disc 01 (Birth of the Dead - The Studio Sides)",
"Disc 02 (Birth of the Dead - The Live Sides)",
"Disc 03 (The Grateful Dead)",
"Disc 04 (Anthem of the Sun)",
"Disc 05 (Aoxomoxoa)",
"Disc 06 (Live; Dead)",
"Disc 07 (Workingman's Dead)",
"Disc 08 (American Beauty)",
"Disc 09 (Grateful Dead)",
"Disc 10 (Europe '72)",
"Disc 11 (Europe '72)",
"Disc 12 (History of the Grateful Dead, Volume One (Bear's Choice))",
}
layout := fstest.MapFS{
"Pop; Rock/Grateful Dead/(2001) The Golden Road/cover.jpg": smallPNG("album-root-cover"),
}
for i, name := range discNames {
discNum := i + 1
prefix := fmt.Sprintf("Pop; Rock/Grateful Dead/(2001) The Golden Road/%s/", name)
layout[prefix+"01 - Track.mp3"] = trackFile(1, fmt.Sprintf("T%d", discNum), map[string]any{"disc": fmt.Sprintf("%d", discNum)})
layout[prefix+"folder.jpg"] = smallPNG(fmt.Sprintf("disc-%02d-folder", discNum))
}
setLayout(layout)
scan()
expectDiscImage(firstAlbum(), 1, "disc1-cover")
al := firstAlbum()
expectAlbumFolderCover(al, "(2001) The Golden Road/cover.jpg")
for i := range discNames {
discNum := i + 1
expectDiscImage(al, discNum, fmt.Sprintf("disc-%02d-folder", discNum))
}
})
})
@ -282,47 +316,19 @@ var _ = Describe("Disc artwork resolution", func() {
})
})
// Reproduces https://github.com/navidrome/navidrome/issues/5456
// Deeply nested layout matching the reporter's actual structure.
When("a deeply nested multi-disc album has cover.jpg and per-disc folder.jpg", func() {
// Pop; Rock/Grateful Dead/(2001) The Golden Road/ ← album root with cover.jpg
// ├── cover.jpg ← album-level cover
// ├── Disc 01 (Subtitle)/
// │ ├── 01 - Track.mp3
// │ └── folder.jpg ← disc 1 art
// ├── Disc 02 (Subtitle)/
// │ ├── 01 - Track.mp3
// │ └── folder.jpg
// └── ... (6 discs)
It("uses album-root cover.jpg for album art and per-disc folder.jpg for each disc", func() {
conf.Server.DiscArtPriority = defaultDiscPriority
conf.Server.CoverArtPriority = defaultCoverPriority
discNames := []string{
"Disc 01 (Birth of the Dead - The Studio Sides)",
"Disc 02 (Birth of the Dead - The Live Sides)",
"Disc 03 (The Grateful Dead)",
"Disc 04 (Anthem of the Sun)",
"Disc 05 (Aoxomoxoa)",
"Disc 06 (Live; Dead)",
}
layout := fstest.MapFS{
"Pop; Rock/Grateful Dead/(2001) The Golden Road/cover.jpg": smallPNG("album-root-cover"),
}
for i, name := range discNames {
discNum := i + 1
prefix := fmt.Sprintf("Pop; Rock/Grateful Dead/(2001) The Golden Road/%s/", name)
layout[prefix+"01 - Track.mp3"] = trackFile(1, fmt.Sprintf("T%d", discNum), map[string]any{"disc": fmt.Sprintf("%d", discNum)})
layout[prefix+"folder.jpg"] = smallPNG(fmt.Sprintf("disc-%02d-folder", discNum))
}
setLayout(layout)
When("discsubtitle is set but no image filename matches the subtitle", func() {
// Artist/
// └── Album/
// ├── 01 - Track.mp3 (discsubtitle="Bonus Tracks")
// └── cover.jpg ← wins (discsubtitle has no match, falls through)
It("falls through to the next priority entry", func() {
conf.Server.DiscArtPriority = "discsubtitle, cover.*"
setLayout(fstest.MapFS{
"Artist/Album/01 - Track.mp3": trackFile(1, "T1", map[string]any{"disc": "1", "discsubtitle": "Bonus Tracks"}),
"Artist/Album/cover.jpg": smallPNG("cover"),
})
scan()
al := firstAlbum()
expectAlbumFolderCover(al, "(2001) The Golden Road/cover.jpg")
for i := range discNames {
discNum := i + 1
expectDiscImage(al, discNum, fmt.Sprintf("disc-%02d-folder", discNum))
}
expectDiscImage(firstAlbum(), 1, "cover")
})
})
})

View File

@ -111,6 +111,29 @@ var _ = Describe("MediaFile artwork resolution", func() {
Expect(storedBytes(ia)).To(Equal(embeddedArtBytes))
})
})
When("EnableMediaFileCoverArt is turned off after the track was scanned", func() {
// Artist/
// └── Album/
// ├── 01 - Track.mp3 ← has embedded picture (must NOT be served)
// └── cover.jpg ← wins (per-track art disabled at serve time)
It("serves the album cover instead of the track's embedded art", func() {
conf.Server.CoverArtPriority = defaultCoverPriority
setLayout(fstest.MapFS{
"Artist/Album/01 - Track.mp3": trackFile(1, "Track", map[string]any{"has_picture": "true"}),
"Artist/Album/cover.jpg": smallPNG("album-cover"),
})
scan()
replaceWithRealMP3("Artist/Album/01 - Track.mp3")
// The setting is not part of the artwork fingerprint, so a direct mf- request must
// honor it at serve time rather than serving previously-eligible embedded art.
conf.Server.EnableMediaFileCoverArt = false
mf := mediafileOn("Artist/Album/01 - Track.mp3")
trackArtID := model.NewArtworkID(model.KindMediaFileArtwork, mf.ID, nil)
Expect(serveBytes(trackArtID)).To(Equal(pngBytes("album-cover")))
})
})
})
func mediafileOn(relPath string) model.MediaFile {

View File

@ -1,6 +1,7 @@
package e2e
import (
"image/color"
"os"
"path/filepath"
"testing/fstest"
@ -153,6 +154,54 @@ var _ = Describe("Playlist artwork resolution", func() {
data := storedBytes(ia)
// The tiled cover is a PNG-encoded image; exact bytes vary (random album order).
Expect(data[:8]).To(Equal([]byte{0x89, 'P', 'N', 'G', 0x0d, 0x0a, 0x1a, 0x0a}))
// Two tiles are mirrored into the 2x2 grid as [A B B A], so opposite corners match.
q := gridQuadrants(data)
Expect(q[0]).To(Equal(q[3]))
Expect(q[1]).To(Equal(q[2]))
Expect(q[0]).ToNot(Equal(q[1]))
})
})
When("a playlist has tracks from four albums, each with its own cover", func() {
// Library:
// Artist/
// ├── AlbumA/{01 - Track.mp3, cover.png} ← tile 1
// ├── AlbumB/{01 - Track.mp3, cover.png} ← tile 2
// ├── AlbumC/{01 - Track.mp3, cover.png} ← tile 3
// └── AlbumD/{01 - Track.mp3, cover.png} ← tile 4
// Four distinct tiles fill the grid outright, with no mirroring.
It("fills all four grid quadrants with distinct album art", func() {
conf.Server.CoverArtPriority = "cover.*"
layout := fstest.MapFS{}
for _, name := range []string{"AlbumA", "AlbumB", "AlbumC", "AlbumD"} {
layout["Artist/"+name+"/01 - Track.mp3"] = trackFile(1, "T"+name, map[string]any{"album": name})
layout["Artist/"+name+"/cover.png"] = smallPNG(name)
}
setLayout(layout)
scan()
mfs, err := rds.MediaFile(rctx).GetAll(model.QueryOptions{})
Expect(err).ToNot(HaveOccurred())
Expect(mfs).To(HaveLen(4))
ids := make([]string, 0, len(mfs))
for _, mf := range mfs {
ids = append(ids, mf.ID)
}
pl := model.Playlist{ID: "pl-8", Name: "Four", OwnerID: "admin-1"}
pl.AddMediaFilesByID(ids)
Expect(rds.Playlist(rctx).Put(&pl)).To(Succeed())
ia := acquire(model.KindPlaylistArtwork, pl.ID)
Expect(ia.Source).To(Equal("generated"))
q := gridQuadrants(storedBytes(ia))
Expect([]color.RGBA{q[0], q[1], q[2], q[3]}).To(HaveLen(4))
Expect(q[0]).ToNot(Equal(q[1]))
Expect(q[0]).ToNot(Equal(q[2]))
Expect(q[0]).ToNot(Equal(q[3]))
Expect(q[1]).ToNot(Equal(q[2]))
Expect(q[1]).ToNot(Equal(q[3]))
Expect(q[2]).ToNot(Equal(q[3]))
})
})
})

View File

@ -11,6 +11,7 @@ import (
"io"
"os"
"path/filepath"
"strings"
"sync"
"testing/fstest"
"time"
@ -178,16 +179,43 @@ func serveErr(artID model.ArtworkID) error {
return err
}
// expectAlbumFolderCover asserts the worker selected a folder image at the given path suffix as the
// album cover. Folder art is file-backed (served via os.Open, which the in-memory FS can't satisfy),
// so the persisted state row — which captures the resolver's selection — is what we assert.
// libFileBytes returns the contents of the one library file whose path ends with suffix.
func libFileBytes(suffix string) []byte {
GinkgoHelper()
var match string
for name := range fakeFS.MapFS {
if strings.HasSuffix(name, suffix) {
Expect(match).To(BeEmpty(), "suffix %q is ambiguous: %q and %q", suffix, match, name)
match = name
}
}
Expect(match).ToNot(BeEmpty(), "no library file ends with %q", suffix)
return fakeFS.MapFS[match].Data
}
// expectAlbumFolderCover asserts the album resolves to the library image at the given path suffix,
// byte-for-byte. The serve happens before acquisition on purpose: with no state row the request
// path resolves locally through the library FS, whereas a settled folder row is file-backed and
// read with os.Open, which the in-memory FS cannot satisfy.
func expectAlbumFolderCover(al model.Album, suffix string) {
GinkgoHelper()
requireNoStateRow(model.KindAlbumArtwork, al.ID)
Expect(serveBytes(al.CoverArtID())).To(Equal(libFileBytes(suffix)))
ia := acquire(model.KindAlbumArtwork, al.ID)
Expect(ia.Source).To(Equal("folder"))
Expect(ia.SourcePath).To(HaveSuffix(suffix))
}
// requireNoStateRow guards the byte-level folder assertions: a drain resolves every ready queue
// row, so acquiring one entity can settle others. Once settled, folder art is file-backed and the
// in-memory FS cannot serve it — so these assertions must come before any acquire in a spec.
func requireNoStateRow(kind model.Kind, id string) {
GinkgoHelper()
_, err := rds.Artwork(rctx).GetItemArtwork(kind, id, model.ImageTypePrimary)
Expect(err).To(MatchError(model.ErrNotFound),
"assert %s %q before acquiring any other entity in this spec", kind, id)
}
// expectAlbumAbsent asserts the album settled absent (no source resolved) and serves unavailable.
func expectAlbumAbsent(al model.Album) {
GinkgoHelper()
@ -200,6 +228,8 @@ func expectAlbumAbsent(al model.Album) {
// as the artist image; like album folder art it is file-backed, so it is asserted on the state row.
func expectArtistFolder(ar model.Artist, suffix string) {
GinkgoHelper()
requireNoStateRow(model.KindArtistArtwork, ar.ID)
Expect(serveBytes(model.NewArtworkID(model.KindArtistArtwork, ar.ID, nil))).To(Equal(libFileBytes(suffix)))
ia := acquire(model.KindArtistArtwork, ar.ID)
Expect(ia.Source).To(Equal("folder"))
Expect(ia.SourcePath).To(HaveSuffix(suffix))
@ -226,6 +256,22 @@ func expectDiscImage(al model.Album, disc int, label string) {
Expect(serveBytes(discArtID(al, disc))).To(Equal(pngBytes(label)))
}
// gridQuadrants decodes a generated 2x2 playlist cover and samples the center of each quadrant,
// in rect() order: top-left, top-right, bottom-left, bottom-right. Each tile is a solid color, so
// the samples identify which album art landed where (and whether tiles were mirrored).
func gridQuadrants(data []byte) [4]color.RGBA {
GinkgoHelper()
img, _, err := image.Decode(bytes.NewReader(data))
Expect(err).ToNot(HaveOccurred())
b := img.Bounds()
qw, qh := b.Dx()/4, b.Dy()/4
at := func(x, y int) color.RGBA {
c := color.RGBAModel.Convert(img.At(b.Min.X+x, b.Min.Y+y))
return c.(color.RGBA)
}
return [4]color.RGBA{at(qw, qh), at(3*qw, qh), at(qw, 3*qh), at(3*qw, 3*qh)}
}
// storedBytes returns the bytes the worker placed in the content-addressed store for a
// store-backed resolution (embedded/generated). Folder/upload sources are file-backed and are
// not in the store; assert those on ia.SourcePath instead.

View File

@ -299,17 +299,14 @@ func (s *service) serveDisc(ctx context.Context, artID model.ArtworkID, size int
if err != nil {
return nil, err
}
// Only multi-disc albums use disc-specific resolution (matching the legacy reader); a
// single-disc album serves album art directly, so a stray disc*/embedded image can't
// shadow higher-priority album art.
if len(dr.album.Discs) > 1 {
funcs := dr.fromDiscArtPriority(ctx, s.ffmpeg, conf.Server.DiscArtPriority)
if r, path, err := selectImageReader(ctx, artID, funcs...); err == nil && r != nil {
defer r.Close()
if data, rerr := readCapped(r); rerr == nil {
if hash, herr := HashImage(bytes.NewReader(data)); herr == nil {
return s.serveBytes(ctx, hash, data, unixMtime(mtimeViaFS(dr.lib.FS, path)), size, square)
}
// Single-disc albums run the chain too: a disc can carry its own art, distinct from the
// album cover, and DiscArtPriority is what expresses that preference.
funcs := dr.fromDiscArtPriority(ctx, s.ffmpeg, conf.Server.DiscArtPriority)
if r, path, err := selectImageReader(ctx, artID, funcs...); err == nil && r != nil {
defer r.Close()
if data, rerr := readCapped(r); rerr == nil {
if hash, herr := HashImage(bytes.NewReader(data)); herr == nil {
return s.serveBytes(ctx, hash, data, unixMtime(mtimeViaFS(dr.lib.FS, path)), size, square)
}
}
}

View File

@ -307,7 +307,9 @@ var _ = Describe("Service", func() {
Expect(readAll(img)).To(Equal(coverBytes))
})
It("delegates a single-disc track straight to the album, skipping disc resolution", func() {
It("routes a single-disc track through disc resolution too", func() {
// A single disc can carry its own art: DiscArtPriority still applies, so the disc
// image is served even when the album has different found art.
folderRepo.result = []model.Folder{{Path: "tests/fixtures/artist/an-album", ImageFiles: []string{"cover.jpg"}}}
albumRepo.SetData(model.Albums{{ID: "alsd", Name: "Album", FolderIDs: []string{"f1"}, Discs: model.Discs{1: ""}}})
seedFoundStore("al", "alsd", []byte("album-art-distinct"))
@ -315,8 +317,7 @@ var _ = Describe("Service", func() {
img, err := svc.Get(ctx, model.MustParseArtworkID("mf-mf6"), 0, false)
Expect(err).ToNot(HaveOccurred())
// Single-disc album: album art wins; the folder disc image must not shadow it.
Expect(readAll(img)).To(Equal([]byte("album-art-distinct")))
Expect(readAll(img)).To(Equal(coverBytes))
})
})