From dafcdf438f8a87fac5248c986eec5708539a1db5 Mon Sep 17 00:00:00 2001 From: Deluan Date: Thu, 23 Jul 2026 20:28:56 -0400 Subject: [PATCH] feat(jellyfin): emit real blurhashes and drop the synthesized fallback --- server/jellyfin/dto/blurhash.go | 36 ------------- server/jellyfin/dto/blurhash_test.go | 27 ---------- server/jellyfin/dto/mappers.go | 22 ++++---- server/jellyfin/dto/mappers_test.go | 73 +++++++++++++++++++++------ server/jellyfin/e2e/playlists_test.go | 23 +++++---- 5 files changed, 82 insertions(+), 99 deletions(-) delete mode 100644 server/jellyfin/dto/blurhash.go delete mode 100644 server/jellyfin/dto/blurhash_test.go diff --git a/server/jellyfin/dto/blurhash.go b/server/jellyfin/dto/blurhash.go deleted file mode 100644 index aaf6ff2af..000000000 --- a/server/jellyfin/dto/blurhash.go +++ /dev/null @@ -1,36 +0,0 @@ -package dto - -import "hash/fnv" - -// base83Alphabet is the blurhash spec's base83 encoding alphabet; order is part of the spec. -const base83Alphabet = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz#$%*+,-.:;=?@[]^_{|}~" - -// base83 encodes value as a fixed-width, big-endian base83 string of the given length. -func base83(value, length int) string { - b := make([]byte, length) - for i := 1; i <= length; i++ { - digit := (value / pow83(length-i)) % 83 - b[i-1] = base83Alphabet[digit] - } - return string(b) -} - -func pow83(n int) int { - result := 1 - for range n { - result *= 83 - } - return result -} - -// 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" + base83(dc, 4) -} diff --git a/server/jellyfin/dto/blurhash_test.go b/server/jellyfin/dto/blurhash_test.go deleted file mode 100644 index a6e36131d..000000000 --- a/server/jellyfin/dto/blurhash_test.go +++ /dev/null @@ -1,27 +0,0 @@ -package dto - -import ( - "strings" - - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" -) - -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"))) - }) -}) diff --git a/server/jellyfin/dto/mappers.go b/server/jellyfin/dto/mappers.go index dac1c1953..eda1fd1df 100644 --- a/server/jellyfin/dto/mappers.go +++ b/server/jellyfin/dto/mappers.go @@ -192,10 +192,13 @@ 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, so this tag + // must version the ALBUM's image, not the track's own art. if mf.AlbumID != "" { - item.AlbumPrimaryImageTag = mf.AlbumID - item.ImageBlurHashes = map[string]map[string]string{"Primary": {mf.AlbumID: blurHash(mf.AlbumID)}} + if tag, blurs := primaryImageTag(mf.AlbumImage, mf.AlbumID); tag != "" { + item.AlbumPrimaryImageTag = tag + item.ImageBlurHashes = blurs + } } return item } @@ -306,10 +309,8 @@ func StudioToBaseItem(t model.Tag) BaseItemDto { // PlaylistToBaseItem maps a playlist to a Playlist BaseItemDto. func PlaylistToBaseItem(p model.Playlist) BaseItemDto { - // Finamp caches covers keyed by blurHash, so the tag (and blurhash) must change with the cover. - // UpdatedAt versions it (Put bumps it on upload); over-invalidation only costs a refetch. - tag := fmt.Sprintf("%s-%x", p.ID, p.UpdatedAt.UnixMilli()) - return BaseItemDto{ + tag, blurs := primaryImageTag(p.ItemImage, p.ID) + item := BaseItemDto{ Name: p.Name, Id: EncodeID(p.ID), Type: "Playlist", @@ -320,11 +321,14 @@ func PlaylistToBaseItem(p model.Playlist) BaseItemDto { MediaType: "Audio", ChildCount: new(p.SongCount), RunTimeTicks: TicksFromSeconds(p.Duration), - ImageTags: map[string]string{"Primary": tag}, - ImageBlurHashes: map[string]map[string]string{"Primary": {tag: blurHash(tag)}}, + ImageBlurHashes: blurs, BackdropImageTags: []string{}, UserData: UserData(p.Annotations, p.ID), } + if tag != "" { + item.ImageTags = map[string]string{"Primary": tag} + } + return item } // LyricDtoFromLyrics maps one lyric track to Jellyfin's LyricDto. Clients infer synced-vs-plain diff --git a/server/jellyfin/dto/mappers_test.go b/server/jellyfin/dto/mappers_test.go index e3ed0b9cb..bc34df19e 100644 --- a/server/jellyfin/dto/mappers_test.go +++ b/server/jellyfin/dto/mappers_test.go @@ -34,8 +34,8 @@ 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")) + Expect(item.ImageBlurHashes).To(BeNil()) Expect(item.Genres).To(Equal([]string{"genre 1", "genre 2"})) Expect(item.GenreItems).To(Equal([]NameGuidPair{{Id: EncodeID("1"), Name: "genre 1"}, {Id: EncodeID("2"), Name: "genre 2"}})) }) @@ -363,26 +363,29 @@ var _ = Describe("mappers", func() { Expect(item.UserData.IsFavorite).To(BeTrue()) Expect(item.UserData.PlayCount).To(Equal(2)) 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)) + Expect(item.ImageTags).To(HaveKeyWithValue("Primary", "pl-1")) + Expect(item.ImageBlurHashes).To(BeNil()) }) - It("changes the playlist image tag and blurhash 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)} + It("changes the playlist image tag when the cover content changes", func() { + p := model.Playlist{ID: "pl-1", Name: "Chill"} + p.ImageHash = "1111111111111111" + before := PlaylistToBaseItem(p) + p.ImageHash = "2222222222222222" + after := PlaylistToBaseItem(p) + + Expect(before.ImageTags["Primary"]).To(Equal("1111111111111111")) + Expect(after.ImageTags["Primary"]).To(Equal("2222222222222222")) + }) + + It("keeps the playlist image tag stable across a metadata-only edit", func() { + p := model.Playlist{ID: "pl-1", UpdatedAt: time.Date(2026, 7, 1, 0, 0, 0, 0, time.UTC)} + p.ImageHash = "1111111111111111" 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. - 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() { - p := model.Playlist{ID: "pl-1", UpdatedAt: time.Date(2026, 7, 1, 0, 0, 0, 0, time.UTC)} - Expect(PlaylistToBaseItem(p).ImageTags).To(Equal(PlaylistToBaseItem(p).ImageTags)) + Expect(after.ImageTags["Primary"]).To(Equal(before.ImageTags["Primary"])) }) Describe("primary image tags", func() { @@ -430,6 +433,44 @@ var _ = Describe("mappers", func() { Expect(item.ImageBlurHashes["Primary"]).To(HaveKeyWithValue("fedcba9876543210", "L6PZfSi_.AyE")) }) }) + + Describe("song and playlist image tags", func() { + It("versions a song's album tag by the album's content hash", func() { + mf := model.MediaFile{ID: "song-1", Title: "Song", AlbumID: "alb-1"} + mf.AlbumImage.ImageHash = "0123456789abcdef" + mf.AlbumImage.BlurHash = "LEHV6nWB2yk8" + + item := SongToBaseItem(mf, nil) + Expect(item.AlbumPrimaryImageTag).To(Equal("0123456789abcdef")) + Expect(item.ImageBlurHashes["Primary"]).To(HaveKeyWithValue("0123456789abcdef", "LEHV6nWB2yk8")) + }) + + It("never synthesizes a song blurhash when the album has none", func() { + mf := model.MediaFile{ID: "song-2", Title: "Song", AlbumID: "alb-2"} + mf.AlbumImage.ImageHash = "0123456789abcdef" + + item := SongToBaseItem(mf, nil) + Expect(item.AlbumPrimaryImageTag).To(Equal("0123456789abcdef")) + Expect(item.ImageBlurHashes).To(BeNil()) + }) + + It("omits a song's album tag when the album art is known absent", func() { + mf := model.MediaFile{ID: "song-3", Title: "Song", AlbumID: "alb-3"} + mf.AlbumImage.ImageAbsent = true + + item := SongToBaseItem(mf, nil) + Expect(item.AlbumPrimaryImageTag).To(BeEmpty()) + Expect(item.ImageBlurHashes).To(BeNil()) + }) + + It("versions a playlist tag by content hash instead of UpdatedAt", func() { + pl := model.Playlist{ID: "pl-1", Name: "Playlist"} + pl.ImageHash = "abcdef0123456789" + + item := PlaylistToBaseItem(pl) + Expect(item.ImageTags).To(HaveKeyWithValue("Primary", "abcdef0123456789")) + }) + }) }) var _ = Describe("LyricDtoFromLyrics", func() { diff --git a/server/jellyfin/e2e/playlists_test.go b/server/jellyfin/e2e/playlists_test.go index d2ff49db9..2ba4b1608 100644 --- a/server/jellyfin/e2e/playlists_test.go +++ b/server/jellyfin/e2e/playlists_test.go @@ -6,8 +6,8 @@ import ( jpeglib "image/jpeg" "net/http" "os" - "time" + "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/server/jellyfin/dto" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -215,26 +215,27 @@ var _ = Describe("Playlists", func() { To(Equal(http.StatusNotImplemented)) }) - // 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() { + // Guards the whole chain: an upload must clear any previously-resolved artwork state, or a + // stale tag stays live under clients' blurhash-keyed cover cache until the next scan (#5798). + It("clears the resolved image tag after a cover upload", func() { plID := createPlaylist("Cover Tag", nil) + Expect(ds.Artwork(ctx).PutItemArtwork(&model.ItemArtwork{ + ItemKind: model.KindPlaylistArtwork.Prefix(), ItemID: plID, Hash: "1111111111111111", + })).To(Succeed()) + imageTag := func() string { q := queryResult(get("/Items?ids=" + enc(plID))) Expect(q.Items).To(HaveLen(1)) return q.Items[0].ImageTags["Primary"] } - before := imageTag() - Expect(before).ToNot(BeEmpty()) + Expect(imageTag()).To(Equal("1111111111111111")) - time.Sleep(2 * time.Millisecond) // UpdatedAt has millisecond resolution in the tag Expect(upload(adminUser, "/Items/"+enc(plID)+"/Images/Primary", "image/jpeg", jpeg).Code). To(Equal(http.StatusNoContent)) - after := imageTag() - Expect(after).ToNot(Equal(before)) - q := queryResult(get("/Items?ids=" + enc(plID))) - Expect(q.Items[0].ImageBlurHashes["Primary"]).To(HaveKey(after)) + // The upload re-queues resolution instead of resolving inline, so the tag falls back to + // unresolved rather than carrying over the pre-upload hash. + Expect(imageTag()).ToNot(Equal("1111111111111111")) }) })