mirror of
https://github.com/navidrome/navidrome.git
synced 2026-04-26 10:30:46 +00:00
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.
This commit is contained in:
parent
bb1a734afc
commit
3a213ef03e
3 changed files with 19 additions and 14 deletions
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue