mixer_api: replaced method "control()" with "{get,set}_volume()"

The method control() is too complicated, and overengineered.  Replace
it with two trivial functions: get_volume() and set_volume().

src/audio.c

@@ -71,10 +71,10 @@ bool mixer_control_setvol(unsigned int device, int volume, int rel)
 	mixer = ao_plugin_get_mixer(output->plugin, output->data);
 	if (mixer != NULL) {
 		if (rel) {
-			int cur_volume;
+			int cur_volume = mixer_get_volume(mixer);
-			if (!mixer_control(mixer, AC_MIXER_GETVOL, &cur_volume)) {
+			if (cur_volume < 0)
 				return false;
-			}
+
 			volume = volume + cur_volume;
 		}
 		if (volume > 100)
@@ -82,7 +82,7 @@ bool mixer_control_setvol(unsigned int device, int volume, int rel)
 		else if (volume < 0)
 			volume = 0;
 
-		return mixer_control(mixer, AC_MIXER_SETVOL, &volume);
+		return mixer_set_volume(mixer, volume);
 	}
 	return false;
 }
@@ -97,8 +97,16 @@ bool mixer_control_getvol(unsigned int device, int *volume)
 
 	output = audio_output_get(device);
 	mixer = ao_plugin_get_mixer(output->plugin, output->data);
-	if (mixer != NULL)
+	if (mixer != NULL) {
-		return mixer_control(mixer, AC_MIXER_GETVOL, volume);
+		int volume2;
+
+		volume2 = mixer_get_volume(mixer);
+		if (volume2 < 0)
+			return false;
+
+		*volume = volume2;
+		return true;
+	}
 
 	return false;
 }

src/mixer/alsa_mixer.c

@@ -146,85 +146,87 @@ alsa_mixer_open(struct mixer *data)
 	return false;
 }
 
-static bool
+static int
-alsa_mixer_control(struct mixer *data, int cmd, void *arg)
+alsa_mixer_get_volume(struct mixer *mixer)
 {
-	struct alsa_mixer *am = (struct alsa_mixer *)data;
+	struct alsa_mixer *am = (struct alsa_mixer *)mixer;
-	switch (cmd) {
+	int err;
-	case AC_MIXER_GETVOL:
+	int ret;
-	{
+	long level;
-		int err;
-		int ret, *volume = arg;
-		long level;
 
-		if (!am->handle && !alsa_mixer_open(data)) {
+	if (am->handle == NULL && !alsa_mixer_open(mixer))
-			return false;
+		return -1;
-		}
+
-		if ((err = snd_mixer_handle_events(am->handle)) < 0) {
+	err = snd_mixer_handle_events(am->handle);
-			g_warning("problems getting alsa volume: %s (snd_mixer_%s)\n",
+	if (err < 0) {
-				snd_strerror(err), "handle_events");
+		g_warning("problems getting alsa volume: %s (snd_mixer_%s)\n",
-			alsa_mixer_close(data);
+			  snd_strerror(err), "handle_events");
-			return false;
+		alsa_mixer_close(mixer);
-		}
+		return false;
-		if ((err = snd_mixer_selem_get_playback_volume(am->elem,
-			       SND_MIXER_SCHN_FRONT_LEFT, &level)) < 0) {
-			g_warning("problems getting alsa volume: %s (snd_mixer_%s)\n",
-				snd_strerror(err), "selem_get_playback_volume");
-			alsa_mixer_close(data);
-			return false;
-		}
-		ret = ((am->volume_set / 100.0) * (am->volume_max - am->volume_min)
-			+ am->volume_min) + 0.5;
-		if (am->volume_set > 0 && ret == level) {
-			ret = am->volume_set;
-		} else {
-			ret = (int)(100 * (((float)(level - am->volume_min)) /
-				(am->volume_max - am->volume_min)) + 0.5);
-		}
-		*volume = ret;
-		return true;
 	}
-	case AC_MIXER_SETVOL:
-	{
-		float vol;
-		long level;
-		int *volume = arg;
-		int err;
 
-		if (!am->handle && !alsa_mixer_open(data)) {
+	err = snd_mixer_selem_get_playback_volume(am->elem,
-			return false;
+						  SND_MIXER_SCHN_FRONT_LEFT,
-		}
+						  &level);
-		vol = *volume;
+	if (err < 0) {
-
+		g_warning("problems getting alsa volume: %s (snd_mixer_%s)\n",
-		am->volume_set = vol + 0.5;
+			  snd_strerror(err), "selem_get_playback_volume");
-		am->volume_set = am->volume_set > 100 ? 100 :
+		alsa_mixer_close(mixer);
-			    (am->volume_set < 0 ? 0 : am->volume_set);
+		return false;
-
-		level = (long)(((vol / 100.0) * (am->volume_max - am->volume_min) +
-			 am->volume_min) + 0.5);
-		level = level > am->volume_max ? am->volume_max : level;
-		level = level < am->volume_min ? am->volume_min : level;
-
-		if ((err = snd_mixer_selem_set_playback_volume_all(am->elem,
-								level)) < 0) {
-			g_warning("problems setting alsa volume: %s\n",
-				snd_strerror(err));
-			alsa_mixer_close(data);
-			return false;
-		}
-		return true;
 	}
-	default:
+
-		g_warning("Unsuported alsa control\n");
+	ret = ((am->volume_set / 100.0) * (am->volume_max - am->volume_min)
-		break;
+	       + am->volume_min) + 0.5;
+	if (am->volume_set > 0 && ret == level) {
+		ret = am->volume_set;
+	} else {
+		ret = (int)(100 * (((float)(level - am->volume_min)) /
+				   (am->volume_max - am->volume_min)) + 0.5);
 	}
-	return false;
+
+	return ret;
+}
+
+static bool
+alsa_mixer_set_volume(struct mixer *mixer, unsigned volume)
+{
+	struct alsa_mixer *am = (struct alsa_mixer *)mixer;
+	float vol;
+	long level;
+	int err;
+
+	if (am->handle == NULL && !alsa_mixer_open(mixer))
+		return false;
+
+	vol = volume;
+
+	am->volume_set = vol + 0.5;
+	am->volume_set = am->volume_set > 100
+		? 100 :
+		(am->volume_set < 0
+		 ? 0 : am->volume_set);
+
+	level = (long)(((vol / 100.0) * (am->volume_max - am->volume_min) +
+			am->volume_min) + 0.5);
+	level = level > am->volume_max ? am->volume_max : level;
+	level = level < am->volume_min ? am->volume_min : level;
+
+	err = snd_mixer_selem_set_playback_volume_all(am->elem, level);
+	if (err < 0) {
+		g_warning("problems setting alsa volume: %s\n",
+			  snd_strerror(err));
+		alsa_mixer_close(mixer);
+		return false;
+	}
+
+	return true;
 }
 
 const struct mixer_plugin alsa_mixer = {
 	.init = alsa_mixer_init,
 	.finish = alsa_mixer_finish,
 	.open = alsa_mixer_open,
-	.control = alsa_mixer_control,
+	.close = alsa_mixer_close,
-	.close = alsa_mixer_close
+	.get_volume = alsa_mixer_get_volume,
+	.set_volume = alsa_mixer_set_volume,
 };

src/mixer/oss_mixer.c

@@ -137,74 +137,64 @@ oss_mixer_open(struct mixer *data)
 	return true;
 }
 
-static bool
+static int
-oss_mixer_control(struct mixer *data, int cmd, void *arg)
+oss_mixer_get_volume(struct mixer *mixer)
 {
-	struct oss_mixer *om = (struct oss_mixer *) data;
+	struct oss_mixer *om = (struct oss_mixer *)mixer;
-	switch (cmd) {
+	int left, right, level;
-	case AC_MIXER_GETVOL:
+	int ret;
-	{
-		int left, right, level;
-		int *ret;
 
-		if (om->device_fd < 0 && !oss_mixer_open(data)) {
+	if (om->device_fd < 0 && !oss_mixer_open(mixer))
-			return false;
+		return false;
-		}
 
-		if (ioctl(om->device_fd, MIXER_READ(om->volume_control), &level) < 0) {
+	ret = ioctl(om->device_fd, MIXER_READ(om->volume_control), &level);
-			oss_mixer_close(data);
+	if (ret < 0) {
-			g_warning("unable to read oss volume\n");
+		oss_mixer_close(mixer);
-			return false;
+		g_warning("unable to read oss volume\n");
-		}
+		return false;
-
-		left = level & 0xff;
-		right = (level & 0xff00) >> 8;
-
-		if (left != right) {
-			g_warning("volume for left and right is not the same, \"%i\" and "
-				"\"%i\"\n", left, right);
-		}
-		ret = (int *) arg;
-		*ret = left;
-		return true;
 	}
-	case AC_MIXER_SETVOL:
-	{
-		int new;
-		int level;
-		int *value = arg;
 
-		if (om->device_fd < 0 && !oss_mixer_open(data)) {
+	left = level & 0xff;
-			return false;
+	right = (level & 0xff00) >> 8;
-		}
 
-		new = *value;
+	if (left != right) {
-		if (new < 0) {
+		g_warning("volume for left and right is not the same, \"%i\" and "
-			new = 0;
+			  "\"%i\"\n", left, right);
-		} else if (new > 100) {
-			new = 100;
-		}
-
-		level = (new << 8) + new;
-
-		if (ioctl(om->device_fd, MIXER_WRITE(om->volume_control), &level) < 0) {
-			g_warning("unable to set oss volume\n");
-			oss_mixer_close(data);
-			return false;
-		}
-		return true;
 	}
-	default:
+
-		g_warning("Unsuported oss control\n");
+	return left;
-		break;
+}
+
+static bool
+oss_mixer_set_volume(struct mixer *mixer, unsigned volume)
+{
+	struct oss_mixer *om = (struct oss_mixer *)mixer;
+	int level;
+	int ret;
+
+	if (om->device_fd < 0 && !oss_mixer_open(mixer))
+		return false;
+
+	if (volume > 100)
+		volume = 100;
+
+	level = (volume << 8) + volume;
+
+	ret = ioctl(om->device_fd, MIXER_WRITE(om->volume_control), &level);
+	if (ret < 0) {
+		g_warning("unable to set oss volume\n");
+		oss_mixer_close(mixer);
+		return false;
 	}
-	return false;
+
+	return true;
 }
 
 const struct mixer_plugin oss_mixer = {
 	.init = oss_mixer_init,
 	.finish = oss_mixer_finish,
 	.open = oss_mixer_open,
-	.control = oss_mixer_control,
+	.close = oss_mixer_close,
-	.close = oss_mixer_close
+	.get_volume = oss_mixer_get_volume,
+	.set_volume = oss_mixer_set_volume,
 };

src/mixer_api.c

@@ -50,12 +50,6 @@ bool mixer_open(struct mixer *mixer)
 	return mixer->plugin->open(mixer);
 }
 
-bool mixer_control(struct mixer *mixer, int cmd, void *arg)
-{
-	assert(mixer != NULL && mixer->plugin != NULL);
-	return mixer->plugin->control(mixer, cmd, arg);
-}
-
 void mixer_close(struct mixer *mixer)
 {
 	assert(mixer != NULL && mixer->plugin != NULL);

src/mixer_api.h

@@ -44,15 +44,26 @@ struct mixer_plugin {
 	 */
 	bool (*open)(struct mixer *data);
 
-        /**
-	 * Control mixer device.
-         */
-	bool (*control)(struct mixer *data, int cmd, void *arg);
-
         /**
     	 * Close mixer device
 	 */
 	void (*close)(struct mixer *data);
+
+	/**
+	 * Reads the current volume.
+	 *
+	 * @return the current volume (0..100 including) or -1 on
+	 * error
+	 */
+	int (*get_volume)(struct mixer *mixer);
+
+	/**
+	 * Sets the volume.
+	 *
+	 * @param volume the new volume (0..100 including)
+	 * @return true on success
+	 */
+	bool (*set_volume)(struct mixer *mixer, unsigned volume);
 };
 
 struct mixer {
@@ -72,7 +83,18 @@ void
 mixer_free(struct mixer *mixer);
 
 bool mixer_open(struct mixer *mixer);
-bool mixer_control(struct mixer *mixer, int cmd, void *arg);
 void mixer_close(struct mixer *mixer);
 
+static inline int
+mixer_get_volume(struct mixer *mixer)
+{
+	return mixer->plugin->get_volume(mixer);
+}
+
+static inline bool
+mixer_set_volume(struct mixer *mixer, unsigned volume)
+{
+	return mixer->plugin->set_volume(mixer, volume);
+}
+
 #endif

src/output_api.h

@@ -28,13 +28,6 @@
 
 struct audio_output;
 
-enum audio_control_command {
-	AC_MIXER_GETVOL = 0,
-	AC_MIXER_SETVOL,
-};
-
-struct audio_output;
-
 const char *audio_output_get_name(const struct audio_output *ao);
 
 #endif