Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add "Sound Quality" inside the options menu. #314

Closed
wants to merge 0 commits into from

Conversation

drHyperion451
Copy link

@drHyperion451 drHyperion451 commented Apr 25, 2024

This add a "Sound Quality" option that lets the user change between original quality and remastered quality setting by chnaging the sndspeed cvar. This solves #169

Also I think it's okay to add a third option named "Custom" if the user adds another value to sndspeed directy to the console, or maybe think about other options. Having a quick look to the code it seems it only checks if the value is 11025 or not, so it shouldn't really matter.

Maybe this is not a necessary change. I think it's a good QoL if I want to choose the KexQuake sound quality instead of the old one, but I leave this up to the maintainers.

Maybe it's good to add a "lock" option when the game is set to original, since this option is useless in this scenario. We should not lock sndspeed though because of mod support.

@drHyperion451 drHyperion451 marked this pull request as ready for review April 26, 2024 00:03
@drHyperion451 drHyperion451 changed the title Add the Sound Quality option. Add "Sound Quality" inside the options menu. Apr 26, 2024
@L-P
Copy link

L-P commented Jul 31, 2024

sndspeed sole purpose is to apply a low-pass filter on everything when sndspeed.value == 11025 && snd_mixspeed->value == 44100. It does nothing else.

I recommend removing the cvar and the low-pass (or at least make it optional behind another snd_lowpass) rather than adding a misleading option to the menu.

snd_mixspeed is the one controlling the SDL audio device output format.

@NightFright2k19
Copy link

Right now we have the situation that sndspeed is removed, but defaults to 11025 while snd_mixspeed is 44100. I believe that means the low-pass filter is applied by default and the only way to change it is to put sndspeed 44100 into autoexec.cfg or change it manually in the console. That's not ideal.

@drHyperion451
Copy link
Author

Right now we have the situation that sndspeed is removed, but defaults to 11025 while snd_mixspeed is 44100. I believe that means the low-pass filter is applied by default and the only way to change it is to put sndspeed 44100 into autoexec.cfg or change it manually in the console. That's not ideal.

That is exactly the main point. Why is that low pass filter on in the first place?. My guess is performace reasons for older sound cards or something like that (having less sample points being played is more efficient than having quadruple the sample rate), but it's completely useless nowadays, since the original quake PAK has all their sound already capped at 11025 Hz.

Right now I don't have enough time to make those changes, but I will just delete the sndspeed option.

@NightFright2k19
Copy link

It's definitely a problem for people who are using the Remaster files which are all 44 kHz. At this point people should rather have to change a variable if they are still using the old paks from the DOS release.

@4LT
Copy link

4LT commented Oct 4, 2024

The low-pass filter is to remove high-frequency noise introduced by resampling; the cutoff is set just below the Nyquist frequency for an 11025 Hz sample rate (half the frequency). The specific values for sound "quality" (corresponds to block size for resampling) and cutoff ratio were chosen originally by @ericwa to mimic the audio for resampling for Mac and Windows, though that has been changed to default to the Windows default (snd_quality = 5).

Without a low-pass filter, 11025 Hz sounds sound very crunchy when resampled to 44100 Hz. My own personal preference is to have the filter cutoff at a point slightly higher than the Nyquist frequency, because I believe there's significant signal at that frequency.

@NightFright2k19
Copy link

But if the samples are at 44kHz, it would make sense to set sndspeed to 44100 as well, wouldn't it?

@drHyperion451
Copy link
Author

drHyperion451 commented Oct 8, 2024

Okay, maybe I think this boils down to two options. Either:

A) Completely discard sndspeed cvar and just automatically switch to 44100Hz when the user selects the "remaster" version and apply the filter when it's on the original version, removing completely the sndspeed option.
B) Do the A option but add a "snd_retrofilter" cvar or "snd_retro" to simplify it to a boolean. Personally I like this the most since you could play an expansion old enough to contain old sounds alongside the remaster ones, so this could mitigate the difference.

We could just discard everything, but I think this should be at least explained or something, because someone would just wonder why this engine sounds worse than the KEX engine version for some reason.

Edit: Recently recompiled the proyect from the andrei-drexler:master branch and saw the complete overhaul of game options. Maybe we could add a "sound filter (retro/off)" options here and keep sndspeed variable?:

image

@L-P
Copy link

L-P commented Oct 8, 2024

In a branch I have rewritten the entire mixer to work with floating point only and let the SDL handle the resampling.
I should get back to it, remove the HRTF parts, and try to upstream it. That'd fix the issue as there would not be a low-pass filter anymore.

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.

4 participants