From a34be430423974702badef6d3ed4e170c90f9e22 Mon Sep 17 00:00:00 2001 From: Deluan Date: Fri, 31 Jul 2026 17:34:14 -0400 Subject: [PATCH] perf(artwork): hand both hash encoders one NRGBA thumbnail makeThumbnail emitted a premultiplied *image.RGBA, so thumbhash allocated and un-premultiplied a full copy on every image while blurhash paid nothing. It now emits *image.NRGBA, which thumbhash wants as-is, and blurhash reads that type directly, premultiplying per pixel to keep its output identical. thumbhash.Encode at the pipeline's 100x100 input: 134200 -> 95300 ns/op, 56188 -> 15163 B/op, 37 -> 35 allocs/op decodeArtwork on a 1000x1000 JPEG: 5014796 -> 4973800 B/op, exactly the conversion that is gone The scaler costs the same into either destination (7.33ms vs 7.34ms measured), so no time is traded for this. --- core/artwork/blurhash/blurhash.go | 46 +++++++++++++----- core/artwork/blurhash/blurhash_bench_test.go | 6 +-- core/artwork/blurhash/blurhash_test.go | 48 +++++++++++++++++++ core/artwork/processor.go | 4 +- core/artwork/processor_internal_test.go | 47 ++++++++++++++++++ .../artwork/thumbhash/thumbhash_bench_test.go | 8 ++-- 6 files changed, 140 insertions(+), 19 deletions(-) create mode 100644 core/artwork/processor_internal_test.go diff --git a/core/artwork/blurhash/blurhash.go b/core/artwork/blurhash/blurhash.go index 405d7e714..ca5bf7026 100644 --- a/core/artwork/blurhash/blurhash.go +++ b/core/artwork/blurhash/blurhash.go @@ -32,9 +32,8 @@ func Encode(img image.Image) (string, error) { } // Pre-downscale: its rounding can flip a component count, and the hash is a client cache key. xComp, yComp := components(img.Bounds().Dx(), img.Bounds().Dy()) - rgba := toRGBA(downscale(img)) - bounds := rgba.Bounds() - w, h := bounds.Dx(), bounds.Dy() + src := pixelsOf(downscale(img)) + w, h := src.w, src.h cosX := make([][]float64, xComp) for i := range cosX { @@ -54,10 +53,14 @@ func Encode(img image.Image) (string, error) { lin := srgbToLinearTable() factors := make([][3]float64, xComp*yComp) for y := range h { - row := rgba.Pix[y*rgba.Stride:] + row := src.pix[y*src.stride:] for x := range w { p := x * 4 - lr, lg, lb := lin[row[p]], lin[row[p+1]], lin[row[p+2]] + r, g, b := row[p], row[p+1], row[p+2] + if src.straight { + r, g, b = premultiply(r, g, b, row[p+3]) + } + lr, lg, lb := lin[r], lin[g], lin[b] for j := range yComp { for i := range xComp { basis := cosX[i][x] * cosY[j][y] @@ -101,15 +104,36 @@ func Encode(img image.Image) (string, error) { return sb.String(), nil } -// toRGBA gives the pixel loop direct Pix access, avoiding a per-pixel allocation via image.At. -func toRGBA(img image.Image) *image.RGBA { - if rgba, ok := img.(*image.RGBA); ok { - return rgba - } +// pixels is direct Pix access for the pixel loop, avoiding a per-pixel allocation via image.At. +type pixels struct { + pix []uint8 + stride int + w, h int + // straight marks non-premultiplied alpha, which the loop premultiplies to keep the hash + // identical to the one an equivalent *image.RGBA produces. + straight bool +} + +// pixelsOf accepts the two types the artwork pipeline produces without copying, and converts +// anything else. +func pixelsOf(img image.Image) pixels { b := img.Bounds() + switch src := img.(type) { + case *image.RGBA: + return pixels{pix: src.Pix, stride: src.Stride, w: b.Dx(), h: b.Dy()} + case *image.NRGBA: + return pixels{pix: src.Pix, stride: src.Stride, w: b.Dx(), h: b.Dy(), straight: true} + } dst := image.NewRGBA(image.Rect(0, 0, b.Dx(), b.Dy())) draw.Draw(dst, dst.Bounds(), img, b.Min, draw.Src) - return dst + return pixels{pix: dst.Pix, stride: dst.Stride, w: b.Dx(), h: b.Dy()} +} + +func premultiply(r, g, b, a uint8) (uint8, uint8, uint8) { + if a == 255 { + return r, g, b + } + return uint8(uint32(r) * uint32(a) / 255), uint8(uint32(g) * uint32(a) / 255), uint8(uint32(b) * uint32(a) / 255) } var srgbToLinearTable = sync.OnceValue(func() *[256]float64 { diff --git a/core/artwork/blurhash/blurhash_bench_test.go b/core/artwork/blurhash/blurhash_bench_test.go index 9347917d7..6adaece53 100644 --- a/core/artwork/blurhash/blurhash_bench_test.go +++ b/core/artwork/blurhash/blurhash_bench_test.go @@ -25,14 +25,14 @@ func benchImage(size int) image.Image { return img } -// BenchmarkEncodeAtInputSize mirrors the thumbhash bench of the same name. *image.RGBA, matching +// BenchmarkEncodeAtInputSize mirrors the thumbhash bench of the same name. *image.NRGBA, matching // makeThumbnail's output, so neither package is measured with a conversion the other avoids. func BenchmarkEncodeAtInputSize(b *testing.B) { const size = 100 - img := image.NewRGBA(image.Rect(0, 0, size, size)) + img := image.NewNRGBA(image.Rect(0, 0, size, size)) for y := range size { for x := range size { - img.SetRGBA(x, y, color.RGBA{ + img.SetNRGBA(x, y, color.NRGBA{ R: uint8(255 * x / size), G: uint8(255 * y / size), B: uint8((x + y) * 255 / (2 * size)), diff --git a/core/artwork/blurhash/blurhash_test.go b/core/artwork/blurhash/blurhash_test.go index b4d7c1437..8cc100b81 100644 --- a/core/artwork/blurhash/blurhash_test.go +++ b/core/artwork/blurhash/blurhash_test.go @@ -40,6 +40,54 @@ func gradientImage(w, h int) image.Image { return img } +var _ = Describe("Encode input types", func() { + // The pipeline hands Encode an *image.NRGBA; reading it must stay equivalent to the + // premultiplied *image.RGBA it used to receive, or every hash silently shifts. + buildPair := func(alpha uint8) (*image.NRGBA, *image.RGBA) { + const size = 40 + nrgba := image.NewNRGBA(image.Rect(0, 0, size, size)) + rgba := image.NewRGBA(image.Rect(0, 0, size, size)) + for y := range size { + for x := range size { + c := color.NRGBA{ + R: uint8(255 * x / size), G: uint8(255 * y / size), + B: uint8((x + y) * 255 / (2 * size)), A: alpha, + } + nrgba.SetNRGBA(x, y, c) + rgba.Set(x, y, c) // image.RGBA.Set premultiplies + } + } + return nrgba, rgba + } + + It("gives an opaque NRGBA the same hash as the equivalent RGBA", func() { + nrgba, rgba := buildPair(255) + fromNRGBA, err := blurhash.Encode(nrgba) + Expect(err).ToNot(HaveOccurred()) + fromRGBA, err := blurhash.Encode(rgba) + Expect(err).ToNot(HaveOccurred()) + Expect(fromNRGBA).To(Equal(fromRGBA)) + }) + + It("premultiplies a partly transparent NRGBA, matching the RGBA it replaces", func() { + nrgba, rgba := buildPair(128) + fromNRGBA, err := blurhash.Encode(nrgba) + Expect(err).ToNot(HaveOccurred()) + fromRGBA, err := blurhash.Encode(rgba) + Expect(err).ToNot(HaveOccurred()) + Expect(fromNRGBA).To(Equal(fromRGBA)) + }) + + It("treats a fully transparent NRGBA as black, as premultiplication does", func() { + nrgba, rgba := buildPair(0) + fromNRGBA, err := blurhash.Encode(nrgba) + Expect(err).ToNot(HaveOccurred()) + fromRGBA, err := blurhash.Encode(rgba) + Expect(err).ToNot(HaveOccurred()) + Expect(fromNRGBA).To(Equal(fromRGBA)) + }) +}) + var _ = Describe("Encode", func() { // The size flag encodes (xComp-1) + (yComp-1)*9. DescribeTable("derives component counts from aspect ratio (Jellyfin formula)", diff --git a/core/artwork/processor.go b/core/artwork/processor.go index 166054164..f8c0c6d80 100644 --- a/core/artwork/processor.go +++ b/core/artwork/processor.go @@ -268,7 +268,9 @@ func makeThumbnail(img image.Image, maxSize int) image.Image { return toFastScaleType(img) } scale := float64(maxSize) / float64(max(w, h)) - dst := image.NewRGBA(image.Rect(0, 0, max(1, int(float64(w)*scale)), max(1, int(float64(h)*scale)))) + // NRGBA, not RGBA: thumbhash requires straight alpha, and blurhash reads this type without + // converting, so neither encoder allocates a second copy of the thumbnail. + dst := image.NewNRGBA(image.Rect(0, 0, max(1, int(float64(w)*scale)), max(1, int(float64(h)*scale)))) xdraw.CatmullRom.Scale(dst, dst.Bounds(), toFastScaleType(img), b, draw.Src, nil) return dst } diff --git a/core/artwork/processor_internal_test.go b/core/artwork/processor_internal_test.go new file mode 100644 index 000000000..9165bfaf3 --- /dev/null +++ b/core/artwork/processor_internal_test.go @@ -0,0 +1,47 @@ +package artwork + +import ( + "image" + "image/color" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("makeThumbnail", func() { + // Both encoders read Pix directly and thumbhash needs straight alpha, so emitting NRGBA is + // what keeps either of them from allocating a converted copy of the thumbnail. + It("emits NRGBA when it downscales", func() { + src := image.NewRGBA(image.Rect(0, 0, 400, 300)) + Expect(makeThumbnail(src, 100)).To(BeAssignableToTypeOf(&image.NRGBA{})) + }) + + It("fits the longest side to maxSize and keeps the aspect ratio", func() { + src := image.NewRGBA(image.Rect(0, 0, 400, 300)) + b := makeThumbnail(src, 100).Bounds() + Expect(b.Dx()).To(Equal(100)) + Expect(b.Dy()).To(Equal(75)) + }) + + It("never upscales an image already within bounds", func() { + src := image.NewRGBA(image.Rect(0, 0, 40, 30)) + Expect(makeThumbnail(src, 100).Bounds()).To(Equal(src.Bounds())) + }) + + // A fully transparent pixel's colour cannot survive any resample, since the scaler works in + // premultiplied space and multiplying by zero is not invertible. Partial alpha is the case + // that distinguishes straight storage from premultiplied. + It("keeps partly transparent colour straight rather than premultiplied", func() { + src := image.NewNRGBA(image.Rect(0, 0, 400, 400)) + for y := range 400 { + for x := range 400 { + src.SetNRGBA(x, y, color.NRGBA{R: 200, G: 40, B: 90, A: 128}) + } + } + thumb, ok := makeThumbnail(src, 100).(*image.NRGBA) + Expect(ok).To(BeTrue()) + // Premultiplied storage would have halved this to ~100. + Expect(thumb.Pix[0]).To(BeNumerically("==", 200)) + Expect(thumb.Pix[3]).To(BeNumerically("==", 128)) + }) +}) diff --git a/core/artwork/thumbhash/thumbhash_bench_test.go b/core/artwork/thumbhash/thumbhash_bench_test.go index fb62078c7..6d3dfcc32 100644 --- a/core/artwork/thumbhash/thumbhash_bench_test.go +++ b/core/artwork/thumbhash/thumbhash_bench_test.go @@ -9,13 +9,13 @@ import ( "github.com/navidrome/navidrome/core/artwork/thumbhash" ) -// benchImage builds the same deterministic gradient the blurhash bench uses, as *image.RGBA — +// benchImage builds the same deterministic gradient the blurhash bench uses, as *image.NRGBA — // the concrete type makeThumbnail hands both encoders in production. -func benchImage(size int) *image.RGBA { - img := image.NewRGBA(image.Rect(0, 0, size, size)) +func benchImage(size int) *image.NRGBA { + img := image.NewNRGBA(image.Rect(0, 0, size, size)) for y := range size { for x := range size { - img.SetRGBA(x, y, color.RGBA{ + img.SetNRGBA(x, y, color.NRGBA{ R: uint8(255 * x / size), G: uint8(255 * y / size), B: uint8((x + y) * 255 / (2 * size)),