Fix three SoundCloud integration gaps surfaced by smoke testing
User report: switched download source to SoundCloud and noticed: 1. Download progress % stays at 0 until "suddenly done" — no live progress 2. Sidebar status indicator next to "SoundCloud" label is red 3. Dashboard service status card still shows "Soulseek" as the source name Fix 1 — Live progress for HLS-segmented SoundCloud downloads (`core/soundcloud_client.py`): - yt-dlp's `total_bytes` / `total_bytes_estimate` for HLS describes the CURRENT FRAGMENT, not the whole download. So the byte-based percentage stayed near 0 the entire time — until 'finished' fired. - Added `_update_download_progress_fragmented` which uses `fragment_index` / `fragment_count` (which yt-dlp DOES populate accurately for HLS) to compute a meaningful percentage. Total size is extrapolated from per-fragment average for the bytes/remaining display. Time-remaining estimate uses elapsed/index seconds-per- fragment. - The progress hook prefers fragment progress when both fragment_index and fragment_count are present; falls back to byte-based for non-fragmented (progressive MP3) downloads. Five new unit tests pin the fragment-progress math, the 99.9% cap, and the defensive zero-index / unknown-id paths. Fix 2 — Sidebar status indicator stays green for SoundCloud mode (`web_server.py`): - The `/api/status` route's `serverless_sources` tuple decides whether to even probe slskd. SoundCloud (and Lidarr) were missing — so when the active source was SoundCloud, the route fell through to "test slskd, mark not-relevant", which set `connected: False` and turned the sidebar dot red even though SoundCloud was working. - Added `'soundcloud'` and `'lidarr'` to the tuple. Both are serverless from slskd's perspective, so the dot now stays green whenever they're the active source. Fix 3 — Dashboard service card title shows the active source (`webui/static/shared-helpers.js`): - The dashboard's "Download Source" card has its own `sourceNames` map at line 3351 (separate from the sidebar map I already updated at 3396). Missed it during the integration PR. - Added `'lidarr'` and `'soundcloud'` so the card title now reads "SoundCloud" / "Lidarr" instead of falling back to "Soulseek". Bonus — Dashboard "Test Connection" button works for SoundCloud (`core/connection_test.py`): - The dashboard's Test Connection button on the download-source card sends `service` based on the active source — so for SoundCloud it was sending `service='soundcloud'`. `run_service_test` had no branch for it, so it fell through to "Unknown service." and the button always failed. - Added a `soundcloud` branch that mirrors `/api/soundcloud/status` behavior: confirms yt-dlp is installed, runs a real cheap probe, returns a meaningful pass/fail. (HiFi has the same gap but no user reported it; out of scope for this fix.) Verified: - 41 unit tests pass (5 new fragment-progress tests added) - Full suite 1732 passed - Ruff clean
This commit is contained in:
Broque Thomas
parent
75fe04907f
commit
8de4a186b7
5 changed files with 154 additions and 4 deletions
|
|
@ -379,6 +379,24 @@ def run_service_test(service, test_config):
|
|||
return False, "Hydrabase not connected. Configure URL + API key and click Connect."
|
||||
except Exception as e:
|
||||
return False, f"Hydrabase connection error: {str(e)}"
|
||||
elif service == "soundcloud":
|
||||
# Anonymous SoundCloud has no auth, so "test" really means
|
||||
# "is yt-dlp installed and can it reach SoundCloud right now."
|
||||
# This mirrors the /api/soundcloud/status check.
|
||||
try:
|
||||
from core.soundcloud_client import SoundcloudClient
|
||||
sc = SoundcloudClient()
|
||||
if not sc.is_available():
|
||||
return False, "SoundCloud unavailable — yt-dlp not installed."
|
||||
# Run a tiny live probe via asyncio so the dashboard test
|
||||
# gives a meaningful pass/fail.
|
||||
import asyncio
|
||||
reachable = asyncio.new_event_loop().run_until_complete(sc.check_connection())
|
||||
if reachable:
|
||||
return True, "SoundCloud reachable (anonymous)"
|
||||
return False, "SoundCloud unreachable — search probe failed. Try again."
|
||||
except Exception as e:
|
||||
return False, f"SoundCloud connection error: {str(e)}"
|
||||
return False, "Unknown service."
|
||||
except AttributeError as e:
|
||||
# This specifically catches the error you reported for Jellyfin
|
||||
|
|
|
|||
|
|
@ -438,7 +438,22 @@ class SoundcloudClient:
|
|||
if status == 'downloading':
|
||||
downloaded = int(progress.get('downloaded_bytes') or 0)
|
||||
total = int(progress.get('total_bytes') or progress.get('total_bytes_estimate') or 0)
|
||||
self._update_download_progress(download_id, downloaded, total, speed_start)
|
||||
|
||||
# SoundCloud serves HLS-segmented audio. yt-dlp doesn't know
|
||||
# the final byte total upfront — `total_bytes` and
|
||||
# `total_bytes_estimate` reflect the CURRENT FRAGMENT size,
|
||||
# not the whole download, so a byte-based percentage stays
|
||||
# near 0 until the very end. Fall back to fragment progress
|
||||
# which yt-dlp DOES populate accurately for HLS.
|
||||
fragment_index = progress.get('fragment_index')
|
||||
fragment_count = progress.get('fragment_count')
|
||||
if (fragment_index is not None and fragment_count
|
||||
and fragment_count > 0):
|
||||
self._update_download_progress_fragmented(
|
||||
download_id, downloaded, fragment_index, fragment_count, speed_start,
|
||||
)
|
||||
else:
|
||||
self._update_download_progress(download_id, downloaded, total, speed_start)
|
||||
elif status == 'finished':
|
||||
# yt-dlp signals 'finished' once the bytes are on disk; the
|
||||
# final size is authoritative. Mark progress at 99% — the
|
||||
|
|
@ -506,6 +521,52 @@ class SoundcloudClient:
|
|||
)
|
||||
return resolved_path
|
||||
|
||||
def _update_download_progress_fragmented(self, download_id: str, downloaded: int,
|
||||
fragment_index: int, fragment_count: int,
|
||||
speed_start: float) -> None:
|
||||
"""HLS-aware progress update.
|
||||
|
||||
SoundCloud's HLS streams arrive as N small fragments. yt-dlp can't
|
||||
compute a true byte-based percentage because each fragment's
|
||||
``total_bytes`` only describes the current fragment, not the
|
||||
whole download. fragment_index / fragment_count gives an accurate
|
||||
progress signal — N fragments downloaded out of M known fragments
|
||||
is the real ratio.
|
||||
"""
|
||||
with self._download_lock:
|
||||
if download_id not in self.active_downloads:
|
||||
return
|
||||
info = self.active_downloads[download_id]
|
||||
info['transferred'] = downloaded
|
||||
|
||||
now = time.time()
|
||||
elapsed = now - speed_start
|
||||
info['speed'] = int(downloaded / elapsed) if elapsed > 0 else 0
|
||||
|
||||
# `fragment_index` is 1-based when yt-dlp finishes a fragment;
|
||||
# use it directly. Cap below 100% so the worker thread owns the
|
||||
# final flip.
|
||||
progress = (fragment_index / fragment_count) * 100
|
||||
info['progress'] = round(min(progress, 99.9), 1)
|
||||
|
||||
# Estimate total size so the UI's bytes-remaining reads match
|
||||
# roughly what users expect — extrapolate from per-fragment
|
||||
# average. Defensive against fragment_index being 0 on the
|
||||
# very first callback.
|
||||
if fragment_index > 0 and downloaded > 0:
|
||||
est_total = int(downloaded * (fragment_count / fragment_index))
|
||||
info['size'] = est_total
|
||||
else:
|
||||
info['size'] = downloaded
|
||||
|
||||
time_remaining: Optional[int] = None
|
||||
remaining_fragments = max(0, fragment_count - fragment_index)
|
||||
if info['speed'] > 0 and remaining_fragments > 0 and fragment_index > 0:
|
||||
# Extrapolate seconds from per-fragment average download time.
|
||||
seconds_per_fragment = elapsed / fragment_index if fragment_index > 0 else 0
|
||||
time_remaining = int(remaining_fragments * seconds_per_fragment)
|
||||
info['time_remaining'] = time_remaining
|
||||
|
||||
def _update_download_progress(self, download_id: str, downloaded: int,
|
||||
total: int, speed_start: float) -> None:
|
||||
"""Push a progress update into the active_downloads ledger.
|
||||
|
|
|
|||
|
|
@ -515,6 +515,74 @@ def test_update_download_progress_silently_skips_unknown_id(tmp_dl: Path) -> Non
|
|||
client._update_download_progress('does_not_exist', 100, 1000, time.time())
|
||||
|
||||
|
||||
def test_update_download_progress_fragmented_uses_fragment_count(tmp_dl: Path) -> None:
|
||||
"""HLS-aware progress: fragment 5 of 10 → ~50%, regardless of byte
|
||||
estimate. Pins the SoundCloud HLS UX fix where byte-based progress
|
||||
stayed stuck near 0."""
|
||||
client = SoundcloudClient(download_path=str(tmp_dl))
|
||||
with client._download_lock:
|
||||
client.active_downloads['hls1'] = {
|
||||
'id': 'hls1', 'filename': '', 'username': 'soundcloud',
|
||||
'state': 'InProgress, Downloading', 'progress': 0.0, 'size': 0,
|
||||
'transferred': 0, 'speed': 0, 'time_remaining': None,
|
||||
}
|
||||
client._update_download_progress_fragmented(
|
||||
'hls1', downloaded=512_000, fragment_index=5, fragment_count=10,
|
||||
speed_start=time.time() - 1.0,
|
||||
)
|
||||
info = client.active_downloads['hls1']
|
||||
assert 49.0 <= info['progress'] <= 51.0
|
||||
# Total size estimate extrapolates from per-fragment average
|
||||
assert info['size'] == 1_024_000 # 512000 * (10/5)
|
||||
# Time remaining computed
|
||||
assert info['time_remaining'] is not None and info['time_remaining'] > 0
|
||||
|
||||
|
||||
def test_update_download_progress_fragmented_caps_at_99_9(tmp_dl: Path) -> None:
|
||||
"""Final fragment shouldn't push percentage to 100 — outer thread
|
||||
owns the final flip. Mirrors byte-based behavior."""
|
||||
client = SoundcloudClient(download_path=str(tmp_dl))
|
||||
with client._download_lock:
|
||||
client.active_downloads['hls2'] = {
|
||||
'id': 'hls2', 'filename': '', 'username': 'soundcloud',
|
||||
'state': 'InProgress, Downloading', 'progress': 0.0, 'size': 0,
|
||||
'transferred': 0, 'speed': 0, 'time_remaining': None,
|
||||
}
|
||||
client._update_download_progress_fragmented(
|
||||
'hls2', downloaded=10_000_000, fragment_index=10, fragment_count=10,
|
||||
speed_start=time.time() - 5.0,
|
||||
)
|
||||
assert client.active_downloads['hls2']['progress'] == 99.9
|
||||
|
||||
|
||||
def test_update_download_progress_fragmented_handles_zero_index(tmp_dl: Path) -> None:
|
||||
"""Defensive: yt-dlp can call the progress hook with fragment_index=0
|
||||
on the very first callback (HLS init segment). Progress is 0% and
|
||||
nothing crashes."""
|
||||
client = SoundcloudClient(download_path=str(tmp_dl))
|
||||
with client._download_lock:
|
||||
client.active_downloads['hls3'] = {
|
||||
'id': 'hls3', 'filename': '', 'username': 'soundcloud',
|
||||
'state': 'InProgress, Downloading', 'progress': 0.0, 'size': 0,
|
||||
'transferred': 0, 'speed': 0, 'time_remaining': None,
|
||||
}
|
||||
client._update_download_progress_fragmented(
|
||||
'hls3', downloaded=0, fragment_index=0, fragment_count=10,
|
||||
speed_start=time.time(),
|
||||
)
|
||||
# No exception, progress stays 0
|
||||
assert client.active_downloads['hls3']['progress'] == 0.0
|
||||
|
||||
|
||||
def test_update_download_progress_fragmented_silently_skips_unknown_id(tmp_dl: Path) -> None:
|
||||
client = SoundcloudClient(download_path=str(tmp_dl))
|
||||
# Must not raise
|
||||
client._update_download_progress_fragmented(
|
||||
'unknown', downloaded=100, fragment_index=1, fragment_count=10,
|
||||
speed_start=time.time(),
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Status / cancel / clear
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -3486,8 +3486,11 @@ def get_status():
|
|||
soulseek_relevant = (download_mode == 'soulseek' or
|
||||
(download_mode == 'hybrid' and 'soulseek' in hybrid_order))
|
||||
|
||||
# Serverless sources (YouTube, HiFi, Qobuz) are always available
|
||||
serverless_sources = ('youtube', 'hifi', 'qobuz', 'tidal', 'deezer_dl')
|
||||
# Serverless sources (YouTube, HiFi, Qobuz, Tidal, Deezer, Lidarr, SoundCloud)
|
||||
# don't depend on slskd being reachable — when one of these is the
|
||||
# active source, surface "connected" without probing slskd so the
|
||||
# dashboard / sidebar indicator stays green.
|
||||
serverless_sources = ('youtube', 'hifi', 'qobuz', 'tidal', 'deezer_dl', 'lidarr', 'soundcloud')
|
||||
is_serverless = (download_mode in serverless_sources or
|
||||
(download_mode == 'hybrid' and
|
||||
hybrid_order and any(s in serverless_sources for s in hybrid_order)))
|
||||
|
|
|
|||
|
|
@ -3348,7 +3348,7 @@ function updateServiceStatus(service, statusData, spotifyStatus = null) {
|
|||
|
||||
// Update download source title on dashboard card
|
||||
if (service === 'soulseek' && statusData.source) {
|
||||
const sourceNames = { soulseek: 'Soulseek', youtube: 'YouTube', tidal: 'Tidal', qobuz: 'Qobuz', hifi: 'HiFi', deezer_dl: 'Deezer', hybrid: 'Hybrid' };
|
||||
const sourceNames = { soulseek: 'Soulseek', youtube: 'YouTube', tidal: 'Tidal', qobuz: 'Qobuz', hifi: 'HiFi', deezer_dl: 'Deezer', lidarr: 'Lidarr', soundcloud: 'SoundCloud', hybrid: 'Hybrid' };
|
||||
const displayName = sourceNames[statusData.source] || 'Soulseek';
|
||||
const titleEl = document.getElementById('download-source-title');
|
||||
if (titleEl) titleEl.textContent = displayName;
|
||||
|
|
|
|||
Loading…
Reference in a new issue