Feature: encoding, UI and performance improvements - #114
Open
C-Achard wants to merge 194 commits into
Open
Conversation
C-Achard
added
enhancement
C-Achard
force-pushed
the
cy/feature-perf-improvement
branch
from
August 20, 2026 10:17
73a61a4 to
c9264f3
Compare
deruyter92
reviewed
Aug 20, 2026
deruyter92
left a comment
deruyter92
left a comment
Contributor
There was a problem hiding this comment.
Great PR, with a lot of changes. I think they are definitely an improvement. great work!
I think good to release as soon as possible, which means we can use the improvements and further test it.
At the same time, as discussed, let's keep in mind that we need to thoroughly test this in practice as well. We need to keep alert about the saving logic; edge-cases that leave the gui unresponsive at the wrong moment; or any risk of data corruption. Together with the planned issues it might be worthwile to organize a test session in practice again. These planned refactors might help as a double review round to surface difficult-to-spot issues.
For now, I found 3 potential issues that I wanted to flag, but for the rest looks very good.
Comment on lines
2484
to
+2493
| def _stop_inference(self, show_message: bool = True) -> None: | ||
| if self._rec_manager.is_active: | ||
| answer = QMessageBox.question( | ||
| self, | ||
| "Stop inference while recording?", | ||
| "This will stop the currently running DLC-live custom processor, if any.\n" | ||
| "File saving will not be handled via the standard 'recording stopped' event hooks." | ||
| "The processor might still save data now, but it will not be paired with the recording.\n\n" | ||
| "Stop inference anyway?", | ||
| ) |
Contributor
There was a problem hiding this comment.
Want to flag here that this currently relies on the processor owning saving logic whenever stop() is invoked. This currenlty is not the case for the example dlc_processor (bare stop does not save).
Extract the RecorderStats dataclass from dlclivegui/services/video_recorder.py into dlclivegui/utils/stats.py and add the needed dataclasses import. Update imports to reflect the new location in dlclivegui/gui/recording_manager.py, dlclivegui/services/video_recorder.py, and tests/tests/gui/test_rec_manager.py. Also add imports for REC_DO_LOG_TIMING and WorkerTimingStats in video_recorder.py. No behavioral changes intended—this is a refactor to centralize recorder-related stats.
Basler backend: add preserve_mono support and configure the pypylon converter to output Mono8 when the camera source PixelFormat is Mono* and preserve_mono is enabled; fall back to BGR8 otherwise. Log the first decoded frame and expose per-backend timing flag BASLER_DO_LOG_TIMING. Update defaults for global timing flags and add REC_DO_LOG_TIMING. Camera settings: add preserve_mono flag and include it in __repr__. RecordingManager: pass convert_grayscale_to_rgb based on the camera preserve_mono setting. VideoRecorder: add convert_grayscale_to_rgb option, avoid unnecessary grayscale->RGB expansion when disabled, forward pixel-format/size hints to WriteGear, add WorkerTimingStats for recorder processing and writer, instrument preprocessing/queue/write steps, log the first frame, and improve frame-size mismatch handling and error reporting. These changes reduce memory/CPU overhead for mono cameras and add better timing/diagnostics for recording.
Introduce a preserve_mono capability and related properties to CameraBackend (actual_pixel_format, recommended_preserve_mono). Add a Preserve Mono checkbox to the camera config UI, persist/load its value, include it in probe detection logic (detect pixel format and apply recommended preserve_mono when supported), and treat changes to preserve_mono as restart-triggering. Update Basler static capabilities test to advertise preserve_mono support and add VideoRecorder tests to verify grayscale frames are preserved when requested and expanded by default. This enables preserving single-channel camera output to reduce bandwidth/overhead for monochrome cameras.
Expose the camera source pixel format via actual_pixel_format and add recommended_preserve_mono to suggest preserving mono images when the source format starts with "Mono". Add "preserve_mono" to reported capability levels. Implement _read_source_pixel_format to centralize reading the PixelFormat feature and call it from _configure_converter when needed so the backend always knows the source format before configuring conversion.
Rename internal source pixel format to _camera_pixel_format and centralize pixel-format handling. Add actual_pixel_format and actual_output_format properties, plus helpers (_read_camera_pixel_format, _is_camera_mono, _should_output_mono) to determine if the camera is mono and whether the backend should output mono frames. Update _configure_converter to use these helpers (emit Mono8 when preserve_mono + mono camera), remove the _output_is_mono flag and the old _read_source_pixel_format, and improve log messages. Also ensure the camera pixel format is read during startup after gain detection.
Add UI and backend support for reporting the camera backend's detected output format and pixel format. Introduce a detected output label with tooltip and move/rename the "preserve mono" checkbox into an Output row. Store/clear detected_output_format and detected_pixel_format in camera props, read actual_output_format from backends during probing, and set detected_output_format when pixel format indicates Mono. Add a mono indicator to camera list entries and update probe early-return logic to require both resolution and output format before skipping probing.
Expose actual_pixel_format and actual_output_format across backends and track the camera-reported formats for UI/telemetry. Aravis: add actual_pixel_format/actual_output_format properties and record _camera_pixel_format when setting pixel format. GenTL: initialize _camera_pixel_format/_actual_output_format, add _output_format_for_frame to infer output format from numpy frames, populate _actual_output_format on read, and record detected camera pixel format in several places. OpenCV: add actual_pixel_format (None) and actual_output_format (BGR8). These changes provide a consistent way to report native and emitted pixel formats to callers.
Replace the manual QWidget/QHBoxLayout construction for the "Output" settings row with the reusable make_two_field_row helper. This simplifies and standardizes the layout while preserving the same widgets (cam_preserve_mono_checkbox and detected_output_format_label) and applies key_width=60 and gap=40 before adding the row to dlg.settings_form.
Add human-friendly display IDs for multi-camera support. Introduce get_display_id(settings) which prefers settings.name, then properties[backend].device_name, and falls back to backend:index. Main window now stores per-camera display IDs, clears them on stop, passes labels to create_tiled_frame, and uses the display label when building the compact camera status lines. This separates internal camera IDs from user-facing labels for clearer UI.
Adjust tests to reflect new human-friendly display_id values and add a fallback case. Updated expectations in tests to assert display_id equals "GenTL cam" / "GenTL Cam" / "C1" where applicable, and added a unit test to verify get_display_id falls back to the backend index (e.g. "gentl:3") when camera name is empty. Also added assertions in the controller test to ensure the stable camera id is present in frames and correctly mapped to the display id. Files changed: tests/gui/test_rec_manager.py, tests/services/test_multicam_controller.py.
Delete the internal _create_tiled_frame implementation and the public frame accessors (get_frame, get_all_frames, get_tiled_frame) from MultiCameraController. This removes the tiled canvas construction logic and convenience getters for retrieving camera frames; update any callers to use the controller's new/alternate APIs or access frames via the updated code paths.
Collect and propagate runtime camera info to improve recorder FPS selection and logging. - Disabled some verbose timing flags in config (REC_DO_LOG_TIMING, BASLER_DO_LOG_TIMING). - MultiCameraController: added runtime_info signal, stores per-camera runtime info, logs it, and exposes actual_fps_by_camera_id(). Workers emit backend runtime properties on open. - MainWindow: pass actual_fps_by_camera to RecordingManager when starting recordings. - RecordingManager: added backend namespace helper and _resolve_recording_fps(cam, cam_id, frame_rates) to prefer measured FPS, then backend-detected FPS, then requested cam.fps (or auto). Use resolved recorder_fps when creating VideoRecorder and log chosen values. - VideoRecorder: if frame_rate is missing/zero, fall back to 30 FPS and emit a warning; added startup info log; removed/commented the old pix_fmt/output_dimensions branch. These changes make recording frame rates more accurate by preferring runtime-measured FPS and provide clearer logging and safe fallbacks when FPS is unknown.
Fix the video encoding error path in VideoRecorder by replacing _process_timing with _writer_timing so encoding failures are recorded and logged against the correct timing object. This ensures error timing and maybe_log are invoked on the writer timing tracker rather than the wrong object.
Add _camera_pixel_format and _actual_output_format attributes to the Aravis backend and update the GenTL backend to set _camera_pixel_format (replacing the previous _actual_pixel_format assignment). This unifies pixel-format state handling across camera backends and prepares for explicit output format tracking.
RecordingManager now captures and exposes a current/last recording file context, including run/session directories plus per-camera video and timestamp sidecar paths, so downstream processor hooks can still resolve finalized files after stop_all(). VideoRecorder adds explicit output_path and timestamp_json_path properties, and timestamp saving now reuses the shared timestamp path accessor.
Wire the main window to notify custom DLC processors when recording starts/stops, with a shared recording context (run dir, filename stem, and file metadata). Refactor processor lookup into a helper and only fall back to generic save() if no stop hook handles persistence. Extend BaseProcessorSocket with recording context/save-path state, start/stop hook methods, and a stop(save=...) option. Update save() to use an explicit path or configured default path, create parent directories, and improve logging for skipped/failed saves.
Expand custom processor test coverage around recording context and save-path behavior. This adds a new test module for BaseProcessorSocket and DLCLiveMainWindow recording hook interactions, including optional hook handling and processor lookup paths. The existing base processor tests were also cleaned up to use pytest `tmp_path` for file outputs instead of writing into module data directories, remove manual cleanup blocks, and tighten assertions/formatting for save and recording flows.
Replaced the service’s direct dependency on `BaseProcessorSocket.do_build_in_worker` with `processor_builds_in_worker` from `processor_utils`, improving reuse and reducing coupling to socket internals. Also updated the recording context docstring to use `timestamp_json_files` for consistent key naming.
Store the processor recording context when recording starts and reuse that same context when notifying `on_recording_stopped`, instead of rebuilding it from recorder state. This also centralizes stop-time processor finalization (stop hook + save) and invokes it both on normal async stop and during shutdown when a recording context exists.
Adjust shutdown/reset flow to avoid cleaning up the processor while the worker thread is still alive. Cleanup now runs immediately only after a successful stop, and is deferred to the reaper path when a pending reset completes after the worker eventually joins.
Corrects the recording-stop cleanup path to clear `_processor_recording_context` instead of an unrelated attribute. This ensures the async stop handler sees the finalized state consistently and avoids stale recording context after shutdown.
Refines recording lifecycle integration by capturing finalized recording file context when notifying `on_recording_stopped`, and introducing tracking for recording-start notifications during DLC initialization. It also tightens processor teardown behavior by distinguishing pending reset vs pending plugin cleanup, and ensures full processor plugin cleanup when stopping DLC from the UI/service.
Improve processor metadata finalization by rebuilding recording context from the current run directory and then overlaying cached values, so missing fields are backfilled before merging file context. Also update socket processor saving so explicitly provided relative filenames are saved under the `data/` directory, matching expected output behavior.
Adds focused tests around processor recording context handling in `DLCLiveMainWindow`, including file metadata propagation, stop-time context refresh, and optional processor hooks. It also introduces a regression test for `BaseProcessorSocket.save()` to ensure explicit relative filenames still write under the legacy `data/` directory. UI label testing for unknown camera IDs was relaxed to assert the neutral text is present rather than requiring an exact string.
Set `_processor_recording_started_notified` immediately after a successful recording-start hook callback so the UI tracks notification state correctly and avoids repeated notifications. Also add a unit test that verifies `ProcessorSpec` processors are instantiated on the `DLCLiveWorker` thread, wired into `DLCLive`, and properly stopped/cleared when resetting the processor.
… during recording When the user clicks "Stop pose inference" before "Stop recording", the processor instance was destroyed by reset() without saving its accumulated data. Later the recording stop flow would find no processor instance and silently skip the save. Now _stop_inference() saves processor data first if recording is still active, so data is preserved regardless of stop-button order.
`shutdown()` was skipping `_cleanup_processor()` when the worker thread stopped cleanly, leaving the custom processor's resources unreleased and its buffered data unsaved. This commit adds the missing `_cleanup_processor()` call before tearing down the DLCLive instance.
Stopping the DLC processor during a recording skips the processor's `on_recording_stopped` hook, which would normally handle legacy output copies and DB-compatible file alignment. Show a confirmation dialog when the user attempts to stop inference while recording is still active, recommending they stop recording first.
Co-authored-by: Cyril Achard <cyril.achard@epfl.ch>
Co-authored-by: Cyril Achard <cyril.achard@epfl.ch>
Updates camera worker and multi-camera startup logic for more reliable runtime behavior. `recording_sink` is now called with `timestamp_metadata` as a keyword argument to match expected callable signatures, and `is_starting()` now reports startup-in-progress until all expected cameras have either started or failed (instead of only checking for zero started cameras). The worker also no longer clears `_stop_event` at run start, preserving pending stop state.
Ensure each frame is copied before enqueueing in `VideoRecorder` so the async writer works with an immutable snapshot. This prevents downstream frame mutation/race issues when control returns immediately to the capture pipeline.
Emit a dedicated async signal when stopping a recording raises an exception, and handle it on the GUI thread. The new failure handler clears the stopping state, restores start/stop button availability based on recorder activity, updates camera controls, and shows a longer status-bar error message so users get clear feedback instead of a silent stuck state.
Improves the recording-stop confirmation text to clearly state that standard recording-stopped save hooks are bypassed, saving depends on processor stop handling, and unsaved processor data may be lost or unpaired. Also adds an inline code comment documenting that reset does not trigger normal recording stop/save hooks.
Deletes `_on_recording_frame_ready` from `main_window.py`, removing a lean per-camera recording path that bypassed processing, DLC routing, display updates, and FPS tracking. This cleanup reduces dead code and keeps frame handling centered on the multi-frame processing flow.
C-Achard
force-pushed
the
cy/feature-perf-improvement
branch
from
August 21, 2026 08:35
c9264f3 to
e1e8f93
Compare
Update GUI preview tests to mock `is_active` alongside `is_running`, aligning with current controller-state checks. Adds a regression test for stopping preview while a camera is still starting (`is_active=True`, `is_running=False`) and verifies the expected shutdown order: recording, inference, then controller stop.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Scope
Grouped PR containing the latest round of improvements.
Warning
For a detailed account of all features see Contents below.
Contents