Add ability to disable looping in holistic replay - #1834
Conversation
📝 WalkthroughWalkthroughThis PR adds environment-controlled replay looping, applies it to video and metadata-only replay nodes, documents the setting, and delays pipeline shutdown by five seconds when replay reaches end of file. ChangesReplay Behavior
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds an environment-variable switch to control whether holistic replay loops, allowing users to disable looping without code changes.
Changes:
- Read
DEPTHAI_REPLAY_LOOP(default: enabled) when setting up holistic replay. - Apply the loop setting to both
ReplayVideoandReplayMetadataOnlynodes created during holistic replay setup. - Document the new environment variable in the README.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/opencv/HolisticRecordReplay.cpp | Reads DEPTHAI_REPLAY_LOOP and propagates it to replay nodes via setLoop(...). |
| README.md | Documents DEPTHAI_REPLAY_LOOP in the environment variable table. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Line 238: Update the README entry for DEPTHAI_REPLAY_LOOP to show how to
disable looping: state it's ON by default and that setting the environment
variable DEPTHAI_REPLAY_LOOP to a falsy value (e.g. 0, false, no, off —
case-insensitive) will disable looping, matching the boolean style used for
DEPTHAI_TELEMETRY; reference the DEPTHAI_REPLAY_LOOP variable name so readers
can find and use it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 66023c8e-a288-499a-b63e-eb3cde0c6ea9
📒 Files selected for processing (2)
README.mdsrc/opencv/HolisticRecordReplay.cpp
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: copilot-pull-request-reviewer
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: MaticTonin
Repo: luxonis/depthai-core PR: 1732
File: src/pipeline/Pipeline.cpp:662-662
Timestamp: 2026-03-24T22:39:00.650Z
Learning: In `src/pipeline/Pipeline.cpp`, the `DEPTHAI_AUTOCALIBRATION` environment variable intentionally defaults to `"ON_START"` (not `""`). This means auto-calibration is initialized by default for stereo pipelines at pipeline build time. The resulting implicit EEPROM/calibration writes on startup are by design and should not be flagged as a backward-incompatible change or unintended side-effect in future reviews.
Learnt from: MaticTonin
Repo: luxonis/depthai-core PR: 1753
File: src/pipeline/Pipeline.cpp:834-839
Timestamp: 2026-04-16T01:04:34.569Z
Learning: In `src/pipeline/Pipeline.cpp` (`PipelineImpl::build()`), when `autoCalibrationModeSetByApi` is false, `DEPTHAI_AUTOCALIBRATION` is read with an empty-string default (`""`). If the env var is unset or explicitly empty, `autoCalibrationMode` falls back to the pipeline-configured default (`ON_START`). This means an explicitly empty env var no longer disables auto-calibration — it simply defers to the API/default mode. This behavior is intentional and should not be flagged in future reviews.
📚 Learning: 2026-03-24T22:39:00.650Z
Learnt from: MaticTonin
Repo: luxonis/depthai-core PR: 1732
File: src/pipeline/Pipeline.cpp:662-662
Timestamp: 2026-03-24T22:39:00.650Z
Learning: In `src/pipeline/Pipeline.cpp`, the `DEPTHAI_AUTOCALIBRATION` environment variable intentionally defaults to `"ON_START"` (not `""`). This means auto-calibration is initialized by default for stereo pipelines at pipeline build time. The resulting implicit EEPROM/calibration writes on startup are by design and should not be flagged as a backward-incompatible change or unintended side-effect in future reviews.
Applied to files:
README.mdsrc/opencv/HolisticRecordReplay.cpp
📚 Learning: 2026-04-16T01:04:34.569Z
Learnt from: MaticTonin
Repo: luxonis/depthai-core PR: 1753
File: src/pipeline/Pipeline.cpp:834-839
Timestamp: 2026-04-16T01:04:34.569Z
Learning: In `src/pipeline/Pipeline.cpp` (`PipelineImpl::build()`), when `autoCalibrationModeSetByApi` is false, `DEPTHAI_AUTOCALIBRATION` is read with an empty-string default (`""`). If the env var is unset or explicitly empty, `autoCalibrationMode` falls back to the pipeline-configured default (`ON_START`). This means an explicitly empty env var no longer disables auto-calibration — it simply defers to the API/default mode. This behavior is intentional and should not be flagged in future reviews.
Applied to files:
README.mdsrc/opencv/HolisticRecordReplay.cpp
🔇 Additional comments (2)
src/opencv/HolisticRecordReplay.cpp (2)
472-472: LGTM!Also applies to: 496-496
452-452: ⚡ Quick win
getEnvAscall is valid (2-argument overload exists).
src/utility/Environment.hppdefinestemplate <typename T> T getEnvAs(const std::string& var, T defaultValue, bool cache = true), which forwards to the logger-backed overload, soutility::getEnvAs<bool>("DEPTHAI_REPLAY_LOOP", true)compiles.
| | DEPTHAI_ZOO_MODELS_PATH | (Default) depthai_models - Folder where zoo model description files are stored | | ||
| | DEPTHAI_RECORD | Enables holistic record to the specified directory. | | ||
| | DEPTHAI_REPLAY | Replays holistic replay from the specified file or directory. | | ||
| | DEPTHAI_REPLAY_LOOP | Loops recorded data (ON by default). | |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Document how to disable looping.
The entry states looping is ON by default but doesn't tell users how to turn it off. Consider noting the accepted falsy values to match the style of other boolean entries (e.g. DEPTHAI_TELEMETRY).
📝 Suggested wording
-| DEPTHAI_REPLAY_LOOP | Loops recorded data (ON by default). |
+| DEPTHAI_REPLAY_LOOP | Loops recorded replay data. Enabled by default; set to `0` or `false` to disable. |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | DEPTHAI_REPLAY_LOOP | Loops recorded data (ON by default). | | |
| | DEPTHAI_REPLAY_LOOP | Loops recorded replay data. Enabled by default; set to `0` or `false` to disable. | |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` at line 238, Update the README entry for DEPTHAI_REPLAY_LOOP to
show how to disable looping: state it's ON by default and that setting the
environment variable DEPTHAI_REPLAY_LOOP to a falsy value (e.g. 0, false, no,
off — case-insensitive) will disable looping, matching the boolean style used
for DEPTHAI_TELEMETRY; reference the DEPTHAI_REPLAY_LOOP variable name so
readers can find and use it.
…e/disable_loop_holistic_replay
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/opencv/HolisticRecordReplay.cpp (1)
450-453: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReset replay state when loop configuration parsing fails.
getEnvAs<bool>throws for invalidDEPTHAI_REPLAY_LOOPvalues, butrecordConfig.stateis alreadyREPLAY. The catch block returnsfalsewithout restoring the state, so callers can observeREPLAYafter replay setup failed.} catch(const std::exception& e) { + recordConfig.state = RecordConfig::RecordReplayState::NONE; spdlog::warn("Replay disabled: {}", e.what());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/opencv/HolisticRecordReplay.cpp` around lines 450 - 453, Update the replay setup flow around recordConfig.state and utility::getEnvAs<bool>("DEPTHAI_REPLAY_LOOP", true) so parsing failures restore recordConfig.state to its pre-replay value before returning false. Ensure the catch path handles invalid DEPTHAI_REPLAY_LOOP values without leaving the object in REPLAY state.
♻️ Duplicate comments (1)
README.md (1)
238-238: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDocument how to disable looping.
The entry states that looping is enabled by default but does not explain how to turn it off. Document values such as
0,false, oroffso users can configure the new behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` at line 238, Update the DEPTHAI_REPLAY_LOOP entry in README.md to document that looping is enabled by default and can be disabled by configuring the variable with values such as 0, false, or off.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/opencv/HolisticRecordReplay.cpp`:
- Around line 450-453: Update the replay setup flow around recordConfig.state
and utility::getEnvAs<bool>("DEPTHAI_REPLAY_LOOP", true) so parsing failures
restore recordConfig.state to its pre-replay value before returning false.
Ensure the catch path handles invalid DEPTHAI_REPLAY_LOOP values without leaving
the object in REPLAY state.
---
Duplicate comments:
In `@README.md`:
- Line 238: Update the DEPTHAI_REPLAY_LOOP entry in README.md to document that
looping is enabled by default and can be disabled by configuring the variable
with values such as 0, false, or off.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 8007fd9c-123a-4c71-846f-a291d4d6cd34
📒 Files selected for processing (3)
README.mdsrc/opencv/HolisticRecordReplay.cppsrc/pipeline/node/host/Replay.cpp
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-03-24T22:39:04.364Z
Learnt from: MaticTonin
Repo: luxonis/depthai-core PR: 1732
File: src/pipeline/Pipeline.cpp:705-705
Timestamp: 2026-03-24T22:39:04.364Z
Learning: Do not flag the `!= ""` part of the auto-calibration condition as redundant when it appears in `PipelineImpl::build()` (or closely related pipeline build logic). If the code uses `utility::getEnvAs<std::string>(..., default)` with a default such as `"ON_START"`, the explicit empty-string guard may still be intentional to treat an explicitly empty env var as “OFF/disabled” (or to avoid special-casing elsewhere). Only consider removing `!= ""` if the codebase has an explicit, enforceable guarantee that `DEPTHAI_AUTOCALIBRATION` can never be set to an empty string (e.g., via validated parsing/CI checks); otherwise, keep the guard.
Applied to files:
src/pipeline/node/host/Replay.cpp
🔇 Additional comments (2)
src/opencv/HolisticRecordReplay.cpp (1)
27-27: LGTM!Also applies to: 52-52, 98-98, 472-472, 496-496
src/pipeline/node/host/Replay.cpp (1)
2-2: LGTM!Also applies to: 208-212, 299-300, 322-323, 450-451
Purpose
Specification
None / not applicable
Dependencies & Potential Impact
None / not applicable
Deployment Plan
None / not applicable
Testing & Validation
None / not applicable
AI Usage
Assisted-by: AGENT_NAME:MODEL_VERSION [TOOL1] [TOOL2]
Submitted code was reviewed by a human: YES/NO
The author is taking the responsibility for the contribution: YES/NO
Summary by CodeRabbit
New Features
DEPTHAI_REPLAY_LOOPenvironment variable to control recorded replay looping behavior (enabled by default).Bug Fixes / Improvements
Documentation
DEPTHAI_REPLAY_LOOP.