New SampleRate audio effect#68768
Conversation
There was a problem hiding this comment.
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.
|
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. |
|
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. |
3.x is accepting new features!
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. |
48eeeb4 to
43819fa
Compare
|
Sounds good! Thank you for your advice! I'll open a proposal ASAP and link it here. |
|
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). |
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
Aside of the formatting change (see below), the documentation looks good to me now 🙂
43819fa to
fb3ce9e
Compare
1e688c7 to
b2187a7
Compare
b2187a7 to
1b5f63c
Compare
|
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? |
Yes, as the
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 |
Ok, I'll open a new PR for 3.x soon.
Sweet, thank you for letting me know Calinou! 👍 |
@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. |
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. |
This comment was marked as outdated.
This comment was marked as outdated.
|
Is there any hope for it to be merged some day? |
This comment was marked as off-topic.
This comment was marked as off-topic.
|
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 & hold", or a type of "bitcrush". | ||
| [b]Note:[/b] To reduce an audio signal's bit depth, use the "LoFi" mode in [AudioEffectDistortion]. |
There was a problem hiding this comment.
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].
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