From 817baa5c1a27b9f6f78ee480afa8347ea57d670a Mon Sep 17 00:00:00 2001 From: Deluan Date: Fri, 17 Jul 2026 23:28:01 -0400 Subject: [PATCH] fix(jellyfin): omit the blurhash when no current hash exists, never fabricate Upstream Jellyfin omits ImageBlurHashes entries when no hash was computed, and clients are built around that: redesign Finamp uses the blurhash as immutable cover identity (year-long cache pins, download dedup) and falls back to id-keyed caching with a short TTL when it is absent. Emitting a rotating fake seeded by id+version fed a fabricated identity into those caches and churned them on every version bump, and a fake accepted under an imprecise artwork version could pin a wrong value for a year. Absence is strictly safer: no LQIP during the first-serve gap, self-healing within the fallback TTL. The staleness gate is kept, its job now being to suppress a stale stored hash rather than to choose between real and fake. Songs no longer carry a fabricated per-album value either; art resolution uses AlbumPrimaryImageTag alone. --- server/jellyfin/dto/blurhash.go | 35 +++++++---------- server/jellyfin/dto/blurhash_test.go | 56 ++++++++------------------- server/jellyfin/dto/mappers.go | 10 ++--- server/jellyfin/dto/mappers_test.go | 27 ++++++------- server/jellyfin/e2e/playlists_test.go | 6 ++- 5 files changed, 51 insertions(+), 83 deletions(-) diff --git a/server/jellyfin/dto/blurhash.go b/server/jellyfin/dto/blurhash.go index ddcbeb08c..8952ab3d9 100644 --- a/server/jellyfin/dto/blurhash.go +++ b/server/jellyfin/dto/blurhash.go @@ -1,32 +1,23 @@ package dto import ( - "fmt" - "hash/fnv" "time" - - "github.com/navidrome/navidrome/core/artwork/blurhash" ) -// blurHash returns a valid 6-char blurhash for a solid color derived from seed. Finamp only needs a -// well-formed, per-tag-stable value (it uses this as a download de-dup key and blur placeholder), so -// a solid color unique to the tag satisfies both without decoding cover art. -func blurHash(seed string) string { - h := fnv.New32a() - _, _ = h.Write([]byte(seed)) - sum := h.Sum(nil) - r, g, b := int(sum[0]), int(sum[1]), int(sum[2]) - dc := (r << 16) | (g << 8) | b - return "00" + blurhash.Encode83(dc, 4) -} - -// primaryBlurHash returns the stored blurhash when it was computed from the entity's current -// artwork version or later (the snapshot folds in image file mtimes, which can exceed row -// timestamps); otherwise a fake seeded by id+version, so the value still rotates on any artwork -// change (Finamp keys its cover caches by this value; tags never reach its image URLs). -func primaryBlurHash(stored string, storedAt *time.Time, id string, version time.Time) string { +// primaryBlurHash returns the stored blurhash when current for the artwork version, else "" so the +// key is omitted (upstream behavior); clients treat it as cover identity, so absence beats a fake. +func primaryBlurHash(stored string, storedAt *time.Time, version time.Time) string { if stored != "" && storedAt != nil && !storedAt.Before(version) { return stored } - return blurHash(fmt.Sprintf("%s-%x", id, version.UnixMilli())) + return "" +} + +// primaryBlurHashes builds the ImageBlurHashes map for a known-current hash, or nil so the field is +// omitted entirely when there is none. +func primaryBlurHashes(tag, hash string) map[string]map[string]string { + if hash == "" { + return nil + } + return map[string]map[string]string{"Primary": {tag: hash}} } diff --git a/server/jellyfin/dto/blurhash_test.go b/server/jellyfin/dto/blurhash_test.go index 0d35ae83f..a661800f9 100644 --- a/server/jellyfin/dto/blurhash_test.go +++ b/server/jellyfin/dto/blurhash_test.go @@ -1,63 +1,41 @@ package dto import ( - "strings" "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) -// base83Alphabet duplicates the spec alphabet on purpose: the test must catch the production copy -// drifting, not drift along with it. -const base83Alphabet = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz#$%*+,-.:;=?@[]^_{|}~" - -var _ = Describe("blurHash", func() { - It("returns a 6-char valid blurhash starting with the 1x1 component prefix", func() { - h := blurHash("x") - Expect(h).To(HaveLen(6)) - Expect(h).To(HavePrefix("00")) - for _, c := range h { - Expect(strings.ContainsRune(base83Alphabet, c)).To(BeTrue(), "unexpected char %q", c) - } - }) - - It("is deterministic for the same seed", func() { - Expect(blurHash("cover-tag-1")).To(Equal(blurHash("cover-tag-1"))) - }) - - It("differs for different seeds", func() { - Expect(blurHash("cover-tag-1")).ToNot(Equal(blurHash("cover-tag-2"))) - }) -}) - var _ = Describe("primaryBlurHash", func() { version := time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC) It("returns the stored hash when it matches the current artwork version", func() { - Expect(primaryBlurHash("LEHV6nWB2yk8", &version, "id-1", version)).To(Equal("LEHV6nWB2yk8")) + Expect(primaryBlurHash("LEHV6nWB2yk8", &version, version)).To(Equal("LEHV6nWB2yk8")) }) It("returns the stored hash when the snapshot is newer than the version (image mtime)", func() { newer := version.Add(time.Hour) - Expect(primaryBlurHash("LEHV6nWB2yk8", &newer, "id-1", version)).To(Equal("LEHV6nWB2yk8")) + Expect(primaryBlurHash("LEHV6nWB2yk8", &newer, version)).To(Equal("LEHV6nWB2yk8")) }) - It("falls back to a fake when there is no stored hash", func() { - h := primaryBlurHash("", nil, "id-1", version) - Expect(h).To(HaveLen(6)) + It("omits when there is no stored hash", func() { + Expect(primaryBlurHash("", nil, version)).To(BeEmpty()) }) - It("falls back to a fake when the stored hash is stale", func() { + It("omits when the stored hash is stale (cover changed, not yet re-served)", func() { stale := version.Add(-time.Hour) - h := primaryBlurHash("LEHV6nWB2yk8", &stale, "id-1", version) - Expect(h).To(HaveLen(6)) - Expect(h).ToNot(Equal("LEHV6nWB2yk8")) - }) - - It("rotates the fake when the artwork version moves", func() { - h1 := primaryBlurHash("", nil, "id-1", version) - h2 := primaryBlurHash("", nil, "id-1", version.Add(time.Hour)) - Expect(h1).ToNot(Equal(h2)) + Expect(primaryBlurHash("LEHV6nWB2yk8", &stale, version)).To(BeEmpty()) + }) +}) + +var _ = Describe("primaryBlurHashes", func() { + It("wraps a hash under the Primary tag", func() { + Expect(primaryBlurHashes("tag-1", "LEHV6nWB2yk8")).To( + Equal(map[string]map[string]string{"Primary": {"tag-1": "LEHV6nWB2yk8"}})) + }) + + It("returns nil when there is no hash, so the field is omitted", func() { + Expect(primaryBlurHashes("tag-1", "")).To(BeNil()) }) }) diff --git a/server/jellyfin/dto/mappers.go b/server/jellyfin/dto/mappers.go index e284ad0de..0ee1c7073 100644 --- a/server/jellyfin/dto/mappers.go +++ b/server/jellyfin/dto/mappers.go @@ -179,10 +179,10 @@ func SongToBaseItem(mf model.MediaFile, fields Fields) BaseItemDto { } else if mf.Genre != "" { item.Genres = []string{mf.Genre} } - // Finamp resolves song art via AlbumId + a non-empty AlbumPrimaryImageTag. + // Finamp resolves song art via AlbumId + a non-empty AlbumPrimaryImageTag. No blurhash for songs: + // there is no stored hash of their own, and clients cache covers by the value as identity. if mf.AlbumID != "" { item.AlbumPrimaryImageTag = mf.AlbumID - item.ImageBlurHashes = map[string]map[string]string{"Primary": {mf.AlbumID: blurHash(mf.AlbumID)}} } return item } @@ -201,7 +201,7 @@ func AlbumToBaseItem(al model.Album) BaseItemDto { RunTimeTicks: TicksFromSeconds(al.Duration), DateCreated: jellyfinDate(&al.CreatedAt), ImageTags: map[string]string{"Primary": al.ID}, - ImageBlurHashes: map[string]map[string]string{"Primary": {al.ID: primaryBlurHash(al.BlurHash, al.BlurHashUpdatedAt, al.ID, al.ArtworkUpdatedAt())}}, + ImageBlurHashes: primaryBlurHashes(al.ID, primaryBlurHash(al.BlurHash, al.BlurHashUpdatedAt, al.ArtworkUpdatedAt())), BackdropImageTags: []string{}, UserData: UserData(al.Annotations, al.ID), } @@ -231,7 +231,7 @@ func ArtistToBaseItem(ar model.Artist) BaseItemDto { SongCount: new(ar.SongCount), DateCreated: jellyfinDate(ar.CreatedAt), ImageTags: map[string]string{"Primary": ar.ID}, - ImageBlurHashes: map[string]map[string]string{"Primary": {ar.ID: primaryBlurHash(ar.BlurHash, ar.BlurHashUpdatedAt, ar.ID, ar.ArtworkUpdatedAt())}}, + ImageBlurHashes: primaryBlurHashes(ar.ID, primaryBlurHash(ar.BlurHash, ar.BlurHashUpdatedAt, ar.ArtworkUpdatedAt())), BackdropImageTags: []string{}, UserData: UserData(ar.Annotations, ar.ID), } @@ -264,7 +264,7 @@ func PlaylistToBaseItem(p model.Playlist) BaseItemDto { ChildCount: new(p.SongCount), RunTimeTicks: TicksFromSeconds(p.Duration), ImageTags: map[string]string{"Primary": tag}, - ImageBlurHashes: map[string]map[string]string{"Primary": {tag: primaryBlurHash(p.BlurHash, p.BlurHashUpdatedAt, p.ID, p.ArtworkUpdatedAt())}}, + ImageBlurHashes: primaryBlurHashes(tag, primaryBlurHash(p.BlurHash, p.BlurHashUpdatedAt, p.ArtworkUpdatedAt())), BackdropImageTags: []string{}, UserData: UserData(p.Annotations, p.ID), } diff --git a/server/jellyfin/dto/mappers_test.go b/server/jellyfin/dto/mappers_test.go index fbd3b16bd..d91d9506e 100644 --- a/server/jellyfin/dto/mappers_test.go +++ b/server/jellyfin/dto/mappers_test.go @@ -33,8 +33,9 @@ var _ = Describe("mappers", func() { Expect(item.UserData.Played).To(BeTrue()) Expect(item.UserData.Key).To(Equal(EncodeID("song-1"))) Expect(item.UserData.ItemId).To(Equal(EncodeID("song-1"))) - Expect(item.ImageBlurHashes["Primary"]).To(HaveKey(item.AlbumPrimaryImageTag)) - Expect(item.ImageBlurHashes["Primary"][item.AlbumPrimaryImageTag]).To(HaveLen(6)) + Expect(item.AlbumPrimaryImageTag).To(Equal("alb-1")) + // Songs never carry a fabricated blurhash; clients cache covers by it as identity. + Expect(item.ImageBlurHashes).To(BeNil()) }) Describe("Fields gating (matches real Jellyfin)", func() { @@ -209,8 +210,8 @@ var _ = Describe("mappers", func() { Expect(item.ArtistItems).To(Equal(item.AlbumArtists)) Expect(*item.ProductionYear).To(Equal(1999)) Expect(*item.ChildCount).To(Equal(10)) - Expect(item.ImageBlurHashes["Primary"]).To(HaveKey(item.ImageTags["Primary"])) - Expect(item.ImageBlurHashes["Primary"][item.ImageTags["Primary"]]).To(HaveLen(6)) + // No stored blurhash: the field is omitted, never fabricated. + Expect(item.ImageBlurHashes).To(BeNil()) }) It("maps an artist to a MusicArtist folder item", func() { @@ -283,19 +284,18 @@ var _ = Describe("mappers", func() { Expect(*item.UserData.Rating).To(Equal(8.0)) tag := item.ImageTags["Primary"] Expect(tag).ToNot(BeEmpty()) - Expect(item.ImageBlurHashes["Primary"]).To(HaveKey(tag)) - Expect(item.ImageBlurHashes["Primary"][tag]).To(HaveLen(6)) + // No stored blurhash on this playlist: omitted, never fabricated. + Expect(item.ImageBlurHashes).To(BeNil()) }) - It("changes the playlist image tag and blurhash when the playlist is updated (cover upload)", func() { + It("changes the playlist image tag when the playlist is updated (cover upload)", func() { p := model.Playlist{ID: "pl-1", Name: "Chill", UpdatedAt: time.Date(2026, 7, 1, 0, 0, 0, 0, time.UTC)} before := PlaylistToBaseItem(p) p.UpdatedAt = time.Date(2026, 7, 2, 0, 0, 0, 0, time.UTC) after := PlaylistToBaseItem(p) - // Finamp caches covers keyed by blurHash, so tag and blurhash must change with the cover. + // The tag is the cover cache-buster: it must rotate when the playlist (cover) is updated. Expect(after.ImageTags["Primary"]).ToNot(Equal(before.ImageTags["Primary"])) - Expect(after.ImageBlurHashes["Primary"]).ToNot(Equal(before.ImageBlurHashes["Primary"])) }) It("keeps the playlist image tag stable when nothing changed", func() { @@ -394,17 +394,14 @@ var _ = Describe("LyricDtoFromLyrics", func() { var _ = Describe("stored blurhashes", func() { version := time.Date(2024, 1, 1, 0, 0, 0, 0, time.UTC) - It("emits the stored album blurhash when fresh, and a rotating fake when stale", func() { + It("emits the stored album blurhash when fresh, and omits it when stale", func() { al := model.Album{ID: "al-1", Name: "A", UpdatedAt: version, ImportedAt: version, BlurHash: "LEHV6nWB2yk8", BlurHashUpdatedAt: &version} Expect(AlbumToBaseItem(al).ImageBlurHashes["Primary"]["al-1"]).To(Equal("LEHV6nWB2yk8")) al.UpdatedAt = version.Add(time.Hour) // artwork version moved; stored hash is now stale - fake := AlbumToBaseItem(al).ImageBlurHashes["Primary"]["al-1"] - Expect(fake).To(HaveLen(6)) - - al.UpdatedAt = version.Add(2 * time.Hour) - Expect(AlbumToBaseItem(al).ImageBlurHashes["Primary"]["al-1"]).ToNot(Equal(fake)) + Expect(AlbumToBaseItem(al).ImageBlurHashes).To(BeNil(), + "a stale hash must be suppressed, not emitted or replaced by a fake") }) It("emits the stored artist blurhash when fresh", func() { diff --git a/server/jellyfin/e2e/playlists_test.go b/server/jellyfin/e2e/playlists_test.go index d2ff49db9..b06834fc6 100644 --- a/server/jellyfin/e2e/playlists_test.go +++ b/server/jellyfin/e2e/playlists_test.go @@ -217,7 +217,7 @@ var _ = Describe("Playlists", func() { // Guards the whole chain: SetImage must go through a full Put (which bumps UpdatedAt), and the // tag must be versioned by it, or clients keep their blurhash-keyed cover cache forever. - It("rotates the playlist's image tag and blurhash after a cover upload", func() { + It("rotates the playlist's image tag after a cover upload", func() { plID := createPlaylist("Cover Tag", nil) imageTag := func() string { q := queryResult(get("/Items?ids=" + enc(plID))) @@ -233,8 +233,10 @@ var _ = Describe("Playlists", func() { after := imageTag() Expect(after).ToNot(Equal(before)) + // The stored hash (if any) is stale for the new cover, so no blurhash is emitted — clients + // fall back to tag-keyed caching until the new cover is served and re-hashed. q := queryResult(get("/Items?ids=" + enc(plID))) - Expect(q.Items[0].ImageBlurHashes["Primary"]).To(HaveKey(after)) + Expect(q.Items[0].ImageBlurHashes).To(BeEmpty()) }) })