[Speex-dev] Possible fixed point overflow/div 0 preprocess.c

[Peter Rowling]

Hi,

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

Description:

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>
...
   st->nb_adapt++;
   st->min_count++;
   
   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)
((spx_word16_t)(((spx_word32_t)(a))/((spx_word16_t)(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;
   else
      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 */ )
       st->nb_adapt++;
...
</>

Thanks.

Peter Rowling