Skip to content

Updated the SignalSmith module to be much easier to work with. #1164

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Arctis-Fireblight merged 3 commits into from
Jan 25, 2026
Merged

Updated the SignalSmith module to be much easier to work with. #1164

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a68b06b
suddenly, life was made easier.
mcdubhghlas Jan 19, 2026
18d3471
SPACES_TABS_SPACES_TABS AAA FAUSTIAN NIGHTMARE CONTINUES PLS SIR I TU…
mcdubhghlas Jan 20, 2026
387f63a
suggested fix.
mcdubhghlas Jan 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions doc/classes/SignalSmith.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,28 @@
<tutorials>
</tutorials>
<methods>
<method name="change_tempo">
<return type="AudioStreamWAV" />
<param index="0" name="path" type="String" />
<param index="1" name="tempo" type="float" />
<param index="2" name="pitch" type="float" default="1.0" />
<description>
Loads an MP3 file, applies time-stretching and pitch-shifting using the Signalsmith engine, and returns a streamable AudioStreamWAV.
The `tempo` parameter controls playback speed without affecting pitch. A value of `1.0` preserves the original speed.
The `pitch` parameter is a transpose factor. A value of `1.0` preserves pitch, `2.0` raises pitch by one octave,
and `0.5` lowers pitch by one octave.
</description>
</method>
<method name="get_last_channels" qualifiers="const">
<return type="int" />
<description>
</description>
</method>
<method name="get_last_sample_rate" qualifiers="const">
<return type="int" />
<description>
</description>
</method>
Comment on lines +24 to +33

@coderabbitai coderabbitai Bot Jan 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add short descriptions for the new getters.
Empty descriptions make it unclear what “last” refers to for users.

📝 Suggested doc update 
 		<method name="get_last_channels" qualifiers="const">
 			<return type="int" />
 			<description>
+				Returns the channel count last set via [method set_channels] or [method change_tempo].
 			</description>
 		</method>
 		<method name="get_last_sample_rate" qualifiers="const">
 			<return type="int" />
 			<description>
+				Returns the sample rate last set via [method set_sample_rate] or [method change_tempo].
 			</description>
 		</method>
🤖 Prompt for AI Agents 
In `@doc/classes/SignalSmith.xml` around lines 24 - 33, The getters
get_last_channels and get_last_sample_rate lack descriptions; add concise
docstrings clarifying that "last" refers to the most recently processed or
configured audio stream values (e.g., the channel count and sample rate of the
last processed buffer/frame or last set input). Update the <description> for
method name="get_last_channels" to state it returns the channel count of the
most recently processed audio buffer/stream, and update the <description> for
method name="get_last_sample_rate" to state it returns the sample rate (in Hz)
of the most recently processed audio buffer/stream.

<method name="process">
<return type="PackedFloat32Array" />
<param index="0" name="input" type="PackedFloat32Array" />
Expand Down
7 changes: 6 additions & 1 deletion modules/signalsmith/SCsub
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,12 @@ Import("env")

signalsmith_env = env.Clone()

signalsmith_env.Append(CPPPATH=["#thirdparty"])
signalsmith_env.Append(
CPPPATH=[
"#thirdparty",
"#thirdparty/minimp3",
]
)

cxx = env.subst("$CXX")
is_clang = "clang" in cxx
Expand Down
100 changes: 100 additions & 0 deletions modules/signalsmith/signalsmith_module.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,14 @@

#include "signalsmith_module.h"

#include "core/io/file_access.h"
#include "core/os/memory.h"

#include "modules/minimp3/audio_stream_mp3.h"
#include "scene/resources/audio_stream_wav.h"
#include "servers/audio/audio_stream.h"
#include "servers/audio_server.h"

#include <cmath>
#include <vector>

Expand All @@ -42,8 +48,11 @@ void SignalSmith::_bind_methods() {
ClassDB::bind_method(D_METHOD("set_channels", "channels"), &SignalSmith::set_channels);
ClassDB::bind_method(D_METHOD("set_pitch", "pitch"), &SignalSmith::set_pitch);
ClassDB::bind_method(D_METHOD("set_tempo", "tempo"), &SignalSmith::set_tempo);
ClassDB::bind_method(D_METHOD("get_last_sample_rate"), &SignalSmith::get_last_sample_rate);
ClassDB::bind_method(D_METHOD("get_last_channels"), &SignalSmith::get_last_channels);
ClassDB::bind_method(D_METHOD("reset"), &SignalSmith::reset);
ClassDB::bind_method(D_METHOD("process", "input"), &SignalSmith::process);
ClassDB::bind_method(D_METHOD("change_tempo", "path", "tempo", "pitch"), &SignalSmith::change_tempo, DEFVAL(1.0f));
}

SignalSmith::SignalSmith() {
Expand Down Expand Up @@ -86,6 +95,14 @@ void SignalSmith::set_tempo(float p_tempo) {
tempo = p_tempo;
}

int SignalSmith::get_last_sample_rate() const {
return sample_rate;
}

int SignalSmith::get_last_channels() const {
return channels;
}

void SignalSmith::reset() {
stretch.reset();
}
Expand Down Expand Up @@ -171,3 +188,86 @@ PackedFloat32Array SignalSmith::process(const PackedFloat32Array &input) {

return output;
}

Ref<AudioStreamWAV> SignalSmith::change_tempo(const String &path, float p_tempo, float p_pitch) {
Ref<AudioStreamWAV> out;

Ref<AudioStreamMP3> mp3 = AudioStreamMP3::load_from_file(path);
ERR_FAIL_COND_V(mp3.is_null(), out);

Ref<AudioStreamPlayback> pb_base = mp3->instantiate_playback();
ERR_FAIL_COND_V(pb_base.is_null(), out);

AudioStreamPlaybackResampled *pb = Object::cast_to<AudioStreamPlaybackResampled>(pb_base.ptr());

ERR_FAIL_COND_V(pb == nullptr, out);

pb->start(0.0);
Comment thread
Arctis-Fireblight marked this conversation as resolved.

const int channels = mp3->is_monophonic() ? 1 : 2;
const int sample_rate = AudioServer::get_singleton()->get_mix_rate();

Vector<AudioFrame> frames;
const int block = 1024;

while (true) {
int old = frames.size();
frames.resize(old + block);
int mixed = pb->mix(frames.ptrw() + old, 1.0f, block);

if (mixed <= 0) {
frames.resize(old + mixed);
break;
}
}
Comment on lines +213 to +222

@coderabbitai coderabbitai Bot Jan 20, 2026 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Handle partial mixes to avoid trailing silence. If pb->mix() returns a positive count smaller than block, the current code keeps the remaining zeroed frames, inflating length and adding silence. On Line 216, treat mixed < block as end-of-stream and resize to old + mixed.

🔧 Proposed fix 
 while (true) {
 	int old = frames.size();
 	frames.resize(old + block);
 	int mixed = pb->mix(frames.ptrw() + old, 1.0f, block);

 	if (mixed <= 0) {
 		frames.resize(old);
 		break;
 	}
+
+	if (mixed < block) {
+		frames.resize(old + mixed);
+		break;
+	}
 }
AudioStreamPlaybackResampled::mix return semantics at end-of-stream: can it return a positive value smaller than the requested frame count?
🤖 Prompt for AI Agents 
In `@modules/signalsmith/signalsmith_module.cpp` around lines 213 - 223, The loop
currently treats only mixed <= 0 as end-of-stream, leaving zeroed frames when
pb->mix(...) returns a positive value less than block; change the logic in the
loop that calls pb->mix(frames.ptrw() + old, 1.0f, block) so that if mixed <
block you resize to old + mixed (not old + block) and then break, preserving
only the actual mixed frames; keep the existing behavior for mixed <= 0 where
you resize back to old and break.

@Arctis-Fireblight Arctis-Fireblight Jan 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mcdubhghlas can pb->mix actually return a partially full buffer here?
I feel like the majority of the time we should be seeing multiples of 1024, but in the off chance it doesnt line up, this could result in an audio buffer underflow with audio artifacts.

We may want to consider @coderabbitai 's suggested fix here.

@coderabbitai coderabbitai Bot Jan 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for AudioStreamPlaybackResampled class definition and mix method
rg -n "class AudioStreamPlaybackResampled" --type=cpp -A 30

Length of output: 2022

🏁 Script executed:

#!/bin/bash
# Search for the mix method implementation in AudioStreamPlaybackResampled
rg -n "AudioStreamPlaybackResampled.*::mix" --type=cpp -A 20

Length of output: 4892

🏁 Script executed:

#!/bin/bash
# Also search for any virtual mix method that might be overridden
rg -n "virtual.*mix.*AudioFrame" --type=cpp -B 2 -A 10

Length of output: 12087

🏁 Script executed:

#!/bin/bash
# Get the full implementation of AudioStreamPlaybackResampled::mix
rg -n "int AudioStreamPlaybackResampled::mix" --type=cpp -A 60

Length of output: 4572

🏁 Script executed:

#!/bin/bash
# Also look at _mix_internal implementations which is called by mix
rg -n "int.*_mix_internal.*AudioFrame.*p_buffer.*p_frames" --type=cpp -A 30 | head -200

Length of output: 14268

@Arctis-Fireblight Yes, pb->mix() can definitely return a partially full buffer!

Based on the source code analysis, AudioStreamPlaybackResampled::mix is designed to return fewer frames than requested when it reaches the end of the stream. Here's what happens:

The method internally calls _mix_internal() and when that returns fewer frames than requested (lines 235-241 in servers/audio/audio_stream.cpp), it marks where the audio data ends. The final return value (line 248) is mixed_frames_total, which can be less than the requested p_frames when the stream ends.

Current issue: When mixed is between 1 and block-1 (i.e., 0 < mixed < 1024), the code keeps the full resized buffer including the zeroed frames at the end, causing exactly the audio artifact you're concerned about.

Recommendation: CodeRabbit's suggested fix is correct and should be applied. The loop should handle the partial buffer case:

if (mixed < block) {
    frames.resize(old + mixed);
    break;
}

This ensures only the actual mixed frames are preserved, preventing trailing silence or buffer underflow artifacts in the processed audio output.

🧬 Code Graph Analysis Results

[modules/signalsmith/signalsmith_module.cpp:change_tempo function]
Ref SignalSmith::change_tempo(const String &path, float p_tempo, float p_pitch) {
Ref out;

Ref<AudioStreamMP3> mp3 = AudioStreamMP3::load_from_file(path);
ERR_FAIL_COND_V(mp3.is_null(), out);

Ref<AudioStreamPlayback> pb_base = mp3->instantiate_playback();
ERR_FAIL_COND_V(pb_base.is_null(), out);

AudioStreamPlaybackResampled *pb = Object::cast_to<AudioStreamPlaybackResampled>(pb_base.ptr());

ERR_FAIL_COND_V(pb == nullptr, out);

pb->start(0.0);

const int channels = mp3->is_monophonic() ? 1 : 2;
const int sample_rate = AudioServer::get_singleton()->get_mix_rate();

Vector<AudioFrame> frames;
const int block = 1024;

while (true) {
	int old = frames.size();
	frames.resize(old + block);
	int mixed = pb->mix(frames.ptrw() + old, 1.0f, block);

	if (mixed <= 0) {
		frames.resize(old);

		break;
	}
}

ERR_FAIL_COND_V(frames.is_empty(), out);

// Convert AudioFrame to float PCM
PackedFloat32Array input;
input.resize(frames.size() * channels);
float *input_w = input.ptrw();

for (int i = 0; i < frames.size(); i++) {
	if (channels == 2) {
		input_w[i * 2 + 0] = frames[i].left;
		input_w[i * 2 + 1] = frames[i].right;
	} else {
		input_w[i] = frames[i].left;
	}
}

set_sample_rate(sample_rate);
set_channels(channels);
set_tempo(p_tempo);
set_pitch(p_pitch);

reset();

PackedFloat32Array processed = process(input);
ERR_FAIL_COND_V(processed.is_empty(), out);

// convert float pcm to pcm16
PackedByteArray pcm16;
pcm16.resize(processed.size() * 2);
uint8_t *pcm_w = pcm16.ptrw();

const float *proc_r = processed.ptr();

for (int i = 0; i < processed.size(); i++) {
	float s = CLAMP(proc_r[i], -1.0f, 1.0f);
	int16_t v = int16_t(s * 32767.0f);

	pcm_w[i * 2 + 0] = uint8_t(v & 0xff);
	pcm_w[i * 2 + 1] = uint8_t((v >> 8) & 0xff);
}

// Builds a streamable WAV
out.instantiate();
out->set_mix_rate(sample_rate);
out->set_stereo(channels == 2);
out->set_format(AudioStreamWAV::FORMAT_16_BITS);
out->set_data(pcm16);

return out;

}

@Arctis-Fireblight Arctis-Fireblight Jan 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks CodeRabbit!

@mcdubhghlas do you mind making the suggested fix and then rebasing off of the current dev branch?
This branch you are working off of seems to contain already merged commits.

@coderabbitai coderabbitai Bot Jan 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!


ERR_FAIL_COND_V(frames.is_empty(), out);

// Convert AudioFrame to float PCM
PackedFloat32Array input;
input.resize(frames.size() * channels);
float *input_w = input.ptrw();

for (int i = 0; i < frames.size(); i++) {
if (channels == 2) {
input_w[i * 2 + 0] = frames[i].left;
input_w[i * 2 + 1] = frames[i].right;
} else {
input_w[i] = frames[i].left;
}
}

set_sample_rate(sample_rate);
set_channels(channels);
set_tempo(p_tempo);
set_pitch(p_pitch);

reset();

PackedFloat32Array processed = process(input);
ERR_FAIL_COND_V(processed.is_empty(), out);

// convert float pcm to pcm16
PackedByteArray pcm16;
pcm16.resize(processed.size() * 2);
uint8_t *pcm_w = pcm16.ptrw();

const float *proc_r = processed.ptr();

for (int i = 0; i < processed.size(); i++) {
float s = CLAMP(proc_r[i], -1.0f, 1.0f);
int16_t v = int16_t(s * 32767.0f);

pcm_w[i * 2 + 0] = uint8_t(v & 0xff);
pcm_w[i * 2 + 1] = uint8_t((v >> 8) & 0xff);
}

// Builds a streamable WAV
out.instantiate();
out->set_mix_rate(sample_rate);
out->set_stereo(channels == 2);
out->set_format(AudioStreamWAV::FORMAT_16_BITS);
out->set_data(pcm16);

return out;
}
6 changes: 6 additions & 0 deletions modules/signalsmith/signalsmith_module.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@

#include "core/object/class_db.h"
#include "core/object/ref_counted.h"
#include "scene/resources/audio_stream_wav.h"
#include "signalsmith-stretch/signalsmith-stretch.h"
#include <random>

Expand All @@ -59,5 +60,10 @@ class SignalSmith : public RefCounted {
void set_tempo(float p_tempo);
void reset();

int get_last_sample_rate() const;
int get_last_channels() const;

PackedFloat32Array process(const PackedFloat32Array &input);

Ref<AudioStreamWAV> change_tempo(const String &path, float p_tempo, float p_pitch = 1.0f);
};
Loading