Skip to content

ToF node enhancements, host-runnable ToF and image filters #1308

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 46 commits into
base: develop
Choose a base branch
from

Conversation

lnotspotl
Copy link
Member

@lnotspotl lnotspotl commented Apr 28, 2025

The goal of this PR is to introduce a new host-runnable node for general image filtering (ImageFilters) as well as a new node called ToFDepthConfidenceFilter designed specifically for depth frame filtering based on data coming from the ToF node.

Additionally, the ToF node was renamed to ToFBase and a new ToF node took its place, combining ToFBase, ToFDepthConfidenceFilter and ImageFilters into a single node. This gives power-users the opportunity to fiddle with the low-level stuff (they can still use the origin ToF node implementation by instantiating the ToFBase class). For an average user, however, the new ToF node with a predefined set of filters will likely suffice.

image

The image filters were taken from the FW codebase and the ToF filter was implemented based on a reference python implementation provided by the CV team. Both the ToFDepthConfidenceFilter node and the ImageFilters node allow for run-time parameter updates thanks to the possibility of sending in input configs. In addition to that, the ImageFilters node need not have a fixed filter pipeline, it is possible to change that filter pipeline completely at runtime.

@lnotspotl lnotspotl self-assigned this Apr 28, 2025
@lnotspotl lnotspotl requested a review from moratom April 28, 2025 06:28
Copy link
Collaborator

@moratom moratom left a comment

Choose a reason for hiding this comment

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

Thanks Jakub!

Left some comments&suggestions, generally looks good.
Let's also add an example (can be similar to stereo_depth_from_host.py) and tests before merging.

@@ -187,6 +187,23 @@ ImgFrame& ImgFrame::setMetadata(const std::shared_ptr<ImgFrame>& sourceFrame) {
return setMetadata(*sourceFrame);
}

ImgFrame& ImgFrame::setDataFrom(const ImgFrame& sourceFrame) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should go with copyDataFrom to make it clearer it's not a move.

Comment on lines 19 to 22
typedef dai::StereoDepthConfig::PostProcessing::SpatialFilter SpatialFilterParams;
typedef dai::StereoDepthConfig::PostProcessing::SpeckleFilter SpeckleFilterParams;
typedef dai::StereoDepthConfig::PostProcessing::TemporalFilter TemporalFilterParams;
typedef std::variant<MedianFilterParams, SpatialFilterParams, SpeckleFilterParams, TemporalFilterParams> FilterParams;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Probbably better to move to the filters to a common location and then use using ... in both StereoDepth and here

Comment on lines 13 to 22
/**
* Index of the filter to be applied
*/
std::int32_t filterIndex;

/**
* Parameters of the filter to be applied
*/
FilterParams filterParams;

Copy link
Collaborator

Choose a reason for hiding this comment

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

Any reason why we didn't go with std::vector here?

Copy link
Member Author

Choose a reason for hiding this comment

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

The idea is as follows: Say we have filters, applied sequentially, (F1 | F2 | F3 | F4 | F5). When one wants to modify the properties any one filter, say F3, one can simply specify its index and properties without modifying or having to worry about all the other filters. This will throw an error if the param's type does not match the expected type of the filter at the given index.

Comment on lines 27 to 33
struct SequentialDepthFiltersProperties : PropertiesSerializable<Properties, SequentialDepthFiltersProperties> {
/**
* List of filters (the type of which is determined by the filter parameters) to apply to the input frame
*/
std::vector<FilterParams> filters;
};

Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's rather use an initialConfig, to not duplicate parameters.

Copy link
Member Author

Choose a reason for hiding this comment

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

Read this, the properties and the config are different, we have a fixed sequence of filters, each of which one can set the properties at runtime for.

Comment on lines 39 to 44
struct DepthConfidenceFilterProperties : PropertiesSerializable<Properties, DepthConfidenceFilterProperties> {
/**
* Threshold for the confidence filter
*/
float confidenceThreshold = 0.0f;
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as for DepthFilter properties


constexpr const size_t PERSISTENCY_LUT_SIZE = 256;

struct TemporalFilterParams {
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not break out each implementation in their own .cpp ?

@@ -0,0 +1,904 @@
#include "depthai/pipeline/node/host/DepthFilters.hpp"
Copy link
Collaborator

Choose a reason for hiding this comment

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

i'd rather call these ImageFilters, they can be used for general purposes, if one wants to.
Only depthConfidence filter is ToF specific?
E.g. temporal filter could be used on input images to stereo depth to reduce noise. Same for median.

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct, the DepthConfidenceFilter is specifically designed to used alongside the ToF node.

@lnotspotl lnotspotl marked this pull request as draft May 6, 2025 08:51
@lnotspotl lnotspotl marked this pull request as ready for review May 6, 2025 14:43
@SzabolcsGergely SzabolcsGergely requested a review from Copilot May 25, 2025 00:34
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Adds two new host-runnable filter nodes (image filters and depth-confidence filter) to the pipeline, along with their data/config types, properties, bindings, and example scripts.

  • Introduce ImgFrame::copyDataFrom and clone methods to duplicate frames
  • Define ImageFilters and DepthConfidenceFilter nodes, their configs, properties, and Python/JS bindings
  • Refactor StereoDepthConfig to use centralized FilterParams, update datatype enums, CMake, and examples

Reviewed Changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/pipeline/datatype/ImgFrame.cpp Implement copyDataFrom and clone for frame duplication
include/depthai/pipeline/datatype/ImgFrame.hpp Declare copyDataFrom/clone and update doc comments
src/pipeline/datatype/DatatypeEnum.cpp Add new datatypes to hierarchy for image/depth-confidence
include/depthai/pipeline/datatype/DatatypeEnum.hpp Extend DatatypeEnum with new config entries
include/depthai/properties/ImageFiltersProperties.hpp Define properties for both filter nodes
include/depthai/pipeline/node/ImageFilters.hpp Create ImageFilters and DepthConfidenceFilter nodes
include/depthai/pipeline/datatype/StereoDepthConfig.hpp Refactor post-processing filters to FilterParams
include/depthai/pipeline/FilterParams.hpp Introduce centralized filter parameter structs/enums
include/depthai/pipeline/datatype/ImageFiltersConfig.hpp New buffer messages for runtime filter reconfiguration
bindings/python/.../FilterParamsBindings.* Python bindings for filter parameters
bindings/python/.../ImageFiltersBindings.cpp Python/node bindings for new filter nodes
bindings/js/bindings.cpp Add ImageFiltersConfig to JS bindings
CMakeLists.txt Link OpenCV calib3d, include new source and binding files
examples/python/.../stereo_depth_filters.py Python example for image filters on stereo disparity
examples/python/RVC2/ToF/tof_host_filter.py Python example for depth-confidence filter with ToF node
Comments suppressed due to low confidence (1)

src/pipeline/datatype/ImgFrame.cpp:200

  • New clone method should have unit tests verifying deep copy of both metadata and pixel data.
std::shared_ptr<ImgFrame> ImgFrame::clone() const {

@@ -310,6 +310,25 @@ class ImgFrame : public Buffer, public ProtoSerializable {
*/
ImgFrame& setMetadata(const std::shared_ptr<ImgFrame>& sourceFrame);

/**
* Convience function to set the data of the ImgFrame
Copy link
Preview

Copilot AI May 25, 2025

Choose a reason for hiding this comment

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

Typo in doc comment: 'Convience' should be 'Convenience'.

Copilot uses AI. Check for mistakes.

@lnotspotl lnotspotl changed the base branch from v3_develop to develop June 16, 2025 19:50
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe it would be worth splitting this into files per filter and removing redundant constructors and other warnings brought up by clangd.

@lnotspotl lnotspotl changed the title Feature/extract stereo filters ToF node enhancements, host-runnable ToF and image filters Jul 1, 2025
@lnotspotl
Copy link
Member Author

@SzabolcsGergely This should be, hopefully, the final version of this PR and only minor changes will be made. Could you go over the PR one more time, pointing out deficiencies and potential improvement points?

We will also have @DavidZah (or someone else from the CV team) add in preset-modes for the filter pipelines and test the overall implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants