feat(health): classify playback impact of missing video segments#759
Merged
Conversation
Health checks and streaming failures previously treated any missing Usenet segment as binary corruption, triggering an ARR re-download even when the lost bytes only removed a few seconds of mid-stream video. Add a container- aware classifier that distinguishes fatal loss (MP4 ftyp/moov, MKV EBML header/Info/Tracks) from degraded loss (media payload, MKV seek index only) so playable-but-glitchy files are no longer needlessly repaired. - New internal/mediaprobe package: pure io.ReaderAt-based MP4 box walker and MKV EBML walker producing a Structure map and fatal/degraded/unknown verdicts. - Container structure is probed once during the immediate post-import health check and persisted as MediaStructure on FileMetadata, so later failures classify offline with zero network reads. Pre-existing files fall back to a bounded lazy probe (internal/usenet/reader_at.go) wrapped in a missing-range guard so it never blocks on dead articles. - usenet.ValidationResult now carries missing segments' file-coordinate byte ranges; DataCorruptionError carries the failing offset, enabling streaming failures to classify the same way. - New "degraded" health status (DB migration 033) and FILE_STATUS_DEGRADED: degraded files stay visible/streamable, skip ARR repair and the safety- folder move, and are exempt from streaming-failure masking. An in-flight repair still wins over a later degraded verdict. - Frontend: playback-impact chip on health rows/cards, Degraded stats card and filter, excluded from "requires action" alert counts. - Also fixes 4 pre-existing golangci-lint failures unrelated to this change (redundant type annotations, dead context.WithValue keys, unused scanIDs). Config: health.media_probe_enabled (default true) to disable probing.
effb105 to
bdc613f
Compare
Replace the hand-rolled MP4 box parser in internal/mediaprobe/mp4.go with abema/go-mp4's ReadBoxStructure. It already handles box-header edge cases (64-bit largesize, extends-to-EOF, versioned mvhd) and decodes mvhd for us, so our code only decides what each box means (critical/payload/seek-only) rather than also parsing the binary format. Behavior is unchanged — the full existing mp4_test.go suite (moov-at-end, largesize mdat, mvhd v1, garbage header, box-exceeds-file, missing-range propagation to VerdictUnknown, zero reads for non-video files) passes without modification. MKV stays hand-rolled: no comparably mature/maintained Go EBML library was worth the swap.
Replace the container-probe playback-impact classifier with the hole model from AIOStreams: a "hole" is a run of consecutive segments confirmed missing on every provider, and a fixed threshold table (run<=4, total<=64, <=2% of file) decides clean/degraded/failed. Small holes in video files are now zero-filled live during streaming so the file plays through the glitch instead of the read hard-failing. Why the rewrite: the probe approach cost 2-6 full article downloads per file to map MP4/MKV container structure. The hole model needs no container awareness and no probing — it decides purely from which segments are missing. - internal/holes: pure, dependency-free port of holes.ts (Classify, ClassifyProjected, Accumulator, caps, EligibleFile). Drops the go-mp4 dep. - On-the-fly zero-fill: UsenetReader gains optional HoleHooks (OnHole/ KnownHoles). A confirmed-missing segment the owner approves is served as zeros instead of erroring; known holes pre-pad without any fetch. Nil hooks preserve today's behavior for the importer/health readers. - Streaming (nzbfilesystem): per-handle hole accumulator seeded from the persisted map; padded files are marked degraded (visible + streamable, no repair trigger, no safety-folder move, exempt from masking) off the hot path. - Persistence: proto field 21 is now `repeated HoleRun known_holes` (replaces the removed MediaStructure); metadata.AddKnownHoles merges idempotently. - Health check = census: classify missing segments via the hole model (full check proves clean, sampled projects), persist observed holes on degraded, never erase persisted holes on a clean sample. - Tolerant imports: import.damage_policy (tolerant default | strict). A standalone video with small confirmed damage imports as degraded instead of being dropped; the post-import health check persists its holes. Archive sets and non-video stay strict. - Deletes the probe stack: internal/mediaprobe, usenet/reader_at.go, metadata/mediastructure.go, media_probe_enabled config, go-mp4 dependency. - Frontend: PlaybackImpactBadge + PlaybackImpact reshaped to the hole verdict.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Health checks and streaming failures currently treat any missing Usenet segment as binary corruption — the file is marked
corrupted/repair_triggered, its metadata is moved to the safety folder, and an ARR re-download is triggered, even when the missing segment only removes a couple of seconds of mid-stream video that a player would skip over. This adds container-aware classification so we can tell the difference.internal/mediaprobepackage — a pure, dependency-freeio.ReaderAt-based prober. Walks MP4 top-level boxes (ftyp/moov/mdat/...) and MKV's EBML top level (header/Info/Tracks/Cues/Cluster), classifying missing byte ranges as:fatal— overlaps a container-critical structure → unplayable → today's repair path.degraded— only hits media payload (or MKVCues, seek index only) → still playable, short glitch or broken seeking → no repair triggered.unknown— non-video, encrypted, unparseable, or a critical region itself unreadable → treated as fatal (safe fallback).nntppooldrains full article bodies, so probing costs a handful of full downloads and must never slow down import. The resultingMediaStructureis persisted onFileMetadata; later failures classify offline with zero network reads, even in cases where the critical segment itself later goes missing (the live probe alone could never determine that). Pre-existing files fall back to a bounded, timeout-guarded lazy probe.usenet.ValidationResultnow carries missing segments' file-coordinate byte ranges (prefix-sum of usable segment lengths);DataCorruptionErrorcarries the failing offset so streaming failures can classify the same way.degradedhealth status (DB migration 033, SQLite rebuild verified against the live schema + trigger/indexes) andFILE_STATUS_DEGRADEDmetadata status: degraded files stay visible and streamable (unlikecorrupted, which hides/blocks them), skip ARR repair and the metadata safety-folder move, and are exempt from streaming-failure masking. A repair already in flight still wins over a later degraded verdict.Playable (glitch ~12:34–12:36)/Unplayable) on health table rows/cards, a Degraded stats card and filter option, excluded from the "requires action" alert count.health.media_probe_enabled(defaulttrue) disables probing entirely.golangci-lintfailures unrelated to this change (redundant type annotations in two files, deadcontext.WithValuestring keys in the health checker replaced with structuredslogattrs, an unusedscanIDshelper).Test plan
internal/mediaprobe: ~34 table-driven cases with in-code MP4/MKV fixture builders (moov-at-end, 64-bitmdat, unknown-size Segment, Cues-before/after-Cluster, garbage headers, non-video extensions) — verified via atrackingReaderAtdouble that the walk never reads inside missing/media-payload ranges and stays within a small read budget.internal/usenet: missing-segment byte-range mapping (index + prefix-sum offsets, 50-entry cap) andSegmentsReaderAt(correct reads across segment boundaries, EOF handling, segment-cache dedup).internal/health: end-to-end health-checker integration tests over a fake NNTP pool — first-check structure persistence, no re-probe on subsequent checks, config-disabled probing, non-video/encrypted skip, degraded vs. fatal/unknown classification, and that stored-structure classification does zero downloads even when the critical segment is the one that died.internal/health/worker_degraded_test.go:prepareUpdateForResultdecision table — degraded skips repair, in-flight repair wins over a later degraded verdict, fatal/unknown/no-classification follow the unchanged retry path.internal/database: migration 033 runs the full chain, acceptsdegraded, rejects unknown statuses, and confirms the SQLite table rebuild preserved the update trigger and every pre-existing index.go test -race ./...— full suite green.bun run check&&bun run build— frontend typecheck/lint/build clean.make(generate, go mod tidy, golangci-lint, test-race) — green.