Files
silo-server/internal/playback/subtitle_stream_test.go
T
854d07cf8f feat(playback): add protocol v3 planning and recovery (#398)
* docs(playback): plan protocol v3 server implementation

* docs(playback): incorporate protocol v3 review

* feat(playback): implement protocol v3 server

* fix(playback): persist empty route diagnostics

* feat(playback): harden protocol v3 HDR routing

* feat(playback): complete protocol v3 client contract

* fix(playback): harden protocol v3 recovery

* fix(playback): restore dovi_rpu strip filter for DV remuxes

The v3 work renamed the Dolby Vision strip recipe to a dovi_split=mode=bl
bitstream filter that does not exist in stock FFmpeg or jellyfin-ffmpeg;
the probe failed closed on every deployment, disabling the new validated
DV7-to-HDR10 route and regressing the previously working dovi_rpu=strip=1
remux path from main. Restore dovi_rpu across the probe, remux and HLS
copy arguments, and the recipe-card constant.

Also from review: validate the remux DV mode for every profile (garbage
modes on non-P7 sources silently no-opped), reject preserve mode for P7
outright (a base-layer-only remux cannot preserve dual-layer DV), tag
dvhe sample entries only for the explicit v3 preserve recipe so legacy
web/jellycompat remuxes keep their pre-v3 hev1 labeling, and honor the
token-frozen DV mode in the proxy remux path instead of legacy-auto.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(playback): correct v3 planner policy and contract validation

Review fixes to the v3 planner and wire contracts:

- Bar Profile 7 sources from the non-strip progressive remux route: a
  base-layer-only remux can never deliver native dual-layer DV, so the
  planner no longer emits plans claiming validated Dolby Vision while
  the executed remux drops the enhancement layer.
- Accept the device-quirks feature flag from either capability location,
  matching every other dual-location feature check.
- Treat legacy hdr_unknown rows as HDR10 for HDR10-capable clients with
  a degradation warning instead of leaving them unplayable under v3.
- Honor bandwidth_cap_kbps as a hard ceiling in every quality mode and
  wire the previously dead Metered signal into conservative auto rungs.
- Degrade to the validated source-quality route instead of a terminal
  when only an implicit quality reduction demanded an unsupported
  transcode; explicit user-selected rungs keep terminal behavior.
- Bound inner capability lists and strings; compare attempt keys exactly
  instead of case-folded; make ParseTrackIDV3 strict about canonical
  numerics; accept dvdsub/pgssub/dvbsub aliases and stop promising
  burn-in for unknown subtitle codecs; probe every h264 encoder rather
  than requiring libx264; normalize the file-level bitrate fallback.
- Evaluate subtitle renderability against the engine each candidate
  route executes on, not always media3_direct.
- Pin the with-quirks attempt-key preimage arity in the cross-language
  fixture so the Kotlin client stays in lockstep.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(playback): harden v3 control-plane reliability

Review fixes to the v3 session, store, and handler layer:

- Bound concurrent replans with a slot semaphore: each replan pins a
  pooled connection for its advisory lock while issuing further store
  queries from the same pool, so an unbounded recovery storm could turn
  every connection into a lock holder and deadlock the server.
- Make CompleteReplan a real compare-and-swap (base-revision predicate,
  ErrReplanSupersededV3) and map BeginReplan insert races to a replay
  instead of a raw unique violation.
- Fingerprint start requests (request_digest column): an attempt ID
  reused with different input is now a 409-style conflict rather than a
  silent replay, and both replay paths check session liveness so dead
  sessions surface as retryable terminals.
- Pre-delete expired attempt rows on SaveAttempt so a retry during the
  cleanup window cannot wedge on an unreachable conflict.
- Align the in-memory store's semantics with Postgres and add DB-backed
  planstore tests (SILO_TEST_DATABASE_URL), including a regression test
  inserting every route-event name against the real CHECK constraint.
- Session manager: v3 route-set updates own RemuxDVMode outright so a
  replan onto an SDR source clears a stale strip mode; replacement
  reservations survive unrelated legacy stream updates; replacement
  admission excludes the replaced session explicitly instead of
  decrementing totals it may no longer be part of; the admission CAS
  loop is bounded and decider errors are logged.
- Map transient store failures to 500s instead of terminal 404/403s;
  authorize route events via identity-only projections after the rate
  limiter; keep sanitized diagnostics deterministic.
- Merge the server-computed durable plan key into replan exclusions so
  unreproducible client history cannot re-select the failed route.
- Remap tracks only when the effective edition changes (a same-file
  replan no longer switches audio to a lookalike track) and remap
  ID-only subtitle selections on edition fallback.
- Cache the v3/shadow feature flags for five seconds instead of one
  settings SELECT per playback request; stop remote transports
  best-effort when the start call times out; carry dvm/tid claims and
  the transport-scoped job identity through the legacy audio-change
  re-mint; index playback_route_events(received_at) for the retention
  delete; run store maintenance for DB-less deployments too.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(transcode): reap idle node jobs and gate WebVTT conversion

- Add an idle reaper to the transcode node: a job untouched by manifest
  or segment requests for ten minutes is closed and unregistered. After
  a v3 replan retires a transport ID, a stale in-flight stream token
  could resurrect the old job via reconstruct and encode to end-of-file
  for nobody; jobs waiting on readiness count registration as access
  and are never reaped mid-wait, and reaping keeps the recipe so a
  still-valid token reconstructs on the next hit.
- Reject bitmap subtitle tracks (PGS) on the .vtt conversion path with
  415 before headers are written instead of spawning an ffmpeg command
  that always fails mid-response, and make the extract-format override
  fall back to source-driven mapping for bitmap codecs.
- Drain error bodies on non-202 node responses so the HTTP transport
  can reuse connections.
- Pin the transcode-dir cleanup separator-boundary semantics with a
  regression test (a session ID sharing another's prefix must not
  retain foreign directories).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(playback): close v3 planner policy gaps from review

- Clamp the final transcode bitrate to bandwidth_cap_kbps: the ladder has
  no rung below 480p/1500kbps, so lower caps were silently exceeded even
  though the cap is documented as a hard delivery ceiling.
- Treat video-only media as audio-compatible instead of forcing an AAC
  conversion (or an audio_conversion_unsupported terminal) onto a file
  with no audio stream. Tracks whose codec failed to probe keep the gate.
- Only promise a bitmap subtitle sidecar for embedded PGS with an engine
  that renders embedded bitmap: external/downloaded bitmap and embedded
  DVD/DVB published artifact URLs that always failed at fetch. They now
  fall through to burn-in or its terminal.
- Accept client_video_transformations_v1 from either client_features or
  the nested context when validating client-executor transformations,
  matching the planner's dual-source reads.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(playback): probe and execute DV remuxes with one ffmpeg binary

The v3 transformation registry probed the configured playback.ffmpeg_path
while progressive remux execution resolved the process-global discovery
path, so a deployment where only one binary carries dovi_rpu could plan a
server_dv7_to_hdr10 route and then fail it at stream time. Resolution now
goes through a shared ResolveFFmpegPath (configured path first, discovery
fallback — the same rule the transcode pipeline already used), the
dovi_rpu probe is cached per binary path, and the stream handler and proxy
worker pass their configured path into ServeRemuxWithDVMode.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(playback): harden v3 replan identity and control-plane limits

- Seed failure-replan track selections from the durable current plan
  before overlaying the request: after an alternate-version fallback the
  normalized request still carries requested-edition track IDs, so a
  replan omitting unchanged tracks was rejected as a track/file mismatch.
- Remap ID-only audio selections across edition changes (parse the ID to
  an index like the subtitle remap already does) instead of leaving a
  stale file-bound ID to fail validation.
- Release the node planner reservation when a prepared remote transport
  rolls back after the node accepted the job; repeated failed starts
  could otherwise pin max-job/bandwidth budgets for the full reservation
  age.
- Size the replan semaphore below the PostgreSQL pool via a store
  capacity advisor: with max_connections at or below the fixed bound,
  advisory-lock holders could starve the inner store queries they need
  to finish.
- Contain shadow-planner panics with a recover boundary; it runs on a
  bare goroutine where an escaped panic kills the process for what is
  telemetry-only work. Document why the memory store's session lock is
  deliberately a no-op.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(transcode): serialize node job teardown against reconstructs

- Look up and touch manifest/segment sessions in one critical section so
  the idle reaper cannot unregister a job between the lookup and its
  liveness refresh.
- Re-validate each reap candidate under the per-session lifecycle lock
  before closing it: Close removes the output directory, and without the
  lock it could race a token reconstruct and wipe the segments the fresh
  ffmpeg is writing.
- Take the lifecycle lock in handleStop so a stop racing a RequireReady
  start's readiness wait blocks until registration and tears the job
  down, instead of 404ing and orphaning the ffmpeg until the reaper.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-07-14 11:51:27 -04:00

268 lines
9.0 KiB
Go

package playback
import (
"net/url"
"slices"
"strings"
"testing"
)
func TestIsPGS(t *testing.T) {
cases := []struct {
codec string
want bool
}{
{"pgs", true},
{"hdmv_pgs_subtitle", true},
{"HDMV_PGS_SUBTITLE", true},
{"dvd_subtitle", false},
{"dvb_subtitle", false},
{"subrip", false},
{"ass", false},
{"", false},
}
for _, tc := range cases {
if got := IsPGS(tc.codec); got != tc.want {
t.Errorf("IsPGS(%q) = %v, want %v", tc.codec, got, tc.want)
}
}
}
func TestStreamExtractOutput(t *testing.T) {
cases := []struct {
codec string
wantCodec string
wantFormat string
}{
{"ass", "copy", "ass"},
{"ssa", "copy", "ass"},
{"pgs", "copy", "sup"},
{"hdmv_pgs_subtitle", "copy", "sup"},
{"subrip", "webvtt", "webvtt"},
{"mov_text", "webvtt", "webvtt"},
}
for _, tc := range cases {
outCodec, outFormat := streamExtractOutput(tc.codec)
if outCodec != tc.wantCodec || outFormat != tc.wantFormat {
t.Errorf("streamExtractOutput(%q) = (%q, %q), want (%q, %q)",
tc.codec, outCodec, outFormat, tc.wantCodec, tc.wantFormat)
}
}
}
func TestStreamExtractArgs_TextCodecIsWindowed(t *testing.T) {
args := streamExtractArgs(StreamExtractOpts{
InputPath: "/media/movie.mkv",
TrackIndex: 2,
SourceCodec: "subrip",
SeekSeconds: 120,
DurationSeconds: 600,
})
joined := strings.Join(args, " ")
if !strings.Contains(joined, "-ss 120.000") {
t.Fatalf("text extract should seek the input: %s", joined)
}
if !strings.Contains(joined, "-t 600.000") {
t.Fatalf("text extract should cap the read duration: %s", joined)
}
if !strings.Contains(joined, "-copyts") {
t.Fatalf("seeked extract must preserve source timestamps: %s", joined)
}
if !strings.Contains(joined, "-c:s webvtt") || !strings.Contains(joined, "-f webvtt") {
t.Fatalf("text extract should transmux to WebVTT: %s", joined)
}
}
// ASS and PGS streams are fetched once and consumed whole by their
// client-side renderers, so by default seek/duration windowing must never
// apply even when the handler passes nonzero values.
func TestStreamExtractArgs_WholeTrackCodecsIgnoreWindow(t *testing.T) {
for _, codec := range []string{"ass", "hdmv_pgs_subtitle"} {
args := streamExtractArgs(StreamExtractOpts{
InputPath: "/media/movie.mkv",
TrackIndex: 0,
SourceCodec: codec,
SeekSeconds: 120,
DurationSeconds: 600,
})
if slices.Contains(args, "-ss") {
t.Errorf("%s extract must not seek the input: %v", codec, args)
}
if slices.Contains(args, "-t") {
t.Errorf("%s extract must not cap the read duration: %v", codec, args)
}
if !slices.Contains(args, "copy") {
t.Errorf("%s extract must copy the source stream: %v", codec, args)
}
}
}
// A client that opts in via AllowWindow gets a seeked, duration-capped PGS
// extract with -copyts preserving absolute source timestamps — the -ss must
// be an input option (before -i) so ffmpeg uses the container index.
func TestStreamExtractArgs_WindowedPGS(t *testing.T) {
args := streamExtractArgs(StreamExtractOpts{
InputPath: "/media/movie.mkv",
TrackIndex: 1,
SourceCodec: "hdmv_pgs_subtitle",
SeekSeconds: 1200,
DurationSeconds: 3600,
AllowWindow: true,
})
joined := strings.Join(args, " ")
if !strings.Contains(joined, "-ss 1200.000") {
t.Fatalf("windowed PGS extract should seek the input: %s", joined)
}
ssIdx := slices.Index(args, "-ss")
inIdx := slices.Index(args, "-i")
if ssIdx < 0 || inIdx < 0 || ssIdx > inIdx {
t.Fatalf("-ss must be an input option (before -i): %s", joined)
}
if !strings.Contains(joined, "-t 3600.000") {
t.Fatalf("windowed PGS extract should cap the read duration: %s", joined)
}
if !strings.Contains(joined, "-copyts") {
t.Fatalf("windowed PGS extract must preserve source timestamps: %s", joined)
}
if !strings.Contains(joined, "-c:s copy") || !strings.Contains(joined, "-f sup pipe:1") {
t.Fatalf("windowed PGS extract should still copy into a sup stream: %s", joined)
}
}
// A windowed extract whose input is a cached full-track .sup must force the
// sup demuxer (the elementary stream has no container header to probe from
// arbitrary offsets), remap to the file's sole stream regardless of the
// original container's track ordinal, and still seek/window with -copyts so
// the cached stream's absolute timestamps survive into the output.
func TestStreamExtractArgs_ExtractedSupInput(t *testing.T) {
args := streamExtractArgs(StreamExtractOpts{
InputPath: "/transcode/subtitle-cache/abc-s3-1-2.sup",
TrackIndex: 3,
SourceCodec: "hdmv_pgs_subtitle",
SeekSeconds: 1200,
DurationSeconds: 3600,
AllowWindow: true,
InputIsExtractedSup: true,
})
joined := strings.Join(args, " ")
if !strings.Contains(joined, "-f sup -i /transcode/subtitle-cache/abc-s3-1-2.sup") {
t.Fatalf("cached sup input must force the sup demuxer before -i: %s", joined)
}
if !strings.Contains(joined, "-map 0:s:0") {
t.Fatalf("cached sup holds exactly one stream; must map 0:s:0: %s", joined)
}
if strings.Contains(joined, "0:s:3") {
t.Fatalf("original container track ordinal must not leak into sup input mapping: %s", joined)
}
if !strings.Contains(joined, "-ss 1200.000") || !strings.Contains(joined, "-t 3600.000") {
t.Fatalf("cached sup extract must still window the input: %s", joined)
}
ssIdx := slices.Index(args, "-ss")
inIdx := slices.Index(args, "-i")
if ssIdx < 0 || inIdx < 0 || ssIdx > inIdx {
t.Fatalf("-ss must be an input option (before -i): %s", joined)
}
if !strings.Contains(joined, "-copyts") {
t.Fatalf("cached sup extract must preserve absolute timestamps: %s", joined)
}
if !strings.Contains(joined, "-c:s copy") || !strings.Contains(joined, "-f sup pipe:1") {
t.Fatalf("cached sup extract should copy into a sup stream: %s", joined)
}
}
// AllowWindow must not override the ASS guard — its [Script Info] header
// only exists at stream offset 0, so a seeked extract would be broken.
func TestStreamExtractArgs_ASSIgnoresAllowWindow(t *testing.T) {
args := streamExtractArgs(StreamExtractOpts{
InputPath: "/media/movie.mkv",
TrackIndex: 0,
SourceCodec: "ass",
SeekSeconds: 120,
DurationSeconds: 600,
AllowWindow: true,
})
if slices.Contains(args, "-ss") {
t.Errorf("ass extract must not seek the input even with AllowWindow: %v", args)
}
if slices.Contains(args, "-t") {
t.Errorf("ass extract must not cap the read duration even with AllowWindow: %v", args)
}
}
// Absent the explicit ?windowed=1 opt-in the request must not window, no
// matter what other params are present — existing clients (Apple, Android,
// jellycompat) send no param and rely on whole-track extraction.
func TestPGSWindowRequest(t *testing.T) {
cases := []struct {
name string
query string
wantAllow bool
wantSeek float64
wantDuration float64
}{
{"no params", "", false, 0, 0},
{"position without opt-in", "position=120&duration=600", false, 0, 0},
{"windowed off", "windowed=0&position=120", false, 0, 0},
{"opt-in with position and duration", "windowed=1&position=120.5&duration=3600", true, 120.5, 3600},
{"opt-in without position", "windowed=1", true, 0, 0},
{"opt-in negative position ignored", "windowed=1&position=-5&duration=600", true, 0, 600},
{"opt-in duration over cap ignored", "windowed=1&position=10&duration=7200", true, 10, 0},
{"opt-in invalid values ignored", "windowed=1&position=abc&duration=xyz", true, 0, 0},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
q, err := url.ParseQuery(tc.query)
if err != nil {
t.Fatalf("ParseQuery(%q): %v", tc.query, err)
}
allow, seek, duration := PGSWindowRequest(q)
if allow != tc.wantAllow || seek != tc.wantSeek || duration != tc.wantDuration {
t.Errorf("PGSWindowRequest(%q) = (%v, %v, %v), want (%v, %v, %v)",
tc.query, allow, seek, duration, tc.wantAllow, tc.wantSeek, tc.wantDuration)
}
})
}
}
// A forced "vtt" target applies only to text sources: bitmap codecs carry no
// text for ffmpeg's webvtt encoder, so the override must fall back to the
// source-driven mapping instead of building a command that always fails.
func TestStreamExtractOutput_TargetFormatVTTGatedToTextSources(t *testing.T) {
cases := []struct {
codec string
wantCodec string
wantFormat string
}{
{"subrip", "webvtt", "webvtt"},
{"mov_text", "webvtt", "webvtt"},
{"ass", "webvtt", "webvtt"},
{"pgs", "copy", "sup"},
{"hdmv_pgs_subtitle", "copy", "sup"},
}
for _, tc := range cases {
outCodec, outFormat := streamExtractOutput(tc.codec, "vtt")
if outCodec != tc.wantCodec || outFormat != tc.wantFormat {
t.Errorf("streamExtractOutput(%q, \"vtt\") = (%q, %q), want (%q, %q)",
tc.codec, outCodec, outFormat, tc.wantCodec, tc.wantFormat)
}
}
}
func TestStreamExtractArgs_PGSProducesSup(t *testing.T) {
args := streamExtractArgs(StreamExtractOpts{
InputPath: "/media/movie.mkv",
TrackIndex: 1,
SourceCodec: "pgs",
})
joined := strings.Join(args, " ")
if !strings.Contains(joined, "-map 0:s:1 -c:s copy -f sup pipe:1") {
t.Fatalf("PGS extract should copy into a sup stream: %s", joined)
}
}