From 547b0d4e374c115021c0323ab2d93edcd27a53a9 Mon Sep 17 00:00:00 2001 From: Deivid Soto Date: Wed, 3 Jun 2026 18:55:49 +0200 Subject: [PATCH] fix(stream): retry thumbnail extraction with output-seek on seek-index failure Fast input seek (-ss before -i) fails on files whose seek index is imprecise or mildly corrupt: the demuxer lands mid-EBML element ("invalid as first byte of an EBML number") and decodes no frame, so the web scrubber showed a broken image (2026-06-03, anime MKVs: 15/15 prewarm thumbnails failed). When the fast path yields no frame, retry once with output seek (-ss after -i, decode from the start) + -err_detect ignore_err. Applied in both the on-demand handler (buildThumbnailArgsAccurate) and the prewarm extractor (ExtractThumbnailJPEG). Cost is paid only when the fast path fails, so healthy files keep the cheap path. Regression test: TestBuildThumbnailArgsAccurate. --- internal/engine/stream_server.go | 43 ++++++++++++++++--- internal/engine/thumbnail_test.go | 29 +++++++++++++ internal/library/mediainfo/sidecar.go | 59 ++++++++++++++++++++++++--- 3 files changed, 120 insertions(+), 11 deletions(-) diff --git a/internal/engine/stream_server.go b/internal/engine/stream_server.go index b382cd4..e3a7bd3 100644 --- a/internal/engine/stream_server.go +++ b/internal/engine/stream_server.go @@ -1050,12 +1050,23 @@ func (ss *StreamServer) thumbnailHandler(w http.ResponseWriter, r *http.Request) cmd.Stderr = &stderr out, err := cmd.Output() if err != nil || len(out) == 0 { - // A seek past EOF yields no frame — a benign empty output, not an error - // worth alarming on. Log at most a short line for diagnosis. - log.Printf("[thumbnail] no frame (pos=%.1f w=%d path=%q): err=%v %s", + // Fast input-seek (-ss before -i) can fail on files whose seek index is + // imprecise or mildly corrupt: the demuxer lands mid-EBML element + // ("invalid as first byte of an EBML number") and no frame decodes. + // Retry once with the slow but robust output-seek path before giving up + // (2026-06-03: anime MKVs returned a broken image in the web scrubber). + log.Printf("[thumbnail] input-seek failed (pos=%.1f w=%d path=%q): err=%v %s — retrying output-seek", pos, width, rawPath, err, strings.TrimSpace(stderr.String())) - http.Error(w, "thumbnail failed", http.StatusInternalServerError) - return + var stderr2 strings.Builder + cmd2 := exec.CommandContext(ctx, ss.ffmpegPath, buildThumbnailArgsAccurate(rawPath, pos, width)...) + cmd2.Stderr = &stderr2 + out, err = cmd2.Output() + if err != nil || len(out) == 0 { + log.Printf("[thumbnail] no frame after output-seek fallback (pos=%.1f w=%d path=%q): err=%v %s", + pos, width, rawPath, err, strings.TrimSpace(stderr2.String())) + http.Error(w, "thumbnail failed", http.StatusInternalServerError) + return + } } // Write-through so the next request (and trickplay re-hover) is a cache hit. if ss.cacheThumbnails { @@ -1194,6 +1205,28 @@ func buildThumbnailArgs(path string, posSec float64, width int) []string { } } +// buildThumbnailArgsAccurate is the robust fallback for files whose seek index +// is imprecise or mildly corrupt, where the fast input seek (-ss before -i) +// lands mid-EBML element and decodes no frame. `-ss` AFTER `-i` is an output +// (decode) seek — slower (decodes from the start) but reliable — and +// `-err_detect ignore_err` tolerates minor stream corruption encountered along +// the way. Only used after buildThumbnailArgs fails, so its extra cost is paid +// solely for files the fast path can't handle. +func buildThumbnailArgsAccurate(path string, posSec float64, width int) []string { + return []string{ + "-nostdin", + "-loglevel", "error", + "-err_detect", "ignore_err", + "-i", path, + "-ss", strconv.FormatFloat(posSec, 'f', 3, 64), + "-frames:v", "1", + "-vf", fmt.Sprintf("scale=%d:-2", width), + "-an", "-sn", + "-f", "mjpeg", + "pipe:1", + } +} + // parseThumbPos parses a non-negative seconds offset; defaults to 0 on garbage. func parseThumbPos(s string) float64 { if s == "" { diff --git a/internal/engine/thumbnail_test.go b/internal/engine/thumbnail_test.go index b309cf4..70be470 100644 --- a/internal/engine/thumbnail_test.go +++ b/internal/engine/thumbnail_test.go @@ -73,6 +73,35 @@ func TestBuildThumbnailArgs(t *testing.T) { } } +// buildThumbnailArgsAccurate is the robust fallback used when the fast input +// seek fails on a file with a corrupt/imprecise seek index (2026-06-03 +// broken-thumbnail bug on anime MKVs). It must use OUTPUT seek (-ss AFTER -i) +// so it decodes from the start, plus -err_detect ignore_err to tolerate minor +// stream corruption — the opposite of the fast buildThumbnailArgs. +func TestBuildThumbnailArgsAccurate(t *testing.T) { + args := buildThumbnailArgsAccurate("/x/movie.mkv", 123.5, 320) + joined := strings.Join(args, " ") + + ssIdx, iIdx := indexOfArg(args, "-ss"), indexOfArg(args, "-i") + if ssIdx < 0 || iIdx < 0 || ssIdx <= iIdx { + t.Errorf("-ss must come AFTER -i (output seek, robust fallback): %v", args) + } + if !strings.Contains(joined, "-err_detect ignore_err") { + t.Errorf("accurate args must tolerate stream errors (-err_detect ignore_err): %v", args) + } + if args[ssIdx+1] != "123.500" { + t.Errorf("pos arg = %q, want 123.500", args[ssIdx+1]) + } + if args[iIdx+1] != "/x/movie.mkv" { + t.Errorf("input arg = %q, want the path", args[iIdx+1]) + } + for _, want := range []string{"-frames:v 1", "scale=320:-2", "-f mjpeg", "pipe:1", "-an", "-sn"} { + if !strings.Contains(joined, want) { + t.Errorf("args missing %q: %v", want, args) + } + } +} + func TestParseThumbPos(t *testing.T) { cases := map[string]float64{"": 0, "abc": 0, "-5": 0, "0": 0, "12.5": 12.5, "600": 600} for in, want := range cases { diff --git a/internal/library/mediainfo/sidecar.go b/internal/library/mediainfo/sidecar.go index 111f535..47bd1a7 100644 --- a/internal/library/mediainfo/sidecar.go +++ b/internal/library/mediainfo/sidecar.go @@ -229,12 +229,35 @@ func WriteCachedThumbnail(mediaPath string, posSec float64, width int, jpeg []by } // ExtractThumbnailJPEG decodes ONE frame at posSec, scaled to `width`, as JPEG -// bytes. Mirrors engine.buildThumbnailArgs so the scan-time prewarm produces -// frames byte-identical to the on-demand handler (`-ss` before `-i` = fast -// input/keyframe seek). Shared by the prewarm; the handler keeps its own inline -// extraction (covered by thumbnail_test.go) and only reuses the cache helpers. +// bytes. The fast path mirrors engine.buildThumbnailArgs (`-ss` before `-i` = +// fast input/keyframe seek); on a seek-index failure both this prewarm path and +// the on-demand handler fall back to the identical output-seek argv +// (thumbnailArgsAccurate / engine.buildThumbnailArgsAccurate), so the two stay +// equivalent in both paths. Shared by the prewarm; the handler keeps its own +// inline extraction (engine package) and only reuses the cache helpers here. func ExtractThumbnailJPEG(ctx context.Context, ffmpegPath, mediaPath string, posSec float64, width int) ([]byte, error) { - args := []string{ + // Fast path: input seek (-ss before -i) — near-constant time, the common case. + out, err := runThumbnailFFmpeg(ctx, ffmpegPath, thumbnailArgsFast(mediaPath, posSec, width)) + if err == nil { + return out, nil + } + // Fallback: output seek (-ss after -i) + error tolerance. Slower (decodes + // from the start) but robust on files whose seek index is imprecise or + // mildly corrupt, where the fast input seek lands mid-EBML element + // ("invalid as first byte of an EBML number") and yields no frame + // (2026-06-03: anime MKVs failed every prewarm thumbnail). Paid only when + // the fast path fails, so healthy files keep the cheap path. + out, err2 := runThumbnailFFmpeg(ctx, ffmpegPath, thumbnailArgsAccurate(mediaPath, posSec, width)) + if err2 == nil { + return out, nil + } + return nil, fmt.Errorf("ffmpeg thumbnail extract: %w (output-seek fallback: %v)", err, err2) +} + +// thumbnailArgsFast is the input-seek (fast keyframe) thumbnail argv. Mirrors +// engine.buildThumbnailArgs so prewarm frames match the on-demand handler. +func thumbnailArgsFast(mediaPath string, posSec float64, width int) []string { + return []string{ "-nostdin", "-loglevel", "error", "-ss", strconv.FormatFloat(posSec, 'f', 3, 64), @@ -245,6 +268,30 @@ func ExtractThumbnailJPEG(ctx context.Context, ffmpegPath, mediaPath string, pos "-f", "mjpeg", "pipe:1", } +} + +// thumbnailArgsAccurate is the output-seek (decode-from-start) fallback with +// error tolerance. Mirrors engine.buildThumbnailArgsAccurate. +func thumbnailArgsAccurate(mediaPath string, posSec float64, width int) []string { + return []string{ + "-nostdin", + "-loglevel", "error", + "-err_detect", "ignore_err", + "-i", mediaPath, + "-ss", strconv.FormatFloat(posSec, 'f', 3, 64), + "-frames:v", "1", + "-vf", fmt.Sprintf("scale=%d:-2", width), + "-an", "-sn", + "-f", "mjpeg", + "pipe:1", + } +} + +// runThumbnailFFmpeg runs one ffmpeg thumbnail extraction and returns the JPEG +// bytes. setIdleIOPriority keeps the background prewarm from starving live +// playback I/O. An empty output (e.g. seek past EOF) is treated as an error so +// the caller can fall back. +func runThumbnailFFmpeg(ctx context.Context, ffmpegPath string, args []string) ([]byte, error) { cmd := exec.CommandContext(ctx, ffmpegPath, args...) var stdout, stderr bytes.Buffer cmd.Stdout = &stdout @@ -256,7 +303,7 @@ func ExtractThumbnailJPEG(ctx context.Context, ffmpegPath, mediaPath string, pos err := cmd.Wait() out := stdout.Bytes() if err != nil { - return nil, fmt.Errorf("ffmpeg thumbnail extract: %w: %s", err, strings.TrimSpace(stderr.String())) + return nil, fmt.Errorf("%w: %s", err, strings.TrimSpace(stderr.String())) } if len(out) == 0 { return nil, errors.New("ffmpeg produced no thumbnail (seek past EOF?)")