SDL2_mixer changed how it upsamples sounds, and some users complained
about the difference. Add an internal resampler that emulates how
SDL_mixer upsamples sounds. Since all Rebirth upsampling is an integer
upsample (11Khz -> 44KHz, 22KHz -> 44Khz), this internal emulation is
considerably simpler than a general purpose resampler.
With this commit, the builder can choose which resamplers to enable.
The available resamplers are chosen by preprocessor directive, and
presently do not have an SConstruct flag. For each resampler, if no
choice is made, then the resampler will be enabled if it is reasonable.
At least one of the resamplers must be enabled, or the build will fail
with a `#error` message.
The user may choose at program start time which of the available
resamplers to use for that execution of the program, through passing one
of the command line arguments:
- `-sdlmixer-resampler=sdl-native`
- `-sdlmixer-resampler=emulate-sdl1`
- `-sdlmixer-resampler=emulate-soundblaster16`
Runtime switching is not supported. If the user does not choose, then
the first enabled resampler from the list below will be used. The
available resamplers are:
- sdl_native (DXX_FEATURE_EXTERNAL_RESAMPLER_SDL_NATIVE) - delegates to
SDL_mixer / SDL2_mixer, the way Rebirth has historically done.
- emulate_sdl1 (DXX_FEATURE_INTERNAL_RESAMPLER_EMULATE_SDL1) - an
internal resampler that emulates how SDL_mixer worked. This should be
equivalent to sdl_native when using SDL_mixer, so by default it is
enabled when Rebirth is built to use SDL2_mixer and disabled when
Rebirth is built to use SDL_mixer. It can still be enabled manually
even when building for SDL_mixer, but this does not seem likely to be
useful.
- emulate_soundblaster16
(DXX_FEATURE_INTERNAL_RESAMPLER_EMULATE_SOUNDBLASTER16) - an internal
resampler submitted by @raptor in
5165efbc46. Some users reported audio
quality issues with this resampler, so it is not presently the
default.
The new resampler sets upFactor to either `2` (for 22Khz sounds) or
`4` (for 11Khz sounds). However, the logic to pick which coefficient
table to use tested for `0` versus non-zero, so it always picked the
coefficient table meant for 22Khz sounds, even when the sample was an
11Khz sound. This caused a high pitched ringing in sampled sounds.
Switch to use an `enum class` to prevent using zero-vs-nonzero tests
without a cast. Change the existing test to test for the 22Khz `enum`
value, as originally intended. In testing, this eliminated the ringing
effect in 11Khz sounds.
Identified-by: Chris <https://forum.zdoom.org/viewtopic.php?p=1234989> (in response to a request by KynikossDragonn)
Fixes: 5165efbc46 ("Use custom fixed-point audio resampler: - Hopefully fixes issues with poor quality resampling when using SDL_AudioCVT - Converts 11025 Hz or 22050 Hz samples to the default 44100 Hz outputs - Uses high order brick wall filter like Sound Blaster 16")
In any given run of the program, either the SDL_mixer code will be used,
or it will not be used. `digi_sample_rate` only needs to vary if a
single run both uses SDL_mixer and avoids it. Make `digi_sample_rate` a
`static const` with an appropriate value for each path.
Add the SDL_mixer-based resampler as a compile-time alternative, which
can be chosen by setting `-DDXX_FEATURE_INTERNAL_RESAMPLER=0`. Keep the
new internal resampler as the default.
Observe that `nn` is at most `signal.size() - 1`, which occurs on the
final pass of the loop. Therefore, `std::min` will always pick `nn`
instead of `signal.size() - 1`, so simplify out the call.
The Rebirth main screen grd_curscreen borrows the memory allocated by
SDL_CreateRGBSurface to store its data. Store the pointer to that
surface inside grd_curscreen and make grd_curscreen responsible for
freeing the surface.
Per the header comments, passing nullptr for source or destination
copies the entire region. Use that instead of explicitly requesting the
entire region. This should be more efficient, since it bypasses the
source rectangle clipping code that SDL would otherwise run.
- Hopefully fixes issues with poor quality resampling when using SDL_AudioCVT
- Converts 11025 Hz or 22050 Hz samples to the default 44100 Hz outputs
- Uses high order brick wall filter like Sound Blaster 16
If the song originates in an m3u playlist, no temporary buffer is
needed. Avoid allocating and initializing one, and instead pass the
pointer directly from the CMLevelMusicPath field.
The contents of the output buffer are undefined if PHYSFSX_getRealPath
fails, so mark the function as [[nodiscard]] and modify all callers to
check that the function succeeded.
Rework the error paths to return path-specific status codes so that the
caller can report exactly which step caused an HMP file to be rejected.
On error, print this reason numerically and, if the reason was a PhysFS
error, also print the PhysFS error code numerically and symbolically.
gr_find_closest_color did not need it. Remove it. For the others,
resetting the count is sufficient. There is no need to reset the
individual elements.
The SDL_mixer library has already been instructed, via Mix_Volume, to
scale the volume of sounds on all channels, by an amount based on
digi_volume. There is no need to manipulate the effective distance of a
particular sound to further scale it by digi_volume. Even if this
second scale was needed, it was done incorrectly, because it was only
applied when the sound was started, but not re-applied when the sound's
volume was updated due to positional changes. As a result, any sound
which was updated would switch to an unscaled version. Sounds which
were never updated, such as those attached to the viewer object, would
retain their original scaled volume.
Update the implementation of digi_mixer_set_channel_volume to call
Mix_SetDistance in the same way as digi_mixer_start_sound, for
readability and consistency.
Add RAII wrappers for unmounting PHYSFS paths. Use them in places that
previously handled unmounting explicitly. Also, use it for descent.hog
/ descent2.hog, which previously were left mounted indefinitely.