Commit Graph
33 Commits
Author SHA1 Message Date
JorenandClaude Opus 5 da4add57fe 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>
2026-10-03 21:55:48 +02:00
JorenandClaude Opus 5 b5d4d5688d refactor: consolidate the duplicated download and worker-pool code
internal/download
  The ~40-line progress-bar decorator stack was copy-pasted into three download
  paths and had started to drift; it is now newDownloadBar, which returns a nil
  bar when progress is off (so callers use it unconditionally) plus a finish
  func to defer. The three hand-rolled read/write loops that differed only in
  whether they touched a bar become copyWithProgress.

  FileDeezerEncrypted and FileYandexEncrypted were ~90% identical: mkdir, GET,
  status check, create output, bar, cleanup-on-failure, Content-Length check,
  fsync. That scaffolding is now streamEncrypted, and each function supplies
  only its decryption step. Also switches the io.EOF/io.ErrUnexpectedEOF
  comparisons to errors.Is.

internal/app
  Three copies of "coerce a JSON array that may be []any or []map[string]any"
  become anySlice, which also reports whether the value was an array at all so
  callers can still distinguish missing from empty.

  The worker-pool dispatch existed three times and the copies disagreed: two
  counted failures under a mutex and logged inline with the track id, while the
  third collected errors in a channel and logged them all after wg.Wait()
  without the id. runTrackWorkers unifies them on the inline form, which
  reports failures as they happen and names the track, and keeps the mutex held
  across the log so failure lines cannot interleave.

Tests
  The progress-bar paths had no coverage at all, because progress is disabled
  when not attached to a terminal; four new tests force it on via the existing
  STREAMRIP_GO_FORCE_PROGRESS escape hatch and cover the determinate bar, the
  unknown-length spinner, Deezer decryption with a bar attached, and the
  failure path still removing the partial file. internal/download coverage
  53% -> 63%.

  Three cancellation tests pin the Ctrl-C fix. Verified as genuine regression
  tests by reverting the guards: without them the rip walks all 40 tracks of a
  cancelled album, logs a failure for each, and Rip() returns nil.

Adds store.SQLite.IsFailed, the read complement to MarkFailed, used by the
bookkeeping test to confirm failure records still land under a cancelled
context.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-10-03 17:23:24 +02:00
JorenandClaude Opus 5 384f32cd67 refactor: move vendor app credentials and signing keys into config
The provider apps' own identifiers and protocol keys were scattered as
constants across six packages. They are not user secrets and stay baked in so
the tool still works out of the box, but they are now mirrored into the config
so that when a provider rotates one, a user can fix their install with a config
edit instead of waiting for a release.

Moved, each with the current value as the default in internal/config:
  qobuz     mobile_app_id, mobile_app_secret
  tidal     app_client_id, app_client_secret
  deezer    api_key, gateway_key, blowfish_secret
  yandex    sign_key, legacy_mp3_sign_salt
  beatport  app_client_id

applyVendorDefaults backfills any empty value, so configs written before these
keys existed keep working and the clients never have to nil-check them. Each
client reads through a small accessor; the package-level signing helpers
(mobileSignRequest, yandexDownloadSign, legacyDirectURL, deriveGatewayAuthToken
and friends, deriveDeezerBlowfishKey) now take the key as a parameter instead
of closing over a constant. The blowfish secret reaches internal/download
through a field on Downloader set by app.New.

The blowfish key derivation XORs 16 bytes of the secret, so a mistyped override
would have panicked; a wrong-length value is now rejected with a warning and
the built-in value is used. Tests pin the length and that the qobuz mobile
secret is valid hex, since the HKDF step requires it.

Also drops soundcloud.client_id and soundcloud.app_version: soundcloud goes
through yt-dlp, which supplies its own, so nothing read them.

config.toml.example documents each new key as "leave empty unless the vendor
rotates theirs".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-10-03 17:11:41 +02:00
JorenandClaude Opus 5 b3b6a2eb1d refactor: delete dead code, dedupe helpers, add tests for untested packages
Dead code
  Removed, each verified to have no non-test caller: promptSearchSelection
  (77 lines, superseded by promptSearchSelectionMenu), AddPlaylistByTrackIDs,
  download.NewWithVerifySSL, qobuz.Client.GetTrackMetadata,
  tidal.Client.trackSupportsAtmos (test-only, and the comment above
  GetDownloadable already documented its removal from the live path) and its
  now-orphaned test, yandex.canonicalTrackRequestID (a no-op returning its
  trimmed input; the call site uses strings.TrimSpace directly) and the unused
  defaultEstimatedKbps constant.

Config keys that did nothing
  qobuz.download_booklets and misc.check_for_updates were declared, defaulted
  and documented in config.toml.example but read by no code. The whole
  [youtube] block went too: there is no YouTube provider. go-toml ignores
  unknown keys, verified, so existing user configs still load.

Duplicated helpers
  tidal.stringify and yandex.firstNonEmpty were byte-identical copies of
  jsonutil.StringFromAny and jsonutil.FirstNonEmpty. displayArtistNames
  existed three times, byte-identical, in internal/app, provider/yandex and
  provider/beatport; it now lives in jsonutil. The CLI's normalizeCodec
  delegates to config.NormalizeCodec so the flag and the config file are
  checked by the same rules.

Tests for packages that had none
  internal/jsonutil (0% -> 100%) and internal/netutil (0% -> 78%), the latter
  covering redactURL, which is a security control, and the zero-timeout
  contract that internal/download and the qobuz body copy depend on.
  internal/config rose to 82% with coverage of the new Validate(), the
  0600 permissions, CloneConfigData not sharing slices, and a concurrent
  UpdateTokens test that exercises the mutex under -race.

Test suite is ~85x faster
  Provider tests built clients with the default 60 requests/minute, so the
  rate limiter slept 1s per HTTP call: deezer took 31s, tidal 11s, qobuz 8s.
  Each provider test package now builds its config through a testConfigData
  helper that disables throttling. Whole suite: ~60s -> 0.7s, and 2.8s
  under -race.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-10-03 17:05:55 +02:00
JorenandClaude Opus 5 c030d1be09 fix: resource leaks, cancellation, races and path traversal
Six correctness bugs that the existing tests could not see. The suite passed
and `go test -race` was clean throughout, because nothing exercised concurrent
token refresh, shared-folder artwork, or hostile metadata.

main(): extract run() int
  main() was a single 1196-line function that registered 20
  `defer mainApp.Close()` handlers and then called os.Exit 150 times, so none
  of them ever ran. Close() is what closes the SQLite handle, flushes the
  progress bars and removes the artwork temp dirs, so every failed rip leaked
  temp directories. main() is now a wrapper around run() int and the command
  bodies return exit codes.

Ctrl-C now stops a rip
  ctx was threaded everywhere but never checked: zero ctx.Err()/ctx.Done()
  in internal/app and internal/download. Cancellation left Rip() iterating
  the whole remaining queue, and the retry ladder retried on a dead context,
  so one Ctrl-C cost three doomed attempts per remaining track. Adds checks to
  the dispatch loops, the worker-spawn loops and the retry ladder, and routes
  the MarkFailed/MarkDownloaded bookkeeping through a detached context so the
  records survive cancellation. Convert and TagFLAC now take a ctx and use
  exec.CommandContext, so a stalled ffmpeg/ffprobe no longer outlives Ctrl-C.

Config race
  Provider token refresh mutated the shared *config.Config and called
  SaveFile() from inside download workers with no lock, racing the struct
  fields and issuing concurrent os.WriteFile against one path. Adds a mutex to
  config.Config plus UpdateTokens(), and routes all five refresh sites through
  it. Each provider client gets an authMu guarding loggedIn and its
  login/refresh path; beatport needed it most, since ensureToken is reached
  from apiRequest.

Artwork cover race
  artwork.Prepare was check-then-write on a path shared by every worker in a
  playlist rip. Two workers both saw a missing cover.jpg, both created it, and
  the loser's cleanup deleted the winner's finished file. Adds a per-folder
  lock and downloads via a unique temp file + rename. The regression test
  fails against the old code with 15 of 16 workers getting no cover.

Path traversal from remote metadata
  naming.CleanName stripped <>:"/\|?* and control chars but not "." , so a
  title of ".." survived into filepath.Join and escaped the download root.
  SoundCloud and Tidal playlist names are user-generated. Now rejects "." and
  "..", trims trailing dots, handles Windows device names, adds DEL to the
  invalid set and caps components at 255 bytes without splitting a rune.
  app.go gains safeJoin as a second line of defence.

Credentials no longer world-readable
  The config holds plaintext provider passwords, Deezer ARL cookies and OAuth
  tokens and was written 0o644. Now 0o600, with 0o700 directories, and an
  existing 0o644 file is tightened on write.

Also: deletes the 9 duplicated *-rip-smoke dev commands (~300 lines fully
subsumed by `rip id`) and the unreachable second dev-help arm; registers
image/png so PNG covers are downscaled instead of being silently saved as
cover.jpg containing PNG bytes; adds the missing .gitignore, which was keeping
a 21MB build artifact one `git add -A` away from the repo.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-10-03 16:52:58 +02:00
Joren eb7854bac3 fix: use all album artists for folder naming 2026-07-22 22:34:42 +02:00
Joren e336bb96f1 fix: set album track totals 2026-07-12 03:28:27 +02:00
Joren 150b5b5d85 fix: omit original mix from titles 2026-07-11 23:43:03 +02:00
Joren 3413de1daa feat: format multi-artist credits 2026-07-11 04:44:54 +02:00
Joren 537959b6ec feat: add beatport provider 2026-07-10 19:21:14 +02:00
Joren 2a7d259e9f Merge branch 'feat/yandex-music'
# Conflicts:
#	internal/audio/tag/tagger.go
#	internal/audio/tag/tagger_test.go
#	internal/urlparse/parse_test.go
2026-07-10 18:18:04 +02:00
Joren b65edb4cce yes 2026-06-15 17:18:47 +02:00
Joren 0ae8c7e008 feat yandex desktop downloads 2026-06-10 12:58:04 +02:00
Joren fa39582849 fix tidal playlist metadata and retries 2026-05-21 23:03:27 +02:00
lb-a 3bc965db77 Merge branch 'feat/verbose' 2026-05-07 23:56:42 +02:00
lb-a 04cc56040b feat: add log hooks 2026-05-07 18:45:05 +02:00
Joren 59b476034e fix qobuz EOF downloads with mobile fallback flow 2026-05-01 17:23:52 +02:00
lb-a 9618108f2a performance: exclude lyrics if they are not enabled 2026-04-30 12:00:58 +02:00
lb-a 945695cea7 performance: tune shared HTTP transport for concurrent CDN downloads 2026-04-29 23:53:51 +02:00
Joren 232901f3eb unify folder naming with resolved audio profiles across providers 2026-04-24 00:35:38 +02:00
Joren 6bc4b3b319 Refactor: comprehensive cleanup and modularization
- Extracted common JSON parsing helpers into internal/jsonutil
- Removed duplicated helper functions from provider packages
- Removed dead code in internal/app/app.go and downloader.go
- Replaced deprecated strings.Title with jsonutil.TitleCase
- Added graceful shutdown with signal handling in main.go
- Split monolithic cmd/rip/main.go into args.go, helpers.go, lastfm.go, search.go
2026-04-21 23:38:41 +02:00
Joren d65dc182f8 harden ffmpeg pipeline failure handling and stream mapping 2026-04-21 23:07:48 +02:00
Joren 654bed17e2 harden deezer auth errors and mixed playlist preflight 2026-04-21 12:19:23 +02:00
Joren 1246a24749 map deezer gain to replaygain track metadata 2026-04-21 11:51:17 +02:00
Joren 9ebddc8316 harden deezer auth and lyrics tagging behavior 2026-04-21 11:14:57 +02:00
Joren 26c9d50fac implement native Deezer download/decrypt pipeline
Replace Deezer yt-dlp usage with native ARL session + media.get_url resolution, add BF_CBC_STRIPE decryption in downloader, and wire cipher-aware Deezer downloads through the main rip pipeline. Includes validation hardening and metadata/source-id improvements used by tagging flows.
2026-04-21 00:48:07 +02:00
Joren 0ba8faa943 improve CLI error semantics and soundcloud canonicalization
Auto-upgrade outdated configs on startup, add actionable SSL verification hints in rip error paths, and harden SoundCloud search/metadata with canonical URL handling and richer source IDs.
2026-04-20 15:16:59 +02:00
Joren 0748d5a325 improve lastfm playlist parity and fallback resolution
Queue resolved last.fm tracks as playlist media to preserve playlist semantics, and add a robust mirror-based parser fallback for last.fm temporary unavailable responses while keeping track order and duplicates.
2026-04-20 02:01:16 +02:00
Joren b5a5368fa8 fix album-level tag consistency for mixed-artist tracks
Use album artist overrides during album ripping so compilation-style tracks do not change album performer tags, and add safer disc-number fallbacks for metadata/path generation when providers omit disc fields.
2026-04-20 00:59:10 +02:00
Joren b2688ce949 add CLI parity flags and expand provider support
This brings the Go CLI closer to upstream behavior with global flag handling and clearer resolve failures, while adding Tidal video downloads plus initial Deezer and SoundCloud no-account flows for broader end-to-end coverage.
2026-04-20 00:56:10 +02:00
Joren 4da5114a70 add format-aware tagging and replaygain fallback mapping
Write replaygain gain and peak tags from provider-specific metadata fields, then apply tagging across supported output containers with graceful cover-art fallback so non-FLAC downloads retain metadata.
2026-04-19 22:54:07 +02:00
Joren d4643d877e fix source-aware download tracking and filter/path regressions
Make download dedupe source-specific to prevent cross-provider ID collisions. Also correct non-remaster filtering, avoid FLAC tagging on non-FLAC files, and use album IDs for singles folder templating.
2026-04-19 21:25:04 +02:00
Joren 97e8b758b3 initial Go port of streamrip 2026-04-19 21:11:38 +02:00