Skip to content

Play sound effects through SDL_mixer on Linux#531

Open
nolt wants to merge 3 commits into
sven-n:mainfrom
nolt:linux-sfx-sdl-mixer
Open

Play sound effects through SDL_mixer on Linux#531
nolt wants to merge 3 commits into
sven-n:mainfrom
nolt:linux-sfx-sdl-mixer

Conversation

@nolt

@nolt nolt commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Implements the sound-effect backend for platforms without DirectSound, so the
native Linux client has working SFX instead of silence. Follow-up to #462
music already played through SDL_mixer; effects were stubbed out as no-ops.

Branch: linux-sfx-sdl-mixer (2 commits, +441/−19)

What changes

  • New src/source/Audio/SdlSfxPlayer.cpp — implements the DSPlaySound.h
    API on top of SDL3_mixer, guarded by #ifndef _WIN32. Windows keeps using
    DirectSound; nothing on that path changes.
  • DSPlaySound.cpp — the non-Windows no-op stubs are removed, since the
    real implementation now provides those symbols.
  • AudioPlayer — exposes GetMixer(), so effects reuse the mixer device
    that music already opened instead of opening a second one.
  • WinCompat.h — adds S_FALSE, needed by the "already loaded" return
    path that mirrors the DirectSound backend.

Design notes

One mixer device, shared with music. AudioPlayer::Initialize() runs first
(Winmain.cpp), and FreeDirectSound() runs before AudioPlayer::Shutdown(),
so the mixer outlives every track. If music failed to initialise there is no
audio device at all, and effects stay disabled rather than opening their own.

Behavioural parity with DirectSound, deliberately, including the quirks:
one decoded buffer per ESound with a round-robin channel pool, S_FALSE on
a repeated load, and SetMasterVolume overwriting per-buffer volumes. Volumes
stay on DirectSound's scale (hundredths of a dB, 0-10000) and are
converted to linear gain, so call sites need no changes.

Effects are predecoded (MIX_LoadAudio(..., predecode=true)) — they are
short and replayed constantly, so the memory trade buys zero decode latency at
play time. Music stays streamed.

No 3D spatialisation. The DirectSound backend never enables it either (its
3D flag has no callers), so parity here is 2D and Set3DSoundPosition() is
empty.

The one non-obvious part

The engine assumes an undocumented DirectSound guarantee: Play() on a
buffer that is already playing is a no-op. Several call sites depend on it —
PlayWorldAmbientSounds() re-requests the ambient loop on every frame, and
ZzzCharacter.cpp does the same for the blacksmith's hammer while its
animation is in range.

SDL_mixer does the opposite: MIX_PlayTrack restarts a playing track. The
first build turned that assumption into audible corruption — ambient loops
never got past a few milliseconds and became a continuous buzz, and the anvil
hit was preceded by a burst of crackling. The blacksmith was worst affected
because its buffer is loaded with a single channel, so there was no round-robin
to spread the retriggers across.

The second commit restores the DirectSound semantics: skip the request when
the selected track is still playing.

This is worth flagging beyond this PR — the assumption is not written down
anywhere, and any future audio backend will hit it.

Testing

Built and run as the native Linux client (Ubuntu 24.04, x86-64):

  • Blacksmith (Hanzo): one clean anvil hit, no crackling.
  • World ambient loops (Lorencia wind, etc.): smooth, no buzz.
  • One-shot effects (UI clicks, combat, item pickup): unaffected.
  • Compiles warning-free; no Windows-path code touched.

nolt added 2 commits July 17, 2026 07:48
The non-Windows sound-effect API was a silent no-op pending a portable
backend (issue sven-n#462). Implement it on SDL3_mixer, reusing the mixer
device that AudioPlayer already creates for music: one decoded buffer per
ESound with a small round-robin track pool, volumes mapped from
DirectSound's hundredths-of-a-decibel scale, and looping requested at
play time. Behavior mirrors the DirectSound backend in 2D; 3D
spatialization is omitted since that backend never enables it either.

Add S_FALSE to the Win32 compatibility shim for the already-loaded case.
DirectSound treats Play() on an already-playing buffer as a no-op, and
the engine relies on that: the world ambient loops (SceneManager) and the
blacksmith's hammer (ZzzCharacter) re-request their sound on every frame.
MIX_PlayTrack restarts the track instead, so the sample was cut a few
milliseconds in, sixty times a second: the ambient loops turned into a
constant buzz and the anvil hit was preceded by a burst of crackling.
The blacksmith was hit hardest because its buffer has a single channel,
with no round-robin to spread the retriggers over.

Skip the play request when the selected track is still playing, matching
the DirectSound behaviour the call sites were written against.
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request introduces a functional sound-effect backend for the Linux client by leveraging SDL3_mixer. It replaces previous no-op stubs with a robust implementation that maintains behavioral parity with the Windows DirectSound backend, including volume scaling and handling of redundant play requests. The changes ensure that sound effects are properly integrated into the existing audio infrastructure without requiring modifications to the game's high-level audio call sites.

Highlights

  • SDL_mixer Backend Implementation: Added a new sound-effect backend using SDL3_mixer for non-Windows platforms, enabling native Linux audio support.
  • DirectSound Semantic Parity: Implemented logic to ensure that re-triggering an already-playing sound is ignored, matching the behavior of the existing DirectSound backend and preventing audio corruption.
  • Resource Management: Shared the existing mixer device with the music system to avoid opening multiple audio devices and implemented pre-decoding for short sound effects to minimize latency.
New Features

🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request implements a non-Windows sound-effect backend using SDL3_mixer, replacing the previous silent no-op implementation for platforms without DirectSound. It also exposes the shared SDL mixer device from the AudioPlayer. The review feedback highlights critical improvements for thread safety and stability, specifically suggesting the use of atomic variables for enableSound_ and mixer_ to prevent data races and potential crashes when the mixer is null. Additionally, it recommends optimizing StopAll() to lock the mutex once rather than repeatedly inside a loop, and adding the header.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/source/Audio/SdlSfxPlayer.cpp
Comment thread src/source/Audio/SdlSfxPlayer.cpp
Comment thread src/source/Audio/SdlSfxPlayer.cpp
Comment thread src/source/Audio/SdlSfxPlayer.cpp
…lock

- IsEnabled() also requires a mixer. SetEnableSound() cannot know that
  Initialize() failed, so without this a caller could enable sound with no
  device behind it. SDL_mixer tolerates a null mixer today, so this is
  defence in depth rather than a crash fix.
- Extract StopEntryTracks() and take the mutex once in StopAll(), instead
  of locking per buffer and repeating the loop body. A single map change
  was taking a thousand lock/unlock pairs.
@Mosch0512 Mosch0512 self-assigned this Jul 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants