Merge lp:~cjcurran/indicator-sound/reconnect-bug into lp:indicator-sound/sound-menu-v2

Proposed by Conor Curran
Status: Merged
Merged at revision: 152
Proposed branch: lp:~cjcurran/indicator-sound/reconnect-bug
Merge into: lp:indicator-sound/sound-menu-v2
Diff against target: 161 lines (+69/-15)
1 file modified
src/pulse-manager.c (+69/-15)
To merge this branch: bzr merge lp:~cjcurran/indicator-sound/reconnect-bug
Reviewer Review Type Date Requested Status
Ted Gould (community) Needs Fixing
Review via email: mp+39987@code.launchpad.net

Description of the change

fixes the bug attached.
when the daemon dies, the state callback should be notified at which point it should schedule a reconnect request after a 5 second delay.
once the pulse context has been renewed it should try to connect again. If this does not work then it should wait idle until pulse decides to start again.

To post a comment you must log in.
Revision history for this message
Ted Gould (ted) wrote :

 * I think you need to do some sort of back off on the reconnection. It seems like there might be cases where people have disabled Pulse, and it wouldn't be coming back anytime soon.
 * I'd only create the hashtable and context in the function, and just call that function at init(). That way there is less likely to be bugs in the future.

review: Needs Fixing
Revision history for this message
Conor Curran (cjcurran) wrote :

Actually doesn't need to continually try to reconnect, should just recreate the context and try to connect once. The new pulse context will then sit idle waiting from any state change for pulse without polling.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/pulse-manager.c'
2--- src/pulse-manager.c 2010-10-05 16:15:34 +0000
3+++ src/pulse-manager.c 2010-11-03 16:32:56 +0000
4@@ -20,7 +20,6 @@
5 with this program. If not, see <http://www.gnu.org/licenses/>.
6 */
7
8-
9 #include <pulse/glib-mainloop.h>
10 #include <pulse/error.h>
11 #include <pulse/gccmacro.h>
12@@ -28,14 +27,18 @@
13 #include "pulse-manager.h"
14 #include "dbus-menu-manager.h"
15
16+#define RECONNECT_DELAY 5
17+
18 static GHashTable *sink_hash = NULL;
19 static SoundServiceDbus *dbus_service = NULL;
20 static gint DEFAULT_SINK_INDEX = -1;
21 static gboolean pa_server_available = FALSE;
22-// PA related
23+static gint reconnect_idle_id = 0;
24 static pa_context *pulse_context = NULL;
25 static pa_glib_mainloop *pa_main_loop = NULL;
26+
27 static void context_state_callback(pa_context *c, void *userdata);
28+static gboolean reconnect_to_pulse();
29 static void pulse_sink_info_callback(pa_context *c, const pa_sink_info *sink_info, int eol, void *userdata);
30 static void context_success_callback(pa_context *c, int success, void *userdata);
31 static void pulse_sink_input_info_callback(pa_context *c, const pa_sink_input_info *info, int eol, void *userdata);
32@@ -61,15 +64,19 @@
33 dbus_service = service;
34 pa_main_loop = pa_glib_mainloop_new(g_main_context_default());
35 g_assert(pa_main_loop);
36- pulse_context = pa_context_new(pa_glib_mainloop_get_api(pa_main_loop), "ayatana.indicator.sound");
37+ pulse_context = pa_context_new(pa_glib_mainloop_get_api(pa_main_loop),
38+ "ayatana.indicator.sound");
39 g_assert(pulse_context);
40
41- sink_hash = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL, destroy_sink_info);
42+ sink_hash = g_hash_table_new_full(g_direct_hash,
43+ g_direct_equal,
44+ NULL,
45+ destroy_sink_info);
46
47 // Establish event callback registration
48- pa_context_set_state_callback(pulse_context, context_state_callback, NULL);
49- dbus_menu_manager_update_pa_state(FALSE, FALSE, FALSE, 0);
50- pa_context_connect(pulse_context, NULL, PA_CONTEXT_NOFAIL, NULL);
51+ pa_context_set_state_callback (pulse_context, context_state_callback, NULL);
52+ dbus_menu_manager_update_pa_state (FALSE, FALSE, FALSE, 0);
53+ pa_context_connect (pulse_context, NULL, PA_CONTEXT_NOFAIL, NULL);
54 }
55
56 /**
57@@ -81,6 +88,41 @@
58 return pulse_context;
59 }
60
61+static gboolean
62+reconnect_to_pulse()
63+{
64+ g_debug("Attempt to reconnect to pulse");
65+ // reset
66+ if (pulse_context != NULL) {
67+ pa_context_unref(pulse_context);
68+ pulse_context = NULL;
69+ }
70+
71+ if (sink_hash != NULL) {
72+ g_hash_table_destroy(sink_hash);
73+ sink_hash = NULL;
74+ }
75+ pulse_context = pa_context_new( pa_glib_mainloop_get_api( pa_main_loop ),
76+ "ayatana.indicator.sound" );
77+ g_assert(pulse_context);
78+ sink_hash = g_hash_table_new_full( g_direct_hash, g_direct_equal,
79+ NULL,
80+ destroy_sink_info );
81+ // Establish event callback registration
82+ pa_context_set_state_callback (pulse_context, context_state_callback, NULL);
83+ int result = pa_context_connect (pulse_context, NULL, PA_CONTEXT_NOFAIL, NULL);
84+
85+ if (result < 0) {
86+ g_warning ("Failed to connect context: %s",
87+ pa_strerror (pa_context_errno (pulse_context)));
88+ }
89+ // we always want to cancel any continious callbacks with the existing timeout
90+ // if the connection failed the new context created above will catch any updates
91+ // to do with the state of pulse and thus take care of business.
92+ reconnect_idle_id = 0;
93+ return FALSE;
94+}
95+
96 /**
97 close_pulse_activites()
98 Gracefully close our connection with the Pulse async library.
99@@ -294,9 +336,6 @@
100 static void pulse_default_sink_info_callback(pa_context *c, const pa_sink_info *info, int eol, void *userdata)
101 {
102 if (eol > 0) {
103- if (pa_context_errno(c) == PA_ERR_NOENTITY)
104- return;
105- g_warning("Default Sink info callback failure");
106 return;
107 } else {
108 DEFAULT_SINK_INDEX = info->index;
109@@ -416,7 +455,9 @@
110 }
111
112
113-static void pulse_server_info_callback(pa_context *c, const pa_server_info *info, void *userdata)
114+static void pulse_server_info_callback(pa_context *c,
115+ const pa_server_info *info,
116+ void *userdata)
117 {
118 /* g_debug("server info callback");*/
119 pa_operation *operation;
120@@ -428,7 +469,10 @@
121 }
122 pa_server_available = TRUE;
123 if (info->default_sink_name != NULL) {
124- if (!(operation = pa_context_get_sink_info_by_name(c, info->default_sink_name, pulse_default_sink_info_callback, userdata))) {
125+ if (!(operation = pa_context_get_sink_info_by_name(c,
126+ info->default_sink_name,
127+ pulse_default_sink_info_callback,
128+ userdata))) {
129 g_warning("pa_context_get_sink_info_by_name() failed");
130 } else {
131 pa_operation_unref(operation);
132@@ -502,16 +546,26 @@
133 /* g_debug("context setting name");*/
134 break;
135 case PA_CONTEXT_FAILED:
136- g_warning("FAILED to retrieve context - Is PulseAudio Daemon running ?");
137+ g_warning("PA_CONTEXT_FAILED - Is PulseAudio Daemon running ?");
138 pa_server_available = FALSE;
139+ dbus_menu_manager_update_pa_state(TRUE,
140+ pa_server_available,
141+ default_sink_is_muted(),
142+ get_default_sink_volume());
143+
144+ if (reconnect_idle_id == 0){
145+ reconnect_idle_id = g_timeout_add_seconds (RECONNECT_DELAY,
146+ reconnect_to_pulse,
147+ NULL);
148+ }
149 break;
150 case PA_CONTEXT_TERMINATED:
151 /* g_debug("context terminated");*/
152 break;
153 case PA_CONTEXT_READY:
154- g_debug("PA daemon is ready");
155+ g_debug("PA_CONTEXT_READY");
156 pa_operation *o;
157-
158+
159 pa_context_set_subscribe_callback(c, subscribed_events_callback, userdata);
160
161 if (!(o = pa_context_subscribe(c, (pa_subscription_mask_t)

Subscribers

People subscribed via source and target branches

to all changes: