fix(dub+win): dialect↔cinematic guidance loop + WinError 193 ffmpeg validation (#377)
* fix(dub): break the dialect↔cinematic guidance loop (#372, #373) - Cinematic toggle refuses the pick when no LLM endpoint is configured, pointing at Settings → Credentials → LLM endpoint - backend Fast fallback now syncs the quality toggle to 'fast' - the dialect warning no longer fires alongside the cinematic-no-LLM warning (the pair formed the loop), and both messages point at the LLM endpoint settings instead of each other Fixes #372 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(ffmpeg): validate resolved ffmpeg/ffprobe actually runs — fall through on WinError 193 (#360, #361, #362) A corrupt or wrong-arch imageio-ffmpeg download (and WindowsApps alias stubs) passes os.path.isfile/shutil.which but explodes at spawn with '[WinError 193] %1 is not a valid Win32 application', killing transcription with an opaque 500. Every resolution step now probes the candidate with '-version' (cached per process), logs the rejected basename, and falls through to the next source. Fixes #362 Fixes #361 Fixes #360 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: mergetest <test@local> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Palash Debnath
co-authored by
Claude Fable 5
mergetest
Claude Fable 5
mergetest
parent
66f2ea7e50
commit
6140f888e1
@@ -23,6 +23,36 @@ def _get_semaphore() -> asyncio.Semaphore:
|
||||
return _FFMPEG_SEMAPHORE
|
||||
|
||||
|
||||
# Candidate paths that exist but won't run (validated once per process).
|
||||
# Windows users hit this as `[WinError 193] %1 is not a valid Win32
|
||||
# application` (#360/#361/#362): a corrupt/wrong-arch imageio-ffmpeg
|
||||
# download or a WindowsApps alias stub passes `os.path.isfile` / `which`
|
||||
# but explodes at spawn. Probe each candidate with `-version` and fall
|
||||
# through to the next source instead of returning a time bomb.
|
||||
_BINARY_OK: dict[str, bool] = {}
|
||||
|
||||
|
||||
def _binary_runs(path: str) -> bool:
|
||||
cached = _BINARY_OK.get(path)
|
||||
if cached is not None:
|
||||
return cached
|
||||
try:
|
||||
subprocess.run(
|
||||
[path, "-version"],
|
||||
stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL,
|
||||
timeout=10, check=False,
|
||||
)
|
||||
ok = True
|
||||
except (OSError, subprocess.TimeoutExpired, subprocess.SubprocessError) as e:
|
||||
logger.warning(
|
||||
"Rejecting non-runnable ffmpeg/ffprobe candidate %s: %s",
|
||||
os.path.basename(str(path)), e,
|
||||
)
|
||||
ok = False
|
||||
_BINARY_OK[path] = ok
|
||||
return ok
|
||||
|
||||
|
||||
def find_ffmpeg():
|
||||
"""Locate an ffmpeg binary.
|
||||
|
||||
@@ -37,15 +67,15 @@ def find_ffmpeg():
|
||||
env_path = os.environ.get("FFMPEG_PATH")
|
||||
if env_path:
|
||||
resolved = shutil.which(env_path)
|
||||
if resolved:
|
||||
if resolved and _binary_runs(resolved):
|
||||
return resolved
|
||||
# 2. imageio-ffmpeg bundled static binary
|
||||
try:
|
||||
import imageio_ffmpeg
|
||||
candidate = imageio_ffmpeg.get_ffmpeg_exe()
|
||||
if candidate and os.path.isfile(candidate):
|
||||
if candidate and os.path.isfile(candidate) and _binary_runs(candidate):
|
||||
return candidate
|
||||
logger.debug("imageio_ffmpeg binary not found at %s", candidate)
|
||||
logger.debug("imageio_ffmpeg binary not usable at %s", candidate)
|
||||
except Exception as e:
|
||||
logger.debug("imageio_ffmpeg unavailable: %s", e)
|
||||
# 3. Well-known system paths + PATH lookup
|
||||
@@ -58,9 +88,10 @@ def find_ffmpeg():
|
||||
"ffmpeg",
|
||||
]
|
||||
for path in common:
|
||||
if shutil.which(path):
|
||||
return path
|
||||
logger.warning("ffmpeg not found in env, imageio, or system PATH")
|
||||
resolved = shutil.which(path)
|
||||
if resolved and _binary_runs(resolved):
|
||||
return resolved
|
||||
logger.warning("ffmpeg not found (or not runnable) in env, imageio, or system PATH")
|
||||
return None
|
||||
|
||||
|
||||
@@ -84,14 +115,14 @@ def resolve_ffprobe() -> str | None:
|
||||
continue
|
||||
# The env var may carry either an absolute path to a file OR a bare
|
||||
# command name (legacy). Accept both shapes — file first.
|
||||
if os.path.isfile(path):
|
||||
if os.path.isfile(path) and _binary_runs(path):
|
||||
return path
|
||||
resolved = shutil.which(path)
|
||||
if resolved:
|
||||
if resolved and _binary_runs(resolved):
|
||||
return resolved
|
||||
|
||||
system_probe = shutil.which("ffprobe")
|
||||
if system_probe:
|
||||
if system_probe and _binary_runs(system_probe):
|
||||
return system_probe
|
||||
return None
|
||||
|
||||
|
||||
@@ -375,12 +375,17 @@ export default function useDubWorkflow({ loadProjects, loadProfiles, loadDubHist
|
||||
};
|
||||
}));
|
||||
if (data.cinematic_skipped === 'no-llm-configured') {
|
||||
toast(t('dub_workflow.cinematic_no_llm'), { icon: 'ℹ️', duration: 7000 });
|
||||
toast(t('dub_workflow.cinematic_no_llm'), { icon: 'ℹ️', duration: 8000 });
|
||||
// #372: the backend fell back to Fast — reflect that in the toggle so
|
||||
// the UI doesn't claim Cinematic while delivering Fast.
|
||||
useAppStore.getState().setTranslateQuality?.('fast');
|
||||
}
|
||||
// #280: the user picked a dialect but the chosen engine can't honor it
|
||||
// (Argos/NLLB/Google in Fast mode). Tell them how to make it count.
|
||||
if (data.dialect && data.dialect_applied === false) {
|
||||
toast(t('dub_workflow.dialect_not_applied'), { icon: 'ℹ️', duration: 7000 });
|
||||
// #372: skip when the cinematic toast above already fired — both at once
|
||||
// sent users in a circle ("pick Cinematic" ↔ "Cinematic needs an LLM").
|
||||
if (data.dialect && data.dialect_applied === false && data.cinematic_skipped !== 'no-llm-configured') {
|
||||
toast(t('dub_workflow.dialect_not_applied'), { icon: 'ℹ️', duration: 8000 });
|
||||
}
|
||||
if (errors.length) {
|
||||
const unique = [...new Set(errors.map(e => e.error))];
|
||||
|
||||
@@ -1574,8 +1574,8 @@
|
||||
"cleaned_other": "Cleaned {{count}} fragments",
|
||||
"segments_clean": "Segments already clean",
|
||||
"cleanup_failed": "Clean up failed: {{message}}",
|
||||
"cinematic_no_llm": "Cinematic quality needs an LLM — set TRANSLATE_BASE_URL + TRANSLATE_API_KEY (Ollama works locally). Falling back to Fast.",
|
||||
"dialect_not_applied": "The selected dialect needs an LLM to apply — switch the Engine to OpenAI/Ollama or pick Cinematic quality.",
|
||||
"cinematic_no_llm": "Cinematic quality needs an LLM. Configure one in Settings → Credentials → LLM endpoint (Ollama runs locally, no key needed). Using Fast quality for now.",
|
||||
"dialect_not_applied": "The selected dialect needs an LLM to apply. Switch the translation engine to OpenAI/Ollama, or configure an LLM in Settings → Credentials → LLM endpoint.",
|
||||
"translate_errors": "{{errorCount}}/{{totalCount}} segment(s) failed: {{firstError}}",
|
||||
"translated_segments": "Translated {{count}} segment(s) → {{lang}}",
|
||||
"translated_cinematic_suffix": " (Cinematic)",
|
||||
|
||||
@@ -133,6 +133,18 @@ export default function DubTab(props) {
|
||||
const activeProjectName = useAppStore(s => s.activeProjectName);
|
||||
const translateQuality = useAppStore(s => s.translateQuality);
|
||||
const setTranslateQuality = useAppStore(s => s.setTranslateQuality);
|
||||
// #372: live LLM availability so the Cinematic toggle can refuse the pick
|
||||
// (instead of looping the user between two warnings). null until loaded.
|
||||
const [llmEndpoint, setLlmEndpoint] = useState(null);
|
||||
useEffect(() => {
|
||||
let cancelled = false;
|
||||
import('../api/client').then(({ apiJson }) =>
|
||||
apiJson('/api/settings/llm-endpoint')
|
||||
.then(d => { if (!cancelled) setLlmEndpoint(d); })
|
||||
.catch(() => { /* backend mid-boot — guard simply stays permissive */ })
|
||||
);
|
||||
return () => { cancelled = true; };
|
||||
}, []);
|
||||
const dualSubs = useAppStore(s => s.dualSubs);
|
||||
const setDualSubs = useAppStore(s => s.setDualSubs);
|
||||
const burnSubs = useAppStore(s => s.burnSubs);
|
||||
@@ -992,7 +1004,16 @@ export default function DubTab(props) {
|
||||
<Segmented
|
||||
size="sm"
|
||||
value={translateQuality}
|
||||
onChange={setTranslateQuality}
|
||||
onChange={(v) => {
|
||||
// #372: picking Cinematic with no LLM configured used to
|
||||
// bounce the user between two warnings forever. Block the
|
||||
// pick at the source and point at the actual fix.
|
||||
if (v === 'cinematic' && llmEndpoint && !llmEndpoint.available) {
|
||||
toast(t('dub.cinematic_needs_llm_hint', { defaultValue: 'Cinematic needs an LLM. Configure one in Settings → Credentials → LLM endpoint (Ollama runs locally, no key needed).' }), { icon: 'ℹ️', duration: 8000 });
|
||||
return;
|
||||
}
|
||||
setTranslateQuality(v);
|
||||
}}
|
||||
items={[
|
||||
{ value: 'fast', label: t('dub.fast_quality') },
|
||||
{ value: 'cinematic', label: t('dub.cinematic_quality') },
|
||||
|
||||
@@ -71,9 +71,35 @@ def test_resolve_ffprobe_falls_back_to_PATH(monkeypatch, tmp_path):
|
||||
return None
|
||||
|
||||
monkeypatch.setattr(ffmpeg_utils, "shutil", _ShutilStub(_fake_which))
|
||||
# The fake path isn't a real binary — bypass the runnability probe here
|
||||
# (rejection behavior has its own test below).
|
||||
monkeypatch.setattr(ffmpeg_utils, "_binary_runs", lambda _p: True)
|
||||
assert ffmpeg_utils.resolve_ffprobe() == fake
|
||||
|
||||
|
||||
def test_resolve_ffprobe_rejects_non_runnable_candidate(monkeypatch, tmp_path):
|
||||
"""#360/#361/#362: a candidate that exists but cannot execute (corrupt /
|
||||
wrong-arch binary → WinError 193 on Windows) is skipped and the cascade
|
||||
falls through to the next runnable source."""
|
||||
from services import ffmpeg_utils
|
||||
|
||||
broken = tmp_path / "ffprobe-broken"
|
||||
broken.write_bytes(b"\x00\x01not-a-binary")
|
||||
broken.chmod(0o755)
|
||||
good = tmp_path / "ffprobe-good"
|
||||
good.write_text("#!/bin/sh\necho 1\n")
|
||||
good.chmod(0o755)
|
||||
|
||||
monkeypatch.setenv("OMNIVOICE_FFPROBE_PATH", str(broken))
|
||||
monkeypatch.setattr(
|
||||
ffmpeg_utils, "shutil",
|
||||
_ShutilStub(lambda name: str(good) if name == "ffprobe" else None),
|
||||
)
|
||||
ffmpeg_utils._BINARY_OK.clear()
|
||||
|
||||
assert ffmpeg_utils.resolve_ffprobe() == str(good)
|
||||
|
||||
|
||||
def test_resolve_ffprobe_returns_None_when_nothing_resolves(monkeypatch):
|
||||
"""No env, no PATH → returns None (no crash)."""
|
||||
from services import ffmpeg_utils
|
||||
|
||||
Reference in New Issue
Block a user