fix(artwork): always select the coverArt of the first disc in multi-disc albums (#3950)

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

Signed-off-by: Deluan <deluan@navidrome.org>

* feat(artwork): improve album cover art selection by considering disc numbers

Signed-off-by: Deluan <deluan@navidrome.org>

---------

Signed-off-by: Deluan <deluan@navidrome.org>
This commit is contained in:
Deluan Quintão
2025-04-11 23:39:57 -04:00
committed by GitHub
parent bcea8b832a
commit 49b8cfc261
4 changed files with 205 additions and 3 deletions
+28 -3
View File
@@ -183,6 +183,8 @@ func (mfs MediaFiles) ToAlbum() Album {
tags := make(TagList, 0, len(mfs[0].Tags)*len(mfs))
a.Missing = true
embedArtPath := ""
embedArtDisc := 0
for _, m := range mfs {
// We assume these attributes are all the same for all songs in an album
a.ID = m.AlbumID
@@ -211,15 +213,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
embedArtPath, embedArtDisc = firstArtPath(embedArtPath, embedArtDisc, m)
if m.ExplicitStatus == "c" && a.ExplicitStatus != "e" {
a.ExplicitStatus = "c"
} else if m.ExplicitStatus == "e" {
@@ -231,6 +233,7 @@ func (mfs MediaFiles) ToAlbum() Album {
a.Missing = a.Missing && m.Missing
}
a.EmbedArtPath = embedArtPath
a.SetTags(tags)
a.FolderIDs = slice.Unique(slice.Map(mfs, func(m MediaFile) string { return m.FolderID }))
a.Date, _ = allOrNothing(dates)
@@ -305,6 +308,28 @@ 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, currentDisc int, m MediaFile) (string, int) {
if !m.HasCoverArt {
return currentPath, currentDisc
}
// If current has no disc number (currentDisc == 0) or new file has lower disc number
if currentDisc == 0 || (m.DiscNumber < currentDisc && m.DiscNumber > 0) {
return m.Path, m.DiscNumber
}
// If disc numbers are equal, use path for ordering
if m.DiscNumber == currentDisc {
if m.Path < currentPath || currentPath == "" {
return m.Path, m.DiscNumber
}
}
return currentPath, currentDisc
}
type MediaFileCursor iter.Seq2[MediaFile, error]
type MediaFileRepository interface {
+95
View File
@@ -305,6 +305,101 @@ 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"))
})
})
When("we have media files with path names that don't correlate with disc numbers", func() {
BeforeEach(func() {
mfs = MediaFiles{
{
Path: "Artist/Album/file-z.mp3", // Path would be sorted last alphabetically
HasCoverArt: true,
DiscNumber: 1, // But it has lowest disc number
},
{
Path: "Artist/Album/file-a.mp3", // Path would be sorted first alphabetically
HasCoverArt: true,
DiscNumber: 2, // But it has higher disc number
},
{
Path: "Artist/Album/file-m.mp3",
HasCoverArt: true,
DiscNumber: 3,
},
}
})
It("selects the cover art from the lowest disc number regardless of path", func() {
album := mfs.ToAlbum()
Expect(album.EmbedArtPath).To(Equal("Artist/Album/file-z.mp3"))
})
})
})
})
})
})