Repository navigation
Add bounding box overlay and frame/video export to jabs-viewer - #568
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Critical export-thread lifecycle issues can crash or corrupt the application during viewer closure or repeated exports, and the advertised macOS shortcut is not correctly registered.
3 open issues
What changed in this PR
Adds toggleable bounding-box overlays plus frame and video export capabilities to jabs-viewer, including pose-only exports rendered over blank frames. Shared rendering, export, dialog, documentation, and test infrastructure are extended while existing main-GUI and CLI defaults remain unchanged.
Changes:
- Adds v8 bounding-box visualization with toolbar/menu controls and identity tabs.
- Adds PNG frame export and configurable background video export.
- Supports pose-only exports using generated blank frames.
- Extends shared renderers, video writers, export threads, dialogs, documentation, and tests.
This pull request requires changes.
| File | Review summary |
|---|---|
tests/video_export/test_video_writer.py |
Tests bounding-box option propagation, blank-frame export, and source-overwrite protection. |
tests/video_export/test_frame_renderer.py |
Tests optional bounding-box rendering in exported frames. |
tests/ui/test_viewer_window.py |
Tests viewer controls, export actions, overlay availability, and pose-only sources. |
tests/ui/test_viewer_overlay_export.py |
Tests frame/video export workflows, worker wiring, and cancellation behavior. |
tests/ui/test_video_export_options_dialog.py |
Tests bounding-box options and configurable label/prediction controls. |
tests/overlay_drawing/test_bounding_box.py |
Tests bounding-box drawing, identity tabs, availability, and scaling. |
src/jabs/video_reader/blank_frame_reader.py |
Makes blank-frame readers usable as context managers. |
src/jabs/video_export/video_writer.py |
Supports optional bounding boxes and blank-frame video sources. |
src/jabs/video_export/frame_renderer.py |
Adds optional bounding-box rendering to exported frames. |
src/jabs/ui/viewer/viewer_window.py |
Wires viewer actions, overlay state, frame export, video export, and pose-only sources. The documented macOS ⌘E shortcut does not match the literal Ctrl+E binding (moderate, 2 votes). |
src/jabs/ui/viewer/overlay_export.py |
Implements frame and asynchronous video export. Export shutdown needs close-time cancellation and thread waiting (critical, 1 vote); concurrent exports must be prevented and thread ownership cleared safely (critical, 1 vote). Also improve unavailable-box wording (nit, 1 vote) and preserve worker tracebacks in error logs (nit, 1 vote). |
src/jabs/ui/video_export_thread.py |
Passes bounding-box and blank-frame options to background exports. |
src/jabs/ui/player_widget/player_widget.py |
Adds configurable overlay rendering and bounding-box toggling. |
src/jabs/ui/player_widget/frame_with_overlays.py |
Exposes the on-screen bounding-box overlay state. |
src/jabs/ui/dialogs/video_export_options_dialog.py |
Adds optional bounding-box selection and configurable label visibility. |
src/jabs/resources/docs/user_guide/viewer.md |
Updates the packaged viewer guide. Clarify that v8 pose files must also contain generated bounding boxes (nit, 1 vote). |
src/jabs/overlay_drawing/bounding_box.py |
Adds shared native-resolution bounding-box and identity-tab drawing utilities. |
src/jabs/overlay_drawing/__init__.py |
Exports the new bounding-box drawing API. |
docs/user-guide/viewer.md |
Updates the source viewer guide. Clarify that v8 pose files must also contain generated bounding boxes (nit, 1 vote). |
Review effort: Lite
| output_path, | ||
| pose_est, | ||
| options.draw_segmentation, | ||
| parent=self._window, |
There was a problem hiding this comment.
Fixed in 2c63eff: ViewerWindow.closeEvent now calls OverlayExporter.shutdown(), which requests termination of a running export and waits for the thread before the window is destroyed.
| self._video_export_thread = thread | ||
| self._run_video_export(thread, output_path) |
There was a problem hiding this comment.
Fixed in 2c63eff: export_video refuses to start while an export thread is still alive (it can outlive the progress dialog after Cancel), and release_thread only clears the field if it still holds the thread that finished.
| self._exporter = OverlayExporter(self, self._player, self.statusBar().showMessage) | ||
|
|
||
| self._export_frame_action = QtGui.QAction("Export &Frame...", self) | ||
| self._export_frame_action.setShortcut(QtGui.QKeySequence("Ctrl+E")) |
There was a problem hiding this comment.
No change needed: Qt maps Ctrl in a QKeySequence to the Command key on macOS (Qt::ControlModifier is Command unless AA_MacDontSwapCtrlAndMeta is set, which this app doesn't). QKeySequence("Ctrl+E") already binds and displays as ⌘E.
Summary
Adds three features to
jabs-viewer:B, also on the toolbar): toggles the bounding box overlay. On by default; enabled only for v8+ pose files that contain boxes. Previously the viewer always drew v8 boxes with no way to turn them off.-overlay.pngcopy drawn with the overlays currently enabled in the View menu. With a pose file open without a video, only the overlaid frame is saved.Changes to shared code
jabs.overlay_drawing.bounding_box(draw_identity_bounding_box,pose_has_bounding_boxes) for native-resolution box + identity tab drawing in exports. The on-screen overlay keeps its own palette-colored, clickable implementation.render_overlay_frame,export_overlay_video,VideoExportThreadandPlayerWidget.get_overlay_framegain adraw_bounding_boxesoption, default off, so the main GUI andjabs-cli export-videoare unchanged.export_overlay_video/VideoExportThreadacceptframes: BlankFrameReaderto export pose data without a video; the source path (the pose file) is still protected from being overwritten.BlankFrameReaderis now a context manager.VideoExportOptionsDialoggains an optional bounding box checkbox (offer_bounding_boxes) and can hide the label/prediction checkboxes (offer_labels). The main GUI does not offer bounding boxes yet.PlayerWidget.overlay_bounding_boxes()toggles the bounding box overlay.docs/user-guide/and in-app copies).Testing
OverlayExporter.