From 3a213ef03e401930977138afe0e84c83290df683 Mon Sep 17 00:00:00 2001 From: Deluan Date: Thu, 2 Apr 2026 17:31:11 -0400 Subject: [PATCH] fix(artwork): address PR review feedback - Add DevJpegCoverArt to logRemovedOptions so users with the old config key get a clear warning instead of a silent ignore. - Include EnableWebPEncoding in the resized artwork cache key to prevent stale WebP responses after toggling the setting. - Skip animated GIF to WebP conversion via ffmpeg when EnableWebPEncoding is false, so the setting is consistent across all image types. - Fix data race in cache warmer by reading UICoverArtSize at construction time instead of per-image, avoiding concurrent access with config cleanup in tests. - Clarify cache warmer docstring to accurately describe caching behavior. --- conf/configuration.go | 2 +- core/artwork/cache_warmer.go | 26 ++++++++++++++------------ core/artwork/reader_resized.go | 5 ++++- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/conf/configuration.go b/conf/configuration.go index de29216cc..d477e55e3 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -423,7 +423,7 @@ func Load(noConfigDump bool) { logDeprecatedOptions("CoverJpegQuality", "CoverArtQuality") // Removed options - logRemovedOptions("Spotify.ID", "Spotify.Secret") + logRemovedOptions("Spotify.ID", "Spotify.Secret", "DevJpegCoverArt") // Call init hooks for _, hook := range hooks { diff --git a/core/artwork/cache_warmer.go b/core/artwork/cache_warmer.go index 3dddd2c6f..487c91abf 100644 --- a/core/artwork/cache_warmer.go +++ b/core/artwork/cache_warmer.go @@ -22,8 +22,8 @@ type CacheWarmer interface { } // NewCacheWarmer creates a new CacheWarmer instance. The CacheWarmer will pre-cache Artwork images in the background -// to speed up the response time when the image is requested by the UI. The cache is pre-populated with the original -// image size, as well as the size defined by the UICoverArtSize config option. +// to speed up the response time when the image is requested by the UI. The cache is pre-populated with the size +// defined by the UICoverArtSize config option (the original-size image is also cached as a side effect of resizing). func NewCacheWarmer(artwork Artwork, cache cache.FileCache) CacheWarmer { // If image cache is disabled, return a NOOP implementation if conf.Server.ImageCacheSize == "0" || !conf.Server.EnableArtworkPrecache { @@ -37,10 +37,11 @@ func NewCacheWarmer(artwork Artwork, cache cache.FileCache) CacheWarmer { } a := &cacheWarmer{ - artwork: artwork, - cache: cache, - buffer: make(map[model.ArtworkID]struct{}), - wakeSignal: make(chan struct{}, 1), + artwork: artwork, + cache: cache, + buffer: make(map[model.ArtworkID]struct{}), + wakeSignal: make(chan struct{}, 1), + coverArtSize: conf.Server.UICoverArtSize, } // Create a context with a fake admin user, to be able to pre-cache Playlist CoverArts @@ -50,11 +51,12 @@ func NewCacheWarmer(artwork Artwork, cache cache.FileCache) CacheWarmer { } type cacheWarmer struct { - artwork Artwork - buffer map[model.ArtworkID]struct{} - mutex sync.Mutex - cache cache.FileCache - wakeSignal chan struct{} + artwork Artwork + buffer map[model.ArtworkID]struct{} + mutex sync.Mutex + cache cache.FileCache + wakeSignal chan struct{} + coverArtSize int } func (a *cacheWarmer) PreCache(artID model.ArtworkID) { @@ -141,7 +143,7 @@ func (a *cacheWarmer) doCacheImage(ctx context.Context, id model.ArtworkID) erro ctx, cancel := context.WithTimeout(ctx, 10*time.Second) defer cancel() - size := conf.Server.UICoverArtSize + size := a.coverArtSize r, _, err := a.artwork.Get(ctx, id, size, true) if err != nil { return fmt.Errorf("caching id='%s', size=%d: %w", id, size, err) diff --git a/core/artwork/reader_resized.go b/core/artwork/reader_resized.go index 85a19a4c3..955b08548 100644 --- a/core/artwork/reader_resized.go +++ b/core/artwork/reader_resized.go @@ -65,6 +65,9 @@ func (a *resizedArtworkReader) Key() string { if a.square { return baseKey + ".square" } + if conf.Server.EnableWebPEncoding { + return fmt.Sprintf("%s.%d.webp", baseKey, conf.Server.CoverArtQuality) + } return fmt.Sprintf("%s.%d", baseKey, conf.Server.CoverArtQuality) } @@ -110,7 +113,7 @@ func (a *resizedArtworkReader) resizeImage(ctx context.Context, reader io.Reader // Preserve animation for animated images if isAnimatedGIF(data) { - if a.a.ffmpeg.IsAvailable() { + if conf.Server.EnableWebPEncoding && a.a.ffmpeg.IsAvailable() { // Animated GIF: convert to animated WebP via ffmpeg (with optional resize) r, err := a.a.ffmpeg.ConvertAnimatedImage(ctx, bytes.NewReader(data), a.size, conf.Server.CoverArtQuality) if err == nil {