[Speex-dev] Possible fixed point overflow/div 0 preprocess.c
jean-marc.valin at usherbrooke.ca
Mon Sep 17 21:45:57 PDT 2007
Thanks a lot for posting this bug report. There's indeed a problem with
the fixed-point code, though not in the floating point (it turns out
that "floating-point exception" is very misleading as it only occurs on
*integer* divisions, not float). Anyway, so the fix should be fairly
simple. I'll fix that in the next few days -- flame me if I forget.
Peter Rowling wrote:
> I'll try to keep this as brief, yet descriptive enough to save everyone
> some time.
> If I'm off with this one please forgive me, but it has fixed my issues.
> I am not
> sure whether it is just my compiler (gcc 3.3.5) doing the wrong thing
> with the cast.
> File: preprocess.c
> Arch affected: x86, (others?)
> svn revision: 12778
> The SpeexPreprocessState_ member 'nb_adapt' is a signed integer.
> nb_adapt is only
> modified during 'speex_preprocess_run' when an unbounded increment is
> performed. When
> calculating 'beta' within 'speex_preprocess_run', I received a floating
> point exception
> even when compiled with -DFIXED_POINT. The reason is as follows:
> <original beta calculation within speex_preprocess_run>
> beta = MAX16(QCONST16(.03,15),DIV32_16(Q15_ONE,st->nb_adapt));
> beta_1 = Q15_ONE-beta;
> On my architecture(x86) at least the DIV32_16 is defined as follows.
> typedef short spx_int16_t;
> typedef spx_int16_t spx_word16_t;
> #define DIV32_16(a,b)
> The nb_adapt overflow occurred when its value reached 0x00010000 when
> the cast is
> performed to a short the value 0x0000 is given and hence the divide by
> zero exception.
> Proposed resolution:
> The SpeexPreprocessState_ member 'nb_adapt' is only checked in a couple
> of places and
> mostly for initial conditions such as within update_noise_prob below:
> <preprocess.c update_noise_prob>
> if (st->nb_adapt==1)
> for (i=0;i<N;i++)
> st->Smin[i] = st->Stmp[i] = 0;
> if (st->nb_adapt < 100)
> min_range = 15;
> else if (st->nb_adapt < 1000)
> min_range = 50;
> else if (st->nb_adapt < 10000)
> min_range = 150;
> min_range = 300;
> Maybe something like this is required within speex_preprocess_run to
> prevent the observed overflow.
> <preprocess.c speex_preprocess_run>
> if ( st->nb_adapt < 0x7FFF /* or 32767 as elsewhere in the code */ )
> Peter Rowling
> Speex-dev mailing list
> Speex-dev at xiph.org
More information about the Speex-dev