From 0325467aba21411078dd000fc7544b4b5ff91879 Mon Sep 17 00:00:00 2001 From: Jonathan Date: Wed, 15 Jul 2026 02:37:08 +0200 Subject: [PATCH] fix(worker): smooth Qobuz download progress via streamrip byte hook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit streamrip reports progress only per concurrent track, so Job.downloadProgress came solely from the file-count poller — 0% until every track landed, then a jump to 100%. Patch streamrip.media.track.get_progress_callback to aggregate per-track byte deltas over an estimated album total (avg started-track size × len(album.tracks)) into one smooth, generally-climbing 0..1 fraction routed through the pipeline's on_progress. Capped at 0.99; the pipeline sets 1.0 on completion. No signature changes — track count read from the resolved album. Co-Authored-By: Claude Opus 4.8 (1M context) --- worker/lyra_worker/adapters/_streamrip.py | 64 ++++++++++++++++++++++- worker/tests/test_streamrip_progress.py | 41 +++++++++++++++ 2 files changed, 104 insertions(+), 1 deletion(-) create mode 100644 worker/tests/test_streamrip_progress.py diff --git a/worker/lyra_worker/adapters/_streamrip.py b/worker/lyra_worker/adapters/_streamrip.py index 30e517a..15a3131 100644 --- a/worker/lyra_worker/adapters/_streamrip.py +++ b/worker/lyra_worker/adapters/_streamrip.py @@ -34,6 +34,53 @@ def _run(coro): return _loop.run_until_complete(coro) +class _ProgressAggregator: + """Turn streamrip's per-track byte callbacks into one album-level 0..1 fraction. + + streamrip downloads an album's tracks concurrently and reports progress only per track + (a byte delta per chunk), so the file-count poller sees 0 complete files until they all + land at once — the 0%→100% jump. Summing downloaded bytes over an *estimated* album total + (average started-track size × track count) gives a smooth, generally-climbing fraction. + Track sizes are similar, so the estimate is stable once a track or two has started; capped + at 0.99 because the pipeline sets 1.0 once the whole download returns. All streamrip + coroutines run on one event loop on the calling thread, so no locking is needed.""" + + def __init__(self, track_count: int, on_progress: Callable[[float], None]): + self._n = track_count or 0 + self._on_progress = on_progress + self._seen_total = 0 # summed byte-size of tracks that have started downloading + self._seen_count = 0 # number of tracks that have started + self._done_bytes = 0 # summed bytes downloaded across all started tracks + + def start_track(self, total) -> None: + self._seen_total += max(int(total or 0), 0) + self._seen_count += 1 + + def advance(self, delta) -> None: + self._done_bytes += max(int(delta or 0), 0) + if self._seen_count == 0: + return + avg = self._seen_total / self._seen_count + n = self._n if self._n > 0 else self._seen_count + est_total = avg * n + if est_total > 0: + self._on_progress(min(self._done_bytes / est_total, 0.99)) + + +class _ProgressHandle: + """streamrip's `Track.download` uses `with get_progress_callback(...) as cb:`; `cb` must be + the per-chunk byte callback. This stand-in yields that callback and no-ops on exit.""" + + def __init__(self, update: Callable[[int], None]): + self.update = update + + def __enter__(self): + return self.update + + def __exit__(self, *_): + return False + + def _flatten_audio(dest: str) -> int: """streamrip writes into a nested `Artist - Album [...]/` subfolder. Move any audio files up into `dest`, remove the now-empty subfolders, and return the count of audio @@ -136,6 +183,7 @@ class StreamripClient: async def _download(self, album_id: str, dest: str, on_progress: Callable[[float], None]) -> None: os.makedirs(dest, exist_ok=True) + import streamrip.media.track as _track_mod from streamrip.client import QobuzClient from streamrip.db import Database, Dummy from streamrip.media import PendingAlbum @@ -153,7 +201,21 @@ class StreamripClient: album = await pending.resolve() if album is None: raise RuntimeError(f"could not resolve Qobuz album {album_id}") - await album.rip() + # Hook streamrip's per-track byte progress into a single album-level fraction so the + # Floor bar climbs smoothly instead of jumping 0→100. `Track.download` binds + # get_progress_callback at import, so patch it on the track module (not progress.py). + agg = _ProgressAggregator(len(getattr(album, "tracks", []) or []), on_progress) + _orig_cb = _track_mod.get_progress_callback + + def _hook(_enabled, total, _desc): + agg.start_track(total) + return _ProgressHandle(agg.advance) + + _track_mod.get_progress_callback = _hook + try: + await album.rip() + finally: + _track_mod.get_progress_callback = _orig_cb on_progress(1.0) finally: await client.session.close() diff --git a/worker/tests/test_streamrip_progress.py b/worker/tests/test_streamrip_progress.py new file mode 100644 index 0000000..db6509f --- /dev/null +++ b/worker/tests/test_streamrip_progress.py @@ -0,0 +1,41 @@ +from lyra_worker.adapters._streamrip import _ProgressAggregator + + +def _record(): + seen: list[float] = [] + return seen, seen.append + + +def test_equal_size_tracks_aggregate_monotonically_to_near_one(): + # 4 tracks of 100 bytes each, all started up front (concurrency covers the album), then + # each downloaded in 25-byte chunks. Fraction must climb monotonically 0→~1. + seen, cb = _record() + agg = _ProgressAggregator(4, cb) + for _ in range(4): + agg.start_track(100) + for _ in range(16): # 16 chunks * 25 bytes = 400 bytes = full album + agg.advance(25) + + assert seen == sorted(seen) # monotonic non-decreasing + assert all(0.0 <= f <= 0.99 for f in seen) # never negative, capped at 0.99 + assert seen[-1] == 0.99 # reaches the cap when all bytes are in + assert seen[len(seen) // 2] > 0.3 # genuinely climbs mid-download (no 0→100 jump) + + +def test_estimate_uses_track_count_before_all_tracks_start(): + # Only the first of 4 tracks has started; with a known track count the estimate already + # spans the whole album, so finishing track 1 reports ~1/4, not ~1/1. + seen, cb = _record() + agg = _ProgressAggregator(4, cb) + agg.start_track(100) + agg.advance(100) # track 1 fully downloaded + assert abs(seen[-1] - 0.25) < 0.01 + + +def test_zero_or_missing_sizes_never_emit_and_never_divide_by_zero(): + seen, cb = _record() + agg = _ProgressAggregator(0, cb) + agg.advance(50) # no track started yet -> no emit + agg.start_track(0) # a track with unknown size + agg.advance(50) # est_total still 0 -> still no emit, no crash + assert seen == []