Bug #2121
closedSDL3 audio not supported in qt, gtk3.22, gtk4 clients.
0%
Description
It appears that this was overlooked when the SDL3 audio support was added, encountered downstream on a system without SDL2/SDL.h -
FAILED: [code=1] freeciv-qt.p/client_gui-qt_gui_main.cpp.o
x86_64-pc-linux-gnu-g++ -Ifreeciv-qt.p -I. -I../freeciv-R3_2_3 -I../freeciv-R3_2_3/dependencies/luasql/src -I../freeciv-R3_2_3/dependencies/tinycthread -I../freeciv-R3_2_3/dependencies/tolua-5.2/include -I../freeciv-R3_2_3/dependencies/cvercmp -I../freeciv-R3_2_3/utility -I../freeciv-R3_2_3/common -
../freeciv-R3_2_3/client/gui-qt/gui_main.cpp:26:10: fatal error: SDL2/SDL.h: No such file or directory
26 | #include <SDL2/SDL.h>
| ^~~~~~~~~~~~
#ifdef AUDIO_SDL /* Though it would happily compile without this include, * it is needed for sound to work. * It defines "main" macro to rename our main() so that * it can install SDL's own. */ #ifdef SDL2_PLAIN_INCLUDE #include <SDL.h> #else // PLAIN_INCLUDE #include <SDL2/SDL.h> #endif // PLAIN_INCLUDE #endif // AUDIO_SDL
and similar. Have a patch for 3.2.5 that looks good, will attach - builds without SDL2/SDL.h and has audio, calling that a win. :)
Files
Updated by Matt Jolly 23 days ago
Okay, looking a bit closer it's just about the main() macro which we don't appear to need for SDL3.
Included a fix for 3.2.5 that applies, however this will need to be ported to the gtk5 client in main.
Playtested the first turn or two on the modified clients - everything seems fine. I have not tested the client/audio.c with SDL2, but it should be fine - This change was required as the menu music was looping on exit in testing with all clients. Suspect SDL3 related but I haven't put much effort into tracking it down - it behaves as it should.
Updated by Marko Lindqvist 23 days ago
Updated by Matt Jolly 21 days ago
Ack, had to build off 3.2.5 due to the state of the clients when I was working off this and didn't check if `main` had been fixed. Feel free to drop that section of my patch or if you'd prefer that I submit one without that change I'm happy to do that.
Updated by Marko Lindqvist 20 days ago
- File 0047-Do-not-include-SDL2-headers-when-SDL3-audio-enabled.patch 0047-Do-not-include-SDL2-headers-when-SDL3-audio-enabled.patch added
- File 0023-Do-not-include-SDL2-headers-when-SDL3-audio-enabled.patch 0023-Do-not-include-SDL2-headers-when-SDL3-audio-enabled.patch added
- Status changed from New to In Review
- Assignee set to Marko Lindqvist
Attached patches are a bit more future-proof as they consider sdl2 - not sdl3 - the special case.
There's a chance that this breaks sdl3 audio support for clients other than sdl3-client on Windows, as the sdl's main() wrapping has always worked very differently there compared to other OSes. But we can't test that at the moment as we still lack sdl3 audio support for Windows in general. I guess we go with this for now, and deal with any possible Windows issues when we implement sdl3 audio for it. sdl2 audio on Windows need to be tested now, though.