From 5526b88b81f48c62d2bb3a42f92182a451f657e2 Mon Sep 17 00:00:00 2001 From: Chris Cannam Date: Fri, 26 Apr 2024 17:42:53 +0100 Subject: [PATCH 1/5] Add header for ptrdiff_t (thanks to rather old PR from Robin Gareus) --- src/common/Allocators.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/common/Allocators.h b/src/common/Allocators.h index 4c529f6..ee62f2e 100644 --- a/src/common/Allocators.h +++ b/src/common/Allocators.h @@ -27,7 +27,8 @@ #include "VectorOps.h" #include // for std::bad_alloc -#include +#include +#include // ptrdiff_t #include From 50f0c0ce1c889719d72da02dcbdccf28c6c20f2e Mon Sep 17 00:00:00 2001 From: Chris Cannam Date: Fri, 26 Apr 2024 17:46:24 +0100 Subject: [PATCH 2/5] Clarify use of debug option thanks to earlier report by triallax --- meson.build | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/meson.build b/meson.build index ebf5a89..09ce220 100644 --- a/meson.build +++ b/meson.build @@ -436,11 +436,11 @@ ladspa_symbol_args = [] lv2_symbol_args = [] vamp_symbol_args = [] -if get_option('buildtype').startswith('release') - config_summary += { 'Build type': 'Release' } - feature_defines += ['-DNO_THREAD_CHECKS', '-DNO_TIMING', '-DNDEBUG'] +if get_option('debug') + config_summary += { 'Debug': 'Enabled' } else - config_summary += { 'Build type': 'Debug' } + config_summary += { 'Debug': 'Disabled' } + feature_defines += ['-DNO_THREAD_CHECKS', '-DNO_TIMING', '-DNDEBUG'] endif if system == 'darwin' From 5d296019ff0fd6085fea0838155b0449a4606397 Mon Sep 17 00:00:00 2001 From: Chris Cannam Date: Fri, 26 Apr 2024 17:53:17 +0100 Subject: [PATCH 3/5] Add Homebrew paths --- otherbuilds/check.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/otherbuilds/check.sh b/otherbuilds/check.sh index f04a754..cf323df 100755 --- a/otherbuilds/check.sh +++ b/otherbuilds/check.sh @@ -29,13 +29,13 @@ else make -f otherbuilds/Makefile.macos echo " *** Linking against static library" - c++ -std=c++11 main/main.cpp lib/librubberband.a -I. -Isrc -o test_static -lsndfile -framework Accelerate + c++ -std=c++11 main/main.cpp lib/librubberband.a -I. -Isrc -o test_static -I/opt/homebrew/include -L/opt/homebrew/lib -lsndfile -framework Accelerate echo " *** Running build from macOS-specific Makefile" ./test_static -V echo " *** Building with single-file source" - c++ -O3 -std=c++11 main/main.cpp single/RubberBandSingle.cpp -o test_single -lsndfile -framework Accelerate + c++ -O3 -std=c++11 main/main.cpp single/RubberBandSingle.cpp -o test_single -I/opt/homebrew/include -L/opt/homebrew/lib -lsndfile -framework Accelerate echo " *** Running build from single-file source" ./test_single -V From 836330b1a135e8dddb020cb229d2c4a3a72a7cb8 Mon Sep 17 00:00:00 2001 From: Chris Cannam Date: Fri, 3 May 2024 11:09:04 +0100 Subject: [PATCH 4/5] Fix warning --- src/common/Thread.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/Thread.h b/src/common/Thread.h index ca41367..3f8a71b 100644 --- a/src/common/Thread.h +++ b/src/common/Thread.h @@ -215,7 +215,7 @@ public: class Condition { public: - Condition(std::string name) { } + Condition(std::string) { } ~Condition() { } void lock() { } From 832f577acbcdfa8c48d19a8381e73428297bec4e Mon Sep 17 00:00:00 2001 From: Chris Cannam Date: Wed, 26 Jun 2024 11:05:19 +0100 Subject: [PATCH 5/5] Fix and test stack overflow in R2 stretcher's time-domain smoothing option 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. --- src/faster/R2Stretcher.cpp | 19 ++++++++++--------- src/faster/StretcherProcess.cpp | 8 +++++++- src/test/TestStretcher.cpp | 28 ++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 10 deletions(-) diff --git a/src/faster/R2Stretcher.cpp b/src/faster/R2Stretcher.cpp index 4159c24..52737a4 100644 --- a/src/faster/R2Stretcher.cpp +++ b/src/faster/R2Stretcher.cpp @@ -993,6 +993,8 @@ R2Stretcher::study(const float *const *input, size_t samples, bool final) mixdown = input[0]; } + std::vector optionalFoldBuffer; + while (consumed < samples) { size_t writable = inbuf.getWriteSpace(); @@ -1042,15 +1044,18 @@ 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); @@ -1058,8 +1063,6 @@ R2Stretcher::study(const float *const *input, size_t samples, bool final) 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) { @@ -1067,8 +1070,6 @@ R2Stretcher::study(const float *const *input, size_t samples, bool final) } 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 @@ -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; diff --git a/src/faster/StretcherProcess.cpp b/src/faster/StretcherProcess.cpp index 36fe27b..74d09e7 100644 --- a/src/faster/StretcherProcess.cpp +++ b/src/faster/StretcherProcess.cpp @@ -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 { @@ -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; } } @@ -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); diff --git a/src/test/TestStretcher.cpp b/src/test/TestStretcher.cpp index fc72d99..a2352c4 100644 --- a/src/test/TestStretcher.cpp +++ b/src/test/TestStretcher.cpp @@ -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 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()