Is anyone able to take a look at this? I can appreciate it might be hard to test without an affected audio setup. I have a (crude) change to the Audio Dummy Driver that could help with testing, if the audio setup is too much of an issue. The change just exposes the speaker mode as a configurable parameter when the dummy driver is active in the editor.

Letting the tests run.
There's very few maintainers that are keenly aware of the audio side of Godot so this may take a while to be taken a look at.

Is anyone able to take a look at this? I can appreciate it might be hard to test without an affected audio setup. I have a (crude) change to the Audio Dummy Driver that could help with testing, if the audio setup is too much of an issue. The change just exposes the speaker mode as a configurable parameter when the dummy driver is active in the editor.

I have a game where the entire gameplay is recording and playing back audio via AudioEffectRecord from players' microphones, and many players have reported that the recording doesn't work. Input is working, because they can hear themselves when the bus isn't muted and AudioEffectSpectrumAnalyzer works just fine. Only the recording is failing. Do you think my game's issue could also be due to the bug you describe? If so, what's the best way to try out your fix in my version of Godot? Apologies, I'm unfamiliar with both Github and how to change the Godot Engine's own code.

I'm also experiencing this - as long as the output is a stereo 2.0 channel, recording works fine, but any HiFi / Surround configuration makes AudioEffectCapture unusable for my use case (speech to text).

I have a game where the entire gameplay is recording and playing back audio via AudioEffectRecord from players' microphones, and many players have reported that the recording doesn't work. Input is working, because they can hear themselves when the bus isn't muted and AudioEffectSpectrumAnalyzer works just fine. Only the recording is failing. Do you think my game's issue could also be due to the bug you describe?

I do think this is the bug. AudioEffectRecord only records from the last channel (because of the bug), which on anything more than a stereo system will be silent (microphones normally only record stereo to the first channel). This should fix it.

The SpectrumAnalyzer is one of the only effects that is channel aware, which is why it works. You select the effect instance (which is per-channel) with AudioServer.get_bus_effect_instance(bus, effect), but there's a third channel argument that defaults to 0 (the main stereo channel).

If so, what's the best way to try out your fix in my version of Godot? Apologies, I'm unfamiliar with both Github and how to change the Godot Engine's own code.

Unfortunately I'm new here too, and am not sure what the best way to test would be. I setup a dev environment to build and test my own releases, but I don't think sharing executables is the way to go. Maybe one of the main contributors can chime in.

I'm also experiencing this - as long as the output is a stereo 2.0 channel, recording works fine, but any HiFi / Surround configuration makes AudioEffectCapture unusable for my use case (speech to text).

If this is the bug, the audio should sound very garbled and static-y. A work around I use in my own project (in GDScript) is to pull frames from the capture buffer in chunks of 512. Then before passing that array on, I loop over it, and check that both channels of every frame is exactly equal to 0.0. If so, I skip that chunk. This works because the AudioServer processes 512 frames at a time, per channel, so your good data should be in amongst 512 frame chunks of silence.

@OpenFormEon I was looking at the Godot Contribution Workflow docs and see that you can download the editor with this PR applied for testing, once the workflow/tests are run on it again, or by using the nightly.link website. Details available here.

is there any solution, I have the same problem in my Ubuntu 24 the same code runs fine in Windows.

AudioEffectCapture get_buffer() will return only [(0,0), (0,0), (0,0)..... (0,0)]

is there any solution, I have the same problem in my Ubuntu 24 the same code runs fine in Windows.

AudioEffectCapture get_buffer() will return only [(0,0), (0,0), (0,0)..... (0,0)]

From the description, I'm not sure this is the same bug. It should be OS agnostic, but maybe the Ubuntu audio driver being used is a surround sound configuration, whereas the Windows system is stereo.

The effect on AudioEffectCapture is to have some of the get_buffer() contents be good, but some be 0.0 frames. If all the frames are empty, that might be another issue. The AudioEffectRecord will return all empty frames, though.

Are you able to pass the audio you're trying to capture out the master audio bus, to confirm it's actually the audio effect that is at fault, rather than the audio device or driver configuration?

Hi!

Instead of adding the "set_current_channel" method to the effect, wouldn't it be better to pass the current channel to the process method?

By changing the method signature like this?
virtual void process(const AudioFrame *p_src_frames, AudioFrame *p_dst_frames, int p_frame_count, int current_channel);

Also, instead of selecting a single channel for the capture effect, wouldn't it be better to move the RingBuffer inside the effect instance?
This way, it would be possible to access the frames of each channel instead of registering a capture effect and enabling it for each available channel.

The methods to retrieve the buffer could be moved to the instance (accessible through AudioServer.get_bus_effect_instance()), and they could also remain on the "total effect," which would handle combining all instance buffers and averaging all frames.

Instead of adding the "set_current_channel" method to the effect, wouldn't it be better to pass the current channel to the process method?

By changing the method signature like this? virtual void process(const AudioFrame *p_src_frames, AudioFrame *p_dst_frames, int p_frame_count, int current_channel);

That could work... I decided on set_current_channel for a few reasons:

• a set_current_channel was already being used by one effect (compressor) so there was precedence, and an existing place in the AudioServer code where it was set
• each effect instance is only ever on a single channel that never changes, so setting it at instantiate seemed sensible
• I didn't want to add extraneous parameters to other existing functions (e.g. process, effect instance instantiate) to minimize the impact on existing code

I felt set_current_channel was the simplest means of achieving that (i.e. effects that don't need it don't have to implement it; nor do they have to update their process or other inherited functions to accommodate a new parameter).

Also, instead of selecting a single channel for the capture effect, wouldn't it be better to move the RingBuffer inside the effect instance? This way, it would be possible to access the frames of each channel instead of registering a capture effect and enabling it for each available channel.

The methods to retrieve the buffer could be moved to the instance (accessible through AudioServer.get_bus_effect_instance()), and they could also remain on the "total effect," which would handle combining all instance buffers and averaging all frames.

This was actually something I had thought of after I finished the current implementation, and was hoping someone might bring up. I do think it could be better by moving the buffer into the instance. My initial criticism of the idea was I didn't want to add the burden of storing up to 4x the memory for the buffers... but actually writing it out now, that seems a little silly. The buffers are tiny, relatively speaking, so it's probably not as much of an issue as I initially thought.

I'm not sure about the averaging frames, though. That doesn't make sense to me from an audio processing point of view. I think a better option would be for the main audio effect to just return the channel 0 instance data. So using the effects works as intended for 99% of cases (and existing code works without change), but if you need other channels, you can access them via the get_bus_effect_instance.

This change would also mean I could remove the set_current_channel function from AudioEffectInstance.

I like this solution better than the current one, so I'll implement it on another branch (for now), and see how it turns out. In the meantime, any contributors/reviewers (or anyone else) have thoughts or questions about it?

As for the "set_current_channel," even if it's no longer used, I would suggest keeping it as a standard so that future effects can generally know which channel they are on.

As for averaging all the ring buffers into a single one, I agree with you. We can start with just channel 0, as long as the individual buffers are accessible—processing the audio in other ways won’t be complicated.

Updated the PR. The changes didn't feel significant enough to justify making a different PR, so I just pushed them here.

I removed the active_channel property, and moved the core capture/record logic from the AudioEffects into the AudioEffectInstances. I duplicated the effect methods and properties on the instances, and changed the base effects to pass through to the channel 0 instances. I did keep, and ended up needing, set_current_channel in order to keep track of the channel 0 instances in the base effect. Because of the way cleanup is done with reference counting, I can't keep a reference to the base effect in the instance, and a reference to the instances in the effect, so I clear the base reference once it's no longer necessary.

I rearranged the cpp files a bit to hopefully give an easier to follow order to them. I put effect instance methods at the top, starting with process, then other important methods, then other setters/getters, and finally _bind_methods and constructor/destructor. Then the base effect methods are after, following the same pattern. It makes the PR kind of messy, but hopefully the long term benefits outweigh the mess.

I updated the built in documentation for the affected classes, but I'm not particularly happy with it, and would welcome changes or feedback on it.

I changed AudioEffectInstance to now expose set_current_channel in GDExtension/GDScript/C#, to match the existing process and process_silence methods.

Hi everyone, sorry to keep bringing this up, but I was wondering if there has been any progress in this topic. Do you know if the issue is expected to be addressed in any upcoming Godot releases? What's a realistic target?

If I am not mistaken the PR #105934 which has passed all the reviews, will provide a work-around by forcing the Godot to work with the standard 2 channels (1 stereo channel) no matter what the hardware, which avoid the bugged condition.

In addition to that, it gives other people (e.g. on the audio team) without a 7.1 setup the tools to actually test and verify this change works, and fixes the issue. I'm gonna push for this PR once #105934 is merged.

I think it might be able to get into 4.6.

Thank you, waiting for 4.6 then :)

So I added some channel parameters to the capture and record effect methods (see updates in top level description above), which all include default parameters to ensure backwards compatibility of the interfaces, but the Linux Editor w/ Mono build is giving validation errors:

Error: Validate extension JSON: Error: Field 'classes/AudioEffectCapture/methods/can_get_buffer/arguments': size changed value in new API, from 1 to 2.
Validate extension JSON: Error: Field 'classes/AudioEffectCapture/methods/get_buffer/arguments': size changed value in new API, from 1 to 2.
Validate extension JSON: Error: Hash changed for 'classes/AudioEffectCapture/methods/can_get_buffer', from 429285F9 to B8008355. This means that the function has changed and no compatibility function was provided.
Validate extension JSON: Error: Hash changed for 'classes/AudioEffectCapture/methods/get_buffer', from 9DECB125 to 6CABA955. This means that the function has changed and no compatibility function was provided.

I thought setting a default value in the ClassDB::bind_method was enough, but I guess C# (or at least the validation script for it) expects something more. Anyone able to point me in the right direction?

Edit: I've been pointed to Handling compatibility breakages, will fix this up then get some eyes on the changes.

I'm not really in the audio team, but what is keeping this pull request from being active? It's currently a draft.

I think I switched it to draft while I was making changes, and was waiting for a check to run, then forgot to recheck it (or rather, thought I would get an email about something...).

I made some more changes to this, to fix some issues I still had with how instances were configured.

I didn't like having a separate initialization function for instances (i.e. init_instance()) that only mattered if the instance needed to know about which channel it's on. I didn't want to change instantiate() to take a channel parameter, since that doesn't follow the instantiate pattern everywhere else in the engine. The only place that seemed to make sense to put the channel info is in the AudioEffect before instantiate is called, letting the effect make config decisions at the same time as the rest of the instance is configured.

So I removed init_instance() from AudioEffect, and set_current_channel() from the AudioEffectInstance. I added a channel member variable to AudioEffect (accessible with get_channel() const), which gets set just before instantiate() is called.

AudioEffect also has a new set_channel_count() virtual function that can be overridden if the effect needs to know the total number of channels on the bus. It gets called whenever audio bus effects gets updated, before any instances are created.

So the bit @Ivorforce picked up on was where you set the temporary channel value in the AudioEffect so it could be picked up by the instantiator of the AudioEffectInstance https://github.com/godotengine/godot/pull/92532/files?diff=unified&w=0#diff-231a4ec4e4b696f12c52f22be20a1d0da220554698c7b6dbabf230ced95308aeL1095

The better option is to set the channel value in the AudioEffectInstance after it is instantiated. However this means you cannot write it in the correct place in the Vector<Ref<AudioEffectRecordInstance>> instances array of the AudioEffect object.

This appears to be more a requirement on the AudioEffect than on the AudioEffectInstance objects.

What if you just pushed the AudioEffectInstances onto the array as they were created, and then called a function like AudioEffect::all_instances_created() to do the cleanup and arrange them in the correct places at the end of the loop? There might also be other important setups that could be done at that point for some effects when all the instances are present.

So the bit @Ivorforce picked up on was where you set the temporary channel value in the AudioEffect so it could be picked up by the instantiator of the AudioEffectInstance https://github.com/godotengine/godot/pull/92532/files?diff=unified&w=0#diff-231a4ec4e4b696f12c52f22be20a1d0da220554698c7b6dbabf230ced95308aeL1095

The better option is to set the channel value in the AudioEffectInstance after it is instantiated. However this means you cannot write it in the correct place in the Vector<Ref<AudioEffectRecordInstance>> instances array of the AudioEffect object.

This appears to be more a requirement on the AudioEffect than on the AudioEffectInstance objects.

What if you just pushed the AudioEffectInstances onto the array as they were created, and then called a function like AudioEffect::all_instances_created() to do the cleanup and arrange them in the correct places at the end of the loop? There might also be other important setups that could be done at that point for some effects when all the instances are present.

I could make that would work, and is almost exactly the same as what I had before with AudioEffect::init_instance(). I could remove the AudioEffect::set_channel, re-add the AudioEffectInstance::set_current_channel(), and add a virtual AudioEffect::finalize().

Thinking about it a bit, I could actually remove AudioEffect::set_channel_count too. In instantiate() I'd add each new instance to a temp list, then reorder and put into the actual list in finalize(); no need to pre-allocate the instances vector.

AudioServer would then look something along the lines of:

void AudioServer::_update_bus_effects(int p_bus) {
	for (int i = 0; i < buses[p_bus]->channels.size(); i++) {
		buses.write[p_bus]->channels.write[i].effect_instances.resize(buses[p_bus]->effects.size());
		for (int j = 0; j < buses[p_bus]->effects.size(); j++) {
			Ref<AudioEffectInstance> fx = buses.write[p_bus]->effects.write[j].effect->instantiate();
			fx->set_current_channel(i);
			buses.write[p_bus]->channels.write[i].effect_instances.write[j] = fx;
		}
	}
	for (int j = 0; j < buses[p_bus]->effects.size(); j++) {
		buses.write[p_bus]->effects.write[j].effect->finalize();
	}
}

Thoughts?

I implemented the changes described in my previous comment.

test-audio-capture.zip

Here is a small demo project that I made for testing and demonstrating changes to AudioEffectCapture.
There's a second scene in the project that shows some of the functionality of this PR, capturing four stereo channels independently.

Attached is a video of the demo scene. The first run is default Godot 4.5 playing back audio captured with 7.1 surround sound.
The second run uses my work around to reject empty audio frames when reading from the capture buffer.
The third run is just what a normal stereo audio config sounds like.