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

Unexpected change in Reverb behavior between 1.21.1 and 1.23.0 #943

Open
s5bug opened this issue Dec 5, 2023 · 18 comments
Open

Unexpected change in Reverb behavior between 1.21.1 and 1.23.0 #943

s5bug opened this issue Dec 5, 2023 · 18 comments

Comments

@s5bug
Copy link

s5bug commented Dec 5, 2023

See also henkelmax/sound-physics-remastered#178

Given that we can't (reasonably) bisect this repository to test the changes in the actual target environment, what steps can be taken to figure out where/why the reverb effect is just not occurring? The code that calls into openal-soft lives in SoundPhysics.java. The Java bindings themselves handle almost all of the openal-soft initialization.

As stated in the above issue, the only meaningful difference I could spot compared to examples/almultireverb.c is that SPR doesn't set

  • AL_EAXREVERB_DECAY_LFRATIO
  • AL_EAXREVERB_REFLECTIONS_DELAY
  • vAL_EAXREVERB_REFLECTIONS_PAN
  • vAL_EAXREVERB_LATE_REVERB_PAN
  • AL_EAXREVERB_ECHO_TIME
  • AL_EAXREVERB_ECHO_DEPTH
  • AL_EAXREVERB_MODULATION_TIME
  • AL_EAXREVERB_MODULATION_DEPTH
  • AL_EAXREVERB_HFREFERENCE
  • AL_EAXREVERB_LFREFERENCE
  • AL_EAXREVERB_DECAY_HFLIMIT

I can't make enough sense of the diff/compare between 1.21.1 and 1.23.0 to get a good grasp on what of this list could matter.

@kcat
Copy link
Owner

kcat commented Dec 5, 2023

There shouldn't be any changes to cause the reverb to stop working. The changes made don't, or at least shouldn't, affect what the reverb sounds like. 1.23 does change how changes to the reverb are handled, though. And there were changes to the room rolloff applied to the source.

How often are reverb properties updated, and are all properties constantly changed, or are only some? (it's fine to keep setting them, it only matters if the value is different). I could see there being issues if certain reverb properties are changed very frequently, since OpenAL Soft tries to let the old environment decay normally while the new one fades in. But if the new environment doesn't have time to become audible before being changed again, it can cause audio glitches (clicks, pops, static) or for it to stay silent.

If this is the issue, then the properties that should only be modified occasionally are (some you say aren't being set, but to be thorough):

  • AL_EAXREVERB_DENSITY
  • AL_EAXREVERB_DIFFUSION
  • AL_EAXREVERB_DECAY_TIME
  • AL_EAXREVERB_DECAY_HFRATIO
  • AL_EAXREVERB_DECAY_LFRATIO
  • AL_EAXREVERB_MODULATION_TIME
  • AL_EAXREVERB_MODULATION_DEPTH
  • AL_EAXREVERB_AIR_ABSORPTION_GAINHF
  • AL_EAXREVERB_HFREFERENCE
  • AL_EAXREVERB_LFREFERENCE
  • AL_EAXREVERB_DECAY_HFLIMIT

The other properties can be changed as frequently as desired, but those should be updated infrequently.

@s5bug
Copy link
Author

s5bug commented Dec 6, 2023

The properties are only set

  1. on initialization
  2. when the configuration is updated

The way the reverb is actually meant to change is via AL_AUXILIARY_SEND_FILTERs:

        if (maxAuxSends >= 4) {
            EXTEfx.alFilterf(sendFilters[0], EXTEfx.AL_LOWPASS_GAIN, sendGain0);
            EXTEfx.alFilterf(sendFilters[0], EXTEfx.AL_LOWPASS_GAINHF, sendCutoff0);
            AL11.alSource3i(sourceID, EXTEfx.AL_AUXILIARY_SEND_FILTER, auxFxSlots[0], 3, sendFilters[0]);
            logALError("Set environment filter0:");
        }
        // etc. for >= 3, 2, 1

        EXTEfx.alFilterf(directFilter0, EXTEfx.AL_LOWPASS_GAIN, directGain);
        EXTEfx.alFilterf(directFilter0, EXTEfx.AL_LOWPASS_GAINHF, directCutoff);
        AL11.alSourcei(sourceID, EXTEfx.AL_DIRECT_FILTER, directFilter0);
        logALError("Set environment directFilter0:");

        AL11.alSourcef(sourceID, EXTEfx.AL_AIR_ABSORPTION_FACTOR, SoundPhysicsMod.CONFIG.airAbsorption.get());
        logALError("Set environment airAbsorption:");

airAbsorption being a config value means it's almost never changed as well.

See evaluateEnvironment and setEnvironment

it can cause audio glitches (clicks, pops, static) or for it to stay silent.

The case is that the sounds are playing normally (in fact, muffling sounds based on occlusion does work). It's just that there isn't any reverb applied alongside the other effects.

@kcat
Copy link
Owner

kcat commented Dec 6, 2023

setReverbParams and/or syncReverbParams would be the more pertinent functions. How often are the reverb parameters in ReverbParams changed and these functions called? Can you get a trace of the values being set for the AL_EAXREVERB_* properties?

And to be sure, there are no AL errors being generated/logged?

@s5bug
Copy link
Author

s5bug commented Dec 6, 2023

No AL errors.

setReverbParams/syncReverbParams are only called on initialization and configuration change. My bad on not making that more clear 😅 So the values on ReverbParams are only changed when the user changes a configuration value, and not during normal gameplay.

Here's the reverb details, in order, from my debugger:

r = {ReverbParams@16996} 
 decayTime = 0.15
 density = 0.0
 diffusion = 1.0
 gain = 0.119
 gainHF = 0.99
 decayHFRatio = 0.6
 reflectionsGain = 2.5
 reflectionsDelay = 0.001
 lateReverbGain = 1.26
 lateReverbDelay = 0.011
 airAbsorptionGainHF = 0.994
 roomRolloffFactor = 0.16
r = {ReverbParams@17005} 
 decayTime = 0.55
 density = 0.0
 diffusion = 1.0
 gain = 0.17850001
 gainHF = 0.99
 decayHFRatio = 0.7
 reflectionsGain = 0.2
 reflectionsDelay = 0.015
 lateReverbGain = 1.26
 lateReverbDelay = 0.011
 airAbsorptionGainHF = 0.994
 roomRolloffFactor = 0.15
r = {ReverbParams@17006} 
 decayTime = 1.68
 density = 0.1
 diffusion = 1.0
 gain = 0.2975
 gainHF = 0.99
 decayHFRatio = 0.7
 reflectionsGain = 0.0
 reflectionsDelay = 0.021
 lateReverbGain = 1.26
 lateReverbDelay = 0.021
 airAbsorptionGainHF = 0.994
 roomRolloffFactor = 0.13
r = {ReverbParams@17007} 
 decayTime = 4.142
 density = 0.5
 diffusion = 1.0
 gain = 0.238
 gainHF = 0.89
 decayHFRatio = 0.7
 reflectionsGain = 0.0
 reflectionsDelay = 0.025
 lateReverbGain = 1.26
 lateReverbDelay = 0.021
 airAbsorptionGainHF = 0.994
 roomRolloffFactor = 0.11

They were set once on game initialization, and once again when I loaded into a world. They were the same values both times.

I added an error logger to every openal-soft call that didn't have one before, and still I don't see any errors in the log. (I know the error logger works because I can cause it to log when I specifically mess up certain parameters.)

@kcat
Copy link
Owner

kcat commented Dec 6, 2023

Modifying alreverb to use those parameters, and only set the same properties that you do with the Java code, shows the reverbs are there, but really really subtle. But this is what I'd expect; since you're setting the gain to 0.119 (-18.489dB), and the reflection gain only being 2.5 (7.9588dB), that results in a starting gain of about -10dB, where it can only get quieter. The late reverb gain is 1.26 and also has a short decay time, meaning it will start lower. If I set the first one to have gain = 1.0, I can definitely hear it, but otherwise it's very hard to hear.

This is before accounting for the source's send filter or automatic send gain adjustment for source distance.

The latter two reverbs are more audible out-of-the-box, due to having a higher starting gain and longer decay time, but it's still subtle.

In either case, I'm not seeing why it would be different for 1.23 compared to 1.21. The actual processing done to the signal looks the same; the same feedback delays and coefficients, same gain adjustments, same all-pass filters... the only thing that's different is how it handles fading when certain reverb parameters are changed, which you say they aren't often.

The only other change is how it calculates the automatic send gain adjustment for source distance. For one, the reverb's room rolloff factor that's applied to the source is now always the inverse distance rolloff model (which is the default for the direct path). I don't see the distance model being changed in that code, but I don't know if Minecraft changes it to something different elsewhere. The rolloff factors in your settings seem fairly low so it's probably not having much of an effect, but it is a potential difference.

The other change to the auto send is how it calculates the initial distance-based decay. This change should actually benefit audibility since it now uses the dry path as a base that it can never go lower than, so it will be a touch higher than before.

I might have to make a build of 1.21 to inspect what's going on there compared to current.

@s5bug
Copy link
Author

s5bug commented Dec 6, 2023

I doubled the global reverb gain in-game and didn't notice a difference. I don't understand enough of this code to know where to look regarding distance-related things, cc @henkelmax?

@kcat
Copy link
Owner

kcat commented Dec 6, 2023

Does it work any better if you set roomRolloffFactor to 0?

@s5bug
Copy link
Author

s5bug commented Dec 6, 2023

Oh my goodness! Yeah, I hear reverb again!

And the reverb does properly decrease the less "in a cave" I am; there's no reverb / not enough reverb for me to notice it when I'm up on the surface & the sounds don't have places to bounce around. It definitely feels too harsh at exactly 0, so I wonder if there's a proper way to adjust the old values to new ones?

@s5bug
Copy link
Author

s5bug commented Dec 6, 2023

Hmm, even with roomRolloffFactors set to 1/100,000,000th of the value they were before, I hear no reverb. I only seem to hear reverb when they are exactly 0.

Edit, same with 1/(10³³). It seems like reverb only works at all when the values are 0.

@kcat
Copy link
Owner

kcat commented Dec 6, 2023

That would suggest Minecraft is setting the source's AL_REFERENCE_DISTANCE to 0, which causes the reverb's room rolloff to fail. It will only work when the effective distance is 0 (the source distance is actually 0, or the reverb room rolloff is 0), where it won't attenuate. This is unfortunately a bug in OpenAL Soft. The best you can do currently is leave the room rolloff at 0, unless you can adjust the source reference distance to be non-0.

Though even when the bug gets fixed in OpenAL Soft, it's not completely clear what the behavior should be, exactly. The EFX spec says about AL_EAXREVERB_ROOM_ROLLOFF_FACTOR:

It's defined the same way as OpenAL’s Rolloff Factor, but operates on reverb sound instead of
direct-path sound. Setting the Room Rolloff Factor value to 1.0 specifies that the reflected sound
will decay by 6 dB every time the distance doubles. Any value other than 1.0 is equivalent to a
scaling factor applied to the quantity specified by ((Source listener distance) - (Reference
Distance)). Reference Distance is an OpenAL source parameter that specifies the inner border
for distance rolloff effects: if the source comes closer to the listener than the reference distance,
the direct-path sound isn’t increased as the source comes closer to the listener, and neither is the
reflected sound.

The "will decay by 6 dB every time the distance doubles" is the classic inverse distance rolloff model, which depends on a non-0 reference distance. If the reference distance is 0, you can't do the 6 dB decay per distance doubling (0*2 == 0), so there would be no distance attenuation resulting from this property. It's possible the wording here is sloppy and it should use the same distance model as the direct path, it just giving the 6 dB decay explanation as the default, but otherwise, aside from the bug with a 0 reference distance and non-0 reverb room rolloff, OpenAL Soft behaves as this says. I'll need to do some testing with other implementations.

@henkelmax
Copy link

Hey, thank you for the excellent support @kcat and thank you for reporting the issue @s5bug!

I can also confirm that the reverb works with AL_EAXREVERB_ROOM_ROLLOFF_FACTOR set to 0. Though it doesn't sound like before, it's at least something.

What I don't understand though is that there is no reverb with LWJGL 3.3.2 (openal-soft 1.23.0) but if I replace it with 3.3.1 (openal-soft 1.21.1) and the same Minecraft version (So no code changes), the reverb is back. Does that mean the "bug" you mentioned was introduced in 1.22 or later? Can this be fixed in openal-soft to work as in previous versions?
Sorry if I misunderstood something and thank you for the help!

@s5bug
Copy link
Author

s5bug commented Dec 6, 2023

I think we could set AL_REFERENCE_DISTANCE, no? The OpenAL docs say

the distance under which the volume for the source would normally drop by half (before being influenced by rolloff factor or AL_MAX_DISTANCE)

which is something we know, right? I wouldn't know what to set it to or where to set it.

@henkelmax
Copy link

henkelmax commented Dec 7, 2023

I think we could set AL_REFERENCE_DISTANCE, no? The OpenAL docs say

the distance under which the volume for the source would normally drop by half (before being influenced by rolloff factor or AL_MAX_DISTANCE)

which is something we know, right? I wouldn't know what to set it to or where to set it.

That unfortunately doesn't work.

I added

AL10.alSourcef(i, AL10.AL_REFERENCE_DISTANCE, 16F);

right after Minecraft calls AL10.alGenSources. But that doesn't seem to affect anything.

EDIT:
Oh, I just saw, Minecraft sets it to 0:
image

Properly overriding Minecrafts value seems to work, but it still does not work 100% as before I need to play around with the values a little.

@kcat
Copy link
Owner

kcat commented Dec 7, 2023

Does that mean the "bug" you mentioned was introduced in 1.22 or later?

Yes. It was introduced in commit 43682a8, which was included in the 1.22.0 release.

Can this be fixed in openal-soft to work as in previous versions?

It can be, yeah. Comparing to Generic Software, it seems the docs are wrong and it should be using the set distance model rather than always doing inverse distance. Though the Generic Software implementation also seems a bit buggy, with AL_EFFECTSLOT_AUXILIARY_SEND_AUTO having no apparent affect on anything, and the reverb and source room rolloff factors always applying even regardless of the source's AL_AUXILIARY_SEND_FILTER_GAIN_AUTO property. AL_AUXILIARY_SEND_FILTER_GAIN_AUTO only seems to control the automatic initial decay. The directional cone never applies to the send gain, despite AL_AUXILIARY_SEND_FILTER_GAIN_AUTO saying it does control that. So it's difficult to tell what it's supposed to be doing vs Generic Software being buggy.

It doesn't help that AL_AUXILIARY_SEND_FILTER_GAIN_AUTO and AL_EFFECTSLOT_AUXILIARY_SEND_AUTO have similar wording:

The AL_EFFECTSLOT_AUXILIARY_SEND_AUTO property is used to enable or disable
automatic send adjustments based on the physical positions of the sources and the listener.
If [AL_AUXILIARY_SEND_FILTER_GAIN_AUTO] is set to AL_TRUE (its default value), the intensity of this Source’s
reflected sound is automatically attenuated according to source-listener distance and source
directivity (as determined by the cone parameters). If it is AL_FALSE, the reflected sound is not
attenuated according to distance and directivity.

In either case, I should be able to fix the immediate problem and make it how it used to be, more or less.

@henkelmax
Copy link

In either case, I should be able to fix the immediate problem and make it how it used to be, more or less.

That would be awesome, Thank you so much!

@henkelmax
Copy link

Properly overriding Minecrafts value seems to work, but it still does not work 100% as before I need to play around with the values a little.

Setting AL_REFERENCE_DISTANCE to half the attenuation value

AL10.alSourcef(source, AL10.AL_REFERENCE_DISTANCE, attenuation / 2F);

seems to sound very similar to how it was before. I'll do some testing and use this approach until the old functionality is restored.

Thank you very much for your help @kcat!

@Jenavieve-Rose
Copy link

I'm having a similar issue with reverb but the opposite I get strong reverb everywhere even when it should not be that strong or present at all. This is noticeable in any game I run that uses EAX.

@kcat
Copy link
Owner

kcat commented Jan 24, 2024

I'm having a similar issue with reverb but the opposite I get strong reverb everywhere even when it should not be that strong or present at all. This is noticeable in any game I run that uses EAX.

Can you provide a recording? And what version exactly are you using (the 1.23.1 release? can you build/try the latest Git commits?). The legacy EAX extensions weren't supported in 1.21, so it would make sense for apps that use EAX to now have reverb in 1.22 and 1.23 where it previously didn't. Different reverb implementations will have differences, though it shouldn't be too loud and distracting, and there have been more recent changes to improve reverb volume levels.

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

No branches or pull requests

4 participants