Skip to content
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

Add median blur operator #4950

Merged
merged 4 commits into from
Aug 1, 2023
Merged

Conversation

banasraf
Copy link
Collaborator

@banasraf banasraf commented Jul 19, 2023

Category:

New feature

Description:

It adds a new operator. It's a proof of concept for using cv-cuda ops in DALI.

Additional information:

Affected modules and functionalities:

New operator was added. I also included small change to improve support for uint16 in DALI.

The operator is mostly a thin layer around the cv-cuda op.
There are two parts that require any logic:

  • converting TensorLists to cvcuda types. This also includes reinterpreting sequences and CHW images as batches of HWC images.
  • setting up the ksize tensor. cvcuda op requires its arguments to be all device tensors, so the argument in DALI has to transferred to gpu (and transformed to tensor witch a proper number of elements).

Key points relevant for the review:

The operator

Tests:

  • Existing tests apply
  • New tests added
    • Python tests
    • GTests
    • Benchmark
    • Other
  • N/A

Checklist

Documentation

  • Existing documentation applies
  • Documentation updated
    • Docstring
    • Doxygen
    • RST
    • Jupyter
    • Other
  • N/A

DALI team only

Requirements

  • Implements new requirements
  • Affects existing requirements
  • N/A

REQ IDs: N/A

JIRA TASK: DALI-3559

@banasraf
Copy link
Collaborator Author

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [9028939]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [9028939]: BUILD FAILED

@@ -122,6 +122,8 @@ DEPS_LIST=(
"${DEPS_PATH}/lib/libzstd.so.1"
"${DEPS_PATH}/lib/libz.so.1"
"${DEPS_PATH}/lib/libcfitsio.so.4"
"lib/libcvcuda.so.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you know if there is any symbol versioning in CV-CUDA? In the case of FFmpeg we append DALI_ prefix to symbols so they won't clash with the symbols from any other FFmpeg build.
I'm just afraid of what may happens when the user will do:

import nvidia.dali```
and I suspect that symbols from the first cvcuda are going to be used.

@banasraf
Copy link
Collaborator Author

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [9039982]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [9039982]: BUILD FAILED

@banasraf
Copy link
Collaborator Author

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [9060724]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [9060724]: BUILD PASSED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [9071120]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [9071120]: BUILD FAILED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [9099699]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [9100969]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [9099699]: BUILD FAILED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [9100969]: BUILD FAILED

Signed-off-by: Rafal Banas <rbanas@nvidia.com>
Signed-off-by: Rafal Banas <rbanas@nvidia.com>
@banasraf
Copy link
Collaborator Author

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [9149266]: BUILD STARTED

@banasraf banasraf marked this pull request as ready for review July 28, 2023 08:58
@banasraf
Copy link
Collaborator Author

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [9150286]: BUILD STARTED

Signed-off-by: Rafal Banas <rbanas@nvidia.com>
@banasraf
Copy link
Collaborator Author

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [9150315]: BUILD STARTED

dali/operators/image/filter/median_blur.cc Outdated Show resolved Hide resolved
add_subdirectory(convolution)
add_subdirectory(distortion)
if (${BUILD_CVCUDA})
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a CMake option? If so, shouldn't it be if (BUILD_CVCUDA) ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It should be. Done

Signed-off-by: Rafal Banas <rbanas@nvidia.com>
@dali-automaton
Copy link
Collaborator

CI MESSAGE: [9149266]: BUILD FAILED

@banasraf
Copy link
Collaborator Author

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [9150739]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [9150315]: BUILD PASSED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [9150739]: BUILD FAILED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [9150739]: BUILD PASSED

@banasraf banasraf merged commit ee44110 into NVIDIA:main Aug 1, 2023
5 checks passed
@JanuszL JanuszL mentioned this pull request Sep 6, 2023
JanuszL pushed a commit to JanuszL/DALI that referenced this pull request Oct 13, 2023
Signed-off-by: Rafal Banas <rbanas@nvidia.com>
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.

None yet

5 participants