Remove maximum volume control from configuration.
Remove individual maximum volume.
Add a checkbox in applet to enable raise maximum volume for all devices. (not applications)
Depends on D26271
ngraham | |
drosca |
VDG | |
Plasma |
Remove maximum volume control from configuration.
Remove individual maximum volume.
Add a checkbox in applet to enable raise maximum volume for all devices. (not applications)
Depends on D26271
Before:
2 options exist to raise maximum volume.
One options controls the default output device(by setting the limit).
And a second one controls individual devices/applications.
After:
A checkbox in the applet will raise the max volume for all devices to 150%.
Automatic diff as part of commit; lint not applicable. |
Automatic diff as part of commit; unit tests not applicable. |
Thanks for this patch. I won't try to make you go forward with something you don't agree with, so let's continue the discussion here and hopefully we can reach a consensus of some sort.
IIRC VDG people decided that 1) it made sense to have only a single way to raise the maximum volume rather than two, 2) that this single method should be global rather than per-app/per-device because global is simpler, more comprehensible, and more likely to be what the user wants to do, and 3) that the option to do this should be more discoverable than being in the config window.
Adding VDG for more comments, and see T10470 for more details. Also adding @drosca as he's the maintainer.
Actually I never touch volume control, so don't have strong opinion. Either way I'm good, no worries.
But in general believe, that should simplify UI and usability, while trying to keep features.
This patch removes 2 options.
While the other could have the mockup as a default, while also giving more options to advanced users.
But if it is about code maintenance, I will not disagree.
Works great. Just a few code change requests:
applet/contents/ui/main.qml | ||
---|---|---|
543 | Don't override this; if it looks bad with the default value, we should fix that in the checkbox control itself | |
544 | This could evaluate to a fractional number. You probably want units.largeSpacing here (the spacing doesn't have to be literally identical to the mockup; it's more important to use standard spacing values) | |
546 | Always use onToggled to handle user input, never onCheckedChanged If you're deliberately using onCheckedChanged because it has the side effect of evaluating the logic in here when the applet is first loaded, then don't do that, and instead explicitly call that logic in an onCompleted block |
Raise maximum volume has the potential to aid in speaker blowout. Although this requires a system not properly configured (an amplifier too powerful with no limiter). I expect this to be the case only with DIY audio setups.
I know that in theory it should be safe, and can only create distortions. And damage to speaker can happen with a very high sound, without raise functionality.
Despite this I feel this option has no place to be so accessible in the applet(calling, for example, kids to try the 'Boost' button).
Because time has been already invested in these patches, the only change I made was disabling the checkbox. That way, YOU can easily enable it.
Sorry for the inconvenience, so late too.