The Sound Menu

Merge lp://staging/~cjcurran/indicator-sound/keyboard-ui-controls into lp://staging/indicator-sound/0.1

  1. keyboard-ui-controls
  2. Merge into trunk
Proposed by Conor Curran
Status: Merged
Merged at revision: not available
Proposed branch: lp://staging/~cjcurran/indicator-sound/keyboard-ui-controls
Merge into: lp://staging/indicator-sound/0.1
Diff against target: 312 lines (+125/-49)
4 files modified
src/indicator-sound.c (+112/-29)
src/pulse-manager.c (+10/-17)
src/sound-service-dbus.c (+1/-1)
src/sound-service.c (+2/-2)
To merge this branch: bzr merge lp://staging/~cjcurran/indicator-sound/keyboard-ui-controls
Reviewer Review Type Date Requested Status
David Barth Approve
Review via email: mp+19430@code.staging.launchpad.net
To post a comment you must log in.
Revision history for this message
Conor Curran (cjcurran) wrote :

In this branch:

Keyboard controls of the sound menu.
 - +/- 5% inc/dec
 - Arrow left/right 5% inc/dec
 - Ctlr left arrow - volume 0
 - Ctlr right arrow - Max vol

New slider.

volume control far smoother - proper use of the API for the current UI spec.

New icons now being pulled in.

Revision history for this message
David Barth (dbarth) wrote :

LGTM, improvements and simplifications. Caution with the GtkRange cast.

+1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/indicator-sound.c'
2--- src/indicator-sound.c 2010-02-15 15:23:54 +0000
3+++ src/indicator-sound.c 2010-02-16 18:00:37 +0000
4@@ -20,10 +20,11 @@
5 You should have received a copy of the GNU General Public License along
6 with this program. If not, see <http://www.gnu.org/licenses/>.
7 */
8-
9+#include <math.h>
10 #include <glib.h>
11 #include <glib-object.h>
12 #include <gtk/gtk.h>
13+#include <gdk/gdkkeysyms.h>
14 #include <libdbusmenu-gtk/menu.h>
15 #include <libido/idoscalemenuitem.h>
16
17@@ -83,6 +84,7 @@
18 static void slider_prop_change_cb (DbusmenuMenuitem * mi, gchar * prop, GValue * value, GtkWidget *widget);
19 static gboolean user_change_value_event_cb(GtkRange *range, GtkScrollType scroll_type, gdouble input_value, gpointer user_data);
20 static gboolean value_changed_event_cb(GtkRange *range, gpointer user_data);
21+static gboolean key_press_cb(GtkWidget* widget, GdkEventKey* event, gpointer data);
22
23 // DBUS communication
24 static DBusGProxy *sound_dbus_proxy = NULL;
25@@ -162,14 +164,13 @@
26 {
27 // TODO we need three more images
28 volume_states = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL, g_free);
29- g_hash_table_insert(volume_states, GINT_TO_POINTER(STATE_MUTED), g_strdup("audio-volume-muted"));
30- g_hash_table_insert(volume_states, GINT_TO_POINTER(STATE_ZERO), g_strdup(/*"audio-volume-zero"*/"audio-volume-muted"));
31- g_hash_table_insert(volume_states, GINT_TO_POINTER(STATE_LOW), g_strdup("audio-volume-low"));
32- g_hash_table_insert(volume_states, GINT_TO_POINTER(STATE_MEDIUM), g_strdup("audio-volume-medium"));
33- g_hash_table_insert(volume_states, GINT_TO_POINTER(STATE_HIGH), g_strdup("audio-volume-high"));
34- g_hash_table_insert(volume_states, GINT_TO_POINTER(STATE_MUTED_WHILE_INPUT), g_strdup(/*"audio-volume-muted-blocking"*/"audio-volume-muted"));
35- g_hash_table_insert(volume_states, GINT_TO_POINTER(STATE_SINKS_NONE), g_strdup(/*"audio-output-none"*/"audio-volume-muted"));
36- //test_images_hash();
37+ g_hash_table_insert(volume_states, GINT_TO_POINTER(STATE_MUTED), g_strdup("audio-volume-muted-panel"));
38+ g_hash_table_insert(volume_states, GINT_TO_POINTER(STATE_ZERO), g_strdup("audio-volume-zero-panel"));
39+ g_hash_table_insert(volume_states, GINT_TO_POINTER(STATE_LOW), g_strdup("audio-volume-low-panel"));
40+ g_hash_table_insert(volume_states, GINT_TO_POINTER(STATE_MEDIUM), g_strdup("audio-volume-medium-panel"));
41+ g_hash_table_insert(volume_states, GINT_TO_POINTER(STATE_HIGH), g_strdup("audio-volume-high-panel"));
42+ g_hash_table_insert(volume_states, GINT_TO_POINTER(STATE_MUTED_WHILE_INPUT), g_strdup("audio-volume-muted-blocking-panel"));
43+ g_hash_table_insert(volume_states, GINT_TO_POINTER(STATE_SINKS_NONE), g_strdup("audio-output-none-panel"));
44 }
45
46 static void
47@@ -257,10 +258,20 @@
48
49 static void catch_signal_sink_volume_update(DBusGProxy *proxy, gdouble volume_percent, gpointer userdata)
50 {
51- g_debug("signal caught - update sink volume with value : %f", volume_percent);
52 GtkWidget *slider = ido_scale_menu_item_get_scale((IdoScaleMenuItem*)volume_slider);
53 GtkRange *range = (GtkRange*)slider;
54- gtk_range_set_value(range, volume_percent);
55+
56+ // DEBUG
57+ gdouble current_value = gtk_range_get_value(range);
58+ g_debug("SIGNAL- update sink volume - current_value : %f and new value : %f", current_value, volume_percent);
59+
60+ // Don't like this solution - too fuzzy
61+ // Need the ability to detect if the slider is grabbed
62+ if(floor(current_value) != floor(volume_percent))
63+ {
64+ g_debug("Going to update slider value");
65+ gtk_range_set_value(range, volume_percent);
66+ }
67 determine_state_from_volume(volume_percent);
68 }
69
70@@ -315,11 +326,11 @@
71
72 static void update_state(const gint state)
73 {
74- g_debug("update state beginning - previous_state = %i", previous_state);
75+/* g_debug("update state beginning - previous_state = %i", previous_state);*/
76
77 previous_state = current_state;
78
79- g_debug("update state 3rd line - previous_state = %i", previous_state);
80+/* g_debug("update state 3rd line - previous_state = %i", previous_state);*/
81
82 current_state = state;
83 gchar* image_name = g_hash_table_lookup(volume_states, GINT_TO_POINTER(current_state));
84@@ -338,7 +349,7 @@
85
86 static void determine_state_from_volume(gdouble volume_percent)
87 {
88- g_debug("determine_state_from_volume - previous_state = %i", previous_state);
89+/* g_debug("determine_state_from_volume - previous_state = %i", previous_state);*/
90 gint state = previous_state;
91 if (volume_percent < 30.0 && volume_percent > 0){
92 state = STATE_LOW;
93@@ -366,6 +377,9 @@
94 DbusmenuGtkClient *client = dbusmenu_gtkmenu_get_client(menu);
95 dbusmenu_client_add_type_handler(DBUSMENU_CLIENT(client), DBUSMENU_SLIDER_MENUITEM_TYPE, new_slider_item);
96
97+ // register Key-press listening on the widget
98+ g_signal_connect(menu, "key-press-event", G_CALLBACK(key_press_cb), NULL);
99+
100 return GTK_MENU(menu);
101 }
102
103@@ -378,22 +392,23 @@
104 g_object_set(volume_slider, "reverse-scroll-events", TRUE, NULL);
105
106 GtkMenuItem *menu_volume_slider = GTK_MENU_ITEM(volume_slider);
107+
108 dbusmenu_gtkclient_newitem_base(DBUSMENU_GTKCLIENT(client), newitem, menu_volume_slider, parent);
109 g_signal_connect(G_OBJECT(newitem), DBUSMENU_MENUITEM_SIGNAL_PROPERTY_CHANGED, G_CALLBACK(slider_prop_change_cb), volume_slider);
110
111+ // register slider changes listening on the range
112 GtkWidget* slider = ido_scale_menu_item_get_scale((IdoScaleMenuItem*)volume_slider);
113 g_signal_connect(slider, "change-value", G_CALLBACK(user_change_value_event_cb), newitem);
114 g_signal_connect(slider, "value-changed", G_CALLBACK(value_changed_event_cb), newitem);
115
116+ // Set images on the ido
117 primary_image = ido_scale_menu_item_get_primary_image((IdoScaleMenuItem*)volume_slider);
118 gtk_image_set_from_icon_name(GTK_IMAGE(primary_image), g_hash_table_lookup(volume_states, GINT_TO_POINTER(STATE_ZERO)), GTK_ICON_SIZE_MENU);
119 GtkWidget* secondary_image = ido_scale_menu_item_get_secondary_image((IdoScaleMenuItem*)volume_slider);
120 gtk_image_set_from_icon_name(GTK_IMAGE(secondary_image), g_hash_table_lookup(volume_states, GINT_TO_POINTER(STATE_HIGH)), GTK_ICON_SIZE_MENU);
121
122-/* GtkRange* range = (GtkRange*)slider; */
123-/* gtk_range_set_value(range, initial_volume_percent); */
124-
125 gtk_widget_show_all(volume_slider);
126+
127 return TRUE;
128 }
129
130@@ -417,18 +432,86 @@
131 static gboolean value_changed_event_cb(GtkRange *range, gpointer user_data)
132 {
133 gdouble current_value = gtk_range_get_value(range);
134- if(current_value == 0 || current_value == 100)
135- {
136- DbusmenuMenuitem *item = (DbusmenuMenuitem*)user_data;
137- GValue value = {0};
138- g_value_init(&value, G_TYPE_DOUBLE);
139- g_value_set_double(&value, current_value);
140- g_debug("Value changed listener - = %f", current_value);
141- dbusmenu_menuitem_handle_event (item, "slider_change", &value, 0);
142- }
143- return FALSE;
144-}
145-
146+/* if(current_value == 0 || current_value == 100)*/
147+/* {*/
148+ DbusmenuMenuitem *item = (DbusmenuMenuitem*)user_data;
149+ GValue value = {0};
150+ g_value_init(&value, G_TYPE_DOUBLE);
151+ g_value_set_double(&value, current_value);
152+ g_debug("Value changed callback - = %f", current_value);
153+ dbusmenu_menuitem_handle_event (item, "slider_change", &value, 0);
154+/* }*/
155+ return FALSE;
156+}
157+
158+/**
159+key_press_cb:
160+**/
161+static gboolean key_press_cb(GtkWidget* widget, GdkEventKey* event, gpointer data)
162+{
163+
164+ if(event->length > 0)
165+ g_debug("The key event's string is '%s'\n", event->string);
166+
167+ GtkWidget* slider = ido_scale_menu_item_get_scale((IdoScaleMenuItem*)volume_slider);
168+ GtkRange* range = (GtkRange*)slider;
169+ gdouble current_value = gtk_range_get_value(range);
170+ gdouble new_value = current_value;
171+ const gdouble five_percent = 5;
172+
173+ switch(event->keyval)
174+ {
175+ case GDK_Right:
176+ if(event->state & GDK_CONTROL_MASK)
177+ {
178+/* g_debug("right key was pressed with ctrl- volume set to 100"); */
179+ new_value = 100;
180+ }
181+ else
182+ {
183+/* g_debug("right key was pressed - normal 5 percent increase"); */
184+ new_value = current_value + five_percent;
185+ }
186+ break;
187+ case GDK_Left:
188+ if(event->state & GDK_CONTROL_MASK)
189+ {
190+/* g_debug("left key was pressed with ctrl- volume set to 0"); */
191+ new_value = 0;
192+ }
193+ else
194+ {
195+/* g_debug("left key was pressed - normal 5 percent decrease"); */
196+ new_value = current_value - five_percent;
197+ }
198+ break;
199+ case GDK_plus:
200+/* g_debug("Plus key was pressed");*/
201+ new_value = current_value + five_percent;
202+ break;
203+ case GDK_minus:
204+/* g_debug("minus key was pressed");*/
205+ new_value = current_value - five_percent;
206+ break;
207+ default:
208+ break;
209+ }
210+
211+/* g_debug("new range value without being clamped = %f", new_value); */
212+
213+ new_value = CLAMP(new_value, 0, 100);
214+ if(new_value != current_value)
215+ {
216+ g_debug("Attempting to set the range to %f", new_value);
217+ gtk_range_set_value(range, new_value);
218+ }
219+ return FALSE;
220+}
221+
222+/**
223+This callback should only be called when the user actually drags the slider.
224+Turned off for now in favour of the non descriminating call back.
225+**/
226 static gboolean user_change_value_event_cb(GtkRange *range, GtkScrollType scroll_type, gdouble input_value, gpointer user_data)
227 {
228 DbusmenuMenuitem *item = (DbusmenuMenuitem*)user_data;
229
230=== modified file 'src/pulse-manager.c'
231--- src/pulse-manager.c 2010-02-15 12:42:08 +0000
232+++ src/pulse-manager.c 2010-02-16 18:00:37 +0000
233@@ -189,18 +189,15 @@
234 g_warning("We have no default sink !!! - returning after not attempting to set any volume of any sink");
235 return;
236 }
237- gdouble linear_input = (gdouble)(percent);
238- linear_input /= 100.0;
239- g_debug("linear double input = %f", linear_input);
240- pa_volume_t new_volume = pa_sw_volume_from_linear(linear_input);
241- // Use this to achieve more accurate scaling using the base volume (in the sink struct already!)
242- //pa_volume_t new_volume = (pa_volume_t) ((GPOINTER_TO_INT(linear_input) * s->base_volume) / 100);
243- g_debug("about to try to set the sw volume to a linear volume of %f", pa_sw_volume_to_linear(new_volume));
244- g_debug("and an actual volume of %f", (gdouble)new_volume);
245+
246+ sink_info *s = g_hash_table_lookup(sink_hash, GINT_TO_POINTER(DEFAULT_SINK_INDEX));
247+
248+ pa_volume_t new_volume = (pa_volume_t) ((percent * PA_VOLUME_NORM) / 100);
249+ g_debug("new_volume double check :%f", pa_sw_volume_to_linear(new_volume));
250+ g_debug("new volume calculated :%f", (gdouble)new_volume);
251 pa_cvolume dev_vol;
252- sink_info *s = g_hash_table_lookup(sink_hash, GINT_TO_POINTER(DEFAULT_SINK_INDEX));
253 pa_cvolume_set(&dev_vol, s->volume.channels, new_volume);
254-
255+ // TODO why don't you update the sink_info here with the appropriate pa_cvolume (&dev_vol)
256 pa_operation_unref(pa_context_set_sink_volume_by_index(pulse_context, DEFAULT_SINK_INDEX, &dev_vol, NULL, NULL));
257 }
258
259@@ -321,7 +318,6 @@
260 if(position >= 0) // => index is within the keys of the hash.
261 {
262 sink_info *s = g_hash_table_lookup(sink_hash, GINT_TO_POINTER(info->index));
263- //g_debug("attempting to update sink with name %s", s->name);
264 s->name = g_strdup(info->name);
265 s->description = g_strdup(info->description);
266 s->icon_name = g_strdup(pa_proplist_gets(info->proplist, PA_PROP_DEVICE_ICON_NAME));
267@@ -336,14 +332,11 @@
268 {
269 //update the UI
270 pa_volume_t vol = pa_cvolume_avg(&s->volume);
271- // Use the base of the device to ensure maximum acceptable levels on the hardware
272- gdouble volume_percent = (vol/s->base_volume) * 100;
273- g_debug("When using base volume => volume = %f", volume_percent);
274- g_debug("about to update ui with linear volume of %f", pa_sw_volume_to_linear(vol));
275- sound_service_dbus_update_sink_volume(dbus_service, pa_sw_volume_to_linear(vol));
276+ gdouble volume_percent = ((gdouble) vol * 100) / PA_VOLUME_NORM;
277+ g_debug("Updating volume from PA manager with volume = %f", volume_percent);
278+ sound_service_dbus_update_sink_volume(dbus_service, volume_percent);
279 if (mute_changed == TRUE)
280 sound_service_dbus_update_sink_mute(dbus_service, s->mute);
281-
282 update_mute_ui(s->mute);
283 }
284 }
285
286=== modified file 'src/sound-service-dbus.c'
287--- src/sound-service-dbus.c 2010-02-10 12:45:23 +0000
288+++ src/sound-service-dbus.c 2010-02-16 18:00:37 +0000
289@@ -194,7 +194,7 @@
290 void sound_service_dbus_update_sink_volume(SoundServiceDbus* obj, gdouble sink_volume)
291 {
292 SoundServiceDbusPrivate *priv = SOUND_SERVICE_DBUS_GET_PRIVATE (obj);
293- priv->volume_percent = sink_volume * 100;
294+ priv->volume_percent = sink_volume;
295
296 g_debug("Emitting signal: SINK_VOLUME_UPDATE, with sink_volme %f", priv->volume_percent);
297 g_signal_emit(obj,
298
299=== modified file 'src/sound-service.c'
300--- src/sound-service.c 2010-02-10 18:12:23 +0000
301+++ src/sound-service.c 2010-02-16 18:00:37 +0000
302@@ -124,8 +124,8 @@
303 if (mainloop != NULL) {
304 g_debug("Service shutdown !");
305 // TODO: uncomment for release !!
306- close_pulse_activites();
307- g_main_loop_quit(mainloop);
308+/* close_pulse_activites();*/
309+/* g_main_loop_quit(mainloop);*/
310 }
311 return;
312 }

Subscribers

People subscribed via source and target branches