View Issue Details

IDCategoryLast Update
0006994bugs2016-08-29 21:45
ReporterzeogradAssigned To 
Reproducibilitysometimes 
Status newResolutionopen 
PlatformOSDebian x86_64OS Version8
Product Version5.X git (version in description) 
Fixed in Version 
Summary0006994: Crash in FiltFilt::process on very small time stretches
Descriptionon Ardour git df4e6c4fccd320bf99823980520ee052f748605e (at least), when doing a very small time stretching we can encounter a crash.

As shown in backtrace, the crash happens in FiltFilt::process, where there is a buffer underflow.
Steps To ReproduceIn my session at least, timestretches from region slightly smaller than 1s ending in stretched region slightly above 1s easily trigger this issue.
TagsNo tags attached.

Activities

zeograd

2016-08-28 20:11

reporter  

ardour_bt.txt (3,015 bytes)
#0  0x00007fffb0b48fb0 in FiltFilt::process (this=0x7fffd0024040, 
    src=0x7fffd0024910, dst=0x7fffd0024950, length=6)
    at ../libs/qm-dsp/dsp/signalconditioning/FiltFilt.cpp:84
#1  0x00007fffb0b4824f in DFProcess::process (this=0x7fffd00248a0, 
    src=0x7fffd0024a10, dst=0x7fffd00249d0)
    at ../libs/qm-dsp/dsp/signalconditioning/DFProcess.cpp:82
#2  0x00007fffb0b4211f in PeakPicking::process (this=0x7fffd9ccf3c0, 
    src=0x7fffd0024a10, len=6, onsets=std::vector of length 0, capacity 0)
    at ../libs/qm-dsp/dsp/onsets/PeakPicking.cpp:74
#3  0x00007fffb0d965e2 in OnsetDetector::getRemainingFeatures (
    this=0x7fffd0022030) at ../libs/vamp-plugins/OnsetDetect.cpp:456
#4  0x00007fffee16bd95 in _VampPlugin::Vamp::PluginAdapterBase::Impl::getRemainingFeatures(_VampPlugin::Vamp::Plugin*) ()
   from /usr/lib/x86_64-linux-gnu/libvamp-sdk.so.2
#5  0x00007fffedf35fd0 in _VampHost::Vamp::PluginHostAdapter::getRemainingFeatures() () from /usr/lib/x86_64-linux-gnu/libvamp-hostsdk.so.3
#6  0x00007fffedf51475 in _VampHost::Vamp::HostExt::PluginWrapper::getRemainingFeatures() () from /usr/lib/x86_64-linux-gnu/libvamp-hostsdk.so.3
#7  0x00007fffedf51475 in _VampHost::Vamp::HostExt::PluginWrapper::getRemainingFeatures() () from /usr/lib/x86_64-linux-gnu/libvamp-hostsdk.so.3
#8  0x00007fffedf51475 in _VampHost::Vamp::HostExt::PluginWrapper::getRemainingFeatures() () from /usr/lib/x86_64-linux-gnu/libvamp-hostsdk.so.3
#9  0x00007ffff63f096c in ARDOUR::AudioAnalyser::analyse (this=0x7fffd9ccf9d0, 
    path="/home/olivier/Musique/echo/analysis/23072.qm-onset", 
    src=0x7fff981e8110, channel=0) at ../libs/ardour/audioanalyser.cc:150
#10 0x00007ffff69df4b4 in ARDOUR::TransientDetector::run (this=0x7fffd9ccf9d0, 
    path="/home/olivier/Musique/echo/analysis/23072.qm-onset", 
    src=0x7fff981e8110, channel=0, results=...)
    at ../libs/ardour/transient_detector.cc:55
#11 0x00007ffff638e381 in ARDOUR::Analyser::analyse_audio_file_source (src=...)
    at ../libs/ardour/analyser.cc:115
#12 0x00007ffff638e1fc in ARDOUR::Analyser::work ()
    at ../libs/ardour/analyser.cc:102
#13 0x00007ffff638dee4 in analyser_work () at ../libs/ardour/analyser.cc:52
#14 0x0000000000e4898d in sigc::pointer_functor0<void>::operator() (
    this=0x19f7648) at /usr/include/sigc++-2.0/sigc++/functors/ptr_fun.h:77
#15 0x0000000000e45dde in sigc::adaptor_functor<sigc::pointer_functor0<void> >::operator() (this=0x19f7640)
    at /usr/include/sigc++-2.0/sigc++/adaptors/adaptor_trait.h:256
#16 0x0000000000e41e67 in sigc::internal::slot_call0<sigc::pointer_functor0<void>, void>::call_it (rep=0x19f7610)
    at /usr/include/sigc++-2.0/sigc++/functors/slot.h:115
#17 0x00007ffff3f1215d in ?? ()
   from /usr/lib/x86_64-linux-gnu/libglibmm-2.4.so.1
#18 0x00007ffff39cebc5 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
#19 0x00007fffef970464 in start_thread (arg=0x7fffd9cd0700)
#20 0x00007fffec95330d in clone ()
    at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109
ardour_bt.txt (3,015 bytes)

zeograd

2016-08-28 20:12

reporter  

ardour_small_timestretch_workaround.patch (600 bytes)
diff --git a/libs/qm-dsp/dsp/signalconditioning/FiltFilt.cpp b/libs/qm-dsp/dsp/signalconditioning/FiltFilt.cpp
index 13dbda0..957f902 100644
--- a/libs/qm-dsp/dsp/signalconditioning/FiltFilt.cpp
+++ b/libs/qm-dsp/dsp/signalconditioning/FiltFilt.cpp
@@ -59,6 +59,9 @@ void FiltFilt::process(double *src, double *dst, unsigned int length)
     unsigned int nFact = 3 * ( nFilt - 1);
     unsigned int nExt	= length + 2 * nFact;
 
+    // Skip filtering when input is too small
+    if (length <= nFact) return;
+
     m_filtScratchIn = new double[ nExt ];
     m_filtScratchOut = new double[ nExt ];
 

zeograd

2016-08-28 20:13

reporter   ~0018506

I propose a tiny patch with a guarding condition in FiltFilt::process to avoid bumping into the mentioned buffer underflow.

paul

2016-08-29 10:10

administrator   ~0018509

I don;t think this is right. ::process() has an obligation to put data into "dst", so just returning under this condition seems wrong.

zeograd

2016-08-29 12:25

reporter   ~0018510

I assumed that the first guarding condition with lenth == 0 returning without filling "dst" meant it was ok to do so too.

To avoid the error later on, maybe it would be better then to limit length with a snippet similar to this:

    if (length >= nFact) {
        length = nFact - 1;
        if (length <= 0) return;
    }


I can test this on the machine and session which trigerred easily this issue in a few hours.

zeograd

2016-08-29 21:45

reporter   ~0018513

My previous snippet attempt was wrong on several level.
First in the comparison check (it's for length too small that the problem appears, not too large) and then regarding the work around, changing length will occur write in buffers which are not dimensioned properly any more and a sigsegv will happen.

So, in order to bypass the onset detection when the length is too small, I propose to check this at a higher level, in OnsetDetector::getRemainingFeatures

At this stage, we can compare the length with the LPOrd and bypass onset detection altogether is length is < 3 * LPOrd (since 3 * LPord is minimum length assumed in FiltFilt::process in this situation).

This proposition is backed by a patch proposal and successful local tests in situation.

zeograd

2016-08-29 21:45

reporter  

buffer_underflow_tiny_timestretch_20160829.patch (2,165 bytes)
Index: libs/vamp-plugins/OnsetDetect.cpp
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- libs/vamp-plugins/OnsetDetect.cpp	(revision b7e8d6f13117b824b66f199b430161c4aae96c3e)
+++ libs/vamp-plugins/OnsetDetect.cpp	(revision )
@@ -452,31 +452,33 @@
         ppSrc[i] = m_d->dfOutput[i];
     }
 
+    if (ppParams.length >= 3 * ppParams.LPOrd) {
-    vector<int> onsets;
-    peakPicker.process(ppSrc, ppParams.length, onsets);
+        vector<int> onsets;
+        peakPicker.process(ppSrc, ppParams.length, onsets);
 
-    for (size_t i = 0; i < onsets.size(); ++i) {
+        for (size_t i = 0; i < onsets.size(); ++i) {
 
-        size_t index = onsets[i];
+            size_t index = onsets[i];
 
-        if (m_dfType != DF_BROADBAND) {
-            double prevDiff = 0.0;
-            while (index > 1) {
+            if (m_dfType != DF_BROADBAND) {
+                double prevDiff = 0.0;
+                while (index > 1) {
-                double diff = ppSrc[index] - ppSrc[index-1];
+                    double diff = ppSrc[index] - ppSrc[index - 1];
-                if (diff < prevDiff * 0.9) break;
-                prevDiff = diff;
-                --index;
-            }
-        }
+                    if (diff < prevDiff * 0.9) break;
+                    prevDiff = diff;
+                    --index;
+                }
+            }
 
-	size_t frame = index * m_d->dfConfig.stepSize;
+            size_t frame = index * m_d->dfConfig.stepSize;
 
-	Feature feature;
-	feature.hasTimestamp = true;
-	feature.timestamp = m_d->origin + Vamp::RealTime::frame2RealTime
-	    (frame, lrintf(m_inputSampleRate));
+            Feature feature;
+            feature.hasTimestamp = true;
+            feature.timestamp = m_d->origin + Vamp::RealTime::frame2RealTime
+                    (frame, lrintf(m_inputSampleRate));
 
-	returnFeatures[0].push_back(feature); // onsets are output 0
+            returnFeatures[0].push_back(feature); // onsets are output 0
+        }
     }
 
     for (unsigned int i = 0; i < ppParams.length; ++i) {

Issue History

Date Modified Username Field Change
2016-08-28 20:11 zeograd New Issue
2016-08-28 20:11 zeograd File Added: ardour_bt.txt
2016-08-28 20:12 zeograd File Added: ardour_small_timestretch_workaround.patch
2016-08-28 20:13 zeograd Note Added: 0018506
2016-08-29 10:10 paul Note Added: 0018509
2016-08-29 12:25 zeograd Note Added: 0018510
2016-08-29 21:45 zeograd Note Added: 0018513
2016-08-29 21:45 zeograd File Added: buffer_underflow_tiny_timestretch_20160829.patch