feat(artwork): sort image files for consistent cover art selection

Signed-off-by: Deluan <deluan@navidrome.org>
This commit is contained in:
Deluan 2025-04-11 18:50:55 -04:00
parent 58367afaea
commit 8ac867429e
4 changed files with 167 additions and 3 deletions

View File

@ -6,6 +6,7 @@ import (
"fmt"
"io"
"path/filepath"
"slices"
"strings"
"time"
@ -113,5 +114,10 @@ func loadAlbumFoldersPaths(ctx context.Context, ds model.DataStore, albums ...mo
imgFiles = append(imgFiles, filepath.Join(path, img))
}
}
// Sort image files to ensure consistent selection of cover art
// This prioritizes files from lower-numbered disc folders by sorting the paths
slices.Sort(imgFiles)
return paths, imgFiles, &updatedAt, nil
}

View File

@ -0,0 +1,76 @@
package artwork
import (
"context"
"path/filepath"
"time"
"github.com/navidrome/navidrome/model"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
var _ = Describe("Album Artwork Reader", func() {
Describe("loadAlbumFoldersPaths", func() {
var (
ctx context.Context
ds *fakeDataStore
repo *fakeFolderRepo
album model.Album
now time.Time
expectedAt time.Time
)
BeforeEach(func() {
ctx = context.Background()
now = time.Now().Truncate(time.Second)
expectedAt = now.Add(5 * time.Minute)
// Set up the test folders with image files
repo = &fakeFolderRepo{
result: []model.Folder{
{
Path: "Artist/Album/Disc1",
ImagesUpdatedAt: expectedAt,
ImageFiles: []string{"cover.jpg", "back.jpg"},
},
{
Path: "Artist/Album/Disc2",
ImagesUpdatedAt: now,
ImageFiles: []string{"cover.jpg"},
},
{
Path: "Artist/Album/Disc10",
ImagesUpdatedAt: now,
ImageFiles: []string{"cover.jpg"},
},
},
err: nil,
}
ds = &fakeDataStore{
folderRepo: repo,
}
album = model.Album{
ID: "album1",
Name: "Album",
FolderIDs: []string{"folder1", "folder2", "folder3"},
}
})
It("returns sorted image files", func() {
_, imgFiles, imagesUpdatedAt, err := loadAlbumFoldersPaths(ctx, ds, album)
Expect(err).ToNot(HaveOccurred())
Expect(*imagesUpdatedAt).To(Equal(expectedAt))
// Check that image files are sorted alphabetically
Expect(imgFiles).To(HaveLen(4))
// The files should be sorted by full path
Expect(imgFiles[0]).To(Equal(filepath.FromSlash("Artist/Album/Disc1/back.jpg")))
Expect(imgFiles[1]).To(Equal(filepath.FromSlash("Artist/Album/Disc1/cover.jpg")))
Expect(imgFiles[2]).To(Equal(filepath.FromSlash("Artist/Album/Disc10/cover.jpg")))
Expect(imgFiles[3]).To(Equal(filepath.FromSlash("Artist/Album/Disc2/cover.jpg")))
})
})
})

View File

@ -211,15 +211,15 @@ func (mfs MediaFiles) ToAlbum() Album {
comments = append(comments, m.Comment)
mbzAlbumIds = append(mbzAlbumIds, m.MbzAlbumID)
mbzReleaseGroupIds = append(mbzReleaseGroupIds, m.MbzReleaseGroupID)
if m.HasCoverArt && a.EmbedArtPath == "" {
a.EmbedArtPath = m.Path
}
if m.DiscNumber > 0 {
a.Discs.Add(m.DiscNumber, m.DiscSubtitle)
}
tags = append(tags, m.Tags.FlattenAll()...)
a.Participants.Merge(m.Participants)
// Find the MediaFile with cover art and the lowest disc number to use for album cover
a.EmbedArtPath = firstArtPath(a.EmbedArtPath, m)
if m.ExplicitStatus == "c" && a.ExplicitStatus != "e" {
a.ExplicitStatus = "c"
} else if m.ExplicitStatus == "e" {
@ -305,6 +305,19 @@ func fixAlbumArtist(a *Album) {
}
}
// firstArtPath determines which media file path should be used for album artwork
// based on disc number (preferring lower disc numbers) and path (for consistency)
func firstArtPath(currentPath string, m MediaFile) string {
if !m.HasCoverArt {
return currentPath
}
if m.Path < currentPath || currentPath == "" {
return m.Path
}
return currentPath
}
type MediaFileCursor iter.Seq2[MediaFile, error]
type MediaFileRepository interface {

View File

@ -305,6 +305,75 @@ var _ = Describe("MediaFiles", func() {
})
})
})
Context("Album Art", func() {
When("we have media files with cover art from multiple discs", func() {
BeforeEach(func() {
mfs = MediaFiles{
{
Path: "Artist/Album/Disc2/01.mp3",
HasCoverArt: true,
DiscNumber: 2,
},
{
Path: "Artist/Album/Disc1/01.mp3",
HasCoverArt: true,
DiscNumber: 1,
},
{
Path: "Artist/Album/Disc3/01.mp3",
HasCoverArt: true,
DiscNumber: 3,
},
}
})
It("selects the cover art from the lowest disc number", func() {
album := mfs.ToAlbum()
Expect(album.EmbedArtPath).To(Equal("Artist/Album/Disc1/01.mp3"))
})
})
When("we have media files with cover art from the same disc number", func() {
BeforeEach(func() {
mfs = MediaFiles{
{
Path: "Artist/Album/Disc1/02.mp3",
HasCoverArt: true,
DiscNumber: 1,
},
{
Path: "Artist/Album/Disc1/01.mp3",
HasCoverArt: true,
DiscNumber: 1,
},
}
})
It("selects the cover art with the lowest path alphabetically", func() {
album := mfs.ToAlbum()
Expect(album.EmbedArtPath).To(Equal("Artist/Album/Disc1/01.mp3"))
})
})
When("we have media files with some missing cover art", func() {
BeforeEach(func() {
mfs = MediaFiles{
{
Path: "Artist/Album/Disc1/01.mp3",
HasCoverArt: false,
DiscNumber: 1,
},
{
Path: "Artist/Album/Disc2/01.mp3",
HasCoverArt: true,
DiscNumber: 2,
},
}
})
It("selects the file with cover art even if from a higher disc number", func() {
album := mfs.ToAlbum()
Expect(album.EmbedArtPath).To(Equal("Artist/Album/Disc2/01.mp3"))
})
})
})
})
})
})