GNOME Bugzilla – Bug 159211
[audioscale] instance can only scale once
Last modified: 2006-01-12 09:43:51 UTC
I'm resampling stereo audio at 44100 to 8000 Hz and valgrind shows invalid reads in libresample. This is a crash waiting to happen. ==18655== Thread 2: ==18655== Invalid read of size 8 ==18655== at 0x1C12B043: functable_fir2 (functable.c:214) ==18655== by 0x1C12C588: gst_resample_sinc_ft_s16 (resample.c:555) ==18655== by 0x1C12B90D: gst_resample_scale (resample.c:211) ==18655== by 0x1C1257D6: gst_audioscale_chain (gstaudioscale.c:656) ==18655== Address 0x1C2F1CB8 is 16 bytes before a block of size 4608 alloc'd ==18655== at 0x1B905EDD: malloc (vg_replace_malloc.c:131) ==18655== by 0x1C12B7E3: gst_resample_scale (resample.c:187) ==18655== by 0x1C1257D6: gst_audioscale_chain (gstaudioscale.c:656) ==18655== by 0x1B94BF63: gst_pad_call_chain_function (gstpad.c:4402) ==18655== ==18655== Thread 2: ==18655== Invalid read of size 8 ==18655== at 0x1C12B05D: functable_fir2 (functable.c:215) ==18655== by 0x1C12C588: gst_resample_sinc_ft_s16 (resample.c:555) ==18655== by 0x1C12B90D: gst_resample_scale (resample.c:211) ==18655== by 0x1C1257D6: gst_audioscale_chain (gstaudioscale.c:656) ==18655== Address 0x1C2F1CC0 is 8 bytes before a block of size 4608 alloc'd ==18655== at 0x1B905EDD: malloc (vg_replace_malloc.c:131) ==18655== by 0x1C12B7E3: gst_resample_scale (resample.c:187) ==18655== by 0x1C1257D6: gst_audioscale_chain (gstaudioscale.c:656) ==18655== by 0x1B94BF63: gst_pad_call_chain_function (gstpad.c:4402)
In gst_resample_sinc_ft_s16(), the function functable_fir2() is called with data = ptr + (start + r->filter_length) * 2 which equals ptr - 2 So the value for the variable "start" is wrong. (start=-33, filter_length=32) Could it be because the algorithm uses floor() on a negative number (maybe ceil() was intended)?
Created attachment 34169 [details] [review] ugly fix Make sure we never try to read before the buffer. This isn't the right fix, but it gets rid of the invalid reads which caused some errors in the resampled stream.
Seems the resampler gives bad results in upsampling with the current "scale" value. Also, the functable is a static variable and is initialized only once even tough it depends on the filter_length and the resampling ratio. This is bad for an application that wants to resample many different streams.
*** Bug 160179 has been marked as a duplicate of this bug. ***
The patch from Sebastien fixes my bug 160179.
Sebastien, can audioscale "reset" the resampler on statechange from PAUSED to READY? That would fix the issue raised in comment 3.
Well, you would need to add a functable_close() in functable.c and this would be called from gst_resample_close(). The functable should also be part of the gst_resample structure instead of being static. But I see that David Schleef has started a new resampling filter (and his new library has functable_free() ;-). I guess we shouldn't waste too much time fixing this one.
I don't want to depend on external libraries for each and every scratch.
Patch committed. I'll leave the bug open for Sebastien concern raised in comment 3.
any thoughts on comment 3 ? Maybe it's better to create a new bug and close this one ?
Closing as obsolete, audioscale was replaced by audioresample in 0.10.