From 4030bfe06f808573b568c47d3d272a563e10f027 Mon Sep 17 00:00:00 2001
From: Deluan <deluan@navidrome.org>
Date: Wed, 1 Apr 2026 08:38:29 -0400
Subject: [PATCH] fix(artwork): preserve animation for square thumbnails with
 animated images

Signed-off-by: Deluan <deluan@navidrome.org>
---
 core/artwork/reader_resized.go      | 24 ++++++++--------
 core/artwork/reader_resized_test.go | 44 ++++++++++++++++-------------
 2 files changed, 36 insertions(+), 32 deletions(-)

diff --git a/core/artwork/reader_resized.go b/core/artwork/reader_resized.go
index 72baad43..88ca8b83 100644
--- a/core/artwork/reader_resized.go
+++ b/core/artwork/reader_resized.go
@@ -98,21 +98,19 @@ func (a *resizedArtworkReader) resizeImage(ctx context.Context, reader io.Reader
 		return nil, 0, fmt.Errorf("reading image data: %w", err)
 	}
 
-	// Preserve animation for animated images (skip for square thumbnails)
-	if !a.square {
-		if isAnimatedGIF(data) {
-			if 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 {
-					return r, 0, nil
-				}
-				log.Warn(ctx, "Could not convert animated GIF, falling back to static", err)
+	// Preserve animation for animated images
+	if isAnimatedGIF(data) {
+		if 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 {
+				return r, 0, nil
 			}
-		} else if isAnimatedWebP(data) || isAnimatedPNG(data) {
-			// Animated WebP/APNG: return original as-is (ffmpeg can't re-encode these)
-			return bytes.NewReader(data), 0, nil
+			log.Warn(ctx, "Could not convert animated GIF, falling back to static", err)
 		}
+	} else if isAnimatedWebP(data) || isAnimatedPNG(data) {
+		// Animated WebP/APNG: return original as-is (ffmpeg can't re-encode these)
+		return bytes.NewReader(data), 0, nil
 	}
 
 	return resizeStaticImage(data, a.size, a.square)
diff --git a/core/artwork/reader_resized_test.go b/core/artwork/reader_resized_test.go
index 7c9e21c0..7c14f5e4 100644
--- a/core/artwork/reader_resized_test.go
+++ b/core/artwork/reader_resized_test.go
@@ -54,17 +54,17 @@ var _ = Describe("resizeImage", func() {
 			Expect(len(output)).To(BeNumerically(">", 0))
 		})
 
-		It("skips animation for square thumbnails even with animated GIF", func() {
+		It("preserves animation for square thumbnails with animated GIF", func() {
 			r.square = true
 			data := createAnimatedGIF(3)
 			result, _, err := r.resizeImage(context.Background(), bytes.NewReader(data))
-			// Should fall through to static resize (not ffmpeg conversion)
-			// The minimal test GIF may or may not resize successfully,
-			// but ffmpeg should NOT have been called for animated conversion
-			_ = result
-			_ = err
-			// Verify by checking the mock wasn't used for animated conversion:
-			// If ffmpeg was called, it would return mock data, not static resize result
+			Expect(err).ToNot(HaveOccurred())
+			Expect(result).ToNot(BeNil())
+
+			// Should have been processed by ffmpeg (mock returns input data)
+			output, err := io.ReadAll(result)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(output).To(Equal(data))
 		})
 	})
 
@@ -81,13 +81,17 @@ var _ = Describe("resizeImage", func() {
 			Expect(output).To(Equal(data))
 		})
 
-		It("does not passthrough animated WebP for square thumbnails", func() {
+		It("preserves animated WebP for square thumbnails", func() {
 			r.square = true
 			data := createAnimatedWebPBytes()
-			// Should fall through to static resize, which will fail on fake WebP data
-			_, _, err := r.resizeImage(context.Background(), bytes.NewReader(data))
-			// Static decode will fail on our minimal test WebP bytes (not a real image)
-			Expect(err).To(HaveOccurred())
+			result, _, err := r.resizeImage(context.Background(), bytes.NewReader(data))
+			Expect(err).ToNot(HaveOccurred())
+			Expect(result).ToNot(BeNil())
+
+			// Should return original data unchanged
+			output, err := io.ReadAll(result)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(output).To(Equal(data))
 		})
 	})
 
@@ -104,15 +108,17 @@ var _ = Describe("resizeImage", func() {
 			Expect(output).To(Equal(data))
 		})
 
-		It("does not passthrough animated PNG for square thumbnails", func() {
+		It("preserves animated PNG for square thumbnails", func() {
 			r.square = true
 			data := createAPNGBytes()
-			// Should fall through to static resize
 			result, _, err := r.resizeImage(context.Background(), bytes.NewReader(data))
-			// Static PNG decode should succeed on our APNG (it's a valid PNG)
-			if err == nil {
-				Expect(result).ToNot(BeNil())
-			}
+			Expect(err).ToNot(HaveOccurred())
+			Expect(result).ToNot(BeNil())
+
+			// Should return original data unchanged
+			output, err := io.ReadAll(result)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(output).To(Equal(data))
 		})
 	})