From eb7c1aa50022bdf5b9481ccb957ec2d246147a32 Mon Sep 17 00:00:00 2001 From: ThePendulum Date: Sun, 26 Jul 2026 03:01:06 +0200 Subject: [PATCH] Add sync preview, restore Detect All Claps button, reorder markers panel - Preview Sync button builds a short (8s) lossless clip around the marked events with the current offset applied, and plays it in a small popup PreviewDialog (its own independent PlayerWidget/mpv instance) -- lets you check the fix sounds/looks right before committing to a full export. ExportWorker gained optional clip_start/clip_duration so both the real export and this preview share the same underlying export_synced() call. - Detect All Claps is back as a button in the Sync markers panel (alongside Mark Audio/Video Event), in addition to staying in the Tools menu -- both widgets' enabled state are kept in sync. - Sync markers panel now shows Mark Audio Event before Mark Video Event. The clip-mode branch of ffmpeg_export.py (the -ss/-t-on-both-inputs path ffdelay's own notes flag as easy to get subtly wrong) hadn't been covered by the existing roundtrip test, which only exercised full-file export -- added a clip-mode case there, independently verified via the same scene-change/silence-detection technique as the other cases: 29.4ms drift after the fix, well under one frame. All of this was built and verified entirely on a virtual Xvfb display (DISPLAY=:99) per the user's request to stop interrupting their real desktop -- real mouse clicks and a real (non-monkeypatched) preview dialog were exercised there, screenshotted, and confirmed correct, without ever touching the real screen. --- main.py | 154 ++++++++++++++++++++++++++--- scripts/smoke_test_interactions.py | 10 +- scripts/smoke_test_preview.py | 113 +++++++++++++++++++++ scripts/test_export_roundtrip.py | 18 +++- 4 files changed, 277 insertions(+), 18 deletions(-) create mode 100644 scripts/smoke_test_preview.py diff --git a/main.py b/main.py index 0da589b..54ba607 100644 --- a/main.py +++ b/main.py @@ -1,4 +1,6 @@ +import os import sys +import tempfile from pathlib import Path from typing import Optional @@ -7,7 +9,7 @@ from PyQt6.QtGui import QShortcut, QKeySequence, QAction from PyQt6.QtWidgets import ( QApplication, QMainWindow, QWidget, QVBoxLayout, QHBoxLayout, QPushButton, QLabel, QSlider, QCheckBox, QDoubleSpinBox, QFileDialog, - QMessageBox, QPlainTextEdit, QGroupBox, + QMessageBox, QPlainTextEdit, QGroupBox, QDialog, ) from player_widget import PlayerWidget @@ -44,15 +46,20 @@ class ExportWorker(QThread): finished_ok = pyqtSignal(object) # ExportResult finished_err = pyqtSignal(str, str) # message, stderr - def __init__(self, input_path, delay_ms, output_path): + def __init__(self, input_path, delay_ms, output_path, clip_start=None, clip_duration=None): super().__init__() self.input_path = input_path self.delay_ms = delay_ms self.output_path = output_path + self.clip_start = clip_start + self.clip_duration = clip_duration def run(self): try: - result: ExportResult = export_synced(self.input_path, self.delay_ms, self.output_path) + result: ExportResult = export_synced( + self.input_path, self.delay_ms, self.output_path, + clip_start=self.clip_start, clip_duration=self.clip_duration, + ) self.finished_ok.emit(result) except FfmpegExportError as e: self.finished_err.emit(str(e), e.stderr) @@ -80,6 +87,51 @@ class DetectWorker(QThread): self.finished_err.emit(str(e)) +class PreviewDialog(QDialog): + """Small popup player for a short, already-synced clip (see + MainWindow._on_preview_clicked) -- a second, independent PlayerWidget/mpv + instance, not a reuse of the main one, so the user can flip back and + forth between the main timeline and the preview without one tearing + down the other. + """ + + def __init__(self, path: str, parent=None): + super().__init__(parent) + self.setWindowTitle("Preview — sync-corrected clip") + self.resize(480, 420) + + self.player = PlayerWidget() + self.player.setMinimumHeight(320) + + layout = QVBoxLayout(self) + layout.addWidget(self.player, stretch=1) + + controls = QHBoxLayout() + self.play_button = QPushButton("Pause") + self.play_button.clicked.connect(self._toggle_play) + controls.addWidget(self.play_button) + controls.addStretch(1) + close_button = QPushButton("Close") + close_button.clicked.connect(self.close) + controls.addWidget(close_button) + layout.addLayout(controls) + + self.player.load(path) + self.player.play() + + def _toggle_play(self): + if self.player.mpv.pause: + self.player.play() + self.play_button.setText("Pause") + else: + self.player.pause() + self.play_button.setText("Play") + + def closeEvent(self, event): + self.player.shutdown() + super().closeEvent(event) + + class MainWindow(QMainWindow): def __init__(self): super().__init__() @@ -92,6 +144,8 @@ class MainWindow(QMainWindow): self.offset: Optional[SyncOffset] = None self._export_worker: Optional[ExportWorker] = None self._detect_worker: Optional[DetectWorker] = None + self._preview_worker: Optional[ExportWorker] = None + self._preview_temp_path: Optional[str] = None self._slider_being_dragged = False self.player = PlayerWidget() @@ -169,22 +223,29 @@ class MainWindow(QMainWindow): marker_group = QGroupBox("Sync markers") marker_layout = QHBoxLayout(marker_group) - self.mark_video_button = QPushButton("Mark Video Event") - self.mark_video_button.clicked.connect(self._mark_video) - marker_layout.addWidget(self.mark_video_button) - self.video_mark_label = QLabel("video: —") - marker_layout.addWidget(self.video_mark_label) - self.mark_audio_button = QPushButton("Mark Audio Event") self.mark_audio_button.clicked.connect(self._mark_audio) marker_layout.addWidget(self.mark_audio_button) self.audio_mark_label = QLabel("audio: —") marker_layout.addWidget(self.audio_mark_label) + self.mark_video_button = QPushButton("Mark Video Event") + self.mark_video_button.clicked.connect(self._mark_video) + marker_layout.addWidget(self.mark_video_button) + self.video_mark_label = QLabel("video: —") + marker_layout.addWidget(self.video_mark_label) + + marker_layout.addStretch(1) + + self.detect_all_button = QPushButton("Detect All Claps") + self.detect_all_button.setEnabled(False) + self.detect_all_button.clicked.connect(self._on_detect_all_clicked) + marker_layout.addWidget(self.detect_all_button) + layout.addWidget(marker_group) - # Offset + export row - export_group = QGroupBox("Offset and export") + # Offset + preview + export row + export_group = QGroupBox("Offset, preview and export") export_layout = QHBoxLayout(export_group) self.offset_label = QLabel("Offset: — (mark both events)") @@ -197,6 +258,11 @@ class MainWindow(QMainWindow): self.override_spinbox.setEnabled(False) export_layout.addWidget(self.override_spinbox) + self.preview_button = QPushButton("Preview Sync…") + self.preview_button.setEnabled(False) + self.preview_button.clicked.connect(self._on_preview_clicked) + export_layout.addWidget(self.preview_button) + self.export_button = QPushButton("Export…") self.export_button.setEnabled(False) self.export_button.clicked.connect(self._on_export_clicked) @@ -271,6 +337,7 @@ class MainWindow(QMainWindow): self.audio_mark_label.setText("audio: —") self.offset_label.setText("Offset: — (mark both events)") self.export_button.setEnabled(False) + self.preview_button.setEnabled(False) self.override_spinbox.setEnabled(False) self.log_view.clear() @@ -283,6 +350,7 @@ class MainWindow(QMainWindow): self.marker_strip.clear() self.statusBar().showMessage(f"Loaded {Path(path).name}", 4000) self.detect_all_action.setEnabled(True) + self.detect_all_button.setEnabled(True) self.detect_next_action.setEnabled(True) def _toggle_play(self): @@ -353,6 +421,67 @@ class MainWindow(QMainWindow): self.override_spinbox.setValue(self.offset.delay_ms) self.override_spinbox.setEnabled(True) self.export_button.setEnabled(True) + self.preview_button.setEnabled(True) + + # -- preview ----------------------------------------------------- + + PREVIEW_MARGIN_SEC = 3.0 + PREVIEW_DURATION_SEC = 8.0 + + def _on_preview_clicked(self): + if self.input_path is None or self.offset is None: + return + delay_ms = self.override_spinbox.value() + + # Build just a short clip around the marked events rather than the + # whole file -- lossless stream-copy of a few seconds is fast + # regardless of how long the source file is, unlike previewing the + # full result. Centering on the midpoint of the two original marks + # keeps both the video and audio events (and wherever the offset + # moves them to) inside the window for typical human-scale marking + # errors. + center = (self.video_mark + self.audio_mark) / 2.0 + clip_start = max(center - self.PREVIEW_MARGIN_SEC, 0.0) + + input_p = Path(self.input_path) + fd, temp_path = tempfile.mkstemp(suffix=input_p.suffix, prefix="avsync_preview_") + os.close(fd) + self._preview_temp_path = temp_path + + self.preview_button.setEnabled(False) + self.preview_button.setText("Building preview…") + self.statusBar().showMessage("Building preview clip…") + + self._preview_worker = ExportWorker( + self.input_path, delay_ms, temp_path, + clip_start=f"{clip_start:.3f}", clip_duration=self.PREVIEW_DURATION_SEC, + ) + self._preview_worker.finished_ok.connect(self._on_preview_ready) + self._preview_worker.finished_err.connect(self._on_preview_failed) + self._preview_worker.start() + + def _on_preview_ready(self, result: ExportResult): + self.preview_button.setEnabled(True) + self.preview_button.setText("Preview Sync…") + self.statusBar().showMessage("Preview ready.", 4000) + if result.stderr.strip(): + self.log_view.appendPlainText("ffmpeg preview log:\n" + result.stderr) + + dialog = PreviewDialog(self._preview_temp_path, parent=self) + dialog.exec() + + if self._preview_temp_path and os.path.exists(self._preview_temp_path): + os.unlink(self._preview_temp_path) + self._preview_temp_path = None + + def _on_preview_failed(self, message: str, stderr: str): + self.preview_button.setEnabled(True) + self.preview_button.setText("Preview Sync…") + self.statusBar().showMessage("Preview failed (see log).", 8000) + self.log_view.appendPlainText(f"Preview FAILED: {message}\n{stderr}") + if self._preview_temp_path and os.path.exists(self._preview_temp_path): + os.unlink(self._preview_temp_path) + self._preview_temp_path = None # -- clap detection ----------------------------------------------------- @@ -371,6 +500,7 @@ class MainWindow(QMainWindow): if self.input_path is None: return self.detect_all_action.setEnabled(False) + self.detect_all_button.setEnabled(False) self.detect_next_action.setEnabled(False) self.statusBar().showMessage(label) @@ -381,6 +511,7 @@ class MainWindow(QMainWindow): def _on_detect_done(self, events: list): self.detect_all_action.setEnabled(True) + self.detect_all_button.setEnabled(True) self.detect_next_action.setEnabled(True) existing = set(self.marker_strip.markers()) @@ -398,6 +529,7 @@ class MainWindow(QMainWindow): def _on_detect_failed(self, message: str): self.detect_all_action.setEnabled(True) + self.detect_all_button.setEnabled(True) self.detect_next_action.setEnabled(True) self.statusBar().showMessage("Detection failed (see log).", 8000) self.log_view.appendPlainText(f"Clap detection FAILED: {message}") diff --git a/scripts/smoke_test_interactions.py b/scripts/smoke_test_interactions.py index 3ceb655..ff9e56c 100644 --- a/scripts/smoke_test_interactions.py +++ b/scripts/smoke_test_interactions.py @@ -30,13 +30,17 @@ def main(): win.show() qapp.processEvents() - # -- Tools menu has the clap detection actions, no buttons in the layout + # -- Tools menu has the clap detection actions, AND Detect All Claps is + # also back as a button in the Sync markers panel (both should reflect + # the same enabled state, kept in sync explicitly since they're two + # separate widgets). menu_titles = [a.text() for a in win.menuBar().actions()] assert "&Tools" in menu_titles, f"expected a Tools menu, got {menu_titles}" - assert not hasattr(win, "detect_all_button"), "old button attribute should be gone" assert win.detect_all_action.text() == "Detect All Claps" assert win.detect_next_action.text() == "Detect Next Clap" - print("PASS: Tools menu has plain-labeled clap detection actions") + assert win.detect_all_button.text() == "Detect All Claps" + assert win.detect_all_action.isEnabled() == win.detect_all_button.isEnabled() + print("PASS: Tools menu has plain-labeled clap detection actions, button back in sync markers panel") # -- Load via drag-and-drop simulation on the player widget mime = QMimeData() diff --git a/scripts/smoke_test_preview.py b/scripts/smoke_test_preview.py new file mode 100644 index 0000000..e4bcccc --- /dev/null +++ b/scripts/smoke_test_preview.py @@ -0,0 +1,113 @@ +"""Verifies the sync preview feature: building a short offset-corrected +clip and playing it in a popup PreviewDialog, plus the Sync markers panel +now showing Mark Audio Event before Mark Video Event with a Detect All +Claps button alongside them. + +PreviewDialog.exec() is a real modal event loop, which would block this +script's own flow with no way to close it from inside. Monkeypatches it to +show() non-modally instead (capturing the instance) purely for test +purposes -- production code still calls the real modal .exec(). +""" +import sys +import time +from pathlib import Path + +from PyQt6.QtWidgets import QApplication + +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) +import main as app_main + +ROOT = Path(__file__).resolve().parent.parent +CLIP = str(ROOT / "test_media" / "sync_test_OUTOFSYNC_audio+350ms.mp4") + + +def main(): + qapp = QApplication(sys.argv) + + # -- Marker panel order: Mark Audio Event before Mark Video Event, with + # Detect All Claps in the same panel. + win = app_main.MainWindow() + win.show() + qapp.processEvents() + + marker_layout = win.mark_audio_button.parentWidget().layout() + widgets = [marker_layout.itemAt(i).widget() for i in range(marker_layout.count())] + widgets = [w for w in widgets if w is not None] + audio_idx = widgets.index(win.mark_audio_button) + video_idx = widgets.index(win.mark_video_button) + detect_idx = widgets.index(win.detect_all_button) + assert audio_idx < video_idx, "Mark Audio Event should come before Mark Video Event" + assert win.detect_all_button in widgets, "Detect All Claps button should be in the same panel" + print(f"PASS: marker panel order is audio({audio_idx}) < video({video_idx}), detect button present at {detect_idx}") + + # -- Preview: mark both events on the known-shifted fixture, build a + # preview clip, and confirm it plays back with the events much closer + # together than the original 350ms. + captured = {} + + def fake_exec(self): + captured["dialog"] = self + # This is the only point guaranteed to run before the real + # _on_preview_ready's cleanup (os.unlink) fires -- a real .exec() + # would block here until the dialog closes, but this fake returns + # immediately, so the app's own cleanup-after-exec() code runs + # right after this function returns, whether or not the dialog is + # still visually open. Check existence here, not after. + captured["existed_while_open"] = Path(win._preview_temp_path).exists() + self.show() + return 0 + + app_main.PreviewDialog.exec = fake_exec + + win._load(CLIP) + qapp.processEvents() + time.sleep(0.2) + qapp.processEvents() + + win.player.seek(5.000) + qapp.processEvents() + win._mark_video() + + win.player.seek(5.350) + qapp.processEvents() + win._mark_audio() + + assert win.preview_button.isEnabled() + + win.preview_button.click() + qapp.processEvents() + assert not win.preview_button.isEnabled(), "preview button should disable while building" + temp_path_ref = win._preview_temp_path + print(f"building preview clip at: {temp_path_ref}") + + deadline = time.time() + 20 + while "dialog" not in captured and time.time() < deadline: + qapp.processEvents() + time.sleep(0.05) + + assert "dialog" in captured, "preview dialog was never shown" + dialog = captured["dialog"] + qapp.processEvents() + time.sleep(0.3) + qapp.processEvents() + + duration = dialog.player.mpv.duration + print(f"preview clip duration: {duration:.3f}s (expected ~{win.PREVIEW_DURATION_SEC}s, clamped to file bounds)") + assert duration is not None and 0 < duration <= win.PREVIEW_DURATION_SEC + 0.5 + + assert captured["existed_while_open"], "temp preview file should have existed while the dialog was showing" + + dialog.close() + qapp.processEvents() + time.sleep(0.2) + qapp.processEvents() + + assert not Path(temp_path_ref).exists(), "temp preview file should be cleaned up after the (real) exec() returns" + print("PASS: preview clip built, played, and cleaned up correctly") + + win.player.shutdown() + print("\nPREVIEW SMOKE TEST PASSED") + + +if __name__ == "__main__": + main() diff --git a/scripts/test_export_roundtrip.py b/scripts/test_export_roundtrip.py index 82ee130..9dd0caa 100644 --- a/scripts/test_export_roundtrip.py +++ b/scripts/test_export_roundtrip.py @@ -54,11 +54,11 @@ def find_audio_event_time(path: Path) -> float: return times[0] -def run_case(fixture_name: str, expected_target: str, output_name: str): +def run_case(fixture_name: str, expected_target: str, output_name: str, clip_start=None, clip_duration=None): input_path = ROOT / "test_media" / fixture_name output_path = OUT_DIR / output_name - print(f"Input fixture: {fixture_name}") + print(f"Input fixture: {fixture_name}" + (f" (clip {clip_start}+{clip_duration}s)" if clip_start else "")) video_mark = find_video_event_time(input_path) audio_mark = find_audio_event_time(input_path) print(f" detected flash at t={video_mark:.3f}s, beep at t={audio_mark:.3f}s") @@ -67,7 +67,7 @@ def run_case(fixture_name: str, expected_target: str, output_name: str): print(f" {offset.describe()}") assert offset.target == expected_target, f"expected {expected_target} branch, got {offset.target}" - result = export_synced(str(input_path), offset.delay_ms, str(output_path)) + result = export_synced(str(input_path), offset.delay_ms, str(output_path), clip_start=clip_start, clip_duration=clip_duration) print(f" ran: {' '.join(result.command)}") after_video_t = find_video_event_time(output_path) @@ -84,7 +84,17 @@ def main(): OUT_DIR.mkdir(exist_ok=True) run_case("sync_test_OUTOFSYNC_audio+350ms.mp4", "video", "_export_roundtrip_negative.mp4") run_case("sync_test_OUTOFSYNC_video+350ms.mp4", "audio", "_export_roundtrip_positive.mp4") - print("PASS: both ffmpeg command branches (positive and negative delay) round-trip correctly.") + # Clip mode (used by the GUI's preview feature) is a genuinely different + # code path -- ffdelay's own documented quirk is that -ss/-t must be + # applied to BOTH inputs, in the right position relative to -itsoffset, + # or the clip covers different real-world windows per input and the + # "preview" would silently show wrong sync. Exercise it explicitly + # rather than assuming the full-file case generalizes. + run_case( + "sync_test_OUTOFSYNC_audio+350ms.mp4", "video", "_export_roundtrip_clip_mode.mp4", + clip_start="2.000", clip_duration=8, + ) + print("PASS: both ffmpeg command branches (positive and negative delay) round-trip correctly, including clip mode.") if __name__ == "__main__":