From 105dd5be88f63ecc8f1896ee31c7b3b612f90a4a Mon Sep 17 00:00:00 2001 From: ThePendulum Date: Sun, 26 Jul 2026 02:19:54 +0200 Subject: [PATCH] LosslessCut-style layout: File menu, timeline above transport, centered controls, right-click play/pause - Load File moved from a button into a new File menu. - Timeline now sits directly under the video; the transport row (frame-step/play/frame-step, centered as a group; mute pinned right) moved below it, matching LosslessCut's layout instead of the previous buttons-then-timeline order. - Right-click on the video toggles play/pause. The right-click implementation took two tries, worth recording: mpv embeds via wid as its own native child window, which receives raw X11 input directly -- a real mouse click never reaches this QWidget's mousePressEvent at all, even though a unit-style test that called the handler directly passed and looked fine. Caught this by testing with an actual xdotool click against a live running window instead of trusting the synthetic test, which is what this project's smoke tests have been doing throughout and what caught the bug here too. Fixed by binding MBTN_RIGHT through mpv's own on_key_press input API instead, which does receive the real clicks -- verified the same way, with a real click on a live window, position/button-text actually changing. Also fixed a genuinely flaky smoke test in the process: QShortcut's active-window check races with real desktop window-manager focus, especially with another instance of this same app already open elsewhere (happened during this session). Switched that test to set Qt's active-window state directly rather than relying on WM-level activation, and independently confirmed the real key bindings with live xdotool keypresses (both directions, plus end-of-file clamping). --- main.py | 58 ++++++++++++-------- player_widget.py | 10 ++++ scripts/smoke_test_arrow_seek.py | 17 ++++++ scripts/smoke_test_rightclick_filemenu.py | 67 +++++++++++++++++++++++ 4 files changed, 128 insertions(+), 24 deletions(-) create mode 100644 scripts/smoke_test_rightclick_filemenu.py diff --git a/main.py b/main.py index 5da02ce..df88370 100644 --- a/main.py +++ b/main.py @@ -97,6 +97,7 @@ class MainWindow(QMainWindow): self.player = PlayerWidget() self.player.setMinimumHeight(400) self.player.fileDropped.connect(self._load) + self.player.rightClicked.connect(self._toggle_play) self._build_ui() self._build_menu() @@ -115,30 +116,8 @@ class MainWindow(QMainWindow): layout.addWidget(self.player, stretch=1) - # Transport row: play/pause, frame step, seek slider, position label - transport = QHBoxLayout() - self.load_button = QPushButton("Load File…") - self.load_button.clicked.connect(self._on_load_clicked) - transport.addWidget(self.load_button) - - self.play_button = QPushButton("Play") - self.play_button.clicked.connect(self._toggle_play) - transport.addWidget(self.play_button) - - self.prev_frame_button = QPushButton("⏮ Frame (,)") - self.prev_frame_button.clicked.connect(lambda: self._step_frame(-1)) - transport.addWidget(self.prev_frame_button) - - self.next_frame_button = QPushButton("Frame ⏭ (.)") - self.next_frame_button.clicked.connect(lambda: self._step_frame(1)) - transport.addWidget(self.next_frame_button) - - self.mute_checkbox = QCheckBox("Mute audio") - self.mute_checkbox.toggled.connect(self._on_mute_toggled) - transport.addWidget(self.mute_checkbox) - - layout.addLayout(transport) - + # Timeline directly under the video (LosslessCut-style), transport + # controls below that. seek_row = QHBoxLayout() self.position_label = QLabel("00:00.000") seek_row.addWidget(self.position_label) @@ -161,6 +140,31 @@ class MainWindow(QMainWindow): seek_row.addWidget(self.duration_label) layout.addLayout(seek_row) + # Transport row: frame-step/play controls centered as a group, mute + # pinned to the right. Load File now lives in the File menu. + transport = QHBoxLayout() + transport.addStretch(1) + + self.prev_frame_button = QPushButton("⏮ Frame (,)") + self.prev_frame_button.clicked.connect(lambda: self._step_frame(-1)) + transport.addWidget(self.prev_frame_button) + + self.play_button = QPushButton("Play") + self.play_button.clicked.connect(self._toggle_play) + transport.addWidget(self.play_button) + + self.next_frame_button = QPushButton("Frame ⏭ (.)") + self.next_frame_button.clicked.connect(lambda: self._step_frame(1)) + transport.addWidget(self.next_frame_button) + + transport.addStretch(1) + + self.mute_checkbox = QCheckBox("Mute audio") + self.mute_checkbox.toggled.connect(self._on_mute_toggled) + transport.addWidget(self.mute_checkbox) + + layout.addLayout(transport) + # Marker row marker_group = QGroupBox("Sync markers") marker_layout = QHBoxLayout(marker_group) @@ -210,6 +214,12 @@ class MainWindow(QMainWindow): self.setCentralWidget(central) def _build_menu(self): + file_menu = self.menuBar().addMenu("&File") + + load_action = QAction("Load File…", self) + load_action.triggered.connect(self._on_load_clicked) + file_menu.addAction(load_action) + view_menu = self.menuBar().addMenu("&View") self.toggle_log_action = QAction("Show ffmpeg Log", self, checkable=True, checked=False) diff --git a/player_widget.py b/player_widget.py index f2fb61c..a146d99 100644 --- a/player_widget.py +++ b/player_widget.py @@ -21,12 +21,14 @@ class PlayerWidget(QWidget): """ fileDropped = pyqtSignal(str) + rightClicked = pyqtSignal() def __init__(self, parent=None): super().__init__(parent) self.setAttribute(Qt.WidgetAttribute.WA_DontCreateNativeAncestors) self.setAttribute(Qt.WidgetAttribute.WA_NativeWindow) self.setAcceptDrops(True) + self.setContextMenuPolicy(Qt.ContextMenuPolicy.NoContextMenu) # QApplication resets LC_NUMERIC to the system locale on construction, # which undoes python-mpv's own import-time fix and makes libmpv abort @@ -44,6 +46,14 @@ class PlayerWidget(QWidget): ) self._fps = None + # mpv owns its own native child window for rendering (that's how + # wid-embedding works), which receives raw X11 input directly -- + # confirmed by testing that a real mouse click never reaches this + # QWidget's mousePressEvent at all, even though it looked fine when + # the handler was called directly in a unit-style test. Mouse + # buttons have to be bound through mpv's own input system instead. + self.mpv.on_key_press("MBTN_RIGHT")(lambda: self.rightClicked.emit()) + def load(self, path: str): # NOTE: we start the MPV instance with pause=True, so `core-idle` # never flips to False and mpv.wait_until_playing() would block diff --git a/scripts/smoke_test_arrow_seek.py b/scripts/smoke_test_arrow_seek.py index a23eb5c..0e8a25b 100644 --- a/scripts/smoke_test_arrow_seek.py +++ b/scripts/smoke_test_arrow_seek.py @@ -2,6 +2,15 @@ that loading a file shows a status bar message. Drives the real QShortcut-bound keys via QTest.keyClick so it's testing the actual key binding, not calling _seek_relative directly. + +NOTE: QShortcut's default context fires based on Qt's notion of the active +window, which can race with real desktop window-manager focus -- this was +observed to be flaky specifically when another real instance of the app is +already open elsewhere on the same desktop (competing for focus). The +behavior itself was independently confirmed correct via real xdotool +keypresses against a live window (both directions, plus end-of-file +clamping) -- if this script flakes, that's the harness racing with desktop +focus, not evidence the feature is broken. """ import sys import time @@ -22,6 +31,14 @@ def main(): qapp = QApplication(sys.argv) win = app_main.MainWindow() win.show() + # QShortcut's default WindowShortcut context fires based on Qt's own + # notion of the active window (QApplication.activeWindow()), not raw X11 + # input focus -- window-manager-level activation (raise_/activateWindow) + # turned out to be a race in this environment (especially with another + # real instance of the app already open elsewhere), flipping back to + # inactive moments after being granted. Setting it directly through Qt + # sidesteps that WM race for test purposes. + qapp.setActiveWindow(win) qapp.processEvents() win._load(CLIP) diff --git a/scripts/smoke_test_rightclick_filemenu.py b/scripts/smoke_test_rightclick_filemenu.py new file mode 100644 index 0000000..ae352ae --- /dev/null +++ b/scripts/smoke_test_rightclick_filemenu.py @@ -0,0 +1,67 @@ +"""Verifies right-click-on-player toggles play/pause, and that Load File +moved from a button into a File menu action. + +mpv owns its own native child window for the embedded video surface, so a +real mouse click there never reaches Qt's mousePressEvent at all -- the +right-click binding is registered through mpv's own input system instead +(see player_widget.py). To exercise that actual code path headlessly, this +drives mpv's own `keypress` command (which simulates input the same way a +real click would, through mpv's own dispatch) rather than posting a +QMouseEvent, since a QMouseEvent would only prove Qt-level dead code works, +not the real behavior -- that mistake produced a false pass earlier in this +session, worth not repeating. +""" +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_INSYNC.mp4") + + +def main(): + qapp = QApplication(sys.argv) + win = app_main.MainWindow() + win.show() + qapp.processEvents() + + win._load(CLIP) + qapp.processEvents() + time.sleep(0.2) + qapp.processEvents() + + # -- File menu has Load File, no more load_button in the layout + menu_titles = [a.text() for a in win.menuBar().actions()] + assert "&File" in menu_titles, f"expected a File menu, got {menu_titles}" + assert not hasattr(win, "load_button"), "old Load File button should be gone" + print("PASS: Load File lives in the File menu") + + # -- Right-click on the player toggles play/pause + assert win.player.mpv.pause is True, "should start paused" + win.player.mpv.command("keypress", "MBTN_RIGHT") + qapp.processEvents() + time.sleep(0.2) + qapp.processEvents() + assert win.player.mpv.pause is False, "right-click should have started playback" + assert win.play_button.text() == "Pause" + print("PASS: right-click on player started playback") + + win.player.mpv.command("keypress", "MBTN_RIGHT") + qapp.processEvents() + time.sleep(0.2) + qapp.processEvents() + assert win.player.mpv.pause is True, "second right-click should pause again" + assert win.play_button.text() == "Play" + print("PASS: right-click on player paused again") + + win.player.shutdown() + print("\nRIGHT-CLICK / FILE MENU SMOKE TEST PASSED") + + +if __name__ == "__main__": + main()