Replace preview popup with Kdenlive-style Source/Preview tabs

Removes PreviewDialog; the video area is now a QTabWidget with two
tabs, each backed by its own independent, long-lived PlayerWidget/mpv
instance:
- Source: the main player (unchanged transport/marking controls below
  it still apply here).
- Preview: a second player with its own minimal transport (play/pause
  + position label), auto-plays and switches to this tab as soon as a
  preview clip finishes building.

The preview player persists for the whole session instead of being
created per-preview -- building a new preview loads the new clip into
the same mpv instance and only deletes the old temp file afterward
(unsafe to unlink while mpv still has it open). Loading a new source
file drops any existing preview, since it belonged to the previous
file's offset.

Found and fixed a real crash bug while testing the close-window path
(not something the earlier popup-based tests exercised): closeEvent
shut down both mpv instances but left the 100ms position-update QTimer
running, so a tick landing between shutdown and process exit called
get_position() on an already-terminated core and hard-crashed the
process (native abort, not a catchable Python exception). Fixed by
stopping the timer first in closeEvent.

Verified on the virtual Xvfb display: automated test covers the tab
switch, independent playback state between tabs, and temp-file
lifecycle across rebuilding a preview and closing the window; also
driven live with real xdotool clicks (mark both events, build preview,
confirm auto-switch to the Preview tab with the clip already playing).

main.py

@@ -9,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, QDialog,
+    QMessageBox, QPlainTextEdit, QGroupBox, QTabWidget,
 )
 
 from player_widget import PlayerWidget
@@ -87,51 +87,6 @@ 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__()
@@ -145,7 +100,8 @@ class MainWindow(QMainWindow):
         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._preview_temp_path: Optional[str] = None  # currently loaded in preview_player
+        self._pending_preview_path: Optional[str] = None  # being built by _preview_worker
         self._slider_being_dragged = False
 
         self.player = PlayerWidget()
@@ -153,6 +109,14 @@ class MainWindow(QMainWindow):
         self.player.fileDropped.connect(self._load)
         self.player.rightClicked.connect(self._toggle_play)
 
+        # Independent second mpv instance for the Preview tab (Kdenlive-style
+        # project/clip monitor tabs) -- kept alive for the whole session
+        # rather than created per-preview, so switching tabs back and forth
+        # doesn't tear down and rebuild an mpv instance each time.
+        self.preview_player = PlayerWidget()
+        self.preview_player.setMinimumHeight(320)
+        self.preview_player.rightClicked.connect(self._toggle_preview_play)
+
         self._build_ui()
         self._build_menu()
         self._wire_shortcuts()
@@ -168,7 +132,26 @@ class MainWindow(QMainWindow):
         central = QWidget()
         layout = QVBoxLayout(central)
 
-        layout.addWidget(self.player, stretch=1)
+        self.video_tabs = QTabWidget()
+        self.video_tabs.addTab(self.player, "Source")
+
+        preview_tab = QWidget()
+        preview_tab_layout = QVBoxLayout(preview_tab)
+        preview_tab_layout.setContentsMargins(0, 0, 0, 0)
+        preview_tab_layout.addWidget(self.preview_player, stretch=1)
+
+        preview_controls = QHBoxLayout()
+        self.preview_play_button = QPushButton("Play")
+        self.preview_play_button.clicked.connect(self._toggle_preview_play)
+        preview_controls.addWidget(self.preview_play_button)
+        self.preview_position_label = QLabel("00:00.000")
+        preview_controls.addWidget(self.preview_position_label)
+        preview_controls.addStretch(1)
+        preview_tab_layout.addLayout(preview_controls)
+
+        self.video_tabs.addTab(preview_tab, "Preview")
+
+        layout.addWidget(self.video_tabs, stretch=1)
 
         # Timeline directly under the video (LosslessCut-style), transport
         # controls below that.
@@ -353,6 +336,12 @@ class MainWindow(QMainWindow):
         self.detect_all_button.setEnabled(True)
         self.detect_next_action.setEnabled(True)
 
+        # A loaded preview belongs to the *previous* source/offset -- drop it.
+        if self._preview_temp_path and os.path.exists(self._preview_temp_path):
+            os.unlink(self._preview_temp_path)
+        self._preview_temp_path = None
+        self.video_tabs.setCurrentIndex(0)
+
     def _toggle_play(self):
         if self.player.mpv.pause:
             self.player.play()
@@ -370,6 +359,14 @@ class MainWindow(QMainWindow):
     def _on_mute_toggled(self, checked: bool):
         self.player.mpv.mute = checked
 
+    def _toggle_preview_play(self):
+        if self.preview_player.mpv.pause:
+            self.preview_player.play()
+            self.preview_play_button.setText("Pause")
+        else:
+            self.preview_player.pause()
+            self.preview_play_button.setText("Play")
+
     def _seek_relative(self, delta_seconds: float):
         # Unlike frame-stepping (which always pauses -- you're precisely
         # inspecting a frame), a 5s skip is a coarse nudge while scrubbing
@@ -392,14 +389,16 @@ class MainWindow(QMainWindow):
         self._update_position_ui()
 
     def _update_position_ui(self):
-        if self.input_path is None:
+        if self.input_path is not None:
-            return
+            pos = self.player.get_position()
-        pos = self.player.get_position()
+            self.position_label.setText(format_timestamp(pos))
-        self.position_label.setText(format_timestamp(pos))
+            if not self._slider_being_dragged:
-        if not self._slider_being_dragged:
+                self.seek_slider.blockSignals(True)
-            self.seek_slider.blockSignals(True)
+                self.seek_slider.setValue(int(pos * 1000))
-            self.seek_slider.setValue(int(pos * 1000))
+                self.seek_slider.blockSignals(False)
-            self.seek_slider.blockSignals(False)
+
+        if self._preview_temp_path is not None:
+            self.preview_position_label.setText(format_timestamp(self.preview_player.get_position()))
 
     # -- markers / offset -----------------------------------------------
 
@@ -446,7 +445,7 @@ class MainWindow(QMainWindow):
         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._pending_preview_path = temp_path
 
         self.preview_button.setEnabled(False)
         self.preview_button.setText("Building preview…")
@@ -467,21 +466,30 @@ class MainWindow(QMainWindow):
         if result.stderr.strip():
             self.log_view.appendPlainText("ffmpeg preview log:\n" + result.stderr)
 
-        dialog = PreviewDialog(self._preview_temp_path, parent=self)
+        # The preview player is long-lived (one mpv instance for the whole
-        dialog.exec()
+        # session), so swap files instead of spinning up a new player each
+        # time: load the new clip first, then delete the old temp file --
+        # only safe to unlink it once mpv is no longer holding it open.
+        old_path = self._preview_temp_path
+        new_path = self._pending_preview_path
+        self.preview_player.load(new_path)
+        self.preview_player.play()
+        self.preview_play_button.setText("Pause")
+        self._preview_temp_path = new_path
+        self._pending_preview_path = None
+        self.video_tabs.setCurrentIndex(1)
 
-        if self._preview_temp_path and os.path.exists(self._preview_temp_path):
+        if old_path and old_path != new_path and os.path.exists(old_path):
-            os.unlink(self._preview_temp_path)
+            os.unlink(old_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):
+        if self._pending_preview_path and os.path.exists(self._pending_preview_path):
-            os.unlink(self._preview_temp_path)
+            os.unlink(self._pending_preview_path)
-        self._preview_temp_path = None
+        self._pending_preview_path = None
 
     # -- clap detection -----------------------------------------------------
 
@@ -574,7 +582,15 @@ class MainWindow(QMainWindow):
         QMessageBox.critical(self, "Export failed", f"{message}\n\n{stderr[-2000:]}")
 
     def closeEvent(self, event):
+        # Stop the timer before tearing down mpv -- otherwise a tick that
+        # fires between shutdown() and the process actually exiting calls
+        # get_position() on an already-terminated core and crashes
+        # (mpv.ShutdownError, observed as a hard crash in testing).
+        self._position_timer.stop()
         self.player.shutdown()
+        self.preview_player.shutdown()
+        if self._preview_temp_path and os.path.exists(self._preview_temp_path):
+            os.unlink(self._preview_temp_path)
         super().closeEvent(event)
 
 

scripts/smoke_test_preview.py

@@ -1,12 +1,8 @@
 """Verifies the sync preview feature: building a short offset-corrected
-clip and playing it in a popup PreviewDialog, plus the Sync markers panel
+clip and playing it in the Preview tab (Kdenlive-style project/clip
-now showing Mark Audio Event before Mark Video Event with a Detect All
+monitor tabs) instead of a popup dialog, plus the Sync markers panel
-Claps button alongside them.
+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
@@ -23,13 +19,12 @@ 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 panel order: Mark Audio Event before Mark Video Event, with
+    # Detect All Claps in the same panel.
     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]
@@ -37,32 +32,20 @@ def main():
     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
+    # -- Two tabs: Source (main player) and Preview
-    # preview clip, and confirm it plays back with the events much closer
+    assert win.video_tabs.count() == 2
-    # together than the original 350ms.
+    assert win.video_tabs.tabText(0) == "Source"
-    captured = {}
+    assert win.video_tabs.tabText(1) == "Preview"
-
+    assert win.video_tabs.widget(0) is win.player
-    def fake_exec(self):
+    print("PASS: video area has Source and Preview tabs")
-        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()
+    assert win.video_tabs.currentIndex() == 0, "loading a new source should land on the Source tab"
 
     win.player.seek(5.000)
     qapp.processEvents()
@@ -73,40 +56,64 @@ def main():
     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
+    pending_path = win._pending_preview_path
-    print(f"building preview clip at: {temp_path_ref}")
+    print(f"building preview clip at: {pending_path}")
 
     deadline = time.time() + 20
-    while "dialog" not in captured and time.time() < deadline:
+    while win.video_tabs.currentIndex() != 1 and time.time() < deadline:
         qapp.processEvents()
         time.sleep(0.05)
 
-    assert "dialog" in captured, "preview dialog was never shown"
+    assert win.video_tabs.currentIndex() == 1, "finishing a preview should switch to the Preview tab"
-    dialog = captured["dialog"]
+    assert win.preview_button.isEnabled()
+    assert win._preview_temp_path == pending_path
+    assert Path(win._preview_temp_path).exists()
+    print("PASS: preview build switched to the Preview tab automatically")
+
     qapp.processEvents()
     time.sleep(0.3)
     qapp.processEvents()
 
-    duration = dialog.player.mpv.duration
+    duration = win.preview_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 win.preview_player.mpv.pause is False, "preview should auto-play"
+    assert win.preview_play_button.text() == "Pause"
 
-    assert captured["existed_while_open"], "temp preview file should have existed while the dialog was showing"
+    # -- Switching back to Source shouldn't disturb the main player, and the
-
+    # preview player keeps running independently (two live mpv instances).
-    dialog.close()
+    main_pos_before_switch = win.player.get_position()
+    win.video_tabs.setCurrentIndex(0)
     qapp.processEvents()
+    main_pos_after_switch = win.player.get_position()
+    print(f"main player position: before switch={main_pos_before_switch:.3f}s after={main_pos_after_switch:.3f}s")
+    assert abs(main_pos_after_switch - main_pos_before_switch) < 0.1, "switching tabs shouldn't move the main player"
+    assert win.player.mpv.pause is True, "main player should still be paused (untouched by preview building)"
+    print("PASS: switching tabs doesn't disturb the main player")
+
+    # -- Building a second preview swaps the file without leaking the first
+    # temp file.
+    first_preview_path = win._preview_temp_path
+    win.preview_button.click()
+    qapp.processEvents()
+    deadline = time.time() + 20
+    while win._pending_preview_path is not None and time.time() < deadline:
+        qapp.processEvents()
+        time.sleep(0.05)
     time.sleep(0.2)
     qapp.processEvents()
+    assert not Path(first_preview_path).exists(), "first preview temp file should be cleaned up after being replaced"
+    assert Path(win._preview_temp_path).exists()
+    print("PASS: rebuilding the preview cleans up the previous temp file")
 
-    assert not Path(temp_path_ref).exists(), "temp preview file should be cleaned up after the (real) exec() returns"
+    win.close()
-    print("PASS: preview clip built, played, and cleaned up correctly")
+    qapp.processEvents()
+    assert not Path(win._preview_temp_path).exists() if win._preview_temp_path else True
 
-    win.player.shutdown()
+    print("\nPREVIEW TAB SMOKE TEST PASSED")
-    print("\nPREVIEW SMOKE TEST PASSED")
 
 
 if __name__ == "__main__":