matrix-react-sdk
matrix-react-sdk copied to clipboard
Reduce volume by 11db
Reduces the volume of the sounds to be roughly at the same level of system sounds, thereby reduced by 11db
The files are either .mp3 or .ogg according to github, but when downloaded, the .ogg files are saved as .oga files - I provided .ogg files here (and element web references .ogg as well, see here: https://github.com/vector-im/element-web/blob/9df3774886cc151bb987bd6cc3a2a54d730c5715/src/vector/index.html#L77
Checklist
- [ ] Tests written for new code (and old code if feasible)
- [ ] Linter and other CI checks pass
- [ ] Sign-off given on the changes (see CONTRIBUTING.md)
Here's what your changelog entry will look like:
🐛 Bug Fixes
- Reduce volume level for notification sounds by 11db (#9143). Contributed by @JMoVS.
should be fixed now, thanks for bringing it up @turt2live
code-wise seems fine, but needs product review.
ok - according to githbub though, you are still requesting changes - anything else to be done in the review so it passes yours?
I'm somewhat intentionally holding the requested changes so it doesn't accidentally merge before the product team gets to it. It looks fine otherwise.
@janogarcia Thanks for your reply
Indeed, I did not intend to change the length of the loop - I can get that changed.
I can figure out how to export at a specific LUFS level in Logic - please prepare the respective LUFS levels - I just tried to adjust the level to be roughly similar to the system sound level - but LUFS is probably a better approach. Looking forward to the levels, then I can update this PR
@janogarcia Ringend, ringback and ring are really loud - if I provide loudness reduced versions with correct looplength now - would you accept that? They are really loud and I would like to avoid having to wait for the perfect LUFS adjusted levels and version and would prefer a quick fix now with a proper fix with proper LUFS levels afterwards - would that work for you?
Bump @janogarcia
Perhaps a volume slider for element sounds might be the better temporary fix if this won't be accepted? Right now the sounds really are too loud, and turning the volume for element down will make things like calls impossible.
@janogarcia Is there any news now on LUFS targets so I could update this PR?
@JMoVS From the research I did, a LUFS reference of -10 RMS and peak at -0.2 seems to be relatively standard for notification sounds/ringtones. Would you be able to normalize the sounds at those levels again so we can take action on this?
a LUFS reference of -10 RMS and peak at -0.2
LUFS and RMS are two separate measurements. I tried LUFS -10 but that would have made them louder.
What I did instead now is look at the sounds from Signal Desktop, which are at a nice volume level and took those values (consistently ~-20LUFS and TP of -8 or higher) as the input. I didn't need to adjust busy as it was already at ~-20.
Let me know your thoughts @justjanne
(Last bit not least, force pushed to get rid of old commits and update with upstream)
@JMoVS awesome, thanks! I'll take a look on tuesday, after the holiday weekend.
@justjanne I went ahead and adjusted the name of the PR and initial comment
any news on this one @justjanne or anything that I still need to adjust?
any news on this one @justjanne or anything that I still need to adjust?
So far I haven't been able to review it yet, but I've put it at the top of my list of PRs to review.
Looks good so far, but please make sure to do the sign-off as required by CONTRIBUTING.md.
Once that's done, you'll get an 👍 from me :)
awesome, if I saw it correctly, adding that I signed off to the pull request comment (first message) is enough - I just added it there - is that sufficient?
@justjanne Let me know if I need to adjust or include it in the commit itself - the Contributing.MD isn't super clear - I interpreted it as in the "pull request comment" to be fulfilled with the addendum in the very first message on top (I edited it)
@justjanne Let me know if I need to adjust or include it in the commit itself - the Contributing.MD isn't super clear - I interpreted it as in the "pull request comment" to be fulfilled with the addendum in the very first message on top (I edited it)
That should be alright, I believe. @justjanne shall we get this landed?
really looking forward to getting this merged finally :)
@justjanne and I synced up in DM. This seems like a clear improvement to me and, therefore, I'm proposing that we land this without sending it through another design review which would further delaying things. Subsequent improvements can still be applied on top if needed.
can't believe it! Almost one year later and it's merged! How long does it take from develop to an SDK release to landing in Element? 🚀
How long does it take from develop to an SDK release to landing in Element? 🚀
Releases are done every 2 weeks. RC happened 2 days ago so this missed that, so it won't be in next week's release but rather the one after.
awesome!