fix(subtitles): address review feedback on AI translation
- Heartbeat queued jobs (before the semaphore), not just running ones, so the stale-job reaper can't fail a job that is alive but waiting for a slot (which would otherwise let it resurrect itself on acquire, or admit a duplicate once the partial unique index no longer sees it as active). - Filter the translate modal's source list to the formats the server accepts (text external/downloaded, non-bitmap embedded), so an unsupported source is prevented up front instead of failing the job asynchronously. - Tell a duplicate requester (e.g. a second viewer, or after a reload) that a translation is already in progress instead of silently closing; the finished track still arrives via the subtitle_ready refresh. - Note in the admin card that AI subtitle settings take effect after a restart (the running service snapshots them at construction). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -279,6 +279,14 @@ func (s *Service) dispatch(job Job) {
|
||||
cancel()
|
||||
}()
|
||||
|
||||
// Heartbeat for the whole lifetime — crucially including while queued
|
||||
// behind the semaphore — so the stale-job reaper never reaps a job that is
|
||||
// alive but merely waiting for a slot (which would otherwise mark it failed
|
||||
// and let it resurrect itself on acquire, or admit a duplicate).
|
||||
stopHeartbeat := make(chan struct{})
|
||||
defer close(stopHeartbeat)
|
||||
go s.heartbeatLoop(runCtx, job.ID, stopHeartbeat)
|
||||
|
||||
// Bound concurrency so translation never starves transcodes.
|
||||
select {
|
||||
case s.sem <- struct{}{}:
|
||||
@@ -292,27 +300,25 @@ func (s *Service) dispatch(job Job) {
|
||||
}()
|
||||
}
|
||||
|
||||
func (s *Service) run(ctx context.Context, job *Job) {
|
||||
// Keep heartbeat_at fresh while the job runs (progress updates also bump it),
|
||||
// so the stale-job reaper never resets a job that is still alive during a long
|
||||
// single LLM call.
|
||||
stopHeartbeat := make(chan struct{})
|
||||
defer close(stopHeartbeat)
|
||||
go func() {
|
||||
ticker := time.NewTicker(heartbeatInterval)
|
||||
defer ticker.Stop()
|
||||
for {
|
||||
select {
|
||||
case <-stopHeartbeat:
|
||||
return
|
||||
case <-ctx.Done():
|
||||
return
|
||||
case <-ticker.C:
|
||||
_ = s.repo.Heartbeat(context.WithoutCancel(ctx), job.ID)
|
||||
}
|
||||
// heartbeatLoop keeps a job's heartbeat_at fresh until the job ends or the
|
||||
// context is cancelled, so the stale-job reaper only ever reaps jobs orphaned by
|
||||
// a crashed worker.
|
||||
func (s *Service) heartbeatLoop(ctx context.Context, jobID int64, stop <-chan struct{}) {
|
||||
ticker := time.NewTicker(heartbeatInterval)
|
||||
defer ticker.Stop()
|
||||
for {
|
||||
select {
|
||||
case <-stop:
|
||||
return
|
||||
case <-ctx.Done():
|
||||
return
|
||||
case <-ticker.C:
|
||||
_ = s.repo.Heartbeat(context.WithoutCancel(ctx), jobID)
|
||||
}
|
||||
}()
|
||||
}
|
||||
}
|
||||
|
||||
func (s *Service) run(ctx context.Context, job *Job) {
|
||||
if err := s.repo.UpdateProgress(ctx, job.ID, JobStatusRunning, 0, "Loading subtitle"); err != nil {
|
||||
s.logger.Warn("failed to mark subtitle ai job running", "job", job.ID, "error", err)
|
||||
}
|
||||
|
||||
@@ -536,6 +536,9 @@ function AISubtitleTranslationCard() {
|
||||
<Button type="button" onClick={save} disabled={updateSetting.isPending}>
|
||||
{updateSetting.isPending ? "Saving..." : "Save AI Translation Settings"}
|
||||
</Button>
|
||||
<p className="text-muted-foreground mt-2 text-xs">
|
||||
Changes take effect after a server restart.
|
||||
</p>
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
|
||||
@@ -0,0 +1,38 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { isTranslatableSource } from "./SubtitleTranslateModal";
|
||||
import type { PlayerSubtitleInfo } from "../types";
|
||||
|
||||
function track(p: Partial<PlayerSubtitleInfo>): PlayerSubtitleInfo {
|
||||
return { index: 0, language: "en", label: "", url: "", ...p };
|
||||
}
|
||||
|
||||
describe("isTranslatableSource", () => {
|
||||
it("accepts text external/downloaded subtitles", () => {
|
||||
expect(isTranslatableSource(track({ source: "external", codec: "srt" }))).toBe(true);
|
||||
expect(isTranslatableSource(track({ source: "downloaded", codec: "subrip" }))).toBe(true);
|
||||
expect(isTranslatableSource(track({ source: "external", codec: "vtt" }))).toBe(true);
|
||||
});
|
||||
|
||||
it("rejects ASS/SSA external/downloaded subtitles the server can't parse", () => {
|
||||
expect(isTranslatableSource(track({ source: "external", codec: "ass" }))).toBe(false);
|
||||
expect(isTranslatableSource(track({ source: "downloaded", codec: "ssa" }))).toBe(false);
|
||||
});
|
||||
|
||||
it("accepts non-bitmap embedded tracks (extracted via ffmpeg, incl. ASS)", () => {
|
||||
expect(isTranslatableSource(track({ source: "embedded", codec: "ass" }))).toBe(true);
|
||||
expect(isTranslatableSource(track({ source: "embedded", codec: "subrip" }))).toBe(true);
|
||||
});
|
||||
|
||||
it("rejects bitmap embedded tracks", () => {
|
||||
expect(isTranslatableSource(track({ source: "embedded", codec: "hdmv_pgs_subtitle" }))).toBe(
|
||||
false,
|
||||
);
|
||||
expect(isTranslatableSource(track({ source: "embedded", codec: "dvd_subtitle" }))).toBe(false);
|
||||
});
|
||||
|
||||
it("rejects the in-progress live track", () => {
|
||||
expect(isTranslatableSource(track({ source: "downloaded", codec: "srt", live: true }))).toBe(
|
||||
false,
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -1,10 +1,29 @@
|
||||
import { useState, useEffect, useCallback, useMemo } from "react";
|
||||
import { createPortal } from "react-dom";
|
||||
import { toast } from "sonner";
|
||||
import type { PlayerConfig } from "../context/PlayerConfigContext";
|
||||
import type { PlayerSubtitleInfo } from "../types";
|
||||
import { playerFetch } from "../player-fetch";
|
||||
import { LANGUAGES, getLanguageName } from "../utils/languageNames";
|
||||
|
||||
// Formats the server can parse directly from an external/downloaded file.
|
||||
const TRANSLATABLE_TEXT_CODECS = new Set(["srt", "subrip", "vtt", "webvtt"]);
|
||||
// Bitmap codecs the server can't extract as text (it would burn them in).
|
||||
const BITMAP_CODECS = new Set(["pgs", "hdmv_pgs_subtitle", "dvd_subtitle", "dvb_subtitle"]);
|
||||
|
||||
// Mirror the server's loadSource acceptance: embedded non-bitmap tracks are
|
||||
// extracted to text via ffmpeg, while external/downloaded sources must already
|
||||
// be a parseable text format. Offering anything else starts a job that fails
|
||||
// asynchronously instead of preventing the choice up front.
|
||||
export function isTranslatableSource(track: PlayerSubtitleInfo): boolean {
|
||||
if (track.live) return false;
|
||||
const codec = (track.codec ?? "").toLowerCase();
|
||||
if (track.source === "embedded") {
|
||||
return !BITMAP_CODECS.has(codec);
|
||||
}
|
||||
return TRANSLATABLE_TEXT_CODECS.has(codec);
|
||||
}
|
||||
|
||||
interface SubtitleTranslateModalProps {
|
||||
mediaFileId: number;
|
||||
playerConfig: PlayerConfig;
|
||||
@@ -30,8 +49,9 @@ export function SubtitleTranslateModal({
|
||||
getStartPosition,
|
||||
onClose,
|
||||
}: SubtitleTranslateModalProps) {
|
||||
// Live (in-progress) tracks can't be a translation source.
|
||||
const sourceTracks = useMemo(() => tracks.filter((t) => !t.live), [tracks]);
|
||||
// Only offer sources the server can actually translate (excludes live tracks,
|
||||
// bitmap embedded tracks, and ASS/non-text external/downloaded tracks).
|
||||
const sourceTracks = useMemo(() => tracks.filter(isTranslatableSource), [tracks]);
|
||||
const [sourceIndex, setSourceIndex] = useState<number | null>(null);
|
||||
const [targetLang, setTargetLang] = useState("en");
|
||||
const [submitting, setSubmitting] = useState(false);
|
||||
@@ -54,18 +74,30 @@ export function SubtitleTranslateModal({
|
||||
setSubmitting(true);
|
||||
setError(null);
|
||||
try {
|
||||
await playerFetch(playerConfig, "/subtitles/ai/translate", {
|
||||
method: "POST",
|
||||
body: JSON.stringify({
|
||||
media_file_id: mediaFileId,
|
||||
source_index: effectiveSourceIndex,
|
||||
source_language: source?.language ?? "",
|
||||
target_language: targetLang,
|
||||
session_id: sessionId ?? "",
|
||||
start_position: getStartPosition?.() ?? 0,
|
||||
}),
|
||||
});
|
||||
// The player takes over from here: it pauses, streams cues in as they're
|
||||
const res = await playerFetch<{ job?: { status?: string } }>(
|
||||
playerConfig,
|
||||
"/subtitles/ai/translate",
|
||||
{
|
||||
method: "POST",
|
||||
body: JSON.stringify({
|
||||
media_file_id: mediaFileId,
|
||||
source_index: effectiveSourceIndex,
|
||||
source_language: source?.language ?? "",
|
||||
target_language: targetLang,
|
||||
session_id: sessionId ?? "",
|
||||
start_position: getStartPosition?.() ?? 0,
|
||||
}),
|
||||
},
|
||||
);
|
||||
// A request that collapses onto an already-running job (e.g. after a
|
||||
// reload, or a second viewer) won't get its own live stream — tell the
|
||||
// user it's underway; it'll appear via the subtitle-ready refresh.
|
||||
if (res?.job?.status === "running") {
|
||||
toast.info(
|
||||
"A translation for this track is already in progress — it'll appear when it's ready.",
|
||||
);
|
||||
}
|
||||
// Otherwise the player takes over: it pauses, streams cues in as they're
|
||||
// translated, then resumes once your position is covered.
|
||||
onClose();
|
||||
} catch (err) {
|
||||
|
||||
Reference in New Issue
Block a user