From 04eab5666a70d6f7d33436cc3d6f3b6b4403bcb6 Mon Sep 17 00:00:00 2001 From: Deluan Date: Thu, 22 Dec 2022 13:53:49 -0500 Subject: [PATCH] Add back CoverArtPriority --- core/artwork.go | 57 +++++++++++++++++++---------------- core/artwork_internal_test.go | 35 ++++++++++++++------- 2 files changed, 55 insertions(+), 37 deletions(-) diff --git a/core/artwork.go b/core/artwork.go index f58889c9f..e954c1cae 100644 --- a/core/artwork.go +++ b/core/artwork.go @@ -89,17 +89,9 @@ func (a *artwork) extractAlbumImage(ctx context.Context, artID model.ArtworkID) log.Error(ctx, "Could not retrieve album", "id", artID.ID, err) return nil, "" } - - return extractImage(ctx, artID, - fromExternalFile(al.ImageFiles, "cover.png", "cover.jpg", "cover.jpeg", "cover.webp"), - fromExternalFile(al.ImageFiles, "folder.png", "folder.jpg", "folder.jpeg", "folder.webp"), - fromExternalFile(al.ImageFiles, "album.png", "album.jpg", "album.jpeg", "album.webp"), - fromExternalFile(al.ImageFiles, "albumart.png", "albumart.jpg", "albumart.jpeg", "albumart.webp"), - fromExternalFile(al.ImageFiles, "front.png", "front.jpg", "front.jpeg", "front.webp"), - fromTag(al.EmbedArtPath), - fromFFmpegTag(ctx, a.ffmpeg, al.EmbedArtPath), - fromPlaceholder(), - ) + var ff = a.fromCoverArtPriority(ctx, conf.Server.CoverArtPriority, *al) + ff = append(ff, fromPlaceholder()) + return extractImage(ctx, artID, ff...) } func (a *artwork) extractMediaFileImage(ctx context.Context, artID model.ArtworkID) (reader io.ReadCloser, path string) { @@ -179,23 +171,36 @@ func (a *artwork) fromAlbum(ctx context.Context, id model.ArtworkID) fromFunc { } } -// This is a bit unoptimized, but we need to make sure the priority order of validNames -// is preserved (i.e. png is better than jpg) -func fromExternalFile(files string, validNames ...string) fromFunc { +func (a *artwork) fromCoverArtPriority(ctx context.Context, priority string, al model.Album) []fromFunc { + var ff []fromFunc + for _, p := range strings.Split(strings.ToLower(priority), ",") { + pat := strings.TrimSpace(p) + if pat == "embedded" { + ff = append(ff, fromTag(al.EmbedArtPath), fromFFmpegTag(ctx, a.ffmpeg, al.EmbedArtPath)) + continue + } + ff = append(ff, fromExternalFile(ctx, al.ImageFiles, p)) + } + return ff +} + +func fromExternalFile(ctx context.Context, files string, pattern string) fromFunc { return func() (io.ReadCloser, string) { - fileList := filepath.SplitList(files) - for _, validName := range validNames { - for _, file := range fileList { - _, name := filepath.Split(file) - if !strings.EqualFold(validName, name) { - continue - } - f, err := os.Open(file) - if err != nil { - continue - } - return f, file + for _, file := range filepath.SplitList(files) { + _, name := filepath.Split(file) + match, err := filepath.Match(pattern, strings.ToLower(name)) + if err != nil { + log.Warn(ctx, "Error matching cover art file to pattern", "pattern", pattern, "file", file) + continue } + if !match { + continue + } + f, err := os.Open(file) + if err != nil { + continue + } + return f, file } return nil, "" } diff --git a/core/artwork_internal_test.go b/core/artwork_internal_test.go index b8dfc917b..2bc5bf6cb 100644 --- a/core/artwork_internal_test.go +++ b/core/artwork_internal_test.go @@ -21,25 +21,26 @@ var _ = Describe("Artwork", func() { var ds model.DataStore var ffmpeg *tests.MockFFmpeg ctx := log.NewContext(context.TODO()) - var alOnlyEmbed, alEmbedNotFound, alOnlyExternal, alExternalNotFound, alAllOptions model.Album + var alOnlyEmbed, alEmbedNotFound, alOnlyExternal, alExternalNotFound, alMultipleCovers model.Album var mfWithEmbed, mfWithoutEmbed, mfCorruptedCover model.MediaFile BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + conf.Server.ImageCacheSize = "0" // Disable cache + conf.Server.CoverArtPriority = "folder.*,cover.*,embedded,front.*" + ds = &tests.MockDataStore{MockedTranscoding: &tests.MockTranscodingRepo{}} alOnlyEmbed = model.Album{ID: "222", Name: "Only embed", EmbedArtPath: "tests/fixtures/test.mp3"} alEmbedNotFound = model.Album{ID: "333", Name: "Embed not found", EmbedArtPath: "tests/fixtures/NON_EXISTENT.mp3"} alOnlyExternal = model.Album{ID: "444", Name: "Only external", ImageFiles: "tests/fixtures/front.png"} alExternalNotFound = model.Album{ID: "555", Name: "External not found", ImageFiles: "tests/fixtures/NON_EXISTENT.png"} - alAllOptions = model.Album{ID: "666", Name: "All options", EmbedArtPath: "tests/fixtures/test.mp3", + alMultipleCovers = model.Album{ID: "666", Name: "All options", EmbedArtPath: "tests/fixtures/test.mp3", ImageFiles: "tests/fixtures/cover.jpg:tests/fixtures/front.png", } mfWithEmbed = model.MediaFile{ID: "22", Path: "tests/fixtures/test.mp3", HasCoverArt: true, AlbumID: "222"} mfWithoutEmbed = model.MediaFile{ID: "44", Path: "tests/fixtures/test.ogg", AlbumID: "444"} mfCorruptedCover = model.MediaFile{ID: "45", Path: "tests/fixtures/test.ogg", HasCoverArt: true, AlbumID: "444"} - DeferCleanup(configtest.SetupConfig()) - conf.Server.ImageCacheSize = "0" // Disable cache - cache := GetImageCache() ffmpeg = tests.NewMockFFmpeg("content from ffmpeg") aw = NewArtwork(ds, cache, ffmpeg).(*artwork) @@ -84,7 +85,6 @@ var _ = Describe("Artwork", func() { BeforeEach(func() { ds.Album(ctx).(*tests.MockAlbumRepo).SetData(model.Albums{ alOnlyExternal, - alAllOptions, }) }) It("returns external cover", func() { @@ -92,17 +92,30 @@ var _ = Describe("Artwork", func() { Expect(err).ToNot(HaveOccurred()) Expect(path).To(Equal("tests/fixtures/front.png")) }) - It("returns the first image if more than one is available", func() { - _, path, err := aw.get(context.Background(), alAllOptions.CoverArtID(), 0) - Expect(err).ToNot(HaveOccurred()) - Expect(path).To(Equal("tests/fixtures/cover.jpg")) - }) It("returns placeholder if external file is not available", func() { _, path, err := aw.get(context.Background(), alExternalNotFound.CoverArtID(), 0) Expect(err).ToNot(HaveOccurred()) Expect(path).To(Equal(consts.PlaceholderAlbumArt)) }) }) + Context("Multiple covers", func() { + BeforeEach(func() { + ds.Album(ctx).(*tests.MockAlbumRepo).SetData(model.Albums{ + alMultipleCovers, + }) + }) + DescribeTable("CoverArtPriority", + func(priority string, expected string) { + conf.Server.CoverArtPriority = priority + _, path, err := aw.get(context.Background(), alMultipleCovers.CoverArtID(), 0) + Expect(err).ToNot(HaveOccurred()) + Expect(path).To(Equal(expected)) + }, + Entry(nil, "folder.*,cover.*,embedded,front.*", "tests/fixtures/cover.jpg"), + Entry(nil, "front.*,cover.*,embedded,folder.*", "tests/fixtures/front.png"), + Entry(nil, "embedded,front.*,cover.*,folder.*", "tests/fixtures/test.mp3"), + ) + }) }) Context("MediaFiles", func() { Context("ID not found", func() {