obsproject
UI: Change clip level to 0.0 dB
Description
This changes the clip level of the volume meters from -0.5 dB to 0.0 dB.
Motivation and Context
I think the original intention was to tell users they had too loud of audio without actually clipping the audio. But if audio was intended to be this high of level, such as music, it would show it was clipping without it actually clipping.
Brought up by @PatTheMav off thread.
How Has This Been Tested?
Tested audio with a high dB level, but not clipping, to make sure the volume controls didn't show the clipping indicators.
Types of changes
- Tweak (non-breaking change to improve existing functionality)
Checklist:
- [x] My code has been run through clang-format.
- [x] I have read the contributing document.
- [x] My code is not on the master branch.
- [x] The code has been tested.
- [x] All commit messages are properly formatted and commits squashed where appropriate.
- [x] I have included updates to all appropriate documentation.
Fixed the code now, so the clip level affects both the input and output meters.
Hi all, I have a rebased copy of this PR's commits, in case it saves anyone the time/tedium. Feel free to grab from there if it's helpful. (If not, apologies for the noise.) Best regards.
https://github.com/obsproject/obs-studio/compare/master...DeeDeeG:obs-studio:cg2121_clip-level_PR11230_rebased
Rebase was a little involved due to https://github.com/obsproject/obs-studio/pull/9407 landing.
One line originally in this PR has already been done separately in another PR, which has already landed on master, so the line does not appear in the rebased version's diff. https://github.com/obsproject/obs-studio/pull/11209
P.S. works well for me, audio from a game mastered to full scale volume no longer indicates clipping during loud sounds. Just red as it should.
EDIT to clarify: I meant that fixing the merge conflict was somewhat difficult/tedious, but unless I missed something, the diff looks effectively identical on my branch vs this PR.
Only the line that was done separately as https://github.com/obsproject/obs-studio/pull/11209 is not necessary anymore and disappears when rebasing, as it is 100% redundant -- the same line is already landed verbatim at master branch now. No change to make when rebasing means no diff when rebasing. And of course the line length of surrounding code is now longer and wrapped differently as a consequence of https://github.com/obsproject/obs-studio/pull/9407, which is unavoidable to deal with in a rebase. That was why the rebase was difficult/tedious, as the diffing algorithm is unsure about how to resolve differences in the context lines before/after this PR's actual changes, given the different line length and wrapping of the surrounding code pre vs post rebase, so it flags them to be resolved manually. Rebasing the actual code changes of the PR was trivial compared to rebasing the context lines, which was harder/more tedious.
So, now that it's done, the resulting diff looks clean and simple. If it's done right, it should match this original PR. Which I believe it does. Other than the one-line change already in master from https://github.com/obsproject/obs-studio/pull/11209 being gone from the diff, as explained above.
I hope my rebase can either be cherry-picked to save time, or else ignored and redone by hand if reviewing my copy constitutes a distraction. Thanks for your understanding, and sorry if it adds any confusion.
I manually resolved the conflict with the laster master on local host and tested.
Because of changing peakPosition to peak in the conditions, the meter gets flashing when the audio level is very close to the clipping level.
https://github.com/user-attachments/assets/4c09a0d6-6143-44e8-b194-235cfbc59667
Screen recording above is tested with Asynchronous Audio Source plugin (an 100-Hz 0-dB sine wave) + Gain Filter (0.1 dB).
@norihiro could you check if the latest code in this PR fixes the issue you were having?
The behavior on the horizontal meter looks good. Though, the vertical meter should be updated as well.
