Use native segmentation parser and mask#304
Open
rolandocortez wants to merge 13 commits into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThis PR replaces the local segmentation-mask message path with ChangesNative SegmentationMask migration
CI and dependency version bump
YOLOExtendedParser stride configuration
SnapData documentation update
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
depthai_nodes/node/utils/message_remapping.py (1)
62-78: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winUse a uint8 border sentinel here
cv2.warpPerspectiveturnsborderValue=-1into0foruint8masks, so remapped border pixels are labeled as class 0 instead of the unassigned sentinel used elsewhere (255). SetborderValue=255or make it dtype-aware.🤖 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 `@depthai_nodes/node/utils/message_remapping.py` around lines 62 - 78, The remapping logic in remap_segmentation_mask_array uses a borderValue that does not preserve the unassigned class for uint8 masks, so border pixels get remapped to class 0 instead of the sentinel used elsewhere. Update the cv2.warpPerspective call to use a uint8-safe sentinel such as 255, or make the borderValue depend on the segmentation_mask dtype, while keeping the existing transformation and interpolation behavior unchanged.depthai_nodes/node/utils/util_constants.py (1)
12-23: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winChange
UNASSIGNED_MASK_LABELto255in mask remapping.depthai_nodes/node/utils/message_remapping.pystill uses this constant asborderValueforcv2.warpPerspective, and-1gets clipped to0foruint8masks, not255. That turns padded/unassigned pixels into class0instead of background.🤖 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 `@depthai_nodes/node/utils/util_constants.py` around lines 12 - 23, The mask remapping background sentinel is wrong: UNASSIGNED_MASK_LABEL should be set to 255 so padded pixels stay as background instead of being clipped to 0 for uint8 masks. Update the constant in util_constants and keep message_remapping’s cv2.warpPerspective borderValue usage aligned with that sentinel so unassigned mask areas remain correctly marked.
🤖 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 @.github/workflows/e2e_tests.yaml:
- Around line 114-115: The `exec hil_runner` command in the workflow is
interpolating `github.ref_name` directly into a shell string, which can allow
template/shell injection on the runner. Move `github.ref_name` and the other
`${{ }}` values used in `--docker-run-args` into step-level `env:` variables,
then reference them as shell variables inside the `run:` command. Keep the fix
localized to this workflow step so the `exec hil_runner` invocation no longer
embeds attacker-influenced values directly.
In `@depthai_nodes/node/parsers/fastsam.py`:
- Around line 380-382: The empty-detection fallback in merge_masks handling is
using an all-255 mask, which gets treated as active pixels and overwrites the
output instead of preserving the background/unassigned sentinel. Update the
no-results branch in fastsam.py around the results_masks initialization so the
fallback is zero-filled before calling merge_masks(), keeping the inactive path
truly inactive.
In `@depthai_nodes/node/parsers/segmentation.py`:
- Around line 47-58: The SegmentationParser currently skips the
`background_class` remap whenever `classes_in_one_layer` is true, so class 0
remains unchanged in that combination. Update the parsing logic in
`SegmentationParser` so the background remap is still applied when
`background_class` is enabled, even if `classes_in_one_layer` is set, or
explicitly validate/document that this flag combination is unsupported. Use the
`SegmentationParser` initializer and the class-remapping branch in its parse
flow to locate the change.
In `@depthai_nodes/node/utils/message_remapping.py`:
- Around line 81-92: remap_segmentation_mask currently rebuilds a new
dai.SegmentationMask but only transfers the CV mask, so labels metadata is lost.
Update remap_segmentation_mask in message_remapping.py to preserve labels the
same way copy_message.py does: read labels from the input segmentation_mask when
available, and set them on new_mask after setCvMask, using the same guarded
getLabels/setLabels pattern as the existing message-copy logic.
---
Outside diff comments:
In `@depthai_nodes/node/utils/message_remapping.py`:
- Around line 62-78: The remapping logic in remap_segmentation_mask_array uses a
borderValue that does not preserve the unassigned class for uint8 masks, so
border pixels get remapped to class 0 instead of the sentinel used elsewhere.
Update the cv2.warpPerspective call to use a uint8-safe sentinel such as 255, or
make the borderValue depend on the segmentation_mask dtype, while keeping the
existing transformation and interpolation behavior unchanged.
In `@depthai_nodes/node/utils/util_constants.py`:
- Around line 12-23: The mask remapping background sentinel is wrong:
UNASSIGNED_MASK_LABEL should be set to 255 so padded pixels stay as background
instead of being clipped to 0 for uint8 masks. Update the constant in
util_constants and keep message_remapping’s cv2.warpPerspective borderValue
usage aligned with that sentinel so unassigned mask areas remain correctly
marked.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: cb782501-6076-47ad-931e-5f93cb42ebc7
📒 Files selected for processing (23)
.github/workflows/e2e_tests.yaml.github/workflows/stability_tests.yamldepthai_nodes/message/README.mddepthai_nodes/message/__init__.pydepthai_nodes/message/creators/segmentation.pydepthai_nodes/message/segmentation.pydepthai_nodes/message/utils/copy_message.pydepthai_nodes/node/README.mddepthai_nodes/node/apply_colormap.pydepthai_nodes/node/parser_generator.pydepthai_nodes/node/parsers/fastsam.pydepthai_nodes/node/parsers/segmentation.pydepthai_nodes/node/parsers/utils/fastsam.pydepthai_nodes/node/parsers/yolo.pydepthai_nodes/node/utils/message_remapping.pydepthai_nodes/node/utils/util_constants.pyrequirements.txttests/stability_tests/check_messages.pytests/unittests/test_creators/test_segmentation.pytests/unittests/test_messages/test_copy_message.pytests/unittests/test_messages/test_segmentation_msg.pytests/unittests/test_nodes/test_host_nodes/test_apply_colormap_node.pytests/utils/messages/creators/arrays.py
💤 Files with no reviewable changes (4)
- depthai_nodes/message/init.py
- depthai_nodes/message/segmentation.py
- tests/unittests/test_messages/test_segmentation_msg.py
- depthai_nodes/message/README.md
klemen1999
reviewed
Jul 3, 2026
klemen1999
left a comment
klemen1999
left a comment
Collaborator
There was a problem hiding this comment.
Few comments but overall looks good
Collaborator
|
TODO: Before merge we need next DepthAI version released. This means that we need to wait with merge and update the requirements.txt |
klemen1999
reviewed
Jul 3, 2026
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 `@tests/stability_tests/run_parser_test.py`:
- Around line 93-96: The stability test currently forces SegmentationParser to
use the host-side path, so it never validates the native
dai.node.SegmentationParser flow. Update the test around parser_generator.build
in run_parser_test.py to add a separate SegmentationParser case or parameterize
hostOnly so both host-only and native segmentation parsing are exercised, while
keeping the existing parser_name-based logic for the other parsers.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: ba14b24d-a601-46b7-846e-fc0328c0b379
📒 Files selected for processing (3)
depthai_nodes/node/parsers/fastsam.pytests/stability_tests/check_messages.pytests/stability_tests/run_parser_test.py
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/stability_tests/check_messages.py
- depthai_nodes/node/parsers/fastsam.py
Contributor
Author
|
Got it. Pushed the follow-up fixes and CI is green now. I fixed the stability failures from the native SegmentationMask migration, addressed the review comments around getLabels/setLabels, background_class handling, and setNNArchiveHead(head), and updated the parser mock to match the new ParserGenerator API usage. |
klemen1999
reviewed
Jul 5, 2026
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
2 participants
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.
Purpose
Use native DepthAI segmentation components in
depthai-nodes.This PR updates the segmentation flow to use native
dai.SegmentationMaskand nativedai.node.SegmentationParserwhere applicable, while preserving the existing host-sidedepthai-nodesparser forhostOnly=True.It also aligns host parser background semantics with the native parser and fixes visualization of binary segmentation masks where class
0is a valid foreground class and255is background.Additionally, this PR adds the missing
setStrides(...)setter toYOLOExtendedParser.Specification
depthai_nodes.message.segmentation.SegmentationMaskmessage.copy_message(dai.SegmentationMask)support.ParserGeneratorto use nativedai.node.SegmentationParserwhenparser_name == "SegmentationParser"andhostOnly=False.depthai_nodes.node.parsers.segmentation.SegmentationParserforhostOnly=True.background_classsupport to the hostSegmentationParser.background_class=Truemaps class0to255.classes_in_one_layer=Trueignoresbackground_class.ApplyColormaphandling for nativedai.SegmentationMask:255background maps to visual background index0.0maps to a visible foreground index.YOLOExtendedParser.setStrides(...)for manually setting YOLO output head strides.Dependencies & Potential Impact
This depends on a DepthAI version exposing native
dai.SegmentationMaskanddai.node.SegmentationParser.Potential impact is limited to segmentation parsing, segmentation mask copying, segmentation visualization paths, and the new YOLO parser strides setter API. The existing host segmentation parser path remains available through
hostOnly=True.Direct Visualizer rendering of binary
dai.SegmentationMaskvalues[0, 255]may not visibly color class0; theApplyColormap -> ImgFrameOverlaypath handles this correctly.Deployment Plan
No special deployment steps required.
Rollback: revert this PR.
Testing & Validation
tests/unittests/test_messages/test_copy_message.pytests/unittests/test_nodes/test_host_nodes/test_apply_colormap_node.pySegmentationParser.run()directly with synthetic NN output.0 -> 255whenbackground_class=Trueclasses_in_one_layer=Trueignoresbackground_class[0, 255]masks.ApplyColormap -> ImgFrameOverlaycorrectly visualizes binary masks where foreground class id is0.YOLOExtendedParser.setStrides(...)locally:python -m compileallpassed for touched parser files.Note: For
luxonis/mediapipe-selfie-segmentation:256x144, a dual-parser MRE feeding the sameNeuralNetworkoutput to both parsers showed nativedai.node.SegmentationParseremitting only[0], while the hostdepthai-nodesparser emitted[0, 255]for the same sequence numbers and mask shape. Since both parsers consumed the same NN output, this appears isolated to the native parser/model metadata path rather than the host parser changes in this PR.AI Usage
Assisted-by: ChatGPT
Submitted code was reviewed by a human: YES
The author is taking the responsibility for the contribution: YES
Summary by CodeRabbit
dai.SegmentationMasksupport across parsing/remapping and ApplyColormap processing.background_classfor segmentation outputs.setStridessupport for extended YOLO parsing.SnapDatatype.uint8, with consistent handling for empty/unassigned pixels.dai.SegmentationMask.3.7.1.