fix(dub): close shared workflow review gaps
This commit is contained in:
@@ -507,13 +507,28 @@ _ingest_gen = dub_pipeline.ingest_pipeline
|
||||
#: container so a mislabelled video can't slip past the video-skipping branch.
|
||||
_AUDIO_EXTS = {".wav", ".mp3", ".m4a", ".aac", ".flac", ".ogg", ".opus", ".wma"}
|
||||
|
||||
# Source-language choices exposed by the first-party dub UI. Keeping this an
|
||||
# allow-list rejects language names and private-use BCP-47 tags before they are
|
||||
# persisted as ASR overrides. Values are normalized to lowercase below.
|
||||
_DUB_SOURCE_LANG_CODES = frozenset({
|
||||
"af", "sq", "am", "ar", "hy", "az", "eu", "be", "bn", "bs", "bg",
|
||||
"my", "ca", "cmn-hans", "cmn-hant", "hr", "cs", "da", "nl", "en",
|
||||
"et", "fi", "fr", "gl", "ka", "de", "el", "gu", "ht", "ha", "haw",
|
||||
"he", "hi", "hu", "is", "id", "it", "ja", "jw", "kn", "kk", "km",
|
||||
"ko", "ku", "ky", "lo", "la", "lv", "lt", "mk", "ms", "ml", "mt",
|
||||
"mi", "mr", "mn", "ne", "no", "ps", "fa", "pl", "pt", "pa", "ro",
|
||||
"ru", "sm", "gd", "sr", "sn", "sd", "si", "sk", "sl", "so", "es",
|
||||
"su", "sw", "sv", "tg", "ta", "te", "th", "tr", "uk", "ur", "uz",
|
||||
"vi", "cy", "xh", "yi", "yo", "zu",
|
||||
})
|
||||
|
||||
|
||||
def _source_lang_override(value: str | None) -> str | None:
|
||||
"""Normalize a user-selected source language; auto/und means detect."""
|
||||
code = (value or "").strip().lower()
|
||||
if code in {"", "auto", "und"}:
|
||||
return None
|
||||
if len(code) > 35 or not all(ch.isalnum() or ch == "-" for ch in code):
|
||||
if code not in _DUB_SOURCE_LANG_CODES:
|
||||
raise HTTPException(status_code=400, detail="Invalid source language code")
|
||||
return code
|
||||
|
||||
@@ -553,6 +568,7 @@ async def dub_upload(
|
||||
detail=f"Audio-only dubbing needs an audio file ({', '.join(sorted(_AUDIO_EXTS))}); got '{ext or 'no extension'}'.",
|
||||
)
|
||||
|
||||
source_lang_override = _source_lang_override(source_lang)
|
||||
os.makedirs(job_dir, exist_ok=True)
|
||||
|
||||
video_path = os.path.join(job_dir, f"original{ext}")
|
||||
@@ -568,7 +584,7 @@ async def dub_upload(
|
||||
"kind": "file",
|
||||
"path": video_path,
|
||||
"input_type": input_type,
|
||||
"source_lang": _source_lang_override(source_lang),
|
||||
"source_lang": source_lang_override,
|
||||
},
|
||||
filename,
|
||||
)
|
||||
@@ -592,6 +608,7 @@ async def dub_ingest_url(req: DubIngestUrlRequest, request: Request):
|
||||
status_code=400,
|
||||
detail="URL must start with http:// or https://. Paste a full video link (e.g. https://youtube.com/watch?v=…) or drop a local file instead.",
|
||||
)
|
||||
source_lang_override = _source_lang_override(req.source_lang)
|
||||
|
||||
try:
|
||||
import yt_dlp # noqa: F401
|
||||
@@ -627,7 +644,7 @@ async def dub_ingest_url(req: DubIngestUrlRequest, request: Request):
|
||||
"fetch_subs": bool(req.fetch_subs),
|
||||
"sub_langs": req.sub_langs or None,
|
||||
"cookie_file": cookie_path,
|
||||
"source_lang": _source_lang_override(req.source_lang),
|
||||
"source_lang": source_lang_override,
|
||||
}
|
||||
try:
|
||||
await task_manager.add_task(
|
||||
|
||||
@@ -755,35 +755,45 @@ async def _ensure_browser_playable_mp4_for_job(job_id: str, video_path: str) ->
|
||||
run_proc = run_proc_factory(job_id)
|
||||
ffmpeg_bin = find_ffmpeg()
|
||||
|
||||
async def attempt(cmd: list[str]) -> int:
|
||||
try:
|
||||
proc, _stdout, _stderr = await run_proc(cmd, timeout=1800.0)
|
||||
return proc.returncode
|
||||
except asyncio.CancelledError:
|
||||
raise
|
||||
except Exception as exc:
|
||||
logger.warning(
|
||||
"Browser-media normalization process failed for %s: %s",
|
||||
log_safe(video_path),
|
||||
log_safe(exc),
|
||||
)
|
||||
return 1
|
||||
|
||||
rc = 1
|
||||
if not is_mp4:
|
||||
proc, _stdout, _stderr = await run_proc(
|
||||
rc = await attempt(
|
||||
[
|
||||
ffmpeg_bin, "-y", "-i", video_path,
|
||||
"-c:v", "copy", "-c:a", "copy",
|
||||
"-movflags", "+faststart", target,
|
||||
],
|
||||
timeout=1800.0,
|
||||
]
|
||||
)
|
||||
rc = proc.returncode
|
||||
if rc != 0 or not os.path.exists(target):
|
||||
rc = 1
|
||||
if rc != 0:
|
||||
proc, _stdout, _stderr = await run_proc(
|
||||
rc = await attempt(
|
||||
[
|
||||
ffmpeg_bin, "-y", "-i", video_path,
|
||||
"-c:v", "libx264", "-preset", "veryfast", "-crf", "23",
|
||||
"-pix_fmt", "yuv420p", "-c:a", "aac", "-b:a", "192k",
|
||||
"-movflags", "+faststart", target,
|
||||
],
|
||||
timeout=1800.0,
|
||||
]
|
||||
)
|
||||
rc = proc.returncode
|
||||
if rc == 0 and os.path.exists(target) and target != video_path:
|
||||
try:
|
||||
os.remove(video_path)
|
||||
except OSError:
|
||||
pass
|
||||
pass # Best effort: the normalized target is already complete.
|
||||
return target
|
||||
logger.warning(
|
||||
"Could not transcode %s to browser-playable mp4 — the in-app "
|
||||
|
||||
@@ -117,17 +117,21 @@ def _merge_segment_extra(target: Segment, incoming: Segment, *, prepend: bool) -
|
||||
else joined(target_original, incoming_original)
|
||||
)
|
||||
|
||||
target_translations = target.extra.get("translations")
|
||||
incoming_translations = incoming.extra.get("translations")
|
||||
if isinstance(target_translations, dict) or isinstance(incoming_translations, dict):
|
||||
raw_target_translations = target.extra.get("translations")
|
||||
raw_incoming_translations = incoming.extra.get("translations")
|
||||
target_translations = raw_target_translations if isinstance(raw_target_translations, dict) else {}
|
||||
incoming_translations = (
|
||||
raw_incoming_translations if isinstance(raw_incoming_translations, dict) else {}
|
||||
)
|
||||
if target_translations or incoming_translations:
|
||||
merged = {}
|
||||
languages = {
|
||||
*(target_translations or {}).keys(),
|
||||
*(incoming_translations or {}).keys(),
|
||||
*target_translations.keys(),
|
||||
*incoming_translations.keys(),
|
||||
}
|
||||
for language in languages:
|
||||
target_text = (target_translations or {}).get(language)
|
||||
incoming_text = (incoming_translations or {}).get(language)
|
||||
target_text = target_translations.get(language)
|
||||
incoming_text = incoming_translations.get(language)
|
||||
merged[language] = (
|
||||
joined(incoming_text, target_text)
|
||||
if prepend
|
||||
|
||||
@@ -12,11 +12,12 @@ from fastapi import UploadFile
|
||||
async def test_preview_ffmpeg_does_not_block_event_loop(monkeypatch, tmp_path):
|
||||
from api.routers import dub_core
|
||||
|
||||
started = threading.Event()
|
||||
loop = asyncio.get_running_loop()
|
||||
started = asyncio.Event()
|
||||
release = threading.Event()
|
||||
|
||||
def slow_ffmpeg(*_args, **_kwargs):
|
||||
started.set()
|
||||
loop.call_soon_threadsafe(started.set)
|
||||
assert release.wait(timeout=2)
|
||||
|
||||
monkeypatch.setattr(dub_core, "PREVIEW_DIR", str(tmp_path))
|
||||
@@ -24,16 +25,10 @@ async def test_preview_ffmpeg_does_not_block_event_loop(monkeypatch, tmp_path):
|
||||
monkeypatch.setattr(dub_core.subprocess, "run", slow_ffmpeg)
|
||||
upload = UploadFile(filename="preview.mp4", file=io.BytesIO(b"video"))
|
||||
|
||||
loop = asyncio.get_running_loop()
|
||||
before = loop.time()
|
||||
task = asyncio.create_task(dub_core.preview_upload(upload))
|
||||
try:
|
||||
deadline = loop.time() + 1.0
|
||||
while not started.is_set():
|
||||
if loop.time() >= deadline:
|
||||
pytest.fail("ffmpeg extraction did not start")
|
||||
await asyncio.sleep(0.01)
|
||||
await asyncio.sleep(0.05)
|
||||
await asyncio.wait_for(started.wait(), timeout=1)
|
||||
assert loop.time() - before < 0.5
|
||||
finally:
|
||||
release.set()
|
||||
|
||||
@@ -70,7 +70,6 @@ export default function useDubWorkflow({
|
||||
const dubLang = useAppStore((s) => s.dubLang);
|
||||
const dubLangCode = useAppStore((s) => s.dubLangCode);
|
||||
const dubSourceLangCode = useAppStore((s) => s.dubSourceLangCode);
|
||||
const setDubSourceLangCode = useAppStore((s) => s.setDubSourceLangCode);
|
||||
const dubInstruct = useAppStore((s) => s.dubInstruct);
|
||||
const setDubFilename = useAppStore((s) => s.setDubFilename);
|
||||
const setDubDuration = useAppStore((s) => s.setDubDuration);
|
||||
@@ -286,9 +285,6 @@ export default function useDubWorkflow({
|
||||
const castSources = m.cast_sources || m.speaker_clones || {};
|
||||
setDubSegments(applySpeakerCloneDefaults(normalized, castSources));
|
||||
setDubTranscript(m.full_transcript || '');
|
||||
if (useAppStore.getState().dubSourceLangCode === 'auto' && m.source_lang) {
|
||||
setDubSourceLangCode(m.source_lang);
|
||||
}
|
||||
if (castSources && typeof castSources === 'object') {
|
||||
setSpeakerClones(castSources);
|
||||
}
|
||||
@@ -365,7 +361,7 @@ export default function useDubWorkflow({
|
||||
).then(reject, reject);
|
||||
});
|
||||
}),
|
||||
[setDubSegments, setDubTranscript, setSpeakerClones, setDubSourceLangCode],
|
||||
[setDubSegments, setDubTranscript, setSpeakerClones],
|
||||
);
|
||||
|
||||
// ── SSE: wait for prep pipeline ──
|
||||
@@ -860,12 +856,18 @@ export default function useDubWorkflow({
|
||||
// that language rather than rendering a wrong-language track.
|
||||
const handleTranslateAll = useCallback(
|
||||
async (langOverride) => {
|
||||
const options =
|
||||
langOverride && typeof langOverride === 'object' && !('preventDefault' in langOverride)
|
||||
? langOverride
|
||||
: {};
|
||||
const targetLang =
|
||||
typeof langOverride === 'string' && langOverride ? langOverride : dubLangCode;
|
||||
typeof langOverride === 'string' && langOverride
|
||||
? langOverride
|
||||
: options.langOverride || dubLangCode;
|
||||
// Snapshot segments at call time: inside the multi-language loop the
|
||||
// click-time closure is stale after the previous pick's translate pass.
|
||||
const allSegments = useAppStore.getState().dubSegments;
|
||||
const retryFailed = !!langOverride?.retryFailed;
|
||||
const retryFailed = !!options.retryFailed;
|
||||
const segs = retryFailed
|
||||
? allSegments.filter((segment) => segment.translate_error)
|
||||
: allSegments;
|
||||
|
||||
@@ -322,28 +322,31 @@ export default function DubTab(props) {
|
||||
// while still restoring the primary target in the editor afterwards. Each
|
||||
// translation lands in segments[].translations[code], so Generate can
|
||||
// reuse the complete maps without retranslating or losing another language.
|
||||
const onTranslateClick = useCallback(async () => {
|
||||
if (!multiLangMode) return handleTranslateAll();
|
||||
if (multiBatchRunningRef.current) return false;
|
||||
multiBatchRunningRef.current = true;
|
||||
setMultiBatchBusy(true);
|
||||
const { lang: primaryLanguage, code: primaryCode } = primaryTargetRef.current;
|
||||
let allOk = true;
|
||||
try {
|
||||
for (const target of batchTargets) {
|
||||
setDubLang(target.lang);
|
||||
switchDubLangCode(target.code);
|
||||
const ok = await handleTranslateAll(target.code);
|
||||
if (!ok) allOk = false;
|
||||
const onTranslateClick = useCallback(
|
||||
async (options = {}) => {
|
||||
if (!multiLangMode) return handleTranslateAll(options);
|
||||
if (multiBatchRunningRef.current) return false;
|
||||
multiBatchRunningRef.current = true;
|
||||
setMultiBatchBusy(true);
|
||||
const { lang: primaryLanguage, code: primaryCode } = primaryTargetRef.current;
|
||||
let allOk = true;
|
||||
try {
|
||||
for (const target of batchTargets) {
|
||||
setDubLang(target.lang);
|
||||
switchDubLangCode(target.code);
|
||||
const ok = await handleTranslateAll({ ...options, langOverride: target.code });
|
||||
if (!ok) allOk = false;
|
||||
}
|
||||
} finally {
|
||||
setDubLang(primaryLanguage);
|
||||
switchDubLangCode(primaryCode);
|
||||
multiBatchRunningRef.current = false;
|
||||
setMultiBatchBusy(false);
|
||||
}
|
||||
} finally {
|
||||
setDubLang(primaryLanguage);
|
||||
switchDubLangCode(primaryCode);
|
||||
multiBatchRunningRef.current = false;
|
||||
setMultiBatchBusy(false);
|
||||
}
|
||||
return allOk;
|
||||
}, [multiLangMode, batchTargets, handleTranslateAll, setDubLang, switchDubLangCode]);
|
||||
return allOk;
|
||||
},
|
||||
[multiLangMode, batchTargets, handleTranslateAll, setDubLang, switchDubLangCode],
|
||||
);
|
||||
|
||||
// Live ETA while generating — elapsed ticks each second; remaining is
|
||||
// extrapolated from the current/total rate so it's only meaningful once
|
||||
|
||||
@@ -117,6 +117,40 @@ describe('Dubbing missing-ASR recovery', () => {
|
||||
expect(useAppStore.getState().dubJobId).not.toBe('job-kept-for-retry');
|
||||
});
|
||||
|
||||
it('keeps auto detection selected across two consecutive jobs', async () => {
|
||||
let uploadNumber = 0;
|
||||
dubApi.dubUpload.mockImplementation(async () => {
|
||||
uploadNumber += 1;
|
||||
return { job_id: `job-${uploadNumber}`, task_id: `prep-${uploadNumber}` };
|
||||
});
|
||||
const { result } = renderWorkflow();
|
||||
|
||||
const runUpload = async (detectedLanguage) => {
|
||||
const streamStart = streams.length;
|
||||
let upload;
|
||||
act(() => {
|
||||
upload = result.current.handleDubUpload(
|
||||
new File(['video'], `job-${uploadNumber + 1}.mp4`, { type: 'video/mp4' }),
|
||||
);
|
||||
});
|
||||
await waitFor(() => expect(streams).toHaveLength(streamStart + 1));
|
||||
streams[streamStart].emit('message', { type: 'ready' });
|
||||
await waitFor(() => expect(streams).toHaveLength(streamStart + 2));
|
||||
streams[streamStart + 1].emit('final', {
|
||||
segments: [{ id: '1', text: 'hello' }],
|
||||
source_lang: detectedLanguage,
|
||||
});
|
||||
streams[streamStart + 1].emit('done');
|
||||
await act(async () => upload);
|
||||
};
|
||||
|
||||
await runUpload('es');
|
||||
await runUpload('de');
|
||||
|
||||
expect(dubApi.dubUpload.mock.calls.map((call) => call[2].sourceLang)).toEqual(['auto', 'auto']);
|
||||
expect(useAppStore.getState().dubSourceLangCode).toBe('auto');
|
||||
});
|
||||
|
||||
it('keeps the job, installs inline, then automatically retranscribes it', async () => {
|
||||
const { result } = renderWorkflow();
|
||||
let firstAttempt;
|
||||
|
||||
@@ -14,14 +14,19 @@ import { useAppStore } from '../store';
|
||||
// and on a failed translate: skip that pick's generate, keep going, report
|
||||
// the skipped languages in a final toast.
|
||||
|
||||
const captured = vi.hoisted(() => ({ header: [] }));
|
||||
const captured = vi.hoisted(() => ({ header: [], left: [] }));
|
||||
vi.mock('../components/dub/DubHeader', () => ({
|
||||
default: (props) => {
|
||||
captured.header.push(props);
|
||||
return null;
|
||||
},
|
||||
}));
|
||||
vi.mock('../components/dub/DubLeftColumn', () => ({ default: () => null }));
|
||||
vi.mock('../components/dub/DubLeftColumn', () => ({
|
||||
default: (props) => {
|
||||
captured.left.push(props);
|
||||
return null;
|
||||
},
|
||||
}));
|
||||
vi.mock('../components/dub/DubRightColumn', () => ({ default: () => null }));
|
||||
vi.mock('../components/dub/DubFooter', () => ({ default: () => null }));
|
||||
vi.mock('../components/dub/DubPipelineStepper', () => ({ default: () => null }));
|
||||
@@ -101,6 +106,7 @@ const PICKS = [
|
||||
{ lang: 'French', code: 'fr' },
|
||||
{ lang: 'German', code: 'de' },
|
||||
];
|
||||
const EXPECTED_CODES = ['bn', ...PICKS.map((pick) => pick.code)];
|
||||
|
||||
/** Render DubTab in multi-lang mode and return { onGenerateClick, calls, mocks }. */
|
||||
function setup({
|
||||
@@ -110,7 +116,8 @@ function setup({
|
||||
segments,
|
||||
} = {}) {
|
||||
const calls = [];
|
||||
const handleTranslateAll = vi.fn(async (code) => {
|
||||
const handleTranslateAll = vi.fn(async (arg) => {
|
||||
const code = typeof arg === 'string' ? arg : arg?.langOverride;
|
||||
calls.push(`translate:${code}`);
|
||||
const ok = translateOk(code);
|
||||
if (ok) {
|
||||
@@ -155,6 +162,19 @@ describe('DubTab — multi-language generate translates each language first (P1.
|
||||
beforeEach(() => {
|
||||
useAppStore.setState(baseState, true);
|
||||
captured.header.length = 0;
|
||||
captured.left.length = 0;
|
||||
});
|
||||
|
||||
it('forwards retry-only options through every language in the wrapper', async () => {
|
||||
const { handleTranslateAll } = setup();
|
||||
|
||||
await act(async () => {
|
||||
await captured.left.at(-1).handleTranslateAll({ retryFailed: true });
|
||||
});
|
||||
|
||||
expect(handleTranslateAll.mock.calls.map(([options]) => options)).toEqual(
|
||||
EXPECTED_CODES.map((langOverride) => ({ retryFailed: true, langOverride })),
|
||||
);
|
||||
});
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
|
||||
@@ -5,6 +5,8 @@ from __future__ import annotations
|
||||
import asyncio
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
|
||||
from services import dub_pipeline as dp
|
||||
|
||||
|
||||
@@ -123,3 +125,25 @@ def test_upload_normalization_propagates_cancellation_to_registered_process(tmp_
|
||||
|
||||
asyncio.run(cancel_normalization())
|
||||
assert seen == {"job_id": "cancel-job", "timeout": 1800.0}
|
||||
|
||||
|
||||
@pytest.mark.parametrize("failure", [RuntimeError("spawn failed"), asyncio.TimeoutError()])
|
||||
def test_upload_normalization_failure_keeps_the_original_media(tmp_path, monkeypatch, failure):
|
||||
source = tmp_path / "source.mp4"
|
||||
source.write_bytes(b"source")
|
||||
monkeypatch.setattr(dp, "_probe_codecs", lambda _path: ("vp9", "opus"))
|
||||
monkeypatch.setattr(dp, "find_ffmpeg", lambda: "ffmpeg")
|
||||
|
||||
def factory(_job_id):
|
||||
async def run_proc(_cmd, *, timeout):
|
||||
assert timeout == 1800.0
|
||||
raise failure
|
||||
|
||||
return run_proc
|
||||
|
||||
monkeypatch.setattr(dp, "run_proc_factory", factory)
|
||||
|
||||
result = asyncio.run(dp._ensure_browser_playable_mp4_for_job("failed-job", str(source)))
|
||||
|
||||
assert result == str(source)
|
||||
assert source.exists()
|
||||
|
||||
@@ -37,3 +37,28 @@ def test_cleanup_preserves_editor_metadata_and_combines_translations():
|
||||
}
|
||||
assert cleaned[0]["profile_id"] == "voice-a"
|
||||
assert cleaned[0]["translate_error"] == "retry me"
|
||||
|
||||
|
||||
def test_cleanup_ignores_legacy_non_mapping_translations():
|
||||
segments = [
|
||||
{
|
||||
"id": "a",
|
||||
"start": 0.0,
|
||||
"end": 2.0,
|
||||
"text": "Hello.",
|
||||
"speaker_id": "Speaker 1",
|
||||
"translations": "legacy-corrupt-value",
|
||||
},
|
||||
{
|
||||
"id": "b",
|
||||
"start": 2.1,
|
||||
"end": 2.4,
|
||||
"text": "Again.",
|
||||
"speaker_id": "Speaker 1",
|
||||
"translations": {"es": "Otra vez."},
|
||||
},
|
||||
]
|
||||
|
||||
cleaned = clean_up_segments(segments)
|
||||
|
||||
assert cleaned[0]["translations"] == {"es": "Otra vez."}
|
||||
|
||||
@@ -292,3 +292,38 @@ class TestAudioOnlyDubbing:
|
||||
)
|
||||
assert r.status_code == 400
|
||||
assert "audio file" in r.json()["detail"].lower()
|
||||
|
||||
def test_ingest_requests_reject_unregistered_source_languages(self, app_client):
|
||||
client, _dc, _dx, _tmp = app_client
|
||||
|
||||
form = client.post(
|
||||
"/dub/upload",
|
||||
files={"video": ("clip.mp4", b"video", "video/mp4")},
|
||||
data={"source_lang": "english"},
|
||||
)
|
||||
json_response = client.post(
|
||||
"/dub/ingest-url",
|
||||
json={"url": "https://example.com/video", "source_lang": "x-123"},
|
||||
)
|
||||
|
||||
assert form.status_code == 400
|
||||
assert json_response.status_code == 400
|
||||
assert form.json()["detail"] == "Invalid source language code"
|
||||
assert json_response.json()["detail"] == "Invalid source language code"
|
||||
|
||||
def test_upload_accepts_a_registered_source_language(self, app_client, monkeypatch):
|
||||
client, dc, _dx, _tmp = app_client
|
||||
queued = []
|
||||
|
||||
async def add_task(*args):
|
||||
queued.append(args)
|
||||
|
||||
monkeypatch.setattr(dc.task_manager, "add_task", add_task)
|
||||
response = client.post(
|
||||
"/dub/upload",
|
||||
files={"video": ("clip.mp4", b"video", "video/mp4")},
|
||||
data={"source_lang": "FR"},
|
||||
)
|
||||
|
||||
assert response.status_code == 202
|
||||
assert queued[0][5]["source_lang"] == "fr"
|
||||
|
||||
Reference in New Issue
Block a user