Merge lp:~cjcurran/indicator-sound/dynamic_sink_switch_smooth_sliding into lp:indicator-sound/0.1

[Conor Curran]

[Conor Curran (cjcurran)]

This branch contains fixes to enable dynamic switching of the output devices and change to enable smooth sliding. I had to change some stuff around for this but it should be thoroughly tested .

[Cody Russell (bratsche)]

It would be nice if you could remove some of the stuff that you're just commenting out, as it's kind of going to clutter up the code for other people reading it. But otherwise, +1

review: Approve

=== modified file 'src/indicator-sound.c'
--- src/indicator-sound.c	2010-02-16 16:38:03 +0000
+++ src/indicator-sound.c	2010-02-17 17:37:14 +0000
@@ -82,7 +82,7 @@
 static GtkWidget *volume_slider = NULL;
 static gboolean new_slider_item (DbusmenuMenuitem * newitem, DbusmenuMenuitem * parent, DbusmenuClient * client);
 static void slider_prop_change_cb (DbusmenuMenuitem * mi, gchar * prop, GValue * value, GtkWidget *widget);
-static gboolean user_change_value_event_cb(GtkRange *range, GtkScrollType scroll_type, gdouble input_value, gpointer  user_data);
+/*static gboolean user_change_value_event_cb(GtkRange *range, GtkScrollType scroll_type, gdouble input_value, gpointer  user_data);*/
 static gboolean value_changed_event_cb(GtkRange *range, gpointer user_data);
 static gboolean key_press_cb(GtkWidget* widget, GdkEventKey* event, gpointer data);
 
@@ -148,15 +148,6 @@
 }
 
 
-/*static void test_images_hash()*/
-/*{*/
-/*    g_debug("about to test the images hash");      */
-/*    gchar* current_name = g_hash_table_lookup(volume_states, GINT_TO_POINTER(current_state));*/
-/*    g_debug("start up current image name  = %s", current_name);       */
-/*    gchar* previous_name = g_hash_table_lookup(volume_states, GINT_TO_POINTER(previous_state));*/
-/*    g_debug("start up previous image name  = %s", previous_name);       */
-/*}*/
-
 /*
 Prepare states Array.
 */
@@ -264,14 +255,7 @@
     // DEBUG
     gdouble current_value = gtk_range_get_value(range);
     g_debug("SIGNAL- update sink volume - current_value : %f and new value : %f", current_value, volume_percent);
-
-    // Don't like this solution - too fuzzy
-    // Need the ability to detect if the slider is grabbed
-    if(floor(current_value) != floor(volume_percent))
-    {
-        g_debug("Going to update slider value");
-        gtk_range_set_value(range, volume_percent); 
-    }
+    gtk_range_set_value(range, volume_percent); 
     determine_state_from_volume(volume_percent);
 }
 
@@ -337,15 +321,6 @@
     gtk_image_set_from_icon_name(speaker_image, image_name, GTK_ICON_SIZE_MENU);
 }
 
-/*static void revert_state()*/
-/*{*/
-
-/*    g_debug("revert state beginning - previous_state = %i", previous_state);*/
-/*    current_state = previous_state;*/
-/*    gchar* image_name = g_hash_table_lookup(volume_states, GINT_TO_POINTER(current_state));*/
-/*    gtk_image_set_from_icon_name(speaker_image, image_name, GTK_ICON_SIZE_MENU);*/
-/*    g_debug("after reverting back to previous state of %i", current_state);*/
-/*}*/
 
 static void determine_state_from_volume(gdouble volume_percent)
 {
@@ -377,12 +352,16 @@
 	DbusmenuGtkClient *client = dbusmenu_gtkmenu_get_client(menu);	
     dbusmenu_client_add_type_handler(DBUSMENU_CLIENT(client), DBUSMENU_SLIDER_MENUITEM_TYPE, new_slider_item);
 
-    // register Key-press listening on the widget
+    // register Key-press listening on the menu widget as the slider does not allow this.
     g_signal_connect(menu, "key-press-event", G_CALLBACK(key_press_cb), NULL);         
 
     return GTK_MENU(menu);
 }
 
+/**
+new_slider_item:
+Create a new dBusMenu Slider item, register the 
+**/
 static gboolean new_slider_item(DbusmenuMenuitem * newitem, DbusmenuMenuitem * parent, DbusmenuClient * client)
 {
 	g_return_val_if_fail(DBUSMENU_IS_MENUITEM(newitem), FALSE);
@@ -398,7 +377,7 @@
     
     // register slider changes listening on the range
     GtkWidget* slider = ido_scale_menu_item_get_scale((IdoScaleMenuItem*)volume_slider);  
-    g_signal_connect(slider, "change-value", G_CALLBACK(user_change_value_event_cb), newitem);     
+/*    g_signal_connect(slider, "change-value", G_CALLBACK(user_change_value_event_cb), newitem);     */
     g_signal_connect(slider, "value-changed", G_CALLBACK(value_changed_event_cb), newitem);     
     
     // Set images on the ido
@@ -412,8 +391,10 @@
 	return TRUE;
 }
 
-/* Whenever we have a property change on a DbusmenuMenuitem
-   we need to be responsive to that. */
+/**
+slider_prop_change_cb:
+Whenever we have a property change on a DbusmenuMenuitem this will be called. 
+**/
 static void slider_prop_change_cb (DbusmenuMenuitem * mi, gchar * prop, GValue * value, GtkWidget *widget)
 {
     g_debug("slider_prop_change_cb - dodgy updater ");
@@ -425,22 +406,21 @@
 }
 
 /**
-This callback will get triggered irregardless of whether its a user change or a programmatic change
-Our usecase for this particular callback is only interested if the slider is changed by the user hitting either icon
-which will result in a programmatic value change of 0 or 100 (work around).
+value_changed_event_cb:
+This callback will get triggered irregardless of whether its a user change or a programmatic change.
 **/
 static gboolean value_changed_event_cb(GtkRange *range, gpointer user_data)
 {
-    gdouble current_value = gtk_range_get_value(range);        
-/*    if(current_value == 0 || current_value == 100)*/
-/*    {*/
+    gdouble current_value =  CLAMP(gtk_range_get_value(range), 0, 100);        
     DbusmenuMenuitem *item = (DbusmenuMenuitem*)user_data;
     GValue value = {0};
     g_value_init(&value, G_TYPE_DOUBLE);
     g_value_set_double(&value, current_value);
     g_debug("Value changed callback - = %f", current_value);
-    dbusmenu_menuitem_handle_event (item, "slider_change", &value, 0);        
-/*    }*/
+    dbusmenu_menuitem_handle_event (item, "slider_change", &value, 0);  
+    // This is not ideal in that the icon ui will update on ui actions and not on actual service feedback.
+    // but necessary for now as the server does not send volume update information if the source of change was this ui.
+    determine_state_from_volume(current_value);
     return FALSE;
 }
 
@@ -450,9 +430,8 @@
 static gboolean key_press_cb(GtkWidget* widget, GdkEventKey* event, gpointer data)
 {
 
-    if(event->length > 0)
-        g_debug("The key event's string is '%s'\n", event->string);
-
+/*    if(event->length > 0)*/
+/*        g_debug("The key event's string is '%s'\n", event->string);*/
     GtkWidget* slider = ido_scale_menu_item_get_scale((IdoScaleMenuItem*)volume_slider);
     GtkRange* range = (GtkRange*)slider;       
     gdouble current_value = gtk_range_get_value(range);  
@@ -502,7 +481,7 @@
         new_value = CLAMP(new_value, 0, 100);
         if(new_value != current_value)
         {
-            g_debug("Attempting to set the range to %f", new_value);        
+            g_debug("Attempting to set the range from the key listener to %f", new_value);        
             gtk_range_set_value(range, new_value);  
         }
     return FALSE;
@@ -510,18 +489,20 @@
 
 /**
 This callback should only be called when the user actually drags the slider.
-Turned off for now in favour of the non descriminating call back.
+Turned off for now in favour of the non descriminating value-changed call back.
+Once the grabbing listener is implemented on the slider may revert to using this.
+Its another tool for filtering unwanted volume change updates.
 **/
-static gboolean user_change_value_event_cb(GtkRange *range, GtkScrollType scroll_type, gdouble input_value, gpointer  user_data)
-{
-    DbusmenuMenuitem *item = (DbusmenuMenuitem*)user_data;
-    gdouble clamped_input = CLAMP(input_value, 0, 100);
-    GValue value = {0};
-    g_debug("User input on SLIDER - = %f", clamped_input);
-    g_value_init(&value, G_TYPE_DOUBLE);
-    g_value_set_double(&value, clamped_input);
-    dbusmenu_menuitem_handle_event (item, "slider_change", &value, 0);
-    return FALSE;  
-} 
+/*static gboolean user_change_value_event_cb(GtkRange *range, GtkScrollType scroll_type, gdouble input_value, gpointer  user_data)*/
+/*{*/
+/*    DbusmenuMenuitem *item = (DbusmenuMenuitem*)user_data;*/
+/*    gdouble clamped_input = CLAMP(input_value, 0, 100);*/
+/*    GValue value = {0};*/
+/*    g_debug("User input on SLIDER - = %f", clamped_input);*/
+/*    g_value_init(&value, G_TYPE_DOUBLE);*/
+/*    g_value_set_double(&value, clamped_input);*/
+/*    dbusmenu_menuitem_handle_event (item, "slider_change", &value, 0);*/
+/*    return FALSE;  */
+/*} */
 
 

=== modified file 'src/pulse-manager.c'
--- src/pulse-manager.c	2010-02-17 11:15:00 +0000
+++ src/pulse-manager.c	2010-02-17 17:37:14 +0000
@@ -43,10 +43,16 @@
 static void pulse_sink_input_info_callback(pa_context *c, const pa_sink_input_info *info, int eol, void *userdata);
 static void pulse_server_info_callback(pa_context *c, const pa_server_info *info, void *userdata);
 static void update_sink_info(pa_context *c, const pa_sink_info *info, int eol, void *userdata);
+static void pulse_source_info_callback(pa_context *c, const pa_source_info *i, int eol, void *userdata); 
 static void destroy_sink_info(void *value);
 static gboolean determine_sink_availability();
 
 
+/**
+Refactoring notes
+Push all UI updates out through update PA state in the service. 
+**/
+
 /*
 Entry point
 */
@@ -167,6 +173,13 @@
 {
     sink_info *info = (sink_info*)value;
     pa_operation_unref(pa_context_set_sink_mute_by_index(pulse_context, info->index, GPOINTER_TO_INT(user_data), context_success_callback,  NULL));
+    if(GPOINTER_TO_INT(user_data) == 1){
+        sound_service_dbus_update_sink_mute(dbus_service, TRUE);
+    }
+    else{
+        sound_service_dbus_update_sink_volume(dbus_service, get_default_sink_volume());
+    }
+
     g_debug("in the pulse manager: mute each sink %i", GPOINTER_TO_INT(user_data));
 }
 
@@ -197,8 +210,9 @@
     g_debug("new volume calculated :%f", (gdouble)new_volume);
     pa_cvolume dev_vol;
     pa_cvolume_set(&dev_vol, s->volume.channels, new_volume);   
-    // TODO why don't you update the sink_info here with the appropriate pa_cvolume (&dev_vol)
+    s->volume = dev_vol;
     pa_operation_unref(pa_context_set_sink_volume_by_index(pulse_context, DEFAULT_SINK_INDEX, &dev_vol, NULL, NULL));
+    
 }
 
 
@@ -242,9 +256,9 @@
         if(device_available == TRUE)
         {
             update_pa_state(TRUE, device_available, default_sink_is_muted(), get_default_sink_volume()); 
-            sound_service_dbus_update_sink_volume(dbus_service, get_default_sink_volume()); 
-            sound_service_dbus_update_sink_mute(dbus_service, default_sink_is_muted()); 
-            g_debug("default sink index : %d", DEFAULT_SINK_INDEX);                    
+            //sound_service_dbus_update_sink_volume(dbus_service, get_default_sink_volume()); 
+            //sound_service_dbus_update_sink_mute(dbus_service, default_sink_is_muted()); 
+            //g_debug("default sink index : %d", DEFAULT_SINK_INDEX);                    
         }
         else{
             //Update the indicator to show PA either is not ready or has no available sink
@@ -281,7 +295,17 @@
     else{
         DEFAULT_SINK_INDEX = info->index;
         g_debug("Just set the default sink index to %i", DEFAULT_SINK_INDEX);    
-        pa_operation_unref(pa_context_get_sink_info_list(c, pulse_sink_info_callback, NULL)); 
+        GList *keys = g_hash_table_get_keys(sink_hash);
+        gint position =  g_list_index(keys, GINT_TO_POINTER(info->index));
+        // Only update sink-list if the index is not in our already fetched list.
+        if(position < 0)
+        {
+            pa_operation_unref(pa_context_get_sink_info_list(c, pulse_sink_info_callback, NULL)); 
+        }
+        else
+        {
+            update_pa_state(TRUE, determine_sink_availability(), default_sink_is_muted(), get_default_sink_volume());             
+        }    
     }
 }
 
@@ -322,19 +346,22 @@
         s->description = g_strdup(info->description);
         s->icon_name = g_strdup(pa_proplist_gets(info->proplist, PA_PROP_DEVICE_ICON_NAME));
         s->active_port = (info->active_port != NULL);
-        // NASTY!!
+        s->mute = !!info->mute;
         gboolean mute_changed = s->mute != !!info->mute;
-        s->mute = !!info->mute;
+        gboolean volume_changed = (pa_cvolume_equal(&info->volume, &s->volume) == 0);
         s->volume = info->volume;
         s->base_volume = info->base_volume;
         s->channel_map = info->channel_map; 
         if(DEFAULT_SINK_INDEX == s->index)
         {
             //update the UI
-            pa_volume_t vol = pa_cvolume_avg(&s->volume);
-            gdouble volume_percent = ((gdouble) vol * 100) / PA_VOLUME_NORM;
-            g_debug("Updating volume from PA manager with volume = %f", volume_percent);
-            sound_service_dbus_update_sink_volume(dbus_service, volume_percent); 
+            if (volume_changed == TRUE)
+            {
+                pa_volume_t vol = pa_cvolume_avg(&s->volume);
+                gdouble volume_percent = ((gdouble) vol * 100) / PA_VOLUME_NORM;
+                g_debug("Updating volume from PA manager with volume = %f", volume_percent);
+                sound_service_dbus_update_sink_volume(dbus_service, volume_percent); 
+            }
             if (mute_changed == TRUE)     
                 sound_service_dbus_update_sink_mute(dbus_service, s->mute);
             update_mute_ui(s->mute);
@@ -382,24 +409,64 @@
     pa_operation_unref(operation);
 }
 
-static void subscribed_events_callback(pa_context *c, enum pa_subscription_event_type t, uint32_t index, void *userdata){
-	switch (t & PA_SUBSCRIPTION_EVENT_FACILITY_MASK) {
+static void pulse_source_info_callback(pa_context *c, const pa_source_info *i, int eol, void *userdata) 
+{
+    g_debug("pulse source info callback");
+}
+
+
+static void subscribed_events_callback(pa_context *c, enum pa_subscription_event_type t, uint32_t index, void *userdata)
+{
+	switch (t & PA_SUBSCRIPTION_EVENT_FACILITY_MASK) 
+    {
         case PA_SUBSCRIPTION_EVENT_SINK:
-            if ((t & PA_SUBSCRIPTION_EVENT_TYPE_MASK) == PA_SUBSCRIPTION_EVENT_REMOVE) {
-                //TODO handle the remove event => if its our default sink - grey out the ui with update_pa_state
-            } else {
+			g_debug("PA_SUBSCRIPTION_EVENT_SINK event triggered");            
+            if ((t & PA_SUBSCRIPTION_EVENT_TYPE_MASK) == PA_SUBSCRIPTION_EVENT_REMOVE) 
+            {
+                //TODO handle the remove event => if its our default sink - update date pa state
+            } else 
+            {
                 pa_operation_unref(pa_context_get_sink_info_by_index(c, index, update_sink_info, userdata));
             }            
             break;
         case PA_SUBSCRIPTION_EVENT_SINK_INPUT:
-			// This will be triggered when the sink receives input from a new stream
-			// If a playback client is paused and then resumed this will NOT trigger this event.
-		    pa_operation_unref(pa_context_get_sink_input_info(c, index, pulse_sink_input_info_callback, userdata));
-		    break;
+			g_debug("PA_SUBSCRIPTION_EVENT_SINK_INPUT event triggered!!");
+            if ((t & PA_SUBSCRIPTION_EVENT_TYPE_MASK) == PA_SUBSCRIPTION_EVENT_REMOVE)
+            {
+                //TODO handle the remove event
+            }            
+            else 
+            {			
+    		    pa_operation_unref(pa_context_get_sink_input_info(c, index, pulse_sink_input_info_callback, userdata));
+    		}
+    	    break;
+        case PA_SUBSCRIPTION_EVENT_SOURCE:
+            g_debug("PA_SUBSCRIPTION_EVENT_SOURCE of some description ???");
+
+            if ((t & PA_SUBSCRIPTION_EVENT_TYPE_MASK) == PA_SUBSCRIPTION_EVENT_REMOVE)
+            {
+                //TODO handle the remove event
+            }
+            else 
+            {
+                pa_operation *o;
+                if (!(o = pa_context_get_source_info_by_index(c, index, pulse_source_info_callback, userdata))) {
+                    g_warning("pa_context_get_source_info_by_index() failed");
+                    return;
+                }
+                pa_operation_unref(o);
+            }
+            break;
         case PA_SUBSCRIPTION_EVENT_SERVER:
-            if ((t & PA_SUBSCRIPTION_EVENT_TYPE_MASK) == PA_SUBSCRIPTION_EVENT_SINK ) {
-                g_debug("server change of some sink type ???");
+            g_debug("PA_SUBSCRIPTION_EVENT_SERVER change of some description ???");
+            pa_operation *o;
+            if(!(o = pa_context_get_server_info(c, pulse_server_info_callback, userdata)))
+            {
+                g_warning("pa_context_get_server_info() failed");
+                return;
             }
+            pa_operation_unref(o);
+            break;
 	}
 }
 

=== modified file 'src/sound-service.c'
--- src/sound-service.c	2010-02-17 11:15:00 +0000
+++ src/sound-service.c	2010-02-17 17:37:14 +0000
@@ -97,6 +97,7 @@
 {
     b_all_muted = incoming_mute_value;
     dbusmenu_menuitem_property_set(mute_all_menuitem, DBUSMENU_MENUITEM_PROP_LABEL, _(b_all_muted == FALSE ? "Mute All" : "Unmute"));    
+    //dbusmenu_menuitem_property_set_bool(DBUSMENU_MENUITEM(volume_slider_menuitem), DBUSMENU_MENUITEM_PROP_ENABLED, b_all_muted);
 }
 
 static void set_global_mute_from_ui()
@@ -104,13 +105,7 @@
     b_all_muted = !b_all_muted;
     toggle_global_mute(b_all_muted); 
     dbusmenu_menuitem_property_set(mute_all_menuitem, DBUSMENU_MENUITEM_PROP_LABEL, _(b_all_muted == FALSE ? "Mute All" : "Unmute"));
-
-/*    GValue value = {0};*/
-/*    g_value_init(&value, G_TYPE_DOUBLE);*/
-/*    g_value_set_double(&value, 99.0);*/
-/*    // Testing*/
-/*    g_debug("BUGGY volume update");*/
-/*    dbusmenu_menuitem_property_set_value(DBUSMENU_MENUITEM(volume_slider_menuitem), DBUSMENU_SLIDER_MENUITEM_PROP_VOLUME, &value);*/
+    //dbusmenu_menuitem_property_set_bool(DBUSMENU_MENUITEM(volume_slider_menuitem), DBUSMENU_MENUITEM_PROP_ENABLED, b_all_muted);
 }
 
 
@@ -137,7 +132,12 @@
     b_pulse_ready = pa_state;
     volume_percent = percent;
 	g_debug("update pa state with state %i, availability of %i, mute value of %i and a volume percent is %f", pa_state, sink_available, sink_muted, volume_percent);
-    rebuild_sound_menu(root_menuitem, dbus_interface);
+    sound_service_dbus_update_sink_volume(dbus_interface, percent); 
+    sound_service_dbus_update_sink_mute(dbus_interface, sink_muted); 
+
+    // Only rebuild the menu on start up...
+    if(volume_slider_menuitem == NULL)
+        rebuild_sound_menu(root_menuitem, dbus_interface);
 }
 
 
@@ -169,7 +169,7 @@
     dbusmenu_server_set_root(server, root_menuitem);
     establish_pulse_activities(dbus_interface);
 
-/*    // THIS DOES NOT WORK FROM HERE*/
+/*    // THIS DOES NOT WORK FROM HERE - race condition ?!?*/
 /*    GValue value = {0};*/
 /*    g_value_init(&value, G_TYPE_DOUBLE);*/
 /*    g_value_set_double(&value, volume_percent * 100);*/