c4dm / qm-dsp

A C++ library of functions for DSP and Music Informatics purposes. Used by the QM Vamp Plugins amongst other things.
https://code.soundsoftware.ac.uk/projects/qm-dsp
GNU General Public License v2.0
52 stars 11 forks source link

Crash in FiltFilt (via qm-onsetdetect) #1

Closed x42 closed 4 years ago

x42 commented 7 years ago

qm-dsp's FiltFilt.cpp is not save to call with short length.

https://github.com/c4dm/qm-dsp/blob/master/dsp/signalconditioning/FiltFilt.cpp#L63 src[ (length - 2) - i ] can become negative. Furthermore using unsigned int length and along with length -2 isn't safe.

A backtrace of the issue in Ardour (Ardour 5.8-249-g2513aad1e -- this is a time-stretch shrink of a very short region at 48KSPS, followed by qm-onsetdetect : Ardour > Preferences > Audio > Enable automatic analysis)

RBEffect: source region: position = 60000, start = 0, length = 5888, ancestral_start = 0, ancestral_length = 0, stretch 1, shift 1
StretchCalculator::calculate(): inputDuration 5888, ratio 0.37483, outputDuration 2207 (rounded up to 2303), df size 24, increment 256
NOTE: extreme increment -91 < 0, adjusting
NOTE: extreme increment -40 < 0, adjusting
NOTE: extreme increment -11 < 0, adjusting
NOTE: extreme increment 7 < 47.9783, adjusting
NOTE: extreme increment 21 < 47.9783, adjusting
NOTE: extreme increment 30 < 47.9783, adjusting
NOTE: extreme increment 38 < 47.9783, adjusting
NOTE: extreme increment 44 < 47.9783, adjusting
NOTE: extreme increment -146 < 0, adjusting
NOTE: extreme increment 15 < 47.9783, adjusting
NOTE: extreme increment -162 < 0, adjusting
NOTE: extreme increment -56 < 0, adjusting
NOTE: extreme increment -11 < 0, adjusting
NOTE: extreme increment 13 < 47.9783, adjusting
NOTE: extreme increment 28 < 47.9783, adjusting
NOTE: extreme increment 39 < 47.9783, adjusting
NOTE: extreme increment 46 < 47.9783, adjusting
NOTE: extreme increment -69 < 0, adjusting
NOTE: extreme increment -29 < 0, adjusting
NOTE: extreme increment -5 < 0, adjusting
NOTE: extreme increment 11 < 47.9783, adjusting
NOTE: extreme increment 23 < 47.9783, adjusting
NOTE: extreme increment 32 < 47.9783, adjusting
NOTE: extreme increment 39 < 47.9783, adjusting
NOTE: extreme increment 45 < 47.9783, adjusting
*** WARNING: increment -10 <= 0, rounding to zero
total input increment = 6144 (= 24 chunks), output = 2303, ratio = 0.374837, ideal output 2303
(region total = 2303)
WARNING: OnsetDetector::initialise: Possibly sub-optimal step size for this sample rate: 512 (wanted 557)
WARNING: OnsetDetector::initialise: Possibly sub-optimal block size for this sample rate: 1024 (wanted 1114)

Thread 5 "ardour-5.8.205" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffd712d700 (LWP 22897)]
0x00007fffa05463fc in FiltFilt::process (this=0x7fffcc0393a0, src=0x7fffcc0076a0, dst=0x7fffcc004780, length=5) at ../libs/qm-dsp/dsp/signalconditioning/FiltFilt.cpp:84
warning: Source file is more recent than executable.
84              m_filtScratchIn[ (nExt - nFact) + index++ ] = sampleN - src[ (length - 2) - i ];
(gdb) bt
#0  0x00007fffa05463fc in FiltFilt::process(double*, double*, unsigned int) (this=0x7fffcc0393a0, src=0x7fffcc0076a0, dst=0x7fffcc004780, length=5)
    at ../libs/qm-dsp/dsp/signalconditioning/FiltFilt.cpp:84
#1  0x00007fffa0545b1b in DFProcess::process(double*, double*) (this=0x7fffcc039500, src=0x7fffcc0395e0, dst=0x7fffcc0395b0) at ../libs/qm-dsp/dsp/signalconditioning/DFProcess.cpp:93
#2  0x00007fffa0541d53 in PeakPicking::process(double*, unsigned int, std::vector<int, std::allocator<int> >&) (this=0x7fffd712c340, src=0x7fffcc0395e0, len=5, onsets=std::vector of length 0, capacity 0) at ../libs/qm-dsp/dsp/onsets/PeakPicking.cpp:83
#3  0x00007fffa051d757 in OnsetDetector::getRemainingFeatures() (this=0x7fffcc00bcb0) at ../libs/vamp-plugins/OnsetDetect.cpp:453
#4  0x00007fffeda9f055 in _VampPlugin::Vamp::PluginAdapterBase::Impl::getRemainingFeatures(_VampPlugin::Vamp::Plugin*) () at /usr/lib/x86_64-linux-gnu/libvamp-sdk.so.2
#5  0x00007fffed871150 in _VampHost::Vamp::PluginHostAdapter::getRemainingFeatures() () at /usr/lib/x86_64-linux-gnu/libvamp-hostsdk.so.3
#6  0x00007fffed886a35 in _VampHost::Vamp::HostExt::PluginWrapper::getRemainingFeatures() () at /usr/lib/x86_64-linux-gnu/libvamp-hostsdk.so.3
#7  0x00007fffed886a35 in _VampHost::Vamp::HostExt::PluginWrapper::getRemainingFeatures() () at /usr/lib/x86_64-linux-gnu/libvamp-hostsdk.so.3
#8  0x00007fffed886a35 in _VampHost::Vamp::HostExt::PluginWrapper::getRemainingFeatures() () at /usr/lib/x86_64-linux-gnu/libvamp-hostsdk.so.3
#9  0x00007ffff62afb20 in ARDOUR::AudioAnalyser::analyse(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, ARDOUR::Readable*, unsigned int) (this=0x7fffd712c950, path="/tmp/Ardour6/analysis/1717.qm-onset", src=0x7fff94099050, channel=0) at ../libs/ardour/audioanalyser.cc:150
#10 0x00007ffff693ab64 in ARDOUR::TransientDetector::run(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, ARDOUR::Readable*, unsigned int, std::__cxx11::list<long, std::allocator<long> >&) (this=0x7fffd712c950, path="/tmp/Ardour6/analysis/1717.qm-onset", src=0x7fff94099050, channel=0, results=empty std::__cxx11::list)
    at ../libs/ardour/transient_detector.cc:55
#11 0x00007ffff624d14f in ARDOUR::Analyser::analyse_audio_file_source(boost::shared_ptr<ARDOUR::AudioFileSource>) (src=...) at ../libs/ardour/analyser.cc:115
#12 0x00007ffff624cfca in ARDOUR::Analyser::work() () at ../libs/ardour/analyser.cc:102
#13 0x00007ffff624ccb2 in analyser_work() () at ../libs/ardour/analyser.cc:52
#14 0x00005555562d9f77 in sigc::pointer_functor0<void>::operator()() const (this=0x555556f46e78) at /usr/include/sigc++-2.0/sigc++/functors/ptr_fun.h:77
#15 0x00005555562d7390 in sigc::adaptor_functor<sigc::pointer_functor0<void> >::operator()() const (this=0x555556f46e70) at /usr/include/sigc++-2.0/sigc++/adaptors/adaptor_trait.h:256
#16 0x00005555562d36ab in sigc::internal::slot_call0<sigc::pointer_functor0<void>, void>::call_it(sigc::internal::slot_rep*) (rep=0x555556f46e40)
    at /usr/include/sigc++-2.0/sigc++/functors/slot.h:114
#17 0x00007ffff3d2a52d in  () at /usr/lib/x86_64-linux-gnu/libglibmm-2.4.so.1
#18 0x00007ffff37e53d5 in g_thread_proxy (data=0x555556eef4a0) at ././glib/gthread.c:784
#19 0x00007fffef567424 in start_thread (arg=0x7fffd712d700) at pthread_create.c:333
#20 0x00007fffec28d9bf in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:105
(gdb) print length
$1 = 5
(gdb) print m_ord
$2 = 2
(gdb) print nFact
$3 = 6
(gdb) 
x42 commented 7 years ago

A workaround for this issue can be found in https://github.com/Ardour/ardour/commit/c0c24aff728b067ef948f68cbb9bef63be9324c8

cannam commented 4 years ago

Fixed in commits a1654dc2c1efdc and 4f54fce6dec in June this year. Thank you for the report. Sorry it took so long, and also for taking such a long time, even after the fix, to respond to the issue here!