HiFi: reject preview manifests/files instead of landing a 30s fake
Fixes #895 — https://github.com/Nezreka/SoulSync/issues/895 HiFi (Tidal via hifi-api) sometimes serves a PREVIEW HLS manifest — only ~30s of segments — for a full-length track. SoulSync downloaded exactly what the manifest contained, and the only validation was a 100KB size floor (a 30s lossless preview is ~3-4MB, so it passed as 'complete') → a junk file that shows full length but plays ~30s. Add two duration guards, both keyed off the track's real length (from get_track_info): - Pre-download: sum the manifest's #EXTINF segment durations; if far short of the expected track length, it's a preview → skip that tier, fall through to the next source without wasting the download. (The parser discarded #EXTINF before.) - Post-download: probe the finished file's real length (mutagen) and reject if short — backs up legacy/direct (no-EXTINF) downloads and catches any truncation. Conservative: unknown/zero durations never reject. Pure sum_hls_segment_seconds / is_short_audio helpers with seam tests.
This commit is contained in:
parent
7bbd069147
commit
1249160ff5
2 changed files with 213 additions and 0 deletions
|
|
@ -140,6 +140,39 @@ def compute_new_default_pushes(all_defaults, offered, legacy_baseline, existing)
|
||||||
return to_add, new_offered
|
return to_add, new_offered
|
||||||
|
|
||||||
|
|
||||||
|
_EXTINF_RE = re.compile(r'#EXTINF:\s*([0-9]+(?:\.[0-9]+)?)')
|
||||||
|
|
||||||
|
|
||||||
|
def sum_hls_segment_seconds(playlist_text: str) -> float:
|
||||||
|
"""Total audio seconds an HLS media playlist actually provides — the sum of its
|
||||||
|
``#EXTINF`` segment durations. This is the authoritative "how much audio is really
|
||||||
|
here" signal: a PREVIEW manifest serves only ~30s of segments even though the track
|
||||||
|
is full-length, so summing EXTINF catches it before we waste the download. Returns
|
||||||
|
0.0 when the playlist has no EXTINF lines (master playlists, legacy manifests) — the
|
||||||
|
caller treats 0 as 'unknown', never as 'preview'."""
|
||||||
|
total = 0.0
|
||||||
|
for m in _EXTINF_RE.finditer(playlist_text or ''):
|
||||||
|
try:
|
||||||
|
total += float(m.group(1))
|
||||||
|
except (TypeError, ValueError):
|
||||||
|
continue
|
||||||
|
return total
|
||||||
|
|
||||||
|
|
||||||
|
def is_short_audio(actual_seconds: float, expected_seconds: float, threshold: float = 0.8) -> bool:
|
||||||
|
"""True when ``actual`` is meaningfully shorter than ``expected`` — i.e. a preview
|
||||||
|
clip or a truncated/corrupt download. Conservative: returns False whenever either
|
||||||
|
value is missing/zero (unknown ⇒ never reject), and only trips below ``threshold``
|
||||||
|
of the expected length (previews are ~15% of full, so the margin is huge)."""
|
||||||
|
try:
|
||||||
|
a, e = float(actual_seconds or 0), float(expected_seconds or 0)
|
||||||
|
except (TypeError, ValueError):
|
||||||
|
return False
|
||||||
|
if a <= 0 or e <= 0:
|
||||||
|
return False
|
||||||
|
return a < e * threshold
|
||||||
|
|
||||||
|
|
||||||
# Run the new-default push at most once per process.
|
# Run the new-default push at most once per process.
|
||||||
_pushed_new_defaults = False
|
_pushed_new_defaults = False
|
||||||
|
|
||||||
|
|
@ -662,6 +695,7 @@ class HiFiClient(DownloadSourcePlugin):
|
||||||
logger.warning(f"Failed to parse HLS playlist for track {track_id}: {e}")
|
logger.warning(f"Failed to parse HLS playlist for track {track_id}: {e}")
|
||||||
return None
|
return None
|
||||||
|
|
||||||
|
media_text = playlist_text # the playlist that actually carries the EXTINF segments
|
||||||
if '#EXT-X-STREAM-INF' in playlist_text and segment_uris:
|
if '#EXT-X-STREAM-INF' in playlist_text and segment_uris:
|
||||||
playlist_uri = segment_uris[0]
|
playlist_uri = segment_uris[0]
|
||||||
try:
|
try:
|
||||||
|
|
@ -669,6 +703,7 @@ class HiFiClient(DownloadSourcePlugin):
|
||||||
variant_resp = self.session.get(playlist_uri, allow_redirects=True, timeout=30)
|
variant_resp = self.session.get(playlist_uri, allow_redirects=True, timeout=30)
|
||||||
variant_resp.raise_for_status()
|
variant_resp.raise_for_status()
|
||||||
variant_text = variant_resp.text
|
variant_text = variant_resp.text
|
||||||
|
media_text = variant_text
|
||||||
init_uri, segment_uris = self._parse_hls_playlist(variant_text, playlist_uri)
|
init_uri, segment_uris = self._parse_hls_playlist(variant_text, playlist_uri)
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.warning(f"Failed to fetch variant playlist for track {track_id}: {e}")
|
logger.warning(f"Failed to fetch variant playlist for track {track_id}: {e}")
|
||||||
|
|
@ -687,6 +722,9 @@ class HiFiClient(DownloadSourcePlugin):
|
||||||
'extension': q_info['extension'],
|
'extension': q_info['extension'],
|
||||||
'codec': q_info['codec'],
|
'codec': q_info['codec'],
|
||||||
'quality': quality,
|
'quality': quality,
|
||||||
|
# Real audio length the manifest provides (sum of EXTINF) — used to reject
|
||||||
|
# preview manifests before downloading. 0.0 = unknown (don't reject).
|
||||||
|
'manifest_duration': sum_hls_segment_seconds(media_text),
|
||||||
}
|
}
|
||||||
|
|
||||||
def _get_legacy_track_manifest(self, track_id: int, quality: str = 'lossless') -> Optional[Dict]:
|
def _get_legacy_track_manifest(self, track_id: int, quality: str = 'lossless') -> Optional[Dict]:
|
||||||
|
|
@ -711,6 +749,20 @@ class HiFiClient(DownloadSourcePlugin):
|
||||||
'quality': quality,
|
'quality': quality,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@staticmethod
|
||||||
|
def _probe_audio_seconds(path) -> float:
|
||||||
|
"""Real decoded audio length of a finished file, via mutagen (already a dep).
|
||||||
|
0.0 on any failure — the caller treats 0 as 'unknown' and never rejects on it."""
|
||||||
|
try:
|
||||||
|
from mutagen import File as _MutagenFile
|
||||||
|
mf = _MutagenFile(str(path))
|
||||||
|
info = getattr(mf, 'info', None) if mf is not None else None
|
||||||
|
if info is not None:
|
||||||
|
return float(getattr(info, 'length', 0) or 0)
|
||||||
|
except Exception:
|
||||||
|
pass
|
||||||
|
return 0.0
|
||||||
|
|
||||||
def _demux_flac(self, input_path: Path, output_path: Path) -> None:
|
def _demux_flac(self, input_path: Path, output_path: Path) -> None:
|
||||||
ffmpeg = shutil.which('ffmpeg')
|
ffmpeg = shutil.which('ffmpeg')
|
||||||
if not ffmpeg:
|
if not ffmpeg:
|
||||||
|
|
@ -836,6 +888,15 @@ class HiFiClient(DownloadSourcePlugin):
|
||||||
|
|
||||||
MIN_AUDIO_SIZE = 100 * 1024
|
MIN_AUDIO_SIZE = 100 * 1024
|
||||||
|
|
||||||
|
# Expected track length (for the preview/truncation guards). Best-effort: a 0
|
||||||
|
# here just disables the duration checks for this track, never rejects.
|
||||||
|
expected_s = 0.0
|
||||||
|
try:
|
||||||
|
info = self.get_track_info(track_id) or {}
|
||||||
|
expected_s = float(info.get('duration_s') or 0)
|
||||||
|
except Exception:
|
||||||
|
expected_s = 0.0
|
||||||
|
|
||||||
for q_key in chain:
|
for q_key in chain:
|
||||||
if self.shutdown_check and self.shutdown_check():
|
if self.shutdown_check and self.shutdown_check():
|
||||||
logger.info("Shutdown detected, aborting HiFi download")
|
logger.info("Shutdown detected, aborting HiFi download")
|
||||||
|
|
@ -852,6 +913,16 @@ class HiFiClient(DownloadSourcePlugin):
|
||||||
logger.warning(f"No HLS manifest at quality {q_key}, trying next")
|
logger.warning(f"No HLS manifest at quality {q_key}, trying next")
|
||||||
continue
|
continue
|
||||||
|
|
||||||
|
# Preview guard #1 (pre-download): a preview manifest serves only ~30s of
|
||||||
|
# segments for a full-length track. Catch it from the EXTINF sum before
|
||||||
|
# wasting the download, so the orchestrator falls through to the next source.
|
||||||
|
manifest_s = float(manifest_info.get('manifest_duration') or 0)
|
||||||
|
if is_short_audio(manifest_s, expected_s):
|
||||||
|
logger.warning(
|
||||||
|
"HiFi served a PREVIEW manifest at %s for '%s' (%.0fs of %.0fs) — skipping",
|
||||||
|
q_key, display_name, manifest_s, expected_s)
|
||||||
|
continue
|
||||||
|
|
||||||
extension = manifest_info['extension']
|
extension = manifest_info['extension']
|
||||||
safe_name = re.sub(r'[<>:"/\\|?*]', '_', display_name)
|
safe_name = re.sub(r'[<>:"/\\|?*]', '_', display_name)
|
||||||
out_filename = f"{safe_name}.{extension}"
|
out_filename = f"{safe_name}.{extension}"
|
||||||
|
|
@ -931,6 +1002,17 @@ class HiFiClient(DownloadSourcePlugin):
|
||||||
out_path.unlink(missing_ok=True)
|
out_path.unlink(missing_ok=True)
|
||||||
continue
|
continue
|
||||||
|
|
||||||
|
# Preview guard #2 (post-download): verify the finished file's REAL audio
|
||||||
|
# length. Backs up the manifest check for legacy/direct (no-EXTINF)
|
||||||
|
# downloads and catches any truncated/corrupt result.
|
||||||
|
actual_s = self._probe_audio_seconds(out_path)
|
||||||
|
if is_short_audio(actual_s, expected_s):
|
||||||
|
logger.warning(
|
||||||
|
"HiFi file too short at %s for '%s' (%.0fs of %.0fs) — likely a "
|
||||||
|
"preview, rejecting", q_key, display_name, actual_s, expected_s)
|
||||||
|
out_path.unlink(missing_ok=True)
|
||||||
|
continue
|
||||||
|
|
||||||
logger.info(f"HiFi download complete ({q_key}): {out_path} "
|
logger.info(f"HiFi download complete ({q_key}): {out_path} "
|
||||||
f"({final_size / (1024*1024):.1f} MB)")
|
f"({final_size / (1024*1024):.1f} MB)")
|
||||||
return str(out_path)
|
return str(out_path)
|
||||||
|
|
|
||||||
131
tests/test_hifi_preview_guard.py
Normal file
131
tests/test_hifi_preview_guard.py
Normal file
|
|
@ -0,0 +1,131 @@
|
||||||
|
"""HiFi sometimes serves a PREVIEW manifest (~30s of segments) for a full-length
|
||||||
|
track, which slipped through the old 100KB-only size floor. The duration guards
|
||||||
|
catch it: a preview manifest is way shorter than the real track length."""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from core.hifi_client import sum_hls_segment_seconds, is_short_audio
|
||||||
|
|
||||||
|
|
||||||
|
_FULL = """#EXTM3U
|
||||||
|
#EXT-X-VERSION:6
|
||||||
|
#EXT-X-MAP:URI="init.mp4"
|
||||||
|
#EXTINF:10.0,
|
||||||
|
seg0.mp4
|
||||||
|
#EXTINF:10.0,
|
||||||
|
seg1.mp4
|
||||||
|
#EXTINF:9.5,
|
||||||
|
seg2.mp4
|
||||||
|
#EXT-X-ENDLIST
|
||||||
|
"""
|
||||||
|
|
||||||
|
_PREVIEW = """#EXTM3U
|
||||||
|
#EXTINF:15.0,
|
||||||
|
p0.mp4
|
||||||
|
#EXTINF:15.0,
|
||||||
|
p1.mp4
|
||||||
|
#EXT-X-ENDLIST
|
||||||
|
"""
|
||||||
|
|
||||||
|
|
||||||
|
def test_sums_extinf_segment_durations():
|
||||||
|
assert sum_hls_segment_seconds(_FULL) == 29.5 # 10 + 10 + 9.5
|
||||||
|
assert sum_hls_segment_seconds(_PREVIEW) == 30.0 # the preview's true length
|
||||||
|
|
||||||
|
|
||||||
|
def test_no_extinf_is_unknown_zero():
|
||||||
|
assert sum_hls_segment_seconds("#EXTM3U\nseg.mp4\n") == 0.0
|
||||||
|
assert sum_hls_segment_seconds("") == 0.0
|
||||||
|
|
||||||
|
|
||||||
|
def test_preview_is_flagged_short_against_full_track():
|
||||||
|
# Save Your Tears ~215s; a 30s preview manifest is obviously short
|
||||||
|
assert is_short_audio(30.0, 215.0) is True
|
||||||
|
|
||||||
|
|
||||||
|
def test_full_length_download_is_not_flagged():
|
||||||
|
assert is_short_audio(213.0, 215.0) is False # ~1% trim → fine
|
||||||
|
assert is_short_audio(215.0, 215.0) is False
|
||||||
|
|
||||||
|
|
||||||
|
def test_unknown_durations_never_reject():
|
||||||
|
assert is_short_audio(0, 215) is False # couldn't probe → don't reject
|
||||||
|
assert is_short_audio(30, 0) is False # expected unknown → don't reject
|
||||||
|
assert is_short_audio(0, 0) is False
|
||||||
|
|
||||||
|
|
||||||
|
def test_legitimately_short_track_is_kept():
|
||||||
|
# a real 40s interlude: actual ≈ expected → not a preview
|
||||||
|
assert is_short_audio(40.0, 41.0) is False
|
||||||
|
|
||||||
|
|
||||||
|
def test_threshold_boundary():
|
||||||
|
assert is_short_audio(79, 100) is True # below 80%
|
||||||
|
assert is_short_audio(85, 100) is False # above 80%
|
||||||
|
|
||||||
|
|
||||||
|
# ── integration: the guards actually wire into _download_sync ────────────────
|
||||||
|
import pytest
|
||||||
|
import core.hifi_client as hc
|
||||||
|
|
||||||
|
|
||||||
|
class _Cfg:
|
||||||
|
"""Stub config so _download_sync just takes its defaults (no DB)."""
|
||||||
|
def get(self, key, default=None):
|
||||||
|
return default
|
||||||
|
|
||||||
|
|
||||||
|
def _bare_client(tmp_path):
|
||||||
|
c = object.__new__(hc.HiFiClient) # skip __init__ (DB / network)
|
||||||
|
c.download_path = tmp_path
|
||||||
|
c._engine = None
|
||||||
|
c.shutdown_check = None
|
||||||
|
return c
|
||||||
|
|
||||||
|
|
||||||
|
def test_download_sync_skips_preview_manifests_and_never_downloads(tmp_path, monkeypatch):
|
||||||
|
monkeypatch.setattr(hc, 'config_manager', _Cfg())
|
||||||
|
c = _bare_client(tmp_path)
|
||||||
|
c.get_track_info = lambda tid: {'duration_s': 215} # real track length
|
||||||
|
tiers = []
|
||||||
|
c._get_hls_manifest = lambda tid, quality='lossless': (
|
||||||
|
tiers.append(quality) or
|
||||||
|
{'segment_uris': ['seg'], 'init_uri': None, 'extension': 'flac',
|
||||||
|
'manifest_duration': 30.0}) # preview at EVERY tier
|
||||||
|
c._download_segment_with_retry = lambda url: pytest.fail("downloaded a preview segment!")
|
||||||
|
|
||||||
|
result = c._download_sync('dl1', 12345, 'The Weeknd - Save Your Tears')
|
||||||
|
assert result is None # → orchestrator falls back
|
||||||
|
assert tiers # it did consult the manifest(s)
|
||||||
|
|
||||||
|
|
||||||
|
def test_download_sync_proceeds_past_the_gate_for_a_full_manifest(tmp_path, monkeypatch):
|
||||||
|
monkeypatch.setattr(hc, 'config_manager', _Cfg())
|
||||||
|
c = _bare_client(tmp_path)
|
||||||
|
c.get_track_info = lambda tid: {'duration_s': 215}
|
||||||
|
c._get_hls_manifest = lambda tid, quality='lossless': {
|
||||||
|
'segment_uris': ['seg'], 'init_uri': None, 'extension': 'flac',
|
||||||
|
'manifest_duration': 215.0} # full length → must NOT skip
|
||||||
|
seg_calls = []
|
||||||
|
|
||||||
|
def _seg(url):
|
||||||
|
seg_calls.append(url)
|
||||||
|
raise RuntimeError("stop after the gate")
|
||||||
|
c._download_segment_with_retry = _seg
|
||||||
|
|
||||||
|
c._download_sync('dl1', 12345, 'x')
|
||||||
|
assert seg_calls # it got PAST the preview gate to download
|
||||||
|
|
||||||
|
|
||||||
|
def test_download_sync_does_not_reject_when_track_length_unknown(tmp_path, monkeypatch):
|
||||||
|
monkeypatch.setattr(hc, 'config_manager', _Cfg())
|
||||||
|
c = _bare_client(tmp_path)
|
||||||
|
c.get_track_info = lambda tid: {'duration_s': 0} # expected unknown
|
||||||
|
c._get_hls_manifest = lambda tid, quality='lossless': {
|
||||||
|
'segment_uris': ['seg'], 'init_uri': None, 'extension': 'flac',
|
||||||
|
'manifest_duration': 30.0} # short, but expected is unknown
|
||||||
|
seg_calls = []
|
||||||
|
c._download_segment_with_retry = lambda url: (seg_calls.append(url), (_ for _ in ()).throw(RuntimeError("stop")))[0]
|
||||||
|
|
||||||
|
c._download_sync('dl1', 12345, 'x')
|
||||||
|
assert seg_calls # unknown length → no rejection, proceeds
|
||||||
Loading…
Reference in a new issue