{{ message }}
feat: capture resolution + detection size dropdowns#1833
Open
5uck1ess wants to merge 4 commits into
Open
Conversation
Contributor
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new
update_statusmessages and some tooltips added in_build_camera_cardare hardcoded English strings; consider wrapping them in_()like the other UI text so they participate in localization. - In
_on_det_size_change, importingreset_face_analyserinside the slot will run on every change; it would be cleaner and slightly more efficient to move this import to the module level unless you are explicitly avoiding a circular import.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `update_status` messages and some tooltips added in `_build_camera_card` are hardcoded English strings; consider wrapping them in `_()` like the other UI text so they participate in localization.
- In `_on_det_size_change`, importing `reset_face_analyser` inside the slot will run on every change; it would be cleaner and slightly more efficient to move this import to the module level unless you are explicitly avoiding a circular import.
## Individual Comments
### Comment 1
<location path="modules/ui.py" line_range="759" />
<code_context>
+ self._det_size_options = [160, 320, 640]
+ for v in self._det_size_options:
+ self.cb_det_size.addItem(f"{v} x {v}")
+ cur_det = int(getattr(modules.globals, 'det_size', 640))
+ idx = self._det_size_options.index(cur_det) if cur_det in self._det_size_options else 2
+ self.cb_det_size.setCurrentIndex(idx)
</code_context>
<issue_to_address>
**suggestion:** Centralize the default det_size handling to avoid repeating literal defaults in multiple places.
The default `640` for `det_size` now lives here, in `face_analyser._current_det_size`, and in the CLI defaults. If one changes and the others don’t, the CLI, backend, and UI could diverge. Consider defining a single default (e.g., a constant in `modules.globals` or `face_analyser`) and referencing it everywhere instead of repeating the literal.
Suggested implementation:
```python
self.cb_det_size = QComboBox()
self._det_size_options = [160, 320, 640]
for v in self._det_size_options:
self.cb_det_size.addItem(f"{v} x {v}")
# Use a centralized default detection size so UI/CLI/backend stay in sync.
default_det_size = getattr(
modules.globals,
"DEFAULT_DET_SIZE",
self._det_size_options[-1],
)
cur_det = int(
getattr(
modules.globals,
"det_size",
default_det_size,
)
)
if cur_det in self._det_size_options:
idx = self._det_size_options.index(cur_det)
elif default_det_size in self._det_size_options:
idx = self._det_size_options.index(default_det_size)
else:
idx = len(self._det_size_options) - 1
self.cb_det_size.setCurrentIndex(idx)
```
To fully implement the centralization you suggested, the following should also be done elsewhere in the codebase:
1. In `modules/globals` (or a similar configuration module), define a single constant:
```python
DEFAULT_DET_SIZE = 640 # or whatever default you want
```
2. Update `face_analyser._current_det_size` to use `modules.globals.DEFAULT_DET_SIZE` instead of an inline literal (e.g. `640`).
3. Update CLI default handling for `det_size` to also use `modules.globals.DEFAULT_DET_SIZE` (e.g. via `default=DEFAULT_DET_SIZE` or equivalent).
4. Ensure `DEFAULT_DET_SIZE` remains consistent with `self._det_size_options` (e.g., is one of `[160, 320, 640]`), or adjust the options list to match.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
5uck1ess
added a commit
to 5uck1ess/Deep-Live-Cam
that referenced
this pull request
May 19, 2026
Addresses sourcery-ai review on PR hacksider#1833: the default detection size (640) was duplicated in UI cb_det_size init, face_analyser._current_det_size, and CLI parse_args. Hoist a single DEFAULT_DET_SIZE constant in modules.globals so UI/CLI/backend stay in sync if it ever needs to change. UI fallback also picks the matching index instead of a positional literal.
5uck1ess
added a commit
to 5uck1ess/Deep-Live-Cam
that referenced
this pull request
Jun 24, 2026
…, i18n Adversarial review of hacksider#1833 surfaced a BLOCKER the bots missed plus the repo's recurring persistence gap: - BLOCKER (race): the det_size dropdown calls reset_face_analyser() which nulls FACE_ANALYSER while the live worker is detecting. get_face_analyser() published the FaceAnalysis object to the global *before* prepare()/optimize(), so a lock-free reader could grab a half-built analyser → crash/garbage. Now built into a local and published only after prepare()/optimize(), so a concurrent reset/rebuild can never expose an unprepared analyser. This makes mid-Live det_size changes safe. - Persist capture_resolution + det_size in save/load_switch_states (with validation: bad/corrupt values fall back to defaults), and save on change — previously both reset to defaults on every restart. - Move 'from modules.face_analyser import reset_face_analyser' to module level (no circular import exists) instead of importing inside the slot each call. - Localization: pre-translate the static part of the two new status messages and append the dynamic size, so they participate in i18n.
Ported behavior from April 2026 Fork: - globals.det_size (160/320/640) — face detection input resolution. Lower = faster, less accurate at distance. - globals.capture_resolution (tuple) — requested webcam resolution. Camera may negotiate down; actual size logged by VideoCapturer. UI changes (camera card now uses QGridLayout): - Resolution dropdown: 640x360, 640x480, 960x540, 1280x720, 1920x1080. Applies on next Live start. - Det size dropdown: 160, 320, 640. Triggers FACE_ANALYSER re-init via reset_face_analyser() so the new size takes effect on next call. Backend changes: - face_analyser.py: DET_SIZE constant replaced with _current_det_size() that reads modules.globals.det_size. Added reset_face_analyser() helper for hot-swap. - ui.py: VideoCapturer.start() now passes globals.capture_resolution instead of hardcoded PREVIEW_DEFAULT_WIDTH/HEIGHT (which are kept for preview window sizing, separate concern). - core.py: --det-size CLI flag (choices: 160, 320, 640).
Addresses sourcery-ai review on PR hacksider#1833: the default detection size (640) was duplicated in UI cb_det_size init, face_analyser._current_det_size, and CLI parse_args. Hoist a single DEFAULT_DET_SIZE constant in modules.globals so UI/CLI/backend stay in sync if it ever needs to change. UI fallback also picks the matching index instead of a positional literal.
…, i18n Adversarial review of hacksider#1833 surfaced a BLOCKER the bots missed plus the repo's recurring persistence gap: - BLOCKER (race): the det_size dropdown calls reset_face_analyser() which nulls FACE_ANALYSER while the live worker is detecting. get_face_analyser() published the FaceAnalysis object to the global *before* prepare()/optimize(), so a lock-free reader could grab a half-built analyser → crash/garbage. Now built into a local and published only after prepare()/optimize(), so a concurrent reset/rebuild can never expose an unprepared analyser. This makes mid-Live det_size changes safe. - Persist capture_resolution + det_size in save/load_switch_states (with validation: bad/corrupt values fall back to defaults), and save on change — previously both reset to defaults on every restart. - Move 'from modules.face_analyser import reset_face_analyser' to module level (no circular import exists) instead of importing inside the slot each call. - Localization: pre-translate the static part of the two new status messages and append the dynamic size, so they participate in i18n.
…view - load_switch_states: reject zero/negative persisted capture_resolution before it reaches cv2 (only type/length was validated before) - det_size dropdown: normalize a bad persisted size to the default (then the last option) so .index() can never raise ValueError - get_face_analyser: capture det_size once and thread it into _optimize_det_model instead of re-reading the global, removing a latent TOCTOU if that step ever moves outside the lock - drop unused numpy import (F401)
5uck1ess
force-pushed
the
pr/resolution-and-det-size
branch
from
a603dfb to
5d1c79d
Compare
Author
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
1 participant
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.
Summary
Adds two dropdowns to the Camera card on the main window:
Also exposes
--det-sizeas a CLI flag for headless runs.Why
Resolution: Previously
VideoCapturer.start()was hardcoded toPREVIEW_DEFAULT_WIDTH/HEIGHTwhich mixes "the preview window's pixel size" with "the camera's capture resolution" — they're separate concerns. With this PR they're decoupled and the capture side becomes user-configurable.Why these choices specifically: 640×480 is native on virtually every USB 2.0 webcam and the safest 30fps mode (USB 2.0 isochronous bandwidth caps ~30fps on common YUYV at this size). Higher tiers are useful for users with capture cards or USB 3.0 webcams; the tooltip explains that 720p+ typically drops to ~10fps over USB 2.0 due to bandwidth, so users know what they're trading.
Det size:
insightfacedefaults to 640×640 detection which is plenty accurate but expensive per frame. At 320 or 160 the detector runs much faster — a meaningful speedup when the face is large in frame (live webcam usage), with negligible accuracy loss for that distance regime. Currently the only way to changedet_sizeis by editing the source.What changed
modules/globals.pydet_size: int = 640andcapture_resolution: tuple = (640, 480)globals with documentation.modules/face_analyser.pyDET_SIZE = (640, 640)constant is replaced by_current_det_size()that readsmodules.globals.det_sizeper call.reset_face_analyser()clears the cachedFACE_ANALYSERso the next call re-prepares insightface with the new size.modules/core.py— new--det-sizeargparse flag withchoices=[160, 320, 640]for headless runs.modules/ui.pyQGridLayout(was a single row) with the two new dropdowns on rows 1 and 2 below the camera selector + Live button._on_resolution_changewrites the selected(w, h)tuple tomodules.globals.capture_resolution._on_det_size_changewrites the selectedintand callsreset_face_analyser()so the new size kicks in on the next analyser call.WebcamPreviewWindowreadsmodules.globals.capture_resolutioninstead ofPREVIEW_DEFAULT_WIDTH/HEIGHT(the constants still drive preview-window sizing, separate concern).Test plan
640x480and Det size=640 x 640. Live mode behaves as before1280 x 720, start Live → console shows[VideoCapturer] 1280x720or the camera's nearest supported sizepython run.py --det-size 320headless honors the flagNo new dependencies.
Summary by Sourcery
Add configurable camera capture resolution and face detection size for both GUI and headless live mode.
New Features:
Enhancements: