fix(playback): fail closed on inconclusive PPS scans

This commit is contained in:
Quick
2026-07-29 09:49:34 -04:00
parent 9281ae61d9
commit a7601028d9
7 changed files with 97 additions and 8 deletions
+4
View File
@@ -236,6 +236,10 @@ type VideoTrack struct {
// bitstream scan and held in memory, never serialized to the database
// (`json:"-"`). nil means "not analyzed in this process yet".
MultiplePPS *bool `json:"-"`
// VideoCopyUnsafe is set when conflicting PPS data is detected or when the
// safety scan cannot establish that video stream-copy is safe. It is
// runtime-only so transient scan failures are retried on a later request.
VideoCopyUnsafe bool `json:"-"`
}
// AudioTrack represents a probed audio stream stored as JSONB.
+4 -2
View File
@@ -270,10 +270,12 @@ func containsStr(slice []string, s string) bool {
// videoCopyUnsafeFile reports whether the file's video stream cannot be safely
// stream-copied into an avc1/fMP4 segment. It is set once by the multi-PPS
// bitstream scan (H.264 sources that redefine a pic_parameter_set_id in-band
// with conflicting content). nil/false means copy is safe or not yet analyzed.
// with conflicting content). Scan failures also disable copy for the current
// decision while remaining eligible for retry on a later request.
func videoCopyUnsafeFile(file *models.MediaFile) bool {
if file == nil || len(file.VideoTracks) == 0 {
return false
}
return file.VideoTracks[0].MultiplePPS != nil && *file.VideoTracks[0].MultiplePPS
track := file.VideoTracks[0]
return track.VideoCopyUnsafe || (track.MultiplePPS != nil && *track.MultiplePPS)
}
+17
View File
@@ -82,6 +82,23 @@ func TestResolver_CopyUnsafeForcesTranscode(t *testing.T) {
}
}
func TestResolver_UnknownCopySafetyForcesTranscode(t *testing.T) {
// An inconclusive safety scan must not fail open to video stream-copy.
file := &models.MediaFile{
CodecVideo: "h264", CodecAudio: "dts", Container: "mkv",
Resolution: "1080p", HDR: false,
VideoTracks: []models.VideoTrack{{
Codec: "h264",
VideoCopyUnsafe: true,
}},
}
decision := playback.Resolve(file, defaultCaps(), defaultSettings())
if decision.Method != playback.PlayTranscode {
t.Errorf("method = %q, want transcode", decision.Method)
}
}
func TestResolver_CopySafeStillRemuxes(t *testing.T) {
safe := false
// The same shape with the copy-safety scan resolved to safe keeps remuxing.
+3 -1
View File
@@ -68,7 +68,9 @@ func annexBHasConflictingPPS(data []byte) bool {
if byID[id] == nil {
byID[id] = make(map[string]struct{})
}
byID[id][string(nal)] = struct{}{}
// nal_ref_idc belongs to the NAL header, not the PPS definition. Two
// otherwise-identical PPS payloads may carry different valid priorities.
byID[id][string(nal[1:])] = struct{}{}
if len(byID[id]) > 1 {
return true
}
+9
View File
@@ -29,6 +29,15 @@ func TestAnnexBHasConflictingPPS(t *testing.T) {
data: annexB(pps(0xee, 0x35, 0x25), pps(0xee, 0x35, 0x25), pps(0xee, 0x35, 0x25)),
want: false,
},
{
name: "same pps payload with different nal priority is safe",
// nal_ref_idc is header metadata, not part of the PPS definition.
data: annexB(
[]byte{0x28, 0xee, 0x35, 0x25},
[]byte{0x68, 0xee, 0x35, 0x25},
),
want: false,
},
{
name: "same id redefined with different content is unsafe",
// both decode to pic_parameter_set_id=0 (leading bit set) but differ.
+17 -5
View File
@@ -2,6 +2,7 @@ package scanner
import (
"context"
"log/slog"
"strings"
"sync"
"time"
@@ -159,9 +160,15 @@ func (e *PlaybackProbeEnsurer) ensureCopySafety(ctx context.Context, file *model
multi, err := DetectMultiplePPSH264(scanCtx, e.ffmpegPath, file.FilePath)
cancel()
if err != nil {
// Leave the flag unset so a transient failure retries on the next play
// rather than caching a wrong answer; the copy path stays available.
return file, err
// Unknown safety must not fail open to the video-copy path this probe is
// intended to guard. Leave MultiplePPS unset and do not cache the result,
// so a later request retries the scan without misreporting the cause.
slog.WarnContext(ctx, "video copy-safety scan failed; disabling stream copy",
"component", "scanner",
"file_id", file.ID,
"error", err,
)
return fileWithCopySafety(file, nil, true), nil
}
e.copySafety.Store(file.ID, copySafetyResult{size: file.FileSize, multi: multi})
@@ -172,11 +179,16 @@ func (e *PlaybackProbeEnsurer) ensureCopySafety(ctx context.Context, file *model
// MultiplePPS flag set on its first video track, without mutating the caller's
// file or its shared VideoTracks slice.
func fileWithMultiplePPS(file *models.MediaFile, multi bool) *models.MediaFile {
value := multi
return fileWithCopySafety(file, &value, multi)
}
func fileWithCopySafety(file *models.MediaFile, multiplePPS *bool, copyUnsafe bool) *models.MediaFile {
updated := *file
tracks := make([]models.VideoTrack, len(file.VideoTracks))
copy(tracks, file.VideoTracks)
value := multi
tracks[0].MultiplePPS = &value
tracks[0].MultiplePPS = multiplePPS
tracks[0].VideoCopyUnsafe = copyUnsafe
updated.VideoTracks = tracks
return &updated
}
@@ -0,0 +1,43 @@
package scanner
import (
"context"
"path/filepath"
"testing"
"github.com/Silo-Server/silo-server/internal/models"
)
func TestEnsureCopySafetyFailureDisablesCopyWithoutCaching(t *testing.T) {
ensurer := &PlaybackProbeEnsurer{
ffmpegPath: filepath.Join(t.TempDir(), "missing-ffmpeg"),
}
file := &models.MediaFile{
ID: 42,
FilePath: "/library/movie.mkv",
FileSize: 1234,
CodecVideo: "h264",
VideoTracks: []models.VideoTrack{{Codec: "h264"}},
}
got, err := ensurer.ensureCopySafety(context.Background(), file)
if err != nil {
t.Fatalf("ensureCopySafety() error = %v", err)
}
if got == file {
t.Fatal("ensureCopySafety() returned the caller's file instead of an annotated copy")
}
track := got.VideoTracks[0]
if !track.VideoCopyUnsafe {
t.Fatal("VideoCopyUnsafe = false, want true after an inconclusive scan")
}
if track.MultiplePPS != nil {
t.Fatalf("MultiplePPS = %v, want nil when the scan did not produce a result", *track.MultiplePPS)
}
if _, ok := ensurer.copySafety.Load(file.ID); ok {
t.Fatal("inconclusive scan result was cached")
}
if file.VideoTracks[0].VideoCopyUnsafe || file.VideoTracks[0].MultiplePPS != nil {
t.Fatal("ensureCopySafety() mutated the caller's file")
}
}