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.
This commit is contained in:
ThePendulum
@@ -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()
|
||||
|
||||
@@ -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()
|
||||
@@ -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__":
|
||||
|
||||
Reference in New Issue
Block a user