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

Impulse perturbation normalization typo #6429

Closed
tabasy opened this issue Apr 14, 2023 · 2 comments
Closed

Impulse perturbation normalization typo #6429

tabasy opened this issue Apr 14, 2023 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@tabasy
Copy link

tabasy commented Apr 14, 2023

As this is my first issue, I MUST thank anyone contributing to this great repository, so I do :).

Without any context, this normalization line seems wrong (first min should be np.mean).
impulse_norm = (impulse.samples - min(impulse.samples)) / (max(impulse.samples) - min(impulse.samples))

This totally changes the RIR signal and adds an overlapping echo to the output. Seems like nobody is using this augmentation :) or they didn't manually check the outputs.

An example RIR file from BUT reverb dataset is attached for anyone like myself who seeks a RIR example to check the augmentation.
rir_1sec.zip

@tabasy tabasy added the bug Something isn't working label Apr 14, 2023
@anteju
Copy link
Collaborator

anteju commented Apr 25, 2023

@tabasy, thank you for reporting this issue.
I agree, subtraction of min appears to be incorrect.
We'll take a look at this and open a PR with a fix.

@anteju
Copy link
Collaborator

anteju commented May 10, 2023

This issue has been fixed in PR #6505

@anteju anteju closed this as completed May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants