-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Implement audio normalization using BASS #27793
base: master
Are you sure you want to change the base?
Conversation
still wip, committing so i can grab on a diff pc
This temporarily disables it until I can get around to completely removing it
So I forgot to free the stream when everything's finished. This would cause the BASS stream to exist eternally, resulting in the garbage collector never collecting this, resulting in a ton of ram usage. Additionally, 60k bytes is a lot for something like this. So I reduced the size of the buffer to 10k bytes.
Hopefully fixes a bug I've been noticing. The bug itself is pretty hard to reproduce so I'm just going to add this in hoping that it fixes things. Will likely talk more about it in the PR description
Seems like this is required since we've effectively added a new "column" to realm
Will remove VolumeOffset in a future commit. This is mainly so that you can change the target level and not have to go through recalculating again
So I've played on my branch for a couple of hours and tried different values. -14 LUFS seems to meet my expectations the most. Plus, youtube and spotify use it so its probably better for music
So while I was looking through the diff, I noticed that I could simply move the processedcount incrementor above the if...continue statement and repurpose the notification to being "Verifying loudness levels" instead of "Calculating loudness levels". This is an easy fix that still offers a sense of transparency to the user.
Quick update: |
Co-authored-by: smallketchup82 <[email protected]>
Improvements for audio normalization
Finished reviewing the new implementation myself. This should be in an acceptable state for review from the core team. I want to raise some concerns, however:
|
Here's an updated video demonstrating normalization in various different scenarios (since its rather hard to test, I figured making this video would be useful to an extent). I would highly recommend making sure that your system volume is at a comfortable level, then watching the video at maximum volume (for the video, not system). This is to give you a feel of what -14 LUFS actually feels like, and what the global reduction feels like in comparison. I threw it together in 20 minutes in premiere pro, so the quality is horrible (sorry lol) Audio.Normalization.Demo.reencoded.mp4 |
Lemme just state up front that I've not looked at or reviewed the code, so take that into account when reading my feedback.
Hmm after giving this some more thought, I'm not 100% sure. My initial gut reaction says that maybe the volume adjustments should only apply to beatmap hitsounds (i.e. mapper/custom hitsounds included with beatmaps)? But on the other hand, if a beatmap was created with legacy/lazer hitsounds in mind, then we'd want those to be adjusted too. As the linked comment brings up, it might be a bit weird for hitsounds to change volume between beatmaps, but I think maybe that's fine? Maybe @ppy can provide a second opinion? This also begs the question as to what happens if someone maps on lazer with normalization on and then a player plays with normalization off... or vice-versa. I don't really have an answer to this.
Does this PR also remove lazer's current hard-coded global volume reduction? If not, that might be what's making things too quiet. I'm also not sure what curve the volume controls currently use, but we could potentially change that if it's ramping volume down too fast?
I don't know how the rest of the team feels about adding more user preferences, but we could do something similar to what Spotify has and include a dropdown to choose between Loud (-11dB LUFS), Normal (-14dB LUFS) and Quiet (-19dB LUFS) normalization options. Alternatively, we could just outright normalize to a louder level and lower the global music volume by default, allowing players to turn the music volume up if they desire - similar to how (I believe) the master volume control works now? It's also probably worth mentioning that some players have complained in the past when we've added volume reductions to lazer, as their computer setups aren't able to increase the output volume enough otherwise, for whatever reasons. Thus normalizing to a louder level and reducing via the global volume controls might be the preferable choice if we don't want players just reactionarily disabling normalization. This might bring the risk of the mixed output going over 0dB and/or clipping, but ideally in the future everything will be running through a global compressor/limiter anyway to prevent this.
I think it's probably fine for the volume to jump up when normalization is disabled - it's common behaviour with music players. If it's that much of a volume jump, we could maybe bump the global master or music volumes down by some amount when normalization is disabled to alleviate it somewhat, but still allow players to undo the reduction if they wish. |
My thoughts on that is that hitsounds will likely be changing in volume regardless. As peppy said in our discussion in the osu!dev discord, mappers will typically try to psuedo-normalize hitsounds to the volume of the track themselves by setting every hitobject to a certain volume. I think that adjusting the volume offset for hitsounds regardless of whether beatmap hitsounds are on or off is probably the correct way to go about it, at least to me, since the goal is to retain the relationship between the track and the hitsounds, and only doing that in certain circumstances sounds counter-intuitive and unexpected. Though, it's probably a question that we'd want more opinions on.
Yes, but I don't think it removes the volume reduction when first starting the game from a fresh install.
When compared to the option of normalizing to a louder level but setting the default volume to a lower level, like 50%. I find myself siding more with the dropdown approach (mostly from personal preference, no real basis for it). But to be honest, either approach would work well and would be easy to implement. I'm pretty indecisive on this so I think it might be something that we'd want more opinions on, or to poll somewhere (in a discussion, osu!dev discord, idk).
Honestly, when its put that way, it kinda makes me want to reconsider adding the ability to enable or disable normalization. I believe that normalization should be on for most players unless there's a very good reason not to, since the pros of normalization outweigh the cons. I'm also sorta indecisive on this.
After reading your thoughts on this, I think that I'll just keep it as is. While reducing the volume globally to get it to be close to -14 LUFS would be nice, I realized that players probably wouldn't like not being able to revert the normalization changes themselves (they would expect volume to be similar to before normalization was implemented if they disable normalization). Thanks for your thoughts! I'd definitely like to get some more opinions on those concerns, minus the 3rd one cause I'm probably just going to leave it alone. |
Transfer TrackNormalizeVolume with Track in WorkingBeatmap
Resolved merge conflicts and fixed some things as a result of ppy/osu-framework#6233 getting rebased onto master. Tested everything and still works as expected. |
this is honestly a nitpick on my behalf, but i figured it's better than what we had before. when i was looking at the setting in the settings menu, simulating a new player, i asked myself "normalize audio? what audio?". for all the user knows, this could be effects, beatmaps, hitsounds, anything. this commit changes the text of the setting to read "Normalize Beatmap Audio" to further describe that it's normalizing only beatmap audio
Currently this does not check if the audio file plays correctly, so if the user has any beatmaps with a broken audio file, this will attempt to process them, fail, then notify the user of the failure. This happens on every startup. It can easily become annoying but the fix is painful. You'd have to attempt to open a bass stream for every audio file, check if that happens without errors, close that stream, then proceed with the processing. You'd need to do this for every unprocessed beatmap, on every startup. The performance of the loop would be greatly reduced, and the complexity the solution is an even bigger problem than that. I didn't do anything back then, and I'll probably still leave it to be addressed in review. Though I might revisit that loop later on, because it can really use a refactor. I mean, as bad as it is, it's honestly a decent (yet very annoying) reminder to the user to clean out beatmap sets that they quite literally cannot play 😆 |
Closes #1600
Prereqs:
Audio Normalization
Audio Normalization is the process of determining the loudness of audio and standardizing (normalizing) it to a set level. If you normalize a number of audio files to the same level, they will all sound the same volume wise. Normalization is useful for making sure that loud songs get made quieter so that they aren't so loud, and quiet songs get made louder so that they aren't so quiet. This reduces the number of times you will need to change your volume, and yes, it means you can finally play The Big Black without having your eardrums reduced to shreds.
Here's a video showing what Audio Normalization looks like in osu!. In the video, all 3 of my volume sliders are set to max. The beatmap songs are being normalized to -14 LUFS. Try to focus on how every song sounds volume wise, you should find that they all sound about the same in terms of volume no matter the instruments in the song. I find that tech maps work the best when trying to tell what normalization does
normalization.mp4
Description of Approach
So, I decided to take a different approach than the old PR. The old PR tried implementing the algorithm manually which isn't good since we'd have to manually maintain it. This PR uses BASSLoud which is maintained by BASS and will get regular updates. I also noticed a lot of missing things in the old PR, such as recalculating the normalization values in the background. I've taken more or less the same structure as the old PR but improved upon it heavily.
When talking about approach in terms of normalization, this implementation calculates loudness using the ITU-R BS.1770-4 standard, but normalizes to -14 LUFS instead. I've spent a couple hours playing the game on my branch to get a feel for the normalization, and I've found -14 LUFS to best meet my expectations. It makes the audio not too quiet, where the effects are really loud, but also loud enough that 100% volume actually feels like 100% volume. Additionally, -14 LUFS is what Youtube & Spotify normalize to, so I believe that it's probably the best level to normalize to when it comes to music. Whereas -23, as recommended by EBU, is best for movies and TV programmes.
My PR uses Integrated Loudness, meaning the average loudness across the entire song. This is different from more conventional methods of loudness detection, such as True Peak, which normalizes based on the peaks (parts which are loudest) in the song. Integrated Loudness is known for working better than True Peak since it takes into account how the song sounds like. Meaning sudden sounds in a song (say sfx), a sudden increase in vocal volume (such as in a chorus), or a sudden increase in the volume of an instrument (say at the end of a song as a finisher), will not be accounted for. These sudden sounds will not influence how the song is normalized, and won't be the reason for a relatively quiet but suddenly loud song to be made more quiet because of that sudden section. The old pr seemed to calculate and use a variety of methods, including True Peak, which was completely unnecessary.
Description of Changes
This PR adds the ability for osu! to normalize audio tracks to a target level using BassLoud. This implementation goes through realm and tries to find maps which do not have loudness detection values stored, it then calculates these values via a background process. Once these values are calculated, they are stored and cached in Realm. These values are grabbed if they exist when a beatmap's audio track is loaded, if they don't exist, the old global audio reduction is applied (0.8). The values are used to create a Volume Fx filter for the track to normalize it.
Changes to Realm
Since we're now tracking Audio Normalization values in Realm, we need to bump the version otherwise a migration error is thrown. Apart from creating object mappings, the only change to realm here is a bump to the version.
Testing
Testing this is a bit difficult since there are many changes in many areas. It should get exponentially easier as things begin to get merged.
osu-framework
, packNativeLibs
into a nupkg, then add a nuget source for it. Downgrade theNativeLibs
version thatosu-framework
is using to1.0.0
.osu
) and runUseLocalFramework.sh
. For good luck, rundotnet restore osu.Desktop
just to make sure everything is correctly restoreddotnet run
(or ide equivalent) inosu.Desktop
with no problems.Feel free to ping me in the
#osu
channel of the osu!dev discord if you need help with this.Final remarks (notes before reviewing)
Please read this section before reviewing
Developed with help from @hwsmm (thanks!!!)