From 3bfddd08622da09718039ab6df4c4d477aee7cf4 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 6 Oct 2026 08:28:29 -0400 Subject: [PATCH 01/18] feat(auth): throttle login, register, password reset and Subsonic auth failures (M462 #4976) Every password-shaped check was mounted bare, so guessing was limited only by bcrypt cost. A shared in-memory AttemptLimiter now sits in front of them: - login: 10 failures per account and 50 per address per 15 min, checked before the user lookup and bcrypt; 429 with Retry-After. A success clears the account's count but not the address's. - unknown usernames run a dummy bcrypt compare, so timing no longer says which accounts exist. - register: 10 per address per hour; forgot-password: 5 per address and 3 per email per hour (applied whether or not the email matches); reset: 20 failed tokens per address per 15 min. - Subsonic /rest: same limits as login, counting only wrong credentials, since clients authenticate on every request. Web login, register, reset and forgot-password screens say how long to wait; web and Android carry copy for the rate_limited code. Co-Authored-By: Claude Opus 5.5 --- .../com/fabledsword/minstrel/api/ErrorCopy.kt | 1 + internal/api/api.go | 14 ++ internal/api/auth.go | 30 ++- internal/api/auth_forgot.go | 14 ++ internal/api/auth_register.go | 11 +- internal/api/auth_reset.go | 12 + internal/api/auth_test.go | 30 +++ internal/api/errors.go | 14 ++ internal/apierror/apierror.go | 4 + internal/auth/ratelimit.go | 205 ++++++++++++++++++ internal/auth/ratelimit_test.go | 114 ++++++++++ internal/server/server.go | 2 +- internal/subsonic/auth.go | 26 ++- internal/subsonic/subsonic.go | 8 +- web/src/lib/api/client.ts | 21 ++ web/src/lib/styles/error-copy.json | 1 + web/src/routes/forgot-password/+page.svelte | 21 +- web/src/routes/login/+page.svelte | 7 +- web/src/routes/register/+page.svelte | 3 +- .../reset-password/[token]/+page.svelte | 6 +- 20 files changed, 528 insertions(+), 16 deletions(-) create mode 100644 internal/auth/ratelimit.go create mode 100644 internal/auth/ratelimit_test.go diff --git a/android/app/src/main/java/com/fabledsword/minstrel/api/ErrorCopy.kt b/android/app/src/main/java/com/fabledsword/minstrel/api/ErrorCopy.kt index 3f345851..9a7314ad 100644 --- a/android/app/src/main/java/com/fabledsword/minstrel/api/ErrorCopy.kt +++ b/android/app/src/main/java/com/fabledsword/minstrel/api/ErrorCopy.kt @@ -75,6 +75,7 @@ object ErrorCopy { "forbidden" to "You don't have permission to do that.", "not_authorized" to "You don't have permission to do that.", "invalid_credentials" to "Wrong username or password.", + "rate_limited" to "Too many attempts. Wait a few minutes and try again.", "wrong_password" to "Current password is incorrect.", "password_too_short" to "Password must be at least 8 characters.", "username_invalid" to "That username isn't valid.", diff --git a/internal/api/api.go b/internal/api/api.go index 6f21467c..f1519c9e 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -7,6 +7,7 @@ package api import ( "log/slog" "math/rand" + "time" "github.com/go-chi/chi/v5" "github.com/jackc/pgx/v5/pgxpool" @@ -58,6 +59,11 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev reacqSettings: reacqSettings, fingerprintSettings: fpSettings, librarySize: recommendation.NewLibrarySize(nil), + loginGuard: auth.NewLoginGuard(), + registerLimit: auth.NewAttemptLimiter(registerPerAddressMax, time.Hour), + forgotAddressLimit: auth.NewAttemptLimiter(forgotPerAddressMax, time.Hour), + forgotEmailLimit: auth.NewAttemptLimiter(forgotPerEmailMax, time.Hour), + resetLimit: auth.NewAttemptLimiter(resetFailuresPerAddressMax, 15*time.Minute), } r.Route("/api", func(api chi.Router) { @@ -313,6 +319,14 @@ type handlers struct { // instance the scanner and the fingerprint workers read, so a save from the // admin card reaches them without a restart. Nil serves the defaults. fingerprintSettings *library.FingerprintSettingsService + // loginGuard throttles failed logins per account and per address, and + // the limiters below cap the other unauthenticated auth routes. All are + // nil-safe, so tests that build handlers directly run unthrottled. + loginGuard *auth.LoginGuard + registerLimit *auth.AttemptLimiter + forgotAddressLimit *auth.AttemptLimiter + forgotEmailLimit *auth.AttemptLimiter + resetLimit *auth.AttemptLimiter // netSettings caches the trusted reverse-proxy depth read by the auth // middleware on every request and edited from the admin network card. netSettings *netsettings.Service diff --git a/internal/api/auth.go b/internal/api/auth.go index ac125463..6624ed39 100644 --- a/internal/api/auth.go +++ b/internal/api/auth.go @@ -19,6 +19,20 @@ import ( // so an abandoned laptop doesn't stay logged in forever. const sessionCookieMaxAge = 30 * 24 * time.Hour +// Limits on the unauthenticated auth routes other than login (which uses +// auth.LoginGuard). Per address, per window as wired in Mount. +const ( + // registerPerAddressMax: 10 registrations an hour. Every attempt counts, + // typos included, which is still far above a household's need. + registerPerAddressMax = 10 + // forgotPerAddressMax / forgotPerEmailMax: reset emails an hour. The + // per-email cap is what keeps one inbox from being mailbombed. + forgotPerAddressMax = 5 + forgotPerEmailMax = 3 + // resetFailuresPerAddressMax: wrong or expired reset tokens per 15 min. + resetFailuresPerAddressMax = 20 +) + func (h *handlers) handleLogout(w http.ResponseWriter, r *http.Request) { // The session token can be on the cookie OR bearer header — RequireUser // accepted either. Re-resolve it here so we can delete the row. @@ -70,10 +84,22 @@ func (h *handlers) handleLogin(w http.ResponseWriter, r *http.Request) { return } + // Checked before any lookup or bcrypt, so a throttled guess costs the + // server nothing and learns nothing. + addr := auth.ClientIP(r, h.netSettings.Hops()) + if blocked, wait := h.loginGuard.Blocked(req.Username, addr); blocked { + writeRateLimited(w, wait) + return + } + q := dbq.New(h.pool) user, err := q.GetUserByUsername(r.Context(), req.Username) if err != nil { if errors.Is(err, pgx.ErrNoRows) { + // Same bcrypt time as a wrong password, so timing doesn't say + // which usernames exist. + auth.DummyVerify(req.Password) + h.loginGuard.Fail(req.Username, addr) writeErr(w, apierror.Unauthorized("invalid_credentials", "invalid username or password")) return } @@ -82,9 +108,11 @@ func (h *handlers) handleLogin(w http.ResponseWriter, r *http.Request) { return } if !auth.VerifyPassword(user.PasswordHash, req.Password) { + h.loginGuard.Fail(req.Username, addr) writeErr(w, apierror.Unauthorized("invalid_credentials", "invalid username or password")) return } + h.loginGuard.Succeed(req.Username) token, err := auth.MintSessionToken() if err != nil { @@ -100,7 +128,7 @@ func (h *handlers) handleLogin(w http.ResponseWriter, r *http.Request) { // the active-sessions surface: a session that was born somewhere the // user recognises but is being used from somewhere they don't is the // case this whole surface exists to surface. - Ip: auth.ClientIP(r, h.netSettings.Hops()), + Ip: addr, }); err != nil { h.logger.Error("api: insert session failed", "err", err) writeErr(w, apierror.InternalMsg("insert failed", err)) diff --git a/internal/api/auth_forgot.go b/internal/api/auth_forgot.go index 45aa26f0..7491f6d3 100644 --- a/internal/api/auth_forgot.go +++ b/internal/api/auth_forgot.go @@ -14,6 +14,7 @@ import ( "github.com/jackc/pgx/v5/pgtype" "git.fabledsword.com/bvandeusen/minstrel/internal/audit" + "git.fabledsword.com/bvandeusen/minstrel/internal/auth" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" "git.fabledsword.com/bvandeusen/minstrel/internal/mailer" ) @@ -47,6 +48,19 @@ func (h *handlers) handleForgotPassword(w http.ResponseWriter, r *http.Request) } email := strings.ToLower(strings.TrimSpace(req.Email)) + // Throttled per address (a spray) and per email (a mailbomb of one + // inbox). Applied whether or not the email matches, so a 429 says + // nothing about which addresses are registered. + addr := auth.ClientIP(r, h.netSettings.Hops()) + blockedAddr, waitAddr := h.forgotAddressLimit.Blocked(addr) + blockedEmail, waitEmail := h.forgotEmailLimit.Blocked(email) + if blockedAddr || blockedEmail { + writeRateLimited(w, max(waitAddr, waitEmail)) + return + } + h.forgotAddressLimit.Record(addr) + h.forgotEmailLimit.Record(email) + q := dbq.New(h.pool) var matched bool var auditTarget pgtype.UUID diff --git a/internal/api/auth_register.go b/internal/api/auth_register.go index 6f098787..710f7602 100644 --- a/internal/api/auth_register.go +++ b/internal/api/auth_register.go @@ -55,6 +55,15 @@ type registerReq struct { // - 409 username_taken (PG unique violation on users.username) // - 500 server_error otherwise func (h *handlers) handleRegister(w http.ResponseWriter, r *http.Request) { + // Every attempt counts, not only failures: a successful registration is + // the thing being sprayed when the mode is open. + addr := auth.ClientIP(r, h.netSettings.Hops()) + if blocked, wait := h.registerLimit.Blocked(addr); blocked { + writeRateLimited(w, wait) + return + } + h.registerLimit.Record(addr) + var req registerReq if err := json.NewDecoder(r.Body).Decode(&req); err != nil { writeErr(w, apierror.BadRequest("invalid_body", "invalid JSON body")) @@ -175,7 +184,7 @@ func (h *handlers) handleRegister(w http.ResponseWriter, r *http.Request) { UserID: user.ID, TokenHash: auth.HashSessionToken(sessionToken), UserAgent: r.UserAgent(), - Ip: auth.ClientIP(r, h.netSettings.Hops()), + Ip: addr, }); err != nil { h.logger.Error("register: insert session failed", "err", err) writeErr(w, apierror.Internal(err)) diff --git a/internal/api/auth_reset.go b/internal/api/auth_reset.go index 48a7b070..3982d918 100644 --- a/internal/api/auth_reset.go +++ b/internal/api/auth_reset.go @@ -10,6 +10,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/apierror" "git.fabledsword.com/bvandeusen/minstrel/internal/audit" + "git.fabledsword.com/bvandeusen/minstrel/internal/auth" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" ) @@ -42,6 +43,15 @@ func (h *handlers) handleResetPassword(w http.ResponseWriter, r *http.Request) { return } + // Tokens carry 256 bits, so guessing one is hopeless; the limit is on + // failed tries per address so that nobody gets to find that out at our + // expense. + addr := auth.ClientIP(r, h.netSettings.Hops()) + if blocked, wait := h.resetLimit.Blocked(addr); blocked { + writeRateLimited(w, wait) + return + } + q := dbq.New(h.pool) // Look up the reset record so we know which user to update before @@ -50,6 +60,7 @@ func (h *handlers) handleResetPassword(w http.ResponseWriter, r *http.Request) { reset, err := q.GetPasswordReset(r.Context(), req.Token) if err != nil { if errors.Is(err, pgx.ErrNoRows) { + h.resetLimit.Record(addr) writeErr(w, apierror.BadRequest("invalid_token", "")) return } @@ -69,6 +80,7 @@ func (h *handlers) handleResetPassword(w http.ResponseWriter, r *http.Request) { return } if rows == 0 { + h.resetLimit.Record(addr) writeErr(w, apierror.BadRequest("invalid_token", "")) return } diff --git a/internal/api/auth_test.go b/internal/api/auth_test.go index 171c0f8a..4e445e6f 100644 --- a/internal/api/auth_test.go +++ b/internal/api/auth_test.go @@ -179,6 +179,36 @@ func TestHandleLogin_UnknownUserReturns401(t *testing.T) { } } +// A guesser who reaches the account limit is refused with 429 before any +// password check, so even the right password is turned away until the window +// passes, and the response says when to come back. +func TestHandleLogin_ThrottledAfterRepeatedFailures(t *testing.T) { + h, pool := testHandlers(t) + h.loginGuard = auth.NewLoginGuard() + seedUser(t, pool, "alice", "hunter2", false) + + login := func(password string) *httptest.ResponseRecorder { + body := strings.NewReader(`{"username":"test-alice","password":"` + password + `"}`) + req := httptest.NewRequest(http.MethodPost, "/api/auth/login", body) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + h.handleLogin(w, req) + return w + } + for i := 0; i < 10; i++ { + if w := login("wrong"); w.Code != http.StatusUnauthorized { + t.Fatalf("attempt %d: status = %d, want 401", i+1, w.Code) + } + } + w := login("hunter2") + if w.Code != http.StatusTooManyRequests { + t.Fatalf("status = %d, want 429 once the account limit is reached", w.Code) + } + if w.Header().Get("Retry-After") == "" { + t.Error("429 without Retry-After") + } +} + func TestHandleLogin_MalformedBodyReturns400(t *testing.T) { h, _ := testHandlers(t) req := httptest.NewRequest(http.MethodPost, "/api/auth/login", diff --git a/internal/api/errors.go b/internal/api/errors.go index 58410641..be7c0427 100644 --- a/internal/api/errors.go +++ b/internal/api/errors.go @@ -3,7 +3,10 @@ package api import ( "encoding/json" "log/slog" + "math" "net/http" + "strconv" + "time" "git.fabledsword.com/bvandeusen/minstrel/internal/apierror" ) @@ -36,6 +39,17 @@ func writeErr(w http.ResponseWriter, err error) { }}) } +// writeRateLimited answers 429 with Retry-After in whole seconds, rounded +// up so a client that honours it never arrives a moment early. +func writeRateLimited(w http.ResponseWriter, wait time.Duration) { + secs := int(math.Ceil(wait.Seconds())) + if secs < 1 { + secs = 1 + } + w.Header().Set("Retry-After", strconv.Itoa(secs)) + writeErr(w, apierror.TooManyRequests("rate_limited", "too many attempts; try again later")) +} + // writeErrWithLog logs the error at Error level and writes the response. // Use for 500-class errors where the operator needs the cause in logs. func writeErrWithLog(w http.ResponseWriter, logger *slog.Logger, msg string, err error) { diff --git a/internal/apierror/apierror.go b/internal/apierror/apierror.go index 3b876499..7d776809 100644 --- a/internal/apierror/apierror.go +++ b/internal/apierror/apierror.go @@ -54,6 +54,10 @@ func Unauthorized(code, message string) *Error { return &Error{Status: 401, Code: code, Message: message} } +func TooManyRequests(code, message string) *Error { + return &Error{Status: 429, Code: code, Message: message} +} + func Internal(cause error) *Error { return &Error{Status: 500, Code: "server_error", Message: "internal server error", Cause: cause} } diff --git a/internal/auth/ratelimit.go b/internal/auth/ratelimit.go new file mode 100644 index 00000000..849aad73 --- /dev/null +++ b/internal/auth/ratelimit.go @@ -0,0 +1,205 @@ +package auth + +import ( + "strings" + "sync" + "time" + + "golang.org/x/crypto/bcrypt" +) + +// AttemptLimiter caps how many attempts a key may make inside a fixed +// window. It is the throttle in front of every password-shaped check: +// native login, register, forgot/reset password and Subsonic /rest auth. +// +// In memory on purpose. Minstrel is a single process, the counts only need +// to outlive a guessing run rather than a restart, and a table would put a +// write on every failed login. Each key costs one small struct, and expired +// keys are swept as the map grows, so a spray across many addresses cannot +// hold memory past one window. +type AttemptLimiter struct { + max int + window time.Duration + now func() time.Time + + mu sync.Mutex + buckets map[string]*attemptBucket + nextSweep int +} + +type attemptBucket struct { + count int + start time.Time +} + +// sweepFloor is the map size below which expired keys are left in place; +// past it, a sweep runs whenever the map doubles from its last swept size. +const sweepFloor = 1024 + +// NewAttemptLimiter returns a limiter allowing max attempts per key per +// window. +func NewAttemptLimiter(max int, window time.Duration) *AttemptLimiter { + return &AttemptLimiter{ + max: max, + window: window, + now: time.Now, + buckets: map[string]*attemptBucket{}, + nextSweep: sweepFloor, + } +} + +// Blocked reports whether key has used its attempts for the current window, +// and if so how long until the window resets. It records nothing, so a check +// can run before the expensive work and the outcome be recorded after. +func (l *AttemptLimiter) Blocked(key string) (bool, time.Duration) { + if l == nil || key == "" { + return false, 0 + } + l.mu.Lock() + defer l.mu.Unlock() + b, ok := l.buckets[key] + if !ok { + return false, 0 + } + now := l.now() + if now.Sub(b.start) >= l.window { + delete(l.buckets, key) + return false, 0 + } + if b.count < l.max { + return false, 0 + } + return true, b.start.Add(l.window).Sub(now) +} + +// Record counts one attempt against key. +func (l *AttemptLimiter) Record(key string) { + if l == nil || key == "" { + return + } + l.mu.Lock() + defer l.mu.Unlock() + now := l.now() + b, ok := l.buckets[key] + if !ok || now.Sub(b.start) >= l.window { + l.buckets[key] = &attemptBucket{count: 1, start: now} + l.maybeSweep(now) + return + } + b.count++ +} + +// Reset forgets key, as after a successful login: the user who finally got +// their password right should not carry their typos into the next window. +func (l *AttemptLimiter) Reset(key string) { + if l == nil || key == "" { + return + } + l.mu.Lock() + defer l.mu.Unlock() + delete(l.buckets, key) +} + +// maybeSweep drops expired buckets once the map has doubled since the last +// sweep. Callers hold l.mu. +func (l *AttemptLimiter) maybeSweep(now time.Time) { + if len(l.buckets) < l.nextSweep { + return + } + for k, b := range l.buckets { + if now.Sub(b.start) >= l.window { + delete(l.buckets, k) + } + } + l.nextSweep = max(sweepFloor, 2*len(l.buckets)) +} + +// LoginGuard pairs a per-account and a per-address limiter, the shape every +// password check uses. The account limit stops a slow guess at one user from +// many addresses; the address limit stops one address spraying many users. +// Only failures are recorded, so a user who signs in correctly is never +// counted at all. +type LoginGuard struct { + account *AttemptLimiter + address *AttemptLimiter +} + +// Login limits: 10 failures per account and 50 per address per 15 minutes, +// the same numbers ThoughtSync settled on. Generous enough that a user +// fumbling a password manager never meets them, tight enough that an online +// guess against bcrypt gets ~1,000 tries a day per account. +const ( + loginWindow = 15 * time.Minute + loginAccountMax = 10 + loginAddressMax = 50 +) + +// NewLoginGuard returns a guard with the default login limits. +func NewLoginGuard() *LoginGuard { + return &LoginGuard{ + account: NewAttemptLimiter(loginAccountMax, loginWindow), + address: NewAttemptLimiter(loginAddressMax, loginWindow), + } +} + +// Blocked reports whether either the account or the address is over its +// limit, with the longer of the two waits. Check it BEFORE verifying the +// password, so a blocked guess costs no bcrypt. +func (g *LoginGuard) Blocked(account, address string) (bool, time.Duration) { + if g == nil { + return false, 0 + } + aBlocked, aWait := g.account.Blocked(accountKey(account)) + ipBlocked, ipWait := g.address.Blocked(address) + return aBlocked || ipBlocked, max(aWait, ipWait) +} + +// Fail records a failed attempt against both the account and the address. +// An unknown username counts against its name all the same, so the limit +// gives away nothing about which accounts exist. +func (g *LoginGuard) Fail(account, address string) { + if g == nil { + return + } + g.account.Record(accountKey(account)) + g.address.Record(address) +} + +// Succeed clears the account's failures. The address keeps its count: one +// address that guessed fifty accounts and got one right is still spraying. +func (g *LoginGuard) Succeed(account string) { + if g == nil { + return + } + g.account.Reset(accountKey(account)) +} + +// accountKey folds case so "Admin" and "admin" share one budget. Usernames +// are compared exactly by the lookup, but the guesser shouldn't get a fresh +// allowance per capitalisation. +func accountKey(account string) string { + return strings.ToLower(strings.TrimSpace(account)) +} + +var ( + dummyHashOnce sync.Once + dummyHash []byte +) + +// DummyVerify spends the same bcrypt time a real password check would, for +// the path where the username doesn't exist. Without it, an unknown user +// answers in microseconds and a known one in ~50ms, and the uniform error +// message hides nothing. +func DummyVerify(plaintext string) { + dummyHashOnce.Do(func() { + // What the hash is of doesn't matter — no account carries it. Its + // cost does: DefaultCost, the same as every stored password. + h, err := bcrypt.GenerateFromPassword([]byte("minstrel-dummy-password"), bcrypt.DefaultCost) + if err == nil { + dummyHash = h + } + }) + if dummyHash != nil { + _ = bcrypt.CompareHashAndPassword(dummyHash, []byte(plaintext)) + } +} diff --git a/internal/auth/ratelimit_test.go b/internal/auth/ratelimit_test.go new file mode 100644 index 00000000..6560fb74 --- /dev/null +++ b/internal/auth/ratelimit_test.go @@ -0,0 +1,114 @@ +package auth + +import ( + "testing" + "time" +) + +// fakeClock lets a test step through a window without sleeping. +type fakeClock struct{ t time.Time } + +func (c *fakeClock) now() time.Time { return c.t } + +func newTestLimiter(max int, window time.Duration) (*AttemptLimiter, *fakeClock) { + c := &fakeClock{t: time.Date(2026, 10, 6, 12, 0, 0, 0, time.UTC)} + l := NewAttemptLimiter(max, window) + l.now = c.now + return l, c +} + +func TestAttemptLimiter_BlocksAtMaxAndReportsWait(t *testing.T) { + l, c := newTestLimiter(3, 15*time.Minute) + for i := 0; i < 3; i++ { + if blocked, _ := l.Blocked("k"); blocked { + t.Fatalf("blocked after %d attempts, want allowed below max", i) + } + l.Record("k") + } + c.t = c.t.Add(5 * time.Minute) + blocked, wait := l.Blocked("k") + if !blocked { + t.Fatal("not blocked after max attempts") + } + if wait != 10*time.Minute { + t.Errorf("wait = %v, want the 10m left in the window", wait) + } +} + +func TestAttemptLimiter_WindowExpiryClears(t *testing.T) { + l, c := newTestLimiter(1, time.Minute) + l.Record("k") + if blocked, _ := l.Blocked("k"); !blocked { + t.Fatal("want blocked inside the window") + } + c.t = c.t.Add(time.Minute) + if blocked, _ := l.Blocked("k"); blocked { + t.Fatal("still blocked once the window has passed") + } +} + +func TestAttemptLimiter_KeysAreIndependentAndResetClears(t *testing.T) { + l, _ := newTestLimiter(1, time.Minute) + l.Record("a") + if blocked, _ := l.Blocked("b"); blocked { + t.Fatal("one key's attempts blocked another") + } + l.Reset("a") + if blocked, _ := l.Blocked("a"); blocked { + t.Fatal("Reset did not clear the key") + } +} + +func TestAttemptLimiter_SweepDropsExpiredKeys(t *testing.T) { + l, c := newTestLimiter(5, time.Minute) + // One short of the floor: no sweep yet, however stale these become. + for i := 0; i < sweepFloor-1; i++ { + l.Record("k" + time.Duration(i).String()) + } + c.t = c.t.Add(2 * time.Minute) + l.Record("fresh") // reaches the floor and triggers a sweep + if n := len(l.buckets); n != 1 { + t.Errorf("buckets after sweep = %d, want only the fresh key", n) + } +} + +func TestAttemptLimiter_NilAndEmptyKeyAreNoOps(t *testing.T) { + var l *AttemptLimiter + l.Record("k") + if blocked, _ := l.Blocked("k"); blocked { + t.Error("nil limiter blocked") + } + real, _ := newTestLimiter(1, time.Minute) + real.Record("") + if blocked, _ := real.Blocked(""); blocked { + t.Error("empty key was counted") + } +} + +func TestLoginGuard_AccountLimitSpansAddressesAndFoldsCase(t *testing.T) { + g := NewLoginGuard() + for i := 0; i < loginAccountMax; i++ { + g.Fail("Alice", "10.0.0."+string(rune('0'+i%10))) + } + if blocked, _ := g.Blocked("alice", "192.0.2.1"); !blocked { + t.Fatal("account limit should hold from a fresh address and any capitalisation") + } + g.Succeed("ALICE") + if blocked, _ := g.Blocked("alice", "192.0.2.1"); blocked { + t.Fatal("Succeed should clear the account's failures") + } +} + +func TestLoginGuard_AddressLimitSpansAccounts(t *testing.T) { + g := NewLoginGuard() + for i := 0; i < loginAddressMax; i++ { + g.Fail("user"+time.Duration(i).String(), "203.0.113.9") + } + if blocked, _ := g.Blocked("someone-new", "203.0.113.9"); !blocked { + t.Fatal("address that sprayed many accounts should be blocked") + } + g.Succeed("someone-new") + if blocked, _ := g.Blocked("someone-new", "203.0.113.9"); !blocked { + t.Fatal("a success must not clear the address count") + } +} diff --git a/internal/server/server.go b/internal/server/server.go index 94c59ddd..b0398379 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -220,7 +220,7 @@ func (s *Server) Router() http.Handler { r.With(auth.RequireUser(s.Pool, netSettings.Hops), auth.RequireAdmin()). Post("/api/admin/scan", s.handleAdminScan) } - subsonic.Mount(r, s.Pool, s.Logger, s.SubsonicCfg, writer) + subsonic.Mount(r, s.Pool, s.Logger, s.SubsonicCfg, writer, netSettings.Hops) } spa := web.Handler(s.BrandingCfg) diff --git a/internal/subsonic/auth.go b/internal/subsonic/auth.go index b651588d..0973011f 100644 --- a/internal/subsonic/auth.go +++ b/internal/subsonic/auth.go @@ -6,12 +6,16 @@ import ( "crypto/subtle" "encoding/hex" "errors" + "fmt" + "math" "net/http" + "strconv" "strings" "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgxpool" + "git.fabledsword.com/bvandeusen/minstrel/internal/auth" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" ) @@ -36,14 +40,34 @@ func UserFromContext(ctx context.Context) (dbq.User, bool) { // users.subsonic_password (t/s or p). On failure it writes a Subsonic failed // envelope using the request's f= format; downstream handlers never see an // unauthenticated request. -func Middleware(pool *pgxpool.Pool, cfg Config) func(http.Handler) http.Handler { +// +// guard throttles wrong credentials per account and per address, the same +// limits as the native login. Only wrong-credential failures count: Subsonic +// clients authenticate on every request, so counting successes would lock +// out a client for playing an album. hops is the live trusted-proxy depth +// used to find the caller's address. A nil guard disables throttling. +func Middleware(pool *pgxpool.Pool, cfg Config, guard *auth.LoginGuard, hops func() int) func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + addr := auth.ClientIP(r, hops()) + // apiKey requests have no account to charge; they count against + // the address alone. + account := r.URL.Query().Get("u") + if blocked, wait := guard.Blocked(account, addr); blocked { + secs := int(math.Ceil(wait.Seconds())) + w.Header().Set("Retry-After", strconv.Itoa(max(secs, 1))) + WriteFail(w, r, ErrGeneric, fmt.Sprintf("Too many failed sign-in attempts; try again in %d seconds", max(secs, 1))) + return + } user, code, msg := authenticate(r, pool, cfg) + if code == ErrWrongCredentials { + guard.Fail(account, addr) + } if code != 0 { WriteFail(w, r, code, msg) return } + guard.Succeed(account) ctx := context.WithValue(r.Context(), userCtxKey, user) next.ServeHTTP(w, r.WithContext(ctx)) }) diff --git a/internal/subsonic/subsonic.go b/internal/subsonic/subsonic.go index 75260ff2..3e5e6d95 100644 --- a/internal/subsonic/subsonic.go +++ b/internal/subsonic/subsonic.go @@ -12,6 +12,7 @@ import ( "github.com/go-chi/chi/v5" "github.com/jackc/pgx/v5/pgxpool" + "git.fabledsword.com/bvandeusen/minstrel/internal/auth" "git.fabledsword.com/bvandeusen/minstrel/internal/playevents" ) @@ -19,11 +20,14 @@ import ( // both /rest/foo and /rest/foo.view because client conventions vary. Both // GET and POST are accepted; Subsonic's auth params live in the query string // either way. -func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, cfg Config, events *playevents.Writer) { +// +// hops is the live trusted-proxy depth, read per request so an admin change +// applies without a restart; it locates the caller for the auth throttle. +func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, cfg Config, events *playevents.Writer, hops func() int) { b := &browseHandlers{pool: pool} m := newMediaHandlers(pool, events, logger) r.Route("/rest", func(sub chi.Router) { - sub.Use(Middleware(pool, cfg)) + sub.Use(Middleware(pool, cfg, auth.NewLoginGuard(), hops)) register(sub, "/ping", handlePing) register(sub, "/getLicense", handleGetLicense) register(sub, "/getMusicFolders", b.getMusicFolders) diff --git a/web/src/lib/api/client.ts b/web/src/lib/api/client.ts index aa0d168e..b984d252 100644 --- a/web/src/lib/api/client.ts +++ b/web/src/lib/api/client.ts @@ -2,8 +2,25 @@ export type ApiError = { code: string; message: string; status: number; + /** Seconds until a 429'd request may be retried, from Retry-After. */ + retryAfter?: number; }; +/** + * The sign-in, register and reset screens' wording for a throttled attempt, + * or null when err isn't one. Rounds up to whole minutes: the server's + * windows are minutes long, and "try again in 1 second" after a lockout + * would be untrue the moment it was read. + */ +export function rateLimitMessage(err: unknown): string | null { + const apiErr = err as ApiError | undefined; + if (apiErr?.status !== 429) return null; + const secs = apiErr.retryAfter; + if (!secs || secs <= 0) return 'Too many attempts. Try again in a few minutes.'; + const mins = Math.ceil(secs / 60); + return `Too many attempts. Try again in ${mins} minute${mins === 1 ? '' : 's'}.`; +} + export type User = { id: string; username: string; @@ -45,6 +62,10 @@ export async function apiFetch(path: string, init?: RequestInit): Promise 0) err.retryAfter = retryAfter; + } throw err; } return body; diff --git a/web/src/lib/styles/error-copy.json b/web/src/lib/styles/error-copy.json index 237d999a..d223bf84 100644 --- a/web/src/lib/styles/error-copy.json +++ b/web/src/lib/styles/error-copy.json @@ -4,6 +4,7 @@ "forbidden": "You don't have permission to do that.", "not_authorized": "You don't have permission to do that.", "invalid_credentials": "Wrong username or password.", + "rate_limited": "Too many attempts. Wait a few minutes and try again.", "wrong_password": "Current password is incorrect.", "password_too_short": "Password must be at least 8 characters.", "username_invalid": "That username isn't valid.", diff --git a/web/src/routes/forgot-password/+page.svelte b/web/src/routes/forgot-password/+page.svelte index a10b7fdd..4aa90b5c 100644 --- a/web/src/routes/forgot-password/+page.svelte +++ b/web/src/routes/forgot-password/+page.svelte @@ -1,23 +1,29 @@ `) + +var srcAttrRe = regexp.MustCompile(`(?i)\bsrc\s*=`) + +// contentSecurityPolicy builds the policy served with index.html. +// +// Scripts are allowed from our own origin plus the exact inline scripts the +// page carries: the theme bootstrap in app.html, the branding global the Vite +// plugin injects, and SvelteKit's start-up block. Their hashes are taken from +// the page AFTER the branding template has run, so they match the bytes the +// browser actually receives, whatever the operator's app name. Any script +// that differs, including one injected through an XSS, is refused. +// +// The rest: +// - style-src allows inline styles: Svelte transitions and style: +// directives write them, and inline style is not a script vector. +// - img-src admits https:/http: because Lidarr suggestion art is a remote +// poster URL. Images cannot run code. +// - media-src/connect-src stay on our own origin: streams, the API and the +// SSE stream are all same-origin. blob: covers Web Audio and object URLs. +func contentSecurityPolicy(indexHTML []byte) string { + scriptSrc := []string{"'self'"} + for _, m := range inlineScriptRe.FindAllSubmatch(indexHTML, -1) { + if srcAttrRe.Match(m[1]) { + continue + } + sum := sha256.Sum256(m[2]) + scriptSrc = append(scriptSrc, "'sha256-"+base64.StdEncoding.EncodeToString(sum[:])+"'") + } + return strings.Join([]string{ + "default-src 'self'", + "base-uri 'self'", + "object-src 'none'", + "frame-ancestors 'none'", + "form-action 'self'", + "script-src " + strings.Join(scriptSrc, " "), + "style-src 'self' 'unsafe-inline'", + "img-src 'self' data: blob: https: http:", + "font-src 'self' data:", + "media-src 'self' blob:", + "connect-src 'self'", + "worker-src 'self' blob:", + "manifest-src 'self'", + }, "; ") +} diff --git a/web/csp_test.go b/web/csp_test.go new file mode 100644 index 00000000..b603597a --- /dev/null +++ b/web/csp_test.go @@ -0,0 +1,60 @@ +package web + +import ( + "crypto/sha256" + "encoding/base64" + "strings" + "testing" +) + +func hashOf(body string) string { + sum := sha256.Sum256([]byte(body)) + return "'sha256-" + base64.StdEncoding.EncodeToString(sum[:]) + "'" +} + +func TestContentSecurityPolicy_HashesInlineScriptsOnly(t *testing.T) { + theme := "(function () { document.documentElement.dataset.theme = 'dark'; })();" + brand := `window.__MINSTREL__ = { appName: "Minstrel" };` + html := "" + + `` + + "" + + csp := contentSecurityPolicy([]byte(html)) + + var scriptSrc string + for _, d := range strings.Split(csp, "; ") { + if strings.HasPrefix(d, "script-src ") { + scriptSrc = d + } + } + if scriptSrc == "" { + t.Fatalf("no script-src in %q", csp) + } + for _, want := range []string{"'self'", hashOf(theme), hashOf(brand)} { + if !strings.Contains(scriptSrc, want) { + t.Errorf("script-src %q missing %s", scriptSrc, want) + } + } + if strings.Count(scriptSrc, "'sha256-") != 2 { + t.Errorf("script-src %q: want exactly the two inline scripts hashed, not the src= one", scriptSrc) + } + if strings.Contains(scriptSrc, "unsafe-inline") || strings.Contains(scriptSrc, "unsafe-eval") { + t.Errorf("script-src must not fall back to unsafe-*: %q", scriptSrc) + } + for _, want := range []string{"frame-ancestors 'none'", "object-src 'none'", "connect-src 'self'"} { + if !strings.Contains(csp, want) { + t.Errorf("csp missing %q", want) + } + } +} + +// The hash must be of the page as served, after the branding template has +// substituted the operator's app name, or a renamed instance would refuse +// its own bootstrap script. +func TestContentSecurityPolicy_TracksTemplatedContent(t *testing.T) { + a := contentSecurityPolicy([]byte(``)) + b := contentSecurityPolicy([]byte(``)) + if a == b { + t.Error("different script bodies produced the same policy") + } +} diff --git a/web/embed.go b/web/embed.go index 2c63b3c9..690c41c7 100644 --- a/web/embed.go +++ b/web/embed.go @@ -47,12 +47,14 @@ func Handler(branding config.BrandingConfig) http.Handler { panic("web: branding template failed: " + err.Error()) } + csp := contentSecurityPolicy(index) + fileServer := http.FileServer(http.FS(sub)) return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { clean := path.Clean(r.URL.Path) if clean == "/" || clean == "." { - serveIndex(w, index) + serveIndex(w, index, csp) return } name := strings.TrimPrefix(clean, "/") @@ -60,12 +62,16 @@ func Handler(branding config.BrandingConfig) http.Handler { fileServer.ServeHTTP(w, r) return } - serveIndex(w, index) + serveIndex(w, index, csp) }) } -func serveIndex(w http.ResponseWriter, index []byte) { +func serveIndex(w http.ResponseWriter, index []byte, csp string) { w.Header().Set("Content-Type", "text/html; charset=utf-8") + // The policy only means anything on the document; JSON, audio and image + // responses can't run script, so it rides here rather than on every + // response. + w.Header().Set("Content-Security-Policy", csp) w.Header().Set("Cache-Control", "no-cache") _, _ = w.Write(index) } From 46194a609d02c433a1c2425ec517af9a9787e75f Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 6 Oct 2026 09:32:10 -0400 Subject: [PATCH 06/18] fix(auth): build password-reset links from an operator-set public address, never the Host header (M462 #4981) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit buildResetURL used r.Host and r.TLS, so a forgot-password request with a forged Host emailed the victim a real reset token on a link to the attacker's server. Links now come only from network_settings.public_url (migration 0062), and no reset email is sent while it is empty; the response stays the same opaque 200 and the log says why. The address is set on a new "Public address" card under Admin → Integrations, which offers the page's own origin and warns while unset. PUT /api/admin/network-settings takes either field alone, so the proxy card and this one can't overwrite each other. Co-Authored-By: Claude Opus 5.5 --- internal/api/admin_network.go | 43 ++++++- internal/api/auth_forgot.go | 31 +++-- internal/api/auth_forgot_test.go | 58 +++++++++- internal/db/dbq/models.go | 1 + internal/db/dbq/network_settings.sql.go | 19 +++- .../0062_network_public_url.down.sql | 1 + .../migrations/0062_network_public_url.up.sql | 8 ++ internal/db/queries/network_settings.sql | 3 + internal/netsettings/publicurl_test.go | 34 ++++++ internal/netsettings/service.go | 62 +++++++++- web/src/lib/api/admin.ts | 10 ++ web/src/lib/api/errors.ts | 4 +- .../lib/components/PublicAddressCard.svelte | 107 ++++++++++++++++++ .../lib/components/PublicAddressCard.test.ts | 75 ++++++++++++ web/src/lib/styles/error-copy.json | 1 + .../routes/admin/integrations/+page.svelte | 2 + 16 files changed, 433 insertions(+), 26 deletions(-) create mode 100644 internal/db/migrations/0062_network_public_url.down.sql create mode 100644 internal/db/migrations/0062_network_public_url.up.sql create mode 100644 internal/netsettings/publicurl_test.go create mode 100644 web/src/lib/components/PublicAddressCard.svelte create mode 100644 web/src/lib/components/PublicAddressCard.test.ts diff --git a/internal/api/admin_network.go b/internal/api/admin_network.go index 4ab12d63..e2e2a4be 100644 --- a/internal/api/admin_network.go +++ b/internal/api/admin_network.go @@ -23,10 +23,16 @@ type networkSettingsResp struct { // arrived and count them rather than guess. ForwardedChain string `json:"forwarded_chain"` RemoteAddr string `json:"remote_addr"` + // PublicURL is where users reach Minstrel; reset emails link to it and + // are not sent while it is empty. + PublicURL string `json:"public_url"` } +// Both fields are optional so the proxy card and the public-address card can +// each save their own value without overwriting the other's. type updateNetworkSettingsReq struct { - TrustedProxyHops int `json:"trusted_proxy_hops"` + TrustedProxyHops *int `json:"trusted_proxy_hops"` + PublicURL *string `json:"public_url"` } func (h *handlers) handleGetNetworkSettings(w http.ResponseWriter, r *http.Request) { @@ -39,13 +45,37 @@ func (h *handlers) handleUpdateNetworkSettings(w http.ResponseWriter, r *http.Re writeErr(w, apierror.BadRequest("invalid_body", "malformed JSON")) return } - if err := h.netSettings.SetHops(r.Context(), req.TrustedProxyHops); err != nil { - if errors.Is(err, netsettings.ErrHopsOutOfRange) { - writeErr(w, apierror.BadRequest("invalid_hops", err.Error())) + if req.TrustedProxyHops == nil && req.PublicURL == nil { + writeErr(w, apierror.BadRequest("invalid_body", "nothing to update")) + return + } + // Validate everything before writing anything, so a bad public URL can't + // leave the hops half-saved. + if req.TrustedProxyHops != nil && (*req.TrustedProxyHops < 0 || *req.TrustedProxyHops > netsettings.MaxTrustedProxyHops) { + writeErr(w, apierror.BadRequest("invalid_hops", netsettings.ErrHopsOutOfRange.Error())) + return + } + if req.PublicURL != nil { + if _, err := netsettings.NormalizePublicURL(*req.PublicURL); err != nil { + writeErr(w, apierror.BadRequest("invalid_public_url", err.Error())) + return + } + } + if req.TrustedProxyHops != nil { + if err := h.netSettings.SetHops(r.Context(), *req.TrustedProxyHops); err != nil { + if errors.Is(err, netsettings.ErrHopsOutOfRange) { + writeErr(w, apierror.BadRequest("invalid_hops", err.Error())) + return + } + writeErrWithLog(w, h.logger, "admin network: update failed", apierror.Internal(err)) + return + } + } + if req.PublicURL != nil { + if err := h.netSettings.SetPublicURL(r.Context(), *req.PublicURL); err != nil { + writeErrWithLog(w, h.logger, "admin network: public URL update failed", apierror.Internal(err)) return } - writeErrWithLog(w, h.logger, "admin network: update failed", apierror.Internal(err)) - return } // Echo the payload recomputed under the NEW value, so the card can show // immediately what the change did to this request's own address rather @@ -61,5 +91,6 @@ func (h *handlers) networkSettingsPayload(r *http.Request) networkSettingsResp { DetectedClientIP: auth.ClientIP(r, hops), ForwardedChain: r.Header.Get("X-Forwarded-For"), RemoteAddr: r.RemoteAddr, + PublicURL: h.netSettings.PublicURL(), } } diff --git a/internal/api/auth_forgot.go b/internal/api/auth_forgot.go index 7491f6d3..6d652437 100644 --- a/internal/api/auth_forgot.go +++ b/internal/api/auth_forgot.go @@ -69,7 +69,7 @@ func (h *handlers) handleForgotPassword(w http.ResponseWriter, r *http.Request) if err == nil && user.Email != nil && *user.Email != "" { matched = true auditTarget = user.ID - if sendErr := h.sendResetEmail(r.Context(), r, user); sendErr != nil { + if sendErr := h.sendResetEmail(r.Context(), user); sendErr != nil { h.logger.Warn("forgot-password: send failed", "email", email, "err", sendErr) // fall through; response is still 200 @@ -89,7 +89,7 @@ func (h *handlers) handleForgotPassword(w http.ResponseWriter, r *http.Request) // sendResetEmail generates a token, persists it, renders + sends the // email. Returns the underlying error (caller logs it but doesn't // surface to the HTTP response). -func (h *handlers) sendResetEmail(ctx context.Context, r *http.Request, user dbq.User) error { +func (h *handlers) sendResetEmail(ctx context.Context, user dbq.User) error { if h.mailer == nil { return errors.New("forgot-password: no mailer configured") } @@ -109,7 +109,10 @@ func (h *handlers) sendResetEmail(ctx context.Context, r *http.Request, user dbq return err } - resetURL := buildResetURL(r, token) + resetURL, err := buildResetURL(h.netSettings.PublicURL(), token) + if err != nil { + return err + } textBody, htmlBody, err := mailer.RenderResetEmail(mailer.ResetEmailVars{ Username: user.Username, ResetURL: resetURL, @@ -127,14 +130,18 @@ func (h *handlers) sendResetEmail(ctx context.Context, r *http.Request, user dbq return h.mailer.Send(ctx, *user.Email, mailer.ResetEmailSubject, textBody, htmlBody) } -func buildResetURL(r *http.Request, token string) string { - scheme := "http" - if r.TLS != nil { - scheme = "https" +// errNoPublicURL stops a reset email from going out before the operator has +// said where Minstrel lives. It surfaces in the log, not the response, which +// stays the same opaque 200 either way. +var errNoPublicURL = errors.New("forgot-password: no public URL set (Admin → Integrations → Public address); reset email not sent") + +// buildResetURL builds the emailed link from the operator-set public URL. +// It deliberately ignores the request: the Host header is whatever the +// requester sent, and building from it let a forged Host plant a real reset +// token on a link to someone else's server. +func buildResetURL(publicURL, token string) (string, error) { + if publicURL == "" { + return "", errNoPublicURL } - host := r.Host - if host == "" { - host = "localhost" - } - return scheme + "://" + host + "/reset-password/" + token + return publicURL + "/reset-password/" + token, nil } diff --git a/internal/api/auth_forgot_test.go b/internal/api/auth_forgot_test.go index 09eb38fa..d5ce4d5a 100644 --- a/internal/api/auth_forgot_test.go +++ b/internal/api/auth_forgot_test.go @@ -7,11 +7,59 @@ import ( "net/http" "net/http/httptest" "os" + "strings" "testing" + "github.com/jackc/pgx/v5/pgxpool" + "git.fabledsword.com/bvandeusen/minstrel/internal/mailer" + "git.fabledsword.com/bvandeusen/minstrel/internal/netsettings" ) +// withPublicURL gives h a network-settings service holding url, restoring an +// empty value afterwards so other tests see the default. +func withPublicURL(t *testing.T, h *handlers, pool *pgxpool.Pool, url string) { + t.Helper() + ns, err := netsettings.New(context.Background(), pool, nil) + if err != nil { + t.Fatalf("netsettings: %v", err) + } + if err := ns.SetPublicURL(context.Background(), url); err != nil { + t.Fatalf("set public url: %v", err) + } + t.Cleanup(func() { _ = ns.SetPublicURL(context.Background(), "") }) + h.netSettings = ns +} + +// With no public URL set, a reset email is not sent at all, and the response +// is still the same opaque 200. +func TestForgotPassword_NoPublicURL_SendsNothing(t *testing.T) { + if os.Getenv("MINSTREL_TEST_DATABASE_URL") == "" { + t.Skip("MINSTREL_TEST_DATABASE_URL not set") + } + h, pool := testHandlers(t) + fake := &mailer.FakeSender{} + h.mailer = fake + withPublicURL(t, h, pool, "") + + user := seedUser(t, pool, "nourl", "pw", false) + if _, err := pool.Exec(context.Background(), + "UPDATE users SET email = 'nourl@example.com' WHERE id = $1", user.ID); err != nil { + t.Fatalf("seed email: %v", err) + } + req := httptest.NewRequest(http.MethodPost, "/api/auth/forgot-password", + bytes.NewReader([]byte(`{"email":"nourl@example.com"}`))) + rec := httptest.NewRecorder() + h.handleForgotPassword(rec, req) + + if rec.Code != http.StatusOK { + t.Errorf("status = %d, want 200", rec.Code) + } + if len(fake.Sent) != 0 { + t.Errorf("sent %d emails with no public URL set, want 0", len(fake.Sent)) + } +} + func TestForgotPassword_KnownEmail_SendsMail(t *testing.T) { if os.Getenv("MINSTREL_TEST_DATABASE_URL") == "" { t.Skip("MINSTREL_TEST_DATABASE_URL not set") @@ -19,6 +67,7 @@ func TestForgotPassword_KnownEmail_SendsMail(t *testing.T) { h, pool := testHandlers(t) fake := &mailer.FakeSender{} h.mailer = fake + withPublicURL(t, h, pool, "https://music.example.com") user := seedUser(t, pool, "forgotuser", "pw", false) if _, err := pool.Exec(context.Background(), @@ -29,7 +78,8 @@ func TestForgotPassword_KnownEmail_SendsMail(t *testing.T) { body := `{"email":"forgot@example.com"}` req := httptest.NewRequest(http.MethodPost, "/api/auth/forgot-password", bytes.NewReader([]byte(body))) - req.Host = "minstrel.example.com" + // A forged Host must not reach the link: it comes from the public URL. + req.Host = "attacker.example.net" rec := httptest.NewRecorder() h.handleForgotPassword(rec, req) @@ -43,6 +93,12 @@ func TestForgotPassword_KnownEmail_SendsMail(t *testing.T) { if got.To != "forgot@example.com" { t.Errorf("To = %q, want forgot@example.com", got.To) } + if !strings.Contains(got.TextBody, "https://music.example.com/reset-password/") { + t.Errorf("reset link not built from the public URL:\n%s", got.TextBody) + } + if strings.Contains(got.TextBody+got.HTMLBody, "attacker.example.net") { + t.Error("the request's Host header reached the emailed link") + } // Token row was inserted. var tokenCount int if err := pool.QueryRow(context.Background(), diff --git a/internal/db/dbq/models.go b/internal/db/dbq/models.go index 17a9c89d..5c719e9c 100644 --- a/internal/db/dbq/models.go +++ b/internal/db/dbq/models.go @@ -430,6 +430,7 @@ type MissingReacquisition struct { type NetworkSetting struct { ID bool TrustedProxyHops int32 + PublicUrl string } type PasswordReset struct { diff --git a/internal/db/dbq/network_settings.sql.go b/internal/db/dbq/network_settings.sql.go index 89573cfd..3d31f0f5 100644 --- a/internal/db/dbq/network_settings.sql.go +++ b/internal/db/dbq/network_settings.sql.go @@ -10,23 +10,34 @@ import ( ) const getNetworkSettings = `-- name: GetNetworkSettings :one -SELECT id, trusted_proxy_hops FROM network_settings WHERE id = true +SELECT id, trusted_proxy_hops, public_url FROM network_settings WHERE id = true ` func (q *Queries) GetNetworkSettings(ctx context.Context) (NetworkSetting, error) { row := q.db.QueryRow(ctx, getNetworkSettings) var i NetworkSetting - err := row.Scan(&i.ID, &i.TrustedProxyHops) + err := row.Scan(&i.ID, &i.TrustedProxyHops, &i.PublicUrl) + return i, err +} + +const updatePublicURL = `-- name: UpdatePublicURL :one +UPDATE network_settings SET public_url = $1 WHERE id = true RETURNING id, trusted_proxy_hops, public_url +` + +func (q *Queries) UpdatePublicURL(ctx context.Context, publicUrl string) (NetworkSetting, error) { + row := q.db.QueryRow(ctx, updatePublicURL, publicUrl) + var i NetworkSetting + err := row.Scan(&i.ID, &i.TrustedProxyHops, &i.PublicUrl) return i, err } const updateTrustedProxyHops = `-- name: UpdateTrustedProxyHops :one -UPDATE network_settings SET trusted_proxy_hops = $1 WHERE id = true RETURNING id, trusted_proxy_hops +UPDATE network_settings SET trusted_proxy_hops = $1 WHERE id = true RETURNING id, trusted_proxy_hops, public_url ` func (q *Queries) UpdateTrustedProxyHops(ctx context.Context, trustedProxyHops int32) (NetworkSetting, error) { row := q.db.QueryRow(ctx, updateTrustedProxyHops, trustedProxyHops) var i NetworkSetting - err := row.Scan(&i.ID, &i.TrustedProxyHops) + err := row.Scan(&i.ID, &i.TrustedProxyHops, &i.PublicUrl) return i, err } diff --git a/internal/db/migrations/0062_network_public_url.down.sql b/internal/db/migrations/0062_network_public_url.down.sql new file mode 100644 index 00000000..418a7cd6 --- /dev/null +++ b/internal/db/migrations/0062_network_public_url.down.sql @@ -0,0 +1 @@ +ALTER TABLE network_settings DROP COLUMN IF EXISTS public_url; diff --git a/internal/db/migrations/0062_network_public_url.up.sql b/internal/db/migrations/0062_network_public_url.up.sql new file mode 100644 index 00000000..0d1f6dac --- /dev/null +++ b/internal/db/migrations/0062_network_public_url.up.sql @@ -0,0 +1,8 @@ +-- The address users reach Minstrel at, e.g. https://music.example.com. +-- +-- Password-reset links used to be built from the request's Host header, +-- which the requester controls: a forgot-password call with a forged Host +-- would email the victim a real reset token on a link to the attacker's +-- server. Links are now built only from this operator-set value, and none +-- are sent while it is empty (M462 #4981). +ALTER TABLE network_settings ADD COLUMN public_url text NOT NULL DEFAULT ''; diff --git a/internal/db/queries/network_settings.sql b/internal/db/queries/network_settings.sql index 9527590e..4a6998e7 100644 --- a/internal/db/queries/network_settings.sql +++ b/internal/db/queries/network_settings.sql @@ -3,3 +3,6 @@ SELECT * FROM network_settings WHERE id = true; -- name: UpdateTrustedProxyHops :one UPDATE network_settings SET trusted_proxy_hops = $1 WHERE id = true RETURNING *; + +-- name: UpdatePublicURL :one +UPDATE network_settings SET public_url = $1 WHERE id = true RETURNING *; diff --git a/internal/netsettings/publicurl_test.go b/internal/netsettings/publicurl_test.go new file mode 100644 index 00000000..1deebde5 --- /dev/null +++ b/internal/netsettings/publicurl_test.go @@ -0,0 +1,34 @@ +package netsettings + +import "testing" + +func TestNormalizePublicURL(t *testing.T) { + ok := map[string]string{ + "": "", + " ": "", + "https://music.example.com": "https://music.example.com", + "https://music.example.com/": "https://music.example.com", + "http://192.168.1.10:4533": "http://192.168.1.10:4533", + " https://Music.Example.com/ ": "https://Music.Example.com", + } + for in, want := range ok { + got, err := NormalizePublicURL(in) + if err != nil || got != want { + t.Errorf("NormalizePublicURL(%q) = %q, %v; want %q", in, got, err, want) + } + } + for _, in := range []string{ + "music.example.com", // no scheme + "ftp://music.example.com", // wrong scheme + "https://", // no host + "https://music.example.com/app", // path + "https://music.example.com/?x=1", // query + "https://music.example.com/#frag", // fragment + "https://user:pw@music.example.com", + "javascript:alert(1)", + } { + if _, err := NormalizePublicURL(in); err == nil { + t.Errorf("NormalizePublicURL(%q) accepted, want ErrInvalidPublicURL", in) + } + } +} diff --git a/internal/netsettings/service.go b/internal/netsettings/service.go index 9b526178..49d23d1b 100644 --- a/internal/netsettings/service.go +++ b/internal/netsettings/service.go @@ -12,6 +12,8 @@ import ( "context" "errors" "log/slog" + "net/url" + "strings" "sync" "github.com/jackc/pgx/v5/pgxpool" @@ -32,13 +34,18 @@ const ( // so the API layer can answer 400 instead of surfacing a constraint violation. var ErrHopsOutOfRange = errors.New("trusted proxy hops must be between 0 and 10") +// ErrInvalidPublicURL is returned by SetPublicURL for anything that isn't a +// bare http(s) origin, so the API layer can answer 400. +var ErrInvalidPublicURL = errors.New("public URL must be an http:// or https:// address with a host and no path, query or fragment") + // Service caches the network settings and owns their persistence. type Service struct { pool *pgxpool.Pool logger *slog.Logger - mu sync.RWMutex - hops int + mu sync.RWMutex + hops int + publicURL string } // New loads the settings once and caches them. @@ -58,6 +65,7 @@ func New(ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger) (*Service return s, err } s.hops = int(row.TrustedProxyHops) + s.publicURL = row.PublicUrl return s, nil } @@ -106,3 +114,53 @@ func (s *Service) SetHops(ctx context.Context, hops int) error { } return nil } + +// PublicURL returns the operator-set address users reach Minstrel at, with no +// trailing slash, or "" when it hasn't been set. Links that leave the app (a +// password-reset email) are built from this and never from the request's +// Host header, which the requester controls. Nil-safe like Hops. +func (s *Service) PublicURL() string { + if s == nil { + return "" + } + s.mu.RLock() + defer s.mu.RUnlock() + return s.publicURL +} + +// NormalizePublicURL validates raw as a bare http(s) origin and returns it +// without a trailing slash. "" is valid and means unset. +func NormalizePublicURL(raw string) (string, error) { + raw = strings.TrimSpace(raw) + if raw == "" { + return "", nil + } + u, err := url.Parse(raw) + if err != nil || (u.Scheme != "http" && u.Scheme != "https") || u.Host == "" || + u.User != nil || (u.Path != "" && u.Path != "/") || u.RawQuery != "" || u.Fragment != "" { + return "", ErrInvalidPublicURL + } + return u.Scheme + "://" + u.Host, nil +} + +// SetPublicURL validates, persists and caches the public URL. "" clears it. +func (s *Service) SetPublicURL(ctx context.Context, raw string) error { + normalized, err := NormalizePublicURL(raw) + if err != nil { + return err + } + if s == nil || s.pool == nil { + return errors.New("network settings unavailable") + } + row, err := dbq.New(s.pool).UpdatePublicURL(ctx, normalized) + if err != nil { + return err + } + s.mu.Lock() + s.publicURL = row.PublicUrl + s.mu.Unlock() + if s.logger != nil { + s.logger.Info("netsettings: public URL updated", "public_url", normalized) + } + return nil +} diff --git a/web/src/lib/api/admin.ts b/web/src/lib/api/admin.ts index 8f97e103..b1b86597 100644 --- a/web/src/lib/api/admin.ts +++ b/web/src/lib/api/admin.ts @@ -708,6 +708,9 @@ export type NetworkSettings = { detected_client_ip: string; forwarded_chain: string; remote_addr: string; + // Where users reach Minstrel. Password-reset emails link here and are not + // sent while it is empty. + public_url: string; }; export async function getNetworkSettings(): Promise { @@ -722,6 +725,13 @@ export async function updateNetworkSettings(hops: number): Promise { + return api.put('/api/admin/network-settings', { + public_url: publicUrl + }); +} + // Duplicates report (#3912) ------------------------------------------------- export async function listDuplicates( diff --git a/web/src/lib/api/errors.ts b/web/src/lib/api/errors.ts index 3de3b71b..c02325cd 100644 --- a/web/src/lib/api/errors.ts +++ b/web/src/lib/api/errors.ts @@ -19,7 +19,9 @@ const DETAIL_CODES: ReadonlySet = new Set([ 'library_not_writable', 'file_delete_failed', // The server names the field and its range (#3913). - 'invalid_setting' + 'invalid_setting', + // The server says what shape of address it wants (#4981). + 'invalid_public_url' ]); /** diff --git a/web/src/lib/components/PublicAddressCard.svelte b/web/src/lib/components/PublicAddressCard.svelte new file mode 100644 index 00000000..2c984af0 --- /dev/null +++ b/web/src/lib/components/PublicAddressCard.svelte @@ -0,0 +1,107 @@ + + +
+
+

Public address

+

+ The address people use to reach Minstrel. Password-reset emails link here. +

+
+ + {#if loadError} +

+ Couldn't load network settings. + +

+ {:else if saved === null} +

Loading…

+ {:else} +
+ + {#if here && value.trim() !== here} + + {/if} + +
+ + {#if !saved} +

+

+ {/if} + {/if} +
diff --git a/web/src/lib/components/PublicAddressCard.test.ts b/web/src/lib/components/PublicAddressCard.test.ts new file mode 100644 index 00000000..56a7d711 --- /dev/null +++ b/web/src/lib/components/PublicAddressCard.test.ts @@ -0,0 +1,75 @@ +import { describe, expect, test, vi, beforeEach } from 'vitest'; +import { render, screen, fireEvent, waitFor } from '@testing-library/svelte'; +import PublicAddressCard from './PublicAddressCard.svelte'; + +const getNetworkSettings = vi.fn(); +const updatePublicUrl = vi.fn(); + +vi.mock('$lib/api/admin', () => ({ + getNetworkSettings: () => getNetworkSettings(), + updatePublicUrl: (url: string) => updatePublicUrl(url) +})); + +const pushToast = vi.fn(); +vi.mock('$lib/stores/toast.svelte', () => ({ + pushToast: (...args: unknown[]) => pushToast(...args) +})); + +function settings(publicUrl: string) { + return { + trusted_proxy_hops: 1, + max_hops: 10, + detected_client_ip: '198.51.100.7', + forwarded_chain: '198.51.100.7', + remote_addr: '172.18.0.1:40000', + public_url: publicUrl + }; +} + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe('PublicAddressCard', () => { + test('warns that reset emails are not sent while the address is unset', async () => { + getNetworkSettings.mockResolvedValue(settings('')); + render(PublicAddressCard); + expect(await screen.findByText(/password-reset emails are not being sent/i)).toBeTruthy(); + }); + + test('no warning once an address is saved', async () => { + getNetworkSettings.mockResolvedValue(settings('https://music.example.com')); + render(PublicAddressCard); + await screen.findByDisplayValue('https://music.example.com'); + expect(screen.queryByText(/not being sent/i)).toBeNull(); + }); + + test('offers this page’s own origin and saves it trimmed', async () => { + getNetworkSettings.mockResolvedValue(settings('')); + updatePublicUrl.mockResolvedValue(settings(window.location.origin)); + render(PublicAddressCard); + + await fireEvent.click(await screen.findByRole('button', { name: /^Use / })); + await fireEvent.click(screen.getByRole('button', { name: /save/i })); + + await waitFor(() => expect(updatePublicUrl).toHaveBeenCalledWith(window.location.origin)); + expect(pushToast).toHaveBeenCalledWith('Public address saved.'); + }); + + test('shows the server’s reason when an address is refused', async () => { + getNetworkSettings.mockResolvedValue(settings('')); + updatePublicUrl.mockRejectedValue({ + code: 'invalid_public_url', + message: 'public URL must be an http:// or https:// address', + status: 400 + }); + render(PublicAddressCard); + + const input = await screen.findByPlaceholderText('https://music.example.com'); + await fireEvent.input(input, { target: { value: 'music.example.com' } }); + await fireEvent.click(screen.getByRole('button', { name: /save/i })); + + await waitFor(() => expect(pushToast).toHaveBeenCalled()); + expect(pushToast.mock.calls[0][1]).toBe('error'); + }); +}); diff --git a/web/src/lib/styles/error-copy.json b/web/src/lib/styles/error-copy.json index d223bf84..349d4b2a 100644 --- a/web/src/lib/styles/error-copy.json +++ b/web/src/lib/styles/error-copy.json @@ -47,6 +47,7 @@ "duplicate_group_not_pending": "That group has already been resolved.", "survivor_not_in_group": "That copy isn't part of this group any more.", "invalid_setting": "That setting is out of range.", + "invalid_public_url": "That address isn't valid.", "album_not_found": "That album no longer exists.", "artist_not_found": "That artist no longer exists.", "playlist_not_found": "That playlist no longer exists.", diff --git a/web/src/routes/admin/integrations/+page.svelte b/web/src/routes/admin/integrations/+page.svelte index e69c25b1..eb749efb 100644 --- a/web/src/routes/admin/integrations/+page.svelte +++ b/web/src/routes/admin/integrations/+page.svelte @@ -28,6 +28,7 @@ import { pushToast } from '$lib/stores/toast.svelte'; import Modal from '$lib/components/Modal.svelte'; import NetworkSettingsCard from '$lib/components/NetworkSettingsCard.svelte'; + import PublicAddressCard from '$lib/components/PublicAddressCard.svelte'; import type { LidarrConfig, LidarrTestResult } from '$lib/api/types'; // Lidarr connection panel. The "saved api key" is masked as "***" on GET — @@ -827,6 +828,7 @@ other card on this page, and it's an operator-wide setting rather than a per-user preference. --> + Date: Tue, 6 Oct 2026 09:35:52 -0400 Subject: [PATCH 07/18] feat(auth): first account on a new server needs the setup token from the server log (M462 #4982) While no accounts exist, the server mints a random setup token at boot and logs it. Registering the first account (which becomes admin) must carry it, so whoever reaches a freshly exposed instance first cannot claim it. The register page asks GET /api/auth/setup-status and shows a "Setup token" field in place of the invite field while setup is pending. Co-Authored-By: Claude Opus 5.5 --- internal/api/api.go | 16 ++++++ internal/api/auth_register.go | 53 ++++++++++++++++- internal/api/auth_register_test.go | 72 ++++++++++++++++++++++++ internal/auth/setup.go | 46 +++++++++++++++ web/src/lib/auth/store.svelte.ts | 10 ++++ web/src/routes/register/+page.svelte | 59 +++++++++++++++---- web/src/routes/register/register.test.ts | 36 +++++++++++- 7 files changed, 276 insertions(+), 16 deletions(-) create mode 100644 internal/auth/setup.go diff --git a/internal/api/api.go b/internal/api/api.go index f1519c9e..66d8ab87 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -36,6 +36,13 @@ import ( // is shared with the Subsonic mount so /rest/scrobble feeds the same store. func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playevents.Writer, recCfg config.RecommendationConfig, recSettings *recsettings.Service, lidarrCfg *lidarrconfig.Service, lidarrReqs *lidarrrequests.Service, lidarrQuar *lidarrquarantine.Service, tracksSvc *tracks.Service, playlistsSvc *playlists.Service, coverEnricher *coverart.Enricher, coverSettings *coverart.SettingsService, tagSettings *tags.SettingsService, scanner *library.Scanner, scanCfg library.RunScanConfig, dataDir string, sender mailer.Sender, bus *eventbus.Bus, playlistScheduler *playlists.Scheduler, streamSecret []byte, netSettings *netsettings.Service, reacqSettings *reacquisition.SettingsService, fpSettings *library.FingerprintSettingsService) { rng := rand.New(rand.NewSource(rand.Int63())) + setupToken, err := auth.NewSetupToken() + if err != nil { + // crypto/rand failing means the platform can't make secrets at all; + // sessions would be minted from the same source. Nothing to degrade to. + panic("api: mint setup token: " + err.Error()) + } + logSetupTokenIfNeeded(pool, logger, setupToken) h := &handlers{ pool: pool, logger: logger, events: events, recCfg: recCfg, recSettings: recSettings, @@ -60,6 +67,8 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev fingerprintSettings: fpSettings, librarySize: recommendation.NewLibrarySize(nil), loginGuard: auth.NewLoginGuard(), + setupToken: setupToken, + requireSetupToken: true, registerLimit: auth.NewAttemptLimiter(registerPerAddressMax, time.Hour), forgotAddressLimit: auth.NewAttemptLimiter(forgotPerAddressMax, time.Hour), forgotEmailLimit: auth.NewAttemptLimiter(forgotPerEmailMax, time.Hour), @@ -69,6 +78,7 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev r.Route("/api", func(api chi.Router) { api.Post("/auth/login", h.handleLogin) api.Post("/auth/register", h.handleRegister) + api.Get("/auth/setup-status", h.handleSetupStatus) api.Post("/auth/forgot-password", h.handleForgotPassword) api.Post("/auth/reset-password", h.handleResetPassword) @@ -319,6 +329,12 @@ type handlers struct { // instance the scanner and the fingerprint workers read, so a save from the // admin card reaches them without a restart. Nil serves the defaults. fingerprintSettings *library.FingerprintSettingsService + // setupToken must accompany the first registration while no users exist + // (see auth.SetupToken). requireSetupToken is set by Mount, the only + // production constructor; tests that build handlers directly leave it + // off unless they are testing it. + setupToken *auth.SetupToken + requireSetupToken bool // loginGuard throttles failed logins per account and per address, and // the limiters below cap the other unauthenticated auth routes. All are // nil-safe, so tests that build handlers directly run unthrottled. diff --git a/internal/api/auth_register.go b/internal/api/auth_register.go index 4ea5cddd..1c14ad64 100644 --- a/internal/api/auth_register.go +++ b/internal/api/auth_register.go @@ -1,8 +1,10 @@ package api import ( + "context" "encoding/json" "errors" + "log/slog" "net/http" "regexp" "time" @@ -10,6 +12,7 @@ import ( "github.com/jackc/pgerrcode" "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgconn" + "github.com/jackc/pgx/v5/pgxpool" "golang.org/x/crypto/bcrypt" "git.fabledsword.com/bvandeusen/minstrel/internal/apierror" @@ -26,9 +29,12 @@ var usernameRe = regexp.MustCompile(`^[a-zA-Z0-9_-]{3,32}$`) const minPasswordLength = 8 type registerReq struct { - Username string `json:"username"` - Password string `json:"password"` - InviteToken string `json:"invite_token"` + Username string `json:"username"` + Password string `json:"password"` + InviteToken string `json:"invite_token"` + // SetupToken is required only for the very first account; see + // auth.SetupToken. + SetupToken string `json:"setup_token"` DisplayName *string `json:"display_name"` } @@ -88,6 +94,19 @@ func (h *handlers) handleRegister(w http.ResponseWriter, r *http.Request) { return } + // The first account becomes admin, so it must prove it can read the + // server log. Checked before the invite logic, which the empty-users + // state skips. + if userCount == 0 && h.requireSetupToken && !h.setupToken.Matches(req.SetupToken) { + // Repeat the token in the log at the moment someone needs it: the + // boot line may have scrolled away, or users may have been deleted + // since boot. + h.logger.Warn("register: first-admin registration needs the setup token", + "setup_token", h.setupToken.Value()) + writeErr(w, apierror.Forbidden("setup_token_invalid", "the setup token from the server log is required to create the first account")) + return + } + // Validate invite (skipped on empty-users state; skipped in 'open' mode). usedInviteToken := "" if userCount > 0 { @@ -232,3 +251,31 @@ func (h *handlers) handleRegister(w http.ResponseWriter, r *http.Request) { }, }) } + +// handleSetupStatus implements GET /api/auth/setup-status. It tells the +// register screen whether to ask for the setup token, i.e. whether no +// account exists yet. Public, since it is needed before anyone can sign in; +// "this server has no users" is not worth hiding from someone who could +// simply try to register. +func (h *handlers) handleSetupStatus(w http.ResponseWriter, r *http.Request) { + n, err := dbq.New(h.pool).CountUsers(r.Context()) + if err != nil { + writeErrWithLog(w, h.logger, "setup status: count users failed", apierror.Internal(err)) + return + } + writeJSON(w, http.StatusOK, map[string]bool{"setup_required": n == 0}) +} + +// logSetupTokenIfNeeded writes the setup token to the log at boot when the +// instance has no accounts yet, with the instruction for using it. +func logSetupTokenIfNeeded(pool *pgxpool.Pool, logger *slog.Logger, token *auth.SetupToken) { + if pool == nil || logger == nil { + return + } + n, err := dbq.New(pool).CountUsers(context.Background()) + if err != nil || n > 0 { + return + } + logger.Warn("no accounts yet: open the web app, choose Create account, and enter this setup token to become the admin", + "setup_token", token.Value()) +} diff --git a/internal/api/auth_register_test.go b/internal/api/auth_register_test.go index 698caf5e..d64de07e 100644 --- a/internal/api/auth_register_test.go +++ b/internal/api/auth_register_test.go @@ -273,3 +273,75 @@ func TestRegister_FirstAdminRace(t *testing.T) { // not "exactly one" (which would require serializable isolation). t.Logf("admin count after race = %d (>=1 is the invariant)", adminCount) } + +// With the gate on (as Mount sets it), the first account needs the setup +// token from the log; a missing or wrong one is refused and creates nothing. +func TestRegister_FirstUserNeedsSetupToken(t *testing.T) { + if os.Getenv("MINSTREL_TEST_DATABASE_URL") == "" { + t.Skip("MINSTREL_TEST_DATABASE_URL not set") + } + h, _ := testHandlers(t) + resetUsers(t, h) + token, err := auth.NewSetupToken() + if err != nil { + t.Fatalf("mint: %v", err) + } + h.setupToken = token + h.requireSetupToken = true + + register := func(body string) int { + req := httptest.NewRequest(http.MethodPost, "/api/auth/register", bytes.NewReader([]byte(body))) + rec := httptest.NewRecorder() + h.handleRegister(rec, req) + return rec.Code + } + + if code := register(`{"username":"squatter","password":"abcd1234"}`); code != http.StatusForbidden { + t.Errorf("no token: status = %d, want 403", code) + } + if code := register(`{"username":"squatter","password":"abcd1234","setup_token":"wrong"}`); code != http.StatusForbidden { + t.Errorf("wrong token: status = %d, want 403", code) + } + var n int + if err := h.pool.QueryRow(context.Background(), `SELECT count(*) FROM users`).Scan(&n); err != nil { + t.Fatalf("count: %v", err) + } + if n != 0 { + t.Fatalf("a refused registration created %d account(s)", n) + } + + if code := register(`{"username":"operator","password":"abcd1234","setup_token":"` + token.Value() + `"}`); code != http.StatusOK { + t.Fatalf("right token: status = %d, want 200", code) + } + + // Once an account exists the token plays no part: the second user is + // governed by registration mode, not the setup token. + if _, err := h.pool.Exec(context.Background(), + `UPDATE registration_settings SET mode = 'open' WHERE id = true`); err != nil { + t.Fatalf("open mode: %v", err) + } + t.Cleanup(func() { + _, _ = h.pool.Exec(context.Background(), + `UPDATE registration_settings SET mode = 'invite_only' WHERE id = true`) + }) + if code := register(`{"username":"second","password":"abcd1234"}`); code != http.StatusOK { + t.Errorf("second user without token: status = %d, want 200", code) + } +} + +func TestSetupToken_MatchesOnlyItself(t *testing.T) { + tok, err := auth.NewSetupToken() + if err != nil { + t.Fatalf("mint: %v", err) + } + if !tok.Matches(tok.Value()) { + t.Error("token does not match itself") + } + if tok.Matches("") || tok.Matches(tok.Value()+"x") { + t.Error("token matched something else") + } + var none *auth.SetupToken + if none.Matches("") || none.Matches("anything") { + t.Error("a nil token must fail closed") + } +} diff --git a/internal/auth/setup.go b/internal/auth/setup.go new file mode 100644 index 00000000..9d576be7 --- /dev/null +++ b/internal/auth/setup.go @@ -0,0 +1,46 @@ +package auth + +import ( + "crypto/rand" + "crypto/subtle" + "encoding/hex" +) + +// SetupToken guards the first-admin registration. Until the first account +// exists, register makes whoever calls it the admin, so a fresh instance on a +// public address belonged to whoever found it first. Now that call also has +// to carry this token, which is generated at startup and written only to the +// server log: proof that the caller can read the operator's logs. +// +// The token lives in memory. A restart mints a new one and logs it again, +// which is the behaviour wanted: an old token from a log line someone else +// saw stops working. +type SetupToken struct { + value string +} + +// NewSetupToken mints a 128-bit token. +func NewSetupToken() (*SetupToken, error) { + b := make([]byte, 16) + if _, err := rand.Read(b); err != nil { + return nil, err + } + return &SetupToken{value: hex.EncodeToString(b)}, nil +} + +// Value returns the token for logging. +func (t *SetupToken) Value() string { + if t == nil { + return "" + } + return t.value +} + +// Matches reports whether supplied is the token, in constant time. A nil +// token never matches anything, so a missing token fails closed. +func (t *SetupToken) Matches(supplied string) bool { + if t == nil || t.value == "" || supplied == "" { + return false + } + return subtle.ConstantTimeCompare([]byte(t.value), []byte(supplied)) == 1 +} diff --git a/web/src/lib/auth/store.svelte.ts b/web/src/lib/auth/store.svelte.ts index 49903a20..b8b26db2 100644 --- a/web/src/lib/auth/store.svelte.ts +++ b/web/src/lib/auth/store.svelte.ts @@ -47,10 +47,19 @@ export async function login(username: string, password: string): Promise { void sendTimezoneIfStale(); } +/** + * Whether the server has no accounts yet, in which case the first + * registration must carry the setup token printed in the server log. + */ +export async function getSetupStatus(): Promise<{ setup_required: boolean }> { + return api.get<{ setup_required: boolean }>('/api/auth/setup-status'); +} + export async function register(opts: { username: string; password: string; inviteToken?: string; + setupToken?: string; displayName?: string; }): Promise { const body: Record = { @@ -58,6 +67,7 @@ export async function register(opts: { password: opts.password, }; if (opts.inviteToken) body.invite_token = opts.inviteToken; + if (opts.setupToken) body.setup_token = opts.setupToken; if (opts.displayName) body.display_name = opts.displayName; const res = await api.post('/api/auth/register', body); setUser(res.user); diff --git a/web/src/routes/register/+page.svelte b/web/src/routes/register/+page.svelte index ae039203..dd37293a 100644 --- a/web/src/routes/register/+page.svelte +++ b/web/src/routes/register/+page.svelte @@ -1,7 +1,8 @@ {pageTitle('Settings')} @@ -553,9 +628,51 @@ - + +
+

Subsonic password

+

+ For Subsonic apps that can't use an API token and ask for a username and password instead. + Sign those apps in with your username and this password, not your login password. + Minstrel generates it, and has to store it readable for these apps to work, so it is never + your login password. Use the API token where the app supports it. +

+ {#if subsonicEnabled !== null} +

+ {subsonicEnabled ? 'A Subsonic password is set.' : "No Subsonic password is set; these apps can't sign in."} +

+ {/if} + {#if subsonicPassword} + + {subsonicPassword} + +

Copy this now. It won't be shown again.

+ {/if} +
+ {#if subsonicPassword} + + {/if} + + {#if subsonicEnabled} + + {/if} +
+
+ +
diff --git a/web/src/routes/settings/settings.test.ts b/web/src/routes/settings/settings.test.ts index 9e2d09c6..5ffd2b16 100644 --- a/web/src/routes/settings/settings.test.ts +++ b/web/src/routes/settings/settings.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, expect, test, vi } from 'vitest'; -import { render, screen, fireEvent, waitFor } from '@testing-library/svelte'; +import { render, screen, fireEvent, waitFor, within } from '@testing-library/svelte'; import { readable, writable } from 'svelte/store'; import type { LBStatus } from '$lib/api/listenbrainz'; @@ -16,7 +16,10 @@ vi.mock('$lib/api/me', () => ({ changePassword: vi.fn(), // Default to a resolved value so the page's $effect doesn't crash // on `.then()` of undefined when individual tests don't override. - regenerateAPIToken: vi.fn() + regenerateAPIToken: vi.fn(), + getSubsonicPasswordStatus: vi.fn(), + generateSubsonicPassword: vi.fn(), + clearSubsonicPassword: vi.fn() })); // Mutable holder so individual tests can inject populated metrics; @@ -45,7 +48,10 @@ import { import { updateProfile, changePassword, - regenerateAPIToken + regenerateAPIToken, + getSubsonicPasswordStatus, + generateSubsonicPassword, + clearSubsonicPassword } from '$lib/api/me'; function mockStatusStore(data: LBStatus) { @@ -361,3 +367,48 @@ describe('Settings page — API Token card', () => { expect(screen.queryByRole('button', { name: /^copy$/i })).toBeNull(); }); }); + +describe('Settings page — Subsonic password card', () => { + function mockLB() { + (createLBStatusQuery as ReturnType).mockReturnValue( + mockStatusStore({ enabled: false, token_set: false, last_scrobbled_at: null }) + ); + (createTokenMutation as ReturnType).mockReturnValue(mockMutationStore()); + (createEnabledMutation as ReturnType).mockReturnValue(mockMutationStore()); + } + + test('with none set, Generate creates one on the first click and shows it once', async () => { + mockLB(); + (getSubsonicPasswordStatus as ReturnType).mockResolvedValue({ enabled: false }); + (generateSubsonicPassword as ReturnType).mockResolvedValue({ password: 'gen_pw_123' }); + render(SettingsPage); + await waitFor(() => expect(screen.getByText(/no subsonic password is set/i)).toBeInTheDocument()); + expect(screen.queryByRole('button', { name: /turn off/i })).not.toBeInTheDocument(); + + await fireEvent.click(screen.getByRole('button', { name: /^generate$/i })); + await waitFor(() => expect(generateSubsonicPassword).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(screen.getByText('gen_pw_123')).toBeInTheDocument()); + expect(screen.getByText(/a subsonic password is set/i)).toBeInTheDocument(); + }); + + test('with one set, Regenerate and Turn off each need a second click', async () => { + mockLB(); + (getSubsonicPasswordStatus as ReturnType).mockResolvedValue({ enabled: true }); + (clearSubsonicPassword as ReturnType).mockResolvedValue(undefined); + render(SettingsPage); + // The API token card has a Regenerate button too; the Subsonic card's is + // the one beside Turn off. + const turnOff = await screen.findByRole('button', { name: /turn off/i }); + const subsonicRegen = within(turnOff.parentElement!).getByRole('button', { name: /^regenerate$/i }); + + await fireEvent.click(subsonicRegen); + expect(generateSubsonicPassword).not.toHaveBeenCalled(); + expect(subsonicRegen).toHaveTextContent(/click again to confirm/i); + + await fireEvent.click(turnOff); + expect(clearSubsonicPassword).not.toHaveBeenCalled(); + await fireEvent.click(turnOff); + await waitFor(() => expect(clearSubsonicPassword).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(screen.getByText(/no subsonic password is set/i)).toBeInTheDocument()); + }); +}); From f3196b3443579c00f4e3243891da15327e126b93 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 6 Oct 2026 13:01:01 -0400 Subject: [PATCH 15/18] feat(library): measure every track's loudness in the background (M464 #4995) The first step of loudness normalization: the server measures each track with ffmpeg's EBU R128 filter (true peak, mono as dual mono) and stores the integrated loudness, true peak and loudness range in track_loudness (migration 0065). It also keeps a histogram of the 400 ms gating blocks at 0.1 LU, so album loudness can be computed exactly later with no second decode (#4996). The histogram reproduces ffmpeg's own figure (-10.68 against -10.7 on the captured fixture), and the analyzer logs a warning if the two ever drift. - A background worker, cloned from the fingerprint backfill, measures every track, new ones included. Measuring inline in the scan was dropped: the analysis decodes the whole file, and a large import could pass the scan's one-hour stuck threshold. The scan only deletes a changed file's measurement; the worker ticks every 10 minutes. - Timeouts, the cancel/missing-binary split and settled verdicts follow the fingerprint runner. Silence and undecodable files are stored as verdicts; stalls are retried. The deadline scales with track length. - loudness_settings (enabled, files at once) and an admin card with the coverage gauge, under GET/PUT /api/admin/library/loudness-settings and GET /api/admin/library/loudness. Co-Authored-By: Claude Opus 5.5 --- cmd/minstrel/main.go | 13 + internal/api/admin_loudness.go | 82 ++++ internal/api/api.go | 9 +- internal/db/dbq/loudness.sql.go | 208 ++++++++++ internal/db/dbq/models.go | 19 + .../migrations/0065_track_loudness.down.sql | 2 + .../db/migrations/0065_track_loudness.up.sql | 65 +++ internal/db/queries/loudness.sql | 77 ++++ internal/dbtest/reset.go | 8 + internal/library/loudness.go | 392 ++++++++++++++++++ internal/library/loudness_backfill.go | 200 +++++++++ internal/library/loudness_backfill_test.go | 230 ++++++++++ internal/library/loudness_settings.go | 106 +++++ internal/library/loudness_test.go | 250 +++++++++++ internal/library/scanner.go | 7 + internal/library/testdata/ebur128_fixture.txt | 149 +++++++ internal/server/server.go | 13 +- web/src/lib/api/admin.ts | 32 ++ .../components/LoudnessSettingsCard.svelte | 173 ++++++++ .../components/LoudnessSettingsCard.test.ts | 94 +++++ web/src/routes/admin/+page.svelte | 5 + web/src/routes/admin/admin.test.ts | 8 +- 22 files changed, 2139 insertions(+), 3 deletions(-) create mode 100644 internal/api/admin_loudness.go create mode 100644 internal/db/dbq/loudness.sql.go create mode 100644 internal/db/migrations/0065_track_loudness.down.sql create mode 100644 internal/db/migrations/0065_track_loudness.up.sql create mode 100644 internal/db/queries/loudness.sql create mode 100644 internal/library/loudness.go create mode 100644 internal/library/loudness_backfill.go create mode 100644 internal/library/loudness_backfill_test.go create mode 100644 internal/library/loudness_settings.go create mode 100644 internal/library/loudness_test.go create mode 100644 internal/library/testdata/ebur128_fixture.txt create mode 100644 web/src/lib/components/LoudnessSettingsCard.svelte create mode 100644 web/src/lib/components/LoudnessSettingsCard.test.ts diff --git a/cmd/minstrel/main.go b/cmd/minstrel/main.go index a616e673..7652bd2f 100644 --- a/cmd/minstrel/main.go +++ b/cmd/minstrel/main.go @@ -132,6 +132,13 @@ func run() error { } scanner := library.New(pool, logger, cfg.Library.ScanPaths, fpSettings) + // Loudness analysis settings (M464 #4995): shared by the loudness backfill + // and the admin API, and falls back to the defaults like the above. + loudSettings, loudErr := library.NewLoudnessSettingsService(ctx, pool) + if loudErr != nil { + logger.Warn("loudness settings: using defaults", "err", loudErr) + } + contact := cfg.Library.ContactEmail if contact == "" { contact = "https://git.fabledsword.com/bvandeusen/minstrel" @@ -228,6 +235,11 @@ func run() error { // internal/library/fingerprint_backfill.go for why. go library.NewFingerprintBackfillWorker(pool, logger.With("component", "fingerprint_backfill"), fpSettings).Run(ctx) + // Loudness backfill (M464 #4995): measures every track's loudness for + // normalization, new tracks included; the scan only drops a changed file's + // measurement. See internal/library/loudness_backfill.go. + go library.NewLoudnessBackfillWorker(pool, logger.With("component", "loudness_backfill"), loudSettings).Run(ctx) + // Duplicate sweep (M400 #3910): proposes groups of tracks holding one // recording, from the fingerprints above. Sweeps only when fingerprints have // changed since the last sweep. @@ -377,6 +389,7 @@ func run() error { srv.RecSettings = recSettings srv.TagSettings = tagSettings srv.FingerprintSettings = fpSettings + srv.LoudnessSettings = loudSettings // The sweeper above holds this same instance, so a save from the admin // card changes what it does on its next tick (#3936). srv.ReacqSettings = reacqSettings diff --git a/internal/api/admin_loudness.go b/internal/api/admin_loudness.go new file mode 100644 index 00000000..13cf850d --- /dev/null +++ b/internal/api/admin_loudness.go @@ -0,0 +1,82 @@ +package api + +import ( + "encoding/json" + "errors" + "net/http" + + "git.fabledsword.com/bvandeusen/minstrel/internal/apierror" + "git.fabledsword.com/bvandeusen/minstrel/internal/library" +) + +// loudnessCoverageResp is the wire shape for GET /api/admin/library/loudness +// (M464 #4995). measured + silent + unreadable + pending = total; missing +// tracks are not counted. Enabled travels with the counts because with +// analysis off, pending never shrinks, and a gauge implying progress would be +// promising work nothing is doing. +type loudnessCoverageResp struct { + Total int64 `json:"total"` + Measured int64 `json:"measured"` + Silent int64 `json:"silent"` + Unreadable int64 `json:"unreadable"` + Pending int64 `json:"pending"` + Enabled bool `json:"enabled"` +} + +// handleGetLoudnessCoverage implements GET /api/admin/library/loudness: how +// far the loudness backfill has got. Always 200; zeros on an empty library. +func (h *handlers) handleGetLoudnessCoverage(w http.ResponseWriter, r *http.Request) { + row, err := library.LoudnessCoverage(r.Context(), h.pool) + if err != nil { + writeErrWithLog(w, h.logger, "admin: get loudness coverage", apierror.InternalMsg("lookup failed", err)) + return + } + writeJSON(w, http.StatusOK, loudnessCoverageResp{ + Total: row.Total, + Measured: row.Measured, + Silent: row.Silent, + Unreadable: row.Unreadable, + Pending: row.Pending, + Enabled: h.loudnessSettings.Get().Enabled, + }) +} + +// loudnessSettingsBody is the wire shape for GET and PUT +// /api/admin/library/loudness-settings. +type loudnessSettingsBody struct { + Enabled bool `json:"enabled"` + BackfillConcurrency int32 `json:"backfill_concurrency"` +} + +func loudnessSettingsBodyOf(s library.LoudnessSettings) loudnessSettingsBody { + return loudnessSettingsBody{Enabled: s.Enabled, BackfillConcurrency: s.BackfillConcurrency} +} + +// handleGetLoudnessSettings implements GET /api/admin/library/loudness-settings. +func (h *handlers) handleGetLoudnessSettings(w http.ResponseWriter, _ *http.Request) { + writeJSON(w, http.StatusOK, loudnessSettingsBodyOf(h.loudnessSettings.Get())) +} + +// handleUpdateLoudnessSettings implements PUT /api/admin/library/loudness-settings. +// A whole-row write: a body that leaves out the concurrency decodes it as zero, +// which is refused rather than saved. +func (h *handlers) handleUpdateLoudnessSettings(w http.ResponseWriter, r *http.Request) { + var req loudnessSettingsBody + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + writeErr(w, apierror.BadRequest("invalid_body", "malformed JSON")) + return + } + saved, err := h.loudnessSettings.Set(r.Context(), library.LoudnessSettings{ + Enabled: req.Enabled, + BackfillConcurrency: req.BackfillConcurrency, + }) + if err != nil { + if errors.Is(err, library.ErrLoudnessSettingOutOfRange) { + writeErr(w, apierror.BadRequest("invalid_setting", err.Error())) + return + } + writeErrWithLog(w, h.logger, "admin loudness settings: update failed", apierror.Internal(err)) + return + } + writeJSON(w, http.StatusOK, loudnessSettingsBodyOf(saved)) +} diff --git a/internal/api/api.go b/internal/api/api.go index cdbff7db..65d4240b 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -34,7 +34,7 @@ import ( // Mount attaches /api/* handlers to r. Public endpoints (login) are outside // RequireUser; everything else is gated by the middleware. The events writer // is shared with the Subsonic mount so /rest/scrobble feeds the same store. -func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playevents.Writer, recCfg config.RecommendationConfig, recSettings *recsettings.Service, lidarrCfg *lidarrconfig.Service, lidarrReqs *lidarrrequests.Service, lidarrQuar *lidarrquarantine.Service, tracksSvc *tracks.Service, playlistsSvc *playlists.Service, coverEnricher *coverart.Enricher, coverSettings *coverart.SettingsService, tagSettings *tags.SettingsService, scanner *library.Scanner, scanCfg library.RunScanConfig, dataDir string, sender mailer.Sender, bus *eventbus.Bus, playlistScheduler *playlists.Scheduler, streamSecret []byte, netSettings *netsettings.Service, reacqSettings *reacquisition.SettingsService, fpSettings *library.FingerprintSettingsService) { +func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playevents.Writer, recCfg config.RecommendationConfig, recSettings *recsettings.Service, lidarrCfg *lidarrconfig.Service, lidarrReqs *lidarrrequests.Service, lidarrQuar *lidarrquarantine.Service, tracksSvc *tracks.Service, playlistsSvc *playlists.Service, coverEnricher *coverart.Enricher, coverSettings *coverart.SettingsService, tagSettings *tags.SettingsService, scanner *library.Scanner, scanCfg library.RunScanConfig, dataDir string, sender mailer.Sender, bus *eventbus.Bus, playlistScheduler *playlists.Scheduler, streamSecret []byte, netSettings *netsettings.Service, reacqSettings *reacquisition.SettingsService, fpSettings *library.FingerprintSettingsService, loudSettings *library.LoudnessSettingsService) { rng := rand.New(rand.NewSource(rand.Int63())) setupToken, err := auth.NewSetupToken() if err != nil { @@ -65,6 +65,7 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev netSettings: netSettings, reacqSettings: reacqSettings, fingerprintSettings: fpSettings, + loudnessSettings: loudSettings, librarySize: recommendation.NewLibrarySize(nil), loginGuard: auth.NewLoginGuard(), setupToken: setupToken, @@ -237,6 +238,9 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev admin.Get("/library/fingerprints", h.handleGetFingerprintCoverage) admin.Get("/library/fingerprint-settings", h.handleGetFingerprintSettings) admin.Put("/library/fingerprint-settings", h.handleUpdateFingerprintSettings) + admin.Get("/library/loudness", h.handleGetLoudnessCoverage) + admin.Get("/library/loudness-settings", h.handleGetLoudnessSettings) + admin.Put("/library/loudness-settings", h.handleUpdateLoudnessSettings) // Duplicates report (#3912): proposals from the duplicate sweep, a // trigger to sweep now, dismissal, and the merge (#3911), which deletes // the removed copies' files after moving their history onto the kept one. @@ -331,6 +335,9 @@ type handlers struct { // instance the scanner and the fingerprint workers read, so a save from the // admin card reaches them without a restart. Nil serves the defaults. fingerprintSettings *library.FingerprintSettingsService + // loudnessSettings is the loudness analysis policy (M464 #4995), the same + // instance the loudness backfill reads. Nil serves the defaults. + loudnessSettings *library.LoudnessSettingsService // setupToken must accompany the first registration while no users exist // (see auth.SetupToken). requireSetupToken is set by Mount, the only // production constructor; tests that build handlers directly leave it diff --git a/internal/db/dbq/loudness.sql.go b/internal/db/dbq/loudness.sql.go new file mode 100644 index 00000000..1c05ba61 --- /dev/null +++ b/internal/db/dbq/loudness.sql.go @@ -0,0 +1,208 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.31.1 +// source: loudness.sql + +package dbq + +import ( + "context" + + "github.com/jackc/pgx/v5/pgtype" +) + +const deleteTrackLoudness = `-- name: DeleteTrackLoudness :exec +DELETE FROM track_loudness WHERE track_id = $1 +` + +// The scan saw new bytes at this path. The stored measurement describes the old +// ones, so it goes, and the backfill measures the file again. +func (q *Queries) DeleteTrackLoudness(ctx context.Context, trackID pgtype.UUID) error { + _, err := q.db.Exec(ctx, deleteTrackLoudness, trackID) + return err +} + +const getLoudnessCoverage = `-- name: GetLoudnessCoverage :one +SELECT count(*)::bigint AS total, + count(*) FILTER ( + WHERE l.analysis_version >= $1 + AND l.integrated_lufs IS NOT NULL + )::bigint AS measured, + count(*) FILTER ( + WHERE l.analysis_version >= $1 + AND l.integrated_lufs IS NULL AND NOT l.unreadable + )::bigint AS silent, + count(*) FILTER ( + WHERE l.analysis_version >= $1 + AND l.unreadable + )::bigint AS unreadable, + count(*) FILTER ( + WHERE l.track_id IS NULL OR l.analysis_version < $1 + )::bigint AS pending + FROM tracks t + LEFT JOIN track_loudness l ON l.track_id = t.id + WHERE t.missing_since IS NULL +` + +type GetLoudnessCoverageRow struct { + Total int64 + Measured int64 + Silent int64 + Unreadable int64 + Pending int64 +} + +// The admin gauge. measured + silent + unreadable + pending = total. Missing +// tracks are excluded, or the gauge could never reach the end. +func (q *Queries) GetLoudnessCoverage(ctx context.Context, currentVersion int16) (GetLoudnessCoverageRow, error) { + row := q.db.QueryRow(ctx, getLoudnessCoverage, currentVersion) + var i GetLoudnessCoverageRow + err := row.Scan( + &i.Total, + &i.Measured, + &i.Silent, + &i.Unreadable, + &i.Pending, + ) + return i, err +} + +const getLoudnessSettings = `-- name: GetLoudnessSettings :one +SELECT id, enabled, backfill_concurrency, updated_at FROM loudness_settings WHERE id = true +` + +func (q *Queries) GetLoudnessSettings(ctx context.Context) (LoudnessSetting, error) { + row := q.db.QueryRow(ctx, getLoudnessSettings) + var i LoudnessSetting + err := row.Scan( + &i.ID, + &i.Enabled, + &i.BackfillConcurrency, + &i.UpdatedAt, + ) + return i, err +} + +const listTracksNeedingLoudness = `-- name: ListTracksNeedingLoudness :many +SELECT t.id, t.file_path, t.duration_ms + FROM tracks t + LEFT JOIN track_loudness l ON l.track_id = t.id + WHERE t.missing_since IS NULL + AND (l.track_id IS NULL OR l.analysis_version < $1) + AND t.id > $2 + ORDER BY t.id + LIMIT $3 +` + +type ListTracksNeedingLoudnessParams struct { + CurrentVersion int16 + AfterID pgtype.UUID + BatchLimit int32 +} + +type ListTracksNeedingLoudnessRow struct { + ID pgtype.UUID + FilePath string + DurationMs int32 +} + +// The backfill's work queue: tracks with no measurement, or one taken by an +// older method. Keyset-paged on id so a pass visits each track at most once; +// an inconclusive attempt writes no row, and without the cursor a file that +// keeps timing out would be listed again straight away. Missing tracks are +// skipped: there is no file to read. +func (q *Queries) ListTracksNeedingLoudness(ctx context.Context, arg ListTracksNeedingLoudnessParams) ([]ListTracksNeedingLoudnessRow, error) { + rows, err := q.db.Query(ctx, listTracksNeedingLoudness, arg.CurrentVersion, arg.AfterID, arg.BatchLimit) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListTracksNeedingLoudnessRow + for rows.Next() { + var i ListTracksNeedingLoudnessRow + if err := rows.Scan(&i.ID, &i.FilePath, &i.DurationMs); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const updateLoudnessSettings = `-- name: UpdateLoudnessSettings :one +UPDATE loudness_settings + SET enabled = $1, + backfill_concurrency = $2, + updated_at = now() + WHERE id = true +RETURNING id, enabled, backfill_concurrency, updated_at +` + +type UpdateLoudnessSettingsParams struct { + Enabled bool + BackfillConcurrency int32 +} + +// Whole-row write from the admin card; migration 0065's CHECK is the backstop +// behind the service's own validation. +func (q *Queries) UpdateLoudnessSettings(ctx context.Context, arg UpdateLoudnessSettingsParams) (LoudnessSetting, error) { + row := q.db.QueryRow(ctx, updateLoudnessSettings, arg.Enabled, arg.BackfillConcurrency) + var i LoudnessSetting + err := row.Scan( + &i.ID, + &i.Enabled, + &i.BackfillConcurrency, + &i.UpdatedAt, + ) + return i, err +} + +const upsertTrackLoudness = `-- name: UpsertTrackLoudness :exec +INSERT INTO track_loudness ( + track_id, integrated_lufs, true_peak_dbtp, loudness_range_lu, + block_hist_start, block_hist, unreadable, analysis_version +) VALUES ( + $1, $2, $3, + $4, $5, $6, + $7, $8 +) +ON CONFLICT (track_id) DO UPDATE SET + integrated_lufs = EXCLUDED.integrated_lufs, + true_peak_dbtp = EXCLUDED.true_peak_dbtp, + loudness_range_lu = EXCLUDED.loudness_range_lu, + block_hist_start = EXCLUDED.block_hist_start, + block_hist = EXCLUDED.block_hist, + unreadable = EXCLUDED.unreadable, + analysis_version = EXCLUDED.analysis_version, + analyzed_at = now() +` + +type UpsertTrackLoudnessParams struct { + TrackID pgtype.UUID + IntegratedLufs *float32 + TruePeakDbtp *float32 + LoudnessRangeLu *float32 + BlockHistStart *int16 + BlockHist []int32 + Unreadable bool + AnalysisVersion int16 +} + +// Written when the backfill measures a track (#4995). Replaces the row +// wholesale: a measurement of the old bytes has no standing once the file has +// changed. +func (q *Queries) UpsertTrackLoudness(ctx context.Context, arg UpsertTrackLoudnessParams) error { + _, err := q.db.Exec(ctx, upsertTrackLoudness, + arg.TrackID, + arg.IntegratedLufs, + arg.TruePeakDbtp, + arg.LoudnessRangeLu, + arg.BlockHistStart, + arg.BlockHist, + arg.Unreadable, + arg.AnalysisVersion, + ) + return err +} diff --git a/internal/db/dbq/models.go b/internal/db/dbq/models.go index 88b1901d..99b5cbf7 100644 --- a/internal/db/dbq/models.go +++ b/internal/db/dbq/models.go @@ -417,6 +417,13 @@ type LidarrRequest struct { LidarrAddConfirmedAt pgtype.Timestamptz } +type LoudnessSetting struct { + ID bool + Enabled bool + BackfillConcurrency int32 + UpdatedAt pgtype.Timestamptz +} + type MissingReacquisition struct { AlbumID pgtype.UUID Attempts int32 @@ -713,6 +720,18 @@ type TrackFingerprint struct { ChromaprintLengthSec int32 } +type TrackLoudness struct { + TrackID pgtype.UUID + IntegratedLufs *float32 + TruePeakDbtp *float32 + LoudnessRangeLu *float32 + BlockHistStart *int16 + BlockHist []int32 + Unreadable bool + AnalysisVersion int16 + AnalyzedAt pgtype.Timestamptz +} + type TrackSimilarity struct { TrackAID pgtype.UUID TrackBID pgtype.UUID diff --git a/internal/db/migrations/0065_track_loudness.down.sql b/internal/db/migrations/0065_track_loudness.down.sql new file mode 100644 index 00000000..0bb9eced --- /dev/null +++ b/internal/db/migrations/0065_track_loudness.down.sql @@ -0,0 +1,2 @@ +DROP TABLE IF EXISTS loudness_settings; +DROP TABLE IF EXISTS track_loudness; diff --git a/internal/db/migrations/0065_track_loudness.up.sql b/internal/db/migrations/0065_track_loudness.up.sql new file mode 100644 index 00000000..ae039dc6 --- /dev/null +++ b/internal/db/migrations/0065_track_loudness.up.sql @@ -0,0 +1,65 @@ +-- 0065_track_loudness.up.sql — measured loudness per track, for loudness +-- normalization (Scribe milestone #464, #4995). +-- +-- Measured with ffmpeg's EBU R128 filter rather than read from ReplayGain tags: +-- tags in the wild are written against four different reference levels, and +-- most files have none. internal/library/loudness.go says how it is measured. +-- +-- A table of its own rather than columns on tracks, for the reason +-- track_fingerprints is (0058): tracks is read with SELECT * on the hot path, +-- and the block histogram is a few hundred integers only album loudness reads. +-- +-- What a row means, which the backfill depends on: +-- no row never analyzed, or the file changed since +-- analysis_version < current measured by an older method; measure again +-- analysis_version = current settled until the file changes: +-- integrated_lufs NOT NULL measured +-- integrated_lufs NULL, unreadable false +-- read fine, but no 400 ms block was above +-- the -70 LUFS gate: silence, or too short +-- unreadable true ffmpeg could not decode the file +-- A failure that says nothing about the file (a timeout, a cancelled pass, a +-- missing ffmpeg) writes no row, so the backfill tries again. +CREATE TABLE track_loudness ( + track_id uuid PRIMARY KEY REFERENCES tracks (id) ON DELETE CASCADE, + -- Gated integrated loudness (ITU-R BS.1770), mono measured as dual mono. + integrated_lufs real, + -- Highest inter-sample peak, from 4x oversampling, in dB relative to full + -- scale. NULL for digital silence, whose peak is -inf. + true_peak_dbtp real, + -- Loudness range (EBU Tech 3342): how much the loudness moves within the + -- track. Not used for gain; kept because it costs nothing here. + loudness_range_lu real, + -- How many 400 ms gating blocks fell in each 0.1 LU bin. Bin i holds blocks + -- measuring -70.0 + (block_hist_start + i) / 10 LUFS; the array is trimmed + -- to the first and last non-empty bins. Album loudness is the gated loudness + -- of every block on the album, so it is computed from these exactly, with no + -- second decode (#4996). + block_hist_start smallint, + block_hist integer[], + unreadable boolean NOT NULL DEFAULT false, + analysis_version smallint NOT NULL, + analyzed_at timestamptz NOT NULL DEFAULT now(), + + CONSTRAINT track_loudness_hist_pair + CHECK ((block_hist IS NULL) = (block_hist_start IS NULL)) +); + +-- Loudness analysis's knobs, in admin Settings. Rule 25: an operator setting is +-- a database row, changed without a restart. Singleton in the style of +-- fingerprint_settings (0061). +CREATE TABLE loudness_settings ( + id boolean PRIMARY KEY DEFAULT true, + -- Off stops the background analysis. Tracks already measured keep their + -- values, so normalization keeps working for them. + enabled boolean NOT NULL DEFAULT true, + -- Files analyzed at once. Each is a full decode, competing with playback + -- transcoding for CPU and with streaming for the mount. + backfill_concurrency integer NOT NULL DEFAULT 2, + updated_at timestamptz NOT NULL DEFAULT now(), + + CONSTRAINT loudness_settings_singleton CHECK (id = true), + CONSTRAINT loudness_settings_concurrency_range + CHECK (backfill_concurrency >= 1 AND backfill_concurrency <= 8) +); +INSERT INTO loudness_settings (id) VALUES (true) ON CONFLICT (id) DO NOTHING; diff --git a/internal/db/queries/loudness.sql b/internal/db/queries/loudness.sql new file mode 100644 index 00000000..821d0ad5 --- /dev/null +++ b/internal/db/queries/loudness.sql @@ -0,0 +1,77 @@ +-- name: UpsertTrackLoudness :exec +-- Written when the backfill measures a track (#4995). Replaces the row +-- wholesale: a measurement of the old bytes has no standing once the file has +-- changed. +INSERT INTO track_loudness ( + track_id, integrated_lufs, true_peak_dbtp, loudness_range_lu, + block_hist_start, block_hist, unreadable, analysis_version +) VALUES ( + sqlc.arg(track_id), sqlc.narg(integrated_lufs), sqlc.narg(true_peak_dbtp), + sqlc.narg(loudness_range_lu), sqlc.narg(block_hist_start), sqlc.narg(block_hist), + sqlc.arg(unreadable), sqlc.arg(analysis_version) +) +ON CONFLICT (track_id) DO UPDATE SET + integrated_lufs = EXCLUDED.integrated_lufs, + true_peak_dbtp = EXCLUDED.true_peak_dbtp, + loudness_range_lu = EXCLUDED.loudness_range_lu, + block_hist_start = EXCLUDED.block_hist_start, + block_hist = EXCLUDED.block_hist, + unreadable = EXCLUDED.unreadable, + analysis_version = EXCLUDED.analysis_version, + analyzed_at = now(); + +-- name: DeleteTrackLoudness :exec +-- The scan saw new bytes at this path. The stored measurement describes the old +-- ones, so it goes, and the backfill measures the file again. +DELETE FROM track_loudness WHERE track_id = $1; + +-- name: ListTracksNeedingLoudness :many +-- The backfill's work queue: tracks with no measurement, or one taken by an +-- older method. Keyset-paged on id so a pass visits each track at most once; +-- an inconclusive attempt writes no row, and without the cursor a file that +-- keeps timing out would be listed again straight away. Missing tracks are +-- skipped: there is no file to read. +SELECT t.id, t.file_path, t.duration_ms + FROM tracks t + LEFT JOIN track_loudness l ON l.track_id = t.id + WHERE t.missing_since IS NULL + AND (l.track_id IS NULL OR l.analysis_version < sqlc.arg(current_version)) + AND t.id > sqlc.arg(after_id) + ORDER BY t.id + LIMIT sqlc.arg(batch_limit); + +-- name: GetLoudnessCoverage :one +-- The admin gauge. measured + silent + unreadable + pending = total. Missing +-- tracks are excluded, or the gauge could never reach the end. +SELECT count(*)::bigint AS total, + count(*) FILTER ( + WHERE l.analysis_version >= sqlc.arg(current_version) + AND l.integrated_lufs IS NOT NULL + )::bigint AS measured, + count(*) FILTER ( + WHERE l.analysis_version >= sqlc.arg(current_version) + AND l.integrated_lufs IS NULL AND NOT l.unreadable + )::bigint AS silent, + count(*) FILTER ( + WHERE l.analysis_version >= sqlc.arg(current_version) + AND l.unreadable + )::bigint AS unreadable, + count(*) FILTER ( + WHERE l.track_id IS NULL OR l.analysis_version < sqlc.arg(current_version) + )::bigint AS pending + FROM tracks t + LEFT JOIN track_loudness l ON l.track_id = t.id + WHERE t.missing_since IS NULL; + +-- name: GetLoudnessSettings :one +SELECT * FROM loudness_settings WHERE id = true; + +-- name: UpdateLoudnessSettings :one +-- Whole-row write from the admin card; migration 0065's CHECK is the backstop +-- behind the service's own validation. +UPDATE loudness_settings + SET enabled = sqlc.arg(enabled), + backfill_concurrency = sqlc.arg(backfill_concurrency), + updated_at = now() + WHERE id = true +RETURNING *; diff --git a/internal/dbtest/reset.go b/internal/dbtest/reset.go index a5b2380e..8d1558dc 100644 --- a/internal/dbtest/reset.go +++ b/internal/dbtest/reset.go @@ -91,6 +91,7 @@ var dataTables = []string{ "duplicate_groups", "duplicate_sweeps", "track_fingerprints", // M400 + "track_loudness", // M464 "tracks", "albums", "artists", @@ -141,4 +142,11 @@ func ResetDB(t *testing.T, pool *pgxpool.Pool) { ); err != nil { t.Fatalf("dbtest.ResetDB reset fingerprint settings: %v", err) } + // Loudness analysis settings (M464 #4995), reset the same way. + if _, err := pool.Exec(ctx, ` + UPDATE loudness_settings + SET enabled = DEFAULT, backfill_concurrency = DEFAULT, updated_at = DEFAULT`, + ); err != nil { + t.Fatalf("dbtest.ResetDB reset loudness settings: %v", err) + } } diff --git a/internal/library/loudness.go b/internal/library/loudness.go new file mode 100644 index 00000000..ffea3c9f --- /dev/null +++ b/internal/library/loudness.go @@ -0,0 +1,392 @@ +package library + +import ( + "bufio" + "context" + "errors" + "fmt" + "io" + "log/slog" + "math" + "os/exec" + "regexp" + "strconv" + "strings" + "time" + + "github.com/jackc/pgx/v5/pgtype" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +// Loudness analysis (M464 #4995). +// +// Every track is measured with ffmpeg's EBU R128 filter, which reports the +// ITU-R BS.1770 values loudness normalization works from: +// +// integrated loudness the gated, K-weighted average loudness of the whole +// track, in LUFS. A client levels a track by playing it +// at (target - integrated) dB. +// true peak the highest inter-sample peak, in dBTP. It bounds how +// far a quiet track can be raised before it clips. +// loudness range how much the loudness moves within the track. +// +// Tags are not trusted: ReplayGain values in the wild are written against four +// different reference levels, and most files have none. +// +// The filter also logs the loudness of every 400 ms gating block (one every +// 100 ms). Those are kept as a histogram, because an album's loudness is the +// gated loudness of every block on the album, not an average of its tracks' +// values. With the histograms stored, album loudness is exact and needs no +// second decode (#4996). + +// loudnessVersion stamps how a track_loudness row was measured. Bump it when +// the measurement changes (the filter's options, the histogram's bins) and the +// backfill measures every row below it again. +const loudnessVersion int16 = 1 + +// Unlike a fingerprint, the analysis decodes the whole file, so a fixed +// deadline would either cut off a long mix or be useless for a three-minute +// song. The deadline is a base plus the track's length at loudnessMinSpeed: +// a decode slower than that is a stall, not a big file. +const ( + loudnessBaseTimeout = 2 * time.Minute + loudnessMinSpeed = 4 +) + +// errLoudnessTimeout marks an analysis that ran out of time: a fact about the +// mount, not about the file. +var errLoudnessTimeout = errors.New("loudness analysis timed out") + +// The block histogram's bins: 0.1 LU wide (the precision ffmpeg prints) from +// the -70 LUFS absolute gate up to +10 LUFS. Blocks below the gate do not take +// part in gating at all, so they are not counted; anything above the top bin, +// which only clipped noise reaches, is counted in it. +const ( + loudnessHistFloor = -70.0 + loudnessHistPerLU = 10 + loudnessHistBins = 800 + loudnessStderrTail = 20 // lines kept for an error message +) + +// histogramCrossCheckLU is how far the loudness recomputed from the histogram +// may stray from ffmpeg's own figure before it is logged. They agree to a few +// hundredths when the log means what the parser assumes, so a larger gap says +// ffmpeg's output has changed underneath us. +const histogramCrossCheckLU = 0.3 + +// ebur128Args measures the file's first audio stream. +// +// peak=true asks for true peak (4x oversampled) rather than sample peak, which +// misses the inter-sample overs a boosted track would clip on. dualmono=true +// measures a mono file as if played on both speakers of a stereo pair, which is +// how it is heard; measured as one channel it would read 3 LU quiet and be +// boosted too far. framelog=info makes the per-block lines print at the log +// level asked for here, independent of ffmpeg's default. +func ebur128Args(path string) []string { + return []string{ + "-hide_banner", "-nostdin", "-nostats", + "-loglevel", "info", + "-i", path, + "-map", "0:a:0", + "-af", "ebur128=peak=true:dualmono=true:framelog=info", + "-f", "null", "-", + } +} + +// loudnessTimeout is the deadline for a track of durationMs. An unknown length +// (0) gets the base alone, which covers an ordinary song. +func loudnessTimeout(durationMs int32) time.Duration { + return loudnessBaseTimeout + time.Duration(max(durationMs, 0))*time.Millisecond/loudnessMinSpeed +} + +// loudnessResult is one attempt at measuring a track. +type loudnessResult struct { + // integratedLUFS is nil when no block passed the gates: silence, or a + // file shorter than one 400 ms block. + integratedLUFS *float32 + truePeakDBTP *float32 + rangeLU *float32 + hist blockHistogram + err error +} + +// inconclusive reports whether the attempt failed for a reason that says +// nothing about the file. Such a result is never stored: stamped at the current +// version it would read as "this file cannot be measured", and the backfill +// would never try it again. +func (r loudnessResult) inconclusive() bool { + return isInconclusive(r.err) || errors.Is(r.err, errLoudnessTimeout) +} + +// blockHistogram counts gating blocks per 0.1 LU bin, trimmed to the occupied +// range: counts[i] is the number of blocks measuring +// loudnessHistFloor + (start+i)/loudnessHistPerLU LUFS. +type blockHistogram struct { + start int16 + counts []int32 +} + +func (h blockHistogram) empty() bool { return len(h.counts) == 0 } + +// gatedLoudness applies BS.1770's two gates to the histogram and returns the +// integrated loudness of what passes. Every counted block is already above the +// absolute gate; the relative gate drops blocks more than 10 LU below the +// loudness of the blocks that passed it. ok is false when nothing passes. +// +// The same function measures an album, over the sum of its tracks' +// histograms (#4996). +func (h blockHistogram) gatedLoudness() (lufs float64, ok bool) { + energy := func(i int) float64 { + l := loudnessHistFloor + float64(int(h.start)+i)/loudnessHistPerLU + return math.Pow(10, (l+0.691)/10) + } + mean := func(threshold float64) (float64, bool) { + var sum, n float64 + for i, c := range h.counts { + if c == 0 { + continue + } + if loudnessHistFloor+float64(int(h.start)+i)/loudnessHistPerLU < threshold { + continue + } + sum += float64(c) * energy(i) + n += float64(c) + } + if n == 0 { + return 0, false + } + return sum / n, true + } + ungated, ok := mean(math.Inf(-1)) + if !ok { + return 0, false + } + relative := -0.691 + 10*math.Log10(ungated) - 10 + gated, ok := mean(relative) + if !ok { + return 0, false + } + return -0.691 + 10*math.Log10(gated), true +} + +// computeLoudness measures the file at path. durationMs sets the deadline. +func computeLoudness(ctx context.Context, path string, durationMs int32) loudnessResult { + timeout := loudnessTimeout(durationMs) + runCtx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + + cmd := exec.CommandContext(runCtx, "ffmpeg", ebur128Args(path)...) + cmd.WaitDelay = fingerprintWaitDelay + stderr, err := cmd.StderrPipe() + if err != nil { + return loudnessResult{err: fmt.Errorf("ffmpeg: %w", err)} + } + if err := cmd.Start(); err != nil { + return loudnessResult{err: fmt.Errorf("ffmpeg: %w", err)} + } + // The per-block log runs to ten lines a second of audio, about 12 MB for a + // two-hour mix, so it is parsed as it streams rather than buffered. + p := newEbur128Parser() + p.consume(stderr) + waitErr := cmd.Wait() + + switch { + case ctx.Err() != nil: + // The caller gave up. Report that rather than the kill it caused, so it + // is never mistaken for a verdict on the file. + return loudnessResult{err: fmt.Errorf("ffmpeg: %w", ctx.Err())} + case errors.Is(runCtx.Err(), context.DeadlineExceeded): + return loudnessResult{err: fmt.Errorf("ffmpeg: no result within %s: %w", timeout, errLoudnessTimeout)} + case waitErr != nil: + var exitErr *exec.ExitError + if errors.As(waitErr, &exitErr) { + return loudnessResult{err: fmt.Errorf("ffmpeg exited %d: %s", exitErr.ExitCode(), p.tail())} + } + return loudnessResult{err: fmt.Errorf("ffmpeg: %w", waitErr)} + } + return p.result() +} + +var ( + // A per-block line: "[Parsed_ebur128_0 @ 0x…] t: 2.49998 TARGET:-23 LUFS + // M: -31.1 S:-120.7 I: -31.1 LUFS ...". M is the 400 ms block just ended. + ebur128BlockRe = regexp.MustCompile(`\bt:\s*\S+\s+TARGET:.*?\bM:\s*(-?(?:\d+(?:\.\d+)?|inf))`) + // The summary's values, each on a line of its own after "Summary:". + ebur128SummaryRe = regexp.MustCompile(`^(I|LRA|Peak):\s+(-?(?:\d+(?:\.\d+)?|inf))\s+(?:LUFS|LU|dBFS)$`) +) + +// ebur128Parser reads the filter's log: the per-block lines into the +// histogram, and the closing summary. +type ebur128Parser struct { + bins [loudnessHistBins]int32 + blocks int + inSummary bool + summary map[string]string + lastLines []string +} + +func newEbur128Parser() *ebur128Parser { + return &ebur128Parser{summary: map[string]string{}} +} + +func (p *ebur128Parser) consume(r io.Reader) { + sc := bufio.NewScanner(r) + sc.Buffer(make([]byte, 0, 4096), 64*1024) + for sc.Scan() { + p.line(sc.Text()) + } + // A line past the buffer (not something ffmpeg prints) stops the scanner; + // drain the rest so ffmpeg is never blocked writing to a full pipe. + _, _ = io.Copy(io.Discard, r) +} + +func (p *ebur128Parser) line(raw string) { + line := strings.TrimSpace(strings.TrimRight(raw, "\r")) + if line == "" { + return + } + if len(p.lastLines) == loudnessStderrTail { + p.lastLines = p.lastLines[1:] + } + p.lastLines = append(p.lastLines, line) + + if strings.HasSuffix(line, "Summary:") { + p.inSummary = true + return + } + if p.inSummary { + if m := ebur128SummaryRe.FindStringSubmatch(line); m != nil { + p.summary[m[1]] = m[2] + } + return + } + m := ebur128BlockRe.FindStringSubmatch(line) + if m == nil { + return + } + v, err := strconv.ParseFloat(m[1], 64) + if err != nil || v < loudnessHistFloor { + // -inf, or below the absolute gate: such a block takes no part in + // gating. + return + } + bin := int(math.Round((v - loudnessHistFloor) * loudnessHistPerLU)) + p.bins[min(bin, loudnessHistBins-1)]++ + p.blocks++ +} + +func (p *ebur128Parser) tail() string { + return strings.Join(p.lastLines, " | ") +} + +func (p *ebur128Parser) histogram() blockHistogram { + if p.blocks == 0 { + return blockHistogram{} + } + first, last := 0, loudnessHistBins-1 + for p.bins[first] == 0 { + first++ + } + for p.bins[last] == 0 { + last-- + } + counts := make([]int32, last-first+1) + copy(counts, p.bins[first:last+1]) + return blockHistogram{start: int16(first), counts: counts} +} + +// result turns a clean exit into a measurement. A run with no summary means +// ffmpeg decoded nothing it could measure, which is a verdict on the file. +func (p *ebur128Parser) result() loudnessResult { + integrated, ok := p.summary["I"] + if !ok { + return loudnessResult{err: fmt.Errorf("ffmpeg printed no loudness summary: %s", p.tail())} + } + r := loudnessResult{ + hist: p.histogram(), + truePeakDBTP: parseLoudnessValue(p.summary["Peak"]), + rangeLU: parseLoudnessValue(p.summary["LRA"]), + } + // ffmpeg reports -70.0 when no block passed the gate. The empty histogram + // says the same thing directly. + if !r.hist.empty() { + r.integratedLUFS = parseLoudnessValue(integrated) + } + if r.integratedLUFS == nil { + r.rangeLU = nil + } + return r +} + +// parseLoudnessValue reads one summary figure; -inf and anything unreadable +// come back nil. +func parseLoudnessValue(s string) *float32 { + v, err := strconv.ParseFloat(s, 32) + if err != nil || math.IsInf(v, 0) || math.IsNaN(v) { + return nil + } + f := float32(v) + return &f +} + +// loudnessOutcome is what storeLoudness did with one attempt. +type loudnessOutcome int + +const ( + loudnessMeasured loudnessOutcome = iota // integrated loudness stored + loudnessSilent // read fine; no block above the gate + loudnessUnreadable // ffmpeg could not decode the file + loudnessInconclusive // nothing stored; worth trying again + loudnessStoreFailed // the write itself failed +) + +// storeLoudness records one attempt. It never fails its caller: an unmeasured +// track simply plays without a gain adjustment. +// +// An inconclusive attempt leaves any existing row alone. The scan deletes a +// row when its file changes, so a row still here describes these bytes, and a +// value from an older method is a better gain than none until it is redone. +func storeLoudness( + ctx context.Context, q *dbq.Queries, logger *slog.Logger, + trackID pgtype.UUID, path string, r loudnessResult, +) loudnessOutcome { + if r.err != nil { + logger.Warn("loudness: analysis failed", "path", path, "err", r.err) + if r.inconclusive() { + return loudnessInconclusive + } + } + params := dbq.UpsertTrackLoudnessParams{ + TrackID: trackID, + Unreadable: r.err != nil, + AnalysisVersion: loudnessVersion, + } + outcome := loudnessUnreadable + if r.err == nil { + outcome = loudnessSilent + params.IntegratedLufs = r.integratedLUFS + params.TruePeakDbtp = r.truePeakDBTP + params.LoudnessRangeLu = r.rangeLU + if !r.hist.empty() { + start := r.hist.start + params.BlockHistStart = &start + params.BlockHist = r.hist.counts + } + if r.integratedLUFS != nil { + outcome = loudnessMeasured + if got, ok := r.hist.gatedLoudness(); ok && math.Abs(got-float64(*r.integratedLUFS)) > histogramCrossCheckLU { + // Stored regardless: ffmpeg's own figure is the track's + // loudness. But album loudness is computed from the + // histogram, and this says it would be wrong. + logger.Warn("loudness: block histogram disagrees with ffmpeg's integrated loudness", + "path", path, "ffmpeg_lufs", *r.integratedLUFS, "histogram_lufs", got) + } + } + } + if err := q.UpsertTrackLoudness(ctx, params); err != nil { + logger.Warn("loudness: storing measurement failed", "path", path, "err", err) + return loudnessStoreFailed + } + return outcome +} diff --git a/internal/library/loudness_backfill.go b/internal/library/loudness_backfill.go new file mode 100644 index 00000000..8dc84dd0 --- /dev/null +++ b/internal/library/loudness_backfill.go @@ -0,0 +1,200 @@ +package library + +import ( + "context" + "fmt" + "log/slog" + "sync" + "time" + + "github.com/jackc/pgx/v5/pgtype" + "github.com/jackc/pgx/v5/pgxpool" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +// Loudness backfill (M464 #4995). +// +// Every track is measured here, the new ones included: the scan only deletes a +// changed file's measurement (see scanFile), and this worker measures it again. +// Measuring inline in the scan was the first plan and was dropped, because the +// analysis decodes the whole file. Added to the scan, a large import would run +// several times longer and could pass StuckScanThreshold (1h), at which point +// the run is reaped and a second scan started beside it. The fingerprint +// backfill is a worker of its own for the same reason. +// +// Until a track is measured it plays with no gain adjustment, which is how +// every track played before normalization existed. + +// loudnessBackfillTick is how often the worker looks for work. Shorter than +// the fingerprint backfill's hour, because a new track plays unleveled until it +// is measured; once the library has caught up, a tick is one indexed query. +const loudnessBackfillTick = 10 * time.Minute + +// loudnessBackfillBatch is how many tracks one query hands the worker. +const loudnessBackfillBatch = 50 + +// loudnessBackfillConcurrency is the shipped value of the concurrency setting. +// Low for the reason fingerprinting's is: each analysis is a full decode, +// competing with playback transcoding and streaming. +const loudnessBackfillConcurrency = 2 + +// BackfillLoudnessResult tallies one pass. +type BackfillLoudnessResult struct { + Processed int + Measured int + Silent int // read fine, no block above the gate (settled) + Unreadable int // ffmpeg could not decode the file (settled) + Inconclusive int // nothing stored; tried again on a later pass +} + +func (r *BackfillLoudnessResult) add(o loudnessOutcome) { + r.Processed++ + switch o { + case loudnessMeasured: + r.Measured++ + case loudnessSilent: + r.Silent++ + case loudnessUnreadable: + r.Unreadable++ + default: + r.Inconclusive++ + } +} + +// LoudnessBackfillWorker measures every track that has no current measurement. +type LoudnessBackfillWorker struct { + pool *pgxpool.Pool + logger *slog.Logger + settings *LoudnessSettingsService + tick time.Duration + batch int32 + // analyze is a field so an integration test pins which tracks a pass + // touches, not what ffmpeg prints. + analyze func(ctx context.Context, path string, durationMs int32) loudnessResult +} + +// NewLoudnessBackfillWorker builds a worker with the production cadence. +// settings is shared with the admin API; nil runs on defaults. +func NewLoudnessBackfillWorker( + pool *pgxpool.Pool, logger *slog.Logger, settings *LoudnessSettingsService, +) *LoudnessBackfillWorker { + return &LoudnessBackfillWorker{ + pool: pool, + logger: logger, + settings: settings, + tick: loudnessBackfillTick, + batch: loudnessBackfillBatch, + analyze: computeLoudness, + } +} + +// Run blocks until ctx is cancelled: one pass at start, then one per tick. +func (w *LoudnessBackfillWorker) Run(ctx context.Context) { + w.runOnce(ctx) + t := time.NewTicker(w.tick) + defer t.Stop() + for { + select { + case <-ctx.Done(): + return + case <-t.C: + w.runOnce(ctx) + } + } +} + +// runOnce contains a pass so that nothing it does (an error, a panic) can stop +// the next tick from firing (rule 157). +func (w *LoudnessBackfillWorker) runOnce(ctx context.Context) { + defer func() { + if r := recover(); r != nil { + w.logger.Error("loudness backfill: pass panicked", "panic", r) + } + }() + res, err := w.pass(ctx) + if err != nil && ctx.Err() == nil { + w.logger.Warn("loudness backfill: pass failed", "err", err, "processed", res.Processed) + } + if res.Processed > 0 { + w.logger.Info("loudness backfill: pass complete", + "processed", res.Processed, "measured", res.Measured, "silent", res.Silent, + "unreadable", res.Unreadable, "inconclusive", res.Inconclusive) + } +} + +// pass walks every track needing a measurement once, keyset-paged on id. The +// cursor is what lets a pass end: an inconclusive attempt writes no row, so a +// file that keeps timing out would otherwise be listed again immediately. +// Settings are read before every batch, so switching analysis off ends the +// pass and a new concurrency applies to the next batch. +func (w *LoudnessBackfillWorker) pass(ctx context.Context) (BackfillLoudnessResult, error) { + q := dbq.New(w.pool) + var ( + res BackfillLoudnessResult + mu sync.Mutex + ) + // The all-zero uuid sorts before every real id. Valid must be true: a NULL + // cursor would make "id > NULL" match nothing and every pass a no-op. + after := pgtype.UUID{Valid: true} + for { + if err := ctx.Err(); err != nil { + return res, err + } + cfg := w.settings.Get() + if !cfg.Enabled { + return res, nil + } + rows, err := q.ListTracksNeedingLoudness(ctx, dbq.ListTracksNeedingLoudnessParams{ + CurrentVersion: loudnessVersion, + AfterID: after, + BatchLimit: w.batch, + }) + if err != nil { + return res, fmt.Errorf("list tracks needing loudness: %w", err) + } + if len(rows) == 0 { + return res, nil + } + + sem := make(chan struct{}, max(1, int(cfg.BackfillConcurrency))) + var wg sync.WaitGroup + for _, row := range rows { + if ctx.Err() != nil { + break + } + sem <- struct{}{} + wg.Add(1) + go func(row dbq.ListTracksNeedingLoudnessRow) { + defer wg.Done() + defer func() { <-sem }() + defer func() { + if r := recover(); r != nil { + w.logger.Error("loudness backfill: track panicked", "path", row.FilePath, "panic", r) + } + }() + outcome := storeLoudness(ctx, q, w.logger, row.ID, row.FilePath, + w.analyzeFile(ctx, row.FilePath, row.DurationMs)) + mu.Lock() + res.add(outcome) + mu.Unlock() + }(row) + } + wg.Wait() + after = rows[len(rows)-1].ID + } +} + +func (w *LoudnessBackfillWorker) analyzeFile(ctx context.Context, path string, durationMs int32) loudnessResult { + if w.analyze == nil { + return computeLoudness(ctx, path, durationMs) + } + return w.analyze(ctx, path, durationMs) +} + +// LoudnessCoverage reports how much of the library carries a current +// measurement, for the admin gauge. It lives beside the backfill so the +// version it counts against is the one the backfill writes. +func LoudnessCoverage(ctx context.Context, pool *pgxpool.Pool) (dbq.GetLoudnessCoverageRow, error) { + return dbq.New(pool).GetLoudnessCoverage(ctx, loudnessVersion) +} diff --git a/internal/library/loudness_backfill_test.go b/internal/library/loudness_backfill_test.go new file mode 100644 index 00000000..4a052bec --- /dev/null +++ b/internal/library/loudness_backfill_test.go @@ -0,0 +1,230 @@ +package library + +import ( + "context" + "errors" + "fmt" + "io" + "log/slog" + "path/filepath" + "sync" + "testing" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +// TestLoudnessBackfill_Integration pins which tracks a pass measures, what it +// stores for each kind of result, that a pass ends, and that the gauge counts +// what the passes wrote. +func TestLoudnessBackfill_Integration(t *testing.T) { + pool := newPool(t) + ctx := context.Background() + q := dbq.New(pool) + dir := t.TempDir() + + _, album, artist := seedTrack(t, pool, filepath.Join(dir, "unmeasured.mp3")) + addTrack := func(name string) dbq.Track { + t.Helper() + tr, err := q.UpsertTrack(ctx, dbq.UpsertTrackParams{ + Title: name, AlbumID: album.ID, ArtistID: artist.ID, + DurationMs: 180000, FilePath: filepath.Join(dir, name+".mp3"), FileSize: 100, FileFormat: "mp3", + }) + if err != nil { + t.Fatalf("track %s: %v", name, err) + } + return tr + } + lufs := func(v float32) *float32 { return &v } + current := addTrack("current") + stale := addTrack("stale") + missing := addTrack("missing") + for _, seed := range []struct { + track dbq.Track + version int16 + }{ + {current, loudnessVersion}, + {stale, loudnessVersion - 1}, + } { + if err := q.UpsertTrackLoudness(ctx, dbq.UpsertTrackLoudnessParams{ + TrackID: seed.track.ID, IntegratedLufs: lufs(-9), AnalysisVersion: seed.version, + }); err != nil { + t.Fatalf("seed loudness: %v", err) + } + } + if _, err := pool.Exec(ctx, "UPDATE tracks SET missing_since = now() WHERE id = $1", missing.ID); err != nil { + t.Fatalf("mark missing: %v", err) + } + + settings, err := NewLoudnessSettingsService(ctx, pool) + if err != nil { + t.Fatalf("loudness settings: %v", err) + } + w := NewLoudnessBackfillWorker(pool, slog.New(slog.NewTextHandler(io.Discard, nil)), settings) + // A batch of one forces the keyset cursor across several queries in a pass. + w.batch = 1 + var mu sync.Mutex + calls := map[string]int{} + durations := map[string]int32{} + start := int16(500) + w.analyze = func(_ context.Context, path string, durationMs int32) loudnessResult { + name := filepath.Base(path) + mu.Lock() + calls[name]++ + durations[name] = durationMs + mu.Unlock() + switch name { + case "stall.mp3": + return loudnessResult{err: fmt.Errorf("ffmpeg: %w", errLoudnessTimeout)} + case "corrupt.mp3": + return loudnessResult{err: errors.New("ffmpeg exited 1: invalid data")} + case "silent.mp3": + return loudnessResult{} + default: + return loudnessResult{ + integratedLUFS: lufs(-12.5), truePeakDBTP: lufs(-0.4), rangeLU: lufs(6), + hist: blockHistogram{start: start, counts: []int32{3, 0, 7}}, + } + } + } + callCount := func(name string) int { + mu.Lock() + defer mu.Unlock() + return calls[name] + } + + // 1. Only the unmeasured track and the stale one are measured, never the + // current one or the missing one, and the track's length reaches the + // analyzer (it sets the deadline). + res, err := w.pass(ctx) + if err != nil { + t.Fatalf("first pass: %v", err) + } + if res.Processed != 2 || res.Measured != 2 { + t.Fatalf("first pass = %+v, want 2 processed, 2 measured", res) + } + for name, want := range map[string]int{ + "unmeasured.mp3": 1, "stale.mp3": 1, "current.mp3": 0, "missing.mp3": 0, + } { + if got := callCount(name); got != want { + t.Errorf("%s measured %d times, want %d", name, got, want) + } + } + if durations["stale.mp3"] != 180000 { + t.Errorf("analyzer got duration %d for stale.mp3, want 180000", durations["stale.mp3"]) + } + var ( + gotLUFS, gotPeak *float32 + gotStart *int16 + gotHist []int32 + gotVersion int16 + ) + if err := pool.QueryRow(ctx, `SELECT integrated_lufs, true_peak_dbtp, block_hist_start, block_hist, analysis_version + FROM track_loudness WHERE track_id = $1`, stale.ID). + Scan(&gotLUFS, &gotPeak, &gotStart, &gotHist, &gotVersion); err != nil { + t.Fatalf("read stale row back: %v", err) + } + if gotLUFS == nil || *gotLUFS != -12.5 || gotPeak == nil || *gotPeak != -0.4 || + gotStart == nil || *gotStart != start || len(gotHist) != 3 || gotHist[2] != 7 || + gotVersion != loudnessVersion { + t.Errorf("stale row after re-measuring = lufs %v peak %v hist %v@%v version %d", + gotLUFS, gotPeak, gotHist, gotStart, gotVersion) + } + + // 2. A pass after a complete one is a no-op. + res, err = w.pass(ctx) + if err != nil { + t.Fatalf("second pass: %v", err) + } + if res.Processed != 0 { + t.Fatalf("second pass processed %d tracks, want 0", res.Processed) + } + + // 3. A stall is tried once and the pass ends; silence and a corrupt file are + // verdicts, stored and not tried again. + addTrack("stall") + addTrack("corrupt") + silent := addTrack("silent") + res, err = w.pass(ctx) + if err != nil { + t.Fatalf("third pass: %v", err) + } + if res.Processed != 3 || res.Inconclusive != 1 || res.Unreadable != 1 || res.Silent != 1 { + t.Fatalf("third pass = %+v, want 3 processed: 1 inconclusive, 1 unreadable, 1 silent", res) + } + if got := callCount("stall.mp3"); got != 1 { + t.Fatalf("stalling file tried %d times in one pass, want exactly 1", got) + } + var silentHist []int32 + var silentUnreadable bool + if err := pool.QueryRow(ctx, "SELECT block_hist, unreadable FROM track_loudness WHERE track_id = $1", + silent.ID).Scan(&silentHist, &silentUnreadable); err != nil { + t.Fatalf("read silent row: %v", err) + } + if silentHist != nil || silentUnreadable { + t.Errorf("silent row = hist %v unreadable %v, want no histogram and readable", silentHist, silentUnreadable) + } + res, err = w.pass(ctx) + if err != nil { + t.Fatalf("fourth pass: %v", err) + } + if res.Processed != 1 || callCount("stall.mp3") != 2 || callCount("corrupt.mp3") != 1 { + t.Fatalf("fourth pass = %+v; want only the stalled file retried", res) + } + + // 4. The gauge counts what the passes wrote, and its buckets add up. Six + // present tracks: unmeasured, current, stale, stall, corrupt, silent. + cov, err := LoudnessCoverage(ctx, pool) + if err != nil { + t.Fatalf("coverage: %v", err) + } + if cov.Total != 6 || cov.Measured != 3 || cov.Silent != 1 || cov.Unreadable != 1 || cov.Pending != 1 { + t.Errorf("coverage = %+v, want total 6, measured 3, silent 1, unreadable 1, pending 1", cov) + } + if cov.Measured+cov.Silent+cov.Unreadable+cov.Pending != cov.Total { + t.Errorf("coverage buckets %+v do not sum to the total", cov) + } + + // 5. A changed file loses its measurement, so it is measured again. + if err := q.DeleteTrackLoudness(ctx, current.ID); err != nil { + t.Fatalf("delete loudness: %v", err) + } + // 6. Switched off, the backfill does nothing, even with work waiting. + off := DefaultLoudnessSettings + off.Enabled = false + if _, err := settings.Set(ctx, off); err != nil { + t.Fatalf("switch analysis off: %v", err) + } + res, err = w.pass(ctx) + if err != nil { + t.Fatalf("pass with analysis off: %v", err) + } + if res.Processed != 0 || callCount("current.mp3") != 0 { + t.Fatalf("pass with analysis off = %+v, want nothing done", res) + } + if _, err := settings.Set(ctx, DefaultLoudnessSettings); err != nil { + t.Fatalf("switch analysis on: %v", err) + } + res, err = w.pass(ctx) + if err != nil { + t.Fatalf("pass after switching back on: %v", err) + } + if callCount("current.mp3") != 1 { + t.Fatalf("changed file measured %d times after switching back on (pass %+v), want 1", + callCount("current.mp3"), res) + } +} + +// The Go defaults must match the migration's, or a database that cannot be +// read would analyze differently from a fresh install. +func TestLoudnessSettings_DefaultsMatchMigration(t *testing.T) { + pool := newPool(t) + s, err := NewLoudnessSettingsService(context.Background(), pool) + if err != nil { + t.Fatalf("load: %v", err) + } + got := s.Get() + got.UpdatedAt = DefaultLoudnessSettings.UpdatedAt + if got != DefaultLoudnessSettings { + t.Errorf("migration defaults = %+v, Go defaults = %+v", got, DefaultLoudnessSettings) + } +} diff --git a/internal/library/loudness_settings.go b/internal/library/loudness_settings.go new file mode 100644 index 00000000..31beca20 --- /dev/null +++ b/internal/library/loudness_settings.go @@ -0,0 +1,106 @@ +package library + +import ( + "context" + "errors" + "fmt" + "sync" + "time" + + "github.com/jackc/pgx/v5/pgxpool" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +// Loudness analysis settings (M464 #4995). Rule 25: an operator setting is a +// database row, changed without a restart. One instance is shared by the +// backfill and the admin API, so a save reaches the worker at once. + +// LoudnessSettings mirrors the loudness_settings row. +type LoudnessSettings struct { + // Enabled off stops the background analysis. Measured tracks keep their + // values, so normalization keeps working for them. + Enabled bool + BackfillConcurrency int32 + // UpdatedAt is set by the database; ignored by Set. + UpdatedAt time.Time +} + +// DefaultLoudnessSettings mirrors migration 0065's column defaults, so a +// database that cannot be read still analyzes the way a fresh install does. +var DefaultLoudnessSettings = LoudnessSettings{ + Enabled: true, + BackfillConcurrency: loudnessBackfillConcurrency, +} + +// ErrLoudnessSettingOutOfRange is returned by Set for a value migration 0065's +// CHECK would reject, so the API answers 400 naming the field. +var ErrLoudnessSettingOutOfRange = errors.New("loudness setting out of range") + +// LoudnessSettingsService caches the settings and owns their persistence. +type LoudnessSettingsService struct { + pool *pgxpool.Pool + + mu sync.RWMutex + cur LoudnessSettings +} + +// NewLoudnessSettingsService loads once and caches. It always returns a usable +// service, holding the defaults when the load fails; the error says so. +func NewLoudnessSettingsService(ctx context.Context, pool *pgxpool.Pool) (*LoudnessSettingsService, error) { + s := &LoudnessSettingsService{pool: pool, cur: DefaultLoudnessSettings} + row, err := dbq.New(pool).GetLoudnessSettings(ctx) + if err != nil { + return s, fmt.Errorf("loudness settings: load: %w", err) + } + s.cur = loudnessSettingsFromRow(row) + return s, nil +} + +// Get returns the cached settings. A nil service answers with the defaults. +func (s *LoudnessSettingsService) Get() LoudnessSettings { + if s == nil { + return DefaultLoudnessSettings + } + s.mu.RLock() + defer s.mu.RUnlock() + return s.cur +} + +// Set validates, persists and re-caches. +func (s *LoudnessSettingsService) Set(ctx context.Context, in LoudnessSettings) (LoudnessSettings, error) { + if err := validateLoudnessSettings(in); err != nil { + return LoudnessSettings{}, err + } + if s == nil { + return LoudnessSettings{}, errors.New("loudness settings: no settings service") + } + row, err := dbq.New(s.pool).UpdateLoudnessSettings(ctx, dbq.UpdateLoudnessSettingsParams{ + Enabled: in.Enabled, + BackfillConcurrency: in.BackfillConcurrency, + }) + if err != nil { + return LoudnessSettings{}, fmt.Errorf("loudness settings: save: %w", err) + } + out := loudnessSettingsFromRow(row) + s.mu.Lock() + s.cur = out + s.mu.Unlock() + return out, nil +} + +func validateLoudnessSettings(in LoudnessSettings) error { + if in.BackfillConcurrency < minBackfillConcurrency || in.BackfillConcurrency > maxBackfillConcurrency { + return fmt.Errorf("%w: backfill_concurrency must be %d-%d", + ErrLoudnessSettingOutOfRange, minBackfillConcurrency, maxBackfillConcurrency) + } + return nil +} + +func loudnessSettingsFromRow(row dbq.LoudnessSetting) LoudnessSettings { + return LoudnessSettings{ + Enabled: row.Enabled, + BackfillConcurrency: row.BackfillConcurrency, + UpdatedAt: row.UpdatedAt.Time, + } +} diff --git a/internal/library/loudness_test.go b/internal/library/loudness_test.go new file mode 100644 index 00000000..d2f40704 --- /dev/null +++ b/internal/library/loudness_test.go @@ -0,0 +1,250 @@ +package library + +import ( + "context" + "errors" + "fmt" + "math" + "os" + "os/exec" + "slices" + "strings" + "testing" + "time" +) + +// parseEbur128 runs the parser over a whole log, as computeLoudness does over +// ffmpeg's stderr. +func parseEbur128(log string) *ebur128Parser { + p := newEbur128Parser() + p.consume(strings.NewReader(log)) + return p +} + +// The fixture is real ffmpeg 6.1 output for 12 s of stereo tone whose first +// second is digital silence (testdata/ebur128_fixture.txt). Pinning the parser +// to captured output, not to a hand-written imitation of it, is the point: the +// per-block lines are where a format drift would hide. +func TestEbur128Parser_RealOutput(t *testing.T) { + raw, err := os.ReadFile("testdata/ebur128_fixture.txt") + if err != nil { + t.Fatalf("read fixture: %v", err) + } + r := parseEbur128(string(raw)).result() + if r.err != nil { + t.Fatalf("result err = %v", r.err) + } + for name, c := range map[string]struct { + got *float32 + want float32 + }{ + "integrated": {r.integratedLUFS, -10.7}, + "true peak": {r.truePeakDBTP, -5.6}, + "range": {r.rangeLU, 2.0}, + } { + if c.got == nil || *c.got != c.want { + t.Errorf("%s = %v, want %v", name, c.got, c.want) + } + } + + // 120 blocks, of which the 10 covering the silent second are below the + // absolute gate and not counted. The loudest is -8.1 LUFS, the quietest + // that passed -22.6. + var blocks int32 + for _, c := range r.hist.counts { + blocks += c + } + if blocks != 110 { + t.Errorf("histogram holds %d blocks, want 110", blocks) + } + if r.hist.start != 474 || len(r.hist.counts) != 146 { + t.Errorf("histogram spans bins %d..%d, want 474..619 (-22.6..-8.1 LUFS)", + r.hist.start, int(r.hist.start)+len(r.hist.counts)-1) + } + if r.hist.counts[0] == 0 || r.hist.counts[len(r.hist.counts)-1] == 0 { + t.Errorf("histogram not trimmed to its occupied bins: %v", r.hist.counts) + } + + // Album loudness is computed from these histograms (#4996), so the + // histogram has to reproduce ffmpeg's own figure. + got, ok := r.hist.gatedLoudness() + if !ok || math.Abs(got-(-10.7)) > 0.05 { + t.Errorf("loudness from the histogram = %.3f (ok=%v), want ffmpeg's -10.7 within 0.05", got, ok) + } +} + +func TestEbur128Parser_SilenceIsAVerdictNotAMeasurement(t *testing.T) { + log := `[Parsed_ebur128_0 @ 0x1] t: 0.1 TARGET:-23 LUFS M:-120.7 S:-120.7 I: -70.0 LUFS LRA: 0.0 LU FTPK: -inf -inf dBFS TPK: -inf -inf dBFS +[Parsed_ebur128_0 @ 0x1] t: 0.2 TARGET:-23 LUFS M:-120.7 S:-120.7 I: -70.0 LUFS LRA: 0.0 LU FTPK: -inf -inf dBFS TPK: -inf -inf dBFS +[Parsed_ebur128_0 @ 0x1] Summary: + + Integrated loudness: + I: -70.0 LUFS + Threshold: 0.0 LUFS + + Loudness range: + LRA: 0.0 LU + Threshold: 0.0 LUFS + LRA low: 0.0 LUFS + LRA high: 0.0 LUFS + + True peak: + Peak: -inf dBFS +` + r := parseEbur128(log).result() + if r.err != nil { + t.Fatalf("err = %v, want a clean result", r.err) + } + if r.integratedLUFS != nil || r.truePeakDBTP != nil || r.rangeLU != nil { + t.Errorf("silence measured as integrated=%v peak=%v range=%v, want all nil", + r.integratedLUFS, r.truePeakDBTP, r.rangeLU) + } + if !r.hist.empty() { + t.Errorf("silence left blocks in the histogram: %+v", r.hist) + } + if r.inconclusive() { + t.Errorf("silence reported as inconclusive; it is settled until the file changes") + } +} + +func TestEbur128Parser_NoSummaryIsUnreadable(t *testing.T) { + r := parseEbur128("[in#0 @ 0x1] Error opening input: Invalid data found when processing input\n").result() + if r.err == nil { + t.Fatal("a log with no summary produced a measurement") + } + if r.inconclusive() { + t.Errorf("err %v reads as inconclusive; ffmpeg ran and found nothing to measure", r.err) + } + if !strings.Contains(r.err.Error(), "Invalid data found") { + t.Errorf("err %q does not carry ffmpeg's reason", r.err) + } +} + +func TestEbur128Parser_LoudBlocksLandInTheTopBin(t *testing.T) { + p := parseEbur128(`[Parsed_ebur128_0 @ 0x1] t: 0.4 TARGET:-23 LUFS M: 12.4 S: 12.4 I: 12.4 LUFS +[Parsed_ebur128_0 @ 0x1] t: 0.5 TARGET:-23 LUFS M: -5.0 S: -5.0 I: -5.0 LUFS +`) + h := p.histogram() + if int(h.start)+len(h.counts) != loudnessHistBins || h.counts[len(h.counts)-1] != 1 { + t.Errorf("a +12.4 LUFS block was not counted in the top bin: %+v", h) + } + if h.start != 650 || h.counts[0] != 1 { + t.Errorf("a -5.0 LUFS block is not in bin 650: start %d, first %d", h.start, h.counts[0]) + } +} + +func TestBlockHistogram_GatedLoudness(t *testing.T) { + bin := func(lufs float64) int { return int(math.Round((lufs - loudnessHistFloor) * loudnessHistPerLU)) } + hist := func(blocks map[float64]int32) blockHistogram { + lo, hi := loudnessHistBins, 0 + for l := range blocks { + lo, hi = min(lo, bin(l)), max(hi, bin(l)) + } + h := blockHistogram{start: int16(lo), counts: make([]int32, hi-lo+1)} + for l, n := range blocks { + h.counts[bin(l)-lo] = n + } + return h + } + + cases := []struct { + name string + blocks map[float64]int32 + want float64 + }{ + // One level throughout is that level. + {"steady", map[float64]int32{-14: 50}, -14}, + // The quiet half is 30 LU below the loud half, past the relative gate + // (10 LU below the ungated loudness, here about -13), so it is dropped + // and the result is the loud half alone. + {"quiet passage gated out", map[float64]int32{-10: 100, -40: 100}, -10}, + // 6 LU apart is inside the gate, so both count, energy-weighted: the + // result sits nearer the louder level than the midpoint (-15) does. + {"both inside the gate", map[float64]int32{-12: 100, -18: 100}, -14.037}, + } + for _, c := range cases { + got, ok := hist(c.blocks).gatedLoudness() + if !ok || math.Abs(got-c.want) > 0.01 { + t.Errorf("%s: gatedLoudness = %.3f (ok=%v), want %.3f", c.name, got, ok, c.want) + } + } + if _, ok := (blockHistogram{}).gatedLoudness(); ok { + t.Errorf("an empty histogram reported a loudness") + } +} + +func TestEbur128Args(t *testing.T) { + args := ebur128Args("/music/a.flac") + joined := strings.Join(args, " ") + for _, want := range []string{"peak=true", "dualmono=true", "framelog=info", "-map 0:a:0", "-nostdin", "-f null -"} { + if !strings.Contains(joined, want) { + t.Errorf("args %q lack %q", joined, want) + } + } + // The path is its own argument, never spliced into the filter string. + if i := slices.Index(args, "-i"); i < 0 || args[i+1] != "/music/a.flac" { + t.Errorf("args %q do not pass the path after -i", args) + } +} + +func TestLoudnessTimeout_ScalesWithLength(t *testing.T) { + if got := loudnessTimeout(0); got != loudnessBaseTimeout { + t.Errorf("unknown length: %s, want the base %s", got, loudnessBaseTimeout) + } + if got, want := loudnessTimeout(int32((2 * time.Hour).Milliseconds())), loudnessBaseTimeout+30*time.Minute; got != want { + t.Errorf("two-hour mix: %s, want %s", got, want) + } + if got := loudnessTimeout(-5); got != loudnessBaseTimeout { + t.Errorf("negative length: %s, want the base %s", got, loudnessBaseTimeout) + } +} + +func TestLoudnessResult_Inconclusive(t *testing.T) { + for _, err := range []error{ + fmt.Errorf("ffmpeg: %w", errLoudnessTimeout), + fmt.Errorf("ffmpeg: %w", context.Canceled), + fmt.Errorf("ffmpeg: %w", exec.ErrNotFound), + } { + if !(loudnessResult{err: err}).inconclusive() { + t.Errorf("%v: not inconclusive, so it would be stored as a verdict", err) + } + } + if (loudnessResult{err: errors.New("ffmpeg exited 1: moov atom not found")}).inconclusive() { + t.Errorf("a decode failure read as inconclusive; it would be retried every pass") + } + if (loudnessResult{}).inconclusive() { + t.Errorf("a clean result read as inconclusive") + } +} + +func TestBackfillLoudnessResult_Add(t *testing.T) { + var r BackfillLoudnessResult + for _, o := range []loudnessOutcome{ + loudnessMeasured, loudnessMeasured, loudnessSilent, loudnessUnreadable, + loudnessInconclusive, loudnessStoreFailed, + } { + r.add(o) + } + // A failed write stored nothing, so it is retried like an inconclusive one. + want := BackfillLoudnessResult{Processed: 6, Measured: 2, Silent: 1, Unreadable: 1, Inconclusive: 2} + if r != want { + t.Errorf("tally = %+v, want %+v", r, want) + } +} + +func TestValidateLoudnessSettings(t *testing.T) { + for _, n := range []int32{minBackfillConcurrency, maxBackfillConcurrency} { + if err := validateLoudnessSettings(LoudnessSettings{BackfillConcurrency: n}); err != nil { + t.Errorf("concurrency %d rejected: %v", n, err) + } + } + for _, n := range []int32{0, maxBackfillConcurrency + 1} { + if err := validateLoudnessSettings(LoudnessSettings{BackfillConcurrency: n}); !errors.Is(err, ErrLoudnessSettingOutOfRange) { + t.Errorf("concurrency %d: err = %v, want ErrLoudnessSettingOutOfRange", n, err) + } + } + var nilSvc *LoudnessSettingsService + if got := nilSvc.Get(); got != DefaultLoudnessSettings { + t.Errorf("nil service Get = %+v, want the defaults", got) + } +} diff --git a/internal/library/scanner.go b/internal/library/scanner.go index f2c00c32..c7d45dcd 100644 --- a/internal/library/scanner.go +++ b/internal/library/scanner.go @@ -402,6 +402,13 @@ func (s *Scanner) scanFile( // must not outlive them, or the sweep would compare audio that is gone. s.logger.Warn("fingerprint: clearing stale fingerprint failed", "path", path, "err", err) } + // Loudness is measured by its own worker, never inline: it decodes the + // whole file (see loudness_backfill.go). The scan's part is to drop a + // measurement of bytes that are gone, so clients stop leveling this + // track by the old file's loudness and the worker measures it again. + if err := q.DeleteTrackLoudness(ctx, track.ID); err != nil { + s.logger.Warn("loudness: clearing stale measurement failed", "path", path, "err", err) + } } if knownTrack { diff --git a/internal/library/testdata/ebur128_fixture.txt b/internal/library/testdata/ebur128_fixture.txt new file mode 100644 index 00000000..cf6eeb3f --- /dev/null +++ b/internal/library/testdata/ebur128_fixture.txt @@ -0,0 +1,149 @@ +Input #0, flac, from 'fixture.flac': + Metadata: + encoder : Lavf60.16.100 + Duration: 00:00:12.00, start: 0.000000, bitrate: 789 kb/s + Stream #0:0: Audio: flac, 44100 Hz, stereo, s32 (24 bit) +Stream mapping: + Stream #0:0 -> #0:0 (flac (native) -> pcm_s16le (native)) +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 0.0999773 TARGET:-23 LUFS M:-120.7 S:-120.7 I: -70.0 LUFS LRA: 0.0 LU FTPK: -inf -inf dBFS TPK: -inf -inf dBFS +Output #0, null, to 'pipe:': + Metadata: + encoder : Lavf60.16.100 + Stream #0:0: Audio: pcm_s16le, 44100 Hz, stereo, s16, 1411 kb/s + Metadata: + encoder : Lavc60.31.102 pcm_s16le +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 0.199977 TARGET:-23 LUFS M:-120.7 S:-120.7 I: -70.0 LUFS LRA: 0.0 LU FTPK: -inf -inf dBFS TPK: -inf -inf dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 0.299977 TARGET:-23 LUFS M:-120.7 S:-120.7 I: -70.0 LUFS LRA: 0.0 LU FTPK: -inf -inf dBFS TPK: -inf -inf dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 0.399977 TARGET:-23 LUFS M:-163.2 S:-120.7 I: -70.0 LUFS LRA: 0.0 LU FTPK: -inf -inf dBFS TPK: -inf -inf dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 0.499977 TARGET:-23 LUFS M:-163.2 S:-120.7 I: -70.0 LUFS LRA: 0.0 LU FTPK: -inf -inf dBFS TPK: -inf -inf dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 0.599977 TARGET:-23 LUFS M:-163.2 S:-120.7 I: -70.0 LUFS LRA: 0.0 LU FTPK: -inf -inf dBFS TPK: -inf -inf dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 0.699977 TARGET:-23 LUFS M:-163.2 S:-120.7 I: -70.0 LUFS LRA: 0.0 LU FTPK: -inf -inf dBFS TPK: -inf -inf dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 0.799977 TARGET:-23 LUFS M:-163.2 S:-120.7 I: -70.0 LUFS LRA: 0.0 LU FTPK: -inf -inf dBFS TPK: -inf -inf dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 0.899977 TARGET:-23 LUFS M:-163.2 S:-120.7 I: -70.0 LUFS LRA: 0.0 LU FTPK: -inf -inf dBFS TPK: -inf -inf dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 0.999977 TARGET:-23 LUFS M:-163.2 S:-120.7 I: -70.0 LUFS LRA: 0.0 LU FTPK: -inf -inf dBFS TPK: -inf -inf dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 1.09998 TARGET:-23 LUFS M: -15.4 S:-120.7 I: -15.4 LUFS LRA: 0.0 LU FTPK: -7.1 -10.7 dBFS TPK: -7.1 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 1.19998 TARGET:-23 LUFS M: -12.2 S:-120.7 I: -13.5 LUFS LRA: 0.0 LU FTPK: -6.6 -10.9 dBFS TPK: -6.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 1.29998 TARGET:-23 LUFS M: -10.3 S:-120.7 I: -12.2 LUFS LRA: 0.0 LU FTPK: -6.3 -11.1 dBFS TPK: -6.3 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 1.39998 TARGET:-23 LUFS M: -9.0 S:-120.7 I: -11.1 LUFS LRA: 0.0 LU FTPK: -6.0 -11.4 dBFS TPK: -6.0 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 1.49998 TARGET:-23 LUFS M: -8.8 S:-120.7 I: -10.6 LUFS LRA: 0.0 LU FTPK: -5.8 -11.7 dBFS TPK: -5.8 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 1.59998 TARGET:-23 LUFS M: -8.7 S:-120.7 I: -10.2 LUFS LRA: 0.0 LU FTPK: -5.7 -12.1 dBFS TPK: -5.7 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 1.69998 TARGET:-23 LUFS M: -8.6 S:-120.7 I: -9.9 LUFS LRA: 0.0 LU FTPK: -5.6 -12.4 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 1.79998 TARGET:-23 LUFS M: -8.5 S:-120.7 I: -9.7 LUFS LRA: 0.0 LU FTPK: -5.6 -12.8 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 1.89998 TARGET:-23 LUFS M: -8.6 S:-120.7 I: -9.6 LUFS LRA: 0.0 LU FTPK: -5.6 -13.3 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 1.99998 TARGET:-23 LUFS M: -8.6 S:-120.7 I: -9.5 LUFS LRA: 0.0 LU FTPK: -5.7 -13.8 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 2.09998 TARGET:-23 LUFS M: -8.8 S:-120.7 I: -9.4 LUFS LRA: 0.0 LU FTPK: -5.8 -14.3 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 2.19998 TARGET:-23 LUFS M: -8.9 S:-120.7 I: -9.4 LUFS LRA: 0.0 LU FTPK: -6.0 -14.9 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 2.29998 TARGET:-23 LUFS M: -9.2 S:-120.7 I: -9.3 LUFS LRA: 0.0 LU FTPK: -6.3 -15.6 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 2.39998 TARGET:-23 LUFS M: -9.5 S:-120.7 I: -9.4 LUFS LRA: 0.0 LU FTPK: -6.7 -16.3 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 2.49998 TARGET:-23 LUFS M: -9.9 S:-120.7 I: -9.4 LUFS LRA: 0.0 LU FTPK: -7.1 -17.2 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 2.59998 TARGET:-23 LUFS M: -10.4 S:-120.7 I: -9.5 LUFS LRA: 0.0 LU FTPK: -7.7 -18.1 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 2.69998 TARGET:-23 LUFS M: -11.0 S:-120.7 I: -9.5 LUFS LRA: 0.0 LU FTPK: -8.3 -19.2 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 2.79998 TARGET:-23 LUFS M: -11.6 S:-120.7 I: -9.6 LUFS LRA: 0.0 LU FTPK: -9.1 -20.4 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 2.89998 TARGET:-23 LUFS M: -12.4 S:-120.7 I: -9.7 LUFS LRA: 0.0 LU FTPK: -10.0 -21.8 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 2.99998 TARGET:-23 LUFS M: -13.4 S: -11.5 I: -9.9 LUFS LRA: 20.0 LU FTPK: -11.1 -23.6 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 3.09998 TARGET:-23 LUFS M: -14.5 S: -11.5 I: -10.0 LUFS LRA: 20.0 LU FTPK: -12.5 -25.8 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 3.19998 TARGET:-23 LUFS M: -15.8 S: -11.4 I: -10.1 LUFS LRA: 20.0 LU FTPK: -14.1 -28.2 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 3.29998 TARGET:-23 LUFS M: -17.3 S: -11.4 I: -10.3 LUFS LRA: 20.0 LU FTPK: -16.2 -25.4 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 3.39998 TARGET:-23 LUFS M: -19.1 S: -11.4 I: -10.5 LUFS LRA: 0.1 LU FTPK: -19.2 -23.3 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 3.49998 TARGET:-23 LUFS M: -21.1 S: -11.4 I: -10.5 LUFS LRA: 0.1 LU FTPK: -23.7 -21.6 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 3.59998 TARGET:-23 LUFS M: -22.6 S: -11.4 I: -10.5 LUFS LRA: 0.1 LU FTPK: -22.6 -20.2 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 3.69998 TARGET:-23 LUFS M: -22.5 S: -11.4 I: -10.5 LUFS LRA: 0.1 LU FTPK: -18.5 -19.0 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 3.79998 TARGET:-23 LUFS M: -20.9 S: -11.3 I: -10.6 LUFS LRA: 0.2 LU FTPK: -15.8 -17.9 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 3.89998 TARGET:-23 LUFS M: -18.9 S: -11.3 I: -10.9 LUFS LRA: 0.2 LU FTPK: -13.8 -17.0 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 3.99998 TARGET:-23 LUFS M: -17.1 S: -11.2 I: -11.0 LUFS LRA: 0.2 LU FTPK: -12.2 -16.2 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 4.09998 TARGET:-23 LUFS M: -15.6 S: -11.4 I: -11.1 LUFS LRA: 0.2 LU FTPK: -10.9 -15.5 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 4.19998 TARGET:-23 LUFS M: -14.3 S: -11.5 I: -11.2 LUFS LRA: 0.2 LU FTPK: -9.8 -14.8 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 4.29998 TARGET:-23 LUFS M: -13.2 S: -11.7 I: -11.3 LUFS LRA: 0.4 LU FTPK: -8.9 -14.2 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 4.39998 TARGET:-23 LUFS M: -12.3 S: -11.8 I: -11.3 LUFS LRA: 0.4 LU FTPK: -8.2 -13.7 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 4.49998 TARGET:-23 LUFS M: -11.5 S: -11.9 I: -11.3 LUFS LRA: 0.5 LU FTPK: -7.6 -13.2 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 4.59998 TARGET:-23 LUFS M: -10.8 S: -12.0 I: -11.3 LUFS LRA: 0.6 LU FTPK: -7.0 -12.8 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 4.69998 TARGET:-23 LUFS M: -10.2 S: -12.0 I: -11.2 LUFS LRA: 0.7 LU FTPK: -6.6 -12.4 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 4.79998 TARGET:-23 LUFS M: -9.7 S: -12.1 I: -11.2 LUFS LRA: 0.7 LU FTPK: -6.3 -12.0 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 4.89998 TARGET:-23 LUFS M: -9.3 S: -12.1 I: -11.1 LUFS LRA: 0.8 LU FTPK: -6.0 -11.7 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 4.99998 TARGET:-23 LUFS M: -8.9 S: -12.1 I: -11.0 LUFS LRA: 0.8 LU FTPK: -5.8 -11.4 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 5.09998 TARGET:-23 LUFS M: -8.6 S: -12.0 I: -11.0 LUFS LRA: 0.8 LU FTPK: -5.7 -11.1 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 5.19998 TARGET:-23 LUFS M: -8.4 S: -11.9 I: -10.8 LUFS LRA: 0.8 LU FTPK: -5.6 -10.8 dBFS TPK: -5.6 -10.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 5.29998 TARGET:-23 LUFS M: -8.3 S: -11.8 I: -10.7 LUFS LRA: 0.8 LU FTPK: -5.6 -10.6 dBFS TPK: -5.6 -10.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 5.39998 TARGET:-23 LUFS M: -8.2 S: -11.7 I: -10.5 LUFS LRA: 0.8 LU FTPK: -5.6 -10.4 dBFS TPK: -5.6 -10.4 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 5.49998 TARGET:-23 LUFS M: -8.1 S: -11.5 I: -10.4 LUFS LRA: 0.8 LU FTPK: -5.7 -10.2 dBFS TPK: -5.6 -10.2 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 5.59998 TARGET:-23 LUFS M: -8.1 S: -11.4 I: -10.4 LUFS LRA: 0.8 LU FTPK: -5.8 -10.1 dBFS TPK: -5.6 -10.1 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 5.69998 TARGET:-23 LUFS M: -8.2 S: -11.2 I: -10.3 LUFS LRA: 0.8 LU FTPK: -6.0 -10.0 dBFS TPK: -5.6 -10.0 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 5.79998 TARGET:-23 LUFS M: -8.3 S: -11.1 I: -10.2 LUFS LRA: 0.8 LU FTPK: -6.3 -9.9 dBFS TPK: -5.6 -9.9 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 5.89998 TARGET:-23 LUFS M: -8.4 S: -10.9 I: -10.2 LUFS LRA: 1.0 LU FTPK: -6.7 -9.8 dBFS TPK: -5.6 -9.8 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 5.99998 TARGET:-23 LUFS M: -8.7 S: -10.7 I: -10.2 LUFS LRA: 1.0 LU FTPK: -7.2 -9.7 dBFS TPK: -5.6 -9.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 6.09998 TARGET:-23 LUFS M: -8.9 S: -10.6 I: -10.1 LUFS LRA: 1.2 LU FTPK: -7.7 -9.7 dBFS TPK: -5.6 -9.7 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 6.19998 TARGET:-23 LUFS M: -9.2 S: -10.4 I: -10.1 LUFS LRA: 1.3 LU FTPK: -8.4 -9.6 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 6.29998 TARGET:-23 LUFS M: -9.6 S: -10.3 I: -10.1 LUFS LRA: 1.5 LU FTPK: -9.2 -9.6 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 6.39998 TARGET:-23 LUFS M: -10.0 S: -10.2 I: -10.1 LUFS LRA: 1.6 LU FTPK: -10.1 -9.6 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 6.49998 TARGET:-23 LUFS M: -10.4 S: -10.0 I: -10.1 LUFS LRA: 1.8 LU FTPK: -11.2 -9.6 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 6.59998 TARGET:-23 LUFS M: -10.9 S: -10.0 I: -10.1 LUFS LRA: 1.9 LU FTPK: -12.6 -9.7 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 6.69998 TARGET:-23 LUFS M: -11.4 S: -9.9 I: -10.1 LUFS LRA: 2.0 LU FTPK: -14.3 -9.7 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 6.79998 TARGET:-23 LUFS M: -11.9 S: -9.8 I: -10.2 LUFS LRA: 2.1 LU FTPK: -16.5 -9.8 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 6.89998 TARGET:-23 LUFS M: -12.5 S: -9.8 I: -10.2 LUFS LRA: 2.1 LU FTPK: -19.5 -9.9 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 6.99998 TARGET:-23 LUFS M: -12.9 S: -9.8 I: -10.2 LUFS LRA: 2.2 LU FTPK: -24.3 -10.0 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 7.09998 TARGET:-23 LUFS M: -13.3 S: -9.8 I: -10.3 LUFS LRA: 2.2 LU FTPK: -22.2 -10.1 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 7.19998 TARGET:-23 LUFS M: -13.5 S: -9.8 I: -10.3 LUFS LRA: 2.2 LU FTPK: -18.2 -10.3 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 7.29998 TARGET:-23 LUFS M: -13.5 S: -9.8 I: -10.3 LUFS LRA: 2.2 LU FTPK: -15.6 -10.5 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 7.39998 TARGET:-23 LUFS M: -13.3 S: -9.9 I: -10.4 LUFS LRA: 2.2 LU FTPK: -13.6 -10.7 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 7.49998 TARGET:-23 LUFS M: -13.0 S: -9.9 I: -10.4 LUFS LRA: 2.2 LU FTPK: -12.0 -10.9 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 7.59998 TARGET:-23 LUFS M: -12.6 S: -10.0 I: -10.4 LUFS LRA: 2.2 LU FTPK: -10.8 -11.2 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 7.69998 TARGET:-23 LUFS M: -12.1 S: -10.0 I: -10.5 LUFS LRA: 2.2 LU FTPK: -9.7 -11.5 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 7.79998 TARGET:-23 LUFS M: -11.7 S: -10.1 I: -10.5 LUFS LRA: 2.2 LU FTPK: -8.9 -11.8 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 7.89998 TARGET:-23 LUFS M: -11.2 S: -10.1 I: -10.5 LUFS LRA: 2.2 LU FTPK: -8.1 -12.1 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 7.99998 TARGET:-23 LUFS M: -10.8 S: -10.2 I: -10.5 LUFS LRA: 2.2 LU FTPK: -7.5 -12.5 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 8.09998 TARGET:-23 LUFS M: -10.4 S: -10.3 I: -10.5 LUFS LRA: 2.2 LU FTPK: -7.0 -12.9 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 8.19998 TARGET:-23 LUFS M: -10.1 S: -10.4 I: -10.5 LUFS LRA: 2.2 LU FTPK: -6.6 -13.4 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 8.29998 TARGET:-23 LUFS M: -9.8 S: -10.4 I: -10.5 LUFS LRA: 2.2 LU FTPK: -6.2 -13.9 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 8.39998 TARGET:-23 LUFS M: -9.5 S: -10.5 I: -10.5 LUFS LRA: 2.2 LU FTPK: -6.0 -14.4 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 8.49998 TARGET:-23 LUFS M: -9.3 S: -10.5 I: -10.4 LUFS LRA: 2.2 LU FTPK: -5.8 -15.0 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 8.59998 TARGET:-23 LUFS M: -9.2 S: -10.6 I: -10.4 LUFS LRA: 2.2 LU FTPK: -5.7 -15.7 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 8.69998 TARGET:-23 LUFS M: -9.1 S: -10.6 I: -10.4 LUFS LRA: 2.2 LU FTPK: -5.6 -16.5 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 8.79998 TARGET:-23 LUFS M: -9.0 S: -10.6 I: -10.4 LUFS LRA: 2.2 LU FTPK: -5.6 -17.3 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 8.89998 TARGET:-23 LUFS M: -9.0 S: -10.6 I: -10.4 LUFS LRA: 2.2 LU FTPK: -5.6 -18.3 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 8.99998 TARGET:-23 LUFS M: -9.1 S: -10.7 I: -10.3 LUFS LRA: 2.2 LU FTPK: -5.7 -19.4 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 9.09998 TARGET:-23 LUFS M: -9.2 S: -10.7 I: -10.3 LUFS LRA: 2.2 LU FTPK: -5.9 -20.6 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 9.19998 TARGET:-23 LUFS M: -9.4 S: -10.7 I: -10.3 LUFS LRA: 2.2 LU FTPK: -6.1 -22.1 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 9.29998 TARGET:-23 LUFS M: -9.7 S: -10.7 I: -10.3 LUFS LRA: 2.2 LU FTPK: -6.4 -23.9 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 9.39998 TARGET:-23 LUFS M: -10.0 S: -10.7 I: -10.3 LUFS LRA: 2.1 LU FTPK: -6.7 -26.3 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 9.49998 TARGET:-23 LUFS M: -10.4 S: -10.7 I: -10.3 LUFS LRA: 2.1 LU FTPK: -7.2 -27.7 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 9.59998 TARGET:-23 LUFS M: -10.9 S: -10.7 I: -10.3 LUFS LRA: 2.1 LU FTPK: -7.8 -25.0 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 9.69998 TARGET:-23 LUFS M: -11.4 S: -10.7 I: -10.3 LUFS LRA: 2.1 LU FTPK: -8.4 -23.0 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 9.79998 TARGET:-23 LUFS M: -12.0 S: -10.7 I: -10.3 LUFS LRA: 2.1 LU FTPK: -9.2 -21.3 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 9.89998 TARGET:-23 LUFS M: -12.7 S: -10.7 I: -10.4 LUFS LRA: 2.1 LU FTPK: -10.2 -20.0 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 9.99998 TARGET:-23 LUFS M: -13.5 S: -10.7 I: -10.4 LUFS LRA: 2.1 LU FTPK: -11.3 -18.8 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 10.1 TARGET:-23 LUFS M: -14.4 S: -10.7 I: -10.4 LUFS LRA: 2.1 LU FTPK: -12.7 -17.8 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 10.2 TARGET:-23 LUFS M: -15.3 S: -10.8 I: -10.4 LUFS LRA: 2.1 LU FTPK: -14.5 -16.9 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 10.3 TARGET:-23 LUFS M: -16.2 S: -10.8 I: -10.5 LUFS LRA: 2.1 LU FTPK: -16.7 -16.1 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 10.4 TARGET:-23 LUFS M: -17.1 S: -10.9 I: -10.5 LUFS LRA: 2.1 LU FTPK: -19.8 -15.4 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 10.5 TARGET:-23 LUFS M: -17.8 S: -11.0 I: -10.6 LUFS LRA: 2.1 LU FTPK: -24.9 -14.7 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 10.6 TARGET:-23 LUFS M: -18.0 S: -11.1 I: -10.6 LUFS LRA: 2.1 LU FTPK: -21.7 -14.1 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 10.7 TARGET:-23 LUFS M: -17.6 S: -11.1 I: -10.6 LUFS LRA: 2.1 LU FTPK: -17.9 -13.6 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 10.8 TARGET:-23 LUFS M: -16.7 S: -11.2 I: -10.7 LUFS LRA: 2.1 LU FTPK: -15.4 -13.1 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 10.9 TARGET:-23 LUFS M: -15.7 S: -11.3 I: -10.7 LUFS LRA: 2.1 LU FTPK: -13.4 -12.7 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 11 TARGET:-23 LUFS M: -14.6 S: -11.4 I: -10.7 LUFS LRA: 2.1 LU FTPK: -11.9 -12.3 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 11.1 TARGET:-23 LUFS M: -13.6 S: -11.5 I: -10.8 LUFS LRA: 2.1 LU FTPK: -10.7 -11.9 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 11.2 TARGET:-23 LUFS M: -12.7 S: -11.6 I: -10.8 LUFS LRA: 2.1 LU FTPK: -9.6 -11.6 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 11.3 TARGET:-23 LUFS M: -11.8 S: -11.7 I: -10.8 LUFS LRA: 2.1 LU FTPK: -8.8 -11.3 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 11.4 TARGET:-23 LUFS M: -11.1 S: -11.7 I: -10.8 LUFS LRA: 2.0 LU FTPK: -8.1 -11.0 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 11.5 TARGET:-23 LUFS M: -10.5 S: -11.7 I: -10.8 LUFS LRA: 2.0 LU FTPK: -7.4 -10.8 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 11.6 TARGET:-23 LUFS M: -9.9 S: -11.8 I: -10.8 LUFS LRA: 2.0 LU FTPK: -6.9 -10.6 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 11.7 TARGET:-23 LUFS M: -9.5 S: -11.8 I: -10.8 LUFS LRA: 2.0 LU FTPK: -6.5 -10.4 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 11.8 TARGET:-23 LUFS M: -9.0 S: -11.7 I: -10.7 LUFS LRA: 2.0 LU FTPK: -6.2 -10.2 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 11.9 TARGET:-23 LUFS M: -8.7 S: -11.7 I: -10.7 LUFS LRA: 2.0 LU FTPK: -5.9 -10.1 dBFS TPK: -5.6 -9.6 dBFS +[Parsed_ebur128_0 @ 0x5ebc43357b00] t: 12 TARGET:-23 LUFS M: -8.4 S: -11.6 I: -10.7 LUFS LRA: 2.0 LU FTPK: -5.8 -10.0 dBFS TPK: -5.6 -9.6 dBFS +[out#0/null @ 0x5ebc43345c00] video:0kB audio:2067kB subtitle:0kB other streams:0kB global headers:0kB muxing overhead: unknown +size=N/A time=00:00:11.90 bitrate=N/A speed= 161x +[Parsed_ebur128_0 @ 0x5ebc43357b00] Summary: + + Integrated loudness: + I: -10.7 LUFS + Threshold: -20.8 LUFS + + Loudness range: + LRA: 2.0 LU + Threshold: -30.9 LUFS + LRA low: -11.8 LUFS + LRA high: -9.8 LUFS + + True peak: + Peak: -5.6 dBFS diff --git a/internal/server/server.go b/internal/server/server.go index efae1029..c833c508 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -105,6 +105,9 @@ type Server struct { // fingerprint workers, so a save from the admin card reaches them without a // restart. Router() constructs a fallback when nil (tests). FingerprintSettings *library.FingerprintSettingsService + // LoudnessSettings is the DB-backed loudness analysis policy (M464 #4995), + // shared with the loudness backfill. Nil makes the router load its own. + LoudnessSettings *library.LoudnessSettingsService // ReacqSettings is the DB-backed missing-file re-acquisition policy // (milestone #290) — the same instance the sweeper in cmd/minstrel/main.go // reads, so a save from the admin card reaches it without a restart @@ -214,7 +217,15 @@ func (s *Server) Router() http.Handler { s.Logger.Warn("fingerprint settings unavailable; serving defaults", "err", err) } } - api.Mount(r, s.Pool, s.Logger, writer, s.RecommendationCfg, recSettings, lidarrCfg, lidarrReqs, lidarrQuar, tracksSvc, playlistsSvc, s.CoverEnricher, s.CoverSettings, s.TagSettings, s.LibraryScanner, s.ScanCfg, s.DataDir, smtpSender, bus, s.PlaylistScheduler, s.StreamSecret, netSettings, reacqSettings, fpSettings) + loudSettings := s.LoudnessSettings + if loudSettings == nil { + var err error + loudSettings, err = library.NewLoudnessSettingsService(context.Background(), s.Pool) + if err != nil { + s.Logger.Warn("loudness settings unavailable; serving defaults", "err", err) + } + } + api.Mount(r, s.Pool, s.Logger, writer, s.RecommendationCfg, recSettings, lidarrCfg, lidarrReqs, lidarrQuar, tracksSvc, playlistsSvc, s.CoverEnricher, s.CoverSettings, s.TagSettings, s.LibraryScanner, s.ScanCfg, s.DataDir, smtpSender, bus, s.PlaylistScheduler, s.StreamSecret, netSettings, reacqSettings, fpSettings, loudSettings) // /api/admin/scan is the only admin route owned by the server package // (it needs the Scanner). Register it as a single inline-middleware // route — using r.Route("/api/admin", ...) here would create a second diff --git a/web/src/lib/api/admin.ts b/web/src/lib/api/admin.ts index b1b86597..fa925e28 100644 --- a/web/src/lib/api/admin.ts +++ b/web/src/lib/api/admin.ts @@ -364,6 +364,38 @@ export async function updateFingerprintSettings( return api.put('/api/admin/library/fingerprint-settings', s); } +// Loudness analysis (#4995) ------------------------------------------------- + +export type LoudnessCoverage = { + total: number; + measured: number; + // Read fine, but no part of the track was loud enough to measure: silence, + // or a file shorter than half a second. + silent: number; + unreadable: number; + pending: number; + // False when the operator has switched analysis off: pending then never + // shrinks, and nothing should read as progress. + enabled: boolean; +}; + +export async function getLoudnessCoverage(): Promise { + return api.get('/api/admin/library/loudness'); +} + +export type LoudnessSettings = { + enabled: boolean; + backfill_concurrency: number; +}; + +export async function getLoudnessSettings(): Promise { + return api.get('/api/admin/library/loudness-settings'); +} + +export async function updateLoudnessSettings(s: LoudnessSettings): Promise { + return api.put('/api/admin/library/loudness-settings', s); +} + // Cover-art providers ------------------------------------------------------ export type CoverProviderCapability = 'album_cover' | 'artist_thumb' | 'artist_fanart'; diff --git a/web/src/lib/components/LoudnessSettingsCard.svelte b/web/src/lib/components/LoudnessSettingsCard.svelte new file mode 100644 index 00000000..6003f1df --- /dev/null +++ b/web/src/lib/components/LoudnessSettingsCard.svelte @@ -0,0 +1,173 @@ + + +
+
+

Loudness analysis

+

+ Measures how loud each track is, so playback can even out the volume between tracks. It runs + in the background; until a track is measured, it plays at its own volume. +

+
+ + {#if coverage && coverage.total > 0} +
+ + {coverage.measured.toLocaleString()} of {coverage.total.toLocaleString()} tracks measured + + {#if coverage.pending > 0} + · + {coverage.pending.toLocaleString()} pending + {/if} + {#if !coverage.enabled && coverage.pending > 0} + · + paused while analysis is off + {/if} + {#if coverage.silent > 0} + · + + {coverage.silent.toLocaleString()} silent + + {/if} + {#if coverage.unreadable > 0} + · + + {coverage.unreadable.toLocaleString()} unreadable + + {/if} +
+ {/if} + + {#if loadError} +

+ Couldn't load loudness analysis settings. + +

+ {:else if form === null} +

Loading…

+ {:else} + + + + + {#if !concurrencyOk} +

+ Files analyzed at once must be a whole number from 1 to 8. +

+ {/if} + +
+ +
+ {/if} +
diff --git a/web/src/lib/components/LoudnessSettingsCard.test.ts b/web/src/lib/components/LoudnessSettingsCard.test.ts new file mode 100644 index 00000000..670676ad --- /dev/null +++ b/web/src/lib/components/LoudnessSettingsCard.test.ts @@ -0,0 +1,94 @@ +import { afterEach, describe, expect, test, vi } from 'vitest'; +import { render, screen, fireEvent, waitFor } from '@testing-library/svelte'; +import type { LoudnessCoverage, LoudnessSettings } from '$lib/api/admin'; + +vi.mock('$lib/api/admin', () => ({ + getLoudnessSettings: vi.fn(), + updateLoudnessSettings: vi.fn(), + getLoudnessCoverage: vi.fn() +})); + +vi.mock('$lib/stores/toast.svelte', () => ({ pushToast: vi.fn() })); + +import LoudnessSettingsCard from './LoudnessSettingsCard.svelte'; +import { getLoudnessCoverage, getLoudnessSettings, updateLoudnessSettings } from '$lib/api/admin'; +import { pushToast } from '$lib/stores/toast.svelte'; + +const base: LoudnessSettings = { enabled: true, backfill_concurrency: 2 }; +const coverage: LoudnessCoverage = { + total: 1200, + measured: 900, + silent: 2, + unreadable: 3, + pending: 295, + enabled: true +}; + +afterEach(() => vi.clearAllMocks()); + +async function renderCard( + over: Partial = {}, + cov: Partial = {} +) { + vi.mocked(getLoudnessSettings).mockResolvedValue({ ...base, ...over }); + vi.mocked(getLoudnessCoverage).mockResolvedValue({ ...coverage, ...cov }); + const r = render(LoudnessSettingsCard); + await screen.findByRole('spinbutton', { name: /files analyzed at once/i }); + return r; +} + +const saveButton = () => screen.getByRole('button', { name: /save/i }); +const concurrency = () => screen.getByRole('spinbutton', { name: /files analyzed at once/i }); + +describe('LoudnessSettingsCard', () => { + test('shows how far the analysis has got', async () => { + await renderCard(); + const gauge = await screen.findByTestId('loudness-coverage'); + expect(gauge.textContent).toMatch(/900 of 1,200 tracks measured/); + expect(gauge.textContent).toMatch(/295 pending/); + expect(gauge.textContent).toMatch(/2 silent/); + expect(gauge.textContent).toMatch(/3 unreadable/); + expect(gauge.textContent).not.toMatch(/paused/); + }); + + // With analysis off, pending never shrinks; the gauge must not read as progress. + test('says the work is paused when analysis is off', async () => { + await renderCard({ enabled: false }, { enabled: false }); + const gauge = await screen.findByTestId('loudness-coverage'); + expect(gauge.textContent).toMatch(/paused while analysis is off/); + }); + + test('save is disabled until something changes, then saves the form', async () => { + vi.mocked(updateLoudnessSettings).mockResolvedValue({ ...base, backfill_concurrency: 4 }); + await renderCard(); + expect(saveButton()).toHaveProperty('disabled', true); + + await fireEvent.input(concurrency(), { target: { value: '4' } }); + await waitFor(() => expect(saveButton()).toHaveProperty('disabled', false)); + await fireEvent.click(saveButton()); + + await waitFor(() => + expect(updateLoudnessSettings).toHaveBeenCalledWith({ enabled: true, backfill_concurrency: 4 }) + ); + await waitFor(() => expect(pushToast).toHaveBeenCalledWith('Loudness analysis settings saved.')); + // The gauge is read again after a save: switching analysis on or off changes it. + expect(getLoudnessCoverage).toHaveBeenCalledTimes(2); + }); + + test('an out-of-range value is named and cannot be saved', async () => { + await renderCard(); + await fireEvent.input(concurrency(), { target: { value: '9' } }); + await waitFor(() => + expect(screen.getByTestId('settings-problems').textContent).toMatch(/1 to 8/) + ); + expect(saveButton()).toHaveProperty('disabled', true); + }); + + test('a failed load offers a retry', async () => { + vi.mocked(getLoudnessSettings).mockRejectedValue(new Error('boom')); + vi.mocked(getLoudnessCoverage).mockResolvedValue(coverage); + render(LoudnessSettingsCard); + await screen.findByText(/couldn't load loudness analysis settings/i); + expect(screen.getByRole('button', { name: /try again/i })).toBeTruthy(); + }); +}); diff --git a/web/src/routes/admin/+page.svelte b/web/src/routes/admin/+page.svelte index 97feda2a..5cc0c1a2 100644 --- a/web/src/routes/admin/+page.svelte +++ b/web/src/routes/admin/+page.svelte @@ -2,6 +2,7 @@ import { pageTitle } from '$lib/branding'; import { Disc3, Album, Music2, Check, X, RotateCcw, Trash2, Cloud, ChevronRight } from 'lucide-svelte'; import StageBadge from '$lib/components/StageBadge.svelte'; + import LoudnessSettingsCard from '$lib/components/LoudnessSettingsCard.svelte'; import { stageState } from './stage-state'; import { useQueryClient } from '@tanstack/svelte-query'; import { @@ -463,6 +464,10 @@ {/if}
+ + +

Cover art

diff --git a/web/src/routes/admin/admin.test.ts b/web/src/routes/admin/admin.test.ts index d80205f0..04f80b6f 100644 --- a/web/src/routes/admin/admin.test.ts +++ b/web/src/routes/admin/admin.test.ts @@ -48,7 +48,13 @@ vi.mock('$lib/api/admin', async () => { deleteQuarantineViaLidarr: vi.fn().mockResolvedValue({}), triggerScan: vi.fn().mockResolvedValue({}), refetchMissingCovers: vi.fn().mockResolvedValue({ started: true }), - researchMissingArt: vi.fn().mockResolvedValue({ version: 1 }) + researchMissingArt: vi.fn().mockResolvedValue({ version: 1 }), + // LoudnessSettingsCard, rendered by the page and tested on its own. + getLoudnessSettings: vi.fn().mockResolvedValue({ enabled: true, backfill_concurrency: 2 }), + updateLoudnessSettings: vi.fn(), + getLoudnessCoverage: vi.fn().mockResolvedValue({ + total: 0, measured: 0, silent: 0, unreadable: 0, pending: 0, enabled: true + }) }; }); From aee4b50bd4fd095d1bc38ec2b66411b053132402 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 6 Oct 2026 13:06:44 -0400 Subject: [PATCH 16/18] test(api): pass the loudness settings to Mount in the library test router (M464 #4995) Co-Authored-By: Claude Opus 5.5 --- internal/api/library_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/api/library_test.go b/internal/api/library_test.go index 81cd1677..db74f2fc 100644 --- a/internal/api/library_test.go +++ b/internal/api/library_test.go @@ -465,7 +465,7 @@ func TestRoutesRegisteredInMount(t *testing.T) { r := chi.NewRouter() w := playevents.NewWriter(h.pool, slog.New(slog.NewTextHandler(io.Discard, nil)), 30*time.Minute, 0.5, 30000) - Mount(r, h.pool, h.logger, w, config.RecommendationConfig{RadioSize: 50, RadioSizeMax: 200, RecentlyPlayedHours: 1}, h.recSettings, h.lidarrCfg, h.lidarrRequests, h.lidarrQuarantine, h.tracks, h.playlists, h.coverart, h.coverSettings, h.tagSettings, h.scanner, h.scanCfg, h.dataDir, nil, eventbus.New(), nil, nil, h.netSettings, nil, nil) + Mount(r, h.pool, h.logger, w, config.RecommendationConfig{RadioSize: 50, RadioSizeMax: 200, RecentlyPlayedHours: 1}, h.recSettings, h.lidarrCfg, h.lidarrRequests, h.lidarrQuarantine, h.tracks, h.playlists, h.coverart, h.coverSettings, h.tagSettings, h.scanner, h.scanCfg, h.dataDir, nil, eventbus.New(), nil, nil, h.netSettings, nil, nil, nil) paths := []string{ "/api/artists", From c2f81bf8df2a3b784f451e0bf9968b4777b2d1d5 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 6 Oct 2026 13:27:02 -0400 Subject: [PATCH 17/18] feat(library): album loudness from the tracks' summed block histograms (M464 #4996) Album-mode normalization plays a whole album at one gain. That gain comes from album_loudness (migration 0066): BS.1770's gated loudness over every block on the album, computed by summing the tracks' stored histograms and gating the sum. No audio is decoded again. Album true peak is the loudest track's. - Recomputed by the loudness worker each tick, after the track pass and whether or not analysis is switched on. ListAlbumsNeedingLoudness lists albums whose md5 over (present track id, measurement version and time) no longer matches the stored digest. One comparison covers every way membership changes (scan retag, duplicate merge, delete, missing and restored) without hooking each. - No album value until every present track has a settled measurement, so an album's gain doesn't shift mid-listen as the rest is measured. Silent and unreadable tracks count as settled. - Rows for albums with no present track left are dropped. - The parser and the merge share trimBins. Co-Authored-By: Claude Opus 5.5 --- internal/db/dbq/loudness.sql.go | 169 ++++++++++++ internal/db/dbq/models.go | 10 + .../migrations/0066_album_loudness.down.sql | 1 + .../db/migrations/0066_album_loudness.up.sql | 31 +++ internal/db/queries/loudness.sql | 66 +++++ internal/dbtest/reset.go | 1 + internal/library/album_loudness.go | 162 ++++++++++++ internal/library/album_loudness_test.go | 245 ++++++++++++++++++ internal/library/loudness.go | 20 +- internal/library/loudness_backfill.go | 36 ++- 10 files changed, 722 insertions(+), 19 deletions(-) create mode 100644 internal/db/migrations/0066_album_loudness.down.sql create mode 100644 internal/db/migrations/0066_album_loudness.up.sql create mode 100644 internal/library/album_loudness.go create mode 100644 internal/library/album_loudness_test.go diff --git a/internal/db/dbq/loudness.sql.go b/internal/db/dbq/loudness.sql.go index 1c05ba61..e4aa5f71 100644 --- a/internal/db/dbq/loudness.sql.go +++ b/internal/db/dbq/loudness.sql.go @@ -11,6 +11,24 @@ import ( "github.com/jackc/pgx/v5/pgtype" ) +const deleteOrphanAlbumLoudness = `-- name: DeleteOrphanAlbumLoudness :execrows +DELETE FROM album_loudness a + WHERE NOT EXISTS ( + SELECT 1 FROM tracks t WHERE t.album_id = a.album_id AND t.missing_since IS NULL + ) +` + +// Album rows whose album has no present track left (every file missing). The +// album row itself survives a missing file, so its loudness would otherwise +// stay behind describing tracks that are gone; it is recomputed if they return. +func (q *Queries) DeleteOrphanAlbumLoudness(ctx context.Context) (int64, error) { + result, err := q.db.Exec(ctx, deleteOrphanAlbumLoudness) + if err != nil { + return 0, err + } + return result.RowsAffected(), nil +} + const deleteTrackLoudness = `-- name: DeleteTrackLoudness :exec DELETE FROM track_loudness WHERE track_id = $1 ` @@ -83,6 +101,120 @@ func (q *Queries) GetLoudnessSettings(ctx context.Context) (LoudnessSetting, err return i, err } +const listAlbumLoudnessInputs = `-- name: ListAlbumLoudnessInputs :many +SELECT t.id, + (l.track_id IS NOT NULL)::boolean AS settled, + l.true_peak_dbtp, + l.block_hist_start, + l.block_hist + FROM tracks t + LEFT JOIN track_loudness l + ON l.track_id = t.id AND l.analysis_version >= $1 + WHERE t.album_id = $2 + AND t.missing_since IS NULL +` + +type ListAlbumLoudnessInputsParams struct { + CurrentVersion int16 + AlbumID pgtype.UUID +} + +type ListAlbumLoudnessInputsRow struct { + ID pgtype.UUID + Settled bool + TruePeakDbtp *float32 + BlockHistStart *int16 + BlockHist []int32 +} + +// Every present track on one album with its current measurement, if any. +// settled is false for a track not yet measured at the current version. +func (q *Queries) ListAlbumLoudnessInputs(ctx context.Context, arg ListAlbumLoudnessInputsParams) ([]ListAlbumLoudnessInputsRow, error) { + rows, err := q.db.Query(ctx, listAlbumLoudnessInputs, arg.CurrentVersion, arg.AlbumID) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListAlbumLoudnessInputsRow + for rows.Next() { + var i ListAlbumLoudnessInputsRow + if err := rows.Scan( + &i.ID, + &i.Settled, + &i.TruePeakDbtp, + &i.BlockHistStart, + &i.BlockHist, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const listAlbumsNeedingLoudness = `-- name: ListAlbumsNeedingLoudness :many +WITH present AS ( + SELECT t.album_id, + md5(string_agg( + t.id::text || ':' || coalesce( + l.analysis_version::text || '@' || l.analyzed_at::text, '-'), + ',' ORDER BY t.id)) AS digest + FROM tracks t + LEFT JOIN track_loudness l + ON l.track_id = t.id AND l.analysis_version >= $3::smallint + WHERE t.missing_since IS NULL + GROUP BY t.album_id +) +SELECT c.album_id, c.digest::text AS digest + FROM present c + LEFT JOIN album_loudness a ON a.album_id = c.album_id + WHERE (a.album_id IS NULL OR a.inputs_digest <> c.digest) + -- Casts: sqlc cannot infer a parameter's type through a CTE alias. + AND c.album_id > $1::uuid + ORDER BY c.album_id + LIMIT $2::integer +` + +type ListAlbumsNeedingLoudnessParams struct { + AfterID pgtype.UUID + BatchLimit int32 + CurrentVersion int16 +} + +type ListAlbumsNeedingLoudnessRow struct { + AlbumID pgtype.UUID + Digest string +} + +// The album pass's work queue (#4996): albums whose present tracks or their +// measurements have changed since album loudness was last computed, or that +// never had it. The digest is over every present track's id and the +// measurement it holds at the current version ('-' for none), so a track +// joining, leaving or being re-measured changes it. Keyset-paged on album id +// so a pass ends even if storing one album keeps failing. +func (q *Queries) ListAlbumsNeedingLoudness(ctx context.Context, arg ListAlbumsNeedingLoudnessParams) ([]ListAlbumsNeedingLoudnessRow, error) { + rows, err := q.db.Query(ctx, listAlbumsNeedingLoudness, arg.AfterID, arg.BatchLimit, arg.CurrentVersion) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListAlbumsNeedingLoudnessRow + for rows.Next() { + var i ListAlbumsNeedingLoudnessRow + if err := rows.Scan(&i.AlbumID, &i.Digest); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const listTracksNeedingLoudness = `-- name: ListTracksNeedingLoudness :many SELECT t.id, t.file_path, t.duration_ms FROM tracks t @@ -159,6 +291,43 @@ func (q *Queries) UpdateLoudnessSettings(ctx context.Context, arg UpdateLoudness return i, err } +const upsertAlbumLoudness = `-- name: UpsertAlbumLoudness :exec +INSERT INTO album_loudness ( + album_id, integrated_lufs, true_peak_dbtp, tracks_total, tracks_settled, inputs_digest +) VALUES ( + $1, $2, $3, + $4, $5, $6 +) +ON CONFLICT (album_id) DO UPDATE SET + integrated_lufs = EXCLUDED.integrated_lufs, + true_peak_dbtp = EXCLUDED.true_peak_dbtp, + tracks_total = EXCLUDED.tracks_total, + tracks_settled = EXCLUDED.tracks_settled, + inputs_digest = EXCLUDED.inputs_digest, + computed_at = now() +` + +type UpsertAlbumLoudnessParams struct { + AlbumID pgtype.UUID + IntegratedLufs *float32 + TruePeakDbtp *float32 + TracksTotal int32 + TracksSettled int32 + InputsDigest string +} + +func (q *Queries) UpsertAlbumLoudness(ctx context.Context, arg UpsertAlbumLoudnessParams) error { + _, err := q.db.Exec(ctx, upsertAlbumLoudness, + arg.AlbumID, + arg.IntegratedLufs, + arg.TruePeakDbtp, + arg.TracksTotal, + arg.TracksSettled, + arg.InputsDigest, + ) + return err +} + const upsertTrackLoudness = `-- name: UpsertTrackLoudness :exec INSERT INTO track_loudness ( track_id, integrated_lufs, true_peak_dbtp, loudness_range_lu, diff --git a/internal/db/dbq/models.go b/internal/db/dbq/models.go index 99b5cbf7..fd55ee9c 100644 --- a/internal/db/dbq/models.go +++ b/internal/db/dbq/models.go @@ -201,6 +201,16 @@ type Album struct { CoverArtSourcesVersion int32 } +type AlbumLoudness struct { + AlbumID pgtype.UUID + IntegratedLufs *float32 + TruePeakDbtp *float32 + TracksTotal int32 + TracksSettled int32 + InputsDigest string + ComputedAt pgtype.Timestamptz +} + type Artist struct { ID pgtype.UUID Name string diff --git a/internal/db/migrations/0066_album_loudness.down.sql b/internal/db/migrations/0066_album_loudness.down.sql new file mode 100644 index 00000000..7e0a1306 --- /dev/null +++ b/internal/db/migrations/0066_album_loudness.down.sql @@ -0,0 +1 @@ +DROP TABLE IF EXISTS album_loudness; diff --git a/internal/db/migrations/0066_album_loudness.up.sql b/internal/db/migrations/0066_album_loudness.up.sql new file mode 100644 index 00000000..9243dcb9 --- /dev/null +++ b/internal/db/migrations/0066_album_loudness.up.sql @@ -0,0 +1,31 @@ +-- 0066_album_loudness.up.sql — album loudness for album-mode normalization +-- (Scribe milestone #464, #4996). +-- +-- An album's loudness is the gated loudness of every 400 ms block on the album, +-- not an average of its tracks' values: a quiet interlude and a loud single +-- should keep their difference when the album plays in order. It is computed +-- from the per-track block histograms in track_loudness (0065), summed, so no +-- audio is decoded again. +-- +-- A derived value, recomputed by the loudness worker whenever its inputs +-- change. inputs_digest is an md5 over the album's present tracks and the +-- measurement each holds; the worker recomputes every album whose stored digest +-- no longer matches. That one comparison covers every way membership changes +-- (a scan moving a track between albums, a duplicate merge, a delete, a file +-- going missing or coming back) without hooking each of them. +CREATE TABLE album_loudness ( + album_id uuid PRIMARY KEY REFERENCES albums (id) ON DELETE CASCADE, + -- NULL until every present track has a settled measurement: an album + -- leveled from half its tracks would jump when the rest arrived. Also NULL + -- for an album with no block above the gate. Clients fall back to track + -- gain while it is NULL. + integrated_lufs real, + -- The loudest true peak of any track on the album, so album gain is held + -- to the headroom of its loudest track. + true_peak_dbtp real, + tracks_total integer NOT NULL, + -- Tracks with a settled measurement (measured, silent or unreadable). + tracks_settled integer NOT NULL, + inputs_digest text NOT NULL, + computed_at timestamptz NOT NULL DEFAULT now() +); diff --git a/internal/db/queries/loudness.sql b/internal/db/queries/loudness.sql index 821d0ad5..94aefb65 100644 --- a/internal/db/queries/loudness.sql +++ b/internal/db/queries/loudness.sql @@ -75,3 +75,69 @@ UPDATE loudness_settings updated_at = now() WHERE id = true RETURNING *; + +-- name: ListAlbumsNeedingLoudness :many +-- The album pass's work queue (#4996): albums whose present tracks or their +-- measurements have changed since album loudness was last computed, or that +-- never had it. The digest is over every present track's id and the +-- measurement it holds at the current version ('-' for none), so a track +-- joining, leaving or being re-measured changes it. Keyset-paged on album id +-- so a pass ends even if storing one album keeps failing. +WITH present AS ( + SELECT t.album_id, + md5(string_agg( + t.id::text || ':' || coalesce( + l.analysis_version::text || '@' || l.analyzed_at::text, '-'), + ',' ORDER BY t.id)) AS digest + FROM tracks t + LEFT JOIN track_loudness l + ON l.track_id = t.id AND l.analysis_version >= sqlc.arg(current_version)::smallint + WHERE t.missing_since IS NULL + GROUP BY t.album_id +) +SELECT c.album_id, c.digest::text AS digest + FROM present c + LEFT JOIN album_loudness a ON a.album_id = c.album_id + WHERE (a.album_id IS NULL OR a.inputs_digest <> c.digest) + -- Casts: sqlc cannot infer a parameter's type through a CTE alias. + AND c.album_id > sqlc.arg(after_id)::uuid + ORDER BY c.album_id + LIMIT sqlc.arg(batch_limit)::integer; + +-- name: ListAlbumLoudnessInputs :many +-- Every present track on one album with its current measurement, if any. +-- settled is false for a track not yet measured at the current version. +SELECT t.id, + (l.track_id IS NOT NULL)::boolean AS settled, + l.true_peak_dbtp, + l.block_hist_start, + l.block_hist + FROM tracks t + LEFT JOIN track_loudness l + ON l.track_id = t.id AND l.analysis_version >= sqlc.arg(current_version) + WHERE t.album_id = sqlc.arg(album_id) + AND t.missing_since IS NULL; + +-- name: UpsertAlbumLoudness :exec +INSERT INTO album_loudness ( + album_id, integrated_lufs, true_peak_dbtp, tracks_total, tracks_settled, inputs_digest +) VALUES ( + sqlc.arg(album_id), sqlc.narg(integrated_lufs), sqlc.narg(true_peak_dbtp), + sqlc.arg(tracks_total), sqlc.arg(tracks_settled), sqlc.arg(inputs_digest) +) +ON CONFLICT (album_id) DO UPDATE SET + integrated_lufs = EXCLUDED.integrated_lufs, + true_peak_dbtp = EXCLUDED.true_peak_dbtp, + tracks_total = EXCLUDED.tracks_total, + tracks_settled = EXCLUDED.tracks_settled, + inputs_digest = EXCLUDED.inputs_digest, + computed_at = now(); + +-- name: DeleteOrphanAlbumLoudness :execrows +-- Album rows whose album has no present track left (every file missing). The +-- album row itself survives a missing file, so its loudness would otherwise +-- stay behind describing tracks that are gone; it is recomputed if they return. +DELETE FROM album_loudness a + WHERE NOT EXISTS ( + SELECT 1 FROM tracks t WHERE t.album_id = a.album_id AND t.missing_since IS NULL + ); diff --git a/internal/dbtest/reset.go b/internal/dbtest/reset.go index 8d1558dc..24507d92 100644 --- a/internal/dbtest/reset.go +++ b/internal/dbtest/reset.go @@ -92,6 +92,7 @@ var dataTables = []string{ "duplicate_sweeps", "track_fingerprints", // M400 "track_loudness", // M464 + "album_loudness", // M464 "tracks", "albums", "artists", diff --git a/internal/library/album_loudness.go b/internal/library/album_loudness.go new file mode 100644 index 00000000..19ee5df4 --- /dev/null +++ b/internal/library/album_loudness.go @@ -0,0 +1,162 @@ +package library + +import ( + "context" + "fmt" + + "github.com/jackc/pgx/v5/pgtype" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +// Album loudness (M464 #4996). +// +// Album-mode normalization plays a whole album at one gain, so the quiet +// interlude stays quieter than the single it sits between. That gain comes +// from the album's loudness: BS.1770's gated loudness over every block on the +// album, which is what summing the tracks' block histograms and gating the sum +// computes. An average of the tracks' values is not the same thing: gating +// over the whole album drops a near-silent hidden track, where averaging +// would let it drag the album quieter. +// +// The values are derived, so they are recomputed rather than maintained: each +// worker tick lists the albums whose inputs digest has moved (see +// ListAlbumsNeedingLoudness) and recomputes those from the stored histograms. + +// albumLoudnessBatch is how many albums one query hands the album pass. +// Recomputing an album is a few small reads and arithmetic, no decode. +const albumLoudnessBatch = 200 + +// albumLoudness is one album's computed values. +type albumLoudness struct { + // integratedLUFS is nil until every track is settled, or when no block on + // the album passed the gate. + integratedLUFS *float32 + truePeakDBTP *float32 + total, settled int32 +} + +// computeAlbumLoudness sums the present tracks' histograms and gates the sum. +func computeAlbumLoudness(inputs []dbq.ListAlbumLoudnessInputsRow) albumLoudness { + a := albumLoudness{total: int32(len(inputs))} + hists := make([]blockHistogram, 0, len(inputs)) + for _, in := range inputs { + if !in.Settled { + continue + } + a.settled++ + if in.TruePeakDbtp != nil && (a.truePeakDBTP == nil || *in.TruePeakDbtp > *a.truePeakDBTP) { + peak := *in.TruePeakDbtp + a.truePeakDBTP = &peak + } + if in.BlockHistStart != nil && len(in.BlockHist) > 0 { + hists = append(hists, blockHistogram{start: *in.BlockHistStart, counts: in.BlockHist}) + } + } + // Leveling from part of an album would change its gain as the rest is + // measured, audibly, mid-listen. Wait for all of it. + if a.total == 0 || a.settled < a.total { + return a + } + if lufs, ok := mergeHistograms(hists).gatedLoudness(); ok { + v := float32(lufs) + a.integratedLUFS = &v + } + return a +} + +// mergeHistograms sums histograms that may cover different bin ranges into +// one, trimmed to the occupied range. A histogram reaching past the bin range +// (only a corrupt row could) is clipped rather than trusted. +func mergeHistograms(hs []blockHistogram) blockHistogram { + var bins [loudnessHistBins]int32 + for _, h := range hs { + for i, c := range h.counts { + if b := int(h.start) + i; b >= 0 && b < loudnessHistBins { + bins[b] += c + } + } + } + return trimBins(&bins) +} + +// AlbumLoudnessResult tallies one album pass. +type AlbumLoudnessResult struct { + Recomputed int + Leveled int // stored with an album loudness + Waiting int // stored without one: a track is not yet measured + Failed int + Orphans int64 // rows dropped because the album has no present track +} + +// albumPass recomputes every album whose inputs changed, keyset-paged on album +// id so a pass ends even when one album keeps failing to store. +// +// The digest stored is the one the list query computed. If a track changes +// between that query and the read of its inputs, the stored digest is already +// stale and the next pass recomputes the album again, so the values always +// converge on the inputs. +func (w *LoudnessBackfillWorker) albumPass(ctx context.Context) (AlbumLoudnessResult, error) { + q := dbq.New(w.pool) + var res AlbumLoudnessResult + after := pgtype.UUID{Valid: true} + for { + if err := ctx.Err(); err != nil { + return res, err + } + rows, err := q.ListAlbumsNeedingLoudness(ctx, dbq.ListAlbumsNeedingLoudnessParams{ + CurrentVersion: loudnessVersion, + AfterID: after, + BatchLimit: w.albumBatch, + }) + if err != nil { + return res, fmt.Errorf("list albums needing loudness: %w", err) + } + if len(rows) == 0 { + break + } + for _, row := range rows { + res.Recomputed++ + if err := storeAlbumLoudness(ctx, q, row.AlbumID, row.Digest, &res); err != nil { + res.Failed++ + w.logger.Warn("album loudness: recompute failed", "album_id", row.AlbumID, "err", err) + } + } + after = rows[len(rows)-1].AlbumID + } + n, err := q.DeleteOrphanAlbumLoudness(ctx) + if err != nil { + return res, fmt.Errorf("drop orphan album loudness: %w", err) + } + res.Orphans = n + return res, nil +} + +func storeAlbumLoudness( + ctx context.Context, q *dbq.Queries, albumID pgtype.UUID, digest string, res *AlbumLoudnessResult, +) error { + inputs, err := q.ListAlbumLoudnessInputs(ctx, dbq.ListAlbumLoudnessInputsParams{ + CurrentVersion: loudnessVersion, + AlbumID: albumID, + }) + if err != nil { + return fmt.Errorf("read inputs: %w", err) + } + a := computeAlbumLoudness(inputs) + if err := q.UpsertAlbumLoudness(ctx, dbq.UpsertAlbumLoudnessParams{ + AlbumID: albumID, + IntegratedLufs: a.integratedLUFS, + TruePeakDbtp: a.truePeakDBTP, + TracksTotal: a.total, + TracksSettled: a.settled, + InputsDigest: digest, + }); err != nil { + return fmt.Errorf("store: %w", err) + } + if a.integratedLUFS != nil { + res.Leveled++ + } else { + res.Waiting++ + } + return nil +} diff --git a/internal/library/album_loudness_test.go b/internal/library/album_loudness_test.go new file mode 100644 index 00000000..50badf13 --- /dev/null +++ b/internal/library/album_loudness_test.go @@ -0,0 +1,245 @@ +package library + +import ( + "context" + "io" + "log/slog" + "math" + "path/filepath" + "testing" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +// histAt builds a histogram of n blocks all at one loudness. +func histAt(lufs float64, n int32) blockHistogram { + return blockHistogram{ + start: int16(math.Round((lufs - loudnessHistFloor) * loudnessHistPerLU)), + counts: []int32{n}, + } +} + +func TestMergeHistograms_SumsAcrossDifferentRanges(t *testing.T) { + a := blockHistogram{start: 500, counts: []int32{1, 0, 2}} // bins 500..502 + b := blockHistogram{start: 501, counts: []int32{4, 0, 0, 5}} // bins 501..504 + got := mergeHistograms([]blockHistogram{a, b}) + want := blockHistogram{start: 500, counts: []int32{1, 4, 2, 0, 5}} + if got.start != want.start || len(got.counts) != len(want.counts) { + t.Fatalf("merged = %+v, want %+v", got, want) + } + for i := range want.counts { + if got.counts[i] != want.counts[i] { + t.Fatalf("merged = %+v, want %+v", got, want) + } + } + // Bins past the range come only from a corrupt row; they are dropped, not + // allowed to index out of the array. + bad := blockHistogram{start: loudnessHistBins - 1, counts: []int32{1, 9}} + if got := mergeHistograms([]blockHistogram{bad}); len(got.counts) != 1 || got.counts[0] != 1 { + t.Errorf("out-of-range bin not dropped: %+v", got) + } + if !mergeHistograms(nil).empty() { + t.Errorf("merging nothing gave a non-empty histogram") + } +} + +func input(settled bool, peak *float32, h blockHistogram) dbq.ListAlbumLoudnessInputsRow { + row := dbq.ListAlbumLoudnessInputsRow{Settled: settled, TruePeakDbtp: peak} + if !h.empty() { + start := h.start + row.BlockHistStart = &start + row.BlockHist = h.counts + } + return row +} + +func TestComputeAlbumLoudness(t *testing.T) { + f := func(v float32) *float32 { return &v } + + // Album loudness gates over the whole album. A near-silent hidden track is + // dropped by the relative gate, where averaging the tracks' values would + // have let it pull the album 15 LU quieter. + a := computeAlbumLoudness([]dbq.ListAlbumLoudnessInputsRow{ + input(true, f(-1.0), histAt(-10, 1800)), + input(true, f(-0.2), histAt(-10, 2400)), + input(true, f(-30), histAt(-40, 600)), + }) + if a.integratedLUFS == nil || math.Abs(float64(*a.integratedLUFS)-(-10)) > 0.01 { + t.Errorf("album loudness = %v, want -10 (the hidden track gated out)", a.integratedLUFS) + } + if a.truePeakDBTP == nil || *a.truePeakDBTP != -0.2 { + t.Errorf("album peak = %v, want the loudest track's -0.2", a.truePeakDBTP) + } + if a.total != 3 || a.settled != 3 { + t.Errorf("total/settled = %d/%d, want 3/3", a.total, a.settled) + } + + // Energy, not an average of LUFS: two equally long tracks at -8 and -14 + // make an album nearer the louder one than the midpoint (-11): -10.037. + a = computeAlbumLoudness([]dbq.ListAlbumLoudnessInputsRow{ + input(true, nil, histAt(-8, 1000)), + input(true, nil, histAt(-14, 1000)), + }) + if a.integratedLUFS == nil || math.Abs(float64(*a.integratedLUFS)-(-10.037)) > 0.01 { + t.Errorf("album loudness = %v, want -10.037", a.integratedLUFS) + } + + // One track not yet measured: no album value until it is. + a = computeAlbumLoudness([]dbq.ListAlbumLoudnessInputsRow{ + input(true, f(-1), histAt(-10, 100)), + input(false, nil, blockHistogram{}), + }) + if a.integratedLUFS != nil || a.settled != 1 || a.total != 2 { + t.Errorf("partly measured album = %+v, want no loudness, 1 of 2 settled", a) + } + + // Settled without blocks (silent, or unreadable): counted as settled and + // leveled from the rest. + a = computeAlbumLoudness([]dbq.ListAlbumLoudnessInputsRow{ + input(true, f(-3), histAt(-12, 100)), + input(true, nil, blockHistogram{}), + }) + if a.integratedLUFS == nil || math.Abs(float64(*a.integratedLUFS)-(-12)) > 0.01 { + t.Errorf("album with a silent track = %v, want -12 from the other track", a.integratedLUFS) + } + + // Every track silent: settled, but nothing to level by. + a = computeAlbumLoudness([]dbq.ListAlbumLoudnessInputsRow{input(true, nil, blockHistogram{})}) + if a.integratedLUFS != nil || a.settled != 1 { + t.Errorf("all-silent album = %+v, want no loudness", a) + } +} + +// TestAlbumLoudness_Integration pins that the album pass computes from the +// stored histograms, does nothing when nothing changed, and recomputes on +// each kind of membership change. +func TestAlbumLoudness_Integration(t *testing.T) { + pool := newPool(t) + ctx := context.Background() + q := dbq.New(pool) + dir := t.TempDir() + + first, album, artist := seedTrack(t, pool, filepath.Join(dir, "a.mp3")) + addTrack := func(name string, albumID dbq.Album) dbq.Track { + t.Helper() + tr, err := q.UpsertTrack(ctx, dbq.UpsertTrackParams{ + Title: name, AlbumID: albumID.ID, ArtistID: artist.ID, + DurationMs: 1000, FilePath: filepath.Join(dir, name+".mp3"), FileSize: 100, FileFormat: "mp3", + }) + if err != nil { + t.Fatalf("track %s: %v", name, err) + } + return tr + } + measure := func(tr dbq.Track, lufs float64, peak float32) { + t.Helper() + h := histAt(lufs, 100) + l := float32(lufs) + if err := q.UpsertTrackLoudness(ctx, dbq.UpsertTrackLoudnessParams{ + TrackID: tr.ID, IntegratedLufs: &l, TruePeakDbtp: &peak, + BlockHistStart: &h.start, BlockHist: h.counts, AnalysisVersion: loudnessVersion, + }); err != nil { + t.Fatalf("measure %s: %v", tr.Title, err) + } + } + read := func() (lufs, peak *float32, total, settled int32) { + t.Helper() + if err := pool.QueryRow(ctx, + "SELECT integrated_lufs, true_peak_dbtp, tracks_total, tracks_settled FROM album_loudness WHERE album_id = $1", + album.ID).Scan(&lufs, &peak, &total, &settled); err != nil { + t.Fatalf("read album loudness: %v", err) + } + return + } + w := NewLoudnessBackfillWorker(pool, slog.New(slog.NewTextHandler(io.Discard, nil)), nil) + w.albumBatch = 1 // forces the keyset cursor across queries + pass := func() AlbumLoudnessResult { + t.Helper() + res, err := w.albumPass(ctx) + if err != nil { + t.Fatalf("album pass: %v", err) + } + return res + } + + second := addTrack("b", album) + measure(first, -10, -1) + + // 1. One track unmeasured: the album is stored, waiting, without loudness. + if res := pass(); res.Recomputed != 1 || res.Waiting != 1 { + t.Fatalf("first pass = %+v, want 1 recomputed, waiting", res) + } + if lufs, _, total, settled := read(); lufs != nil || total != 2 || settled != 1 { + t.Fatalf("waiting album = lufs %v, %d/%d settled; want nil, 1/2", lufs, settled, total) + } + + // 2. Measuring the second track changes the digest; the album is leveled. + measure(second, -10, -0.5) + if res := pass(); res.Recomputed != 1 || res.Leveled != 1 { + t.Fatalf("second pass = %+v, want 1 leveled", res) + } + lufs, peak, _, _ := read() + if lufs == nil || math.Abs(float64(*lufs)-(-10)) > 0.01 || peak == nil || *peak != -0.5 { + t.Fatalf("leveled album = lufs %v peak %v, want -10 and -0.5", lufs, peak) + } + + // 3. Nothing changed: nothing recomputed. + if res := pass(); res.Recomputed != 0 { + t.Fatalf("idle pass = %+v, want nothing recomputed", res) + } + + // 4. A track joins (here, retagged onto this album): recomputed. + other, err := q.UpsertAlbum(ctx, dbq.UpsertAlbumParams{Title: "Other", SortTitle: "Other", ArtistID: artist.ID}) + if err != nil { + t.Fatalf("other album: %v", err) + } + loud := addTrack("loud", other) + measure(loud, -4, 0.3) + pass() + if _, err := pool.Exec(ctx, "UPDATE tracks SET album_id = $1 WHERE id = $2", album.ID, loud.ID); err != nil { + t.Fatalf("move track: %v", err) + } + if res := pass(); res.Recomputed < 1 { + t.Fatalf("pass after a track joined = %+v, want a recompute", res) + } + // Two tracks at -10 and one at -4, equally long: -7.003 by energy. + if lufs, peak, total, _ := read(); total != 3 || lufs == nil || math.Abs(float64(*lufs)-(-7.003)) > 0.01 || + peak == nil || *peak != 0.3 { + t.Fatalf("album after a loud track joined = lufs %v peak %v total %d", lufs, peak, total) + } + + // 5. A track's file goes missing: recomputed without it. + if _, err := pool.Exec(ctx, "UPDATE tracks SET missing_since = now() WHERE id = $1", loud.ID); err != nil { + t.Fatalf("mark missing: %v", err) + } + pass() + if lufs, peak, total, _ := read(); total != 2 || lufs == nil || math.Abs(float64(*lufs)-(-10)) > 0.01 || + peak == nil || *peak != -0.5 { + t.Fatalf("album after the loud track went missing = lufs %v peak %v total %d", lufs, peak, total) + } + + // 6. A track is deleted (as a duplicate merge does): recomputed. + if _, err := pool.Exec(ctx, "DELETE FROM tracks WHERE id = $1", second.ID); err != nil { + t.Fatalf("delete track: %v", err) + } + pass() + if _, _, total, _ := read(); total != 1 { + t.Fatalf("album after a delete has %d tracks, want 1", total) + } + + // 7. Every remaining track missing: the album row is dropped, not left + // describing tracks that are gone. + if _, err := pool.Exec(ctx, "UPDATE tracks SET missing_since = now() WHERE id = $1", first.ID); err != nil { + t.Fatalf("mark missing: %v", err) + } + if res := pass(); res.Orphans != 1 { + t.Fatalf("pass with every track missing = %+v, want 1 orphan dropped", res) + } + var n int + if err := pool.QueryRow(ctx, "SELECT count(*) FROM album_loudness WHERE album_id = $1", album.ID).Scan(&n); err != nil { + t.Fatalf("count: %v", err) + } + if n != 0 { + t.Fatalf("album loudness row survived every track going missing") + } +} diff --git a/internal/library/loudness.go b/internal/library/loudness.go index ffea3c9f..2fd50032 100644 --- a/internal/library/loudness.go +++ b/internal/library/loudness.go @@ -220,7 +220,6 @@ var ( // histogram, and the closing summary. type ebur128Parser struct { bins [loudnessHistBins]int32 - blocks int inSummary bool summary map[string]string lastLines []string @@ -273,7 +272,6 @@ func (p *ebur128Parser) line(raw string) { } bin := int(math.Round((v - loudnessHistFloor) * loudnessHistPerLU)) p.bins[min(bin, loudnessHistBins-1)]++ - p.blocks++ } func (p *ebur128Parser) tail() string { @@ -281,18 +279,24 @@ func (p *ebur128Parser) tail() string { } func (p *ebur128Parser) histogram() blockHistogram { - if p.blocks == 0 { - return blockHistogram{} - } + return trimBins(&p.bins) +} + +// trimBins turns a full-range bin array into a histogram trimmed to its +// occupied bins; an empty array gives an empty histogram. +func trimBins(bins *[loudnessHistBins]int32) blockHistogram { first, last := 0, loudnessHistBins-1 - for p.bins[first] == 0 { + for first <= last && bins[first] == 0 { first++ } - for p.bins[last] == 0 { + if first > last { + return blockHistogram{} + } + for bins[last] == 0 { last-- } counts := make([]int32, last-first+1) - copy(counts, p.bins[first:last+1]) + copy(counts, bins[first:last+1]) return blockHistogram{start: int16(first), counts: counts} } diff --git a/internal/library/loudness_backfill.go b/internal/library/loudness_backfill.go index 8dc84dd0..9bf05025 100644 --- a/internal/library/loudness_backfill.go +++ b/internal/library/loudness_backfill.go @@ -64,11 +64,12 @@ func (r *BackfillLoudnessResult) add(o loudnessOutcome) { // LoudnessBackfillWorker measures every track that has no current measurement. type LoudnessBackfillWorker struct { - pool *pgxpool.Pool - logger *slog.Logger - settings *LoudnessSettingsService - tick time.Duration - batch int32 + pool *pgxpool.Pool + logger *slog.Logger + settings *LoudnessSettingsService + tick time.Duration + batch int32 + albumBatch int32 // analyze is a field so an integration test pins which tracks a pass // touches, not what ffmpeg prints. analyze func(ctx context.Context, path string, durationMs int32) loudnessResult @@ -80,12 +81,13 @@ func NewLoudnessBackfillWorker( pool *pgxpool.Pool, logger *slog.Logger, settings *LoudnessSettingsService, ) *LoudnessBackfillWorker { return &LoudnessBackfillWorker{ - pool: pool, - logger: logger, - settings: settings, - tick: loudnessBackfillTick, - batch: loudnessBackfillBatch, - analyze: computeLoudness, + pool: pool, + logger: logger, + settings: settings, + tick: loudnessBackfillTick, + batch: loudnessBackfillBatch, + albumBatch: albumLoudnessBatch, + analyze: computeLoudness, } } @@ -121,6 +123,18 @@ func (w *LoudnessBackfillWorker) runOnce(ctx context.Context) { "processed", res.Processed, "measured", res.Measured, "silent", res.Silent, "unreadable", res.Unreadable, "inconclusive", res.Inconclusive) } + // Album loudness follows the tracks (#4996). It runs even with analysis + // switched off: it decodes nothing, and membership still changes as the + // library does. + albums, err := w.albumPass(ctx) + if err != nil && ctx.Err() == nil { + w.logger.Warn("album loudness: pass failed", "err", err, "recomputed", albums.Recomputed) + } + if albums.Recomputed > 0 || albums.Orphans > 0 { + w.logger.Info("album loudness: pass complete", + "recomputed", albums.Recomputed, "leveled", albums.Leveled, + "waiting", albums.Waiting, "failed", albums.Failed, "orphans", albums.Orphans) + } } // pass walks every track needing a measurement once, keyset-paged on id. The From e3aa8629d3ca2ee0812eef9dbfcc25aea6d759a5 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 6 Oct 2026 13:54:05 -0400 Subject: [PATCH 18/18] feat(api): deliver loudness gains to every client (M464 #4997) ReplayGain 2.0 values (gain to -18 LUFS, linear peak) derived from the stored track and album loudness: - Web: GET /api/tracks/replay-gain?ids=... (up to 200), a lookup the player calls for its queue, rather than a field on every TrackRef surface. - Android: track_gain/track_peak and album_gain/album_peak on the sync views, so cached tracks level offline. Storing a measurement logs a track change, and an album's values moving logs an album change, both before the write (#2704), so caches pick the gains up. - OpenSubsonic: replayGain on every song (album, getSong, search3, starred), as a JSON object and an XML element. Co-Authored-By: Claude Opus 5.5 --- internal/api/api.go | 3 + internal/api/library_sync.go | 13 +- internal/api/library_sync_views.go | 19 ++- internal/api/library_sync_views_test.go | 40 +++++- internal/api/replay_gain.go | 54 ++++++++ internal/api/replay_gain_test.go | 70 ++++++++++ internal/db/dbq/loudness.sql.go | 149 +++++++++++++++++++-- internal/db/queries/loudness.sql | 32 ++++- internal/library/album_loudness.go | 58 +++++++- internal/library/album_loudness_test.go | 32 ++++- internal/library/loudness.go | 16 ++- internal/library/loudness_backfill.go | 2 +- internal/library/loudness_backfill_test.go | 13 ++ internal/library/replaygain.go | 102 ++++++++++++++ internal/library/replaygain_test.go | 122 +++++++++++++++++ internal/subsonic/browse.go | 8 +- internal/subsonic/replaygain_test.go | 58 ++++++++ internal/subsonic/star.go | 3 +- internal/subsonic/types.go | 41 +++++- 19 files changed, 797 insertions(+), 38 deletions(-) create mode 100644 internal/api/replay_gain.go create mode 100644 internal/api/replay_gain_test.go create mode 100644 internal/library/replaygain.go create mode 100644 internal/library/replaygain_test.go create mode 100644 internal/subsonic/replaygain_test.go diff --git a/internal/api/api.go b/internal/api/api.go index 65d4240b..a495c2fa 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -132,6 +132,9 @@ func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger, events *playev authed.Get("/library/genres", h.handleListGenres) authed.Get("/library/years", h.handleListAlbumYears) authed.Get("/library/sync", h.handleLibrarySync) + // Before /tracks/{id} for readability; chi prefers the static + // segment either way. + authed.Get("/tracks/replay-gain", h.handleGetReplayGain) authed.Get("/tracks/{id}", h.handleGetTrack) // /tracks/{id}/stream is mounted above with OptionalUser so // it can accept either a session or a signed token. diff --git a/internal/api/library_sync.go b/internal/api/library_sync.go index 96363ad1..932d7b93 100644 --- a/internal/api/library_sync.go +++ b/internal/api/library_sync.go @@ -10,6 +10,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/apierror" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/library" syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync" ) @@ -151,8 +152,12 @@ func (h *handlers) hydrateUpserts( if err != nil { return nil, err } + gains, err := library.ReplayGainForAlbums(ctx, q, uuids) + if err != nil { + return nil, err + } for _, a := range albums { - b, _ := json.Marshal(toAlbumSyncView(a)) + b, _ := json.Marshal(toAlbumSyncView(a, gains[a.ID])) out["album"] = append(out["album"], b) } } @@ -162,8 +167,12 @@ func (h *handlers) hydrateUpserts( if err != nil { return nil, err } + gains, err := library.ReplayGainForTracks(ctx, q, uuids) + if err != nil { + return nil, err + } for _, t := range tracks { - b, _ := json.Marshal(toTrackSyncView(t)) + b, _ := json.Marshal(toTrackSyncView(t, gains[t.ID])) out["track"] = append(out["track"], b) } } diff --git a/internal/api/library_sync_views.go b/internal/api/library_sync_views.go index 1e8520f9..c9038c3c 100644 --- a/internal/api/library_sync_views.go +++ b/internal/api/library_sync_views.go @@ -18,6 +18,7 @@ package api import ( "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/library" syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync" ) @@ -49,9 +50,14 @@ type albumSyncView struct { ReleaseDate *string `json:"release_date"` CoverArtPath *string `json:"cover_art_path"` Mbid *string `json:"mbid"` + // AlbumGain and AlbumPeak are the album's ReplayGain 2.0 values (#4997): + // dB to the -18 LUFS reference, and a linear peak. Null until every track + // on the album is measured. + AlbumGain *float32 `json:"album_gain"` + AlbumPeak *float32 `json:"album_peak"` } -func toAlbumSyncView(a dbq.Album) albumSyncView { +func toAlbumSyncView(a dbq.Album, g library.ReplayGain) albumSyncView { var releaseDate *string if a.ReleaseDate.Valid { s := a.ReleaseDate.Time.Format("2006-01-02") @@ -65,6 +71,8 @@ func toAlbumSyncView(a dbq.Album) albumSyncView { ReleaseDate: releaseDate, CoverArtPath: a.CoverArtPath, Mbid: a.Mbid, + AlbumGain: g.AlbumGain, + AlbumPeak: g.AlbumPeak, } } @@ -92,9 +100,14 @@ type trackSyncView struct { // track is playable, which is a yes/no. The "gone since" clock is an // operator concern and lives on the admin surface. Missing bool `json:"missing"` + // TrackGain and TrackPeak are the track's ReplayGain 2.0 values (#4997). + // Null until the track is measured. The album's pair rides on the album + // view, since it changes when the album does, not when this track does. + TrackGain *float32 `json:"track_gain"` + TrackPeak *float32 `json:"track_peak"` } -func toTrackSyncView(t dbq.Track) trackSyncView { +func toTrackSyncView(t dbq.Track, g library.ReplayGain) trackSyncView { return trackSyncView{ ID: syncpkg.FormatUUID(t.ID), AlbumID: syncpkg.FormatUUID(t.AlbumID), @@ -107,6 +120,8 @@ func toTrackSyncView(t dbq.Track) trackSyncView { FileFormat: t.FileFormat, Genre: t.Genre, Missing: t.MissingSince.Valid, + TrackGain: g.TrackGain, + TrackPeak: g.TrackPeak, } } diff --git a/internal/api/library_sync_views_test.go b/internal/api/library_sync_views_test.go index bd322d23..fe627d45 100644 --- a/internal/api/library_sync_views_test.go +++ b/internal/api/library_sync_views_test.go @@ -8,6 +8,7 @@ import ( "github.com/jackc/pgx/v5/pgtype" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/library" ) // These tests pin the wire-format keys for /api/library/sync upserts. @@ -53,13 +54,13 @@ func TestArtistSyncView_WireKeys(t *testing.T) { func TestAlbumSyncView_WireKeys(t *testing.T) { a := dbq.Album{ID: validUUID, ArtistID: validUUID, Title: "Drukqs", SortTitle: "Drukqs"} - b, err := json.Marshal(toAlbumSyncView(a)) + b, err := json.Marshal(toAlbumSyncView(a, library.ReplayGain{})) if err != nil { t.Fatal(err) } assertJSONKeys(t, "album", b, []string{ "id", "artist_id", "title", "sort_title", - "release_date", "cover_art_path", "mbid", + "release_date", "cover_art_path", "mbid", "album_gain", "album_peak", }) } @@ -69,14 +70,14 @@ func TestTrackSyncView_WireKeys(t *testing.T) { Title: "Avril 14th", DurationMs: 121_000, FilePath: "x", FileFormat: "flac", } - b, err := json.Marshal(toTrackSyncView(tr)) + b, err := json.Marshal(toTrackSyncView(tr, library.ReplayGain{})) if err != nil { t.Fatal(err) } assertJSONKeys(t, "track", b, []string{ "id", "album_id", "artist_id", "title", "duration_ms", "track_number", "disc_number", "file_path", "file_format", "genre", - "missing", + "missing", "track_gain", "track_peak", }) } @@ -85,17 +86,44 @@ func TestTrackSyncView_WireKeys(t *testing.T) { // client's library, a missing one stays playable and fails at the speaker. func TestTrackSyncView_MissingReflectsTheMark(t *testing.T) { present := dbq.Track{ID: validUUID, AlbumID: validUUID, ArtistID: validUUID} - if toTrackSyncView(present).Missing { + if toTrackSyncView(present, library.ReplayGain{}).Missing { t.Error("a track with no missing_since must not be marked missing") } gone := present gone.MissingSince = pgtype.Timestamptz{Time: time.Now(), Valid: true} - if !toTrackSyncView(gone).Missing { + if !toTrackSyncView(gone, library.ReplayGain{}).Missing { t.Error("a track with missing_since must be marked missing") } } +// The gains are what Android levels playback by, offline included (#4997): +// the track's pair rides on the track view and the album's on the album view. +func TestSyncViews_CarryReplayGain(t *testing.T) { + gain, peak := float32(-3.25), float32(0.9441) + g := library.ReplayGain{TrackGain: &gain, TrackPeak: &peak, AlbumGain: &gain, AlbumPeak: &peak} + tv := toTrackSyncView(dbq.Track{ID: validUUID, AlbumID: validUUID, ArtistID: validUUID}, g) + if tv.TrackGain == nil || *tv.TrackGain != gain || tv.TrackPeak == nil || *tv.TrackPeak != peak { + t.Errorf("track view gains = %v/%v, want %v/%v", tv.TrackGain, tv.TrackPeak, gain, peak) + } + av := toAlbumSyncView(dbq.Album{ID: validUUID, ArtistID: validUUID}, g) + if av.AlbumGain == nil || *av.AlbumGain != gain || av.AlbumPeak == nil || *av.AlbumPeak != peak { + t.Errorf("album view gains = %v/%v, want %v/%v", av.AlbumGain, av.AlbumPeak, gain, peak) + } + // Unmeasured: explicit nulls, like every other optional sync field. + b, err := json.Marshal(toTrackSyncView(dbq.Track{ID: validUUID, AlbumID: validUUID, ArtistID: validUUID}, library.ReplayGain{})) + if err != nil { + t.Fatal(err) + } + var m map[string]any + if err := json.Unmarshal(b, &m); err != nil { + t.Fatal(err) + } + if v, ok := m["track_gain"]; !ok || v != nil { + t.Errorf("unmeasured track_gain = %v (present %v), want an explicit null", v, ok) + } +} + func TestPlaylistSyncView_WireKeys(t *testing.T) { variant := "discover" p := dbq.Playlist{ diff --git a/internal/api/replay_gain.go b/internal/api/replay_gain.go new file mode 100644 index 00000000..38009db1 --- /dev/null +++ b/internal/api/replay_gain.go @@ -0,0 +1,54 @@ +package api + +import ( + "net/http" + "strings" + + "git.fabledsword.com/bvandeusen/minstrel/internal/apierror" + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/library" +) + +// maxReplayGainIDs caps one lookup. A player asks for the tracks around the +// one it is about to play, never a whole library. +const maxReplayGainIDs = 200 + +// replayGainResp is the wire shape for GET /api/tracks/replay-gain. Tracks +// with no gain known are absent from items: no adjustment. +type replayGainResp struct { + Items map[string]library.ReplayGain `json:"items"` +} + +// handleGetReplayGain implements GET /api/tracks/replay-gain?ids=a,b,c (M464 +// #4997): ReplayGain 2.0 values (dB to -18 LUFS, linear peaks) for each track, +// and for its album when the whole album is measured. +// +// A lookup rather than fields on TrackRef. Tracks reach the player through +// about fifteen surfaces in two wire shapes (TrackRef and playlist entries), +// and only the player uses the gain. One call from the player, made for the +// tracks it is about to play, cannot be forgotten by a surface added later, +// and costs nothing on the pages that never play anything. Android takes the +// same values from the sync feed instead, so they are there offline. +func (h *handlers) handleGetReplayGain(w http.ResponseWriter, r *http.Request) { + raw := strings.TrimSpace(r.URL.Query().Get("ids")) + if raw == "" { + writeJSON(w, http.StatusOK, replayGainResp{Items: map[string]library.ReplayGain{}}) + return + } + parts := strings.Split(raw, ",") + if len(parts) > maxReplayGainIDs { + writeErr(w, apierror.BadRequest("too_many_ids", "at most 200 ids per request")) + return + } + ids := stringsToUUIDs(parts) + gains, err := library.ReplayGainForTracks(r.Context(), dbq.New(h.pool), ids) + if err != nil { + writeErrWithLog(w, h.logger, "replay gain: lookup", apierror.InternalMsg("lookup failed", err)) + return + } + items := make(map[string]library.ReplayGain, len(gains)) + for id, g := range gains { + items[uuidToString(id)] = g + } + writeJSON(w, http.StatusOK, replayGainResp{Items: items}) +} diff --git a/internal/api/replay_gain_test.go b/internal/api/replay_gain_test.go new file mode 100644 index 00000000..150997bf --- /dev/null +++ b/internal/api/replay_gain_test.go @@ -0,0 +1,70 @@ +package api + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/go-chi/chi/v5" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +func TestGetReplayGain(t *testing.T) { + h, pool := testHandlers(t) + user := seedUser(t, pool, "rg1", "pw", false) + measured, _ := seedTrackForRemoveTest(t, h, "rg-measured", "", "") + unmeasured, _ := seedTrackForRemoveTest(t, h, "rg-unmeasured", "", "") + lufs, peak := float32(-12.5), float32(-1) + if err := dbq.New(pool).UpsertTrackLoudness(context.Background(), dbq.UpsertTrackLoudnessParams{ + TrackID: measured.ID, IntegratedLufs: &lufs, TruePeakDbtp: &peak, AnalysisVersion: 1, + }); err != nil { + t.Fatalf("seed loudness: %v", err) + } + r := chi.NewRouter() + r.Get("/api/tracks/replay-gain", h.handleGetReplayGain) + get := func(query string) *httptest.ResponseRecorder { + req := withUser(httptest.NewRequest(http.MethodGet, "/api/tracks/replay-gain"+query, nil), user) + rec := httptest.NewRecorder() + r.ServeHTTP(rec, req) + return rec + } + + rec := get("?ids=" + uuidToString(measured.ID) + "," + uuidToString(unmeasured.ID) + ",not-a-uuid") + if rec.Code != http.StatusOK { + t.Fatalf("status = %d; body=%s", rec.Code, rec.Body.String()) + } + var resp struct { + Items map[string]struct { + TrackGain *float32 `json:"track_gain"` + TrackPeak *float32 `json:"track_peak"` + AlbumGain *float32 `json:"album_gain"` + } `json:"items"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode: %v", err) + } + g, ok := resp.Items[uuidToString(measured.ID)] + if !ok || g.TrackGain == nil || *g.TrackGain != -5.5 || g.TrackPeak == nil { + t.Errorf("measured track = %+v (present %v), want track_gain -5.5 with a peak", g, ok) + } + if g.AlbumGain != nil { + t.Errorf("album_gain present with no album loudness: %v", *g.AlbumGain) + } + if _, ok := resp.Items[uuidToString(unmeasured.ID)]; ok { + t.Errorf("unmeasured track listed; absent means no adjustment") + } + if len(resp.Items) != 1 { + t.Errorf("items = %v, want only the measured track", resp.Items) + } + + if rec := get(""); rec.Code != http.StatusOK || !strings.Contains(rec.Body.String(), `"items":{}`) { + t.Errorf("no ids: %d %s, want 200 with empty items", rec.Code, rec.Body.String()) + } + if rec := get("?ids=" + strings.Repeat("x,", maxReplayGainIDs)); rec.Code != http.StatusBadRequest { + t.Errorf("%d ids: status %d, want 400", maxReplayGainIDs+1, rec.Code) + } +} diff --git a/internal/db/dbq/loudness.sql.go b/internal/db/dbq/loudness.sql.go index e4aa5f71..59d3c143 100644 --- a/internal/db/dbq/loudness.sql.go +++ b/internal/db/dbq/loudness.sql.go @@ -11,22 +11,13 @@ import ( "github.com/jackc/pgx/v5/pgtype" ) -const deleteOrphanAlbumLoudness = `-- name: DeleteOrphanAlbumLoudness :execrows -DELETE FROM album_loudness a - WHERE NOT EXISTS ( - SELECT 1 FROM tracks t WHERE t.album_id = a.album_id AND t.missing_since IS NULL - ) +const deleteAlbumLoudness = `-- name: DeleteAlbumLoudness :exec +DELETE FROM album_loudness WHERE album_id = ANY($1::uuid[]) ` -// Album rows whose album has no present track left (every file missing). The -// album row itself survives a missing file, so its loudness would otherwise -// stay behind describing tracks that are gone; it is recomputed if they return. -func (q *Queries) DeleteOrphanAlbumLoudness(ctx context.Context) (int64, error) { - result, err := q.db.Exec(ctx, deleteOrphanAlbumLoudness) - if err != nil { - return 0, err - } - return result.RowsAffected(), nil +func (q *Queries) DeleteAlbumLoudness(ctx context.Context, albumIds []pgtype.UUID) error { + _, err := q.db.Exec(ctx, deleteAlbumLoudness, albumIds) + return err } const deleteTrackLoudness = `-- name: DeleteTrackLoudness :exec @@ -40,6 +31,56 @@ func (q *Queries) DeleteTrackLoudness(ctx context.Context, trackID pgtype.UUID) return err } +const getAlbumLoudness = `-- name: GetAlbumLoudness :one +SELECT integrated_lufs, true_peak_dbtp FROM album_loudness WHERE album_id = $1 +` + +type GetAlbumLoudnessRow struct { + IntegratedLufs *float32 + TruePeakDbtp *float32 +} + +// The stored album values, read before a recompute so a sync change is +// logged only when they actually move. +func (q *Queries) GetAlbumLoudness(ctx context.Context, albumID pgtype.UUID) (GetAlbumLoudnessRow, error) { + row := q.db.QueryRow(ctx, getAlbumLoudness, albumID) + var i GetAlbumLoudnessRow + err := row.Scan(&i.IntegratedLufs, &i.TruePeakDbtp) + return i, err +} + +const getAlbumLoudnessByIDs = `-- name: GetAlbumLoudnessByIDs :many +SELECT album_id, integrated_lufs, true_peak_dbtp + FROM album_loudness + WHERE album_id = ANY($1::uuid[]) +` + +type GetAlbumLoudnessByIDsRow struct { + AlbumID pgtype.UUID + IntegratedLufs *float32 + TruePeakDbtp *float32 +} + +func (q *Queries) GetAlbumLoudnessByIDs(ctx context.Context, ids []pgtype.UUID) ([]GetAlbumLoudnessByIDsRow, error) { + rows, err := q.db.Query(ctx, getAlbumLoudnessByIDs, ids) + if err != nil { + return nil, err + } + defer rows.Close() + var items []GetAlbumLoudnessByIDsRow + for rows.Next() { + var i GetAlbumLoudnessByIDsRow + if err := rows.Scan(&i.AlbumID, &i.IntegratedLufs, &i.TruePeakDbtp); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const getLoudnessCoverage = `-- name: GetLoudnessCoverage :one SELECT count(*)::bigint AS total, count(*) FILTER ( @@ -101,6 +142,55 @@ func (q *Queries) GetLoudnessSettings(ctx context.Context) (LoudnessSetting, err return i, err } +const getReplayGainByTrackIDs = `-- name: GetReplayGainByTrackIDs :many +SELECT t.id, + tl.integrated_lufs AS track_lufs, + tl.true_peak_dbtp AS track_peak_dbtp, + al.integrated_lufs AS album_lufs, + al.true_peak_dbtp AS album_peak_dbtp + FROM tracks t + LEFT JOIN track_loudness tl ON tl.track_id = t.id + LEFT JOIN album_loudness al ON al.album_id = t.album_id + WHERE t.id = ANY($1::uuid[]) +` + +type GetReplayGainByTrackIDsRow struct { + ID pgtype.UUID + TrackLufs *float32 + TrackPeakDbtp *float32 + AlbumLufs *float32 + AlbumPeakDbtp *float32 +} + +// Track and album loudness for a set of tracks, for every surface that hands +// gains to a client (#4997). A measurement from an older analysis version is +// still delivered: it is a better gain than none until the backfill redoes it. +func (q *Queries) GetReplayGainByTrackIDs(ctx context.Context, ids []pgtype.UUID) ([]GetReplayGainByTrackIDsRow, error) { + rows, err := q.db.Query(ctx, getReplayGainByTrackIDs, ids) + if err != nil { + return nil, err + } + defer rows.Close() + var items []GetReplayGainByTrackIDsRow + for rows.Next() { + var i GetReplayGainByTrackIDsRow + if err := rows.Scan( + &i.ID, + &i.TrackLufs, + &i.TrackPeakDbtp, + &i.AlbumLufs, + &i.AlbumPeakDbtp, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const listAlbumLoudnessInputs = `-- name: ListAlbumLoudnessInputs :many SELECT t.id, (l.track_id IS NOT NULL)::boolean AS settled, @@ -215,6 +305,37 @@ func (q *Queries) ListAlbumsNeedingLoudness(ctx context.Context, arg ListAlbumsN return items, nil } +const listOrphanAlbumLoudness = `-- name: ListOrphanAlbumLoudness :many +SELECT a.album_id + FROM album_loudness a + WHERE NOT EXISTS ( + SELECT 1 FROM tracks t WHERE t.album_id = a.album_id AND t.missing_since IS NULL + ) +` + +// Album rows whose album has no present track left (every file missing). The +// album row itself survives a missing file, so its loudness would otherwise +// stay behind describing tracks that are gone; it is recomputed if they return. +func (q *Queries) ListOrphanAlbumLoudness(ctx context.Context) ([]pgtype.UUID, error) { + rows, err := q.db.Query(ctx, listOrphanAlbumLoudness) + if err != nil { + return nil, err + } + defer rows.Close() + var items []pgtype.UUID + for rows.Next() { + var album_id pgtype.UUID + if err := rows.Scan(&album_id); err != nil { + return nil, err + } + items = append(items, album_id) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const listTracksNeedingLoudness = `-- name: ListTracksNeedingLoudness :many SELECT t.id, t.file_path, t.duration_ms FROM tracks t diff --git a/internal/db/queries/loudness.sql b/internal/db/queries/loudness.sql index 94aefb65..af33fb1b 100644 --- a/internal/db/queries/loudness.sql +++ b/internal/db/queries/loudness.sql @@ -133,11 +133,39 @@ ON CONFLICT (album_id) DO UPDATE SET inputs_digest = EXCLUDED.inputs_digest, computed_at = now(); --- name: DeleteOrphanAlbumLoudness :execrows +-- name: ListOrphanAlbumLoudness :many -- Album rows whose album has no present track left (every file missing). The -- album row itself survives a missing file, so its loudness would otherwise -- stay behind describing tracks that are gone; it is recomputed if they return. -DELETE FROM album_loudness a +SELECT a.album_id + FROM album_loudness a WHERE NOT EXISTS ( SELECT 1 FROM tracks t WHERE t.album_id = a.album_id AND t.missing_since IS NULL ); + +-- name: DeleteAlbumLoudness :exec +DELETE FROM album_loudness WHERE album_id = ANY(sqlc.arg(album_ids)::uuid[]); + +-- name: GetAlbumLoudness :one +-- The stored album values, read before a recompute so a sync change is +-- logged only when they actually move. +SELECT integrated_lufs, true_peak_dbtp FROM album_loudness WHERE album_id = $1; + +-- name: GetReplayGainByTrackIDs :many +-- Track and album loudness for a set of tracks, for every surface that hands +-- gains to a client (#4997). A measurement from an older analysis version is +-- still delivered: it is a better gain than none until the backfill redoes it. +SELECT t.id, + tl.integrated_lufs AS track_lufs, + tl.true_peak_dbtp AS track_peak_dbtp, + al.integrated_lufs AS album_lufs, + al.true_peak_dbtp AS album_peak_dbtp + FROM tracks t + LEFT JOIN track_loudness tl ON tl.track_id = t.id + LEFT JOIN album_loudness al ON al.album_id = t.album_id + WHERE t.id = ANY(sqlc.arg(ids)::uuid[]); + +-- name: GetAlbumLoudnessByIDs :many +SELECT album_id, integrated_lufs, true_peak_dbtp + FROM album_loudness + WHERE album_id = ANY(sqlc.arg(ids)::uuid[]); diff --git a/internal/library/album_loudness.go b/internal/library/album_loudness.go index 19ee5df4..7d029e0c 100644 --- a/internal/library/album_loudness.go +++ b/internal/library/album_loudness.go @@ -2,11 +2,14 @@ package library import ( "context" + "errors" "fmt" + "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgtype" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync" ) // Album loudness (M464 #4996). @@ -117,24 +120,46 @@ func (w *LoudnessBackfillWorker) albumPass(ctx context.Context) (AlbumLoudnessRe } for _, row := range rows { res.Recomputed++ - if err := storeAlbumLoudness(ctx, q, row.AlbumID, row.Digest, &res); err != nil { + if err := w.storeAlbumLoudness(ctx, q, row.AlbumID, row.Digest, &res); err != nil { res.Failed++ w.logger.Warn("album loudness: recompute failed", "album_id", row.AlbumID, "err", err) } } after = rows[len(rows)-1].AlbumID } - n, err := q.DeleteOrphanAlbumLoudness(ctx) + // Listed, logged, then deleted: the same log-first order as above. + orphans, err := q.ListOrphanAlbumLoudness(ctx) if err != nil { + return res, fmt.Errorf("list orphan album loudness: %w", err) + } + if len(orphans) == 0 { + return res, nil + } + ids := make([]string, len(orphans)) + for i, id := range orphans { + ids[i] = syncpkg.FormatUUID(id) + } + if err := syncpkg.LogChanges(ctx, w.pool, syncpkg.EntityAlbum, ids, syncpkg.OpUpsert); err != nil { + return res, fmt.Errorf("log orphan album changes: %w", err) + } + if err := q.DeleteAlbumLoudness(ctx, orphans); err != nil { return res, fmt.Errorf("drop orphan album loudness: %w", err) } - res.Orphans = n + res.Orphans = int64(len(orphans)) return res, nil } -func storeAlbumLoudness( +// storeAlbumLoudness recomputes one album. The sync feed hears about it only +// when the values clients see move: during the backfill an album's digest +// changes with every track measured, and most of those recomputes still end +// with no album value. +func (w *LoudnessBackfillWorker) storeAlbumLoudness( ctx context.Context, q *dbq.Queries, albumID pgtype.UUID, digest string, res *AlbumLoudnessResult, ) error { + before, err := q.GetAlbumLoudness(ctx, albumID) + if err != nil && !errors.Is(err, pgx.ErrNoRows) { + return fmt.Errorf("read stored values: %w", err) + } inputs, err := q.ListAlbumLoudnessInputs(ctx, dbq.ListAlbumLoudnessInputsParams{ CurrentVersion: loudnessVersion, AlbumID: albumID, @@ -143,6 +168,14 @@ func storeAlbumLoudness( return fmt.Errorf("read inputs: %w", err) } a := computeAlbumLoudness(inputs) + // Logged before the write, for the reason storeLoudness gives (#2704). A + // failed log stores nothing, so the digest still differs and the next + // pass tries again. + if albumVisibleChange(before, a) { + if err := syncpkg.LogChange(ctx, w.pool, syncpkg.EntityAlbum, syncpkg.FormatUUID(albumID), syncpkg.OpUpsert); err != nil { + return fmt.Errorf("log album change: %w", err) + } + } if err := q.UpsertAlbumLoudness(ctx, dbq.UpsertAlbumLoudnessParams{ AlbumID: albumID, IntegratedLufs: a.integratedLUFS, @@ -160,3 +193,20 @@ func storeAlbumLoudness( } return nil } + +// albumVisibleChange reports whether clients would see different album gains. +// The peak is only delivered beside a loudness, so a waiting album whose peak +// moves as tracks are measured has nothing new to tell anyone. +func albumVisibleChange(before dbq.GetAlbumLoudnessRow, after albumLoudness) bool { + if !sameFloat(before.IntegratedLufs, after.integratedLUFS) { + return true + } + return after.integratedLUFS != nil && !sameFloat(before.TruePeakDbtp, after.truePeakDBTP) +} + +func sameFloat(a, b *float32) bool { + if a == nil || b == nil { + return a == b + } + return *a == *b +} diff --git a/internal/library/album_loudness_test.go b/internal/library/album_loudness_test.go index 50badf13..a943e024 100644 --- a/internal/library/album_loudness_test.go +++ b/internal/library/album_loudness_test.go @@ -9,6 +9,7 @@ import ( "testing" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync" ) // histAt builds a histogram of n blocks all at one loudness. @@ -162,6 +163,17 @@ func TestAlbumLoudness_Integration(t *testing.T) { return res } + albumChanges := func() int { + t.Helper() + var n int + if err := pool.QueryRow(ctx, `SELECT count(*) FROM library_changes + WHERE entity_type = 'album' AND entity_id = $1`, + syncpkg.FormatUUID(album.ID)).Scan(&n); err != nil { + t.Fatalf("count album changes: %v", err) + } + return n + } + second := addTrack("b", album) measure(first, -10, -1) @@ -173,6 +185,11 @@ func TestAlbumLoudness_Integration(t *testing.T) { t.Fatalf("waiting album = lufs %v, %d/%d settled; want nil, 1/2", lufs, settled, total) } + // Still no album value: clients have nothing new to read (#4997). + if n := albumChanges(); n != 0 { + t.Fatalf("a waiting album logged %d sync changes, want 0", n) + } + // 2. Measuring the second track changes the digest; the album is leveled. measure(second, -10, -0.5) if res := pass(); res.Recomputed != 1 || res.Leveled != 1 { @@ -183,10 +200,18 @@ func TestAlbumLoudness_Integration(t *testing.T) { t.Fatalf("leveled album = lufs %v peak %v, want -10 and -0.5", lufs, peak) } - // 3. Nothing changed: nothing recomputed. + // Leveled: clients are told, once. + if n := albumChanges(); n != 1 { + t.Fatalf("leveling the album logged %d sync changes, want 1", n) + } + + // 3. Nothing changed: nothing recomputed, nothing logged. if res := pass(); res.Recomputed != 0 { t.Fatalf("idle pass = %+v, want nothing recomputed", res) } + if n := albumChanges(); n != 1 { + t.Fatalf("an idle pass logged album changes (now %d), want still 1", n) + } // 4. A track joins (here, retagged onto this album): recomputed. other, err := q.UpsertAlbum(ctx, dbq.UpsertAlbumParams{Title: "Other", SortTitle: "Other", ArtistID: artist.ID}) @@ -232,9 +257,14 @@ func TestAlbumLoudness_Integration(t *testing.T) { if _, err := pool.Exec(ctx, "UPDATE tracks SET missing_since = now() WHERE id = $1", first.ID); err != nil { t.Fatalf("mark missing: %v", err) } + before := albumChanges() if res := pass(); res.Orphans != 1 { t.Fatalf("pass with every track missing = %+v, want 1 orphan dropped", res) } + // Dropping the row takes the album gain away, so clients are told. + if n := albumChanges(); n != before+1 { + t.Fatalf("dropping the orphan logged %d album changes, want 1", n-before) + } var n int if err := pool.QueryRow(ctx, "SELECT count(*) FROM album_loudness WHERE album_id = $1", album.ID).Scan(&n); err != nil { t.Fatalf("count: %v", err) diff --git a/internal/library/loudness.go b/internal/library/loudness.go index 2fd50032..78893553 100644 --- a/internal/library/loudness.go +++ b/internal/library/loudness.go @@ -17,6 +17,7 @@ import ( "github.com/jackc/pgx/v5/pgtype" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync" ) // Loudness analysis (M464 #4995). @@ -351,10 +352,15 @@ const ( // An inconclusive attempt leaves any existing row alone. The scan deletes a // row when its file changes, so a row still here describes these bytes, and a // value from an older method is a better gain than none until it is redone. +// +// A stored measurement is logged to the sync feed as a track upsert, so +// clients that cache the library (Android) pick up the new gain (#4997). If +// the log fails nothing is stored, and the backfill tries the track again. func storeLoudness( - ctx context.Context, q *dbq.Queries, logger *slog.Logger, + ctx context.Context, db dbq.DBTX, logger *slog.Logger, trackID pgtype.UUID, path string, r loudnessResult, ) loudnessOutcome { + q := dbq.New(db) if r.err != nil { logger.Warn("loudness: analysis failed", "path", path, "err", r.err) if r.inconclusive() { @@ -388,6 +394,14 @@ func storeLoudness( } } } + // Logged before the write, as reconcile does (#2704): a log that then + // finds the write failed costs clients one wasted re-read, but a write + // that lands with no log is never retried, since the track is settled, and + // clients would never learn its gain. + if err := syncpkg.LogChange(ctx, db, syncpkg.EntityTrack, syncpkg.FormatUUID(trackID), syncpkg.OpUpsert); err != nil { + logger.Warn("loudness: LogChange track upsert failed; not storing", "path", path, "err", err) + return loudnessStoreFailed + } if err := q.UpsertTrackLoudness(ctx, params); err != nil { logger.Warn("loudness: storing measurement failed", "path", path, "err", err) return loudnessStoreFailed diff --git a/internal/library/loudness_backfill.go b/internal/library/loudness_backfill.go index 9bf05025..3379c973 100644 --- a/internal/library/loudness_backfill.go +++ b/internal/library/loudness_backfill.go @@ -187,7 +187,7 @@ func (w *LoudnessBackfillWorker) pass(ctx context.Context) (BackfillLoudnessResu w.logger.Error("loudness backfill: track panicked", "path", row.FilePath, "panic", r) } }() - outcome := storeLoudness(ctx, q, w.logger, row.ID, row.FilePath, + outcome := storeLoudness(ctx, w.pool, w.logger, row.ID, row.FilePath, w.analyzeFile(ctx, row.FilePath, row.DurationMs)) mu.Lock() res.add(outcome) diff --git a/internal/library/loudness_backfill_test.go b/internal/library/loudness_backfill_test.go index 4a052bec..88451475 100644 --- a/internal/library/loudness_backfill_test.go +++ b/internal/library/loudness_backfill_test.go @@ -11,6 +11,7 @@ import ( "testing" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync" ) // TestLoudnessBackfill_Integration pins which tracks a pass measures, what it @@ -130,6 +131,18 @@ func TestLoudnessBackfill_Integration(t *testing.T) { gotLUFS, gotPeak, gotHist, gotStart, gotVersion) } + // Each stored measurement told the sync feed, so caching clients re-read + // the track and pick up its gain (#4997). + var logged int + if err := pool.QueryRow(ctx, `SELECT count(*) FROM library_changes + WHERE entity_type = 'track' AND op = 'upsert' AND entity_id = $1`, + syncpkg.FormatUUID(stale.ID)).Scan(&logged); err != nil { + t.Fatalf("count sync changes: %v", err) + } + if logged != 1 { + t.Errorf("re-measuring stale.mp3 logged %d track changes, want 1", logged) + } + // 2. A pass after a complete one is a no-op. res, err = w.pass(ctx) if err != nil { diff --git a/internal/library/replaygain.go b/internal/library/replaygain.go new file mode 100644 index 00000000..8c98ae51 --- /dev/null +++ b/internal/library/replaygain.go @@ -0,0 +1,102 @@ +package library + +import ( + "context" + "math" + + "github.com/jackc/pgx/v5/pgtype" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +// ReplayGain delivery (M464 #4997). +// +// Clients are handed loudness in the ReplayGain 2.0 form every player already +// understands: a gain in dB that brings the track (or album) to the RG2 +// reference of -18 LUFS, and a linear peak. A client aiming at another target +// adds (target - ReplayGainReferenceLUFS) to the gain. The same numbers go to +// the native API, the sync feed and OpenSubsonic's replayGain, so every client +// levels a track identically. + +// ReplayGainReferenceLUFS is the RG2 reference loudness gains are relative to. +const ReplayGainReferenceLUFS = -18.0 + +// ReplayGain is one track's gains. Each field is nil when there is nothing to +// say: not yet measured, silent, or (for the album pair) an album still +// waiting on a track. AlbumPeak travels only with AlbumGain. +type ReplayGain struct { + TrackGain *float32 `json:"track_gain,omitempty"` + TrackPeak *float32 `json:"track_peak,omitempty"` + AlbumGain *float32 `json:"album_gain,omitempty"` + AlbumPeak *float32 `json:"album_peak,omitempty"` +} + +// Empty reports whether no gain is known at all. +func (g ReplayGain) Empty() bool { + return g.TrackGain == nil && g.AlbumGain == nil +} + +// GainDB converts an integrated loudness to its RG2 gain, to 0.01 dB. +func GainDB(lufs *float32) *float32 { + if lufs == nil { + return nil + } + v := float32(math.Round((ReplayGainReferenceLUFS-float64(*lufs))*100) / 100) + return &v +} + +// LinearPeak converts a true peak in dBTP to the linear amplitude ReplayGain +// peaks are written in (1.0 is full scale), to 4 decimals. +func LinearPeak(dbtp *float32) *float32 { + if dbtp == nil { + return nil + } + v := float32(math.Round(math.Pow(10, float64(*dbtp)/20)*10000) / 10000) + return &v +} + +// ReplayGainForTracks returns the gains for each of ids that has any. Tracks +// with none are absent from the map, so a lookup of a missing key yields the +// zero ReplayGain: no adjustment. +func ReplayGainForTracks(ctx context.Context, q *dbq.Queries, ids []pgtype.UUID) (map[pgtype.UUID]ReplayGain, error) { + out := make(map[pgtype.UUID]ReplayGain, len(ids)) + if len(ids) == 0 { + return out, nil + } + rows, err := q.GetReplayGainByTrackIDs(ctx, ids) + if err != nil { + return nil, err + } + for _, r := range rows { + g := ReplayGain{TrackGain: GainDB(r.TrackLufs)} + if g.TrackGain != nil { + g.TrackPeak = LinearPeak(r.TrackPeakDbtp) + } + if g.AlbumGain = GainDB(r.AlbumLufs); g.AlbumGain != nil { + g.AlbumPeak = LinearPeak(r.AlbumPeakDbtp) + } + if !g.Empty() { + out[r.ID] = g + } + } + return out, nil +} + +// ReplayGainForAlbums returns the album pair for each of albumIDs that has an +// album loudness. The track fields are left nil. +func ReplayGainForAlbums(ctx context.Context, q *dbq.Queries, albumIDs []pgtype.UUID) (map[pgtype.UUID]ReplayGain, error) { + out := make(map[pgtype.UUID]ReplayGain, len(albumIDs)) + if len(albumIDs) == 0 { + return out, nil + } + rows, err := q.GetAlbumLoudnessByIDs(ctx, albumIDs) + if err != nil { + return nil, err + } + for _, r := range rows { + if g := GainDB(r.IntegratedLufs); g != nil { + out[r.AlbumID] = ReplayGain{AlbumGain: g, AlbumPeak: LinearPeak(r.TruePeakDbtp)} + } + } + return out, nil +} diff --git a/internal/library/replaygain_test.go b/internal/library/replaygain_test.go new file mode 100644 index 00000000..ae18d735 --- /dev/null +++ b/internal/library/replaygain_test.go @@ -0,0 +1,122 @@ +package library + +import ( + "context" + "math" + "path/filepath" + "testing" + + "github.com/jackc/pgx/v5/pgtype" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +func TestGainDBAndLinearPeak(t *testing.T) { + f := func(v float32) *float32 { return &v } + for _, c := range []struct { + lufs, want float32 + }{ + {-18, 0}, // at the reference: no change + {-9.5, -8.5}, // a loud master is turned down + {-23.456, 5.46}, // a quiet one up, to 0.01 dB + } { + if got := GainDB(f(c.lufs)); got == nil || *got != c.want { + t.Errorf("GainDB(%v) = %v, want %v", c.lufs, got, c.want) + } + } + if GainDB(nil) != nil { + t.Errorf("GainDB(nil) is not nil") + } + for _, c := range []struct { + dbtp, want float32 + }{ + {0, 1}, // full scale + {-6.0206, 0.5}, // half amplitude + {1.2, 1.1482}, // an inter-sample over, above 1.0 + } { + got := LinearPeak(f(c.dbtp)) + if got == nil || math.Abs(float64(*got-c.want)) > 0.0001 { + t.Errorf("LinearPeak(%v) = %v, want %v", c.dbtp, got, c.want) + } + } + if LinearPeak(nil) != nil { + t.Errorf("LinearPeak(nil) is not nil") + } +} + +func TestReplayGainForTracks_Integration(t *testing.T) { + pool := newPool(t) + ctx := context.Background() + q := dbq.New(pool) + dir := t.TempDir() + + measured, album, artist := seedTrack(t, pool, filepath.Join(dir, "measured.mp3")) + add := func(name string) dbq.Track { + t.Helper() + tr, err := q.UpsertTrack(ctx, dbq.UpsertTrackParams{ + Title: name, AlbumID: album.ID, ArtistID: artist.ID, + DurationMs: 1000, FilePath: filepath.Join(dir, name+".mp3"), FileSize: 100, FileFormat: "mp3", + }) + if err != nil { + t.Fatalf("track %s: %v", name, err) + } + return tr + } + older := add("older") + unmeasured := add("unmeasured") + f := func(v float32) *float32 { return &v } + for _, m := range []struct { + tr dbq.Track + lufs float32 + version int16 + }{ + {measured, -12, loudnessVersion}, + // A measurement by an older method is still a better gain than none. + {older, -20, loudnessVersion - 1}, + } { + if err := q.UpsertTrackLoudness(ctx, dbq.UpsertTrackLoudnessParams{ + TrackID: m.tr.ID, IntegratedLufs: f(m.lufs), TruePeakDbtp: f(-1), AnalysisVersion: m.version, + }); err != nil { + t.Fatalf("seed: %v", err) + } + } + + gains, err := ReplayGainForTracks(ctx, q, []pgtype.UUID{measured.ID, older.ID, unmeasured.ID}) + if err != nil { + t.Fatalf("lookup: %v", err) + } + if g := gains[measured.ID]; g.TrackGain == nil || *g.TrackGain != -6 || g.TrackPeak == nil { + t.Errorf("measured track gains = %+v, want track gain -6 with a peak", g) + } + if g := gains[older.ID]; g.TrackGain == nil || *g.TrackGain != 2 { + t.Errorf("older-version track gains = %+v, want track gain 2", g) + } + if _, ok := gains[unmeasured.ID]; ok { + t.Errorf("unmeasured track has an entry; absent means no adjustment") + } + // No album value yet (none computed): the album pair is absent. + if g := gains[measured.ID]; g.AlbumGain != nil || g.AlbumPeak != nil { + t.Errorf("album pair present before album loudness exists: %+v", g) + } + + if err := q.UpsertAlbumLoudness(ctx, dbq.UpsertAlbumLoudnessParams{ + AlbumID: album.ID, IntegratedLufs: f(-14), TruePeakDbtp: f(-0.5), + TracksTotal: 3, TracksSettled: 3, InputsDigest: "x", + }); err != nil { + t.Fatalf("seed album: %v", err) + } + gains, err = ReplayGainForTracks(ctx, q, []pgtype.UUID{unmeasured.ID}) + if err != nil { + t.Fatalf("lookup: %v", err) + } + if g := gains[unmeasured.ID]; g.TrackGain != nil || g.AlbumGain == nil || *g.AlbumGain != -4 || g.AlbumPeak == nil { + t.Errorf("unmeasured track on a leveled album = %+v, want only the album pair (gain -4)", g) + } + byAlbum, err := ReplayGainForAlbums(ctx, q, []pgtype.UUID{album.ID}) + if err != nil { + t.Fatalf("album lookup: %v", err) + } + if g := byAlbum[album.ID]; g.AlbumGain == nil || *g.AlbumGain != -4 || g.TrackGain != nil { + t.Errorf("album gains = %+v, want album gain -4 and no track pair", g) + } +} diff --git a/internal/subsonic/browse.go b/internal/subsonic/browse.go index 70225084..2afb0ed8 100644 --- a/internal/subsonic/browse.go +++ b/internal/subsonic/browse.go @@ -185,9 +185,10 @@ func (b *browseHandlers) getAlbum(w http.ResponseWriter, r *http.Request) { WriteFail(w, r, ErrGeneric, "Failed to load tracks") return } + gains := replayGains(r.Context(), q, tracks) songs := make([]SongRef, 0, len(tracks)) for _, t := range tracks { - songs = append(songs, songRef(t, album.Title, artist.Name)) + songs = append(songs, songRef(t, album.Title, artist.Name, gains[t.ID])) } Write(w, r, AlbumResponse{ Envelope: NewEnvelope("ok"), @@ -230,7 +231,7 @@ func (b *browseHandlers) getSong(w http.ResponseWriter, r *http.Request) { } Write(w, r, SongResponse{ Envelope: NewEnvelope("ok"), - Song: songRef(track, album.Title, artist.Name), + Song: songRef(track, album.Title, artist.Name, replayGains(r.Context(), q, []dbq.Track{track})[track.ID]), }) } @@ -386,6 +387,7 @@ func (b *browseHandlers) search3(w http.ResponseWriter, r *http.Request) { WriteFail(w, r, ErrGeneric, "Song search failed") return } + gains := replayGains(r.Context(), q, tracks) for _, t := range tracks { album, aerr := q.GetAlbumByID(r.Context(), t.AlbumID) if aerr != nil { @@ -397,7 +399,7 @@ func (b *browseHandlers) search3(w http.ResponseWriter, r *http.Request) { WriteFail(w, r, ErrGeneric, "Song search failed") return } - result.Songs = append(result.Songs, songRef(t, album.Title, artist.Name)) + result.Songs = append(result.Songs, songRef(t, album.Title, artist.Name, gains[t.ID])) } } diff --git a/internal/subsonic/replaygain_test.go b/internal/subsonic/replaygain_test.go new file mode 100644 index 00000000..774093c5 --- /dev/null +++ b/internal/subsonic/replaygain_test.go @@ -0,0 +1,58 @@ +package subsonic + +import ( + "encoding/json" + "encoding/xml" + "strings" + "testing" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/library" +) + +// OpenSubsonic's replayGain is an object in JSON and an element with +// attributes in XML. Clients parse both, so both are pinned (#4997). +func TestSongRef_ReplayGainEncodesForBothFormats(t *testing.T) { + gain, peak := float32(-4.5), float32(0.8913) + tr := dbq.Track{Title: "Song", FilePath: "/m/a.flac", FileFormat: "flac"} + s := songRef(tr, "Album", "Artist", library.ReplayGain{TrackGain: &gain, TrackPeak: &peak}) + + b, err := json.Marshal(s) + if err != nil { + t.Fatal(err) + } + var m map[string]any + if err := json.Unmarshal(b, &m); err != nil { + t.Fatal(err) + } + rg, ok := m["replayGain"].(map[string]any) + if !ok { + t.Fatalf("JSON has no replayGain object: %s", b) + } + if rg["trackGain"] != -4.5 || rg["trackPeak"] == nil { + t.Errorf("JSON replayGain = %v, want trackGain -4.5 and a trackPeak", rg) + } + if _, ok := rg["albumGain"]; ok { + t.Errorf("JSON replayGain carries albumGain with no album value: %v", rg) + } + + x, err := xml.Marshal(s) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(x), ``) { + t.Errorf("XML lacks the replayGain element: %s", x) + } +} + +func TestSongRef_NoReplayGainWhenUnmeasured(t *testing.T) { + s := songRef(dbq.Track{Title: "Song", FilePath: "/m/a.mp3"}, "Album", "Artist", library.ReplayGain{}) + if s.ReplayGain != nil { + t.Fatalf("unmeasured song carries replayGain %+v", s.ReplayGain) + } + b, _ := json.Marshal(s) + x, _ := xml.Marshal(s) + if strings.Contains(string(b), "replayGain") || strings.Contains(string(x), "replayGain") { + t.Errorf("unmeasured song encodes replayGain: %s / %s", b, x) + } +} diff --git a/internal/subsonic/star.go b/internal/subsonic/star.go index b2b333f5..7d717a3a 100644 --- a/internal/subsonic/star.go +++ b/internal/subsonic/star.go @@ -191,6 +191,7 @@ func loadStarred(ctx context.Context, q *dbq.Queries, userID pgtype.UUID) ([]Son if err != nil { return nil, nil, nil, err } + gains := replayGains(ctx, q, trackRows) for _, t := range trackRows { album, err := q.GetAlbumByID(ctx, t.AlbumID) if err != nil { @@ -200,7 +201,7 @@ func loadStarred(ctx context.Context, q *dbq.Queries, userID pgtype.UUID) ([]Son if err != nil { return nil, nil, nil, err } - songs = append(songs, songRef(t, album.Title, artist.Name)) + songs = append(songs, songRef(t, album.Title, artist.Name, gains[t.ID])) } albumRows, err := q.ListLikedAlbumRows(ctx, dbq.ListLikedAlbumRowsParams{ diff --git a/internal/subsonic/types.go b/internal/subsonic/types.go index 19ffb63d..2414fe60 100644 --- a/internal/subsonic/types.go +++ b/internal/subsonic/types.go @@ -9,6 +9,7 @@ import ( "github.com/jackc/pgx/v5/pgtype" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/library" ) // IDs on the wire are the bare UUID string. Subsonic endpoints know from @@ -180,6 +181,20 @@ type SongRef struct { BitRate int `json:"bitRate,omitempty" xml:"bitRate,attr,omitempty"` IsDir bool `json:"isDir" xml:"isDir,attr"` Type string `json:"type" xml:"type,attr"` + // ReplayGain is OpenSubsonic's replayGain (M464 #4997): Minstrel's own + // measurements, so third-party clients that level by ReplayGain do it + // from the same numbers the Minstrel apps use. Omitted until measured. + ReplayGain *ReplayGain `json:"replayGain,omitempty" xml:"replayGain,omitempty"` +} + +// ReplayGain is the OpenSubsonic replayGain object: gains in dB to the +// -18 LUFS ReplayGain 2.0 reference, peaks as linear amplitude. A field with +// no value is omitted. +type ReplayGain struct { + TrackGain *float32 `json:"trackGain,omitempty" xml:"trackGain,attr,omitempty"` + AlbumGain *float32 `json:"albumGain,omitempty" xml:"albumGain,attr,omitempty"` + TrackPeak *float32 `json:"trackPeak,omitempty" xml:"trackPeak,attr,omitempty"` + AlbumPeak *float32 `json:"albumPeak,omitempty" xml:"albumPeak,attr,omitempty"` } type SongResponse struct { @@ -284,7 +299,10 @@ func albumDetail(a dbq.Album, artistName string, songs []SongRef) AlbumDetail { } } -func songRef(t dbq.Track, albumTitle, artistName string) SongRef { +// songRef takes the track's gains as an argument rather than looking them up, +// so no response that lists songs can leave them out by forgetting a step: +// each caller fetches them once for its whole list (replayGains). +func songRef(t dbq.Track, albumTitle, artistName string, g library.ReplayGain) SongRef { s := SongRef{ ID: uuidToID(t.ID), Parent: uuidToID(t.AlbumID), @@ -313,9 +331,30 @@ func songRef(t dbq.Track, albumTitle, artistName string) SongRef { if t.Genre != nil { s.Genre = *t.Genre } + if !g.Empty() { + s.ReplayGain = &ReplayGain{ + TrackGain: g.TrackGain, AlbumGain: g.AlbumGain, + TrackPeak: g.TrackPeak, AlbumPeak: g.AlbumPeak, + } + } return s } +// replayGains fetches the gains for a list of tracks in one query. A failed +// lookup yields no gains rather than failing the response: the songs still +// play, just unleveled, which is how they played before gains existed. +func replayGains(ctx context.Context, q *dbq.Queries, tracks []dbq.Track) map[pgtype.UUID]library.ReplayGain { + ids := make([]pgtype.UUID, len(tracks)) + for i, t := range tracks { + ids[i] = t.ID + } + gains, err := library.ReplayGainForTracks(ctx, q, ids) + if err != nil { + return map[pgtype.UUID]library.ReplayGain{} + } + return gains +} + // coverArtID returns the album UUID as the cover-art key. getCoverArt uses // the album row to find art either in cover_art_path (when the scanner sets // it) or via sidecar lookup in the album directory, so emitting the id