diff --git a/core/artwork/artwork_internal_test.go b/core/artwork/artwork_internal_test.go index 5ca32f40..4b235989 100644 --- a/core/artwork/artwork_internal_test.go +++ b/core/artwork/artwork_internal_test.go @@ -28,7 +28,7 @@ var _ = Describe("Artwork", func() { var ffmpeg *tests.MockFFmpeg var folderRepo *fakeFolderRepo ctx := log.NewContext(context.TODO()) - var alOnlyEmbed, alEmbedNotFound, alOnlyExternal, alExternalNotFound, alMultipleCovers model.Album + var alOnlyEmbed, alEmbedNotFound, alOnlyExternal, alExternalNotFound, alMultipleCovers, alSingleDisc model.Album var arMultipleCovers model.Artist var mfWithEmbed, mfAnotherWithEmbed, mfWithoutEmbed, mfCorruptedCover model.MediaFile @@ -44,8 +44,9 @@ var _ = Describe("Artwork", func() { } alOnlyEmbed = model.Album{ID: "222", Name: "Only embed", EmbedArtPath: "tests/fixtures/artist/an-album/test.mp3", FolderIDs: []string{"f1"}} alEmbedNotFound = model.Album{ID: "333", Name: "Embed not found", EmbedArtPath: "tests/fixtures/NON_EXISTENT.mp3", FolderIDs: []string{"f1"}} - alOnlyExternal = model.Album{ID: "444", Name: "Only external", FolderIDs: []string{"f1"}} + alOnlyExternal = model.Album{ID: "444", Name: "Only external", FolderIDs: []string{"f1"}, Discs: model.Discs{1: "", 2: ""}} alExternalNotFound = model.Album{ID: "555", Name: "External not found", FolderIDs: []string{"f2"}} + alSingleDisc = model.Album{ID: "888", Name: "Single disc", FolderIDs: []string{"f1"}, Discs: model.Discs{1: ""}} arMultipleCovers = model.Artist{ID: "777", Name: "All options"} alMultipleCovers = model.Album{ ID: "666", @@ -193,6 +194,7 @@ var _ = Describe("Artwork", func() { ds.Album(ctx).(*tests.MockAlbumRepo).SetData(model.Albums{ alOnlyEmbed, alOnlyExternal, + alSingleDisc, }) ds.MediaFile(ctx).(*tests.MockMediaFileRepo).SetData(model.MediaFiles{ mfWithEmbed, @@ -236,6 +238,28 @@ var _ = Describe("Artwork", func() { Expect(err).ToNot(HaveOccurred()) Expect(path).To(Equal("al-444_0")) }) + It("falls back to disc cover art when media file has a disc number on a multi-disc album", func() { + mfWithDisc := model.MediaFile{ID: "46", Path: "tests/fixtures/test.ogg", AlbumID: "444", DiscNumber: 2} + Expect(ds.MediaFile(ctx).(*tests.MockMediaFileRepo).Put(&mfWithDisc)).To(Succeed()) + + aw, err := newMediafileArtworkReader(ctx, aw, model.MustParseArtworkID("mf-"+mfWithDisc.ID)) + Expect(err).ToNot(HaveOccurred()) + _, path, err := aw.Reader(ctx) + Expect(err).ToNot(HaveOccurred()) + // Should fall back to disc art, which itself falls back to album art + Expect(path).To(Equal("dc-444:2_0")) + }) + It("falls back to album cover art for single-disc albums even with a disc number", func() { + mfOnSingleDisc := model.MediaFile{ID: "47", Path: "tests/fixtures/test.ogg", AlbumID: "888", DiscNumber: 1} + Expect(ds.MediaFile(ctx).(*tests.MockMediaFileRepo).Put(&mfOnSingleDisc)).To(Succeed()) + + aw, err := newMediafileArtworkReader(ctx, aw, model.MustParseArtworkID("mf-"+mfOnSingleDisc.ID)) + Expect(err).ToNot(HaveOccurred()) + _, path, err := aw.Reader(ctx) + Expect(err).ToNot(HaveOccurred()) + // Single-disc album should skip disc art and go straight to album art + Expect(path).To(Equal("al-888_0")) + }) }) }) Describe("playlistArtworkReader", func() { diff --git a/core/artwork/reader_album.go b/core/artwork/reader_album.go index 98a2105e..6de1d31d 100644 --- a/core/artwork/reader_album.go +++ b/core/artwork/reader_album.go @@ -59,10 +59,11 @@ func newAlbumArtworkReader(ctx context.Context, artwork *artwork, artID model.Ar } func (a *albumArtworkReader) Key() string { - var hash [16]byte + hashInput := conf.Server.CoverArtPriority if conf.Server.EnableExternalServices { - hash = md5.Sum([]byte(conf.Server.Agents + conf.Server.CoverArtPriority)) + hashInput += conf.Server.Agents } + hash := md5.Sum([]byte(hashInput)) return fmt.Sprintf( "%s.%x.%t", a.cacheKey.Key(), diff --git a/core/artwork/reader_mediafile.go b/core/artwork/reader_mediafile.go index c72d9543..cf25c8f5 100644 --- a/core/artwork/reader_mediafile.go +++ b/core/artwork/reader_mediafile.go @@ -26,16 +26,22 @@ func newMediafileArtworkReader(ctx context.Context, artwork *artwork, artID mode if err != nil { return nil, err } + _, _, imagesUpdatedAt, err := loadAlbumFoldersPaths(ctx, artwork.ds, *al) + if err != nil { + return nil, err + } a := &mediafileArtworkReader{ a: artwork, mediafile: *mf, album: *al, } a.cacheKey.artID = artID - if al.UpdatedAt.After(mf.UpdatedAt) { + a.cacheKey.lastUpdate = mf.UpdatedAt + if al.UpdatedAt.After(a.cacheKey.lastUpdate) { a.cacheKey.lastUpdate = al.UpdatedAt - } else { - a.cacheKey.lastUpdate = mf.UpdatedAt + } + if imagesUpdatedAt != nil && imagesUpdatedAt.After(a.cacheKey.lastUpdate) { + a.cacheKey.lastUpdate = *imagesUpdatedAt } return a, nil } @@ -60,6 +66,12 @@ func (a *mediafileArtworkReader) Reader(ctx context.Context) (io.ReadCloser, str fromFFmpegTag(ctx, a.a.ffmpeg, path), } } - ff = append(ff, fromAlbum(ctx, a.a, a.mediafile.AlbumCoverArtID())) + // For multi-disc albums, fall back to disc artwork first; for single-disc albums, + // skip disc resolution (it would just fall through to album art anyway). + if len(a.album.Discs) > 1 { + ff = append(ff, fromAlbum(ctx, a.a, a.mediafile.DiscCoverArtID())) + } else { + ff = append(ff, fromAlbum(ctx, a.a, a.mediafile.AlbumCoverArtID())) + } return selectImageReader(ctx, a.artID, ff...) } diff --git a/model/mediafile.go b/model/mediafile.go index 20532bfb..ec83b76f 100644 --- a/model/mediafile.go +++ b/model/mediafile.go @@ -119,7 +119,16 @@ func (mf MediaFile) CoverArtID() ArtworkID { if mf.HasCoverArt && conf.Server.EnableMediaFileCoverArt { return artworkIDFromMediaFile(mf) } - // if it does not have a coverArt, fallback to the album cover + // Otherwise fallback to disc (if available) or album cover + return mf.DiscCoverArtID() +} + +// DiscCoverArtID returns the disc artwork ID when the media file has a disc number, +// otherwise it returns the album artwork ID. +func (mf MediaFile) DiscCoverArtID() ArtworkID { + if mf.DiscNumber > 0 { + return NewArtworkID(KindDiscArtwork, DiscArtworkID(mf.AlbumID, mf.DiscNumber), nil) + } return mf.AlbumCoverArtID() } diff --git a/model/mediafile_test.go b/model/mediafile_test.go index 207d3c15..038ac93d 100644 --- a/model/mediafile_test.go +++ b/model/mediafile_test.go @@ -504,13 +504,26 @@ var _ = Describe("MediaFile", func() { Expect(id.Kind).To(Equal(KindMediaFileArtwork)) Expect(id.ID).To(Equal(mf.ID)) }) - It("returns its album id if HasCoverArt is false", func() { + It("returns disc art id if HasCoverArt is false and DiscNumber > 0", func() { + mf := MediaFile{ID: "111", AlbumID: "1", HasCoverArt: false, DiscNumber: 2} + id := mf.CoverArtID() + Expect(id.Kind).To(Equal(KindDiscArtwork)) + Expect(id.ID).To(Equal("1:2")) + }) + It("returns its album id if HasCoverArt is false and DiscNumber is 0", func() { mf := MediaFile{ID: "111", AlbumID: "1", HasCoverArt: false} id := mf.CoverArtID() Expect(id.Kind).To(Equal(KindAlbumArtwork)) Expect(id.ID).To(Equal(mf.AlbumID)) }) - It("returns its album id if EnableMediaFileCoverArt is disabled", func() { + It("returns disc art id if EnableMediaFileCoverArt is disabled and DiscNumber > 0", func() { + conf.Server.EnableMediaFileCoverArt = false + mf := MediaFile{ID: "111", AlbumID: "1", HasCoverArt: true, DiscNumber: 3} + id := mf.CoverArtID() + Expect(id.Kind).To(Equal(KindDiscArtwork)) + Expect(id.ID).To(Equal("1:3")) + }) + It("returns its album id if EnableMediaFileCoverArt is disabled and DiscNumber is 0", func() { conf.Server.EnableMediaFileCoverArt = false mf := MediaFile{ID: "111", AlbumID: "1", HasCoverArt: true} id := mf.CoverArtID()