Re: [Audacity-devel] [PATCH] Improvement for Noise Removal effect V5
A free multi-track audio editor and recorder
Brought to you by:
aosiniao
From: Marco D. A. M. <mar...@gm...> - 2010-07-02 22:58:40
|
On Fri, Jul 2, 2010 at 6:52 PM, Richard Ash <ri...@au...> wrote: > On Fri, 2010-07-02 at 18:15 -0300, Marco Diego Aurélio Mesquita wrote: >> > Yes please, I'd like to commit that separately before we wrestle with >> > the sensitivity slider before we look at more complex features like >> > sending to separate tracks. So probably three separate patches each >> > doing one defined thing, which makes them easy to test. Each patch >> > should apply on top of the previous one, but leave the code in a working >> > state after each patch. >> >> Hmmm... I don't know how to "separate" these patches. > > Usually, a combination of editing the patch file (which may need fixing > with rediff from patchutils) and staring from a clean source tree and > making only the changes needed to achieve the first fix, then continue > onwards. This is obviously easier done as you go along rather than all > at the end. Hmmm... Should have known this from the beginning. I'll have to take the longer road now. > > I've done this for the attack / decay changes, which produces this patch > as a minimal change set. Which leaves me with the question: what is it > actually doing to fix the problem? The point is that the old method was working with dB units. When van Baren changed its behaviour to linear[0], he (likely) forgot to convert from dB to linear mOneBlockAttackDecay units. > I can see that the internal variable is made 10 times bigger, which may > help with rounding or granularity (although as the slider only shows to > 0.01, I'm not convinced). The internal variable remains the same. The "10 times bigger" is only done to increase the resolution of the slider so that rounding errors won't happen anymore. > The bit that really puzzles me is this one: > > --- src/effects/NoiseRemoval.cpp (revisão 10509) > +++ src/effects/NoiseRemoval.cpp (cópia de trabalho) > @@ -350,7 +350,7 @@ > mAttackDecayBlocks = 1 + > (int)(mAttackDecayTime * mSampleRate / (mWindowSize / 2)); > mNoiseAttenFactor = pow(10.0, mNoiseGain/20.0); > - mOneBlockAttackDecay = (int)(mNoiseGain / (mAttackDecayBlocks - 1)); > + mOneBlockAttackDecay = pow(10.0, (mNoiseGain / (10.0 * mAttackDecayBlocks))) > ; > mMinSignalBlocks = > (int)(mMinSignalTime * mSampleRate / (mWindowSize / 2)); > if( mMinSignalBlocks < 1 ) > > Why does the power function come in here? I thought we were dealing with > time? mOneBlockAttackDecay is the rate at which the volume should vary by block. It is similiar to a derivative (with relation to time). Note that mAttackDecayBlocks is dependent on time. The power function comes to change a unit that was additive (dB) to one that is multiplicative (linear). Note that the log message[1] says "[...] also change variable definitions from exp(log(a)+log(b)) to a*b for improved performance", so I think he just forgot to convert this variable as was already the case with the other ones. [0] http://code.google.com/p/audacity/source/diff?spec=svn9290&r=9290&format=side&path=/sf-cvs/trunk/audacity-src/src/effects/NoiseRemoval.cpp [1] http://code.google.com/p/audacity/source/detail?r=9290&path=/sf-cvs/trunk/audacity-src/src/effects/NoiseRemoval.cpp |