Create AudioStreamWithEffects

[RoyBerardo]

This adds a new AudioStream type, AudioStreamEffect AudioStreamWithEffects, which applies audio bus effects directly to audio streams.

This partially addresses godotengine/godot-proposals#8190 ("Partially" because i believe it would be useful to apply effects to audio stream players, but that is a whole other topic)

Effects can be individually bypassed. By enabling process_bypassed_effects, bypassed effects will still process so bypassing and un-bypassing effects like reverb won't result in a burst of sound.

When the child stream is finished playing, this stream fades out for the duration of tail_time, allowing effects like reverb and delay to fade out.

This is currently a draft because I feel it would be useful to get community input on this new stream type. Some things I'd like to get some thoughts/discussion on in particular:

• Should process_bypassed_effects be per-effect? That would allow for more granular control but I worry about cluttering the effect list. UPDATE: Decided to make it per-effect.
• Instead of tail_fade_curve, should there instead just be a bool that simply enables/disables a fade on the tail? UPDATE: Decided to keep the curve.
• Would appreciate it if someone looked over the docs I made, I'm not the best at writing those.
• AudioEffect docs would need to be updated since they talk about audio effects only in the context of busses.

[RoyBerardo]

Should process_bypassed_effects be per-effect? That would allow for more granular control but I worry about cluttering the effect list.

Decided to make it per-effect.

Instead of tail_fade_curve, should there instead just be a bool that simply enables/disables a fade on the tail?

Decided to keep it as a curve, no need to make it less customizable (unless it has a significant perf impact, which I don't believe it does)

Also, updated all the audio effect docs to not have them be bus-exclusive, though I'd love it if someone took a look at those.

[RoyBerardo]

Since there doesn’t seem to be any discussion and I’ve addressed the issues I’ve put forward, I’m opening this PR :)

[fire]

Would it work to make audio mic input a AudioStreamEffect

[RoyBerardo]

Would it work to make audio mic input a AudioStreamEffect

I'm not sure I understand, could you clarify what you mean? If you are asking if AudioStreamMicrophone works with AudioStreamEffect, then the answer is yes :)

[fire]

There's a proposal to move the microphone out of the Godot audio system. #108773 so I was exploring the options.

[RoyBerardo]

Oh I think I understand - If audio input could be an effect placed in the AudioStreamEffect effect list? The effect list houses AudioEffect resources, same things that go on busses, so if someone made an AudioEffect that handled external input then I suppose it would work.

[RoyBerardo]

This also has me wondering if AudioStreamEffect is really the best name for this. Maybe AudioStreamEffectChain or AudioStreamEffectList would be more descriptive. Ultimately this new class isn't an effect, it's a chain/list of AudioEffect resources.

[Mickeon]

This also has me wondering if AudioStreamEffect is really the best name for this. Maybe AudioStreamEffectChain or AudioStreamEffectList would be more descriptive. Ultimately this new class isn't an effect, it's a chain/list of AudioEffect resources.

Indeed, just by the title alone I was rather confused at first. Anything along the lines of AudioStreamEffectChain should be fine, but I believe any word that would convey effects directly applied to the AudioStream would be even better.

[KoBeWi]

AudioStreamWithEffects? AudioStreamProcessed? AudioStreamModifier?

[RoyBerardo]

AudioStreamEffectProcessor? AudioStreamEffectApplicator? (I kinda like AudioStreamEffectApplicator)

[RoyBerardo]

Forgot to comment - Changed it to AudioStreamWithEffects, seems that's the most popular one. Thank you KoBeWi for the suggestion!

[KoBeWi]

I tested and it works correctly, though I found it weird that each effect has a bypass flag and process when bypassed; like, if you want to bypass effect, why de-bypass it? Though then I've read the description (both in the PR and in the documentation), and I can't tell difference when this flag is enabled.

Also I wonder if it's possible to get AudioEffectInstance from the effects? Currently instances can only be retrieved with get_bus_effect_instance() from AudioServer, but it only works with buses.

[KoBeWi]

Suggested change

	[AudioStreamWithEffects] applies [AudioEffect]s to a child [AudioStream]. Effects are applied in the order in which they are listed.
	[AudioStreamWithEffects] applies [AudioEffect]s the assigned [member stream]. Effects are applied in the order in which they are listed.

[Mickeon]

Mmmm

Suggested change

	[AudioStreamWithEffects] applies [AudioEffect]s to a child [AudioStream]. Effects are applied in the order in which they are listed.
	An [AudioStreamWithEffects] is an audio stream that can apply one or more [AudioEffect] to the assigned [member stream]. Each effect is applied in the order in which it is listed.

[RoyBerardo]

The plural [AudioEffect]/[AudioEffect]s is a bit of a head scratcher for me here. Is there a correct way to go about this that's consistent with the other docs? I like Mickeon's suggestion with that whole first sentence, but [AudioEffect] being singular is throwing me off.

Also I'm personally more a fan of "Effects are applied in the order in which they are listed.". Makes more sense to me. Could be modified to this though:
"The effects are applied in the order in which they are listed."

[KoBeWi]

Suggested change

	effect_playback->effect_instances.resize(effects.size());
	for (int i = 0; i < effects.size(); i++) {
	if (effects[i].effect.is_valid()) {
	effect_playback->effect_instances.write[i] = effects[i].effect->instantiate();
	effect_playback->effect_instances.reserve(effects.size());
	for (const EffectEntry &effect_entry : effects) {
	if (effect_entry.effect.is_valid()) {
	effect_playback->effect_instances.append(effect_entry.effect->instantiate());

After you change instances to LocalVector.

[RoyBerardo]

Just updated everything to LocalVector. I originally did what you had done here, but was getting some bugs with syncing the effects. In order to allow adding/removing/reordering/replacing effects without stopping the playback, stream->effects and playback->effect_instances need to have AudioEffects and AudioEffectInstances paired up at the same indices. So if stream->effects has an AudioEffectReverb at index 5, playback->effect_instances needs to have an AudioEffectInstanceReverb at index 5. What this also means is that if the user has some indices with nothing set on them, playback->effect_instances also needs matching empty indices. Because this appends to playback->effect_instances instead of writing to specified indices, a mismatch happens if the stream has null entries in its effects vector when the playback is instantiated.

[Mickeon]

I do have more thoughts but I'll wait a bit on implementation details.

[Mickeon]

Mmmm

Suggested change

	[AudioStreamWithEffects] applies [AudioEffect]s to a child [AudioStream]. Effects are applied in the order in which they are listed.
	An [AudioStreamWithEffects] is an audio stream that can apply one or more [AudioEffect] to the assigned [member stream]. Each effect is applied in the order in which it is listed.

[Mickeon]

This would be the first time "set-up" is used in the class reference

Suggested change

	[b]Note:[/b] Different effects can require vastly different amounts of processing power. Be mindful of how often intensive effects such as reverb and delay are used with your set-up.
	[b]Note:[/b] Different effects can require vastly different amounts of processing power. Be mindful of how often intensive effects, such as reverb and delay, are processed in your project.

or similar.

Also, as true as this is, I believe not anywhere in the class reference or manual it is stated how performance-intensive each effect is. So it's all very up to interpretation, besides the listed examples.

[RoyBerardo]

Also, as true as this is, I believe not anywhere in the class reference or manual it is stated how performance-intensive each effect is. So it's all very up to interpretation, besides the listed examples.

That's true. I wonder if it'd be useful to benchmark and document the performance intensity of each effect, and include the results (even as high/moderate/low) in the audio effects tutorial doc. If that were done, it would be nice if that and this PR were merged side-by-side.

Such a document could be merged before this PR, but IMO there isn't much sense in pointing out performance hits while effects can only go on busses; it could potentially lead to people trying to optimize their project in the wrong places.

[Mickeon]

I don't believe there's a need to separate these sentences in different paragraphs.

Suggested change

	Sets bypass on the effect at the specified index.
	Bypassing an effect disables the effect's influence on the sound, essentially turning it off.
	Sets bypass on the effect at the specified index to [param enabled]. Bypassing an effect disables the effect's influence on the sound, essentially turning it off.

What's the point of toggling bypass when, if desired, one can remove and re-add the audio effect to a AudioStreamWithEffects to achieve a similar effect?

[RoyBerardo]

What's the point of toggling bypass when, if desired, one can remove and re-add the audio effect to a AudioStreamWithEffects to achieve a similar effect?

Some reasons off the top of my head:

• Better for memory management - you're not creating and destroying and creating and destroying objects.
• Allows turning off effects at an index without regard to what the effect is.
• (similar to the above point) if someone adds an effect via the effect dropdown->new->[effect], thus creating a localized effect resource just for that AudioStreamWithEffects, they'd need to figure out a way to hold on to it during runtime before they remove it if they want to bring it back.
• Allows setting/getting an effect at that slot while it is currently bypassed.
• Allows tweaking an effect's parameters while it is currently bypassed.
• Consistency with the bus which already includes bypassing.
• Consistency with DAWs, Wwise, Fmod, Unity, Unreal.

[Mickeon]

Perhaps just...?

Suggested change

	The total amount of effect slots in the [AudioStreamWithEffects].
	The number of effects added to this [AudioStreamWithEffects].

or even

Suggested change

	The total amount of effect slots in the [AudioStreamWithEffects].
	The number of effects added to this audio stream.

[RoyBerardo]

I like your second suggestion, and I'll use that.

I was wondering though, and it's probably nothing, but technically this refers to the size of the effects list, regardless of if effects are set on it or not. I was mentally toying with potential alternate names for this member when I checked out the documentation for AudioStreamPlaylist, AudioStreamRandomizer, and AudioStreamSynchronized. Each has a list of streams and a member that behaves like the one here. They all seem to use similar language to what you wrote above, so I imagine I'm just overthinking and it's fine.

[RoyBerardo]

I tested and it works correctly, though I found it weird that each effect has a bypass flag and process when bypassed; like, if you want to bypass effect, why de-bypass it? Though then I've read the description (both in the PR and in the documentation), and I can't tell difference when this flag is enabled.

When it comes to effects like reverb, if you bypass it while audio is playing, it sort of "stores" the reverb state and holds onto that until you un-bypass. This means that when you un-bypass reverb, it will emit the reverb that it was "storing" when the reverb was bypassed. If the stored reverb is released during silence, that can be quite jarring. Processing the reverb while bypassed means that you won't hear the effect but it will still be running under the hood in order to prevent a burst of sound.

This all being said, your comment is making me think that perhaps this should be something to tackle from the side of the effect, not the side of whatever processes the effects. From what I've seen, effects really have no way of knowing if they're bypassed or not, so they cannot do actions when bypassed/un-bypassed, such as resetting their state in the case of reverb. May be good to open an issue/proposal about this. There would be no need for "process when bypassed" if effects could do this. Plus, this issue isn't exclusive to AudioStreamWithEffects, it happens with effects placed on busses as well.

Also I wonder if it's possible to get AudioEffectInstance from the effects? Currently instances can only be retrieved with get_bus_effect_instance() from AudioServer, but it only works with buses.

Oh actually I completely forgot to do this. I'll add that.

[RoyBerardo]

Updated with the above suggestions, plus a couple more things.

Changes:

• Removed the editor plugin (Switched from ADD_ARRAY to ADD_ARRAY_COUNT)
• Made effect_playback->effect_instances a LocalVector
• Added AudioServer locking/unlocking to avoid thread issues
• All functions that use indexes can now use negative indexes
• Removed "Process when bypassed"
• Added AudioStreamPlaybackWithEffects.get_effect_instance() and AudioStreamPlaybackWithEffects. get_effect_instance_count() (Originally added the second function for testing/debugging, but figured it wouldn't hurt to keep it around)
• Added documentation for AudioStreamPlaybackWithEffects
• Looooots of documentation changes

Appreciate all the review and feedback!