From 6f93fa3141693bcc9adb2dacadc6eed1003a4ec6 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sat, 25 Jul 2026 14:44:08 -0400 Subject: [PATCH] fix(jellyfin): make a track's own cover reachable for Jellyfin clients MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Media files are never enqueued — the scanner drops their state without queueing them and the recheck kinds exclude them — so an unresolved track keeps an empty ImageHash and SongToBaseItem always fell through to AlbumPrimaryImageTag. Finamp then asks for the album image, nothing ever requests mf-, and the read-through that resolves the track never fires: the own-cover branch was unreachable for Jellyfin-only users. An eligible, unresolved, not-known-absent track now advertises its id as the Primary tag, which is what makes the client ask. The request serves the embedded art and queues the track so the worker persists a real hash. No blurhash is sent, since none exists yet and a fake would be cached against that tag forever. Serving gains the album fallback that made this safe to advertise: an eligible track whose frame will not extract now falls back the way CoverArtID does instead of answering with a placeholder. Reported by Codex on #5847. --- core/artwork/serving.go | 7 +++- core/artwork/serving_test.go | 17 ++++++++++ server/jellyfin/dto/mappers.go | 14 ++++++++ server/jellyfin/dto/mappers_test.go | 51 +++++++++++++++++++++++++++++ 4 files changed, 88 insertions(+), 1 deletion(-) 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"}