Fixed off-by-one errors in fft-based cross-correlation computations
ClosedPublic

Authored by raphaelm on Apr 14 2017, 7:33 PM.

Details

Reviewers
mardelle
alcinos
Summary

We were writing out-of-bound elements for a few arrays (leftFFT, rightFFT)
because they were smaller than the expectation of the kiss-fft library, and
also using un-initialized memory because of size mismatch of correlatedFFT.

This was leading to the valgrind error below.

Along the way, made sure sure potentially big chunks of memory are allocated on
the heap and not on the stack in fftCorrelation.cpp.

==19613== Conditional jump or move depends on uninitialised value(s)
==19613==    at 0x80C6D9: AudioCorrelationInfo::maxIndex() const (in /home/raphael/kdenlive_install/bin/kdenlive)
==19613==    by 0x80ADA9: AudioCorrelation::getShift(int) const (in /home/raphael/kdenlive_install/bin/kdenlive)
==19613==    by 0x80AD34: AudioCorrelation::slotProcessChild(AudioEnvelope*) (in /home/raphael/kdenlive_install/bin/kdenlive)
==19613==    by 0x80C4FE: QtPrivate::FunctorCall<QtPrivate::IndexesList<0>, QtPrivate::List<AudioEnvelope*>, void, void (AudioCorrelation::*)(AudioEnvelope*)>::call(void (AudioCorrelation::*)(AudioEnvelope*), AudioCorrelation*, void**) (in /home/raphael/kdenlive_install/bin/kdenlive)
==19613==    by 0x80C35F: void QtPrivate::FunctionPointer<void (AudioCorrelation::*)(AudioEnvelope*)>::call<QtPrivate::List<AudioEnvelope*>, void>(void (AudioCorrelation::*)(AudioEnvelope*), AudioCorrelation*, void**) (in /home/raphael/kdenlive_install/bin/kdenlive)
==19613==    by 0x80C1B0: QtPrivate::QSlotObject<void (AudioCorrelation::*)(AudioEnvelope*), QtPrivate::List<AudioEnvelope*>, void>::impl(int, QtPrivate::QSlotObjectBase*, QObject*, void**, bool*) (in /home/raphael/kdenlive_install/bin/kdenlive)
==19613==    by 0xE2A4A55: call (qobject_impl.h:101)
==19613==    by 0xE2A4A55: QMetaObject::activate(QObject*, int, int, void**) (qobject.cpp:3723)
==19613==    by 0x9C188E: AudioEnvelope::envelopeReady(AudioEnvelope*) (in /home/raphael/kdenlive_install/bin/kdenlive)
==19613==    by 0x80C88C: AudioEnvelope::AudioEnvelope(QString const&, Mlt::Producer*, int, int, int, int)::{lambda()#1}::operator()() const (in /home/raphael/kdenlive_install/bin/kdenlive)
==19613==    by 0x80DA89: QtPrivate::FunctorCall<QtPrivate::IndexesList<>, QtPrivate::List<>, void, AudioEnvelope::AudioEnvelope(QString const&, Mlt::Producer*, int, int, int, int)::{lambda()#1}>::call({lambda()#1}&, void**) (in /home/raphael/kdenlive_install/bin/kdenlive)
==19613==    by 0x80DA6A: void QtPrivate::Functor<AudioEnvelope::AudioEnvelope(QString const&, Mlt::Producer*, int, int, int, int)::{lambda()#1}, 0>::call<QtPrivate::List<>, void>({lambda()#1}&, void*, {lambda()#1}&*) (in /home/raphael/kdenlive_install/bin/kdenlive)
==19613==    by 0x80DA2F: QtPrivate::QFunctorSlotObject<AudioEnvelope::AudioEnvelope(QString const&, Mlt::Producer*, int, int, int, int)::{lambda()#1}, 0, QtPrivate::List<>, void>::impl(int, QtPrivate::QSlotObjectBase*, QObject*, void**, bool*) (in /home/raphael/kdenlive_install/bin/kdenlive)
==19613==
Test Plan

Tested that the valgrind error goes away after this change, and manually tested the alignment feature to make sure it continues working as expected.

Diff Detail

Repository
R158 Kdenlive
Lint
Lint Skipped
Unit
Unit Tests Skipped
raphaelm created this revision.Apr 14 2017, 7:33 PM
raphaelm edited the summary of this revision. (Show Details)
alcinos edited edge metadata.Apr 15 2017, 4:43 PM

LGTM. Thanks!

alcinos accepted this revision.Apr 15 2017, 4:44 PM
This revision is now accepted and ready to land.Apr 15 2017, 4:44 PM

Thanks. I don't have permissions to land the change, so feel free to do it yourself.

aacid closed this revision.Apr 17 2017, 10:47 AM
aacid added a subscriber: aacid.

Committed in https://phabricator.kde.org/R158:3404da90de3ce10377cbebaaf5abc9229f1ce43f

Jean-Baptiste please use git commit --author in the future, it's a better way to say who did the change