output/pulse: call mixer on state changes

Don't let the mixer plugin "override" the libpulse callbacks.
Instead, add a "mixer" attribute to the pulse_output struct, and call
the mixer on all interesting events.

Makefile.am

@@ -105,6 +105,7 @@ mpd_headers = \
 	src/mixer_plugin.h \
 	src/mixer_type.h \
 	src/mixer/software_mixer_plugin.h \
+	src/mixer/pulse_mixer_plugin.h \
 	src/daemon.h \
 	src/normalize.h \
 	src/compress.h \

src/mixer/pulse_mixer_plugin.c

@@ -17,6 +17,7 @@
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  */
 
+#include "pulse_mixer_plugin.h"
 #include "mixer_api.h"
 #include "output/pulse_output_plugin.h"
 #include "conf.h"
@@ -92,22 +93,21 @@ pulse_mixer_volume_cb(G_GNUC_UNUSED pa_context *context, const pa_sink_input_inf
 }
 
 static void
-pulse_mixer_update(struct pulse_mixer *pm)
+pulse_mixer_update(struct pulse_mixer *pm,
+		   struct pa_context *context, struct pa_stream *stream)
 {
 	pa_operation *o;
 
-	assert(pm->output->stream != NULL);
+	assert(context != NULL);
+	assert(stream != NULL);
+	assert(pa_stream_get_state(stream) == PA_STREAM_READY);
 
-	if (pm->output->context == NULL ||
+	o = pa_context_get_sink_input_info(context,
-	    pa_stream_get_state(pm->output->stream) != PA_STREAM_READY)
+					   pa_stream_get_index(stream),
-		return;
-
-	o = pa_context_get_sink_input_info(pm->output->context,
-					   pa_stream_get_index(pm->output->stream),
 					   pulse_mixer_volume_cb, pm);
 	if (o == NULL) {
 		g_warning("pa_context_get_sink_input_info() failed: %s",
-			  pa_strerror(pa_context_errno(pm->output->context)));
+			  pa_strerror(pa_context_errno(context)));
 		pulse_mixer_offline(pm);
 		return;
 	}
@@ -115,51 +115,13 @@ pulse_mixer_update(struct pulse_mixer *pm)
 	pa_operation_unref(o);
 }
 
-static void
+void
-pulse_mixer_handle_sink_input(struct pulse_mixer *pm,
+pulse_mixer_on_connect(G_GNUC_UNUSED struct pulse_mixer *pm,
-			      pa_subscription_event_type_t t,
+		       struct pa_context *context)
-			      uint32_t idx)
 {
-	if (pm->output->stream == NULL) {
-		pulse_mixer_offline(pm);
-		return;
-	}
-
-	if (idx != pa_stream_get_index(pm->output->stream))
-		return;
-
-	if (t == PA_SUBSCRIPTION_EVENT_NEW ||
-	    t == PA_SUBSCRIPTION_EVENT_CHANGE)
-		pulse_mixer_update(pm);
-}
-
-static void
-pulse_mixer_subscribe_cb(G_GNUC_UNUSED pa_context *c, pa_subscription_event_type_t t,
-	     uint32_t idx, void *userdata)
-{
-	struct pulse_mixer *pm = userdata;
-
-	switch (t & PA_SUBSCRIPTION_EVENT_FACILITY_MASK) {
-	case PA_SUBSCRIPTION_EVENT_SINK_INPUT:
-		pulse_mixer_handle_sink_input(pm,
-					      t & PA_SUBSCRIPTION_EVENT_TYPE_MASK,
-					      idx);
-		break;
-	}
-}
-
-static void
-pulxe_mixer_context_state_cb(pa_context *context, void *userdata)
-{
-	struct pulse_mixer *pm = userdata;
 	pa_operation *o;
 
-	/* pass event to the output's callback function */
+	assert(context != NULL);
-	pulse_output_context_state_cb(context, pm->output);
-
-	if (pa_context_get_state(context) == PA_CONTEXT_READY) {
-		/* subscribe to sink_input events after the connection
-		   has been established */
 
 	o = pa_context_subscribe(context,
 				 (pa_subscription_mask_t)PA_SUBSCRIPTION_MASK_SINK_INPUT,
@@ -171,10 +133,19 @@ pulxe_mixer_context_state_cb(pa_context *context, void *userdata)
 	}
 
 	pa_operation_unref(o);
-
-		if (pm->output->stream != NULL)
-			pulse_mixer_update(pm);
 }
+
+void
+pulse_mixer_on_disconnect(struct pulse_mixer *pm)
+{
+	pulse_mixer_offline(pm);
+}
+
+void
+pulse_mixer_on_change(struct pulse_mixer *pm,
+		      struct pa_context *context, struct pa_stream *stream)
+{
+	pulse_mixer_update(pm, context, stream);
 }
 
 static struct mixer *
@@ -182,6 +153,7 @@ pulse_mixer_init(void *ao, G_GNUC_UNUSED const struct config_param *param,
 		 GError **error_r)
 {
 	struct pulse_mixer *pm;
+	struct pulse_output *po = ao;
 
 	if (ao == NULL) {
 		g_set_error(error_r, pulse_mixer_quark(), 0,
@@ -192,24 +164,10 @@ pulse_mixer_init(void *ao, G_GNUC_UNUSED const struct config_param *param,
 	pm = g_new(struct pulse_mixer,1);
 	mixer_init(&pm->base, &pulse_mixer_plugin);
 
-	pm->output = ao;
 	pm->online = false;
+	pm->output = po;
 
-	pa_threaded_mainloop_lock(pm->output->mainloop);
+	pulse_output_set_mixer(po, pm);
-
-	/* register callbacks (override the output's context state
-	   callback) */
-
-	pa_context_set_state_callback(pm->output->context,
-				      pulxe_mixer_context_state_cb, pm);
-	pa_context_set_subscribe_callback(pm->output->context,
-					  pulse_mixer_subscribe_cb, pm);
-
-	/* check the current state now (we might have missed the first
-	   events!) */
-	pulxe_mixer_context_state_cb(pm->output->context, pm);
-
-	pa_threaded_mainloop_unlock(pm->output->mainloop);
 
 	return &pm->base;
 }
@@ -219,46 +177,13 @@ pulse_mixer_finish(struct mixer *data)
 {
 	struct pulse_mixer *pm = (struct pulse_mixer *) data;
 
-	/* restore callbacks */
+	pulse_output_clear_mixer(pm->output, pm);
-
-	pa_threaded_mainloop_lock(pm->output->mainloop);
-
-	if (pm->output->context != NULL) {
-		pa_context_set_state_callback(pm->output->context,
-					      pulse_output_context_state_cb,
-					      pm->output);
-		pa_context_set_subscribe_callback(pm->output->context,
-						  NULL, NULL);
-	}
-
-	pa_threaded_mainloop_unlock(pm->output->mainloop);
 
 	/* free resources */
 
 	g_free(pm);
 }
 
-static bool
-pulse_mixer_open(struct mixer *data, G_GNUC_UNUSED GError **error_r)
-{
-	struct pulse_mixer *pm = (struct pulse_mixer *) data;
-
-	pa_threaded_mainloop_lock(pm->output->mainloop);
-	if (pm->output->stream != NULL)
-		pulse_mixer_update(pm);
-	pa_threaded_mainloop_unlock(pm->output->mainloop);
-
-	return true;
-}
-
-static void
-pulse_mixer_close(struct mixer *data)
-{
-	struct pulse_mixer *pm = (struct pulse_mixer *) data;
-
-	pulse_mixer_offline(pm);
-}
-
 static int
 pulse_mixer_get_volume(struct mixer *mixer, G_GNUC_UNUSED GError **error_r)
 {
@@ -281,12 +206,10 @@ pulse_mixer_set_volume(struct mixer *mixer, unsigned volume, GError **error_r)
 {
 	struct pulse_mixer *pm = (struct pulse_mixer *) mixer;
 	struct pa_cvolume cvolume;
-	pa_operation *o;
+	bool success;
 
 	pa_threaded_mainloop_lock(pm->output->mainloop);
-
+	if (!pm->online) {
-	if (!pm->online || pm->output->stream == NULL ||
-	    pm->output->context == NULL) {
 		pa_threaded_mainloop_unlock(pm->output->mainloop);
 		g_set_error(error_r, pulse_mixer_quark(), 0, "disconnected");
 		return false;
@@ -294,27 +217,17 @@ pulse_mixer_set_volume(struct mixer *mixer, unsigned volume, GError **error_r)
 
 	pa_cvolume_set(&cvolume, pm->volume.channels,
 		       (pa_volume_t)volume * PA_VOLUME_NORM / 100 + 0.5);
-
+	success = pulse_output_set_volume(pm->output, &cvolume, error_r);
-	o = pa_context_set_sink_input_volume(pm->output->context,
+	if (success)
-					     pa_stream_get_index(pm->output->stream),
+		pm->volume = cvolume;
-					     &cvolume, NULL, NULL);
 	pa_threaded_mainloop_unlock(pm->output->mainloop);
-	if (o == NULL) {
-		g_set_error(error_r, pulse_mixer_quark(), 0,
-			    "failed to set PulseAudio volume");
-		return false;
-	}
 
-	pa_operation_unref(o);
+	return success;
-
-	return true;
 }
 
 const struct mixer_plugin pulse_mixer_plugin = {
 	.init = pulse_mixer_init,
 	.finish = pulse_mixer_finish,
-	.open = pulse_mixer_open,
-	.close = pulse_mixer_close,
 	.get_volume = pulse_mixer_get_volume,
 	.set_volume = pulse_mixer_set_volume,
 };

src/mixer/pulse_mixer_plugin.h

@@ -0,0 +1,39 @@
+/*
+ * Copyright (C) 2003-2009 The Music Player Daemon Project
+ * http://www.musicpd.org
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#ifndef MPD_PULSE_MIXER_PLUGIN_H
+#define MPD_PULSE_MIXER_PLUGIN_H
+
+#include <pulse/def.h>
+
+struct pulse_mixer;
+struct pa_context;
+struct pa_stream;
+
+void
+pulse_mixer_on_connect(struct pulse_mixer *pm, struct pa_context *context);
+
+void
+pulse_mixer_on_disconnect(struct pulse_mixer *pm);
+
+void
+pulse_mixer_on_change(struct pulse_mixer *pm,
+		      struct pa_context *context, struct pa_stream *stream);
+
+#endif

src/output/pulse_output_plugin.c

@@ -20,12 +20,15 @@
 #include "pulse_output_plugin.h"
 #include "output_api.h"
 #include "mixer_list.h"
+#include "mixer/pulse_mixer_plugin.h"
 
 #include <glib.h>
 
 #include <pulse/thread-mainloop.h>
 #include <pulse/context.h>
 #include <pulse/stream.h>
+#include <pulse/introspect.h>
+#include <pulse/subscribe.h>
 #include <pulse/error.h>
 
 #include <assert.h>
@@ -41,6 +44,65 @@ pulse_output_quark(void)
 	return g_quark_from_static_string("pulse_output");
 }
 
+void
+pulse_output_set_mixer(struct pulse_output *po, struct pulse_mixer *pm)
+{
+	assert(po != NULL);
+	assert(po->mixer == NULL);
+	assert(pm != NULL);
+
+	po->mixer = pm;
+
+	pa_threaded_mainloop_lock(po->mainloop);
+
+	if (po->context != NULL &&
+	    pa_context_get_state(po->context) == PA_CONTEXT_READY) {
+		pulse_mixer_on_connect(pm, po->context);
+
+		if (po->stream != NULL &&
+		    pa_stream_get_state(po->stream) == PA_STREAM_READY)
+			pulse_mixer_on_change(pm, po->context, po->stream);
+	}
+
+	pa_threaded_mainloop_unlock(po->mainloop);
+}
+
+void
+pulse_output_clear_mixer(struct pulse_output *po, struct pulse_mixer *pm)
+{
+	assert(po != NULL);
+	assert(pm != NULL);
+	assert(po->mixer == pm);
+
+	po->mixer = NULL;
+}
+
+bool
+pulse_output_set_volume(struct pulse_output *po,
+			const struct pa_cvolume *volume, GError **error_r)
+{
+	pa_operation *o;
+
+	if (po->context == NULL || po->stream == NULL ||
+	    pa_stream_get_state(po->stream) != PA_STREAM_READY) {
+		g_set_error(error_r, pulse_output_quark(), 0, "disconnected");
+		return false;
+	}
+
+	o = pa_context_set_sink_input_volume(po->context,
+					     pa_stream_get_index(po->stream),
+					     volume, NULL, NULL);
+	if (o == NULL) {
+		g_set_error(error_r, pulse_output_quark(), 0,
+			    "failed to set PulseAudio volume: %s",
+			    pa_strerror(pa_context_errno(po->context)));
+		return false;
+	}
+
+	pa_operation_unref(o);
+	return true;
+}
+
 /**
  * \brief waits for a pulseaudio operation to finish, frees it and
  *     unlocks the mainloop
@@ -81,15 +143,24 @@ pulse_output_stream_success_cb(G_GNUC_UNUSED pa_stream *s,
 	pa_threaded_mainloop_signal(po->mainloop, 0);
 }
 
-void
+static void
 pulse_output_context_state_cb(struct pa_context *context, void *userdata)
 {
 	struct pulse_output *po = userdata;
 
 	switch (pa_context_get_state(context)) {
 	case PA_CONTEXT_READY:
+		if (po->mixer != NULL)
+			pulse_mixer_on_connect(po->mixer, context);
+
+		pa_threaded_mainloop_signal(po->mainloop, 0);
+		break;
+
 	case PA_CONTEXT_TERMINATED:
 	case PA_CONTEXT_FAILED:
+		if (po->mixer != NULL)
+			pulse_mixer_on_disconnect(po->mixer);
+
 		/* the caller thread might be waiting for these
 		   states */
 		pa_threaded_mainloop_signal(po->mainloop, 0);
@@ -103,6 +174,27 @@ pulse_output_context_state_cb(struct pa_context *context, void *userdata)
 	}
 }
 
+static void
+pulse_output_subscribe_cb(pa_context *context,
+			  pa_subscription_event_type_t t,
+			  uint32_t idx, void *userdata)
+{
+	struct pulse_output *po = userdata;
+	pa_subscription_event_type_t facility
+		= t & PA_SUBSCRIPTION_EVENT_FACILITY_MASK;
+	pa_subscription_event_type_t type
+		= t & PA_SUBSCRIPTION_EVENT_TYPE_MASK;
+
+	if (po->mixer != NULL &&
+	    facility == PA_SUBSCRIPTION_EVENT_SINK_INPUT &&
+	    po->stream != NULL &&
+	    pa_stream_get_state(po->stream) == PA_STREAM_READY &&
+	    idx == pa_stream_get_index(po->stream) &&
+	    (type == PA_SUBSCRIPTION_EVENT_NEW ||
+	     type == PA_SUBSCRIPTION_EVENT_CHANGE))
+		pulse_mixer_on_change(po->mixer, context, po->stream);
+}
+
 /**
  * Attempt to connect asynchronously to the PulseAudio server.
  *
@@ -143,6 +235,8 @@ pulse_output_setup_context(struct pulse_output *po, GError **error_r)
 
 	pa_context_set_state_callback(po->context,
 				      pulse_output_context_state_cb, po);
+	pa_context_set_subscribe_callback(po->context,
+					  pulse_output_subscribe_cb, po);
 
 	if (!pulse_output_connect(po, error_r)) {
 		pa_context_unref(po->context);
@@ -283,8 +377,17 @@ pulse_output_stream_state_cb(pa_stream *stream, void *userdata)
 
 	switch (pa_stream_get_state(stream)) {
 	case PA_STREAM_READY:
+		if (po->mixer != NULL)
+			pulse_mixer_on_change(po->mixer, po->context, stream);
+
+		pa_threaded_mainloop_signal(po->mainloop, 0);
+		break;
+
 	case PA_STREAM_FAILED:
 	case PA_STREAM_TERMINATED:
+		if (po->mixer != NULL)
+			pulse_mixer_on_disconnect(po->mixer);
+
 		pa_threaded_mainloop_signal(po->mainloop, 0);
 		break;
 

src/output/pulse_output_plugin.h

@@ -23,6 +23,8 @@
 #include <stdbool.h>
 #include <stddef.h>
 
+#include <glib.h>
+
 #if !defined(PA_CHECK_VERSION)
 /**
  * This macro was implemented in libpulse 0.9.16.
@@ -31,12 +33,15 @@
 #endif
 
 struct pa_operation;
+struct pa_cvolume;
 
 struct pulse_output {
 	const char *name;
 	const char *server;
 	const char *sink;
 
+	struct pulse_mixer *mixer;
+
 	struct pa_threaded_mainloop *mainloop;
 	struct pa_context *context;
 	struct pa_stream *stream;
@@ -53,6 +58,13 @@ struct pulse_output {
 };
 
 void
-pulse_output_context_state_cb(struct pa_context *context, void *userdata);
+pulse_output_set_mixer(struct pulse_output *po, struct pulse_mixer *pm);
+
+void
+pulse_output_clear_mixer(struct pulse_output *po, struct pulse_mixer *pm);
+
+bool
+pulse_output_set_volume(struct pulse_output *po,
+			const struct pa_cvolume *volume, GError **error_r);
 
 #endif

test/read_mixer.c

@@ -31,11 +31,25 @@
 #include <unistd.h>
 
 void
-pulse_output_context_state_cb(G_GNUC_UNUSED struct pa_context *context,
+pulse_output_set_mixer(G_GNUC_UNUSED struct pulse_output *po,
-			      G_GNUC_UNUSED void *userdata)
+		       G_GNUC_UNUSED struct pulse_mixer *pm)
 {
 }
 
+void
+pulse_output_clear_mixer(G_GNUC_UNUSED struct pulse_output *po,
+			 G_GNUC_UNUSED struct pulse_mixer *pm)
+{
+}
+
+bool
+pulse_output_set_volume(G_GNUC_UNUSED struct pulse_output *po,
+			G_GNUC_UNUSED const struct pa_cvolume *volume,
+			G_GNUC_UNUSED GError **error_r)
+{
+	return false;
+}
+
 void
 event_pipe_emit(G_GNUC_UNUSED enum pipe_event event)
 {