Raise a descriptive error when unscaled integer audio is passed (fixes #407) - #491
Open
deekshaNVIDIA wants to merge 1 commit into
Open
Conversation
Fixes spotify#407. Users have repeatedly (spotify#406, spotify#390, spotify#369) passed integer PCM data (e.g. int16) that was converted to float32/float64 without rescaling into the [-1.0, 1.0] range Pedalboard expects. Scale-invariant plugins (Reverb, Delay) appear to work while others (Compressor, Limiter, PitchShift) fail confusingly. This adds a fast check in the shared process() entry point (used by Pedalboard, individual plugins, and pedalboard.process) that raises a ValueError explaining the likely cause and fix when every sample is an exact integer and the peak magnitude exceeds 1.0. The check bails out on the first fractional or non-finite sample, so it is effectively free for real audio.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #407.
In several reported cases (#406, #390, #369), users passed integer PCM data (e.g.
int16) that had been converted tofloat32/float64without rescaling it into the[-1.0, 1.0]range that Pedalboard expects. Scale-invariant plugins (Reverb, Delay) appear to work, while others (Compressor, Limiter, PitchShift) fail in confusing ways.This adds a small check in the shared C++
process()entry point inpedalboard/process.h— the single code path used bypedalboard.process(...),Plugin.process(...)/Plugin.__call__, andPedalboard.__call__(viaChain). If every sample is an exact integer and the peak magnitude exceeds1.0, it raises aValueErrorexplaining the likely cause and the fix.Behavior
ValueErrorfor buffers that look like unscaled integer PCM data (e.g.(audio * 32767).astype(np.int16).astype(np.float32)).±1.0square wave, DC at1.0) are still accepted, since those are valid audio.Why this location
process()inprocess.his the single chokepoint that all Python-facing processing entry points route through, so the validation covers every case (Pedalboard, individual plugins, and the module-levelprocessfunction) with one implementation. The check runs while the GIL is still held, before the heavy processing begins.Notes on testing
I validated the detection heuristic numerically against the released
pedalboardwheel across many signal types (sine waves, random noise, silence, full-scale DC/square waves, hot non-integer signals, empty and stereo buffers): zero false positives, and the documentedint16-as-float32case is caught (its peak of ~29490 otherwise reaches the Limiter).tests/test_integer_input_validation.pyis added to cover:int16-as-float32and-as-float64raisingValueErrorviaPedalboard, a singlePlugin, andpedalboard.process.I was unable to compile the native extension locally (no C++ toolchain on my machine), so I have not run the compiled test suite end-to-end — CI will build and run the new tests.
ruff,black(line length 100), andflake8all pass on the new test file.