Open
Conversation
…r integration - Implemented `get_device_count` method in `GenTLCameraBackend` to retrieve the number of GenTL devices detected. - Added `max_devices` configuration option in `CameraSettings` to limit device probing. - Introduced `BoundingBoxSettings` for bounding box visualization, integrated into the main GUI. - Enhanced `DLCLiveProcessor` to accept a processor instance during configuration. - Updated GUI to support processor selection and auto-recording based on processor commands. - Refactored camera properties handling and removed deprecated advanced properties editor. - Improved error handling and logging for processor connections and recording states.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
… options to config
- Update configuration settings to include support for single-animal models in DLCProcessorSettings. - Improve user guide with a link to DLC documentation for model export.
…le_animal in DLCLiveProcessor settings
- Implemented MultiCameraController to manage multiple cameras simultaneously.
…fix for basler backend
…me data without tiling and track source camera ID
…ulti-camera controller
Update tests/utils/test_utils.py to import Engine from dlclivegui.temp instead of dlclivegui.temp.engine. Aligns the test import with the package re-export or module relocation so tests reference the correct top-level import.
Ensure model_check_path is set for non-.pb selections before calling DLCLiveProcessor.get_model_backend. .pb files still use the parent directory (TensorFlow expectation); other file types now pass the file path itself to avoid using an undefined or incorrect path.
Introduce a ModelType Literal type ("pytorch" | "tensorflow") and change DLCProcessorSettings.model_type from a plain str to this constrained type, keeping the default as "pytorch". This ensures pydantic validation enforces allowed model backends.
Set a static version (2.0.0rc1) in pyproject.toml and remove the dynamic setuptools configuration that read dlclivegui.__version__. Also remove the placeholder __version__ from dlclivegui/__init__.py so version metadata is centralized in pyproject.toml.
Introduce a cti_files_source marker (auto/user) and wiring to track whether CTI file lists were user-specified or auto-discovered. Treat legacy top-level properties.cti_file(s) as strict user overrides; treat properties.gentl.cti_file(s) as either an auto-cache (falls back to discovery if stale) or a user override depending on the marker. Persist the resolved source back into the namespace when resolving CTIs, and update harvester-selection/rebind logic to fall back to discovery when auto-cached CTIs are stale while still raising for strict user overrides. Also add a small import and internal field (_cti_files_source_used) to track the chosen source, plus logging when falling back from a stale auto-cache.
Add preflight checks, pattern validation and safer globbing for GenTL (.cti) discovery and loading. Renamed default CTI pattern constant to _LEGACY_DEFAULT_CTI_PATTERNS (Windows-only comment) and imported Path. Introduced _cti_preflight to detect missing/locked/permissioned files before Harvester.add_file and applied it where CTIs are loaded (skipping and logging problematic entries). Harden gentl_discovery with: redact-able diagnostics.summarize, _validate_glob_pattern, bounded _glob_limited, static-prefix checks, allowed-roots option and new discover_cti_files params (allow_globs, root_globs_allowed, max_glob_hits_per_pattern) to limit expensive scans. Also adjusted Harvester.update failure handling to treat update errors as discovery failures (return empty loaded list) and improved logging messages for discovery/load failures.
Introduce a single _expand_user_and_env() helper to consistently expand environment variables and '~', and use it across path normalization and glob validation. Update _normalize_path() to rely on the helper and Path handling, switch file existence checks to use a Path object, and add _dedup_key() to normalize deduplication on case-insensitive filesystems (Windows). Replace scattered os.path.expandvars/os.path.expanduser calls with the new helper for more predictable cross-platform behavior.
Adjust Harvester.update() error log string concatenation in gentl_backend.py to ensure proper spaces between message parts. Also remove an extraneous comment line from the gentl backend test to clean up test output.
Upgrade Gentl backend cti discovery
Pre-release cleanup for PySide6 GUI
Update dlclive requirement & add deploy workflow
Update the testing CI workflow to pass the CODECOV_TOKEN (secrets.CODECOV_TOKEN) to the Codecov action and set fail_ci_if_error to false so a failed upload won't break CI. Also add an inline note on the step name indicating the upload may need to be disabled if the token isn't configured.
Update .github/workflows/python-package.yml to trigger the pull_request workflow for branches [main, master] instead of [main, public]. This ensures PRs targeting the master branch will run the python-package CI workflow.
Add an Ubuntu-specific step to the python-package workflow to install Qt/OpenGL runtime dependencies (libegl1, libgl1, libopengl0, libxkbcommon-x11-0, libxcb-cursor0). This ensures CI runners have the required libraries for Qt/OpenGL-based install checks; the step runs apt-get update and installs the packages before the existing "Install tools" step.
This reverts commit 09a8364.
Simplify and reorganize the CI workflow: normalize tag pattern quoting and narrow pull_request branches/types. Add permissions.contents=read. Rename the main job to a test_matrix job that runs a Python matrix, streamline setup and pip installs, remove verbose cache steps (use setup-python cache option), and simplify build/twine checks and wheel smoke test (only imports the package; CLI smoke test removed). Add a separate build_release job (runs on tag pushes) to build distributions and upload them as artifacts. Update publish job to download artifacts, install twine, and upload to PyPI using non-interactive --skip-existing; make publish depend on build_release.
Split the workflow into validation and release paths and add clearer step names and safeguards. PRs against main/master now run a validation matrix (3.10, 3.11, 3.12) that builds the package, runs twine check, and smoke-tests installing the wheel; fail-fast is disabled to surface version-specific issues. Tag pushes matching v*.*.* trigger a canonical release build (single Python 3.12) that produces dist artifacts, uploads them as workflow artifacts, and a separate publish job downloads those artifacts and uploads them to PyPI. Other improvements: minimal permissions (contents: read), explicit checkout/setup steps, pip caching enabled, comments for clarity, and use of --skip-existing when uploading to PyPI.
Capture tox output to a file and append the coverage summary to the GitHub Actions job summary only for the ubuntu-latest / Python 3.12 matrix. Restrict Codecov uploads to that same matrix for PRs targeting main/master and point the upload at a per-environment coverage XML (coverage.py312.xml). Move coverage settings into tox.ini so tests generate env-specific .coverage files (COVERAGE_FILE) and XML reports ({toxinidir}/.coverage.{envname}.xml) for more reliable CI artifact handling.
Update GitHub Actions and tox/pyproject coverage settings so coverage output is picked up reliably. Read the tox output from tox-output.log and point the codecov action at ./.coverage.py312.xml. Stop excluding site-packages in pyproject (commented out) because it caused our installed package to be omitted from reports. Also tidy tox.ini coverage command formatting and ensure XML/term reports are emitted per envname.
Update tox.ini to set pytest --cov to {envsitepackagesdir}/dlclivegui instead of the project module name. This ensures coverage collects from the package installed into the tox environment (matching the GitHub Actions job) and avoids missing coverage when the package is not imported from the project root.
Replace the sed extraction with an awk script that starts printing at the coverage header and stops when the "Coverage XML written to file" marker is seen, preventing extraneous trailing output from being appended to the GitHub job summary. Retains the code block wrapping and || true to avoid step failures, and includes minor whitespace cleanup in the workflow file.
Select the built wheel dynamically and echo its path, install the package via a file:// wheel URL including the [pytorch] extras and add PyTorch CPU index to pip, replace the multi-line import check with a single python -c import verification, and run dlclivegui --help in offscreen Qt mode. These changes make the smoke test more robust by ensuring PyTorch dependencies are resolved and exercising the CLI in a headless environment.
Author
|
Note : deploy workflow smoke tests failures should be fixed by #55; may require subsequent PRs |
|
I'll be stress-testing and reviewing changes today, with a focus on TensorFlow models. Like yourself, I cannot test all backends, but will look if I spot any vulnerabilities in the code. Good to have feedback from people that use it in an actual experimental setup. @C-Achard let me know if there are things that you'd like a second-opinion on, or if I can assist with anything specific. |
Author
|
Thanks a lot @deruyter92 ! |
deruyter92
force-pushed
the
cy/pre-release-fixes-2.0
branch
from
ba20dac to
fd605f8
Compare
Deploy workflow candidate for 2.0.0rc1
Update Codecov upload behavior in .github/workflows/testing-ci.yml to set fail_ci_if_error to false. This prevents the workflow from failing when the Codecov upload step encounters transient errors, ensuring CI jobs aren't blocked by Codecov availability or upload issues.
deruyter92
approved these changes
Feb 27, 2026
deruyter92
left a comment
deruyter92
left a comment
There was a problem hiding this comment.
Left some comments. Overall, really clean code and when testing it, the GUI works fantastic for me.
Few general remarks:
- TensorFlow model worked out of the box for me!
- No reason to postpone beta-testing, but we might need to look at a clean solution for teardown in some places, when the thread did not exit cleanly. Setting shared fields to None could have side-effects (such as missing tails frames for JSON writing or hitting invalid states where _queue == None or _writer gone).
- A big applause for the user experience, I really think you did a great job in making the GUI very intuitive, flexible and giving it a very clean appearance!!
Approving it advance, to let you decide which ones you will address / ignore / or write down for later.
| ] | ||
| dependencies = [ | ||
| "cv2-enumerate-cameras", | ||
| "deeplabcut-live==1.1", |
There was a problem hiding this comment.
Shouldn't we allow for slightly decoupled development? In case we release a new version of DeepLabCut-live, we should keep it entirely backward compatible anyway.
Suggested change
| "deeplabcut-live==1.1", | |
| "deeplabcut-live>=1.1", |
Comment on lines
+454
to
+465
|
|
||
| def start(self): | ||
| self._proc.reset() | ||
| self.active = True | ||
| self.initialized = False | ||
|
|
||
| def stop(self): | ||
| self.active = False | ||
| self.initialized = False | ||
| self._proc.reset() | ||
| self._last_pose = None | ||
|
|
There was a problem hiding this comment.
This was assigning to a property, should it be:
Suggested change
| def start(self): | |
| self._proc.reset() | |
| self.active = True | |
| self.initialized = False | |
| def stop(self): | |
| self.active = False | |
| self.initialized = False | |
| self._proc.reset() | |
| self._last_pose = None | |
| def start(self): | |
| self._proc.reset() | |
| self.active = True | |
| self._proc.initialized = False | |
| def stop(self): | |
| self.active = False | |
| self._proc.initialized = False | |
| self._proc.reset() | |
| self._last_pose = None | |
Comment on lines
+136
to
+142
|
|
||
| def write_frame(self, cam_id: str, frame: np.ndarray, timestamp: float | None = None) -> None: | ||
| rec = self._recorders.get(cam_id) | ||
| if not rec or not rec.is_running: | ||
| return | ||
| try: | ||
| rec.write(frame, timestamp=timestamp or time.time()) |
There was a problem hiding this comment.
timestamp=0.0 is currently overwritten with time.time(), consider:
Suggested change
| def write_frame(self, cam_id: str, frame: np.ndarray, timestamp: float | None = None) -> None: | |
| rec = self._recorders.get(cam_id) | |
| if not rec or not rec.is_running: | |
| return | |
| try: | |
| rec.write(frame, timestamp=timestamp or time.time()) | |
| def write_frame(self, cam_id: str, frame: np.ndarray, timestamp: float | None = None) -> None: | |
| rec = self._recorders.get(cam_id) | |
| if not rec or not rec.is_running: | |
| return | |
| try: | |
| rec.write(frame, timestamp=timestamp if timestamp is not None else time.time()) |
Comment on lines
+145
to
+152
| def start(self, camera_settings: list[CameraSettings]) -> None: | ||
| """Start multiple cameras; accepts dataclasses, pydantic models, or dicts.""" | ||
| if self._running: | ||
| LOGGER.warning("Multi-camera controller already running") | ||
| return | ||
|
|
||
| active_settings = [s for s in camera_settings if s.enabled][: self.MAX_CAMERAS] | ||
| if not active_settings: |
There was a problem hiding this comment.
This docstring seems a bit misleading as dicts will fail (has no attribute 'enabled'):
Suggested change
| def start(self, camera_settings: list[CameraSettings]) -> None: | |
| """Start multiple cameras; accepts dataclasses, pydantic models, or dicts.""" | |
| if self._running: | |
| LOGGER.warning("Multi-camera controller already running") | |
| return | |
| active_settings = [s for s in camera_settings if s.enabled][: self.MAX_CAMERAS] | |
| if not active_settings: | |
| def start(self, camera_settings: list[CameraSettings]) -> None: | |
| """Start multiple cameras""" | |
| if self._running: | |
| LOGGER.warning("Multi-camera controller already running") | |
| return | |
| active_settings = [s for s in camera_settings if s.enabled][: self.MAX_CAMERAS] | |
| if not active_settings: |
Comment on lines
+237
to
+259
|
|
||
| def _writer_loop(self) -> None: | ||
| assert self._queue is not None | ||
| try: | ||
| while True: | ||
| try: | ||
| item = self._queue.get(timeout=0.1) | ||
| except queue.Empty: | ||
| if self._stop_event.is_set(): | ||
| break | ||
| continue | ||
| if item is _SENTINEL: | ||
| self._queue.task_done() | ||
| break | ||
| frame, timestamp = item | ||
| start = time.perf_counter() | ||
| try: | ||
| assert self._writer is not None | ||
| self._writer.write(frame) | ||
| except OSError as exc: | ||
| with self._stats_lock: | ||
| self._encode_error = exc | ||
| logger.exception("Video encoding failed while writing frame") |
There was a problem hiding this comment.
Other than OSErrors were not captured, consider making this a general exception (like below suggestion) or adding other blocks.
Suggested change
| def _writer_loop(self) -> None: | |
| assert self._queue is not None | |
| try: | |
| while True: | |
| try: | |
| item = self._queue.get(timeout=0.1) | |
| except queue.Empty: | |
| if self._stop_event.is_set(): | |
| break | |
| continue | |
| if item is _SENTINEL: | |
| self._queue.task_done() | |
| break | |
| frame, timestamp = item | |
| start = time.perf_counter() | |
| try: | |
| assert self._writer is not None | |
| self._writer.write(frame) | |
| except OSError as exc: | |
| with self._stats_lock: | |
| self._encode_error = exc | |
| logger.exception("Video encoding failed while writing frame") | |
| def _writer_loop(self) -> None: | |
| assert self._queue is not None | |
| try: | |
| while True: | |
| try: | |
| item = self._queue.get(timeout=0.1) | |
| except queue.Empty: | |
| if self._stop_event.is_set(): | |
| break | |
| continue | |
| if item is _SENTINEL: | |
| self._queue.task_done() | |
| break | |
| frame, timestamp = item | |
| start = time.perf_counter() | |
| try: | |
| assert self._writer is not None | |
| self._writer.write(frame) | |
| except Exception as exc: | |
| with self._stats_lock: | |
| self._encode_error = exc | |
| logger.exception("Video encoding failed while writing frame") |
Comment on lines
+180
to
+200
|
|
||
| def stop(self) -> None: | ||
| if self._writer is None and not self.is_running: | ||
| return | ||
| self._stop_event.set() | ||
| if self._queue is not None: | ||
| try: | ||
| self._queue.put_nowait(_SENTINEL) | ||
| except queue.Full: | ||
| pass | ||
| # self._queue.put(_SENTINEL) | ||
| if self._writer_thread is not None: | ||
| self._writer_thread.join(timeout=5.0) | ||
| if self._writer_thread.is_alive(): | ||
| logger.warning("Video recorder thread did not terminate cleanly") | ||
| if self._writer is not None: | ||
| try: | ||
| self._writer.close() | ||
| except Exception: | ||
| logger.exception("Failed to close WriteGear cleanly") | ||
|
|
There was a problem hiding this comment.
Not sure how to fix this cleanly, but only warning that the thread did not terminate may not be enough here right?
We should avoid setting self._queue or self._writer_thread to None when the thread is not terminated: the property is_running falsely would report False even if the real thread was still alive.
Maybe returning and moving some teardown to the writer_loop finally block could resolve this?
Suggested change
| def stop(self) -> None: | |
| if self._writer is None and not self.is_running: | |
| return | |
| self._stop_event.set() | |
| if self._queue is not None: | |
| try: | |
| self._queue.put_nowait(_SENTINEL) | |
| except queue.Full: | |
| pass | |
| # self._queue.put(_SENTINEL) | |
| if self._writer_thread is not None: | |
| self._writer_thread.join(timeout=5.0) | |
| if self._writer_thread.is_alive(): | |
| logger.warning("Video recorder thread did not terminate cleanly") | |
| if self._writer is not None: | |
| try: | |
| self._writer.close() | |
| except Exception: | |
| logger.exception("Failed to close WriteGear cleanly") | |
| def stop(self) -> None: | |
| if self._writer is None and not self.is_running: | |
| return | |
| self._stop_event.set() | |
| if self._queue is not None: | |
| try: | |
| self._queue.put_nowait(_SENTINEL) | |
| except queue.Full: | |
| pass | |
| # self._queue.put(_SENTINEL) | |
| if self._writer_thread is not None: | |
| self._writer_thread.join(timeout=5.0) | |
| if self._writer_thread.is_alive(): | |
| logger.warning("Video recorder thread did not terminate cleanly") | |
| return # <- Return to prevent teardown | |
| if self._writer is not None: | |
| try: | |
| self._writer.close() | |
| except Exception: | |
| logger.exception("Failed to close WriteGear cleanly") | |
Comment on lines
+276
to
+278
| finally: | ||
| self._finalize_writer() | ||
|
|
There was a problem hiding this comment.
...and move this to the finally block of the _writer_loop:
Suggested change
| finally: | |
| self._finalize_writer() | |
| finally: | |
| self._finalize_writer() | |
| self._save_timestamps() | |
| self._queue = None | |
| if self._writer_thread is threading.current_thread(): | |
| self._writer_thread = None |
Comment on lines
+200
to
+207
|
|
||
| # Save timestamps to JSON file | ||
| self._save_timestamps() | ||
|
|
||
| self._writer = None | ||
| self._writer_thread = None | ||
| self._queue = None | ||
|
|
There was a problem hiding this comment.
Maybe remove these here...
Suggested change
| # Save timestamps to JSON file | |
| self._save_timestamps() | |
| self._writer = None | |
| self._writer_thread = None | |
| self._queue = None | |
| self._writer = None |
Comment on lines
+392
to
+398
| # Normal operation: timed get | ||
| try: | ||
| wait_start = time.perf_counter() | ||
| item = self._queue.get(timeout=0.05) | ||
| queue_wait_time = time.perf_counter() - wait_start | ||
| except queue.Empty: | ||
| continue |
There was a problem hiding this comment.
if queue ever becomes None or is replaced with something unexpected, what should happen? At least maybe log this exception?
Suggested change
| # Normal operation: timed get | |
| try: | |
| wait_start = time.perf_counter() | |
| item = self._queue.get(timeout=0.05) | |
| queue_wait_time = time.perf_counter() - wait_start | |
| except queue.Empty: | |
| continue | |
| # Normal operation: timed get | |
| try: | |
| if self._queue is None: | |
| break | |
| wait_start = time.perf_counter() | |
| item = self._queue.get(timeout=0.05) | |
| queue_wait_time = time.perf_counter() - wait_start | |
| except queue.Empty: | |
| continue | |
| except Exception as exc: | |
| logger.exception("Could not retrieve item from queue", exc_info=exc) | |
| self.error.emit(str(exc)) | |
| continue |
Comment on lines
+220
to
+226
| # Just wait for the timed get() loop to observe the flag and drain | ||
| self._worker_thread.join(timeout=2.0) | ||
| if self._worker_thread.is_alive(): | ||
| logger.warning("DLC worker thread did not terminate cleanly") | ||
|
|
||
| self._worker_thread = None | ||
| self._queue = None |
There was a problem hiding this comment.
Not sure how to cleanly address this, but the shared queue and worker thread should probably not be nulled when the worker thread did not terminate cleanly. (similar to previous comment in video_recorder)
Suggested change
| # Just wait for the timed get() loop to observe the flag and drain | |
| self._worker_thread.join(timeout=2.0) | |
| if self._worker_thread.is_alive(): | |
| logger.warning("DLC worker thread did not terminate cleanly") | |
| self._worker_thread = None | |
| self._queue = None | |
| # Just wait for the timed get() loop to observe the flag and drain | |
| self._worker_thread.join(timeout=2.0) | |
| if self._worker_thread.is_alive(): | |
| logger.warning("DLC worker thread did not terminate cleanly") | |
| return | |
| # Safe cleanup only after the thread is actually dead | |
| self._worker_thread = None | |
| self._queue = None |
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
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.
Finalizing the PySide6+multi-camera GUI
Further refinement of GUI added in #35 by @arturoptophys
Remaining TODOs
Features
Improved settings UX/UIWorks as is, delayedMake processor socket design a bit safer by defaultAlso tweaks error handling, UI and UX.
Additional TODOs
see Create 2.0 documentation #39See Update docs TOC and Sphinx excludes DeepLabCut#3201Camera backends unit/smoke testRelated