Skip to content

Start improving high-throughput multi-camera recording path and diagnostics - #85

Open
C-Achard wants to merge 18 commits into
cy/preserve-mono-supportfrom
cy/improve-encoding-speed
Open

Start improving high-throughput multi-camera recording path and diagnostics#85
C-Achard wants to merge 18 commits into
cy/preserve-mono-supportfrom
cy/improve-encoding-speed

Conversation

@C-Achard

Copy link
Copy Markdown
Contributor

Summary

Improves multi-camera recording behavior under high-throughput conditions by separating recording frame delivery from the heavier GUI/DLC processing path, adding clearer recorder backlog/queue diagnostics, and introducing an optional faster encoding mode for supported FFmpeg codecs.
This is a first quick remediation to the deeper architectural concern requiring the recording to be moved closer to camera SDK frame grabbing.

Main changes

  1. Adds a dedicated recording_frame_ready(camera_id, frame, timestamp) signal for full-rate recording frames.
  2. Recording frames are now handled by a lean recording-only path instead of _on_multi_frame_processing_ready().
  3. Recording-frame emission is disabled by default and only enabled while recording is active.
  4. Adds a "Use faster encoding" parameters checkbox in the Recording panel.
    For libx264/libx265, this applies:
    • Preset ultrafast
    • Tune zerolatency
  5. Centralizes WriteGear/FFmpeg option generation in RecordingSettings.writegear_options().
  6. Adds recorder better diagnostics
  7. Updates multi-camera recorder summaries to aggregate queue/backlog/writer throughput across cameras.
  8. Switches Basler grabbing to GrabStrategy_OneByOne so recording-oriented acquisition does not silently prefer only the latest image.
  9. Reduces noisy Basler trigger debug logs behind an explicit DEBUG_TRIGGER_LOGS flag.
  10. Adds profiling artifacts to .gitignore.
  11. Updates tests for the new recording signal, fast encoding options, recorder stats formatting, and multi-camera recording aggregation.

What this improves

This makes it easier to diagnose when the recorder is falling behind before drops occur. Previously, dropped = 0 could hide the fact that recorder queues were filling. The new stats expose that state directly through queue fill and backlog.
The fast encoding option also provides a user-controlled way to trade compression efficiency/file size for higher recording throughput.

Not solved yet

  • Recording frames still pass through the Qt/main-window path before reaching VideoRecorder.write().
  • The actual encoder write is already threaded, but the producer/enqueue side can still add main-thread pressure at very high frame rates.
  • This PR does not introduce a dedicated recording router thread or direct camera-worker-to-recorder routing.
  • This does not guarantee full-rate recording for extreme cases, but makes bottlenecks more visible and improves slightly the x264 fast path at the cost of worst file size.

Testing

Added/updated unit tests for:

  • Recording-frame signal enable/disable behavior
  • Recording overlay routing
  • RecordingSettings.writegear_options()
  • Fast encoding option propagation
  • Recorder queue/backlog stats formatting
  • Multi-recorder stats aggregation

  • Note before merge: disable debug logging

@C-AchardC-Achard self-assigned this Jun 26, 2026
@C-AchardC-Achard added enhancement New feature or request camera Related to cameras and camera backends config Related to user configs, oading, saving, etc labels Jun 26, 2026
@C-AchardC-Achard added this to the Trigger config feature milestone Jun 26, 2026
@C-Achard
C-Achard requested a review from CopilotJune 29, 2026 09:45

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves multi-camera recording robustness and diagnostics under high-throughput load by adding a dedicated full-rate recording signal/path, enhancing recorder backlog/queue reporting, and introducing optional “fast encoding” FFmpeg parameters via centralized WriteGear option generation.

Changes:

  • Added a dedicated recording_frame_ready(camera_id, frame, timestamp) signal and routed recording through a leaner handler in the main window.
  • Expanded recorder stats to include buffer capacity, queue fill, and backlog; updated aggregated multi-camera summaries accordingly.
  • Added RecordingSettings.writegear_options() (incl. optional “fast encoding”), updated Basler grab strategy, and extended tests/ignores/extras to support these features.

Reviewed changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated 7 comments.

Show a summary per file
FileDescription
tests/utils/test_stats.pyUpdates recorder stats formatting tests for backlog/buffer-capacity output.
tests/test_config.pyAdds unit tests for RecordingSettings.writegear_options() and fast-encoding behavior.
tests/services/test_multicam_controller.pyAdds test for recording-frame emission enable/disable behavior.
tests/gui/test_rec_manager.pyUpdates multi-recorder summary aggregation tests and checks writer options propagation.
tests/gui/test_pose_overlay.pyUpdates overlay-recording test to use the new recording-frame handler.
tests/conftest.pyExtends FakeVideoRecorder to accept new constructor args (buffer/writer options).
tests/cameras/backends/conftest.pyUpdates Basler fake to include GrabStrategy_OneByOne.
pyproject.tomlAdds optional “profiling” dependency group.
dlclivegui/utils/stats.pyAdds recorder backlog/fill helpers and extends recorder stats formatting output.
dlclivegui/services/video_recorder.pyAdds writer_options, improves logging, and adjusts stats computation.
dlclivegui/services/multi_camera_controller.pyAdds recording_frame_ready signal and togglable emission during capture.
dlclivegui/gui/recording_manager.pyCentralizes writer option generation and expands stats aggregation/summaries.
dlclivegui/gui/main_window.pyAdds “fast encoding” UI toggle, connects new recording signal, and routes recording via lean handler.
dlclivegui/config.pyAdds fast_encoding to recording settings; changes timing-log defaults.
dlclivegui/cameras/backends/basler_backend.pySwitches Basler grab strategy and gates trigger debug logging behind a flag.
.gitignoreIgnores common profiling artifacts (scalene/profile outputs).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadtests/utils/test_stats.py Outdated
Comment threaddlclivegui/services/video_recorder.py Outdated
Comment threaddlclivegui/services/video_recorder.py Outdated
Comment threaddlclivegui/services/video_recorder.py
Comment threaddlclivegui/config.py Outdated
Comment threaddlclivegui/gui/main_window.py Outdated
Comment threaddlclivegui/gui/main_window.py
@C-AchardC-Achard linked an issue Jul 1, 2026 that may be closed by this pull request
2 tasks
@C-Achard
C-Achardforce-pushed the cy/improve-encoding-speed branch from d5782e6 to 9fcdc1dCompareJuly 14, 2026 08:56
@C-Achard
C-Achardforce-pushed the cy/improve-encoding-speed branch from 9fcdc1d to 889537aCompareJuly 14, 2026 09:44
@C-Achard
C-Achardforce-pushed the cy/improve-encoding-speed branch from 889537a to d303e97CompareJuly 16, 2026 09:47
@C-Achard
C-Achardforce-pushed the cy/improve-encoding-speed branch from d303e97 to f4f056eCompareJuly 16, 2026 11:15
@C-Achard
C-Achard requested a review from CopilotJuly 17, 2026 08:21

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 18 out of 19 changed files in this pull request and generated 1 comment.

Comment threaddlclivegui/services/video_recorder.py
@C-Achard
C-Achardforce-pushed the cy/improve-encoding-speed branch from f4f056e to 515de8fCompareAugust 10, 2026 12:11
@C-Achard
C-Achardforce-pushed the cy/improve-encoding-speed branch from 515de8f to 03d73a6CompareAugust 10, 2026 14:27

@deruyter92deruyter92 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

amazing start, great improvement. Good that you have the recording_frame_ready split and the Recorder stats seem very helpful for diagnosis.

I think the main concern worth discussing would be the unconditional OneByOne strategy (see comments below). For the rest no blocking issues.

genicam = None # type: ignore[assignment]
pylon = None # type: ignore[assignment]

DEBUG_TRIGGER_LOGS = False

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shall we make this an env var that defaults to False? i.e. any scenario where it still would be useful?

@C-AchardC-AchardAug 11, 2026

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated in d2f6125, I would keep it for debugging

Comment on lines 641 to 646

self._camera.StartGrabbing(
pylon.GrabStrategy_LatestImageOnly,
# pylon.GrabStrategy_LatestImageOnly,
pylon.GrabStrategy_OneByOne,
)
LOG.info(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a big unconditional change which we should give some consideration.

I understand that we want to prevent discarding intermediate frames (which is mostly useful for viewing, but not recording), but processing all frames in arrival order also has it's risks/downsides. Some remarks/questions:

  • If the buffer is full (currently 10 -> ~100ms ?), does this mean that new incoming frames are lost?
  • Does unconditionally adopting this new strategy mean that viewing (not recording) operates on a growing backlog of stale frames?
  • Do we have proper diagnostics on the acquisition-side, not the recorder side? i.e. do we know the number of ready frames in the buffer, if frames are dropped, etc? or does the recorder never see this?

Comment on lines +183 to +192
writer_kwargs: dict[str, Any] = {
"compression_mode": True,
"logging": False,
"-input_framerate": fps_value,
"-vcodec": (self._codec or "libx264").strip() or "libx264",
"-vcodec": codec_value,
"-crf": int(self._crf),
}

if self._writer_options is not None:
writer_kwargs.update(self._writer_options)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems slightly duplicated with writegear_options in config.py

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is, but it does leave VideoRecorder indendently usable, and I do like the overrides (renamed the param so it is clearer that it has precedence).
There is still overlap this way but it may be the simplest, best of both worlds approach.

Let me know what you think!

Comment threaddlclivegui/config.py
Comment on lines +562 to +570
if self.fast_encoding:
if codec_value in {"libx264", "libx265"}:
opts.update(
{
"-preset": "ultrafast",
"-tune": "zerolatency",
}
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good addition, nice win! Flagging here as a potential follow-up later: the multiple processes still compete for CPU, so hardware encoding might help a lot. But this would need their own option profile (e.g NVENC uses different presets).

@C-Achard
C-Achardforce-pushed the cy/improve-encoding-speed branch 2 times, most recently from d98aeb5 to d2f6125CompareAugust 11, 2026 09:24
@C-Achard
C-Achardforce-pushed the cy/improve-encoding-speed branch from d2f6125 to 973e19cCompareAugust 11, 2026 11:56
Switch Basler camera startup to `pylon.GrabStrategy_OneByOne` instead of `LatestImageOnly`, and update the nearby identity-persistence comment for clarity.
Add ignore patterns for generated profiling files (`profile*.svg`, `scalene*.json`, and `scalene*.html`) so local performance analysis outputs are not accidentally committed.
Introduce a new `profiling` optional dependency group in `pyproject.toml` and include `scalene` so profiling tools can be installed independently from test and framework extras.
Adds a dedicated full-rate `recording_frame_ready` signal path so recording is decoupled from the inference/display frame flow, reducing processing overhead during capture. The GUI now exposes a fast-encoding toggle and persists it into recording settings, and recorder startup passes codec-specific writer options through to `VideoRecorder`.
Recording telemetry was expanded to report enqueued vs written frames, writer FPS, queue fill against buffer size, backlog, and drops, improving visibility into recording throughput and pressure.
Turns on timing logging for multi-camera worker, recorder, and Basler backend diagnostics. Recording settings now include a `fast_encoding` flag, with `writegear_options` made more robust for missing/invalid FPS and optional low-latency FFmpeg options (`ultrafast` + `zerolatency`) for x264/x265. Recorder stats were expanded with buffer capacity awareness (`buffer_size`), derived backlog/fill-ratio properties, and richer formatted output showing queue fill and backlog.
Updates test fixtures and unit tests around recording flow changes: FakeVideoRecorder now mirrors new constructor/runtime fields, Basler fake includes one-by-one grab strategy, and GUI/controller tests validate recording-frame emission gating plus overlay recording via `_on_recording_frame_ready`. Tests also cover `RecordingSettings.writegear_options` (including fast x264 options and FPS fallback), RecordingManager writer option wiring, and richer recorder stats formatting/aggregation with backlog and queue capacity output.
Wire the fast encoding checkbox to QSettings so its value is restored on startup and saved when toggled. This adds typed get/set helpers for `recording/fast_encoding` in `DLCLiveGUISettingsStore`, updates `main_window` to prefer persisted values over config defaults, and includes a roundtrip unit test for the new setting. It also removes an obsolete commented-out recording block in the frame processing path.
Remove defensive `hasattr` checks when connecting recording settings signals in `DLCLiveMainWindow`. The widgets are expected to exist, so connecting unconditionally avoids silently skipping persistence and recording path preview updates if an expected widget is missing or renamed.
Turns off multi-camera, recorder, and Basler timing logs by default to reduce debug noise in normal runs. Also cleans up stale priority wording in main window comments, updates VideoRecorder docstrings to reflect `writer_options`, and fixes stats tests to import `RecorderStats` from `dlclivegui.utils.stats`.
Ensure ffmpeg writer defaults (`-input_framerate`, `-vcodec`, `-crf`) are always applied, even when custom writer options are provided, while still allowing overrides via `writer_options`. Also improve recorder stats by estimating `buffer_seconds` from average or last frame latency when write FPS is unavailable, avoiding zero/underreported buffer duration.
Adds a pending-recording flow in the main window so "Start recording" while preview is stopped first starts preview, then begins recording only after all active cameras have produced frames. The pending state is cleared on stop/error/init failure to avoid stale triggers and duplicate starts. Adds GUI tests covering deferred start, waiting for all camera frames, frame-ready trigger behavior, and no double-starts.
In the multi-camera recording flow, the `QTimer.singleShot` call that automatically triggered `_start_multi_camera_recording` after starting preview is commented out. This stops the delayed automatic recording start when preview is not yet running.
Moves `DEBUG_TRIGGER_LOGS` into `config.py` so camera backends share a single toggle for trigger diagnostics. Basler now imports the shared flag instead of defining a local constant, and GenTL’s trigger-node dump is now guarded by both DEBUG log level and the config flag to avoid noisy logs unless explicitly enabled.
Refactors recorder option handling to use `writer_options_overrides` instead of replacing all defaults, and updates the recording manager call site accordingly. Recorder creation now normalizes and validates key FFmpeg/WriteGear options (`-input_framerate`, `-crf`, `-vcodec`) to enforce numeric/string types before initializing `WriteGear`. Recording settings were also updated to emit native numeric values for framerate and CRF, aligning config output with the new normalization path.
@C-Achard
C-Achardforce-pushed the cy/improve-encoding-speed branch from 973e19c to b73839eCompareAugust 11, 2026 13:23
@C-Achard
C-Achard marked this pull request as ready for review August 11, 2026 13:27
This reverts part of commit b73839e by ensuring the options dict remains typecasted as previously
Update test fixtures and GUI recorder-manager assertions to use `writer_options_overrides` instead of `writer_options`. This keeps the fake recorder interface consistent with the current recorder API and ensures writegear option checks target the renamed field.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cameraRelated to cameras and camera backendsconfigRelated to user configs, oading, saving, etcenhancementNew feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Recording and trigger mode improvements

3 participants

@C-Achard@deruyter92