search

xiph / speexdsp

Speex audio processing library - THIS IS A MIRROR, DEVELOPMENT HAPPENS AT https://gitlab.xiph.org/xiph/speexdsp
https://speex.org
Other
490 stars 191 forks source link

loudness_accum never initialized? #34

Closed seantalts closed 2 years ago

seantalts commented 3 years ago

Hey there,

I was just taking a look trying to understand how the automated gain control works and did not understand how loudness_accum works in particular - it seems like it does not receive a value before being used here https://github.com/xiph/speexdsp/blob/64cbfa9bca7479a758351aa02bb4abdd76baa9e7/libspeexdsp/preprocess.c#L585

tmatth commented 3 years ago

Good catch, it looks like this has been the case since loundess_accum was added back in: https://gitlab.xiph.org/xiph/speexdsp/-/commit/73851dc56983ffaae14d61f881930cc24b0da893k

From a cursory look, I'm guessing this was intended to be initialized to 0.0 in speex_preprocess_state_init (since a small 1e-4 is added to it when it's used as a denominator, this should be safe) but @jmvalin can confirm.

You can submit a merge request to fix this here if you like: https://gitlab.xiph.org/xiph/speexdsp

Note: this was introduced back before speex and speexdsp were split into two separate libraries.

santaryan commented 3 years ago

The default speex_alloc function clears memory during allocation, and explicitly declares that any overriding allocator must also zeroize memory before returning the block. As a side effect, this means that loudness_accum is zeroized by default (an IEEE-754 float with a value of 0 has a binary representation of all 0). While it is good practice to initialize the structure in totality, it is not strictly necessary in this case due to the implementation of the allocation function.

tmatth commented 2 years ago

As per @santaryan's explanation, as speex_alloc is calloc by default, closing.