Skip to content

New SampleRate audio effect#68768

Open
M-O-Marmalade wants to merge 1 commit intogodotengine:masterfrom
M-O-Marmalade:audio-effect-sample-rate
Open

New SampleRate audio effect#68768
M-O-Marmalade wants to merge 1 commit intogodotengine:masterfrom
M-O-Marmalade:audio-effect-sample-rate

Conversation

@M-O-Marmalade
Copy link
Copy Markdown
Contributor

@M-O-Marmalade M-O-Marmalade commented Nov 17, 2022

Implements a new audio effect, which lowers the sample rate of the input audio by using a Sample & Hold algorithm.

This is a common tool in sound design; especially for things like chiptune, but it's also used as a creative effect in modern hi-fi audio productions as well.

The "LoFi" mode in the Distortion audio effect reduces bit depth, but not sample rate. With the addition of the SampleRate audio effect, users will have control over both the vertical, and horizontal resolution of audio buses.

Here is an example project, with the SampleRate effect on the Master audio bus. The scene provides some controls to get a quick idea of the effect's sound.

AudioEffectSampleRate-example.zip

I have the appropriate edits prepared for the Audio Effects page in the Godot Docs as well, if you'd like me to open a pull request in the godot-docs repo and link it here.


Proposal / Pull Requests

Proposal at godotengine/godot-proposals#5810
Pull request for 3.x branch at #70085

@M-O-Marmalade M-O-Marmalade requested review from a team as code owners November 17, 2022 04:57
Copy link
Copy Markdown
Contributor Author

@M-O-Marmalade M-O-Marmalade Nov 17, 2022

Choose a reason for hiding this comment

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

11025 Hz was chosen as the default sample rate, as it has an immediately audible effect on the sound, with room to move down towards 1 Hz, or upwards towards 44.1/48 kHz.

22050 Hz would also be a good default, since it's exactly half of 44.1 kHz (the most common sample rate), but I found 22050 Hz had a more subtle effect on the sound, whereas 11025 Hz has an immediately more obvious, but still relatively subtle effect.

@clayjohn
Copy link
Copy Markdown
Member

This looks very interesting!

As a heads up, we are in a feature freeze right now which means that we don't intend to merge any substantially new features to 4.0. Accordingly I have marked this PR for the 4.x milestone so it can hopefully be merged shortly after 4.0 releases to be included with 4.1.

@M-O-Marmalade
Copy link
Copy Markdown
Contributor Author

M-O-Marmalade commented Nov 17, 2022

Thank you!

This code is also compatible with 3.x. Is 3.x accepting new features, or only backporting from 4.0?

Also, I read this in the "feature freeze" article you linked:

"Smaller enhancements can also be approved, given that they have reasonably high support."

If I make a proposal in the proposal system, and users express wanting this in 4.0 (or 3.x if possible), would an inclusion in 4.0 then be a possibility? Just curious.

@clayjohn
Copy link
Copy Markdown
Member

This code is also compatible with 3.x. Is 3.x accepting new features, or only backporting from 4.0?

3.x is accepting new features!

Also, I read this in the "feature freeze" article you linked:

"Smaller enhancements can also be approved, given that they have reasonably high support."

If I make a proposal in the proposal system, and users express wanting this in 4.0 (or 3.x if possible), would an inclusion in 4.0 then be a possibility? Just curious.

In theory yes, but in practice the closer we get to release the more resistant we are to adding new features. I recommend you open a proposal in either case so that this can be evaluated for 3.x as well.

@M-O-Marmalade M-O-Marmalade force-pushed the audio-effect-sample-rate branch 2 times, most recently from 48eeeb4 to 43819fa Compare November 17, 2022 06:53
@M-O-Marmalade
Copy link
Copy Markdown
Contributor Author

Sounds good! Thank you for your advice! I'll open a proposal ASAP and link it here.

@Calinou
Copy link
Copy Markdown
Member

Calinou commented Nov 17, 2022

How does this effect handle sample rates that are greater than the original audio's sample rate (or greater than the audio mix rate configured in the project setting)? This should be documented in the class reference to make it clear that this is not intended to "enhance" existing audio (but rather, intentionally degrade it).

Copy link
Copy Markdown
Contributor Author

@M-O-Marmalade M-O-Marmalade Nov 18, 2022

Choose a reason for hiding this comment

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

@Calinou How does this effect handle sample rates that are greater than the original audio's sample rate (or greater than the audio mix rate configured in the project setting)?

When SampleRate's rate is set equal-to or greater-than AudioServer.mix_rate, there is no audible effect. The sample & hold algorithm "keeps up" with the mix rate, and just passes the samples through at the same rate as they come in. It can only sample as fast as the samples come in, so even setting it ridiculously higher would have no further impact on the audio, nor the performance.

Even AudioStreams with a sample rate lower than the SampleRate effect's rate parameter will have an audible effect, since AudioStreams are resampled to AudioServer.mix_rate with Cubic interpolation before being processed by effects. The only frequency users need to know to stay under, is AudioServer.mix_rate (usually 44100Hz or 48000Hz).

@Calinou This should be documented in the class reference to make it clear that this is not intended to "enhance" existing audio (but rather, intentionally degrade it).

I updated the class reference. Let me know what you think about the changes.

Copy link
Copy Markdown
Member

@Calinou Calinou Nov 19, 2022

Choose a reason for hiding this comment

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

Aside of the formatting change (see below), the documentation looks good to me now 🙂

@M-O-Marmalade M-O-Marmalade force-pushed the audio-effect-sample-rate branch 2 times, most recently from 1e688c7 to b2187a7 Compare November 21, 2022 04:46
@M-O-Marmalade M-O-Marmalade force-pushed the audio-effect-sample-rate branch from b2187a7 to 1b5f63c Compare November 23, 2022 00:42
@M-O-Marmalade
Copy link
Copy Markdown
Contributor Author

M-O-Marmalade commented Nov 23, 2022

If this were going to be merged into the 3.x branch, I assume I would need to open another PR targeted at that branch, correct?

Also, is there anything else I should do at this point? Is this PR and the related proposal all that's needed on my part, in regards to targeting the master branch?

@Calinou
Copy link
Copy Markdown
Member

Calinou commented Nov 23, 2022

If this were going to be merged into the 3.x branch, I assume I would need to open another PR targeted at that branch, correct?

Yes, as the 3.x codebase differs significantly from master at this point.

Also, is there anything else I should do at this point? Is this PR and the related proposal all that's needed on my part, in regards to targeting the master branch?

It's all that's needed on your part. Since 4.0 is currently in feature freeze, it'll take a while for this pull request to be formally reviewed and merged. Feature merges in master will only resume after 4.0 is released.

@M-O-Marmalade
Copy link
Copy Markdown
Contributor Author

Yes, as the 3.x codebase differs significantly from master at this point.

Ok, I'll open a new PR for 3.x soon.

It's all that's needed on your part. Since 4.0 is currently in feature freeze, it'll take a while for this pull request to be formally reviewed and merged. Feature merges in master will only resume after 4.0 is released.

Sweet, thank you for letting me know Calinou! 👍

@M-O-Marmalade
Copy link
Copy Markdown
Contributor Author

M-O-Marmalade commented Dec 15, 2022

It's all that's needed on your part.

@Calinou If you don't mind me asking: should I wait for this feature to be merged before submitting a PR for any required documentation changes (mainly to the Audio Effects page)? Or, should I just submit the docs PR now?

@Calinou
Copy link
Copy Markdown
Member

Calinou commented Aug 8, 2023

@Calinou If you don't mind me asking: should I wait for this feature to be merged before submitting a PR for any required documentation changes (mainly to the Audio Effects page)? Or, should I just submit the docs PR now?

Sorry for the late response. You can open a PR on godot-docs before this PR is merged, but it's not required.

@Calinou
Copy link
Copy Markdown
Member

Calinou commented Jul 24, 2025

Well, feature freeze is over, what's the fate of this pr?

Feature freeze for 4.5 isn't over yet, as 4.5 is currently in beta. At this stage, any new features can only be considered for merging in 4.6 at the earliest.

@AThousandShips

This comment was marked as outdated.

@ObaniGemini
Copy link
Copy Markdown

Is there any hope for it to be merged some day?
There's pretty much zero risk in merging this and I see it's been a few years since it was opened, that's why I'm asking.

@Juicimated

This comment was marked as off-topic.

@elvisish
Copy link
Copy Markdown

A discussion I opened years ago that might be relevant: godotengine/godot-proposals#5103

</brief_description>
<description>
Reduces the input audio's sample rate, without interpolating the output. Also known as "sample &amp; hold", or a type of "bitcrush".
[b]Note:[/b] To reduce an audio signal's bit depth, use the "LoFi" mode in [AudioEffectDistortion].
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.

I think it would be better to refer to the constant directly. Consider:
[b]Note:[/b] To reduce an audio signal's bit depth, use [constant AudioStreamDistortion.MODE_LOFI].

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add an audio effect to reduce the sample rate of processed audio

8 participants