From 44677f1bb5d2990d7519d6c17feaad36e4950765 Mon Sep 17 00:00:00 2001 From: TN019 Date: Fri, 7 Aug 2026 02:46:03 +1000 Subject: [PATCH 1/2] Player controls that keep up: a burst of 10s skips coalesces into one backend seek instead of one per click (twelve clicks used to mean twelve decode restarts and a frozen dialog), drawn transport icons and a click-anywhere seek bar replace the emoji and the stock slider, and switching both subtitle tracks off finally hides them. --- src/scripto/gui_qt/icons.py | 121 +++++++++++++ src/scripto/gui_qt/player.py | 321 +++++++++++++++++++++++++++++++---- tests/test_player.py | 114 ++++++++++++- 3 files changed, 522 insertions(+), 34 deletions(-) create mode 100644 src/scripto/gui_qt/icons.py diff --git a/src/scripto/gui_qt/icons.py b/src/scripto/gui_qt/icons.py new file mode 100644 index 0000000..9e56dce --- /dev/null +++ b/src/scripto/gui_qt/icons.py @@ -0,0 +1,121 @@ +"""Painted transport icons for the player. + +Emoji glyphs (⏪ ⏩ ⏸) render differently on every platform and never match +the app's own weight or color, so the transport controls draw their own: +a circular arrow with the skip amount inside, plus play/pause. Everything +is a vector path painted into a device-pixel-ratio-correct pixmap, so the +icons follow the theme's text color and stay crisp on retina displays. +""" + +from __future__ import annotations + +import math + +from PySide6.QtCore import QPointF, QRectF, Qt +from PySide6.QtGui import QColor, QFont, QIcon, QPainter, QPainterPath, QPen, QPixmap + +# Gap left at the top of the circle for the arrowhead, in degrees. +_ARC_START = 120 +_ARC_SPAN = 300 + + +def _canvas(size: int, ratio: float) -> QPixmap: + pixmap = QPixmap(round(size * ratio), round(size * ratio)) + pixmap.setDevicePixelRatio(ratio) + pixmap.fill(Qt.GlobalColor.transparent) + return pixmap + + +def _painter(pixmap: QPixmap) -> QPainter: + painter = QPainter(pixmap) + painter.setRenderHint(QPainter.RenderHint.Antialiasing) + painter.setRenderHint(QPainter.RenderHint.TextAntialiasing) + return painter + + +def skip_icon(seconds: int, *, forward: bool, color: str, + size: int = 24, ratio: float = 2.0) -> QIcon: + """A circular arrow with ``seconds`` inside — ⟳10 / ⟲10.""" + pixmap = _canvas(size, ratio) + painter = _painter(pixmap) + tint = QColor(color) + + painter.save() + if not forward: # the back arrow is the forward one, mirrored + painter.translate(size, 0) + painter.scale(-1, 1) + + margin = size * 0.14 + rect = QRectF(margin, margin, size - 2 * margin, size - 2 * margin) + pen = QPen(tint) + pen.setWidthF(size * 0.085) + pen.setCapStyle(Qt.PenCapStyle.RoundCap) + painter.setPen(pen) + painter.setBrush(Qt.BrushStyle.NoBrush) + painter.drawArc(rect, _ARC_START * 16, _ARC_SPAN * 16) + + # Arrowhead at the arc's open end, aimed along the clockwise tangent. + theta = math.radians(_ARC_START) + radius = rect.width() / 2 + end_x = rect.center().x() + radius * math.cos(theta) + end_y = rect.center().y() - radius * math.sin(theta) + dir_x, dir_y = math.sin(theta), math.cos(theta) # clockwise tangent + perp_x, perp_y = -dir_y, dir_x + # The head continues the stroke rather than straddling its end, so the + # arrow reads as one line that grew a point. + length, half = size * 0.21, size * 0.105 + base_x, base_y = end_x - dir_x * length * 0.25, end_y - dir_y * length * 0.25 + head = QPainterPath() + head.moveTo(base_x + dir_x * length, base_y + dir_y * length) + head.lineTo(base_x + perp_x * half, base_y + perp_y * half) + head.lineTo(base_x - perp_x * half, base_y - perp_y * half) + head.closeSubpath() + painter.setPen(Qt.PenStyle.NoPen) + painter.setBrush(tint) + painter.drawPath(head) + painter.restore() + + font = QFont() + font.setPixelSize(max(7, round(size * 0.40))) + font.setBold(True) + painter.setFont(font) + painter.setPen(tint) + painter.drawText( + rect.adjusted(0, size * 0.045, 0, size * 0.045), + Qt.AlignmentFlag.AlignCenter, + str(seconds), + ) + painter.end() + return QIcon(pixmap) + + +def play_icon(color: str, size: int = 24, ratio: float = 2.0) -> QIcon: + pixmap = _canvas(size, ratio) + painter = _painter(pixmap) + painter.setPen(Qt.PenStyle.NoPen) + painter.setBrush(QColor(color)) + triangle = QPainterPath() + triangle.moveTo(QPointF(size * 0.30, size * 0.20)) + triangle.lineTo(QPointF(size * 0.80, size * 0.50)) + triangle.lineTo(QPointF(size * 0.30, size * 0.80)) + triangle.closeSubpath() + painter.drawPath(triangle) + painter.end() + return QIcon(pixmap) + + +def pause_icon(color: str, size: int = 24, ratio: float = 2.0) -> QIcon: + pixmap = _canvas(size, ratio) + painter = _painter(pixmap) + painter.setPen(Qt.PenStyle.NoPen) + painter.setBrush(QColor(color)) + bar, gap = size * 0.15, size * 0.12 + top, height = size * 0.21, size * 0.58 + left = (size - (2 * bar + gap)) / 2 + radius = bar * 0.35 + painter.drawRoundedRect(QRectF(left, top, bar, height), radius, radius) + painter.drawRoundedRect( + QRectF(left + bar + gap, top, bar, height), radius, radius + ) + painter.end() + return QIcon(pixmap) diff --git a/src/scripto/gui_qt/player.py b/src/scripto/gui_qt/player.py index c61bf3a..987d1cf 100644 --- a/src/scripto/gui_qt/player.py +++ b/src/scripto/gui_qt/player.py @@ -17,6 +17,16 @@ - Overlong cues are split at word boundaries into chunks of at most ``MAX_CUE_CHARS`` characters, and the chunks share the cue's time span evenly — long paragraphs page through instead of flooding the screen. + +Controls: +- Every position change — buttons, arrow keys, clicking or dragging the bar + — funnels through ``_queue_seek``: the UI moves at once, the backend is + seeked once the burst of input stops. Seeking is the expensive operation + (a decode restart), so one per burst is the difference between instant + and stuck buffering. +- The transport icons and the seek bar are drawn here rather than borrowed + from the platform: emoji glyphs and stock slider parts neither match the + theme nor behave like a player's. """ from __future__ import annotations @@ -25,8 +35,8 @@ import re from pathlib import Path -from PySide6.QtCore import QSizeF, Qt, QUrl -from PySide6.QtGui import QColor, QFont, QTextOption +from PySide6.QtCore import QPoint, QPointF, QRectF, QSize, QSizeF, Qt, QTimer, QUrl +from PySide6.QtGui import QColor, QFont, QPainter, QTextOption from PySide6.QtMultimedia import QAudioOutput, QMediaPlayer from PySide6.QtMultimediaWidgets import QGraphicsVideoItem from PySide6.QtWidgets import ( @@ -40,10 +50,12 @@ QHBoxLayout, QPushButton, QSlider, + QToolTip, QVBoxLayout, ) from ..translate.srt import parse_srt +from . import icons, theme from .widgets import subtext _TIME_RE = re.compile(r"(\d+):(\d+):(\d+)[,.](\d+)") @@ -54,6 +66,18 @@ SKIP_MS = 10_000 RATES = (0.25, 0.5, 1.0, 1.5, 2.0, 2.5, 3.0) +# A seek is a full decode restart in the media backend, so a burst of skip +# clicks must not become a burst of seeks — the backend would spend the +# whole burst buffering and the UI would freeze on the last one. Clicks +# accumulate into a single pending target, committed once the burst stops. +SEEK_COMMIT_MS = 140 +# The position the backend reports lags a committed seek (it lands on a +# keyframe, and stale positions keep arriving meanwhile). Until it settles +# within this tolerance — or the backstop below fires — reported positions +# are ignored, so the slider never snaps back under the user's cursor. +SEEK_SETTLE_MS = 400 +SEEK_SETTLE_TIMEOUT_MS = 1200 + def timestamp_ms(text: str) -> int: """`00:01:02,345` (or `.345`) → milliseconds; 0 when unparsable.""" @@ -131,6 +155,150 @@ def format_ms(ms: int) -> str: return f"{minutes}:{secs:02d}" +def _controls_qss(p) -> str: + """Flat, round icon buttons for the transport controls.""" + return f""" +QPushButton[transport="true"] {{ + background: transparent; + border: none; + border-radius: 17px; +}} +QPushButton[transport="true"]:hover {{ + background: {p.selection}; +}} +""" + + +class _SeekBar(QSlider): + """Progress bar that behaves like a player's, not like a scrollbar. + + A plain QSlider page-steps when you click the groove, which is useless + for seeking: clicking at 40% must jump to 40%. Dragging scrubs live + (``on_scrub``) and releasing commits (``on_commit``); hovering shows the + time under the cursor so you can aim before clicking, and thickens the + bar so a 5px target becomes an easy one. + + It paints itself rather than going through QSS: a styled ``::sub-page`` + ignores the groove's height and floods the whole widget rect, and the + painted geometry is the same arithmetic that maps clicks to positions, + so what you point at is what you get. + """ + + TRACK = 5.0 + TRACK_HOVER = 7.0 + DOT = 5.5 + DOT_HOVER = 7.0 + + def __init__(self, tokens, on_scrub, on_commit): + super().__init__(Qt.Orientation.Horizontal) + self.setObjectName("SeekBar") + self._tokens = tokens + self._on_scrub = on_scrub + self._on_commit = on_commit + self._scrubbing = False + self._hovered = False + self.setFixedHeight(20) + self.setMinimumWidth(120) + self.setMouseTracking(True) + self.setCursor(Qt.CursorShape.PointingHandCursor) + self.setFocusPolicy(Qt.FocusPolicy.NoFocus) # arrows belong to the dialog + + @property + def scrubbing(self) -> bool: + return self._scrubbing + + def value_at(self, x: int) -> int: + """The position (ms) the bar maps to at widget x. + + The mapping spans the full width — the far right edge is the end of + the media, with no dead margin to miss by. + """ + fraction = min(1.0, max(0.0, x / max(1.0, float(self.width())))) + return self.minimum() + round( + (self.maximum() - self.minimum()) * fraction + ) + + def _x_for(self, value: int) -> float: + span = self.maximum() - self.minimum() + fraction = 0.0 if span <= 0 else (value - self.minimum()) / span + return self.width() * min(1.0, max(0.0, fraction)) + + def paintEvent(self, event) -> None: # noqa: N802 + active = self._hovered or self._scrubbing + track = self.TRACK_HOVER if active else self.TRACK + dot = self.DOT_HOVER if active else self.DOT + top = (self.height() - track) / 2 + radius = track / 2 + played = self._x_for(self.value()) + + painter = QPainter(self) + painter.setRenderHint(QPainter.RenderHint.Antialiasing) + painter.setPen(Qt.PenStyle.NoPen) + painter.setBrush(QColor(self._tokens.border)) + painter.drawRoundedRect( + QRectF(0, top, self.width(), track), radius, radius + ) + if self.maximum() > self.minimum(): + painter.setBrush(QColor(self._tokens.accent)) + painter.drawRoundedRect( + QRectF(0, top, played, track), radius, radius + ) + painter.setBrush(QColor(self._tokens.accent if active + else self._tokens.subtext)) + # Kept a dot-radius inside the ends so the head never half-clips. + center = min(max(played, dot), self.width() - dot) + painter.drawEllipse(QPointF(center, self.height() / 2), dot, dot) + painter.end() + + def enterEvent(self, event) -> None: # noqa: N802 + self._hovered = True + self.update() + super().enterEvent(event) + + def mousePressEvent(self, event) -> None: # noqa: N802 + if event.button() != Qt.MouseButton.LeftButton or self.maximum() <= 0: + super().mousePressEvent(event) + return + self._scrubbing = True + self.setSliderDown(True) + self._scrub_to(event.position().toPoint().x()) + event.accept() + + def mouseMoveEvent(self, event) -> None: # noqa: N802 + x = event.position().toPoint().x() + if self._scrubbing: + self._scrub_to(x) + event.accept() + return + if self.maximum() > 0: + QToolTip.showText( + self.mapToGlobal(QPoint(x, -28)), + format_ms(self.value_at(x)), + self, + ) + super().mouseMoveEvent(event) + + def mouseReleaseEvent(self, event) -> None: # noqa: N802 + if not self._scrubbing: + super().mouseReleaseEvent(event) + return + self._scrubbing = False + self.setSliderDown(False) + self.setValue(self.value_at(event.position().toPoint().x())) + self._on_commit(self.value()) + event.accept() + + def leaveEvent(self, event) -> None: # noqa: N802 + self._hovered = False + self.update() + QToolTip.hideText() + super().leaveEvent(event) + + def _scrub_to(self, x: int) -> None: + self.setValue(self.value_at(x)) + self._on_scrub(self.value()) + + class _StageView(QGraphicsView): """Black letterbox stage; notifies the dialog on every resize.""" @@ -171,8 +339,19 @@ def __init__(self, parent, window, video_path: str, except Exception: continue self._active: list[list[Cue]] = [] - self._current: list[str] = ["", ""] - self._dragging = False + # None means "not decided yet", which is different from "no text": + # only the sentinel forces a slot that just went empty to be hidden. + self._current: list[str | None] = [None, None] + + self._seek_target: int | None = None + self._seek_timer = QTimer(self) + self._seek_timer.setSingleShot(True) + self._seek_timer.setInterval(SEEK_COMMIT_MS) + self._seek_timer.timeout.connect(self._commit_seek) + self._settle_timer = QTimer(self) + self._settle_timer.setSingleShot(True) + self._settle_timer.setInterval(SEEK_SETTLE_TIMEOUT_MS) + self._settle_timer.timeout.connect(self._settle_seek) self.player = QMediaPlayer(self) self.audio = QAudioOutput(self) @@ -216,21 +395,31 @@ def __init__(self, parent, window, video_path: str, scene.addItem(item) self.sub_items.append(item) - # Controls: skip / play / skip · slider · time · rate · subtitles - self.back_btn = QPushButton("⏪ 10") + # Controls: skip / play / skip · seek bar · time · rate · subtitles + tokens = getattr(window, "palette_tokens", theme.DARK) + ratio = QApplication.primaryScreen().devicePixelRatio() + seconds = SKIP_MS // 1000 + self._play_icon = icons.play_icon(tokens.text, ratio=ratio) + self._pause_icon = icons.pause_icon(tokens.text, ratio=ratio) + + self.back_btn = self._transport( + icons.skip_icon(seconds, forward=False, color=tokens.text, ratio=ratio) + ) self.back_btn.clicked.connect(lambda: self._skip(-SKIP_MS)) - self.play_btn = QPushButton("⏸") - self.play_btn.setFixedWidth(44) + self.play_btn = self._transport(self._pause_icon) self.play_btn.clicked.connect(self._toggle) - self.fwd_btn = QPushButton("10 ⏩") + self.fwd_btn = self._transport( + icons.skip_icon(seconds, forward=True, color=tokens.text, ratio=ratio) + ) self.fwd_btn.clicked.connect(lambda: self._skip(SKIP_MS)) - self.slider = QSlider(Qt.Orientation.Horizontal) + self.slider = _SeekBar(tokens, self._queue_seek, self._commit_now) self.slider.setRange(0, 0) - self.slider.sliderPressed.connect(lambda: setattr(self, "_dragging", True)) - self.slider.sliderReleased.connect(self._seek_released) - self.slider.sliderMoved.connect(self._preview_position) self.time_label = subtext("0:00 / 0:00") + self.time_label.setMinimumWidth( + self.time_label.fontMetrics().horizontalAdvance("0:00:00 / 0:00:00") + ) + self.setStyleSheet(_controls_qss(tokens)) self.rate_combo = QComboBox() for rate in RATES: @@ -254,8 +443,10 @@ def __init__(self, parent, window, video_path: str, self.sub_combos[0].setCurrentIndex(1) if len(track_labels) == 2: self.sub_combos[1].setCurrentIndex(2) + # Visible whenever there is anything to choose: with a single track + # the selector is still the only way to turn subtitles off. for combo in self.sub_combos: - combo.setVisible(len(track_labels) >= 2) + combo.setVisible(bool(track_labels)) controls = QHBoxLayout() controls.setContentsMargins(12, 8, 12, 10) @@ -285,13 +476,23 @@ def __init__(self, parent, window, video_path: str, # ------------------------------------------------------------------ # + def _transport(self, icon) -> QPushButton: + button = QPushButton() + button.setProperty("transport", "true") + button.setIcon(icon) + button.setIconSize(QSize(24, 24)) # the icons' natural size: no rescale + button.setFixedSize(34, 34) + button.setFocusPolicy(Qt.FocusPolicy.NoFocus) + button.setCursor(Qt.CursorShape.PointingHandCursor) + return button + def _sync_tracks(self) -> None: self._active = [ self.tracks.get(combo.currentData() or "", []) for combo in self.sub_combos ] - self._current = ["", ""] - self._update_subtitles(int(self.player.position())) + self._current = [None, None] # force a redraw, including "now empty" + self._update_subtitles(self._display_ms()) def _toggle(self) -> None: if self.player.playbackState() == QMediaPlayer.PlaybackState.PlayingState: @@ -299,29 +500,66 @@ def _toggle(self) -> None: else: self.player.play() + def _display_ms(self) -> int: + """Where the UI says we are: the pending seek wins over the backend.""" + if self._seek_target is not None: + return self._seek_target + return int(self.player.position()) + + # ---- seeking ------------------------------------------------------ # + def _skip(self, delta_ms: int) -> None: - target = int(self.player.position()) + delta_ms - target = max(0, min(target, int(self.player.duration()))) - self.player.setPosition(target) - self._update_subtitles(target) + # Chained off the pending target, so ten quick clicks are +100s and + # one seek — not ten seeks racing each other from the same origin. + self._queue_seek(self._display_ms() + delta_ms) + + def _queue_seek(self, ms: int) -> None: + """Show ``ms`` immediately; seek there once the burst settles.""" + target = max(0, int(ms)) + duration = int(self.player.duration()) + if duration > 0: # unknown while the source is still loading + target = min(target, duration) + self._seek_target = target + self._settle_timer.stop() + if not self.slider.scrubbing: + self.slider.setValue(self._seek_target) + self._update_time(self._seek_target) + self._update_subtitles(self._seek_target) + self._seek_timer.start() + + def _commit_now(self, ms: int) -> None: + """Release of a scrub: no reason to wait out the debounce.""" + self._queue_seek(ms) + self._commit_seek() + + def _commit_seek(self) -> None: + if self._seek_target is None: + return + self._seek_timer.stop() + self.player.setPosition(self._seek_target) + self._settle_timer.start() + + def _settle_seek(self) -> None: + """Backstop: a seek that never reports its target must not wedge us.""" + self._seek_target = None def _on_state(self, state) -> None: playing = state == QMediaPlayer.PlaybackState.PlayingState - self.play_btn.setText("⏸" if playing else "▶") - - def _seek_released(self) -> None: - self._dragging = False - self.player.setPosition(self.slider.value()) - - def _preview_position(self, ms: int) -> None: - self._update_subtitles(ms) - self._update_time(ms) + self.play_btn.setIcon(self._pause_icon if playing else self._play_icon) def _on_position(self, ms: int) -> None: - if not self._dragging: - self.slider.setValue(int(ms)) - self._update_time(int(ms)) - self._update_subtitles(int(ms)) + ms = int(ms) + if self._seek_target is not None: + pending = self._seek_timer.isActive() + if pending or abs(ms - self._seek_target) > SEEK_SETTLE_MS: + return # still queued, or the backend has not landed yet + self._seek_target = None + self._settle_timer.stop() + if self.slider.scrubbing: + return # the cursor owns the position, not the backend + self.slider.setValue(ms) + self._update_time(ms) + self._update_subtitles(ms) def _update_time(self, ms: int) -> None: self.time_label.setText( @@ -390,6 +628,23 @@ def _position_subtitles(self) -> None: backdrop.setRect(x - 6, y - 1, rect.width() + 12, rect.height() + 2) y -= 6 + def keyPressEvent(self, event) -> None: # noqa: N802 + # Space/arrows go through the same coalescing path as the buttons, + # so held-down arrows scrub instead of drowning the backend. + key = event.key() + if key == Qt.Key.Key_Space: + self._toggle() + elif key == Qt.Key.Key_Left: + self._skip(-SKIP_MS) + elif key == Qt.Key.Key_Right: + self._skip(SKIP_MS) + else: + super().keyPressEvent(event) + return + event.accept() + def closeEvent(self, event) -> None: # noqa: N802 + self._seek_timer.stop() + self._settle_timer.stop() self.player.stop() super().closeEvent(event) diff --git a/tests/test_player.py b/tests/test_player.py index ea7c45a..bca6489 100644 --- a/tests/test_player.py +++ b/tests/test_player.py @@ -1,4 +1,9 @@ -"""Player cue logic: timestamp parsing, cue lookup, time formatting.""" +"""Player cue logic: timestamp parsing, cue lookup, time formatting. + +The dialog tests below drive the widget offscreen: subtitle slots really +hide when a track is switched off, and a burst of skip clicks collapses +into a single backend seek instead of one per click. +""" from __future__ import annotations @@ -60,6 +65,113 @@ def test_split_text_hard_cuts_spaceless_cjk(): assert "".join(chunks) == text +DURATION_MS = 600_000 + + +@pytest.fixture(scope="module") +def qapp(): + from PySide6.QtWidgets import QApplication + + return QApplication.instance() or QApplication([]) + + +@pytest.fixture +def dialog(qapp, tmp_path, monkeypatch): + """A player over a stub source, with a known duration and no real seeks.""" + from scripto.gui_qt.player import PlayerDialog + + (tmp_path / "en.srt").write_text( + "1\n00:00:01,000 --> 00:00:09,000\nhello\n", encoding="utf-8" + ) + (tmp_path / "zh.srt").write_text( + "1\n00:00:01,000 --> 00:00:09,000\n你好\n", encoding="utf-8" + ) + video = tmp_path / "clip.mp4" + video.write_bytes(b"") + + dlg = PlayerDialog( + None, None, str(video), + {"EN": str(tmp_path / "en.srt"), "ZH": str(tmp_path / "zh.srt")}, + ) + monkeypatch.setattr(dlg.player, "duration", lambda: DURATION_MS) + dlg.slider.setRange(0, DURATION_MS) + yield dlg + dlg.close() + + +def test_clearing_both_tracks_hides_the_subtitles(dialog): + dialog._update_subtitles(2000) + assert [item.isVisible() for item in dialog.sub_items] == [True, True] + + for combo in dialog.sub_combos: + combo.setCurrentIndex(0) # "no subtitle" in both slots + assert [item.isVisible() for item in dialog.sub_items] == [False, False] + assert [b.isVisible() for b in dialog.sub_backdrops] == [False, False] + + dialog.sub_combos[1].setCurrentIndex(2) # and back on again + dialog._update_subtitles(2000) + assert [item.isVisible() for item in dialog.sub_items] == [False, True] + + +def test_skip_burst_collapses_into_one_seek(dialog): + seeks: list[int] = [] + dialog.player.setPosition = lambda ms: seeks.append(ms) + + for _ in range(10): + dialog._skip(10_000) + # Nothing has reached the backend yet, but the UI is already there. + assert seeks == [] + assert dialog.slider.value() == 100_000 + assert dialog.time_label.text().startswith("1:40 /") + + dialog._commit_seek() + assert seeks == [100_000] + + +def test_skips_clamp_to_the_media_bounds(dialog): + seeks: list[int] = [] + dialog.player.setPosition = lambda ms: seeks.append(ms) + + for _ in range(100): + dialog._skip(10_000) + dialog._commit_seek() + assert seeks[-1] == DURATION_MS + + for _ in range(100): + dialog._skip(-10_000) + dialog._commit_seek() + assert seeks[-1] == 0 + + +def test_stale_positions_do_not_snap_the_slider_back(dialog): + dialog.player.setPosition = lambda ms: None + + dialog._queue_seek(300_000) + dialog._on_position(12_345) # pre-seek position, still arriving + assert dialog.slider.value() == 300_000 + + dialog._commit_seek() + dialog._on_position(300_100) # backend lands on the target + assert dialog._seek_target is None + dialog._on_position(301_000) # normal tracking resumes + assert dialog.slider.value() == 301_000 + + +def test_seek_target_recovers_if_the_backend_never_lands(dialog): + dialog.player.setPosition = lambda ms: None + dialog._queue_seek(300_000) + dialog._commit_seek() + dialog._settle_seek() # the backstop timer's slot + assert dialog._seek_target is None + + +def test_clicking_the_bar_seeks_to_that_point(dialog): + dialog.slider.resize(400, 20) + assert dialog.slider.value_at(200) == pytest.approx(DURATION_MS / 2, abs=5_000) + assert dialog.slider.value_at(0) == 0 + assert dialog.slider.value_at(400) == DURATION_MS + + def test_long_cue_splits_time_evenly(): from scripto.gui_qt.player import build_cues, cue_at From 91ca14f1b518ae2bde2c2b07dcf786c9ad255c51 Mon Sep 17 00:00:00 2001 From: TN019 Date: Fri, 7 Aug 2026 02:46:11 +1000 Subject: [PATCH 2/2] =?UTF-8?q?Recognise=20the=20subtitles=20a=20video=20a?= =?UTF-8?q?rrives=20with:=20same-stem=20siblings=20are=20scanned=20and=20n?= =?UTF-8?q?amed=20by=20their=20second-level=20suffix=20(no=20suffix=20mean?= =?UTF-8?q?s=20English,=20aliases=20like=20.cn=20and=20.zh-Hans=20included?= =?UTF-8?q?,=20.part2=20is=20not=20a=20language),=20so=20an=20already=20su?= =?UTF-8?q?btitled=20file=20is=20skipped=20instead=20of=20re-transcribed?= =?UTF-8?q?=20and=20lands=20in=20history=20with=20every=20language=20it=20?= =?UTF-8?q?has=20=E2=80=94=20not=20silently=20absent,=20which=20is=20what?= =?UTF-8?q?=20skipped=20files=20were=20until=20now.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/scripto/core/output.py | 104 +++++++++++++++++++++++++++++++---- src/scripto/core/pipeline.py | 46 +++++++++++++--- tests/test_output.py | 46 ++++++++++++++++ tests/test_pipeline.py | 47 ++++++++++++++++ 4 files changed, 224 insertions(+), 19 deletions(-) diff --git a/src/scripto/core/output.py b/src/scripto/core/output.py index 3787887..0445648 100644 --- a/src/scripto/core/output.py +++ b/src/scripto/core/output.py @@ -5,6 +5,12 @@ map is configurable; a language with no mapping gets ``.`` so new languages work without code changes (R3). +Reading that naming back is just as important: a video usually arrives with +subtitles beside it, and those are named the same way by everyone else too. +``sibling_transcripts`` finds them and names their languages, so an already +subtitled file is recognised for what it is instead of re-transcribed, and +history can show every language the file actually has. + Location: next to the source file by default; a user-chosen export directory collects outputs instead (filename collisions across source folders get a numeric disambiguator). @@ -13,6 +19,7 @@ from __future__ import annotations import json +import re from pathlib import Path from ..engines.base import TranscribeResult @@ -21,6 +28,16 @@ DEFAULT_SUFFIXES = suffix_map() # single source of truth: core/languages.py FORMATS = ("srt", "txt", "vtt", "json") +# ``lecture.srt`` next to ``lecture.mp4`` — no language in the name at all — +# is how subtitles ship from everywhere that isn't us, and they are English +# nearly every time. Better a named default than an "unknown" bucket. +UNSUFFIXED_LANGUAGE = "en" + +# What we accept in the language position: "zh", "eng", "pt-BR". Anything +# else there (``lecture.part2.srt``, ``lecture.final.srt``) is part of the +# file's name, not a language, and must not be read as one. +_LANG_TAG_RE = re.compile(r"^[a-z]{2,3}(?:[-_][a-z]{2,4})?$") + def lang_suffix(language: str | None, suffix_map: dict[str, str] | None = None) -> str: suffixes = suffix_map or DEFAULT_SUFFIXES @@ -49,6 +66,71 @@ def output_path( return candidate +def language_from_suffix( + suffix: str, suffix_map: dict[str, str] | None = None +) -> str | None: + """The language a filename's middle suffix names, or None if it names none. + + ``.zh`` → ``zh``, an alias like ``.cn`` → ``zh``, nothing at all → English + (see ``UNSUFFIXED_LANGUAGE``), a well-formed but unregistered tag → itself + (writing does the mirror image: an unknown code becomes ``.``), and + ``.part2`` → None, because that is a filename, not a language. + """ + tag = suffix.lstrip(".").lower() + if not tag: + return UNSUFFIXED_LANGUAGE + for code, mapped in (suffix_map or DEFAULT_SUFFIXES).items(): + if tag == mapped.lstrip(".").lower(): + return code + for spec in known_languages(): + if any(tag == alias.lstrip(".").lower() for alias in spec.aliases): + return spec.code + return tag if _LANG_TAG_RE.match(tag) else None + + +def sibling_transcripts( + source: Path, + *, + fmt: str, + suffix_map: dict[str, str] | None = None, + export_dir: Path | None = None, +) -> dict[str, Path]: + """Language code → same-stem transcript already on disk beside ``source``. + + Matches ``.`` and ``..`` — one directory + listing, ordered by the language registry so callers get a stable + preference rather than whatever order the filesystem hands back. + """ + directory = export_dir if export_dir is not None else source.parent + prefix, tail = source.stem, f".{fmt}" + try: + entries = sorted(directory.iterdir()) + except OSError: + return {} + + found: dict[str, Path] = {} + for path in entries: + name = path.name + if not name.startswith(prefix) or not name.lower().endswith(tail.lower()): + continue + middle = name[len(prefix):len(name) - len(tail)] + if middle and not middle.startswith("."): + continue # `lecture-draft.srt` is a different file, not a language + language = language_from_suffix(middle, suffix_map) + if language is None or language in found: + continue + try: + if path.is_file(): + found[language] = path + except OSError: + continue + + order = {spec.code: i for i, spec in enumerate(known_languages())} + return dict( + sorted(found.items(), key=lambda kv: (order.get(kv[0], len(order)), kv[0])) + ) + + def existing_transcript( source: Path, *, @@ -59,11 +141,10 @@ def existing_transcript( ) -> Path | None: """Skip-check before transcribing (overwrite=False). - With a forced language the exact path is checked. With auto-detect the - language isn't known yet, so any known-suffix sibling counts as done. - Export-dir runs always re-check the exact default name only. + With a forced language the exact path is checked, plus that language's + alias spellings — and, for English, the suffix-less name. With auto-detect + the language isn't known yet, so any sibling transcript counts as done. """ - suffixes = suffix_map or DEFAULT_SUFFIXES directory = export_dir if export_dir is not None else source.parent if language: path = output_path( @@ -76,14 +157,15 @@ def existing_transcript( candidate = directory / f"{source.stem}{suffix}.{fmt}" if candidate.exists(): return candidate + if language == UNSUFFIXED_LANGUAGE: + bare = directory / f"{source.stem}.{fmt}" + if bare.exists(): + return bare return None - detect = list(suffixes.values()) - detect += [a for spec in known_languages() for a in spec.aliases] - for suffix in detect: - candidate = directory / f"{source.stem}{suffix}.{fmt}" - if candidate.exists(): - return candidate - return None + found = sibling_transcripts( + source, fmt=fmt, suffix_map=suffix_map, export_dir=export_dir + ) + return next(iter(found.values()), None) # --------------------------------------------------------------------------- diff --git a/src/scripto/core/pipeline.py b/src/scripto/core/pipeline.py index 62e9996..39c4fca 100644 --- a/src/scripto/core/pipeline.py +++ b/src/scripto/core/pipeline.py @@ -238,6 +238,10 @@ def _transcribe_loop( job.outputs.append(existing) stats.skipped += 1 self._emit_status(job) + # Skipping still belongs in history: the file has subtitles, + # and the user wants to reach them (view, play, translate) + # from there like any other result. + self._record(job) self._queue_translation(job, existing, translate_q, deferred, translate_now, stop) continue @@ -431,19 +435,45 @@ def _record_translation(self, job: Job, produced: list[Path]) -> None: except Exception: logger.exception("could not write translation history entry") + def _output_rows(self, job: Job) -> list[dict[str, str]]: + """Every transcript this source has now, with its language named. + + The job's own products first — their language is the detected one, + which beats guessing from a filename — then any same-stem siblings + already on disk: subtitles that shipped with the video, or that an + earlier run produced in another language. A source appears in + history with all of its languages, not just the one this run touched. + """ + rows: dict[str, dict[str, str]] = {} + + def row(path: Path, lang: str) -> dict[str, str]: + return { + "lang": lang, + "format": path.suffix.lstrip("."), + "path": str(path), + } + + for path in job.outputs: + rows[str(path)] = row(path, job.language or "") + if job.status in (JobStatus.DONE, JobStatus.SKIPPED): + siblings = out.sibling_transcripts( + job.source, fmt=self._s.fmt, + suffix_map=self._s.suffix_map, export_dir=self._s.export_dir, + ) + for language, path in siblings.items(): + known = rows.get(str(path)) + if known is None: + rows[str(path)] = row(path, language) + elif not known["lang"]: + known["lang"] = language # skipped job: the name is all we have + return list(rows.values()) + def _record(self, job: Job, duration: float = 0.0) -> None: try: self._history.append( HistoryEntry( source=str(job.source), - outputs=[ - { - "lang": job.language or "", - "format": path.suffix.lstrip("."), - "path": str(path), - } - for path in job.outputs - ], + outputs=self._output_rows(job), model=self._s.model.key, engine=self._s.engine_label, status=job.status.value if job.status != JobStatus.PENDING else "failed", diff --git a/tests/test_output.py b/tests/test_output.py index 6834292..553323e 100644 --- a/tests/test_output.py +++ b/tests/test_output.py @@ -54,6 +54,52 @@ def test_existing_transcript_auto_checks_known_suffixes(tmp_path): assert out.existing_transcript(src, fmt="srt", language=None) is not None +def test_language_from_suffix_reads_names_the_way_we_write_them(): + assert out.language_from_suffix(".zh") == "zh" + assert out.language_from_suffix(".cn") == "zh" # alias spelling + assert out.language_from_suffix(".ENG") == "en" # case-insensitive + assert out.language_from_suffix("") == "en" # no suffix: English + assert out.language_from_suffix(".fr") == "fr" # unregistered, well-formed + assert out.language_from_suffix(".pt-br") == "pt-br" + assert out.language_from_suffix(".part2") is None # a filename, not a language + assert out.language_from_suffix(".final") is None + + +def test_sibling_transcripts_maps_every_language_beside_the_video(tmp_path): + src = tmp_path / "lecture.mp4" + src.write_bytes(b"x") + for name in ("lecture.srt", "lecture.zh.srt", "lecture.ja.srt", + "lecture.part2.srt", "lecture-draft.srt", "other.zh.srt"): + (tmp_path / name).write_text("x", encoding="utf-8") + + found = out.sibling_transcripts(src, fmt="srt") + assert {k: v.name for k, v in found.items()} == { + "en": "lecture.srt", # no suffix counts as English + "zh": "lecture.zh.srt", + "ja": "lecture.ja.srt", + } + # Registry order, so callers get a stable first choice. + assert list(found) == ["en", "zh", "ja"] + + +def test_sibling_transcripts_ignores_other_formats_and_missing_dirs(tmp_path): + src = tmp_path / "lecture.mp4" + (tmp_path / "lecture.zh.srt").write_text("x", encoding="utf-8") + (tmp_path / "lecture.en.txt").write_text("x", encoding="utf-8") + assert list(out.sibling_transcripts(src, fmt="srt")) == ["zh"] + assert list(out.sibling_transcripts(src, fmt="txt")) == ["en"] + assert out.sibling_transcripts(tmp_path / "gone" / "x.mp4", fmt="srt") == {} + + +def test_existing_transcript_accepts_a_suffixless_subtitle(tmp_path): + src = tmp_path / "lecture.mp4" + (tmp_path / "lecture.srt").write_text("x", encoding="utf-8") + # The shape subtitles arrive in from elsewhere: no language in the name. + assert out.existing_transcript(src, fmt="srt", language=None) is not None + assert out.existing_transcript(src, fmt="srt", language="en") is not None + assert out.existing_transcript(src, fmt="srt", language="zh") is None + + def test_srt_writer_structure(tmp_path): target = tmp_path / "o.srt" out.write_result(_result(), target, "srt") diff --git a/tests/test_pipeline.py b/tests/test_pipeline.py index 30099a9..435b301 100644 --- a/tests/test_pipeline.py +++ b/tests/test_pipeline.py @@ -7,6 +7,7 @@ import threading import time +from pathlib import Path import pytest @@ -143,6 +144,52 @@ def test_skip_existing_without_overwrite(tmp_path, fake_extract): assert (files[0].parent / (files[0].stem + ".en.srt")).read_text(encoding="utf-8") == "old" +def test_skipped_file_lands_in_history_with_all_its_languages(tmp_path, fake_extract): + """A video that arrives already subtitled is a history row, not a no-op.""" + (video,) = make_media(tmp_path, 1) + (tmp_path / f"{video.stem}.srt").write_text("en", encoding="utf-8") + (tmp_path / f"{video.stem}.zh.srt").write_text("zh", encoding="utf-8") + (tmp_path / f"{video.stem}.ja.srt").write_text("ja", encoding="utf-8") + + pipe, _bus = make_pipeline(tmp_path) + jobs, stats = pipe.run([video], threading.Event()) + assert stats.skipped == 1 and jobs[0].status == JobStatus.SKIPPED + + entries = HistoryStore(tmp_path / "history.json").entries() + assert len(entries) == 1 + entry = entries[0] + assert entry.status == "skipped" + assert {o["lang"]: Path(o["path"]).name for o in entry.outputs} == { + "en": f"{video.stem}.srt", # suffix-less file read as English + "zh": f"{video.stem}.zh.srt", + "ja": f"{video.stem}.ja.srt", + } + + +def test_transcribed_file_reports_subtitles_it_arrived_with(tmp_path, fake_extract): + (video,) = make_media(tmp_path, 1) + (tmp_path / f"{video.stem}.zh.srt").write_text("zh", encoding="utf-8") + + # Forced English: auto-detect would treat the Chinese file as "done". + pipe, _bus = make_pipeline(tmp_path, language="en") + _jobs, stats = pipe.run([video], threading.Event()) + assert stats.done == 1 + + (entry,) = HistoryStore(tmp_path / "history.json").entries() + langs = {o["lang"] for o in entry.outputs} + assert langs == {"en", "zh"} # the one produced plus the one found + + +def test_failed_job_does_not_claim_unrelated_subtitles(tmp_path, fake_extract): + (video,) = make_media(tmp_path, 1) + (tmp_path / f"{video.stem}.zh.srt").write_text("zh", encoding="utf-8") + pipe, _bus = make_pipeline(tmp_path, StubEngine(fail_on={video.stem}), language="en") + pipe.run([video], threading.Event()) + + (entry,) = HistoryStore(tmp_path / "history.json").entries() + assert entry.status == "failed" and entry.outputs == [] + + def test_overwrite_regenerates(tmp_path, fake_extract): files = make_media(tmp_path, 1) target = files[0].parent / (files[0].stem + ".en.srt")