Proposed fix for volume ramp based fade-in cutting off the beginning of sounds

[ellenhp]

Proposed fix for #66452. Implements an explicit fade-in based on the same exponential method used for fade-out.

Bugsquad edit:

• Fixes Godot 4 - Audio clips have their beginning cut off #66452

[reduz]

I don't think this is a good idea, and you don't want to do this universally. Many times there are sounds with high transient speed (specially sound effects) and the fade in will muffle them. Additionally, doing it in the audio server is also a bad idea because in many places the mixing happens at later places in the stream (as example, interactive music).

I would approach this problem a bit different.

• For MP3 and OGG, check the beginning and the end of the sounds to see if they begin at DC or there is some significant offset.
• If this is the case, store flags that ramping is needed on fade in and fade out.
• Apply this fade in only for those streams. Fade out only if they don't loop.
• For wav, this fade can be done directly in the audio data on import.

This also does not fix the issue that it makes reference to, which is a different problem and likely needs a different solution. For OGG and MP3 there is already a fade, so it may need to be fixed for wav... but this is a different problem IMO.

[ellenhp]

There's a linear volume ramp over the entire 512 sample mixing window right now. This PR reduces that to a 64 sample exponential ramp-up. Whether we want to remove fade-ins entirely or do them for only some playbacks is another question, but I'd be opposed to doing it selectively because doing so is pretty complex.

[reduz]

@ellenhp then the linear volume ramp is also wrong and should be fixed, we should never force ramp in audio when it starts, this is not the right approach.

[ellenhp]

What about for seeks? You can't ensure that the arbitrary position in the file is at a zero crossing so it might make sense there. Removing this is a very easy fix though so I'm not necessarily opposed as long as we're okay with pops on $Player.play(x)

[reduz]

@ellenhp Again, all this should be taken care at stream level, not at audio server level. AudioServer lacks the context to do any sort of fixing for this, so ramping in is a hack.

[reduz]

We don't need to have pops on play if we fix this at stream level as I described above, and we should only actually apply ramp if there is a need to ramp (stream begins/ends at DC != 0). If there is not, then we should not ramp else we muffle transients unnecessarily.

[ellenhp]

Okay, I'll remove this after I'm done with #64775.

[ellenhp]

Also I just want to confirm that it's okay to keep fade-out in the audio server? Right now it happens unconditionally over 64 samples, same as this PR.

[bluenote10]

Also I just want to confirm that it's okay to keep fade-out in the audio server? Right now it happens unconditionally over 64 samples, same as this PR.

Good question. I was wondering about that as well. My first impression was that the notion of the lookahead buffer makes the logic in the audio server more complex than it needs to be. There is also a performance penalty for use cases that don't require fade out at all. Basically the lookahead buffer needs to be copied twice per mix step (first here and then here). Copying 2 x 64 samples over isn't much, for unnecessary overhead for streams that are playing constantly.

My gut feeling is similar to what @reduz suggests and solve such things at the stream level and keep the audio server as simple (and performant!) as possible.

[ellenhp]

I forgot to respond to you @bluenote10 but that will cause pops when a player is removed from the tree without being stopped at least one mix before that. This is a bug we fixed for 4.x. I've spent literally hundreds of hours designing and implementing major changes in the 4.x audio system and I've put a lot of thought into it. It hurts on a very visceral level when people talk about undoing or reverting major changes I made in ways that will introduce bugs that I will be unable to fix.