NoiseRemoval.cpp

[Paul L]

Who can help me understand this file? I see no names in it besides Dominic Mazzoni. A couple things puzzle me.

[Gale Andrews]

Try saying what puzzles you.


Gale

[Paul L]

1) ApplyFreqSmoothing is called in two places. The call from Cleanup is unreachable because mDoProfile is always false at that point. A mistake?

2) I understand why there is division by 20 calculating mNoiseAttenFactor. I understand why division is by 10 not 20 when computing mSensitivityFactor because it is used in comparison of squared amplitudes. I do not understand why division is by 10 not 20 when computing mOneBlockAttackDecay which affects decay of gain factors that are not squared.

3) Would it make more sense to take geometric means, not arithmetic means, in ApplyFreqSmoothing, since these are multiplicative factors that are being averaged? But there may be zeroes if isolating noise. On the other hand, what Isolate does is questionable. See point 6.

4) In this line:

Code: Select all

   mOutSampleCount += mWindowSize / 2; // what is this for?  Not used when we are getting the profile?

Isn't the simple answer, "to make processSamples progress toward termination" ?

5) In these lines:

Code: Select all

   // Apply gain to FFT
   for (j = 0; j < (mSpectrumSize-1); j++) {
      mFFTBuffer[j*2  ] = mRealFFTs[out][j] * mGains[out][j];
      mFFTBuffer[j*2+1] = mImagFFTs[out][j] * mGains[out][j];
   }
   // The Fs/2 component is stored as the imaginary part of the DC component
   mFFTBuffer[1] = mRealFFTs[out][mSpectrumSize-1] * mGains[out][mSpectrumSize-1];

I believe the result is that mFFTBuffer[0] and mFFTBuffer[1] always become 0 because the corresponding entries of mRealFFTs are always 0. The squares of the untransformed coefficients are in the first and last entries of mSpectrums[out] and were never copied into mRealFFTs. We have lost the signs of ths coefficients by now.

6) The manual says of Isolate:

Select this option to leave just the noise - useful if you want to hear exactly what the Noise Removal effect is removing.

But the effect is not in fact to compute the difference between what would be returned with Remove, and the original signal. Maybe that is proper, because with phase shifts of passed frequencies, the aural impression would not be correct. But perhaps the better way to compute this would be to compute all gain factors as if for noise removal, then apply one minus gain to the frequencies just before the inverse FFT?

7) There are two hidden magic numbers in the code, the FFT window size of 2048 and the 50 millisecond minimum signal time that influences the exact noise detection criterion. Might these make sense as advanced controls?

[Paul L]

8) Is FinishTrack wasting time in case the user canccels?

[steve]

2) I understand why there is division by 20 calculating mNoiseAttenFactor. I understand why division is by 10 not 20 when computing mSensitivityFactor because it is used in comparison of squared amplitudes. I do not understand why division is by 10 not 20 when computing mOneBlockAttackDecay which affects decay of gain factors that are not squared.

What happens if you change it from 10 to 20?

3) Would it make more sense to take geometric means, not arithmetic means, in ApplyFreqSmoothing, since these are multiplicative factors that are being averaged?

What happens if you change it?

4) In this line:

Code: Select all

mOutSampleCount += mWindowSize / 2; // what is this for?  Not used when we are getting the profile?

Isn't the simple answer, "to make processSamples progress toward termination" ?

You're correct, but in NoiseRemoval.h it has a contradictory comment:

Code: Select all

   // Variables that only exist during processing
   WaveTrack            *mOutputTrack;
   sampleCount       mInSampleCount;
   sampleCount       mOutSampleCount;

How would you explain that so that?

But perhaps the better way to compute this would be to compute all gain factors as if for noise removal, then apply one minus gain to the frequencies just before the inverse FFT?

Does that work better?

7) There are two hidden magic numbers in the code, the FFT window size of 2048 and the 50 millisecond minimum signal time that influences the exact noise detection criterion. Might these make sense as advanced controls?

Regarding the FFT window size, a larger window size can give (subjectively) better noise reduction with some material, so yes there is a case for making that an advanced control. On the other hand, many users already find this effect intimidating / too difficult and additional "advanced controls" would make that worse.
(I've not tested the 50 millisecond signal time)

[Robert J. H.]

5) In these lines:

Code: Select all

// Apply gain to FFT
   for (j = 0; j < (mSpectrumSize-1); j++) {
      mFFTBuffer[j*2  ] = mRealFFTs[out][j] * mGains[out][j];
      mFFTBuffer[j*2+1] = mImagFFTs[out][j] * mGains[out][j];
   }
   // The Fs/2 component is stored as the imaginary part of the DC component
   mFFTBuffer[1] = mRealFFTs[out][mSpectrumSize-1] * mGains[out][mSpectrumSize-1];

I believe the result is that mFFTBuffer[0] and mFFTBuffer[1] always become 0 because the corresponding entries of mRealFFTs are always 0. The squares of the untransformed coefficients are in the first and last entries of mSpectrums[out] and were never copied into mRealFFTs. We have lost the signs of ths coefficients by now.

The real part holds DC and Nyquist respectively, it's the imaginary part (sine) that is always 0.
It is a little strange to put DC and Nyquist into one bin pair though. Is it correctly re-allocated in the IFFFT?

[steve]

If I recall (I've not checked the code), the DC component is discarded.

[Robert J. H.]

If I recall (I've not checked the code), the DC component is discarded.

It's correctly handled, as far as I can see.

Another interesting improvement could be to transfer the smoothing factor from being linear to logarithmic.
In other words, it is currently fixed over a certain frequency range. We could try to translate this into bandwidth (in fractions of an octave for instance).
In this way, the upper frequencies are more smoothed than the lower ones.

[Paul L]

The DC and the Nyquist frequencies are not correctly handled. I knew it from the code but I verified it too on a sample in the debugger.

You must look at more code than what I quoted to see why that is. mRealFFTs (and array of arrays) is initialized to 0's in StartNewTrack. It is filled in at FillFirstHistoryWindow after the forward FFT fills mFFTBuffer. Look closely at the loop index and you see the error.

Code: Select all

   for(i = 1; i < (mSpectrumSize-1); i++) {
      mRealFFTs[0][i] = mFFTBuffer[hFFT->BitReversed[i]  ];
      mImagFFTs[0][i] = mFFTBuffer[hFFT->BitReversed[i]+1];
      mSpectrums[0][i] = mRealFFTs[0][i]*mRealFFTs[0][i] + mImagFFTs[0][i]*mImagFFTs[0][i];
      mGains[0][i] = mNoiseAttenFactor;
   }
   // DC and Fs/2 bins need to be handled specially
   mSpectrums[0][0] = mFFTBuffer[0]*mFFTBuffer[0];
   mSpectrums[0][mSpectrumSize-1] = mFFTBuffer[1]*mFFTBuffer[1];

The "special handling" did not remember the coefficients in the place where they are presumed to be when we set up for inverse FFT! And forgot their signs anyway by only storing squares.

So I think Steve is right that DC offset is zeroed, but it looks like an error in the code, not by design.

Though if I look at noise removal results in spectrogram, DC appears not to be zero. But that must be because the lobes still pick up other frequencies.

[Paul L]

Steve, I recall you wrote some help in the wiki about finding good settings for noise removal, but I forget where that was.