From d5a8cd52943658dea32a88467e7b15c149c1bd76 Mon Sep 17 00:00:00 2001 From: Tomislav Filipcic Date: Tue, 11 Aug 2026 18:13:06 +0200 Subject: [PATCH] fix(chapterthumbs): add software HDR tone-map fallback (#576) * fix(chapterthumbs): add software HDR tone-map fallback * fix(chapterthumbs): tighten filter capability detection * fix(chapterthumbs): harden tone-map fallback retries * fix(chapterthumbs): coalesce filter probes * fix(chapterthumbs): name disabled accelerator * feat(chapterthumbs): add CPU tone-map toggle * fix(config): reuse CPU tone-map setting key --------- Co-authored-by: Quick104 <31828688+Quick104@users.noreply.github.com> --- internal/chapterthumbs/extractor.go | 344 ++++++++-- internal/chapterthumbs/extractor_test.go | 611 ++++++++++++++++++ internal/chapterthumbs/remote.go | 32 +- internal/chapterthumbs/service.go | 73 ++- internal/chapterthumbs/service_test.go | 196 +++++- internal/config/admin_settings.go | 3 + internal/config/admin_settings_test.go | 8 + internal/playback/gpudetect.go | 11 +- internal/playback/gpudetect_test.go | 6 + internal/playback/remux.go | 2 +- internal/transcodenode/server.go | 13 +- web/src/lib/adminSettingsSearch.ts | 1 + .../admin-settings/PlaybackSettings.test.tsx | 72 +++ .../pages/admin-settings/PlaybackSettings.tsx | 11 + 14 files changed, 1262 insertions(+), 121 deletions(-) create mode 100644 internal/chapterthumbs/extractor_test.go create mode 100644 web/src/pages/admin-settings/PlaybackSettings.test.tsx diff --git a/internal/chapterthumbs/extractor.go b/internal/chapterthumbs/extractor.go index aaf8976d..1dd2052d 100644 --- a/internal/chapterthumbs/extractor.go +++ b/internal/chapterthumbs/extractor.go @@ -5,49 +5,193 @@ import ( "context" "errors" "fmt" + "os" "os/exec" + "path/filepath" "strings" + "sync" "time" "github.com/Silo-Server/silo-server/internal/playback" ) type FrameExtractOptions struct { - InputPath string - SeekSeconds float64 - FFmpegPath string - HWAccel string - HWDevice string - ToneMap bool - RunFunc func(ctx context.Context, ffmpegPath string, args []string) ([]byte, error) + InputPath string + SeekSeconds float64 + FFmpegPath string + HWAccel string + HWDevice string + ToneMap bool + AllowSoftwareToneMap bool + RunFunc func(ctx context.Context, ffmpegPath string, args []string) ([]byte, error) + + softwareToneMapResolver *softwareToneMapFilterResolver +} + +const ( + hwAccelNone = "none" + hwAccelQSV = "qsv" + hwAccelVAAPI = "vaapi" + reasonChapterExtractFailed = "chapter_extract_failed" + reasonDecodeInvalidData = "decode_invalid_data" + reasonFFmpegProbeFailed = "ffmpeg_probe_failed" + reasonToneMapUnsupported = "tonemap_unsupported" + softwareToneMapProbeTimeout = 3 * time.Second + softwareToneMapFilterBT2390 = "zscale=t=linear:npl=100,format=gbrpf32le,tonemapx=tonemap=bt2390,zscale=p=bt709:t=bt709:m=bt709:r=tv,format=yuv420p" + softwareToneMapFilterHable = "zscale=t=linear:npl=100,format=gbrpf32le,tonemap=hable,zscale=p=bt709:t=bt709:m=bt709:r=tv,format=yuv420p" +) + +type softwareToneMapProbeResult struct { + filter string + failureReason string + detail string + cacheable bool +} + +type softwareToneMapProbeCall struct { + done chan struct{} + result softwareToneMapProbeResult + waiters int +} + +type softwareToneMapFilterResolver struct { + mu sync.Mutex + byPath map[string]softwareToneMapProbeResult + inFlight map[string]*softwareToneMapProbeCall + probeFn func(ffmpegPath string) ([]byte, error) +} + +var defaultSoftwareToneMapFilterResolver = newSoftwareToneMapFilterResolver(runFFmpegFilterProbe) + +func newSoftwareToneMapFilterResolver(probeFn func(ffmpegPath string) ([]byte, error)) *softwareToneMapFilterResolver { + return &softwareToneMapFilterResolver{ + byPath: make(map[string]softwareToneMapProbeResult), + inFlight: make(map[string]*softwareToneMapProbeCall), + probeFn: probeFn, + } +} + +func (r *softwareToneMapFilterResolver) resolve(ffmpegPath string) (string, string, error) { + cacheKey := softwareToneMapCacheKey(ffmpegPath) + r.mu.Lock() + if result, ok := r.byPath[cacheKey]; ok { + r.mu.Unlock() + return softwareToneMapProbeResultValue(result) + } + if call, ok := r.inFlight[cacheKey]; ok { + call.waiters++ + r.mu.Unlock() + <-call.done + return softwareToneMapProbeResultValue(call.result) + } + call := &softwareToneMapProbeCall{done: make(chan struct{})} + r.inFlight[cacheKey] = call + r.mu.Unlock() + + result := probeSoftwareToneMapFilter(ffmpegPath, r.probeFn) + r.mu.Lock() + if result.cacheable { + r.byPath[cacheKey] = result + } + call.result = result + delete(r.inFlight, cacheKey) + close(call.done) + r.mu.Unlock() + return softwareToneMapProbeResultValue(result) +} + +func softwareToneMapCacheKey(ffmpegPath string) string { + if strings.ContainsRune(ffmpegPath, os.PathSeparator) { + if absolutePath, err := filepath.Abs(ffmpegPath); err == nil { + return filepath.Clean(absolutePath) + } + return filepath.Clean(ffmpegPath) + } + return ffmpegPath +} + +func softwareToneMapProbeResultValue(result softwareToneMapProbeResult) (string, string, error) { + if result.filter != "" { + return result.filter, "", nil + } + if result.failureReason == "" { + result.failureReason = reasonChapterExtractFailed + } + if result.detail == "" { + result.detail = "configured FFmpeg software HDR tone-map capability could not be determined" + } + return "", result.failureReason, errors.New(result.detail) +} + +func probeSoftwareToneMapFilter( + ffmpegPath string, + probeFn func(ffmpegPath string) ([]byte, error), +) softwareToneMapProbeResult { + if probeFn == nil { + return softwareToneMapProbeResult{ + failureReason: reasonFFmpegProbeFailed, + detail: "FFmpeg filter probe is unavailable", + } + } + output, err := probeFn(ffmpegPath) + if err != nil { + return softwareToneMapProbeResult{ + failureReason: reasonFFmpegProbeFailed, + detail: "FFmpeg filter probe failed: " + playback.FormatFFmpegProbeFailure(err, output), + } + } + if !ffmpegFilterOutputHasToken(output, "zscale") { + return softwareToneMapProbeResult{ + failureReason: reasonToneMapUnsupported, + detail: "configured FFmpeg lacks the required zscale filter", + cacheable: true, + } + } + if ffmpegFilterOutputHasToken(output, "tonemapx") { + return softwareToneMapProbeResult{filter: softwareToneMapFilterBT2390, cacheable: true} + } + if ffmpegFilterOutputHasToken(output, "tonemap") { + return softwareToneMapProbeResult{filter: softwareToneMapFilterHable, cacheable: true} + } + return softwareToneMapProbeResult{ + failureReason: reasonToneMapUnsupported, + detail: "configured FFmpeg lacks the required tonemapx or tonemap filter", + cacheable: true, + } } func ExtractFrame(ctx context.Context, opts FrameExtractOptions) ([]byte, string, error) { - ffmpegPath := opts.FFmpegPath - if ffmpegPath == "" { - ffmpegPath = "ffmpeg" - } + ffmpegPath := playback.ResolveFFmpegPath(opts.FFmpegPath) runExtract := opts.RunFunc if runExtract == nil { runExtract = runFFmpegFrameExtract } - - resolvedAccel := playback.ResolveHWAccelWithFFmpeg(opts.HWAccel, ffmpegPath) - // Resolve a multi-device hw_device list to one concrete GPU for this - // extraction; the reservation spans only the hardware attempt below. - resolvedDevice, releaseHWDevice := playback.AcquireHWDevice(opts.HWDevice, resolvedAccel) - defer releaseHWDevice() - if resolvedDevice == "" && (resolvedAccel == "qsv" || resolvedAccel == "vaapi") { - resolvedDevice = playback.PickRenderDevice("") + softwareToneMapResolver := opts.softwareToneMapResolver + if softwareToneMapResolver == nil { + softwareToneMapResolver = defaultSoftwareToneMapFilterResolver + } + cpuOpts := cpuFrameExtractOptions{ + ctx: ctx, + inputPath: opts.InputPath, + seekSeconds: opts.SeekSeconds, + toneMap: opts.ToneMap, + runExtract: runExtract, + ffmpegPath: ffmpegPath, + softwareToneMapResolver: softwareToneMapResolver, } - if resolvedAccel != "none" { + resolvedAccel := playback.ResolveHWAccelWithFFmpeg(opts.HWAccel, ffmpegPath) + if supportsHardwareFrameExtract(resolvedAccel) { + // Resolve a multi-device hw_device list to one concrete GPU for this + // extraction; the reservation spans only the hardware attempt below. + resolvedDevice, releaseHWDevice := playback.AcquireHWDevice(opts.HWDevice, resolvedAccel) + defer releaseHWDevice() + if resolvedDevice == "" { + resolvedDevice = playback.PickRenderDevice("") + } + args, buildErr := buildFrameExtractArgs(opts.InputPath, opts.SeekSeconds, resolvedAccel, resolvedDevice, opts.ToneMap) - if buildErr != nil { - if opts.ToneMap { - return nil, "tonemap_unsupported", wrapReason("tonemap_unsupported", buildErr) - } - } else { + if buildErr == nil { attemptCtx, cancel := context.WithTimeout(ctx, extractTimeoutForAttempt(true, opts.ToneMap)) data, err := runExtract(attemptCtx, ffmpegPath, args) cancel() @@ -57,44 +201,107 @@ func ExtractFrame(ctx context.Context, opts FrameExtractOptions) ([]byte, string } hwReason := classifyExtractError("hw", err) - if opts.ToneMap { + if hwReason == reasonDecodeInvalidData { return nil, hwReason, wrapReason(hwReason, err) } - - cpuData, cpuReason, cpuErr := extractFrameCPU( - ctx, - opts.InputPath, - opts.SeekSeconds, - false, - runExtract, - ffmpegPath, - ) - if cpuErr == nil { - return cpuData, "", nil + if opts.ToneMap && !opts.AllowSoftwareToneMap { + return nil, hwReason, wrapReason(hwReason, err) } - return nil, cpuReason, fmt.Errorf("hardware extraction failed: %v; cpu fallback failed: %w", wrapReason(hwReason, err), cpuErr) + return extractFrameCPUFallback( + cpuOpts, + hwReason, + err, + ) } + + releaseHWDevice() + if opts.ToneMap && !opts.AllowSoftwareToneMap { + return nil, reasonToneMapUnsupported, wrapReason(reasonToneMapUnsupported, buildErr) + } + return extractFrameCPUFallback( + cpuOpts, + reasonChapterExtractFailed, + buildErr, + ) } - if opts.ToneMap { - err := errors.New("hardware HDR tone mapping unavailable") - return nil, "tonemap_unsupported", wrapReason("tonemap_unsupported", err) + if opts.ToneMap && !opts.AllowSoftwareToneMap { + err := errors.New("software HDR tone mapping is disabled") + return nil, reasonToneMapUnsupported, wrapReason(reasonToneMapUnsupported, err) } - return extractFrameCPU(ctx, opts.InputPath, opts.SeekSeconds, opts.ToneMap, runExtract, ffmpegPath) + if resolvedAccel != hwAccelNone && !opts.ToneMap { + return extractFrameUnsupportedSDRWithRetry(cpuOpts) + } + + return extractFrameCPU(cpuOpts) +} + +func supportsHardwareFrameExtract(hwAccel string) bool { + return hwAccel == hwAccelQSV || hwAccel == hwAccelVAAPI +} + +type cpuFrameExtractOptions struct { + ctx context.Context + inputPath string + seekSeconds float64 + toneMap bool + runExtract func(ctx context.Context, ffmpegPath string, args []string) ([]byte, error) + ffmpegPath string + softwareToneMapResolver *softwareToneMapFilterResolver +} + +func extractFrameUnsupportedSDRWithRetry(opts cpuFrameExtractOptions) ([]byte, string, error) { + attemptCtx, cancel := context.WithTimeout(opts.ctx, extractTimeoutForAttempt(true, false)) + data, err := opts.runExtract( + attemptCtx, + opts.ffmpegPath, + buildCPUFrameExtractArgs(opts.inputPath, opts.seekSeconds, ""), + ) + cancel() + if err == nil { + return data, "", nil + } + + hwReason := classifyExtractError("hw", err) + return extractFrameCPUFallback(opts, hwReason, err) +} + +func extractFrameCPUFallback( + opts cpuFrameExtractOptions, + hwReason string, + hwErr error, +) ([]byte, string, error) { + cpuData, cpuReason, cpuErr := extractFrameCPU(opts) + if cpuErr == nil { + return cpuData, "", nil + } + return nil, cpuReason, fmt.Errorf( + "hardware extraction failed: %w; cpu fallback failed: %w", + wrapReason(hwReason, hwErr), + cpuErr, + ) } func extractFrameCPU( - ctx context.Context, - inputPath string, - seekSeconds float64, - toneMap bool, - runExtract func(ctx context.Context, ffmpegPath string, args []string) ([]byte, error), - ffmpegPath string, + opts cpuFrameExtractOptions, ) ([]byte, string, error) { - attemptCtx, cancel := context.WithTimeout(ctx, extractTimeoutForAttempt(false, toneMap)) + softwareToneMapFilter := "" + if opts.toneMap { + filter, reason, err := opts.softwareToneMapResolver.resolve(opts.ffmpegPath) + if err != nil { + return nil, reason, wrapReason(reason, err) + } + softwareToneMapFilter = filter + } + + attemptCtx, cancel := context.WithTimeout(opts.ctx, extractTimeoutForAttempt(false, opts.toneMap)) defer cancel() - data, err := runExtract(attemptCtx, ffmpegPath, buildCPUFrameExtractArgs(inputPath, seekSeconds, toneMap)) + data, err := opts.runExtract( + attemptCtx, + opts.ffmpegPath, + buildCPUFrameExtractArgs(opts.inputPath, opts.seekSeconds, softwareToneMapFilter), + ) if err != nil { reason := classifyExtractError("cpu", err) return nil, reason, wrapReason(reason, err) @@ -110,12 +317,12 @@ func classifyExtractError(stage string, err error) string { lower := strings.ToLower(message) switch { case strings.Contains(message, "No such filter") || strings.Contains(message, "tonemap") && strings.Contains(message, "Error"): - return "tonemap_unsupported" + return reasonToneMapUnsupported case strings.Contains(lower, "invalid nal unit size"), strings.Contains(lower, "error splitting the input into nal units"), strings.Contains(lower, "invalid data found when processing input"), strings.Contains(lower, "invalid as first byte of an ebml number"): - return "decode_invalid_data" + return reasonDecodeInvalidData case stage == "hw" && strings.Contains(message, "signal: killed"): return "hw_killed" case stage == "hw" && isDeadlineError(err): @@ -123,7 +330,7 @@ func classifyExtractError(stage string, err error) string { case stage == "cpu" && isDeadlineError(err): return "cpu_timeout" default: - return "chapter_extract_failed" + return reasonChapterExtractFailed } } @@ -147,15 +354,15 @@ func extractTimeoutForAttempt(hardware bool, hdr bool) time.Duration { return cpuExtractTimeoutSDR } -func buildCPUFrameExtractArgs(inputPath string, seekSeconds float64, toneMap bool) []string { +func buildCPUFrameExtractArgs(inputPath string, seekSeconds float64, softwareToneMapFilter string) []string { args := []string{ "-hide_banner", "-loglevel", "error", "-ss", fmt.Sprintf("%.3f", seekSeconds), "-i", inputPath, } - if toneMap { - args = append(args, "-vf", "zscale=t=linear:npl=100,format=gbrpf32le,tonemap=bt2390,zscale=p=bt709:t=bt709:m=bt709:r=tv,format=yuv420p") + if softwareToneMapFilter != "" { + args = append(args, "-vf", softwareToneMapFilter) } args = append(args, "-frames:v", "1", @@ -172,7 +379,7 @@ func buildFrameExtractArgs(inputPath string, seekSeconds float64, hwAccel string "-loglevel", "error", } switch hwAccel { - case "qsv": + case hwAccelQSV: if hwDevice == "" { return nil, fmt.Errorf("qsv requires a render device") } @@ -183,7 +390,7 @@ func buildFrameExtractArgs(inputPath string, seekSeconds float64, hwAccel string "-hwaccel", "vaapi", "-hwaccel_output_format", "vaapi", ) - case "vaapi": + case hwAccelVAAPI: if hwDevice == "" { return nil, fmt.Errorf("vaapi requires a render device") } @@ -194,7 +401,7 @@ func buildFrameExtractArgs(inputPath string, seekSeconds float64, hwAccel string "-hwaccel_output_format", "vaapi", ) default: - return buildCPUFrameExtractArgs(inputPath, seekSeconds, toneMap), nil + return nil, fmt.Errorf("hardware chapter thumbnail extraction does not support %q", hwAccel) } filter := "hwdownload,format=nv12" @@ -213,6 +420,25 @@ func buildFrameExtractArgs(inputPath string, seekSeconds float64, hwAccel string return args, nil } +func runFFmpegFilterProbe(ffmpegPath string) ([]byte, error) { + ctx, cancel := context.WithTimeout(context.Background(), softwareToneMapProbeTimeout) + defer cancel() + return exec.CommandContext(ctx, ffmpegPath, "-hide_banner", "-filters").CombinedOutput() +} + +func ffmpegFilterOutputHasToken(output []byte, token string) bool { + for _, line := range strings.Split(string(output), "\n") { + fields := strings.Fields(line) + if len(fields) < 3 || !strings.Contains(fields[2], "->") { + continue + } + if strings.EqualFold(fields[1], token) { + return true + } + } + return false +} + func runFFmpegFrameExtract(ctx context.Context, ffmpegPath string, args []string) ([]byte, error) { cmd := exec.CommandContext(ctx, ffmpegPath, args...) var stdout, stderr bytes.Buffer diff --git a/internal/chapterthumbs/extractor_test.go b/internal/chapterthumbs/extractor_test.go new file mode 100644 index 00000000..0c9b10da --- /dev/null +++ b/internal/chapterthumbs/extractor_test.go @@ -0,0 +1,611 @@ +package chapterthumbs + +import ( + "context" + "errors" + "path/filepath" + "runtime" + "slices" + "strings" + "sync" + "sync/atomic" + "testing" + "time" +) + +func TestSoftwareToneMapFilterResolver(t *testing.T) { + tests := []struct { + name string + output string + probeErr error + wantFilter string + wantReason string + wantError string + wantCalls int + }{ + { + name: "prefers tonemapx bt2390", + output: " .S. zscale V->V\n .S. tonemap V->V\n .S. tonemapx V->V\n", + wantFilter: softwareToneMapFilterBT2390, + wantCalls: 1, + }, + { + name: "falls back to standard hable", + output: " .S. zscale V->V\n .S. tonemap V->V\n", + wantFilter: softwareToneMapFilterHable, + wantCalls: 1, + }, + { + name: "requires zscale", + output: " .S. tonemapx V->V\n", + wantReason: reasonToneMapUnsupported, + wantError: "lacks the required zscale filter", + wantCalls: 1, + }, + { + name: "requires a tone map filter", + output: " .S. zscale V->V Description mentions tonemap but does not provide it.\n", + wantReason: reasonToneMapUnsupported, + wantError: "lacks the required tonemapx or tonemap filter", + wantCalls: 1, + }, + { + name: "does not cache transient probe failure", + output: "probe stderr", + probeErr: errors.New("exit status 1"), + wantReason: reasonFFmpegProbeFailed, + wantError: "FFmpeg filter probe failed: exit status 1: probe stderr", + wantCalls: 2, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + calls := 0 + resolver := newSoftwareToneMapFilterResolver(func(ffmpegPath string) ([]byte, error) { + calls++ + if cacheKey := softwareToneMapCacheKey(ffmpegPath); cacheKey != "/test/ffmpeg" { + t.Fatalf("ffmpegPath = %q, cache key = %q, want /test/ffmpeg", ffmpegPath, cacheKey) + } + return []byte(tt.output), tt.probeErr + }) + + for _, ffmpegPath := range []string{"/test/ffmpeg", "/test/../test/ffmpeg"} { + filter, reason, err := resolver.resolve(ffmpegPath) + if tt.wantError != "" { + if err == nil || !strings.Contains(err.Error(), tt.wantError) { + t.Fatalf("resolve() error = %v, want containing %q", err, tt.wantError) + } + if reason != tt.wantReason { + t.Fatalf("resolve() reason = %q, want %q", reason, tt.wantReason) + } + continue + } + if err != nil { + t.Fatalf("resolve() error = %v", err) + } + if filter != tt.wantFilter { + t.Fatalf("resolve() filter = %q, want %q", filter, tt.wantFilter) + } + if reason != "" { + t.Fatalf("resolve() reason = %q, want empty", reason) + } + } + if calls != tt.wantCalls { + t.Fatalf("probe calls = %d, want %d", calls, tt.wantCalls) + } + }) + } +} + +func TestSoftwareToneMapFilterResolverKeepsExecutableSeparateFromCacheKey(t *testing.T) { + workingDirectory, err := filepath.Abs(".") + if err != nil { + t.Fatalf("resolve working directory: %v", err) + } + + calls := 0 + resolver := newSoftwareToneMapFilterResolver(func(ffmpegPath string) ([]byte, error) { + calls++ + if ffmpegPath != "./ffmpeg" { + t.Fatalf("probe path = %q, want ./ffmpeg", ffmpegPath) + } + return []byte(" .S. zscale V->V\n .S. tonemap V->V\n"), nil + }) + + for _, ffmpegPath := range []string{"./ffmpeg", filepath.Join(workingDirectory, "ffmpeg")} { + filter, reason, err := resolver.resolve(ffmpegPath) + if err != nil || reason != "" || filter != softwareToneMapFilterHable { + t.Fatalf("resolve(%q) = %q, %q, %v", ffmpegPath, filter, reason, err) + } + } + if calls != 1 { + t.Fatalf("probe calls = %d, want 1 cached call", calls) + } +} + +func TestSoftwareToneMapFilterResolverCoalescesTransientProbeFailures(t *testing.T) { + const callers = 8 + + probeStarted := make(chan struct{}) + releaseProbe := make(chan struct{}) + var probeCalls atomic.Int32 + resolver := newSoftwareToneMapFilterResolver(func(string) ([]byte, error) { + if probeCalls.Add(1) == 1 { + close(probeStarted) + <-releaseProbe + return []byte("temporary stderr"), errors.New("resource temporarily unavailable") + } + return []byte(" .S. zscale V->V\n .S. tonemap V->V\n"), nil + }) + + type resolveResult struct { + filter string + reason string + err error + } + results := make(chan resolveResult, callers) + start := make(chan struct{}) + var callersReady sync.WaitGroup + callersReady.Add(callers) + for range callers { + go func() { + callersReady.Done() + <-start + filter, reason, err := resolver.resolve("/test/ffmpeg") + results <- resolveResult{filter: filter, reason: reason, err: err} + }() + } + callersReady.Wait() + close(start) + <-probeStarted + + deadline := time.Now().Add(time.Second) + for { + resolver.mu.Lock() + call := resolver.inFlight["/test/ffmpeg"] + waiters := 0 + if call != nil { + waiters = call.waiters + } + resolver.mu.Unlock() + if waiters == callers-1 { + break + } + if time.Now().After(deadline) { + t.Fatalf("coalesced waiters = %d, want %d", waiters, callers-1) + } + runtime.Gosched() + } + close(releaseProbe) + + for range callers { + result := <-results + if result.filter != "" || result.reason != reasonFFmpegProbeFailed || result.err == nil { + t.Fatalf("resolve() = %q, %q, %v, want shared transient failure", result.filter, result.reason, result.err) + } + } + if calls := probeCalls.Load(); calls != 1 { + t.Fatalf("concurrent probe calls = %d, want 1", calls) + } + + filter, reason, err := resolver.resolve("/test/ffmpeg") + if err != nil || reason != "" || filter != softwareToneMapFilterHable { + t.Fatalf("retry resolve() = %q, %q, %v", filter, reason, err) + } + if calls := probeCalls.Load(); calls != 2 { + t.Fatalf("probe calls after retry = %d, want 2", calls) + } + + filter, reason, err = resolver.resolve("/test/ffmpeg") + if err != nil || reason != "" || filter != softwareToneMapFilterHable { + t.Fatalf("cached resolve() = %q, %q, %v", filter, reason, err) + } + if calls := probeCalls.Load(); calls != 2 { + t.Fatalf("probe calls after cached resolve = %d, want 2", calls) + } +} + +func TestExtractFrameSoftwareHDRWithoutHardware(t *testing.T) { + resolver := resolverWithFilters(t, "zscale", "tonemapx") + var remaining time.Duration + data, reason, err := ExtractFrame(context.Background(), FrameExtractOptions{ + InputPath: "/media/movie.mkv", + SeekSeconds: 42.5, + FFmpegPath: "/test/ffmpeg", + HWAccel: hwAccelNone, + ToneMap: true, + AllowSoftwareToneMap: true, + softwareToneMapResolver: resolver, + RunFunc: func(ctx context.Context, _ string, args []string) ([]byte, error) { + deadline, ok := ctx.Deadline() + if !ok { + t.Fatal("software HDR extraction has no deadline") + } + remaining = time.Until(deadline) + if !slices.Contains(args, softwareToneMapFilterBT2390) { + t.Fatalf("software extraction args missing BT.2390 filter: %#v", args) + } + return []byte("frame"), nil + }, + }) + if err != nil { + t.Fatalf("ExtractFrame() error = %v", err) + } + if reason != "" { + t.Fatalf("ExtractFrame() reason = %q, want empty", reason) + } + if string(data) != "frame" { + t.Fatalf("ExtractFrame() data = %q, want frame", data) + } + assertApproximateDeadline(t, remaining, cpuExtractTimeoutHDR) +} + +func TestExtractFrameSoftwareHDRDisabledByDefault(t *testing.T) { + resolver := newSoftwareToneMapFilterResolver(func(string) ([]byte, error) { + t.Fatal("software filter probe should not run while CPU tone mapping is disabled") + return nil, nil + }) + + _, reason, err := ExtractFrame(context.Background(), FrameExtractOptions{ + InputPath: "/media/movie.mkv", + FFmpegPath: "/test/ffmpeg", + HWAccel: hwAccelNone, + ToneMap: true, + softwareToneMapResolver: resolver, + RunFunc: func(context.Context, string, []string) ([]byte, error) { + t.Fatal("CPU extraction should not run while CPU tone mapping is disabled") + return nil, nil + }, + }) + if err == nil || !strings.Contains(err.Error(), "software HDR tone mapping is disabled") { + t.Fatalf("ExtractFrame() error = %v, want disabled software tone-map error", err) + } + if reason != reasonToneMapUnsupported { + t.Fatalf("ExtractFrame() reason = %q, want %q", reason, reasonToneMapUnsupported) + } +} + +func TestExtractFrameUnsupportedHardwareUsesCPU(t *testing.T) { + var args []string + data, reason, err := ExtractFrame(context.Background(), FrameExtractOptions{ + InputPath: "/media/movie.mkv", + SeekSeconds: 42.5, + HWAccel: "nvenc", + RunFunc: func(_ context.Context, _ string, got []string) ([]byte, error) { + args = append([]string(nil), got...) + return []byte("frame"), nil + }, + }) + if err != nil || reason != "" || string(data) != "frame" { + t.Fatalf("ExtractFrame() = %q, %q, %v", data, reason, err) + } + joined := strings.Join(args, " ") + if strings.Contains(joined, "-hwaccel") || strings.Contains(joined, "nvenc") { + t.Fatalf("unsupported chapter-thumbnail accelerator used hardware args: %s", joined) + } +} + +func TestExtractFrameUnsupportedHardwarePreservesSDRRetry(t *testing.T) { + calls := 0 + var firstRemaining, retryRemaining time.Duration + data, reason, err := ExtractFrame(context.Background(), FrameExtractOptions{ + InputPath: "/media/movie.mkv", + SeekSeconds: 42.5, + HWAccel: "nvenc", + RunFunc: func(ctx context.Context, _ string, args []string) ([]byte, error) { + calls++ + deadline, ok := ctx.Deadline() + if !ok { + t.Fatalf("extraction attempt %d has no deadline", calls) + } + if strings.Contains(strings.Join(args, " "), "-hwaccel") { + t.Fatalf("unsupported accelerator used hardware args: %#v", args) + } + if calls == 1 { + firstRemaining = time.Until(deadline) + return nil, errors.New("transient software decode failure") + } + retryRemaining = time.Until(deadline) + return []byte("frame"), nil + }, + }) + if err != nil || reason != "" || string(data) != "frame" { + t.Fatalf("ExtractFrame() = %q, %q, %v", data, reason, err) + } + if calls != 2 { + t.Fatalf("extract calls = %d, want 2", calls) + } + assertApproximateDeadline(t, firstRemaining, hwExtractTimeoutSDR) + assertApproximateDeadline(t, retryRemaining, cpuExtractTimeoutSDR) +} + +func TestExtractFrameProbesAndRunsSameRelativeFFmpeg(t *testing.T) { + var probedPath, extractPath string + resolver := newSoftwareToneMapFilterResolver(func(ffmpegPath string) ([]byte, error) { + probedPath = ffmpegPath + return []byte(" .S. zscale V->V\n .S. tonemap V->V\n"), nil + }) + + _, reason, err := ExtractFrame(context.Background(), FrameExtractOptions{ + InputPath: "/media/movie.mkv", + FFmpegPath: " ./ffmpeg ", + HWAccel: hwAccelNone, + ToneMap: true, + AllowSoftwareToneMap: true, + softwareToneMapResolver: resolver, + RunFunc: func(_ context.Context, ffmpegPath string, _ []string) ([]byte, error) { + extractPath = ffmpegPath + return []byte("frame"), nil + }, + }) + if err != nil || reason != "" { + t.Fatalf("ExtractFrame() reason = %q, error = %v", reason, err) + } + if probedPath != "./ffmpeg" || extractPath != "./ffmpeg" { + t.Fatalf("probe path = %q, extract path = %q, want ./ffmpeg for both", probedPath, extractPath) + } +} + +func TestExtractFrameRetriesTransientSoftwareProbeFailure(t *testing.T) { + probeCalls := 0 + resolver := newSoftwareToneMapFilterResolver(func(string) ([]byte, error) { + probeCalls++ + if probeCalls == 1 { + return []byte("temporary stderr"), errors.New("resource temporarily unavailable") + } + return []byte(" .S. zscale V->V\n .S. tonemap V->V\n"), nil + }) + opts := FrameExtractOptions{ + InputPath: "/media/movie.mkv", + FFmpegPath: "/test/ffmpeg", + HWAccel: hwAccelNone, + ToneMap: true, + AllowSoftwareToneMap: true, + softwareToneMapResolver: resolver, + RunFunc: func(context.Context, string, []string) ([]byte, error) { + return []byte("frame"), nil + }, + } + + if _, reason, err := ExtractFrame(context.Background(), opts); err == nil || reason != reasonFFmpegProbeFailed { + t.Fatalf("first ExtractFrame() reason = %q, error = %v, want retryable probe failure", reason, err) + } + data, reason, err := ExtractFrame(context.Background(), opts) + if err != nil || reason != "" || string(data) != "frame" { + t.Fatalf("second ExtractFrame() = %q, %q, %v", data, reason, err) + } + if probeCalls != 2 { + t.Fatalf("probe calls = %d, want 2", probeCalls) + } +} + +func TestExtractFrameHardwareHDRSuccessSkipsSoftwareProbe(t *testing.T) { + resolver := newSoftwareToneMapFilterResolver(func(string) ([]byte, error) { + t.Fatal("software filter probe should not run after hardware success") + return nil, nil + }) + calls := 0 + data, reason, err := ExtractFrame(context.Background(), FrameExtractOptions{ + InputPath: "/media/movie.mkv", + SeekSeconds: 42.5, + HWAccel: "vaapi", + HWDevice: "/dev/dri/renderD128", + ToneMap: true, + softwareToneMapResolver: resolver, + RunFunc: func(_ context.Context, _ string, args []string) ([]byte, error) { + calls++ + if !strings.Contains(strings.Join(args, " "), "tonemap_vaapi") { + t.Fatalf("hardware extraction args missing VAAPI tone map: %#v", args) + } + return []byte("frame"), nil + }, + }) + if err != nil || reason != "" || string(data) != "frame" { + t.Fatalf("ExtractFrame() = %q, %q, %v", data, reason, err) + } + if calls != 1 { + t.Fatalf("extract calls = %d, want 1", calls) + } +} + +func TestExtractFrameHardwareHDRFailureDoesNotUseSoftwareWhenDisabled(t *testing.T) { + resolver := newSoftwareToneMapFilterResolver(func(string) ([]byte, error) { + t.Fatal("software filter probe should not run while CPU tone mapping is disabled") + return nil, nil + }) + calls := 0 + _, reason, err := ExtractFrame(context.Background(), FrameExtractOptions{ + InputPath: "/media/movie.mkv", + HWAccel: hwAccelVAAPI, + HWDevice: "/dev/dri/renderD128", + ToneMap: true, + softwareToneMapResolver: resolver, + RunFunc: func(context.Context, string, []string) ([]byte, error) { + calls++ + return nil, context.DeadlineExceeded + }, + }) + if err == nil { + t.Fatal("ExtractFrame() error = nil, want hardware failure") + } + if reason != "hw_timeout" { + t.Fatalf("ExtractFrame() reason = %q, want hw_timeout", reason) + } + if calls != 1 { + t.Fatalf("extract calls = %d, want hardware attempt only", calls) + } +} + +func TestExtractFrameHardwareHDRFailureFallsBackWithFreshDeadline(t *testing.T) { + resolver := resolverWithFilters(t, "zscale", "tonemapx") + calls := 0 + var hwRemaining, cpuRemaining time.Duration + data, reason, err := ExtractFrame(context.Background(), FrameExtractOptions{ + InputPath: "/media/movie.mkv", + SeekSeconds: 42.5, + HWAccel: "vaapi", + HWDevice: "/dev/dri/renderD128", + ToneMap: true, + AllowSoftwareToneMap: true, + softwareToneMapResolver: resolver, + RunFunc: func(ctx context.Context, _ string, args []string) ([]byte, error) { + calls++ + deadline, ok := ctx.Deadline() + if !ok { + t.Fatalf("extraction attempt %d has no deadline", calls) + } + if calls == 1 { + hwRemaining = time.Until(deadline) + return nil, context.DeadlineExceeded + } + cpuRemaining = time.Until(deadline) + if !slices.Contains(args, softwareToneMapFilterBT2390) { + t.Fatalf("CPU fallback args missing BT.2390 filter: %#v", args) + } + return []byte("frame"), nil + }, + }) + if err != nil || reason != "" || string(data) != "frame" { + t.Fatalf("ExtractFrame() = %q, %q, %v", data, reason, err) + } + if calls != 2 { + t.Fatalf("extract calls = %d, want 2", calls) + } + assertApproximateDeadline(t, hwRemaining, hwExtractTimeoutHDR) + assertApproximateDeadline(t, cpuRemaining, cpuExtractTimeoutHDR) +} + +func TestExtractFrameInvalidHardwareMediaDoesNotRetryOnCPU(t *testing.T) { + resolver := newSoftwareToneMapFilterResolver(func(string) ([]byte, error) { + t.Fatal("software filter probe should not run for invalid media") + return nil, nil + }) + calls := 0 + _, reason, err := ExtractFrame(context.Background(), FrameExtractOptions{ + InputPath: "/media/movie.mkv", + SeekSeconds: 42.5, + HWAccel: "vaapi", + HWDevice: "/dev/dri/renderD128", + ToneMap: true, + softwareToneMapResolver: resolver, + RunFunc: func(context.Context, string, []string) ([]byte, error) { + calls++ + return nil, errors.New("Invalid NAL unit size") + }, + }) + if err == nil { + t.Fatal("ExtractFrame() error = nil, want invalid-media error") + } + if reason != "decode_invalid_data" { + t.Fatalf("ExtractFrame() reason = %q, want decode_invalid_data", reason) + } + if calls != 1 { + t.Fatalf("extract calls = %d, want 1", calls) + } +} + +func TestExtractFrameMissingSoftwareFiltersIsActionable(t *testing.T) { + resolver := resolverWithFilters(t, "zscale") + calls := 0 + _, reason, err := ExtractFrame(context.Background(), FrameExtractOptions{ + InputPath: "/media/movie.mkv", + SeekSeconds: 42.5, + HWAccel: hwAccelNone, + ToneMap: true, + AllowSoftwareToneMap: true, + softwareToneMapResolver: resolver, + RunFunc: func(context.Context, string, []string) ([]byte, error) { + calls++ + return nil, nil + }, + }) + if err == nil || !strings.Contains(err.Error(), "lacks the required tonemapx or tonemap filter") { + t.Fatalf("ExtractFrame() error = %v, want actionable missing-filter error", err) + } + if reason != "tonemap_unsupported" { + t.Fatalf("ExtractFrame() reason = %q, want tonemap_unsupported", reason) + } + if calls != 0 { + t.Fatalf("extract calls = %d, want 0", calls) + } +} + +func TestExtractFramePreservesHardwareAndCPUFailures(t *testing.T) { + resolver := resolverWithFilters(t, "zscale", "tonemap") + calls := 0 + _, reason, err := ExtractFrame(context.Background(), FrameExtractOptions{ + InputPath: "/media/movie.mkv", + SeekSeconds: 42.5, + HWAccel: "vaapi", + HWDevice: "/dev/dri/renderD128", + ToneMap: true, + AllowSoftwareToneMap: true, + softwareToneMapResolver: resolver, + RunFunc: func(context.Context, string, []string) ([]byte, error) { + calls++ + if calls == 1 { + return nil, context.DeadlineExceeded + } + return nil, errors.New("software decode failed") + }, + }) + if err == nil { + t.Fatal("ExtractFrame() error = nil, want combined failure") + } + if reason != "chapter_extract_failed" { + t.Fatalf("ExtractFrame() reason = %q, want chapter_extract_failed", reason) + } + for _, want := range []string{"hardware extraction failed", "hw_timeout", "cpu fallback failed", "software decode failed"} { + if !strings.Contains(err.Error(), want) { + t.Fatalf("ExtractFrame() error = %q, want containing %q", err, want) + } + } +} + +func TestRemoteExtractTimeoutBudgetsOnlyAllowedAttempts(t *testing.T) { + sdrExtractBudget := extractTimeoutForAttempt(true, false) + extractTimeoutForAttempt(false, false) + if got := remoteExtractTimeout(false, false); got <= sdrExtractBudget { + t.Fatalf("remoteExtractTimeout(SDR) = %s, want more than extract budget %s", got, sdrExtractBudget) + } + + hardwareHDRBudget := extractTimeoutForAttempt(true, true) + softwareHDRBudget := extractTimeoutForAttempt(false, true) + if got := remoteExtractTimeout(true, false); got <= hardwareHDRBudget || got >= hardwareHDRBudget+softwareHDRBudget { + t.Fatalf("remoteExtractTimeout(HDR hardware only) = %s, want hardware plus overhead without CPU budget", got) + } + + hdrExtractBudget := hardwareHDRBudget + softwareHDRBudget + if got := remoteExtractTimeout(true, true); got-hdrExtractBudget <= softwareToneMapProbeTimeout { + t.Fatalf( + "remoteExtractTimeout(HDR with CPU) overhead = %s, want more than probe budget %s", + got-hdrExtractBudget, + softwareToneMapProbeTimeout, + ) + } +} + +func TestRemoteProbeFailureAllowsPreferredLocalFallback(t *testing.T) { + if !isInfrastructureRemoteFailure(reasonFFmpegProbeFailed) { + t.Fatalf("isInfrastructureRemoteFailure(%q) = false, want true", reasonFFmpegProbeFailed) + } +} + +func resolverWithFilters(t *testing.T, filters ...string) *softwareToneMapFilterResolver { + t.Helper() + lines := make([]string, 0, len(filters)) + for _, filter := range filters { + lines = append(lines, " .S. "+filter+" V->V") + } + output := strings.Join(lines, "\n") + return newSoftwareToneMapFilterResolver(func(string) ([]byte, error) { + return []byte(output), nil + }) +} + +func assertApproximateDeadline(t *testing.T, got time.Duration, want time.Duration) { + t.Helper() + if got < want-time.Second || got > want+time.Second { + t.Fatalf("deadline remaining = %s, want about %s", got, want) + } +} diff --git a/internal/chapterthumbs/remote.go b/internal/chapterthumbs/remote.go index 0e7536b5..ba6e2c4b 100644 --- a/internal/chapterthumbs/remote.go +++ b/internal/chapterthumbs/remote.go @@ -24,10 +24,15 @@ const ( authJWTSecretSetting = "auth.jwt_secret" ) +// remoteExtractOverheadBuffer covers HTTP transport and node-side scheduling +// overhead on top of the extraction attempt deadlines. +const remoteExtractOverheadBuffer = 3 * time.Second + type RemoteExtractRequest struct { - InputPath string `json:"input_path"` - SeekSeconds float64 `json:"seek_seconds"` - ToneMap bool `json:"tone_map"` + InputPath string `json:"input_path"` + SeekSeconds float64 `json:"seek_seconds"` + ToneMap bool `json:"tone_map"` + AllowSoftwareToneMap bool `json:"allow_software_tone_map"` } type RemoteExtractErrorResponse struct { @@ -58,10 +63,10 @@ func (e *httpRemoteFrameExtractor) ExtractFrame( body, err := json.Marshal(req) if err != nil { - return nil, "chapter_extract_failed", fmt.Errorf("chapter thumbnail remote extract: marshal request: %w", err) + return nil, reasonChapterExtractFailed, fmt.Errorf("chapter thumbnail remote extract: marshal request: %w", err) } - timeout := remoteExtractTimeout(req.ToneMap) + timeout := remoteExtractTimeout(req.ToneMap, req.AllowSoftwareToneMap) requestCtx, cancel := context.WithTimeout(ctx, timeout) defer cancel() @@ -103,22 +108,25 @@ func (e *httpRemoteFrameExtractor) ExtractFrame( return nil, chapterThumbnailNodeUnavailableReason, fmt.Errorf("chapter thumbnail remote extract: read response: %w", err) } if len(data) == 0 { - return nil, "chapter_extract_failed", fmt.Errorf("chapter thumbnail remote extract: empty response") + return nil, reasonChapterExtractFailed, fmt.Errorf("chapter thumbnail remote extract: empty response") } return data, "", nil } -func remoteExtractTimeout(toneMap bool) time.Duration { - timeout := extractTimeoutForAttempt(true, toneMap) - if !toneMap { - timeout += extractTimeoutForAttempt(false, false) +func remoteExtractTimeout(toneMap bool, allowSoftwareToneMap bool) time.Duration { + timeout := extractTimeoutForAttempt(true, toneMap) + remoteExtractOverheadBuffer + if !toneMap || allowSoftwareToneMap { + timeout += extractTimeoutForAttempt(false, toneMap) } - return timeout + 3*time.Second + if toneMap && allowSoftwareToneMap { + timeout += softwareToneMapProbeTimeout + } + return timeout } func isInfrastructureRemoteFailure(reason string) bool { switch reason { - case chapterThumbnailNodeUnavailableReason, chapterThumbnailNodeCapacityExhaustedReason: + case chapterThumbnailNodeUnavailableReason, chapterThumbnailNodeCapacityExhaustedReason, reasonFFmpegProbeFailed: return true default: return false diff --git a/internal/chapterthumbs/service.go b/internal/chapterthumbs/service.go index c84e34de..12f6d2f5 100644 --- a/internal/chapterthumbs/service.go +++ b/internal/chapterthumbs/service.go @@ -31,10 +31,11 @@ const ( cpuExtractTimeoutSDR = 10 * time.Second cpuExtractTimeoutHDR = 25 * time.Second - chapterThumbnailHDRPolicySetting = "playback.chapter_thumbnail_hdr_policy" - chapterThumbnailHDRPolicyDefault = "best_effort" - chapterThumbnailHDRPolicyDisabled = "disabled" - chapterThumbnailHDRPolicyBestEffort = "best_effort" + chapterThumbnailHDRPolicySetting = "playback.chapter_thumbnail_hdr_policy" + chapterThumbnailHDRPolicyDefault = "best_effort" + chapterThumbnailHDRPolicyDisabled = "disabled" + chapterThumbnailHDRPolicyBestEffort = "best_effort" + chapterThumbnailSoftwareToneMapSetting = "playback.chapter_thumbnail_software_tone_map_enabled" ) var chapterThumbnailRetrySchedule = []time.Duration{ @@ -436,7 +437,7 @@ func (s *Service) processRequest(ctx context.Context, req ChapterThumbnailReques mutated = true failed++ if shouldApplyFileFailure(reason) { - hardFileFailure = buildPersistentFileFailure(&updated, now, reason, err) + hardFileFailure = buildFileFailure(&updated, now, reason, err) slog.WarnContext(ctx, "chapter thumbnail file marked failed", "component", "chapterthumbs", "file_id", @@ -460,11 +461,11 @@ func (s *Service) processRequest(ctx context.Context, req ChapterThumbnailReques "chapter_index", chapter.Index, "reason", - "chapter_extract_failed", + reasonChapterExtractFailed, "error", err, ) - recordChapterFailure(&updated.Chapters[candidate.offset], now, "chapter_extract_failed", err) + recordChapterFailure(&updated.Chapters[candidate.offset], now, reasonChapterExtractFailed, err) mutated = true failed++ continue @@ -587,9 +588,10 @@ func (s *Service) extractFrame( return s.extractFrameFunc(ctx, file, seekSeconds, hdrPolicy) } toneMap := needsTonemap(file) && hdrPolicy == chapterThumbnailHDRPolicyBestEffort + allowSoftwareToneMap := toneMap && s.chapterThumbnailSoftwareToneMapEnabled(ctx) mode := s.chapterThumbnailExecutionMode(ctx) if mode == chapterThumbnailExecutionLocal { - return s.extractFrameLocal(ctx, file.FilePath, seekSeconds, toneMap) + return s.extractFrameLocal(ctx, file.FilePath, seekSeconds, toneMap, allowSoftwareToneMap) } node, release, nodeReason := s.reserveRemoteNode(ctx) @@ -602,7 +604,7 @@ func (s *Service) extractFrame( "reason", nodeReason, ) - return s.extractFrameLocal(ctx, file.FilePath, seekSeconds, toneMap) + return s.extractFrameLocal(ctx, file.FilePath, seekSeconds, toneMap, allowSoftwareToneMap) } return nil, nodeReason, wrapReason(nodeReason, fmt.Errorf("no transcode node available for chapter thumbnail extraction")) } @@ -610,9 +612,10 @@ func (s *Service) extractFrame( jwtSecret := s.chapterThumbnailJWTSecret(ctx) data, reason, err := s.remoteExtractor.ExtractFrame(ctx, node, jwtSecret, RemoteExtractRequest{ - InputPath: file.FilePath, - SeekSeconds: seekSeconds, - ToneMap: toneMap, + InputPath: file.FilePath, + SeekSeconds: seekSeconds, + ToneMap: toneMap, + AllowSoftwareToneMap: allowSoftwareToneMap, }) if err == nil { return data, "", nil @@ -630,22 +633,29 @@ func (s *Service) extractFrame( "error", err, ) - return s.extractFrameLocal(ctx, file.FilePath, seekSeconds, toneMap) + return s.extractFrameLocal(ctx, file.FilePath, seekSeconds, toneMap, allowSoftwareToneMap) } return nil, reason, err } -func (s *Service) extractFrameLocal(ctx context.Context, inputPath string, seekSeconds float64, toneMap bool) ([]byte, string, error) { +func (s *Service) extractFrameLocal( + ctx context.Context, + inputPath string, + seekSeconds float64, + toneMap bool, + allowSoftwareToneMap bool, +) ([]byte, string, error) { resolvedAccel, resolvedDevice := s.resolveHWConfig() return ExtractFrame(ctx, FrameExtractOptions{ - InputPath: inputPath, - SeekSeconds: seekSeconds, - FFmpegPath: s.ffmpegPath, - HWAccel: resolvedAccel, - HWDevice: resolvedDevice, - ToneMap: toneMap, - RunFunc: s.runFFmpegFrameExtractFunc, + InputPath: inputPath, + SeekSeconds: seekSeconds, + FFmpegPath: s.ffmpegPath, + HWAccel: resolvedAccel, + HWDevice: resolvedDevice, + ToneMap: toneMap, + AllowSoftwareToneMap: allowSoftwareToneMap, + RunFunc: s.runFFmpegFrameExtractFunc, }) } @@ -1043,6 +1053,14 @@ func (s *Service) chapterThumbnailHDRPolicy(ctx context.Context) string { } } +func (s *Service) chapterThumbnailSoftwareToneMapEnabled(ctx context.Context) bool { + if s == nil || s.settings == nil { + return false + } + value, err := s.settings.Get(ctx, chapterThumbnailSoftwareToneMapSetting) + return err == nil && strings.EqualFold(strings.TrimSpace(value), "true") +} + func hasEligibleMissingChapter(chapters []models.MediaChapter, now time.Time) bool { for _, chapter := range chapters { if isChapterEligible(chapter, now) { @@ -1179,21 +1197,26 @@ func retryDurationForCount(failureCount int) time.Duration { func shouldApplyFileFailure(reason string) bool { switch reason { - case "decode_invalid_data": + case reasonDecodeInvalidData, reasonFFmpegProbeFailed, reasonToneMapUnsupported: return true default: return false } } -func buildPersistentFileFailure( +func buildFileFailure( file *models.MediaFile, now time.Time, reason string, err error, ) *scanner.ChapterThumbnailFailureState { - failureCount := len(chapterThumbnailRetrySchedule) - if file != nil && file.ChapterThumbnailFailureCount >= failureCount { + failureCount := 1 + if reason == reasonDecodeInvalidData { + failureCount = len(chapterThumbnailRetrySchedule) + if file != nil && file.ChapterThumbnailFailureCount >= failureCount { + failureCount = file.ChapterThumbnailFailureCount + 1 + } + } else if file != nil { failureCount = file.ChapterThumbnailFailureCount + 1 } retryAfter := now.Add(retryDurationForCount(failureCount)) diff --git a/internal/chapterthumbs/service_test.go b/internal/chapterthumbs/service_test.go index ca244ba7..ce179905 100644 --- a/internal/chapterthumbs/service_test.go +++ b/internal/chapterthumbs/service_test.go @@ -66,14 +66,10 @@ func TestBuildFrameExtractArgs(t *testing.T) { } }) - t.Run("unknown hw accel falls back to cpu args", func(t *testing.T) { - args, err := buildFrameExtractArgs("/media/movie.mkv", 42.5, "none", "", false) - if err != nil { - t.Fatalf("buildFrameExtractArgs() error = %v", err) - } - want := buildCPUFrameExtractArgs("/media/movie.mkv", 42.5, false) - if !slices.Equal(args, want) { - t.Fatalf("args = %#v, want %#v", args, want) + t.Run("unsupported hw accel does not masquerade as hardware extraction", func(t *testing.T) { + _, err := buildFrameExtractArgs("/media/movie.mkv", 42.5, "nvenc", "/dev/dri/renderD128", false) + if err == nil || !strings.Contains(err.Error(), "does not support") { + t.Fatalf("buildFrameExtractArgs() error = %v, want unsupported accelerator error", err) } }) } @@ -275,21 +271,23 @@ func (r testSettingsReader) Get(_ context.Context, key string) (string, error) { } type testRemoteFrameExtractor struct { - data []byte - reason string - err error - nodes []string + data []byte + reason string + err error + nodes []string + requests []RemoteExtractRequest } func (e *testRemoteFrameExtractor) ExtractFrame( _ context.Context, node *nodepool.Node, _ string, - _ RemoteExtractRequest, + req RemoteExtractRequest, ) ([]byte, string, error) { if node != nil { e.nodes = append(e.nodes, node.URL) } + e.requests = append(e.requests, req) return e.data, e.reason, e.err } @@ -426,6 +424,42 @@ func TestProcessRequestSkipsFileDuringCooldown(t *testing.T) { } } +func TestProcessRequestSkipsHDRWhenPolicyDisabled(t *testing.T) { + fileRepo := &testFileRepo{ + file: &models.MediaFile{ + ID: 42, + MediaFolderID: 9, + FilePath: "/media/movie.mkv", + HDR: true, + Chapters: []models.MediaChapter{ + {Index: 0, StartSeconds: 0, EndSeconds: 10}, + }, + }, + } + service := &Service{ + fileRepo: fileRepo, + folderRepo: &testFolderRepo{folder: &models.MediaFolder{ID: 9, Enabled: true, ChapterThumbnailsEnabled: true}}, + settings: testSettingsReader{values: map[string]string{ + chapterThumbnailHDRPolicySetting: chapterThumbnailHDRPolicyDisabled, + }}, + extractFrameFunc: func(context.Context, *models.MediaFile, float64, string) ([]byte, string, error) { + t.Fatal("HDR extraction should not run when the policy is disabled") + return nil, "", nil + }, + } + + requeue, err := service.processRequest(context.Background(), ChapterThumbnailRequest{FileID: 42}, false) + if err != nil { + t.Fatalf("processRequest() error = %v", err) + } + if requeue { + t.Fatal("processRequest() requeue = true, want false") + } + if fileRepo.updateCalls != 0 { + t.Fatalf("updateCalls = %d, want 0", fileRepo.updateCalls) + } +} + func TestProcessRequestMarksDecodeInvalidDataAsFileFailure(t *testing.T) { now := time.Unix(1_700_000_000, 0).UTC() fileRepo := &testFileRepo{ @@ -485,6 +519,87 @@ func TestProcessRequestMarksDecodeInvalidDataAsFileFailure(t *testing.T) { } } +func TestProcessRequestReportsToneMapCapabilityFailuresOnceWithNormalRetry(t *testing.T) { + tests := []struct { + name string + reason string + err error + }{ + { + name: "unsupported filters", + reason: reasonToneMapUnsupported, + err: errors.New("configured FFmpeg lacks the required zscale filter"), + }, + { + name: "transient probe failure", + reason: reasonFFmpegProbeFailed, + err: errors.New("FFmpeg filter probe failed: resource temporarily unavailable"), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + now := time.Unix(1_700_000_000, 0).UTC() + fileRepo := &testFileRepo{ + file: &models.MediaFile{ + ID: 42, + MediaFolderID: 9, + FilePath: "/media/movie.mkv", + HDR: true, + Chapters: []models.MediaChapter{ + {Index: 0, StartSeconds: 0, EndSeconds: 10}, + {Index: 1, StartSeconds: 10, EndSeconds: 20}, + }, + }, + } + callCount := 0 + service := &Service{ + fileRepo: fileRepo, + folderRepo: &testFolderRepo{folder: &models.MediaFolder{ID: 9, Enabled: true, ChapterThumbnailsEnabled: true}}, + clock: func() time.Time { + return now + }, + extractFrameFunc: func(context.Context, *models.MediaFile, float64, string) ([]byte, string, error) { + callCount++ + return nil, tt.reason, tt.err + }, + } + + requeue, err := service.processRequest(context.Background(), ChapterThumbnailRequest{FileID: 42}, false) + if err != nil { + t.Fatalf("processRequest() error = %v", err) + } + if requeue { + t.Fatal("processRequest() requeue = true, want false") + } + if callCount != 1 { + t.Fatalf("callCount = %d, want 1", callCount) + } + if fileRepo.updateCalls != 1 { + t.Fatalf("updateCalls = %d, want 1", fileRepo.updateCalls) + } + if fileRepo.file.ChapterThumbnailFailureCount != 1 { + t.Fatalf("failureCount = %d, want 1", fileRepo.file.ChapterThumbnailFailureCount) + } + if fileRepo.file.ChapterThumbnailRetryAfter == nil { + t.Fatal("expected file-level retry_after to be set") + } + if got, want := *fileRepo.file.ChapterThumbnailRetryAfter, now.Add(15*time.Minute); !got.Equal(want) { + t.Fatalf("retryAfter = %v, want %v", got, want) + } + if !strings.HasPrefix(fileRepo.file.ChapterThumbnailLastError, tt.reason+":") { + t.Fatalf("lastError = %q, want %s prefix", fileRepo.file.ChapterThumbnailLastError, tt.reason) + } + if fileRepo.file.Chapters[0].ThumbnailRetryAfter == nil { + t.Fatal("expected first chapter retry_after to be set") + } + if fileRepo.file.Chapters[1].ThumbnailRetryAfter != nil { + t.Fatal("expected later chapters to be skipped after file-level failure") + } + }) + } +} + func TestExtractFrameCPUFallbackGetsFreshDeadline(t *testing.T) { service := &Service{ hwAccel: "vaapi", @@ -569,6 +684,61 @@ func TestExtractFramePrefersRemoteNodeWhenEnabled(t *testing.T) { } } +func TestExtractFramePropagatesSoftwareToneMapSettingToRemoteNode(t *testing.T) { + tests := []struct { + name string + settingValue string + wantAllowed bool + }{ + {name: "disabled by default", wantAllowed: false}, + {name: "explicitly enabled", settingValue: "true", wantAllowed: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + remote := &testRemoteFrameExtractor{data: []byte("remote-frame")} + settings := map[string]string{ + chapterThumbnailExecutionSetting: chapterThumbnailExecutionPreferTranscode, + authJWTSecretSetting: "secret", + } + if tt.settingValue != "" { + settings[chapterThumbnailSoftwareToneMapSetting] = tt.settingValue + } + service := &Service{ + settings: testSettingsReader{values: settings}, + transcodePool: &nodepool.TranscodePool{}, + remoteReservations: make(map[string]int), + remoteExtractor: remote, + } + service.transcodePool.SetNodes([]*nodepool.Node{{ + URL: "http://node-1", + Enabled: true, + Healthy: true, + }}) + + _, _, err := service.extractFrame( + context.Background(), + &models.MediaFile{FilePath: "/media/movie.mkv", HDR: true}, + 5, + chapterThumbnailHDRPolicyBestEffort, + ) + if err != nil { + t.Fatalf("extractFrame() error = %v", err) + } + if len(remote.requests) != 1 { + t.Fatalf("remote requests = %d, want 1", len(remote.requests)) + } + request := remote.requests[0] + if !request.ToneMap { + t.Fatal("remote request tone_map = false, want true for HDR") + } + if request.AllowSoftwareToneMap != tt.wantAllowed { + t.Fatalf("remote request allow_software_tone_map = %t, want %t", request.AllowSoftwareToneMap, tt.wantAllowed) + } + }) + } +} + func TestExtractFrameFallsBackLocalWhenPreferredNodeUnavailable(t *testing.T) { service := &Service{ settings: testSettingsReader{values: map[string]string{ diff --git a/internal/config/admin_settings.go b/internal/config/admin_settings.go index a3f1aae1..960b6511 100644 --- a/internal/config/admin_settings.go +++ b/internal/config/admin_settings.go @@ -14,6 +14,7 @@ import ( ) const cloudflareURLMode = "cloudflare_token" +const chapterThumbnailSoftwareToneMapKey = "playback.chapter_thumbnail_software_tone_map_enabled" // adminSettingDefaults is the effective value shown by the Admin UI when no // row exists in server_settings. Keep these values aligned with the runtime @@ -57,6 +58,7 @@ var adminSettingDefaults = map[string]string{ "playback.chapter_thumbnail_execution": "local", "playback.chapter_thumbnail_node_capacity": "1", "playback.chapter_thumbnail_hdr_policy": "best_effort", + chapterThumbnailSoftwareToneMapKey: "false", "playback.watched_threshold": "90", "playback.min_resume_threshold": "5", "allow_4k_transcode": "false", @@ -267,6 +269,7 @@ func NormalizeAdminSetting(key, raw string) (string, error) { switch key { case "metadata.cache_images", "playback.transcode_enabled", "playback.local_transcode_fallback", + chapterThumbnailSoftwareToneMapKey, "allow_4k_transcode", "enable_transcode_throttle", "audiobookshelf_compat.enabled", "jellyfin_compat.enabled", "jellyfin_compat.web_enabled", "recommendations.enabled", "subtitle_ai.enabled", "subtitle_ai.transcribe_enabled", "metadata_ai.enabled", diff --git a/internal/config/admin_settings_test.go b/internal/config/admin_settings_test.go index 6351534e..ddbeed96 100644 --- a/internal/config/admin_settings_test.go +++ b/internal/config/admin_settings_test.go @@ -179,6 +179,13 @@ func TestAdminSettingDefaultsAlignWithConfigRuntimeDefaults(t *testing.T) { } } +func TestChapterThumbnailSoftwareToneMapDefaultsDisabled(t *testing.T) { + effective := EffectiveAdminSettings(nil) + if got := effective[chapterThumbnailSoftwareToneMapKey]; got != "false" { + t.Fatalf("software tone-map default = %q, want false", got) + } +} + func normalizeEffectiveRuntimeDefaults(cfg *Config) { if cfg.S3.Public.URLAuth == "" { cfg.S3.Public.URLAuth = "presigned" @@ -209,6 +216,7 @@ func TestNormalizeAdminSettingRejectsInvalidValues(t *testing.T) { }{ {key: "database.max_connections", value: "0"}, {key: "metadata.cache_images", value: "maybe"}, + {key: chapterThumbnailSoftwareToneMapKey, value: "maybe"}, {key: "auth.access_token_expiry", value: "forever"}, {key: "recommendations.embeddings_cron", value: "not a cron"}, {key: "notifications.server_channels.batch_seconds", value: "119"}, diff --git a/internal/playback/gpudetect.go b/internal/playback/gpudetect.go index c9b2842f..5710a4e1 100644 --- a/internal/playback/gpudetect.go +++ b/internal/playback/gpudetect.go @@ -181,13 +181,13 @@ func normalizeFFmpegPath(ffmpegPath string) string { func probeFFmpegNVENC(ffmpegPath string) nvencProbeResult { if output, err := runFFmpegProbe(ffmpegPath, "-hide_banner", "-hwaccels"); err != nil { - return nvencProbeResult{reason: "hwaccels probe failed: " + probeFailure(err, output)} + return nvencProbeResult{reason: "hwaccels probe failed: " + FormatFFmpegProbeFailure(err, output)} } else if !ffmpegOutputHasToken(output, "cuda") { return nvencProbeResult{reason: "cuda hwaccel unavailable"} } if output, err := runFFmpegProbe(ffmpegPath, "-hide_banner", "-encoders"); err != nil { - return nvencProbeResult{reason: "encoders probe failed: " + probeFailure(err, output)} + return nvencProbeResult{reason: "encoders probe failed: " + FormatFFmpegProbeFailure(err, output)} } else if !ffmpegOutputHasToken(output, "h264_nvenc") { return nvencProbeResult{reason: "h264_nvenc encoder unavailable"} } else if !ffmpegOutputHasToken(output, "hevc_nvenc") { @@ -195,7 +195,7 @@ func probeFFmpegNVENC(ffmpegPath string) nvencProbeResult { } if output, err := runFFmpegProbe(ffmpegPath, "-hide_banner", "-filters"); err != nil { - return nvencProbeResult{reason: "filters probe failed: " + probeFailure(err, output)} + return nvencProbeResult{reason: "filters probe failed: " + FormatFFmpegProbeFailure(err, output)} } else if !ffmpegOutputHasToken(output, "scale_cuda") { return nvencProbeResult{reason: "scale_cuda filter unavailable"} } else if !ffmpegOutputHasToken(output, "hwupload_cuda") { @@ -213,7 +213,7 @@ func probeFFmpegNVENC(ffmpegPath string) nvencProbeResult { "-f", "null", "-", ); err != nil { - return nvencProbeResult{reason: "h264_nvenc smoke encode failed: " + probeFailure(err, output)} + return nvencProbeResult{reason: "h264_nvenc smoke encode failed: " + FormatFFmpegProbeFailure(err, output)} } return nvencProbeResult{available: true} @@ -234,7 +234,8 @@ func ffmpegOutputHasToken(output []byte, token string) bool { return false } -func probeFailure(err error, output []byte) string { +// FormatFFmpegProbeFailure combines a probe error with bounded command output. +func FormatFFmpegProbeFailure(err error, output []byte) string { message := strings.TrimSpace(err.Error()) if trimmed := strings.TrimSpace(string(output)); trimmed != "" { if len(trimmed) > 240 { diff --git a/internal/playback/gpudetect_test.go b/internal/playback/gpudetect_test.go index 27adb147..439e20f4 100644 --- a/internal/playback/gpudetect_test.go +++ b/internal/playback/gpudetect_test.go @@ -91,6 +91,12 @@ func TestExplicitNVENCBypassesFFmpegProbe(t *testing.T) { } } +func TestResolveFFmpegPathTrimsWithoutCleaningRelativeExecutable(t *testing.T) { + if got := ResolveFFmpegPath(" ./ffmpeg "); got != "./ffmpeg" { + t.Fatalf("ResolveFFmpegPath() = %q, want ./ffmpeg", got) + } +} + func TestFFmpegSupportsNVENCRequiresCUDAEncodersFiltersAndSmoke(t *testing.T) { setupHWAccelTest(t) tests := []struct { diff --git a/internal/playback/remux.go b/internal/playback/remux.go index d3159d8e..f76034da 100644 --- a/internal/playback/remux.go +++ b/internal/playback/remux.go @@ -44,7 +44,7 @@ func ffmpegBinary() string { // probes must resolve through this same function so a feature advertised at // planning time is guaranteed present in the binary that later runs. func ResolveFFmpegPath(configured string) string { - if strings.TrimSpace(configured) != "" { + if configured = strings.TrimSpace(configured); configured != "" { return configured } return ffmpegBinary() diff --git a/internal/transcodenode/server.go b/internal/transcodenode/server.go index 18a8f498..24606026 100644 --- a/internal/transcodenode/server.go +++ b/internal/transcodenode/server.go @@ -425,12 +425,13 @@ func (s *Server) handleChapterThumbnailExtract(w http.ResponseWriter, r *http.Re cfg := s.watcher.Config() frame, reason, err := chapterthumbs.ExtractFrame(r.Context(), chapterthumbs.FrameExtractOptions{ - InputPath: req.InputPath, - SeekSeconds: req.SeekSeconds, - FFmpegPath: cfg.Playback.FFmpegPath, - HWAccel: cfg.Playback.HWAccel, - HWDevice: cfg.Playback.HWDevice, - ToneMap: req.ToneMap, + InputPath: req.InputPath, + SeekSeconds: req.SeekSeconds, + FFmpegPath: cfg.Playback.FFmpegPath, + HWAccel: cfg.Playback.HWAccel, + HWDevice: cfg.Playback.HWDevice, + ToneMap: req.ToneMap, + AllowSoftwareToneMap: req.AllowSoftwareToneMap, }) if err != nil { writeChapterThumbnailError(w, http.StatusUnprocessableEntity, reason, err.Error()) diff --git a/web/src/lib/adminSettingsSearch.ts b/web/src/lib/adminSettingsSearch.ts index 99dff5db..5c24c6dc 100644 --- a/web/src/lib/adminSettingsSearch.ts +++ b/web/src/lib/adminSettingsSearch.ts @@ -269,6 +269,7 @@ export const ADMIN_SETTINGS_GROUPS: AdminSettingsSearchGroup[] = [ "Chapter Thumbnail Execution", "Chapter Thumbnail Node Capacity", "HDR Chapter Thumbnail Policy", + "Enable CPU Tone Mapping", "Watched Threshold (%)", "Min Resume Threshold (%)", ), diff --git a/web/src/pages/admin-settings/PlaybackSettings.test.tsx b/web/src/pages/admin-settings/PlaybackSettings.test.tsx new file mode 100644 index 00000000..d3da2f66 --- /dev/null +++ b/web/src/pages/admin-settings/PlaybackSettings.test.tsx @@ -0,0 +1,72 @@ +import { renderToStaticMarkup } from "react-dom/server"; +import { describe, expect, it, vi } from "vitest"; + +import PlaybackSettings from "./PlaybackSettings"; + +const useSettingsFormMock = vi.fn(); + +vi.mock("@/hooks/useSettingsForm", () => ({ + useSettingsForm: (...args: unknown[]) => useSettingsFormMock(...args), +})); + +vi.mock("@/hooks/queries/admin/system", () => ({ + useHWAccelDetection: () => ({ data: undefined, isLoading: false }), +})); + +function makeForm(values: Record) { + return { + isLoading: false, + getValue: (key: string) => values[key] ?? "", + setValue: vi.fn(), + dirtyCount: 0, + save: vi.fn(), + discard: vi.fn(), + isSaving: false, + restartRequired: false, + }; +} + +function cpuToneMapSwitch(markup: string): Element { + const container = document.createElement("div"); + container.innerHTML = markup; + const label = Array.from(container.querySelectorAll("label")).find( + (candidate) => candidate.textContent === "Enable CPU Tone Mapping", + ); + const toggle = label?.htmlFor ? container.querySelector(`[id="${label.htmlFor}"]`) : null; + if (!toggle) throw new Error("CPU tone-mapping toggle was not rendered"); + return toggle; +} + +describe("PlaybackSettings CPU tone mapping", () => { + it("includes the setting and renders it off by default", () => { + useSettingsFormMock.mockReturnValue( + makeForm({ + "playback.hw_accel": "none", + "playback.chapter_thumbnail_hdr_policy": "best_effort", + }), + ); + + const toggle = cpuToneMapSwitch(renderToStaticMarkup()); + + expect(useSettingsFormMock.mock.calls[0]?.[0]?.keys).toContain( + "playback.chapter_thumbnail_software_tone_map_enabled", + ); + expect(toggle).toHaveAttribute("aria-checked", "false"); + expect(toggle).not.toHaveAttribute("disabled"); + }); + + it("disables the toggle while HDR chapter thumbnails are disabled", () => { + useSettingsFormMock.mockReturnValue( + makeForm({ + "playback.hw_accel": "none", + "playback.chapter_thumbnail_hdr_policy": "disabled", + "playback.chapter_thumbnail_software_tone_map_enabled": "true", + }), + ); + + const toggle = cpuToneMapSwitch(renderToStaticMarkup()); + + expect(toggle).toHaveAttribute("aria-checked", "true"); + expect(toggle).toHaveAttribute("disabled"); + }); +}); diff --git a/web/src/pages/admin-settings/PlaybackSettings.tsx b/web/src/pages/admin-settings/PlaybackSettings.tsx index 0df24b52..2f0c5653 100644 --- a/web/src/pages/admin-settings/PlaybackSettings.tsx +++ b/web/src/pages/admin-settings/PlaybackSettings.tsx @@ -27,6 +27,7 @@ const KEYS = [ "playback.chapter_thumbnail_execution", "playback.chapter_thumbnail_node_capacity", "playback.chapter_thumbnail_hdr_policy", + "playback.chapter_thumbnail_software_tone_map_enabled", "playback.watched_threshold", "playback.min_resume_threshold", ]; @@ -233,6 +234,16 @@ export default function PlaybackSettings() { value={form.getValue("playback.chapter_thumbnail_hdr_policy") || "best_effort"} onChange={(v) => form.setValue("playback.chapter_thumbnail_hdr_policy", v)} /> + + form.setValue("playback.chapter_thumbnail_software_tone_map_enabled", v) + } + disabled={form.getValue("playback.chapter_thumbnail_hdr_policy") === "disabled"} + />