Upgrade & fix GenTL multi-camera mode#76
Conversation
Introduce thread-safe SharedHarvesterEntry and SharedHarvesterPool to share Harvester instances keyed by canonical CTI file sets. The pool provides acquire/release/refcount/refresh semantics with RLock protection, reference counting, initial device enumeration, and safe reset on final release. Add optional import handling for harvesters (raising a clear error when missing). Enhance _normalize_path with a casefold_windows flag to produce a canonical path form for pool keys (used when sorting/canonicalizing CTI file lists) and import threading to support locking.
Introduce a process-shared Harvester (SharedHarvesterPool) and per-backend _shared_entry to manage GenTL producer state and locking. CTI preflight now records CTIs that passed validation and acquires a shared Harvester instance; device list refresh and acquirer creation/start/stop/destroy are performed under the shared lock to avoid race conditions/hotplug issues. On acquire failure the shared entry is released and a descriptive RuntimeError is raised. _reset_harvester now releases the shared entry when present. Minor diagnostics updates: persist CTI lists and adjust reporting of loaded CTIs.
Wrap harvester acquisition and camera initialization in a try/except to ensure resources are cleaned up on failure and to raise a clearer RuntimeError that includes loaded/failed CTIs and the original exception. Improve device selection logic: when a target_device_id or explicit serial is provided but not found, raise a descriptive error listing available serials; otherwise keep index-based selection and validate index range.
Add explicit lists of color and mono GenTL pixel formats and default pixel_format to "auto" with normalization. Rewrite _configure_pixel_format to handle missing PixelFormat node, choose a suitable format when "auto" is requested (prefer color formats, then mono, then first available), warn and fallback when a requested format is unavailable, and persist the selected format. Update frame postprocessing to use the normalized pixel format for proper Bayer demosaicing (BayerRG/GB/GR/BG) and correct RGB->BGR conversion while leaving BGR8 native. Minor logging message tweaks and added defensive checks to improve robustness.
Capture and surface CTI load diagnostics when acquiring the shared GenTL Harvester: record loaded and failed CTI files from the shared entry and from acquire-time exceptions, and slightly reformat the open() error message. Add a FakeSharedHarvesterPool test double (with FakeSharedEntry and custom acquire/release/refcount behavior) and integrate it into the test fixture patch_gentl_sdk so tests can exercise shared-harvester reuse, update counting and failure release semantics. Also adjust FakeImageAcquirer to clear its queue on start and to synthesize payloads when the queue is empty, wrap FakeHarvester.update to track update calls, and update tests to reflect new rebind/open behavior and to add coverage for shared harvester reuse and error propagation.
There was a problem hiding this comment.
Pull request overview
This PR updates the GenTL backend to support reliable multi-camera startup by introducing a process-local shared-Harvester model (keyed by canonicalized CTI file sets), and expands tests and logging to validate and debug the new behavior.
Changes:
- Added a shared Harvester pool (
SharedHarvesterEntry/SharedHarvesterPool) and path canonicalization to avoid per-camera Harvester enumeration conflicts. - Updated
GenTLCameraBackend.open()to acquire/reuse shared Harvester instances and avoid callingHarvester.update()per camera. - Improved multi-camera controller stability (settings deep-copies, duplicate detection) and added tests + a new debug logging flag.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
dlclivegui/cameras/backends/utils/gentl_discovery.py |
Adds shared Harvester pooling and path normalization for CTI-based sharing. |
dlclivegui/cameras/backends/gentl_backend.py |
Switches GenTL open/close to shared-Harvester lifecycle; adds selection/config/telemetry updates. |
dlclivegui/services/multi_camera_controller.py |
Deep-copies settings per worker and adds duplicate camera configuration detection + logging. |
dlclivegui/main.py |
Adds --debug-log and env override to control logging verbosity. |
tests/cameras/backends/conftest.py |
Adds Fake shared pool + updates fake acquisition behavior and SDK patching for pooled Harvester. |
tests/cameras/backends/test_gentl_backend.py |
Updates/extends tests for serial-ID rebinding behavior and shared Harvester reuse. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Treat failed CTI loads as a mapping of CTI->error and record failures when initializing the shared Harvester. gentl_discovery.py now logs exceptions, captures failed_files as a dict, and raises a runtime error if no CTI producers were successfully loaded; it also attaches loaded_files and failed_files to raised exceptions and attempts to reset the harvester. gentl_backend.py updated to consume failed_files as a dict (using .items()) when building reporting data. Added a helper to reset the harvester and propagate context for callers.
Change seen from a set to a mapping of identity key -> camera_id to record which camera produced each key. Add get_camera_id and fallback to camera_id if camera_identity_key raises, logging the exception. Emit a more informative initialization_failed message that includes the camera_id and the conflicting camera, improving diagnostics and robustness when computing identity keys.
Make the pool key order-independent by sorting normalized absolute file paths before forming the tuple. This prevents different keys for the same set of CTI files when they are provided in a different order (while preserving normcase/abspath normalization).
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Update tests/services/test_multicam_controller.py to import and use get_display_id instead of get_camera_id when accessing mfd.frames. This aligns the test with the frames dict keys (display IDs) and the updated multi_camera_controller API.
deruyter92
left a comment
There was a problem hiding this comment.
Overal looks good! One potential bug, worth looking into (inconsistency between get_display_id vs get_camera_id), but once that is resolved should be good to go. Some minor other comments as well.
| def _start_camera(self, settings: CameraSettings) -> None: | ||
| """Start a single camera.""" | ||
| cam_id = get_camera_id(settings) | ||
| settings_copy = copy.deepcopy(settings) | ||
| cam_id = get_display_id(settings_copy) | ||
| if cam_id in self._workers: | ||
| LOGGER.warning(f"Camera {cam_id} already has a worker") | ||
| return |
There was a problem hiding this comment.
start_all in recording_manager stores recorders under get_camera_id() ("gentl:serial:SER0"), which is different from here: get_display_id() ("gentl:0"). This could be a bug causing no frames to be written.
There was a problem hiding this comment.
same affects _refresh_dlc_camera_list()
There was a problem hiding this comment.
Thanks, indeed it seems I sort of did this halfway, but the true intent was really only to use display_id for the GUI and keep everything internal with the more stable ID.
The mapping is now done later to avoid issues with mixing the two, which I think is a better design. Let me know
Add support for display_id labels throughout the multi-camera flow. MultiFrameData now includes display_ids; MultiCameraController stores per-camera display_ids (set on start, cleared on stop/reset) and emits them with frame_ready. Also switch to getting camera_id and display_id separately when starting workers. The tiled-frame helper create_tiled_frame gained an optional labels mapping and will draw the display label (falling back to camera_id) on each tile so the GUI can show friendly names.
Treat target IDs starting with "fp:" as valid and return settings early so open() can match devices by fingerprint via existing _select_device/_match_device logic. Also change CTI file load failures from logger.exception to logger.warning to reduce noisy stack traces while still recording failed files for diagnostics.
Add unit tests to ensure services use stable camera IDs (get_camera_id) rather than GUI display IDs (get_display_id). Tests added to tests/gui/test_rec_manager.py verify RecordingManager routes frames by stable ID, doesn't accept frames keyed by display ID, and doesn't infer frame size from display-only keys. Tests added/updated in tests/services/test_multicam_controller.py assert MultiCameraController emits frames and timestamps keyed by stable IDs, exposes a display_id mapping, and no internal use of display IDs; also update an existing rotation test to reference the stable camera ID. Imported get_camera_id/get_display_id and CameraSettings where needed.
Scope
This PR fixes multi-camera startup for GenTL cameras by replacing the per-camera Harvester() with a shared-Harvester model.
Once the first Imaging Source USB3 Vision camera is opened/streaming, a second independent Harvester() instance can no longer reliably enumerate devices.
Key behavior
Other changes in brief
Automated summary
GenTL backend improvements
SharedHarvesterEntryandSharedHarvesterPoolclasses togentl_discovery.pyfor process-local sharing and reference counting of Harvester instances, keyed by canonicalized CTI file sets. Prevents first Harvester from keeping all cti to itself and making second instance see none. (dlclivegui/cameras/backends/utils/gentl_discovery.py)_normalize_pathto handle Windows case folding and canonicalization, ensuring consistent CTI file keys. (dlclivegui/cameras/backends/utils/gentl_discovery.py)Multi-camera controller robustness
SingleCameraWorker, now deep-copiesCameraSettingsto prevent side effects from shared references, and adds detailed debug logging before and after backend creation. (dlclivegui/services/multi_camera_controller.py) [1] [2]dlclivegui/services/multi_camera_controller.py) [1] [2]dlclivegui/services/multi_camera_controller.py)Logging and configuration
--debug-logCLI option and aconfigure_loggingfunction to control application logging verbosity, with support for an environment variable override. (dlclivegui/main.py) [1] [2]Test suite improvements
FakeSharedHarvesterPooland related helpers to simulate the shared Harvester pool in tests, ensuring the test environment matches production pooling and reference counting behavior. (tests/cameras/backends/conftest.py) [1] [2]tests/cameras/backends/conftest.py)tests/cameras/backends/test_gentl_backend.py) [1] [2]