-
Notifications
You must be signed in to change notification settings - Fork 85
Adding changes for batchPD removal #1597
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
base: develop
Are you sure you want to change the base?
Adding changes for batchPD removal #1597
Conversation
…orJitter, and Water augmentations This commit adds OpenVX kernel wrappers for 6 new RPP tensor augmentations: - GaussianNoise: Adds Gaussian noise with configurable mean and stddev - ShotNoise: Adds Poisson-distributed shot noise with configurable factor - Spatter: Creates spatter effect with configurable color - Log: Computes natural logarithm of input element-wise - ColorJitter: Applies brightness, contrast, hue, and saturation adjustments - Water: Creates water ripple distortion effect with configurable amplitude, frequency, and phase
…essionDistortion, Lut, Posterize, and Solarize augmentations This commit adds vx_rpp OpenVX kernel extensions for the second set of RPP tensor augmentations: - ChannelPermute: Rearranges channels according to a permutation tensor - ColorToGreyscale: Converts color images to greyscale - JpegCompressionDistortion: Applies JPEG compression artifacts - Lut: Applies look-up table transformations - Posterize: Reduces the number of bits per channel - Solarize: Inverts pixels above a threshold
…, and TensorStdDev augmentations This commit adds vx_rpp OpenVX kernel extensions for tensor reduction operations: - TensorSum: Computes the sum of tensor elements - TensorMin: Computes the minimum value across tensor elements - TensorMax: Computes the maximum value across tensor elements - TensorMean: Computes the mean of tensor elements - TensorStdDev: Computes the standard deviation of tensor elements
There was a problem hiding this comment.
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 removes deprecated batchPD (batch partial descriptor) RPP extensions and migrates to the newer tensor-based API (rppt_*). The changes align with RPP PR #657 which removes legacy batchPD support from the underlying library.
Key Changes
- Removed 100+ legacy batchPD kernel implementations and registrations
- Updated tensor operations (Snow, Saturation, Hue, FishEye) to use new
rppt_*APIs - Updated Snow kernel to accept 3 parameters (brightnessCoefficient, snowThreshold, darkMode) instead of 1
- Removed legacy RPP header includes and conditional compilation blocks
- Bumped version from 3.1.1 to 3.1.2
Reviewed changes
Copilot reviewed 73 out of 95 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Snow.cpp | Updated to use rppt_snow_* API with 3 new parameters; added proper GPU memory allocation |
| Saturation.cpp | Migrated from rppi_saturationRGB_batchPD to rppt_saturation; added GPU memory handling |
| Hue.cpp | Migrated from rppi_hueRGB_batchPD to rppt_hue; added GPU memory handling |
| FishEye.cpp | Migrated from rppi_fisheye_batchPD to rppt_fisheye; removed unused dimension tracking |
| kernel_rpp.cpp | Updated vxExtRppSnow signature and removed 1800+ lines of legacy batchPD node creation code |
| internal_publishKernels.cpp | Removed registration of 85+ legacy batchPD kernels |
| vx_ext_rpp_version.h | Bumped patch version from 1 to 2 |
| internal_rpp.h | Removed conditional legacy rppi.h include |
| Multiple batchPD files | Deleted entire legacy implementation files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@SundarRajan98 can you please resolve conflicts |
|
@kiritigowda Resolved all merge conflicts |
|
@kiritigowda This PR is not building the vx_rpp extension if the RPP required version is not satisfied. Only when the corresponding RPP PR is merged, this PR will build the vx_rpp extensions From precheckin CI compile log But the mivisionx tests adds a test for the vx_rpp without checking if the library is present or not. This throws a CMake error |
|
@SundarRajan98 -- VX_RPP is a required lib for MIVisionX, missing this lib has to flag an error. Can this PR build with existing RPP? This is disabling the feature on MIVisionX, it should not matter if corresponding API is absent in RPP? This would be a good if this is not a breaking change. |
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@kiritigowda This is a breaking change in vx_rpp extensions. We are coupling this PR with this RPP PR to be merged together at once. Currently this PR will build without any errors with existing RPP but the tests will throw error due to a missing vx_rpp library. |
|
@SundarRajan98 can you please resolve conflicts? |
| find_package(FFmpeg QUIET) | ||
| endif() | ||
| find_package(rpp 2.2.2 QUIET) | ||
| find_package(rpp 2.3.0 QUIET) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SundarRajan28 please change this to match new rpp version 3.0
| list(APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/../../cmake) | ||
|
|
||
| find_package(rpp 2.2.2 REQUIRED) | ||
| find_package(rpp 2.3.0 REQUIRED) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above
Motivation
This PR removes the batchPD vx_rpp extensions and adds the required CMake changes
Technical Details
This PR needs to be merged alongside the rpp PR #657
Test Plan
Test Result
Submission Checklist