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

[Fix] Use number of channels when calculating BAN #37

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

anteju
Copy link

@anteju anteju commented Mar 20, 2023

It seems the current implementation is missing a scaling by $M^{-1/2}$ when calculating BAN.
This results in a gain of $10 \log_{10} M~\text{dB}$, which sometimes results in clipping depending on $M$ and the input signal level.

Please refer to eq. (17) in Warsitz, Blind Acoustic Beamforming Based on Generalized Eigenvalue Decomposition, 2007.

@anteju
Copy link
Author

anteju commented Mar 20, 2023

@boeddeker, possibly of interest -- this seems to apply to pb_* repos as well.

@desh2608
Copy link
Owner

@boeddeker since you are the expert on this, I will defer to your opinion.

@boeddeker
Copy link
Contributor

Yes, it is missing. It was already missing, when we translated the MATLAB code.
I checked it once, but with [1, 0, 0, ...] as beamformer, then the function works as expected, but when you change the beamformer to [1, 1, 1, ...], the scale is too large.

Depending on your application, you may want to think about a normalization before writing files to the disk.
For ASR we observed until now, always positive effects, hence we never had any issue with this scaling error, since we remove the scale afterward.

@desh2608
Copy link
Owner

I will keep this PR open (for visibility). As pointed out by @boeddeker, it does not seem to impact ASR much. For the CHiME-7 DASR challenge, participants can choose whether or not they want to apply it in their system.

@anteju
Copy link
Author

anteju commented Mar 21, 2023

@boeddeker & @desh2608, I just wanted to let you know, up to you whether to include it or not.
Since you mentioned CHiME: processed audio is saved to fixed point format and some examples are clipped.
This likely does not impact ASR significantly. However, it is nevertheless incorrect.

@desh2608
Copy link
Owner

Thanks for the heads up, in any case.

@boeddeker
Copy link
Contributor

processed audio is saved to fixed point format and some examples are clipped.

This depends on how you dump the data to the disk. We use internally a normalization, before writing an audio file
(see paderbox.io.dump_audio) to minimize the quantization issue.

@desh2608
Copy link
Owner

@popcornell could this explain some of the clipping issues you had observed, or were you able to resolve them?

@popcornell
Copy link
Contributor

popcornell commented Mar 22, 2023

I think they will possibly still occur because there is also clipping in some arrays in CHiME-6.
Only way to prevent it is using peak normalization when the peak is outside [-1, 1].
But it also reduces dynamic range and also that could have an impact.

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

4 participants