Commit graph

3 commits

Author SHA1 Message Date
BoulderBadgeDad
319b6483a2 integrity: don't quarantine longer masters/versions as 'truncated' (#937)
the duration-agreement check used abs() drift, so a file running LONGER than the metadata
(a remaster with a longer outro, an extended cut) was rejected the same as a truncated one —
e.g. A-Ha 'Take on Me' remaster at 228.5s vs 225.0s expected, quarantined for +3.5s.

but a longer file is the OPPOSITE of truncated. make the auto tolerance asymmetric: keep the
tight 3s/5s bound for SHORTER files (the truncation case the check exists for), allow up to
15s in the LONGER direction for version/master differences. a wrong song still trips it (off
by far more than 15s), and a user-pinned tolerance is honoured symmetrically. direction-aware
rejection message too. 4 new tests; 274 integrity/import tests green.
2026-06-27 15:25:13 -07:00
BoulderBadgeDad
d6f37f9667 Integrity check: don't quarantine valid streamed FLAC as 'zero-length' (#756)
HiFi assembles FLAC from HLS segments and demuxes with `ffmpeg -c copy`,
which preserves total_samples=0 in the STREAMINFO of Tidal's fragmented/
streamed FLAC. Every audio frame is present and the file plays fine, but
mutagen computes length = total_samples / sample_rate = 0, so the integrity
check rejected it as 'zero-length audio' and quarantined nearly every HiFi
download. Users confirmed the quarantined files play normally once restored.

Length 0 is not proof of corruption at that point: the file already passed
the size gate, was identified as a real audio format, and has a valid info
block — a genuinely empty/truncated/stub file fails one of those earlier
checks instead. Treat length 0 as 'length unknown': accept the file and skip
the duration cross-check we can't perform without a length. mutagen never
decoded/validated frame data anyway, so this doesn't weaken real corruption
detection — size, parse, format, info-block, and duration-drift guards all
remain.

Tests: a large valid-parse length-0 file (streamed-FLAC signature) is now
accepted; a tiny length-0 stub still fails (size gate fires first).
2026-05-31 11:28:40 -07:00
Broque Thomas
42f3026eef Reject broken downloads before tagging via universal integrity check
Discord report (fresh.dumbledore [VRN]): slskd sometimes ships broken files
(truncated transfers, corrupt FLAC, wrong file substituted on filename match).
They flowed through post-processing and only surfaced later — Plex/Jellyfin
scan failures, dead-air playback, duplicate detector tripping over the wrong
length. By that point the file was already tagged, copied, mirrored to the
media server, and recorded in provenance.

New module `core/imports/file_integrity.py`:
- `check_audio_integrity(path, expected_duration_ms=None) -> IntegrityResult`
- Three tiered checks, cheapest to most expensive:
  1. File size sanity (catches 0-byte stubs and stub transfers)
  2. Mutagen parse (catches header damage, wrong-format-with-right-extension)
  3. Duration agreement vs. metadata source's expected length, ±3s tolerance
     (5s for tracks over 10 minutes — long tracks naturally drift more)
- Returns IntegrityResult with `ok`, human-readable `reason`, and per-check
  `checks` dict for debugging
- Never raises; pathological inputs return ok=False with explanation

Pipeline integration in `core/imports/pipeline.py:post_process_matched_download`:
- Hooks between the existing file-stability wait and AcoustID verification
- On failure: quarantine via existing `move_to_quarantine` helper, mark task
  failed with descriptive error, clear matched-context, fire
  `on_download_completed(success=False)` so the slot is released for retry
- Mirrors the existing AcoustID-failure path so retry behavior stays consistent
- Wrapped in try/except so an unexpected failure inside the check itself
  cannot block downloads — logs and continues

This is intentionally tier 1: universal across formats, no external deps.
A future tier could verify FLAC STREAMINFO MD5 by decoding audio (needs
flac binary or libflac wrapper) — skipped for now since tier 1 catches the
dominant Discord-reported cases (truncated, 0-byte, wrong file).

Tests:
- `tests/imports/test_file_integrity.py` — 14 cases covering all three check
  tiers, edge cases (zero/negative expected duration, long-track wider
  tolerance, caller tolerance override), and the mutagen-unavailable
  degradation path
- `tests/imports/test_import_pipeline.py` — two existing tests use 5-byte
  fixture files that the new check would reject; they monkeypatch the
  integrity check since they're testing plumbing (notification +
  metadata_runtime forwarding), not integrity behavior

WHATS_NEW entry under '2.4.2' dev cycle.
2026-05-03 08:21:01 -07:00