diff --git a/core/artwork/serving.go b/core/artwork/serving.go index 9d30fd47f..9199c4ed6 100644 --- a/core/artwork/serving.go +++ b/core/artwork/serving.go @@ -289,8 +289,13 @@ func (s *service) provisionalEmbedded(ctx context.Context, artID model.ArtworkID if err != nil { return nil, err } - res, _ := resolveEmbedded(ctx, lib, s.ffmpeg, mf.Path) + res, ok := resolveEmbedded(ctx, lib, s.ffmpeg, mf.Path) s.enqueue(ctx, artID, model.ArtworkPriorityBump) + if !ok { + // Eligible but unextractable (truncated frame, no ffmpeg): fall back the way + // CoverArtID does rather than answer with a placeholder. + return s.Get(ctx, mf.DiscCoverArtID(), size, square) + } return s.serveResolution(ctx, res, size, square) } diff --git a/core/artwork/serving_test.go b/core/artwork/serving_test.go index 9cef4d106..03499d8bf 100644 --- a/core/artwork/serving_test.go +++ b/core/artwork/serving_test.go @@ -307,6 +307,23 @@ var _ = Describe("Service", func() { Expect(readAll(img)).To(Equal(coverBytes)) }) + // Jellyfin clients are now told to request the track image before it resolves, so an + // unextractable frame must not answer with a placeholder where the album has art. + It("falls back to the album when an eligible track's embedded art will not extract", func() { + conf.Server.EnableMediaFileCoverArt = true + // HasCoverArt is set, but the file yields no extractable image (it is not audio). + mfRepo.SetData(model.MediaFiles{{ + ID: "mfbad", AlbumID: "albad", LibraryID: 0, HasCoverArt: true, + Path: "tests/fixtures/artist/an-album/front.png", + }}) + albumRepo.SetData(model.Albums{{ID: "albad", Name: "Album"}}) + seedFoundStore("al", "albad", coverBytes) + + img, err := svc.Get(ctx, model.MustParseArtworkID("mf-mfbad"), 0, false) + Expect(err).ToNot(HaveOccurred()) + Expect(readAll(img)).To(Equal(coverBytes), "a placeholder here would be worse than the album cover") + }) + It("routes a single-disc track through disc resolution too", func() { // A single disc can carry its own art: DiscArtPriority still applies, so the disc // image is served even when the album has different found art. diff --git a/server/jellyfin/dto/mappers.go b/server/jellyfin/dto/mappers.go index f47b3788d..1beadac11 100644 --- a/server/jellyfin/dto/mappers.go +++ b/server/jellyfin/dto/mappers.go @@ -5,6 +5,7 @@ import ( "fmt" "time" + "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/utils/slice" ) @@ -197,6 +198,12 @@ func SongToBaseItem(mf model.MediaFile, fields Fields) BaseItemDto { tag, blurs := primaryImageTag(mf.ItemImage, mf.ID) item.ImageTags = map[string]string{"Primary": tag} item.ImageBlurHashes = blurs + } else if embeddedArtPending(mf) { + // Nothing enqueues media files, so an unresolved track only resolves when someone asks + // for its image. Advertising the id here is what makes a Jellyfin client ask; the + // request extracts the embedded art and queues the track for the worker. No blurhash: + // there is no resolved image to have one yet, and a fake would be cached forever. + item.ImageTags = map[string]string{"Primary": mf.ID} } else if mf.AlbumID != "" { if tag, blurs := primaryImageTag(mf.AlbumImage, mf.AlbumID); tag != "" { item.AlbumPrimaryImageTag = tag @@ -206,6 +213,13 @@ func SongToBaseItem(mf model.MediaFile, fields Fields) BaseItemDto { return item } +// embeddedArtPending reports a track whose own art is eligible but not resolved yet, and not +// known to be absent. +func embeddedArtPending(mf model.MediaFile) bool { + return mf.HasCoverArt && conf.Server.EnableMediaFileCoverArt && + mf.ImageHash == "" && !mf.ItemImage.ImageAbsent +} + // primaryImageTag never synthesizes a blurhash: Finamp keys its cover cache on the value, // so a fake one pins a stale cover forever (#5798). func primaryImageTag(img model.ItemImage, fallback string) (string, map[string]map[string]string) { diff --git a/server/jellyfin/dto/mappers_test.go b/server/jellyfin/dto/mappers_test.go index 2b80a42d6..1251918e1 100644 --- a/server/jellyfin/dto/mappers_test.go +++ b/server/jellyfin/dto/mappers_test.go @@ -4,6 +4,8 @@ import ( "encoding/json" "time" + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/model" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -388,6 +390,55 @@ var _ = Describe("mappers", func() { Expect(after.ImageTags["Primary"]).To(Equal(before.ImageTags["Primary"])) }) + // Nothing enqueues media files, so a track's own art only resolves when something requests + // it. A Jellyfin client that is only told about the album image never asks, so the track's + // own cover would stay unreachable for Jellyfin-only users. + Describe("unresolved embedded art", func() { + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + conf.Server.EnableMediaFileCoverArt = true + }) + + It("advertises the track id so the client triggers the read-through", func() { + mf := model.MediaFile{ID: "mf-1", AlbumID: "alb-1", HasCoverArt: true} + mf.AlbumImage.ImageHash = "0123456789abcdef" + + item := SongToBaseItem(mf, nil) + Expect(item.ImageTags).To(HaveKeyWithValue("Primary", "mf-1")) + Expect(item.ImageBlurHashes).To(BeNil(), "no resolved image means no blurhash to send") + Expect(item.AlbumPrimaryImageTag).To(BeEmpty()) + }) + + It("falls back to the album when the track has no art of its own", func() { + mf := model.MediaFile{ID: "mf-2", AlbumID: "alb-1", HasCoverArt: false} + mf.AlbumImage.ImageHash = "0123456789abcdef" + + item := SongToBaseItem(mf, nil) + Expect(item.ImageTags).To(BeEmpty()) + Expect(item.AlbumPrimaryImageTag).To(Equal("0123456789abcdef")) + }) + + It("falls back to the album once the track's art is known absent", func() { + mf := model.MediaFile{ID: "mf-3", AlbumID: "alb-1", HasCoverArt: true} + mf.ItemImage.ImageAbsent = true + mf.AlbumImage.ImageHash = "0123456789abcdef" + + item := SongToBaseItem(mf, nil) + Expect(item.ImageTags).To(BeEmpty()) + Expect(item.AlbumPrimaryImageTag).To(Equal("0123456789abcdef")) + }) + + It("falls back to the album when per-track art is disabled", func() { + conf.Server.EnableMediaFileCoverArt = false + mf := model.MediaFile{ID: "mf-4", AlbumID: "alb-1", HasCoverArt: true} + mf.AlbumImage.ImageHash = "0123456789abcdef" + + item := SongToBaseItem(mf, nil) + Expect(item.ImageTags).To(BeEmpty()) + Expect(item.AlbumPrimaryImageTag).To(Equal("0123456789abcdef")) + }) + }) + Describe("primary image tags", func() { It("uses the content hash as the tag and emits the real blurhash", func() { al := model.Album{ID: "alb-1", Name: "Album"}