Skip to content

Commit 3652e22

Browse files
committed
fix: address 2 new review comments and CI mypy failures on PR #20
Two new review comments arrived after the previous round. This commit addresses both plus the CI mypy failure they exposed. CodeRabbit Major (comment #36) — race in _ensure_jobs initialization: - plexmuxy_gui/api.py: pywebview runs exposed js_api methods on separate threads, so the lazy first-access path in _ensure_jobs could create duplicate JobStore/JobQueue instances over the same state store. Added a threading.Lock (_jobs_lock) to __init__ and wrapped the lazy initialization so concurrent callers cannot double-initialize. CodeRabbit Minor (comment #37) — gate _embed_ass_subtitles on format: - plexmuxy/muxer.py: scanner.py forwards configured subtitle_extensions unchanged and the default config includes .ssa, so non-ASS inputs can reach _embed_ass_subtitles. Added _ASS_SUBTITLE_SUFFIXES = {'.ass', '.ssa'} and skip tracks whose suffix is not ASS/SSA compatible, leaving them for normal muxing instead of passing them to embed_fonts_into_ass. CI mypy failure (exposed by the earlier os.startfile change): - plexmuxy_gui/api.py: os.startfile is Windows-only and absent from the Linux CI type stubs. Kept the direct os.startfile(path) call inside the sys.platform == 'win32' guard with '# type: ignore[attr-defined]'. Verified with 'mypy --platform linux' (Success) since the ignore is required and consumed on the Linux CI where os.startfile does not exist in the stubs. All 341 non-integration tests pass; ruff and mypy (Linux platform) report no issues.
1 parent bb29b20 commit 3652e22

2 files changed

Lines changed: 26 additions & 7 deletions

File tree

plexmuxy/muxer.py

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,12 @@ def _embed_ass_subtitles(
8686
if not track.path or not track.path.exists():
8787
produced.append(None)
8888
continue
89+
if track.path.suffix.lower() not in _ASS_SUBTITLE_SUFFIXES:
90+
# Only ASS/SSA formats support the embedded [Fonts] section; skip
91+
# incompatible subtitle tracks (e.g. .srt, .sub, .smi) so they are
92+
# left untouched for normal muxing.
93+
produced.append(None)
94+
continue
8995
suffix = f".embedded.{index}.ass" if multi else ".embedded.ass"
9096
out = parent / f"{stem}{suffix}"
9197
try:
@@ -147,6 +153,11 @@ def execute_prepared_mux_plan(
147153
".ttf", ".otf", ".ttc", ".otc", ".woff", ".woff2",
148154
})
149155

156+
# Subtitle formats that share the ASS/SSA ``[Fonts]`` section syntax and can
157+
# receive embedded fonts. Other formats (e.g. .srt, .sub) are not compatible
158+
# with embed_fonts_into_ass and must be skipped during ASS embedding.
159+
_ASS_SUBTITLE_SUFFIXES = frozenset({".ass", ".ssa"})
160+
150161

151162
def _execute_runtime_plan(
152163
original_plan: MuxPlan,

plexmuxy_gui/api.py

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ def __init__(
190190
self._state_path = state_path
191191
self._job_store: JobStore | None = None
192192
self._job_queue: JobQueue | None = None
193+
self._jobs_lock = threading.Lock()
193194
self._preview = AudioPreviewManager(preview_root)
194195
self._active_plan_ids: dict[Path, str] = {}
195196
self._last_diagnostics_path: Path | None = None
@@ -1107,11 +1108,15 @@ def run() -> dict[str, Any]:
11071108
return self.guarded(run)
11081109

11091110
def _ensure_jobs(self) -> tuple[JobStore, JobQueue]:
1110-
if self._job_store is None:
1111-
self._job_store = JobStore(self._state_path or platform_state_path())
1112-
if self._job_queue is None:
1113-
self._job_queue = JobQueue(self._job_store, terminal_callback=self._notify_job_terminal)
1114-
return self._job_store, self._job_queue
1111+
# pywebview exposed methods run on separate threads, so the lazy
1112+
# first-access path must be guarded to avoid creating duplicate
1113+
# JobStore/JobQueue instances over the same state store.
1114+
with self._jobs_lock:
1115+
if self._job_store is None:
1116+
self._job_store = JobStore(self._state_path or platform_state_path())
1117+
if self._job_queue is None:
1118+
self._job_queue = JobQueue(self._job_store, terminal_callback=self._notify_job_terminal)
1119+
return self._job_store, self._job_queue
11151120

11161121
def _request_context(self, payload: dict[str, Any]):
11171122
if not isinstance(payload, dict):
@@ -1376,8 +1381,11 @@ def requires_delete_confirmation(config) -> bool:
13761381

13771382

13781383
def open_path(path: Path) -> None:
1379-
if os.name == "nt":
1380-
os.startfile(path)
1384+
if sys.platform == "win32":
1385+
# os.startfile is a Windows-only API; the guard above ensures this
1386+
# branch only executes on Windows. The ignore is required because the
1387+
# Linux CI type stubs have no os.startfile attribute.
1388+
os.startfile(path) # type: ignore[attr-defined]
13811389
return
13821390
if sys.platform == "darwin":
13831391
subprocess.Popen(["open", str(path)])

0 commit comments

Comments
 (0)