spotify / pedalboard

🎛 🔊 A Python library for audio.
https://spotify.github.io/pedalboard
GNU General Public License v3.0
4.96k stars 249 forks source link

Segfault in time_stretch #340

Closed f0k closed 1 week ago

f0k commented 2 weeks ago

All parameter combinations for time_stretch() that have high_quality=False and use_time_domain_smoothing=True cause a segfault. Example:

import pedalboard
import numpy as np
sr = 44100
x = np.random.default_rng().uniform(size=44100*5).astype(np.float32)
pedalboard.time_stretch(x, sr, high_quality=False, use_time_domain_smoothing=True)  # segfaults

Saving as ts.py and running gdb --args python3 ts.py, this is the beginning of the backtrace:

#0  0x00007fffebc07d5f in RubberBand::R2Stretcher::study(float const* const*, unsigned long, bool) ()
   from /usr/local/lib/python3.10/dist-packages/pedalboard_native.cpython-310-x86_64-linux-gnu.so
#1  0x00007fffebb743b5 in Pedalboard::timeStretch(juce::AudioBuffer<float>, double, double, double, bool, std::string, std::string, bool, std::optional<bool>, bool, bool) [clone .lto_priv.0] ()
   from /usr/local/lib/python3.10/dist-packages/pedalboard_native.cpython-310-x86_64-linux-gnu.so

I did not investigate further. This is on pedalboard 0.9.6.

psobot commented 1 week ago

Thanks @f0k! We have pretty comprehensive tests for the extra time stretch options, but there's a gap in the test coverage; it seems this happens with any sufficiently-large buffer (5 seconds at 44.1kHz does it, but 0.5 seconds at 22050Hz does not).

I can replicate this locally and should be able to put up a quick patch.

psobot commented 1 week ago

This appears to be a bug in the underlying Rubber Band library we use for time stretching; the alloca function is called in a loop, but its allocations are not cleared until the end of the function (by design), resulting in a stack overflow if the input is long enough.

I'll send a PR over to Rubber Band to solve the underlying issue, but I think we can get around this in Pedalboard by calling study in a loop and cap the maximum number of samples provided at once.

f0k commented 5 days ago

Thanks a lot @psobot for following up on this and even sorting out the problem for rubberband! :detective: