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>
This commit is contained in:
committed by
GitHub
co-authored by
Claude Fable 5
parent
fadd8ff456
commit
0eb87fa0f5
@@ -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));
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user