perf: allow request bursts, reuse album track metadata; feat: qobuz lyrics

Two related changes: a 68-track Qobuz album was gated by the API throttle
rather than the network, and Qobuz lyrics are now supported, which is what the
faster album path had to be designed around.

--- perf ---

ratelimit: token bucket instead of fixed spacing
  The limiter spaced every request exactly interval apart with no burst, so at
  the default 60 requests/minute it allowed exactly one request per second no
  matter how many workers ran. Measured: 8 requests took 7s with 1 worker and
  7s with 6, i.e. max_connections had no effect on anything touching the API.
  Since a track needs its metadata and stream URL before downloading, workers
  could only start a track every two seconds, which is why an album looked
  serial even at 57 MiB/s per track.

  New() now takes a burst (providers pass max_connections), idle time accrues
  budget up to that burst, and burst is clamped to one minute's worth of
  requests so a large max_connections cannot bypass a small configured rate.

app: reuse the track objects already in the album payload
  ripAlbum fetched the album, kept only the track ids, then asked the provider
  for every track again, doubling the request count. Providers opt in via
  AlbumTracksAreComplete(); only Qobuz does, because it is only verified there
  and because Tidal does extra work in its track lookup.

  Verified against the live Qobuz API on a 68-track release: the embedded track
  objects already carry title, version, track_number, media_number,
  parental_warning, isrc, copyright, performer, performers, composer, duration,
  audio_info, maximum_bit_depth, maximum_sampling_rate and
  release_date_original. The only field a per-track lookup adds that this app
  reads is the nested album object, so albumTrackMetadata attaches the parent
  album with its own track list stripped, or a 68-track album would hold 68
  copies of itself. trackMetaUsable refetches anyway when title or track number
  is missing, so a thin payload cannot produce a wrongly named file.

  Measured end-to-end through the real app and limiter at the shipped defaults,
  68 tracks: 137 API calls / 2m11s before, 69 calls / 1m3s after.

--- qobuz lyrics ---

Qobuz's main API does not serve lyrics: has_lyrics is only a flag, extra=lyrics
adds nothing, and track/getLyrics does not exist. Its apps use a two-step flow
instead, confirmed live:

  GET track/lyricsUrl?track_id=&app_id=&request_ts=&request_sig=
    -> {"track_id","album_id","lyrics_url"}      (signed CloudFront URL)
  GET <lyrics_url>
    -> {"original":{"type","lang","lines":[{"line","start","end"}]},
        "translation_langs":[...],"writers","publishers":[...]}

type is plain, lsync or wsync; start/end are milliseconds; the line text is
under "line". Passing a language returns a translation instead of the original,
and only for languages that exist for the track (en 403s on an English song),
so the language parameter is deliberately omitted. A 403 on the signed URL is
how Qobuz represents "no lyrics", so it is not treated as an error.

Lyrics are exposed through a TrackLyrics method rather than inside GetMetadata,
because album rips skip the per-track metadata call. That keeps the fast path:
with lyrics on, an album costs the same two provider calls per track it always
did, and the lyrics document itself comes from a CDN that does not consume the
provider's request budget. With metadata.exclude = ["lyrics"] it drops to one
call per track and the full speedup applies.

Existing behaviour is preserved: Tidal and Deezer already attach lyrics in
their metadata, so attachLyrics leaves them alone, and a lyrics failure is
logged and ignored rather than failing a download.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
JorenandClaude Opus 5 committed 2026-10-03 21:55:48 +02:00
1 parent 26fb466afc
commit da4add57fe
13 files changed
+1029 -31

No files matched your search

+402
View File
@@ -1138,3 +1138,405 @@ func TestBookkeepingSurvivesCancellation(t *testing.T) {
t.Fatal("failure record not written under a cancelled context")
}
}
// countingMetaProvider records how many per-track metadata lookups happen, so
// the album-payload optimization can be asserted rather than assumed.
type countingMetaProvider struct {
url string
complete bool
mu sync.Mutex
trackMeta int
albumMeta int
downloads int
}
func (f *countingMetaProvider) Source() string { return "qobuz" }
func (f *countingMetaProvider) Login(context.Context) error { return nil }
func (f *countingMetaProvider) LoggedIn() bool { return true }
func (f *countingMetaProvider) Close() error { return nil }
func (f *countingMetaProvider) Search(context.Context, string, string, int) ([]map[string]any, error) {
return nil, nil
}
// AlbumTracksAreComplete is only consulted when complete is true; the false
// case still satisfies the interface, which mirrors a provider that opts out.
func (f *countingMetaProvider) AlbumTracksAreComplete() bool { return f.complete }
func (f *countingMetaProvider) GetMetadata(_ context.Context, id string, mediaType string) (map[string]any, error) {
f.mu.Lock()
if mediaType == "album" {
f.albumMeta++
} else {
f.trackMeta++
}
f.mu.Unlock()
if mediaType == "album" {
items := make([]any, 0, 3)
for i := 1; i <= 3; i++ {
items = append(items, map[string]any{
"id": fmt.Sprintf("t%d", i),
"title": fmt.Sprintf("Track %d", i),
"track_number": float64(i),
"media_number": float64(1),
"isrc": fmt.Sprintf("ISRC%d", i),
"copyright": "(C) Label",
"performer": map[string]any{"name": "Fleetwood Mac"},
})
}
return map[string]any{
"title": "Rumours",
"release_date_original": "1977-02-04",
"artist": map[string]any{"name": "Fleetwood Mac"},
"tracks": map[string]any{"items": items},
}, nil
}
return map[string]any{
"title": "Fetched " + id,
"track_number": float64(1),
"performer": map[string]any{"name": "Fleetwood Mac"},
"album": map[string]any{"title": "Rumours"},
}, nil
}
func (f *countingMetaProvider) GetDownloadable(_ context.Context, _ string, _ int) (*provider.Downloadable, error) {
f.mu.Lock()
f.downloads++
f.mu.Unlock()
return &provider.Downloadable{URL: f.url, Extension: "flac", Source: "qobuz"}, nil
}
func (f *countingMetaProvider) counts() (album, track, downloads int) {
f.mu.Lock()
defer f.mu.Unlock()
return f.albumMeta, f.trackMeta, f.downloads
}
func runAlbumRip(t *testing.T, prov *countingMetaProvider) string {
t.Helper()
tmp := t.TempDir()
d := config.DefaultConfigData()
d.Downloads.Folder = tmp
d.Downloads.SourceSubdirectories = false
d.Downloads.Concurrency = false
d.Artwork.Embed = false
d.Artwork.SaveArtwork = false
cfg := &config.Config{File: d, Session: d}
m := &Main{
Config: cfg,
Providers: map[string]provider.Client{"qobuz": prov},
Store: store.NewDummy(),
DL: download.New(),
Tagger: noopTagger{},
}
ctx := context.Background()
if err := m.AddByID(ctx, "qobuz", "album", "a1"); err != nil {
t.Fatalf("AddByID() error = %v", err)
}
if err := m.Resolve(ctx); err != nil {
t.Fatalf("Resolve() error = %v", err)
}
if err := m.Rip(ctx); err != nil {
t.Fatalf("Rip() error = %v", err)
}
return tmp
}
// TestAlbumRipSkipsPerTrackMetadataWhenEmbedded is the performance regression
// test: a provider that embeds complete track objects in its album payload
// must not be asked for each track again. Every such call goes through the
// requests-per-minute limiter, so on a 68-track album the redundant lookups
// were ~68 seconds of pure waiting.
func TestAlbumRipSkipsPerTrackMetadataWhenEmbedded(t *testing.T) {
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
_, _ = w.Write([]byte("fLaC"))
}))
defer ts.Close()
prov := &countingMetaProvider{url: ts.URL, complete: true}
tmp := runAlbumRip(t, prov)
album, track, _ := prov.counts()
if album != 1 {
t.Errorf("album metadata fetches = %d, want 1", album)
}
if track != 0 {
t.Errorf("per-track metadata fetches = %d, want 0 (album payload is complete)", track)
}
// The embedded titles must be what lands on disk, not the fetched ones.
albumDir := soleSubdir(t, tmp)
for i := 1; i <= 3; i++ {
name := fmt.Sprintf("%02d. Fleetwood Mac - Track %d.flac", i, i)
if _, err := os.Stat(filepath.Join(albumDir, name)); err != nil {
entries, _ := os.ReadDir(albumDir)
var got []string
for _, e := range entries {
got = append(got, e.Name())
}
t.Fatalf("expected %q from the embedded metadata; album dir contains %v", name, got)
}
}
}
// TestAlbumRipFetchesPerTrackMetadataWhenNotEmbedded pins the opt-out path, so
// providers like Tidal (whose track lookup also fetches lyrics) keep working.
func TestAlbumRipFetchesPerTrackMetadataWhenNotEmbedded(t *testing.T) {
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
_, _ = w.Write([]byte("fLaC"))
}))
defer ts.Close()
prov := &countingMetaProvider{url: ts.URL, complete: false}
runAlbumRip(t, prov)
_, track, _ := prov.counts()
if track != 3 {
t.Errorf("per-track metadata fetches = %d, want 3 (provider opted out)", track)
}
}
// TestAlbumTrackMetadataAttachesParentAlbum covers the one field the live
// Qobuz check showed track/get adds: the nested album object.
func TestAlbumTrackMetadataAttachesParentAlbum(t *testing.T) {
albumMeta := map[string]any{
"title": "Rumours",
"replaygain_album_gain": -7.5,
"tracks": map[string]any{"items": []any{
map[string]any{"id": "t1", "title": "Dreams", "track_number": float64(1)},
}},
}
items, _ := anySlice(albumMeta["tracks"].(map[string]any)["items"])
got := albumTrackMetadata(albumMeta, items)
meta, ok := got["t1"]
if !ok {
t.Fatal("no metadata built for t1")
}
alb, ok := meta["album"].(map[string]any)
if !ok {
t.Fatal("parent album not attached")
}
if alb["title"] != "Rumours" {
t.Errorf("album title = %v", alb["title"])
}
// Album-level ReplayGain is read through trackMeta["album"].
if alb["replaygain_album_gain"] != -7.5 {
t.Errorf("album replaygain lost: %v", alb["replaygain_album_gain"])
}
// The attached album must not carry the whole track list back in.
if _, has := alb["tracks"]; has {
t.Error("attached album still has its track list; that is O(n^2) memory on a large album")
}
}
func TestTrackMetaUsable(t *testing.T) {
tests := []struct {
name string
meta map[string]any
want bool
}{
{"nil", nil, false},
{"empty", map[string]any{}, false},
{"complete", map[string]any{"title": "A", "track_number": float64(1)}, true},
{"no title", map[string]any{"track_number": float64(1)}, false},
{"blank title", map[string]any{"title": " ", "track_number": float64(1)}, false},
{"no track number", map[string]any{"title": "A"}, false},
{"zero track number", map[string]any{"title": "A", "track_number": float64(0)}, false},
{"string track number", map[string]any{"title": "A", "track_number": "2"}, true},
}
for _, tc := range tests {
if got := trackMetaUsable(tc.meta); got != tc.want {
t.Errorf("%s: trackMetaUsable = %v, want %v", tc.name, got, tc.want)
}
}
}
// soleSubdir returns the single directory inside dir, which for an album rip
// is the generated album folder.
func soleSubdir(t *testing.T, dir string) string {
t.Helper()
entries, err := os.ReadDir(dir)
if err != nil {
t.Fatal(err)
}
var dirs []string
for _, e := range entries {
if e.IsDir() {
dirs = append(dirs, filepath.Join(dir, e.Name()))
}
}
if len(dirs) != 1 {
t.Fatalf("expected exactly one album folder in %s, got %v", dir, dirs)
}
return dirs[0]
}
// lyricsProvider is a countingMetaProvider that also serves lyrics, to prove
// lyrics still reach the tagger on the album fast path, where the per-track
// metadata call is skipped.
type lyricsProvider struct {
*countingMetaProvider
plain string
synced string
calls int
err error
}
func (f *lyricsProvider) TrackLyrics(context.Context, string) (string, string, error) {
f.mu.Lock()
f.calls++
f.mu.Unlock()
if f.err != nil {
return "", "", f.err
}
return f.plain, f.synced, nil
}
type capturingTagger struct {
mu sync.Mutex
seen []tag.Metadata
}
func (c *capturingTagger) TagFLAC(_ context.Context, _ string, meta tag.Metadata, _ string) error {
c.mu.Lock()
defer c.mu.Unlock()
c.seen = append(c.seen, meta)
return nil
}
func (c *capturingTagger) lyrics() []string {
c.mu.Lock()
defer c.mu.Unlock()
out := make([]string, 0, len(c.seen))
for _, m := range c.seen {
out = append(out, m.Lyrics)
}
return out
}
func runAlbumRipWithTagger(t *testing.T, prov provider.Client, tagger trackTagger, exclude ...string) {
t.Helper()
d := config.DefaultConfigData()
d.Downloads.Folder = t.TempDir()
d.Downloads.Concurrency = false
d.Artwork.Embed = false
d.Artwork.SaveArtwork = false
d.Metadata.Exclude = exclude
cfg := &config.Config{File: d, Session: d}
m := &Main{
Config: cfg,
Providers: map[string]provider.Client{"qobuz": prov},
Store: store.NewDummy(),
DL: download.New(),
Tagger: tagger,
}
ctx := context.Background()
if err := m.AddByID(ctx, "qobuz", "album", "a1"); err != nil {
t.Fatalf("AddByID() error = %v", err)
}
if err := m.Resolve(ctx); err != nil {
t.Fatalf("Resolve() error = %v", err)
}
if err := m.Rip(ctx); err != nil {
t.Fatalf("Rip() error = %v", err)
}
}
func TestAlbumRipAttachesLyricsOnFastPath(t *testing.T) {
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
_, _ = w.Write([]byte("fLaC"))
}))
defer ts.Close()
prov := &lyricsProvider{
countingMetaProvider: &countingMetaProvider{url: ts.URL, complete: true},
plain: "line one\nline two",
synced: "[00:01.00]line one",
}
tagger := &capturingTagger{}
runAlbumRipWithTagger(t, prov, tagger)
_, trackMeta, _ := prov.counts()
if trackMeta != 0 {
t.Errorf("per-track metadata fetches = %d, want 0 (fast path must be preserved)", trackMeta)
}
if prov.calls != 3 {
t.Errorf("TrackLyrics calls = %d, want 3 (one per track)", prov.calls)
}
got := tagger.lyrics()
if len(got) != 3 {
t.Fatalf("tagged %d tracks, want 3", len(got))
}
for i, l := range got {
// Synced lyrics win over plain, matching the existing behaviour.
if l != "[00:01.00]line one" {
t.Errorf("track %d lyrics = %q", i, l)
}
}
}
func TestAlbumRipSkipsLyricsWhenExcluded(t *testing.T) {
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
_, _ = w.Write([]byte("fLaC"))
}))
defer ts.Close()
prov := &lyricsProvider{
countingMetaProvider: &countingMetaProvider{url: ts.URL, complete: true},
plain: "should not be fetched",
}
runAlbumRipWithTagger(t, prov, noopTagger{}, "lyrics")
if prov.calls != 0 {
t.Errorf("TrackLyrics calls = %d, want 0 when metadata.exclude contains lyrics", prov.calls)
}
}
// A lyrics failure is cosmetic and must never fail the download.
func TestAlbumRipSurvivesLyricsFailure(t *testing.T) {
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
_, _ = w.Write([]byte("fLaC"))
}))
defer ts.Close()
prov := &lyricsProvider{
countingMetaProvider: &countingMetaProvider{url: ts.URL, complete: true},
err: errors.New("lyrics service down"),
}
tagger := &capturingTagger{}
runAlbumRipWithTagger(t, prov, tagger)
if n := len(tagger.lyrics()); n != 3 {
t.Fatalf("tagged %d tracks, want 3 despite the lyrics failure", n)
}
for i, l := range tagger.lyrics() {
if l != "" {
t.Errorf("track %d should have no lyrics, got %q", i, l)
}
}
}
// Providers that already attach lyrics in their metadata (Tidal, Deezer) must
// not be asked again.
func TestLyricsNotRefetchedWhenMetadataAlreadyHasThem(t *testing.T) {
d := config.DefaultConfigData()
m := &Main{Config: &config.Config{File: d, Session: d}, DL: download.New()}
prov := &lyricsProvider{countingMetaProvider: &countingMetaProvider{}, plain: "fresh"}
meta := map[string]any{"lyrics_synced": "[00:00.00]already here"}
m.attachLyrics(context.Background(), prov, meta, "t1")
if prov.calls != 0 {
t.Errorf("TrackLyrics calls = %d, want 0 when metadata already carries lyrics", prov.calls)
}
if meta["lyrics_synced"] != "[00:00.00]already here" {
t.Errorf("existing lyrics overwritten: %v", meta["lyrics_synced"])
}
}