From 2bee12f2fbf2d79cef46eecab135e9be9ef0d33b Mon Sep 17 00:00:00 2001 From: Deluan Date: Tue, 28 Jul 2026 22:08:28 -0400 Subject: [PATCH] refactor: simplify the artwork enqueue and blurhash paths Cleanup pass over the three preceding commits. Tabulate the cosine terms in the UI blurhash decoder instead of calling Math.cos per pixel per component: 248us -> 75us for a 32x32 decode, and an album grid mounts one decoder per tile. Output is unchanged, which the pinned pixel specs enforce. The Go encoder already tabulated the same terms. Drop the dead paths that deriving components inside Encode left behind: the zero-size guard in components, the post-downscale empty check, and the no-AC-factor branch, which cannot be reached now that the counts are always at least 1x9. The empty-image check moves ahead of the derivation, where it belongs. In the queue mock, look up item_artwork by its existing iaKey rather than scanning the map, and hold the lock across EnqueueIfMissing through a shared unlocked helper instead of releasing it mid-operation. Extract the duplicated drain-and-resolve block in the scanner specs into one helper. --- core/artwork/blurhash/blurhash.go | 27 +++++++----------- persistence/artwork_queue_repository.go | 2 +- scanner/scanner_test.go | 38 ++++++++++++------------- tests/mock_artwork_queue_repo.go | 36 +++++++++-------------- ui/src/utils/blurhash.js | 20 +++++++++++-- 5 files changed, 60 insertions(+), 63 deletions(-) diff --git a/core/artwork/blurhash/blurhash.go b/core/artwork/blurhash/blurhash.go index b5fddcf79..5dfd8754f 100644 --- a/core/artwork/blurhash/blurhash.go +++ b/core/artwork/blurhash/blurhash.go @@ -20,9 +20,6 @@ const maxInputSize = 128 // components picks x/y component counts targeting ~16 near-square tiles. func components(width, height int) (int, int) { - if width <= 0 || height <= 0 { - return 0, 0 - } xf := math.Sqrt(16.0 * float64(width) / float64(height)) yf := xf * float64(height) / float64(width) return min(int(xf)+1, 9), min(int(yf)+1, 9) @@ -30,14 +27,14 @@ func components(width, height int) (int, int) { // Encode returns the blurhash of img, deriving the component counts from its aspect ratio. func Encode(img image.Image) (string, error) { + if img.Bounds().Dx() == 0 || img.Bounds().Dy() == 0 { + return "", errors.New("blurhash: empty image") + } // 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() - if w == 0 || h == 0 { - return "", errors.New("blurhash: empty image") - } cosX := make([][]float64, xComp) for i := range cosX { @@ -86,19 +83,15 @@ func Encode(img image.Image) (string, error) { var sb strings.Builder sb.WriteString(Encode83((xComp-1)+(yComp-1)*9, 1)) + // Derived counts are at least 1x9, so there is always at least one AC factor. ac := factors[1:] - maxVal := 1.0 - if len(ac) > 0 { - actualMax := 0.0 - for _, f := range ac { - actualMax = max(actualMax, math.Abs(f[0]), math.Abs(f[1]), math.Abs(f[2])) - } - quantMax := int(max(0, min(82, math.Floor(actualMax*166-0.5)))) - maxVal = float64(quantMax+1) / 166 - sb.WriteString(Encode83(quantMax, 1)) - } else { - sb.WriteString(Encode83(0, 1)) + actualMax := 0.0 + for _, f := range ac { + actualMax = max(actualMax, math.Abs(f[0]), math.Abs(f[1]), math.Abs(f[2])) } + quantMax := int(max(0, min(82, math.Floor(actualMax*166-0.5)))) + maxVal := float64(quantMax+1) / 166 + sb.WriteString(Encode83(quantMax, 1)) dc := factors[0] sb.WriteString(Encode83(linearToSRGB(dc[0])<<16|linearToSRGB(dc[1])<<8|linearToSRGB(dc[2]), 4)) diff --git a/persistence/artwork_queue_repository.go b/persistence/artwork_queue_repository.go index e9f7e06a9..69cb17d2f 100644 --- a/persistence/artwork_queue_repository.go +++ b/persistence/artwork_queue_repository.go @@ -13,7 +13,7 @@ import ( "github.com/pocketbase/dbx" ) -// Keeps each multi-row insert under SQLite's bind-variable limit (7 cols -> 700 vars). +// Keeps each multi-row insert under SQLite's bind-variable limit (at most 7 vars per row). const enqueueChunkSize = 100 type artworkQueueRepository struct { diff --git a/scanner/scanner_test.go b/scanner/scanner_test.go index 755ddd9e5..6098cbd66 100644 --- a/scanner/scanner_test.go +++ b/scanner/scanner_test.go @@ -97,6 +97,22 @@ var _ = Describe("Scanner", Ordered, func() { return err } + // Stands in for the artwork worker: drains the queue and records every item as resolved, + // so a later scan can only queue genuine reprocessing. + resolveQueuedArtwork := func() []model.ArtworkQueueItem { + GinkgoHelper() + queued, err := ds.ArtworkQueue(ctx).DequeueBatch(1000) + Expect(err).ToNot(HaveOccurred()) + for _, it := range queued { + Expect(ds.Artwork(ctx).PutItemArtwork(&model.ItemArtwork{ + ItemKind: it.ItemKind, ItemID: it.ItemID, ImageType: it.ImageType, + Hash: "resolved", Source: "embedded", UpdatedAt: time.Now(), + })).To(Succeed()) + Expect(ds.ArtworkQueue(ctx).DeleteIfUnchanged(it.ItemKind, it.ItemID, it.ImageType, it.RetryAt)).To(Succeed()) + } + return queued + } + Context("Simple library, 'artis/album/track - title.mp3'", func() { var help, revolver func(...map[string]any) *fstest.MapFile var fsys storagetest.FakeFS @@ -177,17 +193,7 @@ var _ = Describe("Scanner", Ordered, func() { It("should not re-enqueue already resolved artwork on a repeat full scan", func() { Expect(runScanner(ctx, true)).To(Succeed()) - // Stand in for the worker: once resolved, a second scan can only queue reprocessing. - queued, err := ds.ArtworkQueue(ctx).DequeueBatch(1000) - Expect(err).ToNot(HaveOccurred()) - Expect(queued).ToNot(BeEmpty()) - for _, it := range queued { - Expect(ds.Artwork(ctx).PutItemArtwork(&model.ItemArtwork{ - ItemKind: it.ItemKind, ItemID: it.ItemID, ImageType: it.ImageType, - Hash: "resolved", Source: "embedded", UpdatedAt: time.Now(), - })).To(Succeed()) - Expect(ds.ArtworkQueue(ctx).DeleteIfUnchanged(it.ItemKind, it.ItemID, it.ImageType, it.RetryAt)).To(Succeed()) - } + Expect(resolveQueuedArtwork()).ToNot(BeEmpty()) Expect(runScanner(ctx, true)).To(Succeed()) @@ -216,15 +222,7 @@ var _ = Describe("Scanner", Ordered, func() { tests.SkipOnWindows("path separator bug (#TBD-path-sep-scanner)") Expect(runScanner(ctx, true)).To(Succeed()) - queued, err := ds.ArtworkQueue(ctx).DequeueBatch(1000) - Expect(err).ToNot(HaveOccurred()) - for _, it := range queued { - Expect(ds.Artwork(ctx).PutItemArtwork(&model.ItemArtwork{ - ItemKind: it.ItemKind, ItemID: it.ItemID, ImageType: it.ImageType, - Hash: "resolved", Source: "embedded", UpdatedAt: time.Now(), - })).To(Succeed()) - Expect(ds.ArtworkQueue(ctx).DeleteIfUnchanged(it.ItemKind, it.ItemID, it.ImageType, it.RetryAt)).To(Succeed()) - } + resolveQueuedArtwork() fsys.UpdateTags("The Beatles/Help!/01 - Help!.mp3", _t{"producer": "George Martin"}) Expect(runScanner(ctx, false)).To(Succeed()) diff --git a/tests/mock_artwork_queue_repo.go b/tests/mock_artwork_queue_repo.go index b3b3a04d0..a6e5173a6 100644 --- a/tests/mock_artwork_queue_repo.go +++ b/tests/mock_artwork_queue_repo.go @@ -31,6 +31,11 @@ func (m *MockArtworkQueueRepo) Enqueue(items ...model.ArtworkQueueItem) error { if m.Err != nil { return m.Err } + m.enqueueLocked(items) + return nil +} + +func (m *MockArtworkQueueRepo) enqueueLocked(items []model.ArtworkQueueItem) { now := time.Now() for _, it := range items { if it.ImageType == "" { @@ -51,43 +56,30 @@ func (m *MockArtworkQueueRepo) Enqueue(items ...model.ArtworkQueueItem) error { it.EnqueuedAt = now m.Data[k] = it } - return nil } // EnqueueIfMissing mirrors the SQL anti-join: skip anything that already has an item_artwork row. func (m *MockArtworkQueueRepo) EnqueueIfMissing(items ...model.ArtworkQueueItem) error { m.mu.Lock() + defer m.mu.Unlock() if m.Err != nil { - m.mu.Unlock() return m.Err } - hasRow := func(kind, id, imageType string) bool { - if m.ItemArtworkSource == nil { - return false - } - for _, ia := range m.ItemArtworkSource.ItemData { - if ia.ItemKind == kind && ia.ItemID == id && ia.ImageType == imageType { - return true - } - } - return false - } var fresh []model.ArtworkQueueItem for _, it := range items { - imageType := cmp.Or(it.ImageType, model.ImageTypePrimary) - if hasRow(it.ItemKind, it.ItemID, imageType) { - continue + k := iaKey(it.ItemKind, it.ItemID, cmp.Or(it.ImageType, model.ImageTypePrimary)) + if m.ItemArtworkSource != nil { + if _, ok := m.ItemArtworkSource.ItemData[k]; ok { + continue + } } - if _, ok := m.Data[iaKey(it.ItemKind, it.ItemID, imageType)]; ok { // DO NOTHING + if _, ok := m.Data[k]; ok { // DO NOTHING continue } fresh = append(fresh, it) } - m.mu.Unlock() - if len(fresh) == 0 { - return nil - } - return m.Enqueue(fresh...) + m.enqueueLocked(fresh) + return nil } func (m *MockArtworkQueueRepo) DequeueBatch(n int, kinds ...string) ([]model.ArtworkQueueItem, error) { diff --git a/ui/src/utils/blurhash.js b/ui/src/utils/blurhash.js index 5b220f450..38899f28e 100644 --- a/ui/src/utils/blurhash.js +++ b/ui/src/utils/blurhash.js @@ -64,6 +64,21 @@ export const decode = (hash, width, height) => { ) } + // Tabulated rather than called per pixel per component: a 32x32 decode would otherwise make + // tens of thousands of Math.cos calls, and a grid page mounts one of these per tile. + const cosX = new Float64Array(width * numX) + for (let i = 0; i < numX; i++) { + for (let x = 0; x < width; x++) { + cosX[i * width + x] = Math.cos((Math.PI * x * i) / width) + } + } + const cosY = new Float64Array(height * numY) + for (let j = 0; j < numY; j++) { + for (let y = 0; y < height; y++) { + cosY[j * height + y] = Math.cos((Math.PI * y * j) / height) + } + } + const bytesPerRow = width * 4 const pixels = new Uint8ClampedArray(bytesPerRow * height) for (let y = 0; y < height; y++) { @@ -72,10 +87,9 @@ export const decode = (hash, width, height) => { let g = 0 let b = 0 for (let j = 0; j < numY; j++) { + const basisY = cosY[j * height + y] for (let i = 0; i < numX; i++) { - const basis = - Math.cos((Math.PI * x * i) / width) * - Math.cos((Math.PI * y * j) / height) + const basis = cosX[i * width + x] * basisY const color = colors[i + j * numX] r += color[0] * basis g += color[1] * basis