-
Notifications
You must be signed in to change notification settings - Fork 0
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
DownMix Node and to_decibels augmentations #32
base: swbs_m2/audio/pr5
Are you sure you want to change the base?
Conversation
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.
Please Address review comments
* \param[in] pSrcDims The input tensor of batch size in <tt>unsigned int<tt> containing the roi values for the input. | ||
* \param [out] pDst The output tensor in <tt>\ref VX_TYPE_FLOAT32</tt> format data. | ||
* \param[in] pDstDims The input tensor of batch size in <tt>unsigned int<tt> containing the roi values for the output. | ||
* \param[in] cutOffDB The input scalar in <tt>\ref VX_TYPE_FLOAT32</tt> format containing minimum or cut-off ratio in dB |
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.
Is it dims or ROI, please change name accordingly
For both srcDims and dstDims
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.
Also is it of xywh or ltrb format? For audio
* \param[in] pSrcDims The input tensor of batch size in <tt>unsigned int<tt> containing the roi values for the input. | ||
* \param [out] pDst The output tensor in <tt>\ref VX_TYPE_FLOAT32</tt> format data. | ||
* \param[in] pDstDims The input tensor of batch size in <tt>unsigned int<tt> containing the roi values for the output. | ||
* \param[in] cutOffDB The input scalar in <tt>\ref VX_TYPE_FLOAT32</tt> format containing minimum or cut-off ratio in dB |
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.
Change the supported data types
// Check for input parameters | ||
size_t num_tensor_dims; | ||
STATUS_ERROR_CHECK(vxQueryTensor((vx_tensor)parameters[0], VX_TENSOR_NUMBER_OF_DIMS, &num_tensor_dims, sizeof(num_tensor_dims))); | ||
if (num_tensor_dims < 3) return ERRMSG(VX_ERROR_INVALID_DIMENSION, "validate: Downmix: tensor: #0 dimensions=%lu (must be greater than or equal to 3)\n", num_tensor_dims); |
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.
Should the dims be greater than 3? Modify comments accordingly
} | ||
|
||
//! \brief The kernel target support callback. | ||
// TODO::currently the node is setting the same affinity as context. This needs to change when we have hubrid modes in the same graph |
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.
Change the comment as hybrid
Please check this in Spectrogram and Pre-Emphasis filter PR as well @swetha097 Please note
else | ||
supported_target_affinity = AGO_TARGET_AFFINITY_CPU; | ||
|
||
// hardcode the affinity to CPU for OpenCL backend to avoid VerifyGraph failure since there is no codegen callback for amd_rpp nodes |
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.
Is this comment applicable here, Please check this
@swetha097 Please check in other PR's too
} | ||
|
||
return status; | ||
} |
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.
Add new line at EOF
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 address this
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.
Please address review comments
Also Rajy is suggesting to use uint32_t for unsigned and int32_t for int where it is used in MIVisionX for new PR's
Please do that change also in this PR for new functions added
@@ -2597,6 +2632,27 @@ RpptDataType getRpptDataType(vx_enum vxDataType) { | |||
} | |||
} | |||
|
|||
RpptLayout getRpptLayout(vxTensorLayout layout) { |
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.
Rajy is suggesting to have this a map, instead of switch case
ROCm#1320 (comment)
Please remove this function and add a constant map here, and use this map in openvx extensions
@@ -2663,6 +2719,7 @@ void fillAudioDescriptionPtrFromDims(RpptDescPtr &descPtr, size_t *maxTensorDims | |||
descPtr->strides.wStride = descPtr->c; | |||
descPtr->strides.cStride = 1; | |||
descPtr->numDims = 4; | |||
descPtr->layout = getRpptLayout(layout); |
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.
Use map here for the layout
STATUS_ERROR_CHECK(vxQueryTensor((vx_tensor)parameters[2], VX_TENSOR_BUFFER_HOST, &roi_tensor_ptr_src, sizeof(roi_tensor_ptr_src))); | ||
} | ||
RpptROI *src_roi = reinterpret_cast<RpptROI *>(roi_tensor_ptr_src); | ||
for (int n = 0; n < data->inputTensorDims[0]; n++) { |
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.
Restructure the loop as below so that we have multiplication as part of loop counter
for (int n = 0, k = 0; n < data->inputTensorDims[0]; n++, k += 2)
{
data->psrcRoi[k] = src_roi[n].xywhROI.roiWidth;
data->psrcRoi[k + 1] = src_roi[n].xywhROI.roiHeight;
}
Rpp32u deviceType; | ||
RppPtr_t pSrc; | ||
RppPtr_t pDst; | ||
vx_int32 *psrcRoi; |
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.
Please change psrcRoi to pSrcRoi
#endif | ||
} else if (data->deviceType == AGO_TARGET_AFFINITY_CPU) { | ||
refreshDownmix(node, parameters, num, data); | ||
rpp_status = rppt_down_mixing_host((float *)data->pSrc, data->pSrcDesc, (float *)data->pDst, data->pDstDesc, (Rpp32s *)data->psrcRoi, false, data->handle->rppHandle); |
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.
Please remove this casting to (float *) since it is not needed
|
||
static vx_status VX_CALLBACK initializeDownmix(vx_node node, const vx_reference *parameters, vx_uint32 num) { | ||
DownmixLocalData *data = new DownmixLocalData; | ||
if (data) { |
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.
Since we are allocating the memory anyway at L106 and it is a simple structure
why do we need to check if (data) and return vx_FAILURE if not?
fillAudioDescriptionPtrFromDims(data->pDstDesc, data->outputTensorDims); | ||
|
||
data->psrcRoi = new vx_int32[data->pSrcDesc->n * 2]; | ||
|
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.
Remove one blank line
either at L130/L132
* Bump rocm-docs-core[api_reference] from 0.34.0 to 0.34.2 in /docs/sphinx (ROCm#1286) Bumps [rocm-docs-core[api_reference]](https://github.com/RadeonOpenCompute/rocm-docs-core) from 0.34.0 to 0.34.2. - [Release notes](https://github.com/RadeonOpenCompute/rocm-docs-core/releases) - [Changelog](https://github.com/ROCm/rocm-docs-core/blob/develop/CHANGELOG.md) - [Commits](ROCm/rocm-docs-core@v0.34.0...v0.34.2) --- updated-dependencies: - dependency-name: rocm-docs-core[api_reference] dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump cryptography from 42.0.0 to 42.0.2 in /docs/sphinx (ROCm#1289) Bumps [cryptography](https://github.com/pyca/cryptography) from 42.0.0 to 42.0.2. - [Changelog](https://github.com/pyca/cryptography/blob/main/CHANGELOG.rst) - [Commits](pyca/cryptography@42.0.0...42.0.2) --- updated-dependencies: - dependency-name: cryptography dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Add PreEmphasis filter support * Fix ROI - change from xy to width & height * Adding openvx changes for downmix node * Audio Augmentations 1 PR - NSR and Spectrogram * Revert "Adding openvx changes for downmix node" This reverts commit d53f81d. * PR comments resolution in the PreEmphais Filter * Minor Changes * Change the borderType enum to int32 from uint32 dtype * Fix validation of preemphasis * Remove the memcopy of the src and dest rois as it can be handled in the rocAL - since the src and dst rois remain same * Formatting change - minor * Remove NSR * Minor formatting changes * Minor fix * Minor update - remove the 2nd instance of preemphasis filter * Enum dtype - change from uint to int * Remove roi_tensor_ptr_dst as its unused after latest changes * Remove the dst_roi arg from vxExtRppPreemphasisFilter call as its unused * Add MFB to MIVisisonX * Revert "Add MFB to MIVisisonX" This reverts commit dc4200b. * Resolve the PR comments * Change the dims[0] and dims[1] positioning for Spectrogram and AudioFillDescPointers * Change function name to camelCase * Revert "Change the dims[0] and dims[1] positioning for Spectrogram and AudioFillDescPointers" This reverts commit 886d6af. * Fix Spectrogram * Docs - update TOC for API Ref (ROCm#1327) * Bump rocm-docs-core[api_reference] from 0.38.0 to 0.38.1 in /docs/sphinx (ROCm#1328) Bumps [rocm-docs-core[api_reference]](https://github.com/RadeonOpenCompute/rocm-docs-core) from 0.38.0 to 0.38.1. - [Release notes](https://github.com/RadeonOpenCompute/rocm-docs-core/releases) - [Changelog](https://github.com/ROCm/rocm-docs-core/blob/develop/CHANGELOG.md) - [Commits](ROCm/rocm-docs-core@v0.38.0...v0.38.1) --- updated-dependencies: - dependency-name: rocm-docs-core[api_reference] dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Update CHANGELOG.md * Update CHANGELOG.md * Documents - Bump idna from 3.4 to 3.7 in /docs/sphinx (ROCm#1330) Bumps [idna](https://github.com/kjd/idna) from 3.4 to 3.7. - [Release notes](https://github.com/kjd/idna/releases) - [Changelog](https://github.com/kjd/idna/blob/master/HISTORY.rst) - [Commits](kjd/idna@v3.4...v3.7) --- updated-dependencies: - dependency-name: idna dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Update changelog * Resolve minor PR comments * Remove comments * Docs - Bump tqdm from 4.65.0 to 4.66.3 in /docs/sphinx (ROCm#1339) Bumps [tqdm](https://github.com/tqdm/tqdm) from 4.65.0 to 4.66.3. - [Release notes](https://github.com/tqdm/tqdm/releases) - [Commits](tqdm/tqdm@v4.65.0...v4.66.3) --- updated-dependencies: - dependency-name: tqdm dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Docs - Bump jinja2 from 3.1.3 to 3.1.4 in /docs/sphinx (ROCm#1340) Bumps [jinja2](https://github.com/pallets/jinja) from 3.1.3 to 3.1.4. - [Release notes](https://github.com/pallets/jinja/releases) - [Changelog](https://github.com/pallets/jinja/blob/main/CHANGES.rst) - [Commits](pallets/jinja@3.1.3...3.1.4) --- updated-dependencies: - dependency-name: jinja2 dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Find Half - Fix (ROCm#1341) * MIVisionX Setup - Updates (ROCm#1343) * SWDEV-459739 - Remove the package obsolete setting (ROCm#1345) The package was obsoleting itself and was causing upgrade issues. Removed the same. * Fix the layout issue with spec * Add layouts for Audio in vxTensorLayout Remove spectrogram layout param and pass layout in descriptor * Check the validity of pointers * Audio PR - Augmentation support [ Spectrogram ] (ROCm#1319) * Bump rocm-docs-core[api_reference] from 0.34.0 to 0.34.2 in /docs/sphinx (ROCm#1286) Bumps [rocm-docs-core[api_reference]](https://github.com/RadeonOpenCompute/rocm-docs-core) from 0.34.0 to 0.34.2. - [Release notes](https://github.com/RadeonOpenCompute/rocm-docs-core/releases) - [Changelog](https://github.com/ROCm/rocm-docs-core/blob/develop/CHANGELOG.md) - [Commits](ROCm/rocm-docs-core@v0.34.0...v0.34.2) --- updated-dependencies: - dependency-name: rocm-docs-core[api_reference] dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Bump cryptography from 42.0.0 to 42.0.2 in /docs/sphinx (ROCm#1289) Bumps [cryptography](https://github.com/pyca/cryptography) from 42.0.0 to 42.0.2. - [Changelog](https://github.com/pyca/cryptography/blob/main/CHANGELOG.rst) - [Commits](pyca/cryptography@42.0.0...42.0.2) --- updated-dependencies: - dependency-name: cryptography dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Add PreEmphasis filter support * Fix ROI - change from xy to width & height * Adding openvx changes for downmix node * Audio Augmentations 1 PR - NSR and Spectrogram * Revert "Adding openvx changes for downmix node" This reverts commit d53f81d. * PR comments resolution in the PreEmphais Filter * Minor Changes * Change the borderType enum to int32 from uint32 dtype * Fix validation of preemphasis * Remove the memcopy of the src and dest rois as it can be handled in the rocAL - since the src and dst rois remain same * Formatting change - minor * Remove NSR * Minor formatting changes * Minor fix * Minor update - remove the 2nd instance of preemphasis filter * Enum dtype - change from uint to int * Remove roi_tensor_ptr_dst as its unused after latest changes * Remove the dst_roi arg from vxExtRppPreemphasisFilter call as its unused * Add MFB to MIVisisonX * Revert "Add MFB to MIVisisonX" This reverts commit dc4200b. * Resolve the PR comments * Change the dims[0] and dims[1] positioning for Spectrogram and AudioFillDescPointers * Change function name to camelCase * Revert "Change the dims[0] and dims[1] positioning for Spectrogram and AudioFillDescPointers" This reverts commit 886d6af. * Fix Spectrogram * Docs - update TOC for API Ref (ROCm#1327) * Bump rocm-docs-core[api_reference] from 0.38.0 to 0.38.1 in /docs/sphinx (ROCm#1328) Bumps [rocm-docs-core[api_reference]](https://github.com/RadeonOpenCompute/rocm-docs-core) from 0.38.0 to 0.38.1. - [Release notes](https://github.com/RadeonOpenCompute/rocm-docs-core/releases) - [Changelog](https://github.com/ROCm/rocm-docs-core/blob/develop/CHANGELOG.md) - [Commits](ROCm/rocm-docs-core@v0.38.0...v0.38.1) --- updated-dependencies: - dependency-name: rocm-docs-core[api_reference] dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Update CHANGELOG.md * Update CHANGELOG.md * Documents - Bump idna from 3.4 to 3.7 in /docs/sphinx (ROCm#1330) Bumps [idna](https://github.com/kjd/idna) from 3.4 to 3.7. - [Release notes](https://github.com/kjd/idna/releases) - [Changelog](https://github.com/kjd/idna/blob/master/HISTORY.rst) - [Commits](kjd/idna@v3.4...v3.7) --- updated-dependencies: - dependency-name: idna dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Update changelog * Resolve minor PR comments * Remove comments * Fix the layout issue with spec * Check the validity of pointers --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: root <root@ixt-sjc2-54.local.lan> Co-authored-by: Swetha B S <swetha@mutlicorewareinc.com> Co-authored-by: SundarRajan28 <sundarrajan@multicorewareinc.com> Co-authored-by: Swetha B S <swetha@multiocrewareinc.com> Co-authored-by: randyh62 <42045079+randyh62@users.noreply.github.com> * Introduce API to obtain RPP layout * Add comments * Use RPP_AUDIO flag to disable RPP audio calls * Add Audio flag for PreEmphasis filter --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: root <root@ixt-sjc2-54.local.lan> Co-authored-by: Swetha B S <swetha@mutlicorewareinc.com> Co-authored-by: Swetha B S <swetha@multicorewareinc.com> Co-authored-by: SundarRajan28 <sundarrajan@multicorewareinc.com> Co-authored-by: Swetha B S <swetha@multiocrewareinc.com> Co-authored-by: randyh62 <42045079+randyh62@users.noreply.github.com> Co-authored-by: swetha097 <59434434+swetha097@users.noreply.github.com> Co-authored-by: Kiriti Gowda <kiritigowda@gmail.com> Co-authored-by: raramakr <91213141+raramakr@users.noreply.github.com> Co-authored-by: Sundar Rajan Vaithiyanathan <99159823+SundarRajan28@users.noreply.github.com> Co-authored-by: Lakshmi Kumar <lakshmi.kumar@amd.com>
No description provided.