From 0eb87fa0f5a1945fcda7264389f2c14d30cb1114 Mon Sep 17 00:00:00 2001
From: Quick <31828688+Quick104@users.noreply.github.com>
Date: Wed, 10 Jun 2026 20:28:46 -0400
Subject: [PATCH] fix(player): flip ASS subtitle delay sign to match VTT
 semantics (#130)

JASSUB renders the ASS event matching video.currentTime + timeOffset,
so an event at source time S appears at video time S - timeOffset.
Adding the user delay to the offset therefore showed ASS subtitles
EARLIER on positive delay, while the VTT path (and the subtitle menu's
"later" label) shifts cues later. At +2000ms an ASS track diverged from
an SRT track by a full 4 seconds.

Subtract the delay instead, correct the comment to state the actual
JASSUB render equation, and add tests covering the constructed
timeOffset for positive/negative delays plus the live-instance update
path.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
---
 web/src/player/hooks/useASSSubtitles.test.tsx | 48 ++++++++++++++++++-
 web/src/player/hooks/useASSSubtitles.ts       | 11 +++--
 2 files changed, 54 insertions(+), 5 deletions(-)

diff --git a/web/src/player/hooks/useASSSubtitles.test.tsx b/web/src/player/hooks/useASSSubtitles.test.tsx
index b2f8f340..12afc0fd 100644
--- a/web/src/player/hooks/useASSSubtitles.test.tsx
+++ b/web/src/player/hooks/useASSSubtitles.test.tsx
@@ -4,8 +4,10 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import { useASSSubtitles } from "./useASSSubtitles";
 import type { PlayerSubtitleInfo } from "../types";
 
-// Capture the options every JASSUB instance is constructed with.
+// Capture the options every JASSUB instance is constructed with, plus the
+// instances themselves so tests can observe later timeOffset updates.
 const constructorOpts: Array<Record<string, unknown>> = [];
+const instances: Array<{ timeOffset: number }> = [];
 
 vi.mock("jassub", () => {
   class MockJASSUB {
@@ -14,6 +16,8 @@ vi.mock("jassub", () => {
     renderer = { setTrackByUrl: vi.fn().mockResolvedValue(undefined) };
     constructor(opts: Record<string, unknown>) {
       constructorOpts.push(opts);
+      this.timeOffset = (opts.timeOffset as number) ?? 0;
+      instances.push(this);
     }
     resize = vi.fn().mockResolvedValue(undefined);
     destroy = vi.fn();
@@ -80,6 +84,7 @@ function mockFontBundleResponse(bytes: string): Response {
 
 beforeEach(() => {
   constructorOpts.length = 0;
+  instances.length = 0;
   vi.stubGlobal("fetch", vi.fn().mockResolvedValue(mockFetchResponse("")));
 });
 
@@ -176,3 +181,44 @@ describe("useASSSubtitles font fallback", () => {
     expect(constructorOpts[0]!.queryFonts).toBe(false);
   });
 });
+
+describe("useASSSubtitles time offset", () => {
+  // JASSUB renders the ASS event matching `video.currentTime + timeOffset`,
+  // so an event at source time S appears at video time S - timeOffset.
+  // Positive user delay means "show subtitles later" (VTTCue semantics in
+  // useSubtitleTracks shifts cues by `start - origin + delay`), which for
+  // JASSUB requires SUBTRACTING the delay from the stream origin.
+
+  it("subtracts a positive user delay from the constructed timeOffset", async () => {
+    renderHook(() => useASSSubtitles(makeVideoRef(), [germanTrack], 6, false, 30, 2000));
+
+    await waitFor(() => expect(constructorOpts).toHaveLength(1));
+
+    // origin 30s, +2000ms delay → event at source time S renders at video
+    // time S - 28 = (S - 30) + 2, i.e. 2s later than the undelayed position.
+    expect(constructorOpts[0]!.timeOffset).toBe(28);
+  });
+
+  it("adds a negative user delay to the constructed timeOffset", async () => {
+    renderHook(() => useASSSubtitles(makeVideoRef(), [germanTrack], 6, false, 30, -2000));
+
+    await waitFor(() => expect(constructorOpts).toHaveLength(1));
+
+    expect(constructorOpts[0]!.timeOffset).toBe(32);
+  });
+
+  it("updates the live instance's timeOffset when the delay changes", async () => {
+    const videoRef = makeVideoRef();
+    const { rerender } = renderHook(
+      ({ delay }) => useASSSubtitles(videoRef, [germanTrack], 6, false, 30, delay),
+      { initialProps: { delay: 0 } },
+    );
+
+    await waitFor(() => expect(instances).toHaveLength(1));
+    expect(instances[0]!.timeOffset).toBe(30);
+
+    rerender({ delay: 2000 });
+
+    await waitFor(() => expect(instances[0]!.timeOffset).toBe(28));
+  });
+});
diff --git a/web/src/player/hooks/useASSSubtitles.ts b/web/src/player/hooks/useASSSubtitles.ts
index 6ba84816..3efa129d 100644
--- a/web/src/player/hooks/useASSSubtitles.ts
+++ b/web/src/player/hooks/useASSSubtitles.ts
@@ -31,10 +31,13 @@ export function useASSSubtitles(
 ): { isActive: boolean } {
   const jassubRef = useRef<JASSUB | null>(null);
   const jassubImportRef = useRef<Promise<typeof JASSUB> | null>(null);
-  // Effective JASSUB time offset. `streamOriginSeconds` accounts for HLS
-  // PTS rebasing; the user-facing delay (ms → s) adds on top. Positive
-  // delay = subtitles shown later, matching VTTCue semantics.
-  const effectiveOffset = streamOriginSeconds + subtitleDelayMs / 1000;
+  // Effective JASSUB time offset. JASSUB renders the ASS event matching
+  // `video.currentTime + timeOffset`, so an event at source time S appears
+  // at video time S - timeOffset. `streamOriginSeconds` accounts for HLS
+  // PTS rebasing; the user-facing delay (ms → s) must be SUBTRACTED so that
+  // positive delay = subtitles shown later, matching the VTT path's
+  // `start - origin + delay` cue shift.
+  const effectiveOffset = streamOriginSeconds - subtitleDelayMs / 1000;
   const streamOriginRef = useRef(effectiveOffset);
   streamOriginRef.current = effectiveOffset;