[Speex-dev] innov_save, what is it? why does it hurt me so?

[Jim Crichton]

This must have been an enormous pain to track down.

The manual alloc routine in the TI directory (user_misc.h) clears the 
allocated memory, but maybe you have changed this.

>> it will just start filling data in, which it shouldn't.  I see that
>> innov_save is set at the beginning of a for loop at:
>> for (sub=0;sub<st->nbSubframes;sub++)
>>    {
> ...
>>       spx_word16_t *innov_save = NULL;
>>       spx_word16_t tmp;...
>
>> but for some reason that never made a difference even though I know I
>> stepped over it in the code.

That is because this is a local variable used only within this loop, and is 
different from the field of the same name in the decoder state.
st->innov_save is initialized only in sb_celp.c, which is not in your build.

The big lesson here is that Speex depends upon all allocated memory being 
zeroed.  This is to your advantage in the long run, because it saves code 
size by eliminating all those individual sets to zero.  There probably are 
not very many variables that have this dependence, but it is best to assume 
that any variable might, and clear all allocated memory.

- Jim

----- Original Message ----- 
From: "Jean-Marc Valin" <jean-marc.valin at usherbrooke.ca>
To: "Michael Jacobson" <Michael.Jacobson at ultratec.com>
Cc: <speex-dev at xiph.org>
Sent: Thursday, September 13, 2007 7:40 PM
Subject: Re: [Speex-dev] innov_save, what is it? why does it hurt me so?


>> I have been trying to get speex to work for a while now, and it's been
>> a real teeter-totter ride.  For a long time I noticed that I will get a
>> project to work and then without changing any code and programming it to
>> an eprom/flash the project will not work.
>
> That's an Heisenbug and the most common cause is uninitialised data.
>
> I found innov_save will write over memory it shouldn't be when
>> I fill memory with 1's but not when I fill it with zeros.
>
> Which is normal, see below.
>
>>  When my DSP
>> gets to this code chunk:
>> if (innov_save){
>>    for (i=0;i<st->subframeSize;i++)
>>       innov_save[i] = EXTRACT16(PSHR32(innov[i], SIG_SHIFT));
>> }
>
> This bit of code is for copying the innov variable to a buffer owned by
> the wideband decoder -- but only if there is actually a wideband decoder.
>
>> it will just start filling data in, which it shouldn't.  I see that
>> innov_save is set at the beginning of a for loop at:
>> for (sub=0;sub<st->nbSubframes;sub++)
>>    {
> ...
>>       spx_word16_t *innov_save = NULL;
>>       spx_word16_t tmp;...
>
>> but for some reason that never made a difference even though I know I
>> stepped over it in the code.
>
> The reason is doesn't make a difference is that just a few lines later,
> you have:
>
>      if (st->innov_save)
>         innov_save = st->innov_save+offset;
>
> It means that the innov variable gets copied *unless* st->innov_save
> (notice is a field of the encoder struct). So the problem is that this
> field is probably not set to NULL. Normally, this is automatically set
> to zero when the init function does:
> st = (EncState*)speex_alloc(sizeof(EncState));
>
> Is it possible that you did an override of speex_alloc and replaced it
> by malloc() instead of calloc() as I'm using? That would explain the
> garbage you're getting and it may actually explain other bugs you've had.
>
> Jean-Marc
>
>> I commented innov save out for now but I
>> don't know if there is some case where I might need it.  What does it do
>> and do I need it?  Are there any more similar data pointers like that
>> that you know of that may cause similar issues?
>
> In any case, about innov_save in general, you can disable all of that if
> you're not using wideband.
>
> Jean-Marc
>
> _______________________________________________
> Speex-dev mailing list
> Speex-dev at xiph.org
> http://lists.xiph.org/mailman/listinfo/speex-dev
>