diff --git a/core/artwork.go b/core/artwork.go index e1d5ca831..c8899cccc 100644 --- a/core/artwork.go +++ b/core/artwork.go @@ -1,12 +1,17 @@ package core import ( + "bytes" "context" + "errors" _ "image/gif" _ "image/png" "io" + "os" + "github.com/dhowden/tag" "github.com/navidrome/navidrome/consts" + "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/resources" _ "golang.org/x/image/webp" @@ -25,5 +30,67 @@ type artwork struct { } func (a *artwork) Get(ctx context.Context, id string, size int) (io.ReadCloser, error) { - return resources.FS().Open(consts.PlaceholderAlbumArt) + r, _, err := a.get(ctx, id, size) + return r, err +} + +func (a *artwork) get(ctx context.Context, id string, size int) (io.ReadCloser, string, error) { + artId, err := model.ParseArtworkID(id) + if err != nil { + return nil, "", errors.New("invalid ID") + } + id = artId.ID + al, err := a.ds.Album(ctx).Get(id) + if errors.Is(err, model.ErrNotFound) { + r, path := fromPlaceholder()() + return r, path, nil + } + if err != nil { + return nil, "", err + } + r, path := extractImage(ctx, artId, + fromTag(al.EmbedArtPath), + fromPlaceholder(), + ) + return r, path, nil +} + +func extractImage(ctx context.Context, artId model.ArtworkID, extractFuncs ...func() (io.ReadCloser, string)) (io.ReadCloser, string) { + for _, f := range extractFuncs { + r, path := f() + if r != nil { + log.Trace(ctx, "Found artwork", "artId", artId, "path", path) + return r, path + } + } + log.Error(ctx, "extractImage should never reach this point!", "artId", artId, "path") + return nil, "" +} + +func fromTag(path string) func() (io.ReadCloser, string) { + return func() (io.ReadCloser, string) { + f, err := os.Open(path) + if err != nil { + return nil, "" + } + defer f.Close() + + m, err := tag.ReadFrom(f) + if err != nil { + return nil, "" + } + + picture := m.Picture() + if picture == nil { + return nil, "" + } + return io.NopCloser(bytes.NewReader(picture.Data)), path + } +} + +func fromPlaceholder() func() (io.ReadCloser, string) { + return func() (io.ReadCloser, string) { + r, _ := resources.FS().Open(consts.PlaceholderAlbumArt) + return r, consts.PlaceholderAlbumArt + } } diff --git a/core/artwork_internal_test.go b/core/artwork_internal_test.go new file mode 100644 index 000000000..64b9f05af --- /dev/null +++ b/core/artwork_internal_test.go @@ -0,0 +1,60 @@ +package core + +import ( + "context" + + "github.com/navidrome/navidrome/consts" + "github.com/navidrome/navidrome/log" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/tests" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = FDescribe("Artwork", func() { + var aw *artwork + var ds model.DataStore + ctx := log.NewContext(context.TODO()) + var alOnlyEmbed, alEmbedNotFound model.Album + + BeforeEach(func() { + 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"} + // {ID: "666", Name: "All options", EmbedArtPath: "tests/fixtures/test.mp3", + // ImageFiles: "tests/fixtures/cover.jpg:tests/fixtures/front.png"}, + //}) + aw = NewArtwork(ds).(*artwork) + }) + + When("cover art is not found", func() { + BeforeEach(func() { + ds.Album(ctx).(*tests.MockAlbumRepo).SetData(model.Albums{ + alOnlyEmbed, + }) + }) + It("returns placeholder if album is not in the DB", func() { + _, path, err := aw.get(context.Background(), "al-999-0", 0) + Expect(err).ToNot(HaveOccurred()) + Expect(path).To(Equal(consts.PlaceholderAlbumArt)) + }) + }) + When("album has only embed images", func() { + BeforeEach(func() { + ds.Album(ctx).(*tests.MockAlbumRepo).SetData(model.Albums{ + alOnlyEmbed, + alEmbedNotFound, + }) + }) + It("returns embed cover", func() { + _, path, err := aw.get(context.Background(), alOnlyEmbed.CoverArtID().String(), 0) + Expect(err).ToNot(HaveOccurred()) + Expect(path).To(Equal("tests/fixtures/test.mp3")) + }) + It("returns placeholder if embed path is not available", func() { + _, path, err := aw.get(context.Background(), alEmbedNotFound.CoverArtID().String(), 0) + Expect(err).ToNot(HaveOccurred()) + Expect(path).To(Equal(consts.PlaceholderAlbumArt)) + }) + }) +}) diff --git a/core/artwork_test.go b/core/artwork_test.go deleted file mode 100644 index 1aa6838eb..000000000 --- a/core/artwork_test.go +++ /dev/null @@ -1,29 +0,0 @@ -package core - -import ( - "context" - - "github.com/navidrome/navidrome/log" - "github.com/navidrome/navidrome/model" - "github.com/navidrome/navidrome/tests" - . "github.com/onsi/ginkgo/v2" -) - -var _ = Describe("Artwork", func() { - var ds model.DataStore - ctx := log.NewContext(context.TODO()) - - BeforeEach(func() { - ds = &tests.MockDataStore{MockedTranscoding: &tests.MockTranscodingRepo{}} - ds.Album(ctx).(*tests.MockAlbumRepo).SetData(model.Albums{ - {ID: "222", EmbedArtPath: "tests/fixtures/test.mp3"}, - {ID: "333"}, - {ID: "444", EmbedArtPath: "tests/fixtures/cover.jpg"}, - }) - ds.MediaFile(ctx).(*tests.MockMediaFileRepo).SetData(model.MediaFiles{ - {ID: "123", AlbumID: "222", Path: "tests/fixtures/test.mp3", HasCoverArt: true}, - {ID: "456", AlbumID: "222", Path: "tests/fixtures/test.ogg", HasCoverArt: false}, - }) - }) - -}) diff --git a/model/artwork_id.go b/model/artwork_id.go index 83a0855c4..9a750f255 100644 --- a/model/artwork_id.go +++ b/model/artwork_id.go @@ -22,7 +22,11 @@ type ArtworkID struct { } func (id ArtworkID) String() string { - return fmt.Sprintf("%s-%s-%x", id.Kind.prefix, id.ID, id.LastAccess.Unix()) + s := fmt.Sprintf("%s-%s", id.Kind.prefix, id.ID) + if id.LastAccess.Unix() < 0 { + return s + "-0" + } + return fmt.Sprintf("%s-%x", s, id.LastAccess.Unix()) } func ParseArtworkID(id string) (ArtworkID, error) { diff --git a/model/mediafile.go b/model/mediafile.go index 83275cc6e..8b79523fa 100644 --- a/model/mediafile.go +++ b/model/mediafile.go @@ -69,11 +69,11 @@ func (mf MediaFile) ContentType() string { } func (mf MediaFile) CoverArtID() ArtworkID { - // If it is a mediaFile, and it has cover art, return it (if feature is disabled, skip) + // If it has a cover art, return it (if feature is disabled, skip) if mf.HasCoverArt && !conf.Server.DevFastAccessCoverArt { return artworkIDFromMediaFile(mf) } - // if the mediaFile does not have a coverArt, fallback to the album cover + // if it does not have a coverArt, fallback to the album cover return artworkIDFromAlbum(Album{ID: mf.AlbumID, UpdatedAt: mf.UpdatedAt}) } diff --git a/tests/fixtures/front.png b/tests/fixtures/front.png new file mode 100644 index 000000000..d4a366391 Binary files /dev/null and b/tests/fixtures/front.png differ diff --git a/ui/src/reducers/playerReducer.js b/ui/src/reducers/playerReducer.js index 428be7fcf..ff39499aa 100644 --- a/ui/src/reducers/playerReducer.js +++ b/ui/src/reducers/playerReducer.js @@ -37,7 +37,7 @@ const mapToAudioLists = (item) => { musicSrc: subsonic.streamUrl(trackId), cover: subsonic.getCoverArtUrl( { - coverArtId: config.devFastAccessCoverArt ? item.albumId : trackId, + id: config.devFastAccessCoverArt ? item.albumId : trackId, updatedAt: item.updatedAt, }, 300 diff --git a/ui/src/subsonic/index.js b/ui/src/subsonic/index.js index 54b71a018..9908ee442 100644 --- a/ui/src/subsonic/index.js +++ b/ui/src/subsonic/index.js @@ -51,10 +51,14 @@ const getCoverArtUrl = (record, size) => { ...(size && { size }), } - if (record.coverArtId) { - return baseUrl(url('getCoverArt', record.coverArtId, options)) + const lastUpdate = Math.floor(Date.parse(record.updatedAt) / 1000).toString( + 16 + ) + const id = record.id + '-' + lastUpdate + if (record.album) { + return baseUrl(url('getCoverArt', 'mf-' + id, options)) } else { - return baseUrl(url('getCoverArt', 'not_found', size && { size })) + return baseUrl(url('getCoverArt', 'al-' + id, options)) } }