fix(scanner): stop re-probing unchanged files ffprobe rejected

internal/scanner/extras.go

@@ -6,6 +6,7 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+	"time"
 
 	"github.com/Silo-Server/silo-server/internal/contentid"
 	"github.com/Silo-Server/silo-server/internal/librarykind"
@@ -371,10 +372,7 @@ func (s *Scanner) processExtraFiles(
 
 		fileModifiedAt := normalizeFileModifiedAt(info.ModTime())
 		existing := existingByPath[candidate.Path]
-		if existing != nil && existing.ExtraID == extraID &&
-			existing.FileSize == info.Size() &&
-			existing.FileModifiedAt != nil && existing.FileModifiedAt.Equal(fileModifiedAt) &&
-			existing.ProbeUpdatedAt != nil && existing.MissingSince == nil {
+		if extraFileUnchanged(existing, extraID, info.Size(), fileModifiedAt) {
 			stats.Unchanged++
 			continue
 		}
@@ -481,3 +479,14 @@ func extraParentLookupDir(candidate extraCandidate) string {
 	}
 	return firstNonSupplementalAncestor(candidate.SupplementalDir)
 }
+
+// extraFileUnchanged reports whether an extra's row already describes the file
+// on disk, so a scan can leave it alone. A row ffprobe rejected for these same
+// bytes counts as unchanged too: probing them again would only fail again.
+func extraFileUnchanged(existing *scanStateFile, extraID string, fileSize int64, fileModifiedAt time.Time) bool {
+	return existing != nil && existing.ExtraID == extraID &&
+		existing.FileSize == fileSize &&
+		existing.FileModifiedAt != nil && existing.FileModifiedAt.Equal(fileModifiedAt) &&
+		(existing.ProbeUpdatedAt != nil || probeRejectionStands(existing, fileSize, fileModifiedAt)) &&
+		existing.MissingSince == nil
+}

internal/scanner/probe.go

@@ -121,13 +121,19 @@ func ProbeFile(ctx context.Context, ffprobePath string, filePath string) (*Probe
 // the failure without depending on ffprobe's wording. Either one finding an
 // access problem logs it and answers false. A canceled or timed-out probe, a
 // process killed by a signal, a missing binary, or unparseable output is not a
-// rejection either.
+// rejection either, and neither is an exit status of 126 or 127, which
+// means ffprobe never got as far as opening the file.
 func IsProbeRejection(ctx context.Context, filePath string, err error) bool {
 	if err == nil || ctx.Err() != nil {
 		return false
 	}
 	var exitErr *exec.ExitError
-	if !errors.As(err, &exitErr) || exitErr.ExitCode() <= 0 {
+	if !errors.As(err, &exitErr) {
+		return false
+	}
+	// ffprobe refuses input with exit status 1; 126 and 127 come from the
+	// shell or the dynamic loader when ffprobe itself cannot start.
+	if code := exitErr.ExitCode(); code <= 0 || code == 126 || code == 127 {
 		return false
 	}
 	accessErr := probeStderrAccessFailure(exitErr.Stderr)

internal/scanner/probe_rejection_test.go

@@ -57,6 +57,17 @@ func TestIsProbeRejectionSeparatesRefusedFilesFromProbeTrouble(t *testing.T) {
 	if IsProbeRejection(ctx, media, nil) {
 		t.Fatal("nil error reported as a rejection")
 	}
+
+	// The shell and the dynamic loader exit 126 or 127 when ffprobe itself
+	// cannot start, such as a shared library it cannot map.
+	for _, code := range []int{126, 127} {
+		broken := filepath.Join(t.TempDir(), "ffprobe")
+		writeFakeTool(t, broken, fmt.Sprintf("#!/bin/sh\necho 'ffprobe: error while loading shared libraries' >&2\nexit %d\n", code))
+		_, err = ProbeFile(ctx, broken, media)
+		if err == nil || IsProbeRejection(ctx, media, err) {
+			t.Fatalf("ffprobe that could not start (exit %d) = %v, want a non-rejection error", code, err)
+		}
+	}
 }
 
 // The files from issue #1791: real ffprobe refuses zero-byte, random-byte and

internal/scanner/scan_state.go

@@ -50,6 +50,7 @@ type scanStateFile struct {
 	MultiEpisodeEnd        int
 	ProbeSource            string
 	ProbeUpdatedAt         *time.Time
+	ProbeFailedAt          *time.Time
 	MissingSince           *time.Time
 	HasVideoTracks         bool
 	HasNonImageVideoTracks bool
@@ -67,7 +68,7 @@ const scanStateColumns = `id, content_id, episode_id, extra_id,
 	edition_raw, edition_key, edition_confidence, edition_source,
 	presentation_kind, presentation_group_key, presentation_part_index,
 	multi_episode_start, multi_episode_end,
-	probe_source, probe_updated_at, missing_since,
+	probe_source, probe_updated_at, probe_failed_at, missing_since,
 	COALESCE(jsonb_typeof(video_tracks) = 'array' AND jsonb_array_length(video_tracks) > 0, FALSE) AS has_video_tracks,
 	COALESCE((
 		SELECT bool_or(lower(btrim(COALESCE(track->>'codec', ''))) NOT IN ('mjpeg', 'jpeg', 'png', 'webp', 'gif', 'bmp'))
@@ -143,6 +144,7 @@ func scanScanStateRow(row pgx.Row) (*scanStateFile, error) {
 		&multiEpisodeEnd,
 		&probeSource,
 		&state.ProbeUpdatedAt,
+		&state.ProbeFailedAt,
 		&state.MissingSince,
 		&state.HasVideoTracks,
 		&state.HasNonImageVideoTracks,
@@ -362,6 +364,7 @@ func scanStateFromMediaFile(file *models.MediaFile) *scanStateFile {
 		MultiEpisodeEnd:        file.MultiEpisodeEnd,
 		ProbeSource:            file.ProbeSource,
 		ProbeUpdatedAt:         file.ProbeUpdatedAt,
+		ProbeFailedAt:          file.ProbeFailedAt,
 		MissingSince:           file.MissingSince,
 		HasVideoTracks:         len(file.VideoTracks) > 0,
 		HasNonImageVideoTracks: probeFacts.HasNonImageVideoTracks,

internal/scanner/scan_state_probe_rejection_db_test.go

@@ -0,0 +1,130 @@
+package scanner
+
+import (
+	"context"
+	"fmt"
+	"os"
+	"path/filepath"
+	"testing"
+	"time"
+
+	"github.com/jackc/pgx/v5/pgxpool"
+
+	"github.com/Silo-Server/silo-server/internal/models"
+)
+
+// TestScanStateCarriesProbeRejectionDB checks the scan state a library scan
+// reads: a file ffprobe rejected keeps its mark there, so a rescan of the
+// unchanged file skips the probe repair.
+func TestScanStateCarriesProbeRejectionDB(t *testing.T) {
+	dsn := os.Getenv("SILO_TEST_DATABASE_URL")
+	if dsn == "" {
+		t.Skip("SILO_TEST_DATABASE_URL is not set")
+	}
+	ctx := context.Background()
+	pool, err := pgxpool.New(ctx, dsn)
+	if err != nil {
+		t.Fatalf("connect test database: %v", err)
+	}
+	t.Cleanup(pool.Close)
+
+	suffix := time.Now().UnixNano()
+	var folderID int
+	if err := pool.QueryRow(ctx, `INSERT INTO media_folders (type, name, enabled) VALUES ('movies', $1, true) RETURNING id`,
+		fmt.Sprintf("probe-rejection-%d", suffix)).Scan(&folderID); err != nil {
+		t.Fatalf("seed folder: %v", err)
+	}
+	t.Cleanup(func() {
+		_, _ = pool.Exec(ctx, `DELETE FROM media_files WHERE media_folder_id = $1`, folderID)
+		_, _ = pool.Exec(ctx, `DELETE FROM media_folders WHERE id = $1`, folderID)
+	})
+
+	modifiedAt := time.Now().UTC().Truncate(time.Microsecond)
+	rejectedAt := modifiedAt.Add(time.Minute)
+	repo := NewFileRepository(pool)
+	if _, err := repo.Upsert(ctx, models.MediaFile{
+		MediaFolderID:  folderID,
+		FilePath:       fmt.Sprintf("/tmp/probe-rejection-%d.mkv", suffix),
+		FileSize:       1_000,
+		FileModifiedAt: &modifiedAt,
+		ProbeFailedAt:  &rejectedAt,
+	}); err != nil {
+		t.Fatalf("seed rejected file: %v", err)
+	}
+
+	states, err := repo.GetScanStateByFolder(ctx, folderID)
+	if err != nil {
+		t.Fatalf("GetScanStateByFolder: %v", err)
+	}
+	if len(states) != 1 {
+		t.Fatalf("scan state rows = %d, want 1", len(states))
+	}
+	state := states[0]
+	if state.ProbeFailedAt == nil || state.ProbeUpdatedAt != nil {
+		t.Fatalf("scan state probe_failed_at = %v, probe_updated_at = %v, want a rejection and no probe", state.ProbeFailedAt, state.ProbeUpdatedAt)
+	}
+	reasons := scanStateUpdateReasons(state, 1_000, modifiedAt, nil, false, fileRootAssignment{}, fileGroupAssignment{}, "movies", true)
+	if testStringSliceContains(reasons, "probe_repair") {
+		t.Fatalf("rescan reasons = %#v, want no probe_repair for the unchanged rejected file", reasons)
+	}
+}
+
+// TestRescanKeepsProbeFactsOfARejectedUnchangedFileDB covers a row probed
+// before rejections were tracked: it keeps old probe facts without
+// probe_updated_at, and ffprobe now rejects it. A rescan for a new subtitle
+// sidecar probes it again and fails; that must record the subtitle without
+// overwriting the probe facts the row still has.
+func TestRescanKeepsProbeFactsOfARejectedUnchangedFileDB(t *testing.T) {
+	pool := newDeadRootTestPool(t)
+	ctx := t.Context()
+	root := t.TempDir()
+	folderID := seedDeadRootTestFolder(t, pool, "movies", fmt.Sprintf("rejected-legacy-%d", time.Now().UnixNano()))
+	folder := &models.MediaFolder{ID: folderID, Type: "movies", Paths: []string{root}}
+	moviePath := filepath.Join(root, "Legacy Movie (2020).mkv")
+	if err := os.WriteFile(moviePath, []byte("not a video"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	info, err := os.Stat(moviePath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	modifiedAt := info.ModTime().UTC()
+	rejectedAt := time.Now().UTC()
+	repo := NewFileRepository(pool)
+	if _, err := repo.Upsert(ctx, models.MediaFile{
+		MediaFolderID:     folderID,
+		FilePath:          moviePath,
+		FileSize:          info.Size(),
+		FileModifiedAt:    &modifiedAt,
+		CodecVideo:        "h264",
+		Container:         "matroska",
+		Duration:          5400,
+		ProbeFailedAt:     &rejectedAt,
+		SubtitleTracks:    []models.SubtitleTrack{},
+		ExternalSubtitles: []models.ExternalSubtitle{},
+	}); err != nil {
+		t.Fatalf("seed rejected legacy file: %v", err)
+	}
+	if err := os.WriteFile(filepath.Join(root, "Legacy Movie (2020).en.srt"), []byte("1\n00:00:01,000 --> 00:00:02,000\nHello\n"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+
+	s := NewScanner(repo, writeRejectingFFprobe(t), nil, 1, false, 0)
+	if _, err := s.ScanFolder(ctx, folder); err != nil {
+		t.Fatalf("rescan: %v", err)
+	}
+
+	stored, err := repo.GetByPath(ctx, moviePath)
+	if err != nil {
+		t.Fatalf("GetByPath: %v", err)
+	}
+	if stored.CodecVideo != "h264" || stored.Container != "matroska" || stored.Duration != 5400 {
+		t.Fatalf("probe facts after rescan = %q %q %d, want h264 matroska 5400", stored.CodecVideo, stored.Container, stored.Duration)
+	}
+	if len(stored.ExternalSubtitles) != 1 {
+		t.Fatalf("external subtitles after rescan = %+v, want the new sidecar", stored.ExternalSubtitles)
+	}
+	if stored.ProbeFailedAt == nil || stored.ProbeUpdatedAt != nil {
+		t.Fatalf("probe state after rescan: failed=%v updated=%v, want the rejection kept", stored.ProbeFailedAt, stored.ProbeUpdatedAt)
+	}
+}

internal/scanner/scanner.go

@@ -3033,7 +3033,8 @@ func (s *Scanner) processFile(
 
 		// Try to get probe data.
 		probe, probeSource, probeRejected := s.probeFile(ctx, filePath)
-		if shouldPreserveExistingProbeAfterProbeFailure(updateReasons, probe) {
+		rejectionStands := probeRejectionStands(existing, fileSize, fileModifiedAt)
+		if shouldPreserveExistingProbeAfterProbeFailure(updateReasons, probe, rejectionStands) {
 			if probeRejected {
 				// Nothing else about the row changes here, so the rejection
 				// is recorded on its own. The repository only marks rows with
@@ -3043,8 +3044,10 @@ func (s *Scanner) processFile(
 				}
 			}
 			// Leave the migrated row's probe_updated_at NULL so a later scan
-			// retries without replacing valid metadata with zero values.
-			if len(updateReasons) > 1 {
+			// retries without replacing valid metadata with zero values. A
+			// standing rejection has no probe_repair reason, so every reason
+			// it has is an identity or subtitle change to write.
+			if rejectionStands || len(updateReasons) > 1 {
 				mf := models.MediaFile{MediaFolderID: folder.ID, FilePath: filePath}
 				populateScanIdentity(&mf, filePath, folder.Type, assignment, groupAssignment, existing)
 				mf.ExternalSubtitles = externalSubtitleModels(loadExternalSubs())
@@ -3332,7 +3335,7 @@ func scanStateUpdateReasons(
 	if existing.MissingSince != nil {
 		reasons = append(reasons, "was_missing")
 	}
-	if canRepairProbe && needsCriticalProbeRepairScanState(existing) {
+	if scanStateNeedsProbeRepair(existing, fileSize, fileModifiedAt, canRepairProbe) {
 		reasons = append(reasons, "probe_repair")
 	}
 	if externalSubtitlesChecked {
@@ -3425,7 +3428,7 @@ func shouldSkipStableConfirmedScanState(
 	if existing.MissingSince != nil {
 		return false
 	}
-	if canRepairProbe && needsCriticalProbeRepairScanState(existing) {
+	if scanStateNeedsProbeRepair(existing, fileSize, fileModifiedAt, canRepairProbe) {
 		return false
 	}
 	if len(updateReasons) > 0 {
@@ -3512,6 +3515,28 @@ func normalizeFileModifiedAt(ts time.Time) time.Time {
 	return models.NormalizeFileModifiedAt(ts)
 }
 
+// scanStateNeedsProbeRepair reports whether a scan should probe an unchanged
+// file again to fill in playback-critical metadata. ffprobe has already
+// rejected a file marked probe_failed_at that never probed successfully, and
+// the mark only stands while its size and modification time are the ones that
+// were probed, so probing the same bytes again would only fail again. Changed
+// bytes drop the mark and are probed as usual. Playback still probes a marked
+// file when it is played.
+func scanStateNeedsProbeRepair(existing *scanStateFile, fileSize int64, fileModifiedAt time.Time, canRepairProbe bool) bool {
+	if !canRepairProbe || probeRejectionStands(existing, fileSize, fileModifiedAt) {
+		return false
+	}
+	return needsCriticalProbeRepairScanState(existing)
+}
+
+// probeRejectionStands reports that ffprobe rejected the bytes the row still
+// describes: it is marked probe_failed_at, never probed successfully, and its
+// size and modification time match the file on disk.
+func probeRejectionStands(existing *scanStateFile, fileSize int64, fileModifiedAt time.Time) bool {
+	return existing != nil && existing.ProbeFailedAt != nil && existing.ProbeUpdatedAt == nil &&
+		existing.FileSize == fileSize && sameFileModifiedAt(existing.FileModifiedAt, fileModifiedAt)
+}
+
 func needsCriticalProbeRepairScanState(file *scanStateFile) bool {
 	if file == nil {
 		return true
@@ -3562,11 +3587,15 @@ func needsCriticalProbeRepairScanState(file *scanStateFile) bool {
 	return false
 }
 
-func shouldPreserveExistingProbeAfterProbeFailure(updateReasons []string, probe *ProbeData) bool {
+// shouldPreserveExistingProbeAfterProbeFailure reports whether a failed probe
+// must leave the row's stored probe metadata alone: the scan was there to
+// repair it, alongside at most identity and subtitle changes. A standing
+// rejection skips the repair reason but is the same case.
+func shouldPreserveExistingProbeAfterProbeFailure(updateReasons []string, probe *ProbeData, rejectionStands bool) bool {
 	if probe != nil || len(updateReasons) == 0 {
 		return false
 	}
-	foundRepair := false
+	foundRepair := rejectionStands
 	for _, reason := range updateReasons {
 		switch reason {
 		case "probe_repair":