HiFi preview: abort the source, don't cascade to a lower-tier preview — #895
The log from a fresh run showed detection working but a second bug: rejecting the
lossless preview ('decoded 30s of 180s — rejecting') just dropped to the SAME track's
'high' (lossy m4a) tier — the same 30s clip — which the bitrate check can't see (m4a,
not FLAC), so it was accepted. A preview at any tier means the SOURCE only has a
preview of the track; lower tiers are the same clip. So on preview detection (manifest
OR post-download) we now return None to FAIL HiFi entirely, letting the orchestrator's
hybrid fallback try the next SOURCE (soulseek/youtube) instead of landing a lower-tier
preview. (A genuinely-missing tier still falls through to the next tier as before.)
Integration test drives the post-download abort end-to-end and pins 'no tier cascade'.
This commit is contained in:
parent
4bce1932ba
commit
b6cb244cbd
2 changed files with 38 additions and 9 deletions
|
|
@ -984,14 +984,16 @@ class HiFiClient(DownloadSourcePlugin):
|
|||
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.
|
||||
# segments for a full-length track. A preview means THIS SOURCE only has a
|
||||
# preview of the track — lower quality tiers are the SAME preview — so abort
|
||||
# HiFi entirely and let the orchestrator fall through to the next SOURCE
|
||||
# (soulseek/youtube/…), rather than landing a lower-tier preview.
|
||||
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
|
||||
"HiFi has only a PREVIEW of '%s' (manifest %.0fs of %.0fs at %s) — "
|
||||
"failing HiFi so the next source is tried", display_name, manifest_s, expected_s, q_key)
|
||||
return None
|
||||
|
||||
extension = manifest_info['extension']
|
||||
safe_name = re.sub(r'[<>:"/\\|?*]', '_', display_name)
|
||||
|
|
@ -1087,11 +1089,15 @@ class HiFiClient(DownloadSourcePlugin):
|
|||
bits_per_sample=(props[1] if props else 0),
|
||||
channels=(props[2] if props else 0))
|
||||
if fake:
|
||||
# A preview at this tier means the SOURCE only has a preview — every
|
||||
# lower tier is the same 30s clip (and the lossy ones dodge the
|
||||
# bitrate check). Abort HiFi so the orchestrator tries the next
|
||||
# SOURCE, instead of cascading down into an accepted lower-tier preview.
|
||||
logger.warning(
|
||||
"HiFi served a PREVIEW/truncated file at %s for '%s' (%s) — rejecting",
|
||||
q_key, display_name, why)
|
||||
"HiFi has only a PREVIEW of '%s' (%s at %s) — failing HiFi so the "
|
||||
"next source is tried", display_name, why, q_key)
|
||||
out_path.unlink(missing_ok=True)
|
||||
continue
|
||||
return None
|
||||
|
||||
logger.info(f"HiFi download complete ({q_key}): {out_path} "
|
||||
f"({final_size / (1024*1024):.1f} MB)")
|
||||
|
|
|
|||
|
|
@ -96,7 +96,9 @@ def test_download_sync_skips_preview_manifests_and_never_downloads(tmp_path, mon
|
|||
|
||||
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)
|
||||
# A preview means the SOURCE only has a preview — every lower tier is the same clip,
|
||||
# so it must ABORT on the first preview, not cascade down into a lower-tier preview.
|
||||
assert tiers == ['lossless']
|
||||
|
||||
|
||||
def test_download_sync_proceeds_past_the_gate_for_a_full_manifest(tmp_path, monkeypatch):
|
||||
|
|
@ -117,6 +119,27 @@ def test_download_sync_proceeds_past_the_gate_for_a_full_manifest(tmp_path, monk
|
|||
assert seg_calls # it got PAST the preview gate to download
|
||||
|
||||
|
||||
def test_download_sync_aborts_on_a_faked_full_length_file_no_tier_cascade(tmp_path, monkeypatch):
|
||||
# The real #895 case: manifest + container claim FULL length, but the finished file
|
||||
# decodes to 30s. It must abort HiFi (return None) on the first tier — NOT drop to the
|
||||
# lossy 'high' tier (the same 30s preview, which dodges the bitrate check).
|
||||
monkeypatch.setattr(hc, 'config_manager', _Cfg())
|
||||
c = _bare_client(tmp_path)
|
||||
c.get_track_info = lambda tid: {'duration_s': 215}
|
||||
tiers = []
|
||||
c._get_hls_manifest = lambda tid, quality='lossless': (
|
||||
tiers.append(quality) or
|
||||
{'segment_uris': ['seg'], 'init_uri': None, 'extension': 'flac', 'manifest_duration': 215.0})
|
||||
c._download_segment_with_retry = lambda url: b'\x00' * 200_000 # > MIN_AUDIO_SIZE
|
||||
c._demux_flac = lambda i, o: o.write_bytes(b'\x00' * 200_000) # produce the 'flac'
|
||||
c._probe_real_seconds = lambda p: 30.0 # decodes to 30s
|
||||
c._probe_audio_seconds = lambda p: 215.0 # faked container claim
|
||||
c._flac_props = lambda p: (44100, 16, 2)
|
||||
|
||||
result = c._download_sync('dl1', 12345, 'The Weeknd - Save Your Tears')
|
||||
assert result is None and tiers == ['lossless'] # aborted, did NOT try 'high'
|
||||
|
||||
|
||||
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)
|
||||
|
|
|
|||
Loading…
Reference in a new issue