GNOME Bugzilla – Bug 791272
Respect the allow-amplified-volume gsettings key
Last modified: 2018-01-12 20:50:22 UTC
Gsettings now has a key for allowing setting the output volume over 100% (see bug 790281). The sound panel should respect this key and only allow setting the volume over 100% when it is set to true. The change should be fairly straightforward in UI terms. Nevertheless, mockups can be found here: https://raw.githubusercontent.com/gnome-design-team/gnome-mockups/master/system-settings/sound/sound-over-amplification.png
Created attachment 366742 [details] [review] sound: Respect allow-volume-above-100-percent setting Instead of always allowing above 100% volumes for hardware that supports it, only enable it when the allow-volume-above-100-percent setting is set to true as well. See https://bugzilla.gnome.org/show_bug.cgi?id=790988
Review of attachment 366742 [details] [review]: Couple of comments. ::: panels/sound/gvc-mixer-dialog.c @@ +81,3 @@ + GSettings *settings; + gboolean allow_volume_above_100_percent; Nitpick (not a blocker, feel free to ignore): maybe that's too long of a struct field name... what about "allow_volume_overdrive" or maybe "allow_overvolume" or something like that? @@ +1918,3 @@ + gboolean output_amplified, input_amplified; + + g_message ("got that shit ofing"); Stray g_message() @@ +1923,3 @@ + "allow-volume-above-100-percent"); + + g_message ("val %d", dialog->allow_volume_above_100_percent); Stray g_message()
Created attachment 366743 [details] [review] sound: Respect allow-volume-above-100-percent setting Instead of always allowing above 100% volumes for hardware that supports it, only enable it when the allow-volume-above-100-percent setting is set to true as well. See https://bugzilla.gnome.org/show_bug.cgi?id=790988
(In reply to Georges Basile Stavracas Neto from comment #2) > Review of attachment 366742 [details] [review] [review]: > > Couple of comments. > > ::: panels/sound/gvc-mixer-dialog.c > @@ +81,3 @@ > > + GSettings *settings; > + gboolean allow_volume_above_100_percent; > > Nitpick (not a blocker, feel free to ignore): maybe that's too long of a > struct field name... what about "allow_volume_overdrive" or maybe > "allow_overvolume" or something like that? I'd rather keep it the same name as the config option. As in the GSettings key, I'd rather not introduce a new term that makes it more complicated to understand for the sake of a few characters. > @@ +1918,3 @@ > + gboolean output_amplified, input_amplified; > + > + g_message ("got that shit ofing"); > > Stray g_message() > > @@ +1923,3 @@ > + > "allow-volume-above-100-percent"); > + > + g_message ("val %d", dialog->allow_volume_above_100_percent); > > Stray g_message() Both removed. Yay left-over debug ;)
Review of attachment 366743 [details] [review]: Right. Yeah, it makes sense to keep the struct field name == gsettings schema key. In that case, I'd consider this a code review pass.
Attachment 366743 [details] pushed as 7ce6bd1 - sound: Respect allow-volume-above-100-percent setting