Merge lp:~didrocks/gnome-settings-daemon/sound-above-100 into lp:~ubuntu-desktop/gnome-settings-daemon/ubuntu

Proposed by Didier Roche on 2017-07-26
Status: Merged
Approved by: Sebastien Bacher on 2017-07-26
Approved revision: 532
Merged at revision: 535
Proposed branch: lp:~didrocks/gnome-settings-daemon/sound-above-100
Merge into: lp:~ubuntu-desktop/gnome-settings-daemon/ubuntu
Diff against target: 172 lines (+120/-9)
5 files modified
debian/changelog (+8/-0)
debian/control (+1/-1)
debian/patches/70_allow_sound_above_100.patch (+109/-0)
debian/patches/series (+1/-0)
debian/rules (+1/-8)
To merge this branch: bzr merge lp:~didrocks/gnome-settings-daemon/sound-above-100
Reviewer Review Type Date Requested Status
Sebastien Bacher 2017-07-26 Approve on 2017-07-26
Review via email: mp+328068@code.launchpad.net

Description of the change

* Add patch to allow setting volume above 100%:
    - debian/patches/70_allow_sound_above_100.patch, adapt the patch from
      ubuntu-settings-daemon to latest g-s-d. (LP: #1706524)

To post a comment you must log in.
Sebastien Bacher (seb128) wrote :

looks fine, some small nitpicks:
- Laney pointed out on IRC that "Forwarded" should be "Bug" since the patch is not forwarded

- that shouldn't be a problem in practice, but you use com.ubuntu.sound without a direct depends but relying on the ubuntu-session one which I guess is on purpose?

We shouldn't get XDG_SESSION_DESKTOP=ubuntu without ubuntu-session installed but who knows (and the depends could be dropped in the futur by mistake or something) so maybe it's worth checking if the schemas is installed before using it?

Setting as approved since those are nitpicks and not mandatory changes and the code looks good otherwise

review: Approve
Didier Roche (didrocks) wrote :

On Forwarded -> Bug: will do.

On the 2nd, yeah, we can't have the session without the schema. I didn't want to add an additional check on the schema, but I could, that will just be at the price of the cost logic, but if you feel that should be done, I could (it will still be guarded by the session name). What do you prefer?
My plan is to have that accepted/discussed again with upstream.

Sebastien Bacher (seb128) wrote :

> On the 2nd, yeah, we can't have the session without the schema. I didn't want to add an additional check on the schema, but I could, that will just be at the price of the cost logic,

You are right, let's not get the code more complex for just hypothetical cases, if things ever change the gsettings abort is going to tell us anyway

Thanks for changing the header, setting the status as approved now

review: Approve
533. By Didier Roche on 2017-08-04

Make it dependent on XDG_CURRENT_DESKTOP (which is what we are going
to use on other places).

534. By Didier Roche on 2017-08-10

Protect by detecting if gsettings schema is here

535. By Didier Roche on 2017-08-16

releasing package gnome-settings-daemon version 3.24.3-0ubuntu3

Amr Ibrahim (amribrahim1987) wrote :

There are some typos in the code. See the inlined comments.

Didier Roche (didrocks) wrote :

We don't change old changelogs, the rest is applied now, thanks!

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/changelog'
2--- debian/changelog 2017-07-23 12:51:49 +0000
3+++ debian/changelog 2017-08-16 06:29:52 +0000
4@@ -1,3 +1,11 @@
5+gnome-settings-daemon (3.24.3-0ubuntu3) artful; urgency=medium
6+
7+ * Add patch to allow setting volume above 100%:
8+ - debian/patches/70_allow_sound_above_100.patch, adapt the patch from
9+ ubuntu-settings-daemon to latest g-s-d. (LP: #1706524)
10+
11+ -- Didier Roche <didrocks@ubuntu.com> Wed, 16 Aug 2017 08:29:43 +0200
12+
13 gnome-settings-daemon (3.24.3-0ubuntu2) artful; urgency=medium
14
15 * Make dh_auto_test non-fatal on s390x
16
17=== modified file 'debian/control'
18--- debian/control 2017-05-17 14:37:19 +0000
19+++ debian/control 2017-08-16 06:29:52 +0000
20@@ -7,7 +7,7 @@
21 Priority: optional
22 Maintainer: Ubuntu Desktop Team <ubuntu-desktop@lists.ubuntu.com>
23 XSBC-Original-Maintainer: Debian GNOME Maintainers <pkg-gnome-maintainers@lists.alioth.debian.org>
24-Uploaders: Andreas Henriksson <andreas@fatal.se>, Jeremy Bicha <jbicha@ubuntu.com>, Laurent Bigonville <bigon@debian.org>, Michael Biebl <biebl@debian.org>
25+Uploaders: Jeremy Bicha <jbicha@ubuntu.com>
26 Build-Depends: debhelper (>= 10),
27 gnome-pkg-tools,
28 gtk-doc-tools,
29
30=== added file 'debian/patches/70_allow_sound_above_100.patch'
31--- debian/patches/70_allow_sound_above_100.patch 1970-01-01 00:00:00 +0000
32+++ debian/patches/70_allow_sound_above_100.patch 2017-08-16 06:29:52 +0000
33@@ -0,0 +1,109 @@
34+Description: Allow volume to bet set above 100%.
35+ Some systems have low maximum volume set (like x220), allow, from an option
36+ in gnome-control-center to set it above that 100% limit.
37+ Modified from original patch in ubuntu-settings-daemon from Lars Uebernickel.
38+Origin: ubuntu
39+Bug-Ubuntu: https://launchpad.net/bugs/1706524
40+Bug: https://bugzilla.gnome.org/show_bug.cgi?id=710424
41+Index: gnome-settings-daemon-3.24.3/plugins/media-keys/gsd-media-keys-manager.c
42+===================================================================
43+--- gnome-settings-daemon-3.24.3.orig/plugins/media-keys/gsd-media-keys-manager.c
44++++ gnome-settings-daemon-3.24.3/plugins/media-keys/gsd-media-keys-manager.c
45+@@ -109,7 +109,6 @@ static const gchar introspection_xml[] =
46+ #define HIGH_CONTRAST "HighContrast"
47+
48+ #define VOLUME_STEP 6 /* percents for one volume button press */
49+-#define MAX_VOLUME 65536.0
50+
51+ #define SYSTEMD_DBUS_NAME "org.freedesktop.login1"
52+ #define SYSTEMD_DBUS_PATH "/org/freedesktop/login1"
53+@@ -166,6 +165,7 @@ struct GsdMediaKeysManagerPrivate
54+
55+ GSettings *settings;
56+ GHashTable *custom_settings;
57++ GSettings *sound_settings;
58+
59+ GPtrArray *keys;
60+
61+@@ -1152,6 +1152,18 @@ ensure_canberra (GsdMediaKeysManager *ma
62+ if (manager->priv->ca != NULL)
63+ return;
64+
65++ if (strstr (g_getenv("XDG_CURRENT_DESKTOP"), "ubuntu") != NULL) {
66++ GSettingsSchema *schema;
67++ schema = g_settings_schema_source_lookup(g_settings_schema_source_get_default(),
68++ "com.ubuntu.sound", TRUE);
69++
70++ if (schema != NULL)
71++ {
72++ manager->priv->sound_settings = g_settings_new_full(schema, NULL, NULL);
73++ g_settings_schema_unref(schema);
74++ }
75++ }
76++
77+ ca_context_create (&manager->priv->ca);
78+ ca_context_set_driver (manager->priv->ca, "pulse");
79+ ca_context_change_props (manager->priv->ca, 0,
80+@@ -1172,6 +1184,7 @@ update_dialog (GsdMediaKeysManager *mana
81+ guint vol,
82+ gboolean muted,
83+ gboolean sound_changed,
84++ pa_volume_t max_volume,
85+ gboolean quiet)
86+ {
87+ GvcMixerUIDevice *device;
88+@@ -1179,7 +1192,7 @@ update_dialog (GsdMediaKeysManager *mana
89+ const char *icon;
90+
91+ if (!muted) {
92+- vol = (int) (100 * (double) vol / PA_VOLUME_NORM);
93++ vol = (int) (100 * (double) vol / max_volume);
94+ vol = CLAMP (vol, 0, 100);
95+ } else {
96+ vol = 0.0;
97+@@ -1330,6 +1343,7 @@ do_sound_action (GsdMediaKeysManager *ma
98+ gboolean old_muted, new_muted;
99+ guint old_vol, new_vol, norm_vol_step;
100+ gboolean sound_changed;
101++ pa_volume_t max_volume;
102+
103+ /* Find the stream that corresponds to the device, if any */
104+ stream = NULL;
105+@@ -1347,6 +1361,11 @@ do_sound_action (GsdMediaKeysManager *ma
106+ if (stream == NULL)
107+ return;
108+
109++ if ((manager->priv->sound_settings != NULL) && g_settings_get_boolean (manager->priv->sound_settings, "allow-amplified-volume"))
110++ max_volume = PA_VOLUME_UI_MAX;
111++ else
112++ max_volume = PA_VOLUME_NORM;
113++
114+ norm_vol_step = PA_VOLUME_NORM * VOLUME_STEP / 100;
115+
116+ /* FIXME: this is racy */
117+@@ -1370,7 +1389,7 @@ do_sound_action (GsdMediaKeysManager *ma
118+ new_muted = FALSE;
119+ /* When coming out of mute only increase the volume if it was 0 */
120+ if (!old_muted || old_vol == 0)
121+- new_vol = MIN (old_vol + norm_vol_step, MAX_VOLUME);
122++ new_vol = MIN (old_vol + norm_vol_step, max_volume);
123+ break;
124+ }
125+
126+@@ -1386,7 +1405,7 @@ do_sound_action (GsdMediaKeysManager *ma
127+ }
128+ }
129+
130+- update_dialog (manager, stream, new_vol, new_muted, sound_changed, quiet);
131++ update_dialog (manager, stream, new_vol, new_muted, sound_changed, max_volume, quiet);
132+ }
133+
134+ static void
135+@@ -3032,6 +3051,7 @@ gsd_media_keys_manager_stop (GsdMediaKey
136+ g_clear_object (&priv->power_proxy);
137+ g_clear_object (&priv->power_screen_proxy);
138+ g_clear_object (&priv->power_keyboard_proxy);
139++ g_clear_object (&priv->sound_settings);
140+ g_clear_object (&priv->composite_device);
141+ g_clear_object (&priv->mpris_controller);
142+ g_clear_object (&priv->screencast_proxy);
143
144=== modified file 'debian/patches/series'
145--- debian/patches/series 2017-07-23 12:17:49 +0000
146+++ debian/patches/series 2017-08-16 06:29:52 +0000
147@@ -3,6 +3,7 @@
148 53_sync_input_sources_to_accountsservice.patch
149 64_restore_terminal_keyboard_shortcut_schema.patch
150 correct_logout_action.patch
151+70_allow_sound_above_100.patch
152 ubuntu-lid-close-suspend.patch
153 revert-wacom-migration.patch
154 revert-gsettings-removals.patch
155
156=== modified file 'debian/rules'
157--- debian/rules 2017-07-23 12:51:26 +0000
158+++ debian/rules 2017-08-16 06:29:52 +0000
159@@ -28,12 +28,5 @@
160 override_dh_install:
161 dh_install --fail-missing -X.la
162
163+
164 override_dh_auto_test:
165-ifeq (,$(findstring nocheck,$(DEB_BUILD_OPTIONS)))
166-ifeq (,$(filter $(DEB_HOST_ARCH), s390x hurd-i386 kfreebsd-i386 kfreebsd-amd64))
167- xvfb-run dh_auto_test
168-else
169- -xvfb-run dh_auto_test
170-endif
171-endif
172-

Subscribers

People subscribed via source and target branches

to all changes: