Skip to content

Add support for SDL's audio driver#109362

Draft
kus04e4ek wants to merge 1 commit intogodotengine:masterfrom
kus04e4ek:sdl_audio
Draft

Add support for SDL's audio driver#109362
kus04e4ek wants to merge 1 commit intogodotengine:masterfrom
kus04e4ek:sdl_audio

Conversation

@kus04e4ek
Copy link
Copy Markdown
Contributor

@kus04e4ek kus04e4ek commented Aug 6, 2025

Addresses: godotengine/godot-proposals#12908
Fixes: #98500
Fixes: #82823
Fixes: #30313

Keeping it as a draft until I'll finish the TODO list.

This PR doesn't bring support for the Web platform as the SDL web audio driver functions the same way as Godot's web audio driver pre-#91382, so it'll most likely break web's audio again.

TODO:

  • Refactor the code, it's not yet ready for review;
  • Solve potential deadlocks;
  • Check all of the platforms;
  • Check other GitHub issues to see if this PR solves them.

After merge (sdl_audio_extra branch contains every change marked off in this section):

  • Support changing the SDL backend via the audio/driver/driver setting (here);
  • Report the currently used SDL backend when calling AudioDriverSDL::get_name() (here);
  • Support setting the mix rate. SDL does support it, but it might force devices to use the non-default format which might reduce the number of channels and etc. Requires SDL source code modifications, but again wanted to keep the PR minimal (here);
  • Support getting/setting the latency. SDL doesn't provide the support for that and it requires modifications to the SDL source code, it is possible, but I wanted to keep this PR pretty minimal (getting the latency is implemented here);
  • Support Android and iOS/visionOS (Depending on the merge order, either this PR or Add support for SDL3 joystick input driver for Android, iOS, visionOS and Web #109645 should be modified, I'll gladly help with porting to these platforms either way);
  • Add sdl_audio and sdl_joypad build options instead of the one sdl option. Not really sure how to tackle it and I don't think it's necessary for this PR;
  • Expose the float buffer for input and output to make SDL type conversion more fast. More about it here;
  • Delete AudioDevice implementations that are no longer required. Want to make this after making AudioDriverSDL feature-complete (latency and mix rate support) as some projects might depend on these features;
  • Evaluate support for the backends not currently supported by Godot.

Some of these After merge goals or all of them can be changed to be included in this PR (especially the ones making the driver feature-complete), curious what y'all think.

To check this PR I used slightly modified versions of the demo projects: demos.zip, the driver is set to be SDL and the initialized driver is printed to the console at start, but the device_changer demo also has a new button Update devices, it just updates the list of available devices and if the device currently used got disconnected, the chosen device in the list will change to the new current device

@Nintorch
Copy link
Copy Markdown
Member

That's quite awesome! :D

I have a suggestion, should the driver also report the underlying audio backend when calling AudioDriverSDL::get_name()? See also https://wiki.libsdl.org/SDL3/SDL_GetAudioDriver This way we will know for sure what system audio driver is used for playback.

@kus04e4ek
Copy link
Copy Markdown
Contributor Author

That's quite awesome! :D

I have a suggestion, should the driver also report the underlying audio backend when calling AudioDriverSDL::get_name()? See also https://wiki.libsdl.org/SDL3/SDL_GetAudioDriver This way we will know for sure what system audio driver is used for playback.

Thanks)

I think it should, it would probably make the most sense to implement it with Support changing the SDL backend via the audio/driver/driver setting thing, as that change will be all about get_name, I'll probably try to do this change in a different branch to not make this PR have too much stuff in it, if this is something that will be wanted to be included in this PR, I'll move the commit to this branch

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This file was created due to the fact that if for whatever reason JoypadSDL fails to initialize, but AudioDriverSDL does initialize, there will still be a way to poll sdl events, it's important because AudioDriverSDL doesn't poll events and if nothing polls events the events array will eventually run out of memory

Comment on lines -29 to +30
rm -f $target/include/build_config/{*.cmake,SDL_build_config_*.h} $target
rm -f $target/include/SDL3/SDL_{egl,gpu,oldnames,opengl*,test*,vulkan}.h $target
rm -f $target/include/build_config/{*.cmake,SDL_build_config_*.h}
rm -f $target/include/SDL3/SDL_{egl,gpu,oldnames,opengl*,test*,vulkan}.h
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It was probably just a mistake, without this change there will be an attempt to delete $target

Comment on lines -82 to +90
cp -v hidapi/$dir/*.{c,h} $target/hidapi/$dir
for f in hidapi/$dir/*.{c,h}; do
[ -e "$f" ] && cp -v $f $target/hidapi/$dir
done
Copy link
Copy Markdown
Contributor Author

@kus04e4ek kus04e4ek Sep 28, 2025

Choose a reason for hiding this comment

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

For some reason this fails on my system (Fedora 42), some of the hidapi/$dir$ don't have *.c or *.h files, so I get the cp: cannot stat 'hidapi/hidapi/*.c': No such file or directory error and because it's run like this #!/bin/bash -e, the execution just stops

Comment thread servers/audio_server.cpp
Comment on lines +116 to +129
void AudioDriver::input_buffer_wrote(unsigned int p_frames) {
if ((int)input_position < input_buffer.size()) {
input_position += p_frames;
if ((int)input_position >= input_buffer.size()) {
input_position -= input_buffer.size();
}
if ((int)input_size < input_buffer.size()) {
input_size = MAX(input_size + p_frames, input_buffer.size());
}
} else {
WARN_PRINT("input_buffer_write: Invalid input_position=" + itos(input_position) + " input_buffer.size()=" + itos(input_buffer.size()));
}
}

Copy link
Copy Markdown
Contributor Author

@kus04e4ek kus04e4ek Sep 28, 2025

Choose a reason for hiding this comment

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

Just a function to optimize reading from the SDL buffer, instead of writing 1 frame at a time we'll just write the whole SDL buffer and update input_position/input_size after writing

@Nintorch
Copy link
Copy Markdown
Member

Nintorch commented Sep 28, 2025

Also, another suggestion, should the current audio drivers be removed, i.e. replaced by the SDL ones, in this PR? See Calinou's comment: godotengine/godot-proposals#12908 (comment)

EDIT: I just noticed that you already have this in your TODO list 😅

Delete AudioDevice implementations that are no longer required. Want to make this after making AudioDriverSDL feature-complete (latency and mix rate support) as some projects might depend on these features;

@kus04e4ek
Copy link
Copy Markdown
Contributor Author

Also, another suggestion, should the current audio drivers be removed, i.e. replaced by the SDL ones, in this PR? See Calinou's comment: godotengine/godot-proposals#12908 (comment)

It think it's fine to do it in a similar way to how it was done with the Jolt physics driver, add the driver first, receive more tests from people with different equipment, add more features and then replace existing drivers

EDIT: I just noticed that you already have this in your TODO list 😅

Yeeeeahh, thought it was still important for me to say what I think about it lol

@kus04e4ek
Copy link
Copy Markdown
Contributor Author

That's quite awesome! :D

I have a suggestion, should the driver also report the underlying audio backend when calling AudioDriverSDL::get_name()? See also https://wiki.libsdl.org/SDL3/SDL_GetAudioDriver This way we will know for sure what system audio driver is used for playback.

jfyi, finished this feature in this branch, not really sure if the branch passes all of the tests and haven't really tested it a lot, but it seems to work fine on my linux laptop

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

Projects

None yet

3 participants