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

Audio support in RPP - HOST #78

Open
wants to merge 126 commits into
base: ar/audio_support
Choose a base branch
from

Conversation

sampath1117
Copy link

@sampath1117 sampath1117 commented Jun 23, 2022

  1. Added support for below audio functions in RPP for HOST backend
  • Non Silent Region detection
  • Pre Emphasis
  • To Decibels
  • Down Mixing
  • Slice
  • Spectrogram
  • MelFilterBank
  • Resample
  1. Added basic test suite for testing audio functions

made changes to test .wav files for nsr detection
added sample test suite for audio files
added sample audio file in seperate folder
minor changes in the ways weights are being accesed
… region

updated CMakeLists.txt in src/modules to exclude fma compilation flag for audio codes
Copy link
Owner

@r-abishek r-abishek left a comment

Choose a reason for hiding this comment

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

Pls take a look at these changes

include/rppdefs.h Outdated Show resolved Hide resolved
src/modules/cpu/kernel/non_silent_region_detection.hpp Outdated Show resolved Hide resolved
src/modules/cpu/kernel/non_silent_region_detection.hpp Outdated Show resolved Hide resolved
src/modules/cpu/kernel/non_silent_region_detection.hpp Outdated Show resolved Hide resolved
src/modules/cpu/kernel/down_mixing.hpp Outdated Show resolved Hide resolved
src/modules/cpu/kernel/down_mixing.hpp Outdated Show resolved Hide resolved
src/modules/cpu/kernel/down_mixing.hpp Outdated Show resolved Hide resolved
src/modules/cpu/kernel/down_mixing.hpp Show resolved Hide resolved
src/modules/cpu/kernel/down_mixing.hpp Outdated Show resolved Hide resolved
#include <chrono>
#include <complex>

inline Rpp32f reduce_add_ps1(__m256 pSrc)
Copy link
Author

Choose a reason for hiding this comment

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

Can be renamed as rpp_horizontal_add_avx and move to L280 in rpp_cpu_simd.hpp
https://github.com/sampath1117/rpp/blob/sr/audio_augmentations/src/include/cpu/rpp_cpu_simd.hpp#L271

Choose a reason for hiding this comment

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

Done

{
Rpp32f* cosfTemp = cosf + (k * nfft);
Rpp32f* sinfTemp = sinf + (k * nfft);
__m256 pJN = _mm256_set_ps(7,6,5,4,3,2,1,0);
Copy link
Author

Choose a reason for hiding this comment

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

What does JN mean here?
Can we rename it to more intuitive name?

Choose a reason for hiding this comment

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

Done

{
Rpp32f* cosfTemp = cosf + (k * nfft);
Rpp32f* sinfTemp = sinf + (k * nfft);
__m256 pJN = _mm256_set_ps(7,6,5,4,3,2,1,0);
Copy link
Author

@sampath1117 sampath1117 Jan 5, 2023

Choose a reason for hiding this comment

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

Please add space after , Can be changed as
__m256 pJN = _mm256_set_ps(7, 6, 5, 4, 3, 2, 1, 0);

Applicable to all such instances in this file

Choose a reason for hiding this comment

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

Done

Rpp32f* sinfTemp = sinf + (k * nfft);
__m256 pJN = _mm256_set_ps(7,6,5,4,3,2,1,0);
__m256 pMulFactorN = _mm256_set1_ps(k*mulFactor);
__m256 pAddFactorN = _mm256_set1_ps(8);
Copy link
Author

Choose a reason for hiding this comment

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

Can use avx_p8 defined rpp_cpu_simd.hpp
https://github.com/sampath1117/rpp/blob/sr/audio_augmentations/src/include/cpu/rpp_cpu_simd.hpp#L99

Instead of using _mm256_set1_ps(8)

Choose a reason for hiding this comment

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

Done

__m256 pMulFactorN = _mm256_set1_ps(k*mulFactor);
__m256 pAddFactorN = _mm256_set1_ps(8);
Rpp32s i = 0;
for (; i < alignedNfftLength; i += 8) {
Copy link
Author

@sampath1117 sampath1117 Jan 5, 2023

Choose a reason for hiding this comment

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

We can follow current RPP style for brackets for now
bracket in next line like below
for (; i < alignedNfftLength; i += 8)
{

Applicable to all such instances in this file

Choose a reason for hiding this comment

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

Done

__m256 pAddFactorN = _mm256_set1_ps(8);
Rpp32s i = 0;
for (; i < alignedNfftLength; i += 8) {
__m256 pSrc = _mm256_mul_ps(pJN,pMulFactorN);
Copy link
Author

@sampath1117 sampath1117 Jan 5, 2023

Choose a reason for hiding this comment

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

Please add space after ,
Please go through entire file and update instances like this

Choose a reason for hiding this comment

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

Done

*sinfTemp++ = -std::sin(k * i * mulFactor);
}
}
std::vector<Rpp32f> windowFn;
Copy link
Author

Choose a reason for hiding this comment

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

We can add empty lines between 2 logical code blocks similar to here
We are computing sin and cos values from L91 to L102 which is one logical code block and codes from L103 is another code block, So we can add empty line here

Please add empty lines to all such cases in this file

Choose a reason for hiding this comment

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

Done

*windowOutputTemp++ = (*windowFnTemp++) * (*srcPtrWindowTemp++);
}
}
// Generate specgram output
Copy link
Author

Choose a reason for hiding this comment

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

We can add blank line before L164

Choose a reason for hiding this comment

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

Done

if (power == 1)
total = std::sqrt(total);
if (vertical) {
int64_t outIdx = (i * hStride + w);
Copy link
Author

Choose a reason for hiding this comment

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

Please replace int64_t with Rpp64s

Choose a reason for hiding this comment

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

Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
5 participants