From 58511369cb7e9145524dd07107be1b93555e6080 Mon Sep 17 00:00:00 2001 From: Quick <31828688+Quick104@users.noreply.github.com> Date: Sun, 26 Jul 2026 01:54:40 +0000 Subject: [PATCH 1/3] fix(scanner): reject durations that imply an impossible bitrate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The duration-plausibility rule only rejected videos of 10 seconds or less, so a feature film that probed as 61 seconds passed untouched and persisted. Clients then had nothing trustworthy to anchor on: Android's grow-only duration ratchet has no floor to hold when the catalog value is wrong, so the playback engine's growing-HLS-window duration won and a 90-minute movie displayed as ~1 minute. Size and duration together pin an implied bitrate, which separates the two cases the absolute floor conflates. A genuine short clip has an ordinary bitrate; a 100 GB file claiming 61 seconds implies ~13 Gbps. The ceiling sits far above any real medium, so legitimate content cannot trip it — and unlike the absolute floor, it does not false-positive on a genuine high-bitrate short. Also bump the repair-rule revision marker so rows judged by the previous, weaker rule are re-checked once under this one. Without that bump an improved rule never reaches the rows it was written for. Co-Authored-By: Claude Opus 5 (1M context) --- internal/scanner/probe.go | 56 +++++++++++++----- internal/scanner/probe_duration_test.go | 77 +++++++++++++++++++++++++ internal/scanner/probe_repair.go | 23 +++++--- 3 files changed, 132 insertions(+), 24 deletions(-) diff --git a/internal/scanner/probe.go b/internal/scanner/probe.go index 36376a267..5a3db0a8c 100644 --- a/internal/scanner/probe.go +++ b/internal/scanner/probe.go @@ -272,20 +272,46 @@ const ( maxReasonableAudioDurationSeconds = 1_000_000 ) -// A large video file whose derived duration is only a few seconds is the -// signature of malformed container timestamps (and of the legacy probe that -// divided large durations by one million). The shape is shared with the -// repair triggers in probe_repair.go and scanner.go so the probe parser and -// the repair layers cannot drift apart. +// A video duration is implausible when it is either far too short in absolute +// terms, or when it implies a bitrate no real medium reaches. Both are +// signatures of malformed container timestamps (and of the legacy probe that +// divided large durations by one million). +// +// The absolute rule alone cannot catch a feature film that probed as, say, 61 +// seconds — well past the floor, yet still wrong by two orders of magnitude. +// Size and duration together pin an implied bitrate, which separates the two +// cases the absolute rule conflates: a genuine short clip has an ordinary +// bitrate, while a 100 GB file claiming 61 seconds implies ~13 Gbps. +// +// The ceiling sits far above any real medium — UHD Blu-ray peaks near +// 150 Mbps and ProRes 4444 XQ at 4K near 500 Mbps — so legitimate content +// cannot trip it. This also makes the rule safer than the absolute floor +// alone, which false-positives on a genuine high-bitrate short. +// +// The shape is shared with the repair triggers in probe_repair.go and +// scanner.go so the probe parser and the repair layers cannot drift apart. const ( implausiblyShortVideoMaxSeconds = 10 implausiblyShortVideoMinBytes = 100 * 1024 * 1024 + implausibleVideoBitrateBps = 1_000_000_000 ) -func videoDurationImplausiblyShort(durationSeconds float64, sizeBytes int64, hasVideo bool) bool { - return hasVideo && - durationSeconds > 0 && durationSeconds <= implausiblyShortVideoMaxSeconds && - sizeBytes >= implausiblyShortVideoMinBytes +func videoDurationImplausible(durationSeconds float64, sizeBytes int64, hasVideo bool) bool { + if !hasVideo || durationSeconds <= 0 || sizeBytes <= 0 { + return false + } + if durationSeconds <= implausiblyShortVideoMaxSeconds && sizeBytes >= implausiblyShortVideoMinBytes { + return true + } + return impliedBitrateBps(sizeBytes, durationSeconds) > implausibleVideoBitrateBps +} + +// impliedBitrateBps is the bitrate a file's size and duration imply. Callers +// use it as a duration-sanity signal, not as a real bitrate estimate: it +// counts container overhead and every stream, which is precisely what makes it +// a conservative upper bound. +func impliedBitrateBps(sizeBytes int64, durationSeconds float64) float64 { + return float64(sizeBytes) * 8 / durationSeconds } func durationFromProbeMetadata(raw *ffprobeOutput) (int, bool) { @@ -298,7 +324,7 @@ func durationFromProbeMetadata(raw *ffprobeOutput) (int, bool) { durationIsPositiveFinite(formatDuration) && formatDuration <= maxReasonableAudioDurationSeconds { return truncatedDuration(formatDuration), true } - if durationIsReasonable(formatDuration) && !durationLooksImplausiblyShort(raw, formatDuration) { + if durationIsReasonable(formatDuration) && !durationLooksImplausible(raw, formatDuration) { return truncatedDuration(formatDuration), true } @@ -307,28 +333,28 @@ func durationFromProbeMetadata(raw *ffprobeOutput) (int, bool) { continue } streamDuration := parseFloat(stream.Duration) - if durationIsReasonable(streamDuration) && !durationLooksImplausiblyShort(raw, streamDuration) { + if durationIsReasonable(streamDuration) && !durationLooksImplausible(raw, streamDuration) { return truncatedDuration(streamDuration), true } duration := durationAfterStart(streamDuration, parseFloat(stream.StartTime)) - if duration > 0 && !durationLooksImplausiblyShort(raw, duration) { + if duration > 0 && !durationLooksImplausible(raw, duration) { return truncatedDuration(duration), true } } duration := durationAfterStart(formatDuration, parseFloat(raw.Format.StartTime)) - if duration > 0 && !durationLooksImplausiblyShort(raw, duration) { + if duration > 0 && !durationLooksImplausible(raw, duration) { return truncatedDuration(duration), true } return 0, false } -func durationLooksImplausiblyShort(raw *ffprobeOutput, duration float64) bool { +func durationLooksImplausible(raw *ffprobeOutput, duration float64) bool { if raw == nil { return false } size := int64(parseFloat(raw.Format.Size)) - return videoDurationImplausiblyShort(duration, size, hasVideoStream(raw.Streams)) + return videoDurationImplausible(duration, size, hasVideoStream(raw.Streams)) } func durationAfterStart(end, start float64) float64 { diff --git a/internal/scanner/probe_duration_test.go b/internal/scanner/probe_duration_test.go index 371d62bd4..92706e3a0 100644 --- a/internal/scanner/probe_duration_test.go +++ b/internal/scanner/probe_duration_test.go @@ -237,3 +237,80 @@ func TestDurationFromProbeMetadataRejectsCollapsedTimestampSpanForLargeVideo(t * t.Fatalf("durationFromProbeMetadata() = %d, %v; want 0, false", got, ok) } } + +// A feature film that probes far short of its real runtime clears the absolute +// floor but implies an impossible bitrate. This is the case that reached +// clients as a 90-minute movie displayed as ~1 minute. +func TestDurationFromProbeMetadataRejectsImpossibleImpliedBitrate(t *testing.T) { + t.Parallel() + + raw := &ffprobeOutput{ + Format: ffprobeFormat{ + Duration: "61.000000", + Size: "107374182400", // 100 GiB => ~14 Gbps at 61s + }, + Streams: []ffprobeStream{{ + CodecType: "video", + AvgFrameRate: "24/1", + }}, + } + + got, ok := durationFromProbeMetadata(raw) + if ok || got != 0 { + t.Fatalf("durationFromProbeMetadata() = %d, %v; want 0, false", got, ok) + } +} + +// The implied-bitrate rule must not reject genuinely short clips. A 30-second +// 4K clip at 100 MiB implies ~28 Mbps, which is ordinary. +func TestDurationFromProbeMetadataKeepsGenuineShortHighBitrateClip(t *testing.T) { + t.Parallel() + + raw := &ffprobeOutput{ + Format: ffprobeFormat{ + Duration: "30.000000", + Size: "104857600", + }, + Streams: []ffprobeStream{{ + CodecType: "video", + AvgFrameRate: "60/1", + }}, + } + + got, ok := durationFromProbeMetadata(raw) + if !ok || got != 30 { + t.Fatalf("durationFromProbeMetadata() = %d, %v; want 30, true", got, ok) + } +} + +func TestVideoDurationImplausible(t *testing.T) { + t.Parallel() + + const gib = int64(1024 * 1024 * 1024) + tests := []struct { + name string + duration float64 + size int64 + hasVideo bool + want bool + }{ + {name: "feature film probed as one minute", duration: 61, size: 100 * gib, want: true, hasVideo: true}, + {name: "legacy microsecond collapse", duration: 3, size: 2 * gib, want: true, hasVideo: true}, + {name: "genuine short clip", duration: 30, size: 100 * 1024 * 1024, want: false, hasVideo: true}, + {name: "ordinary feature film", duration: 5400, size: 8 * gib, want: false, hasVideo: true}, + {name: "uhd remux at full runtime", duration: 7200, size: 80 * gib, want: false, hasVideo: true}, + {name: "audio only is never flagged", duration: 1, size: 100 * gib, want: false, hasVideo: false}, + {name: "unknown size cannot be judged", duration: 61, size: 0, want: false, hasVideo: true}, + {name: "unknown duration is not this rule's job", duration: 0, size: 100 * gib, want: false, hasVideo: true}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + if got := videoDurationImplausible(tc.duration, tc.size, tc.hasVideo); got != tc.want { + t.Fatalf("videoDurationImplausible(%v, %d, %v) = %v; want %v", + tc.duration, tc.size, tc.hasVideo, got, tc.want) + } + }) + } +} diff --git a/internal/scanner/probe_repair.go b/internal/scanner/probe_repair.go index 255a76b90..7f0349be4 100644 --- a/internal/scanner/probe_repair.go +++ b/internal/scanner/probe_repair.go @@ -125,16 +125,21 @@ func reprobeMayScanPackets(file *models.MediaFile) bool { return false } return file.Duration <= 0 || - videoDurationImplausiblyShort(float64(file.Duration), file.FileSize, true) + videoDurationImplausible(float64(file.Duration), file.FileSize, true) } -// legacyProbeDurationFixTime is when the probe duration parser stopped -// treating large ffprobe durations as microseconds. Rows probed before this -// may carry the collapsed durations that conversion produced. Rows probed -// after it are authoritative: a still-short duration was re-derived from -// packet timestamps, and re-flagging it would reprobe genuinely short clips -// on every playback decision forever. Adjust if this fix ships later. -var legacyProbeDurationFixTime = time.Date(2026, time.July, 18, 0, 0, 0, 0, time.UTC) +// legacyProbeDurationFixTime marks the revision of the duration-validity rule +// in probe.go. Rows probed before it were judged by an older, weaker rule and +// are re-checked once under the current one. Rows probed after it are +// authoritative: their duration already passed the current rule, and +// re-flagging them would reprobe genuinely short clips on every playback +// decision forever. +// +// Bump this whenever videoDurationImplausible changes, or existing rows never +// re-converge on the improved rule. Last bumped when the implied-bitrate +// ceiling was added, which catches durations the absolute floor missed — +// a feature film probing as 61 seconds passed the old rule untouched. +var legacyProbeDurationFixTime = time.Date(2026, time.July, 26, 0, 0, 0, 0, time.UTC) func needsLegacyDurationRepair(file *models.MediaFile) bool { if file == nil { @@ -144,7 +149,7 @@ func needsLegacyDurationRepair(file *models.MediaFile) bool { } func legacyDurationRepairNeeded(duration int, sizeBytes int64, hasVideo bool, probeUpdatedAt *time.Time) bool { - if !videoDurationImplausiblyShort(float64(duration), sizeBytes, hasVideo) { + if !videoDurationImplausible(float64(duration), sizeBytes, hasVideo) { return false } return probeUpdatedAt == nil || probeUpdatedAt.Before(legacyProbeDurationFixTime) From a06316a7293557229556a45fb66a236cc35f0f80 Mon Sep 17 00:00:00 2001 From: Quick <31828688+Quick104@users.noreply.github.com> Date: Sun, 26 Jul 2026 02:03:38 +0000 Subject: [PATCH 2/3] fix(playback): publish source runtime in v3 plans and stop faking the copy seek window MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects with one root: a v3 plan described where playback sits without ever stating how long the media is. Add source.duration_seconds. It is the file's full runtime, never `total - source_start` and never adjusted by timeline_offset_seconds, and it is omitted rather than null when unknown — clients that coerce null to a numeric default would read it as zero, the exact value this field exists to stop them inventing. It is set in SourceDescriptorFromFileV3, the single place every delivery already flows through, so direct play, progressive remux, HLS remux and HLS transcode all carry it. Until now the v3 plan omitted duration entirely, so clients fell back to the playback engine. On an HLS copy remux the server intentionally serves FFmpeg's still-growing playlist, so the engine reports the length produced so far. With no server-supplied runtime to anchor on, a feature film played back as a couple of minutes. The legacy protocol already answered this correctly via fileDurationSeconds; this restores parity. Separately, the copy branch published seek_window_end_seconds as the media runtime. That made the window look *complete*, which clients read as proof that any target inside it is locally seekable, so they native-seek past the produced head of a growing playlist instead of asking for a reanchor. Leave the end open: an incomplete window plus can_seek_anywhere=false routes every seek through the server, which is what legacy did before v3 added the bound. Advertise plan_source_duration_v1 so a client can distinguish "this server does not populate the field" from "this server knows the runtime is genuinely unknown" — without it, both look like an absent field and a client cannot tell whether its own catalog fallback is still required. Co-Authored-By: Claude Opus 5 (1M context) --- internal/api/handlers/playback_v3.go | 19 +++++++++++++------ internal/api/handlers/playback_v3_test.go | 7 ++++++- internal/playback/capabilities_v3.go | 13 +++++++++++++ internal/playback/protocol_v3.go | 20 +++++++++++++++++++- internal/playback/protocol_v3_test.go | 1 + 5 files changed, 52 insertions(+), 8 deletions(-) diff --git a/internal/api/handlers/playback_v3.go b/internal/api/handlers/playback_v3.go index a5617eba3..c1f026ee4 100644 --- a/internal/api/handlers/playback_v3.go +++ b/internal/api/handlers/playback_v3.go @@ -2004,12 +2004,19 @@ func configureHLSTimelineV3(plan *playback.PlanV3, videoCodec string, segmentDur plan.Timeline.TimelineOffsetSeconds = seek windowStart := seek plan.Timeline.SeekWindowStartSeconds = &windowStart - if durationSeconds > 0 { - windowEnd := durationSeconds - plan.Timeline.SeekWindowEndSeconds = &windowEnd - } else { - plan.Timeline.SeekWindowEndSeconds = nil - } + // A copy remux is served from FFmpeg's live, still-growing playlist + // (see BuildPlaybackManifest), so the seekable extent is whatever has + // been produced so far — a value this plan cannot know and could not + // keep current if it did. Publishing the media runtime here instead + // made the window look *complete*, which clients read as proof that + // any target inside it is locally seekable; they then native-seek past + // the produced head instead of asking for a reanchor. Leaving the end + // open marks the window incomplete, which with can_seek_anywhere=false + // routes every seek back through the server. + // + // The media runtime is published on source.duration_seconds, which is + // a fact about the file rather than a claim about this transport. + plan.Timeline.SeekWindowEndSeconds = nil plan.Timeline.CanSeekAnywhere = false plan.Timeline.SeekRestoration = "source_position" } else { diff --git a/internal/api/handlers/playback_v3_test.go b/internal/api/handlers/playback_v3_test.go index ed7a54b03..4cbe529d3 100644 --- a/internal/api/handlers/playback_v3_test.go +++ b/internal/api/handlers/playback_v3_test.go @@ -1195,11 +1195,16 @@ func TestHandleStartPlaybackLegacyBranchPreservesTrailingBodyBehavior(t *testing } func TestConfigureHLSTimelineV3MatchesTransportSeekSemantics(t *testing.T) { + // A copy remux streams FFmpeg's growing playlist, so its seek window must + // stay open-ended even though the runtime is known. A bounded window reads + // as "complete", which clients treat as proof that any target inside it is + // locally seekable — sending them past the produced head instead of back + // to the server. The runtime belongs on source.duration_seconds. copyPlan := &playback.PlanV3{Timeline: playback.TimelineV3{SourceStartSeconds: 17.3}} copySeek, copySegment := configureHLSTimelineV3(copyPlan, "copy", 2, 600) if copySeek != 17.3 || copySegment != 8 || copyPlan.Timeline.StreamOriginSeconds != 17.3 || copyPlan.Timeline.TimelineOffsetSeconds != 17.3 || copyPlan.Timeline.PlayerStartSeconds != 0 || copyPlan.Timeline.CanSeekAnywhere || copyPlan.Timeline.SeekWindowStartSeconds == nil || *copyPlan.Timeline.SeekWindowStartSeconds != 17.3 || - copyPlan.Timeline.SeekWindowEndSeconds == nil || *copyPlan.Timeline.SeekWindowEndSeconds != 600 || + copyPlan.Timeline.SeekWindowEndSeconds != nil || copyPlan.Timeline.SeekRestoration != "source_position" { t.Fatalf("copy timeline=%#v seek=%v segment=%d", copyPlan.Timeline, copySeek, copySegment) } diff --git a/internal/playback/capabilities_v3.go b/internal/playback/capabilities_v3.go index 59b34c3a6..1ee767d90 100644 --- a/internal/playback/capabilities_v3.go +++ b/internal/playback/capabilities_v3.go @@ -7,12 +7,25 @@ import ( "github.com/Silo-Server/silo-server/internal/models" ) +// SourceDurationSecondsV3 reports a media file's runtime, or nil when it is +// unknown. models.MediaFile.Duration stores 0 for "probe failed", so a zero +// must never reach a client as a duration. This mirrors the legacy start +// response's fileDurationSeconds so both protocols answer identically. +func SourceDurationSecondsV3(file *models.MediaFile) *float64 { + if file == nil || file.Duration <= 0 { + return nil + } + duration := float64(file.Duration) + return &duration +} + func SourceDescriptorFromFileV3(file *models.MediaFile, audioIndex int) SourceDescriptorV3 { if file == nil { return SourceDescriptorV3{DVEnhancementLayer: EnhancementUnknownV3} } source := SourceDescriptorV3{ MediaFileID: file.ID, + DurationSeconds: SourceDurationSecondsV3(file), Container: normalizeCodecV3(file.Container), VideoCodec: normalizeCodecV3(file.CodecVideo), AudioCodec: normalizeCodecV3(file.CodecAudio), diff --git a/internal/playback/protocol_v3.go b/internal/playback/protocol_v3.go index b8157451c..9d761c4d4 100644 --- a/internal/playback/protocol_v3.go +++ b/internal/playback/protocol_v3.go @@ -21,6 +21,7 @@ const ( FeatureDeviceQuirksV3 = "device_quirks_v1" FeatureSeekReanchorV3 = "seek_reanchor_v1" FeatureDirectStreamResumeV3 = "direct_stream_resume_v1" + FeaturePlanSourceDurationV3 = "plan_source_duration_v1" PlanRecipeVersionV3 = "v3.2" ClientDV7ToDV81V3 = "client_dv7_to_dv81" ClientDV7ToHDR10V3 = "client_dv7_to_hdr10" @@ -44,6 +45,12 @@ func ServerFeaturesV3() []string { FeatureDeviceQuirksV3, FeatureSeekReanchorV3, FeatureDirectStreamResumeV3, + // Advertised so a client can tell "this server does not populate + // source.duration_seconds" apart from "this server knows the runtime + // is genuinely unknown". Without the distinction both look like an + // absent field, and a client cannot decide whether its own catalog + // fallback is still required. + FeaturePlanSourceDurationV3, } } @@ -423,7 +430,18 @@ type EffectiveRecipeV3 struct { } type SourceDescriptorV3 struct { - MediaFileID int `json:"media_file_id"` + MediaFileID int `json:"media_file_id"` + // DurationSeconds is the full runtime of this source, independent of where + // the delivery's timeline is anchored: never `total - source_start`, and + // never adjusted by timeline_offset_seconds. + // + // Absent means the server does not know the runtime. It is omitted rather + // than sent as null: clients that coerce null to a numeric default would + // read it as zero, which is the value this field exists to stop them + // inventing. A client must not substitute the playback engine's reported + // duration for it — on an HLS copy remux the engine reports the length + // produced so far, not the runtime. + DurationSeconds *float64 `json:"duration_seconds,omitempty"` Container string `json:"container,omitempty"` VideoCodec string `json:"video_codec,omitempty"` VideoProfile string `json:"video_profile,omitempty"` diff --git a/internal/playback/protocol_v3_test.go b/internal/playback/protocol_v3_test.go index 898a94def..65f757eac 100644 --- a/internal/playback/protocol_v3_test.go +++ b/internal/playback/protocol_v3_test.go @@ -30,6 +30,7 @@ func TestServerFeaturesV3ReturnsCompleteIndependentSlices(t *testing.T) { FeatureDeviceQuirksV3: {}, FeatureSeekReanchorV3: {}, FeatureDirectStreamResumeV3: {}, + FeaturePlanSourceDurationV3: {}, } if len(first) != len(expected) { t.Fatalf("server features = %v, want %d entries", first, len(expected)) From ebec4141a8fd619dc45c58b95f7b0abda56ef9ff Mon Sep 17 00:00:00 2001 From: Quick <31828688+Quick104@users.noreply.github.com> Date: Sun, 26 Jul 2026 02:10:41 +0000 Subject: [PATCH 3/3] fix(web): pair the exit position with the media runtime, not the element duration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The player's exit state converts its position to media time but took the duration from the video element, which is player-local. On a remux or transcode stream the element only covers the window produced so far, so the two values live in different coordinate systems. Resuming a movie 50 minutes in makes that concrete: the exit position is ~3060s of media time while the element reports ~120s. The progress cache then evaluates `position >= duration`, marks the item completed, latches the watched badge, and — because completion clears the resume point — resets position to 0. Exiting a resumed movie destroyed the resume point and claimed it had been watched. The server's runtime is authoritative and already expressed in media time, so prefer it and fall back to the element only when no server value exists. The rule moves into mediaTimeline.ts next to the coordinate conversions it depends on, which is also what makes it testable — VideoPlayer itself has no test harness. Co-Authored-By: Claude Opus 5 (1M context) --- web/src/player/components/VideoPlayer.tsx | 10 ++--- web/src/player/utils/mediaTimeline.test.ts | 45 ++++++++++++++++++++++ web/src/player/utils/mediaTimeline.ts | 26 +++++++++++++ 3 files changed, 74 insertions(+), 7 deletions(-) create mode 100644 web/src/player/utils/mediaTimeline.test.ts diff --git a/web/src/player/components/VideoPlayer.tsx b/web/src/player/components/VideoPlayer.tsx index 229781ca7..0e819e38b 100644 --- a/web/src/player/components/VideoPlayer.tsx +++ b/web/src/player/components/VideoPlayer.tsx @@ -56,7 +56,7 @@ import type { SeriesContext, SubtitleMode, } from "../types"; -import { toMediaTime, toPlayerTime } from "../utils/mediaTimeline"; +import { mediaDurationSeconds, toMediaTime, toPlayerTime } from "../utils/mediaTimeline"; import { copyWatchTogetherInvite, endWatchTogetherRoom, @@ -744,12 +744,8 @@ export function VideoPlayer({ const buildExitState = useCallback((): PlaybackExitState => { const video = videoRef.current; const positionSeconds = toMediaTime(video?.currentTime ?? currentTime, streamOriginRef.current); - const durationSeconds = - duration > 0 - ? duration - : backendDurationRef.current > 0 - ? backendDurationRef.current - : undefined; + // positionSeconds is media time, so the runtime paired with it must be too. + const durationSeconds = mediaDurationSeconds(backendDurationRef.current, duration); return { positionSeconds, diff --git a/web/src/player/utils/mediaTimeline.test.ts b/web/src/player/utils/mediaTimeline.test.ts new file mode 100644 index 000000000..f71c75e8b --- /dev/null +++ b/web/src/player/utils/mediaTimeline.test.ts @@ -0,0 +1,45 @@ +import { describe, expect, it } from "vitest"; + +import { mediaDurationSeconds, toMediaTime, toPlayerTime } from "./mediaTimeline"; + +describe("toMediaTime / toPlayerTime", () => { + it("round-trips a position through a stream origin", () => { + expect(toMediaTime(60, 3000)).toBe(3060); + expect(toPlayerTime(3060, 3000)).toBe(60); + }); + + it("never returns a negative time", () => { + expect(toMediaTime(-10, 0)).toBe(0); + expect(toPlayerTime(10, 3000)).toBe(0); + }); +}); + +describe("mediaDurationSeconds", () => { + it("prefers the server runtime over the element duration", () => { + expect(mediaDurationSeconds(5400, 120)).toBe(5400); + }); + + // The regression this function exists for: a copy remux resumed at 50 + // minutes reports a player-local duration covering only the produced + // window. Pairing that with a media-time position of ~3060 would read as + // "finished", latching the item watched and clearing its resume point. + it("does not let a produced-window duration stand in for the runtime", () => { + const positionSeconds = toMediaTime(60, 3000); + const duration = mediaDurationSeconds(5400, 120); + + expect(duration).toBe(5400); + expect(positionSeconds >= (duration ?? 0)).toBe(false); + }); + + it("falls back to the element duration only when the server has none", () => { + expect(mediaDurationSeconds(0, 120)).toBe(120); + expect(mediaDurationSeconds(null, 120)).toBe(120); + expect(mediaDurationSeconds(undefined, 120)).toBe(120); + }); + + it("returns undefined when neither runtime is known, so callers omit it", () => { + expect(mediaDurationSeconds(0, 0)).toBeUndefined(); + expect(mediaDurationSeconds(null, undefined)).toBeUndefined(); + expect(mediaDurationSeconds(undefined, NaN)).toBeUndefined(); + }); +}); diff --git a/web/src/player/utils/mediaTimeline.ts b/web/src/player/utils/mediaTimeline.ts index c92a6bd51..33f4c9752 100644 --- a/web/src/player/utils/mediaTimeline.ts +++ b/web/src/player/utils/mediaTimeline.ts @@ -5,3 +5,29 @@ export function toMediaTime(playerTimeSeconds: number, streamOriginSeconds = 0): export function toPlayerTime(mediaTimeSeconds: number, streamOriginSeconds = 0): number { return Math.max(0, mediaTimeSeconds - streamOriginSeconds); } + +/** + * Picks the runtime that belongs alongside a media-time position. + * + * The server's runtime is authoritative and already in media time. The + * element's duration is player-local and, on a remux or transcode stream, + * covers only the window produced so far — pairing it with a media-time + * position makes a resumed movie look finished within seconds of starting, + * which latches the item watched and clears its resume point. + * + * The element duration is therefore only a last resort, for the case where the + * server supplied no runtime at all. Returns undefined when neither is known, + * so callers can omit the value instead of publishing a zero. + */ +export function mediaDurationSeconds( + backendDurationSeconds: number | null | undefined, + elementDurationSeconds: number | null | undefined, +): number | undefined { + if (backendDurationSeconds != null && backendDurationSeconds > 0) { + return backendDurationSeconds; + } + if (elementDurationSeconds != null && elementDurationSeconds > 0) { + return elementDurationSeconds; + } + return undefined; +}