Skip to content

Commit

Permalink
Fix and test stack overflow in R2 stretcher's time-domain smoothing o…
Browse files Browse the repository at this point in the history
…ption with long input buffers. The code in question is non-RT and has no need to be using stack allocation here at all. Thanks to Peter Sobot for the report.
  • Loading branch information
cannam committed Jun 26, 2024
1 parent 836330b commit 832f577
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 10 deletions.
19 changes: 10 additions & 9 deletions src/faster/R2Stretcher.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -993,6 +993,8 @@ R2Stretcher::study(const float *const *input, size_t samples, bool final)
mixdown = input[0];
}

std::vector<float> optionalFoldBuffer;

while (consumed < samples) {

size_t writable = inbuf.getWriteSpace();
Expand Down Expand Up @@ -1042,33 +1044,32 @@ R2Stretcher::study(const float *const *input, size_t samples, bool final)

// Note that we can't do this in-place. Pity

float *tmp = (float *)alloca
(std::max(m_fftSize, m_aWindowSize) * sizeof(float));
// This doesn't have to be allocation-free, as we're
// in offline-mode study

optionalFoldBuffer.resize(std::max(m_fftSize, m_aWindowSize));

if (m_aWindowSize > m_fftSize) {
m_afilter->cut(cd.accumulator);
}

cutShiftAndFold(tmp, m_fftSize, cd.accumulator, m_awindow);
v_copy(cd.accumulator, tmp, m_fftSize);
cutShiftAndFold(optionalFoldBuffer.data(), m_fftSize,
cd.accumulator, m_awindow);
v_copy(cd.accumulator, optionalFoldBuffer.data(), m_fftSize);
}

m_studyFFT->forwardMagnitude(cd.accumulator, cd.fltbuf);

float df = m_phaseResetAudioCurve->processFloat(cd.fltbuf, m_increment);
m_phaseResetDf.push_back(df);

// cout << m_phaseResetDf.size() << " [" << final << "] -> " << df << " \t: ";

df = m_silentAudioCurve->processFloat(cd.fltbuf, m_increment);
bool silent = (df > 0.f);
if (silent) {
m_log.log(2, "silence at", m_inputDuration);
}
m_silence.push_back(silent);

// cout << df << endl;

// We have augmented the input by m_aWindowSize/2 so that
// the first chunk is centred on the first audio sample.
// We want to ensure that m_inputDuration contains the
Expand All @@ -1080,7 +1081,7 @@ R2Stretcher::study(const float *const *input, size_t samples, bool final)
inbuf.skip(m_increment);
}
}

if (final) {
int rs = inbuf.getReadSpace();
m_inputDuration += rs;
Expand Down
8 changes: 7 additions & 1 deletion src/faster/StretcherProcess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,9 @@ R2Stretcher::consumeChannel(size_t c,

inbuf.write(cd.resamplebuf, toWrite);
cd.inCount += samples;

m_log.log(2, "consumeChannel: wrote to inbuf from resamplebuf, inCount now", toWrite, cd.inCount);

return samples;

} else {
Expand All @@ -249,6 +252,9 @@ R2Stretcher::consumeChannel(size_t c,

inbuf.write(input, toWrite);
cd.inCount += toWrite;

m_log.log(2, "consumeChannel: wrote to inbuf from input, inCount now", toWrite, cd.inCount);

return toWrite;
}
}
Expand Down Expand Up @@ -340,7 +346,7 @@ R2Stretcher::processOneChunk()
return false;
}
ChannelData &cd = *m_channelData[c];
m_log.log(3, "read space and draining", cd.inbuf->getReadSpace(), cd.draining);
m_log.log(2, "read space and draining", cd.inbuf->getReadSpace(), cd.draining);
if (!cd.draining) {
size_t ready = cd.inbuf->getReadSpace();
assert(ready >= m_aWindowSize || cd.inputSize >= 0);
Expand Down
28 changes: 28 additions & 0 deletions src/test/TestStretcher.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1516,4 +1516,32 @@ BOOST_AUTO_TEST_CASE(with_resets_2x_5up_realtime_faster)
with_resets(RubberBandStretcher::OptionProcessRealTime | RubberBandStretcher::OptionEngineFaster, 2.0, 1.5);
}

BOOST_AUTO_TEST_CASE(long_blocksize_with_smoothing)
{
// Test added because the smoothing option was calling alloca in a
// loop, which could run us out of stack. This test is best used
// with a small stack artificially enforced via e.g. ulimit -s 32

int n = 10000;
float freq = 440.f;
int rate = 44100;
RubberBandStretcher stretcher
(rate, 1,
RubberBandStretcher::OptionEngineFaster |
RubberBandStretcher::OptionProcessOffline |
RubberBandStretcher::OptionSmoothingOn);

vector<float> in(n);
for (int i = 0; i < n; ++i) {
in[i] = sinf(float(i) * freq * M_PI * 2.f / float(rate));
}
float *inp = in.data();

stretcher.setMaxProcessSize(n);
stretcher.setExpectedInputDuration(n);

stretcher.study(&inp, n, true);
stretcher.process(&inp, n, true);
}

BOOST_AUTO_TEST_SUITE_END()

0 comments on commit 832f577

Please sign in to comment.