pbbuttons-users — Everything regarding pbbuttons' usage and develpment

Re: [PATCH] update to ALSA module

[Matthias G. <mat...@us...>]

On Sun, 23 Jan 2005 12:58:38 +1030
John Steele Scott <to...@to...> wrote:

> Matthias,
> 
> Here is the much improved version of the ALSA module. I hope it addresses all
> of your concerns.

Thanks. It worked fine now :-)

I introduced a new tag for the card called MixerCard and fixed the well known
segfault. After some more testing I will release shortly. I you like to test the
version in CVS is up-to-date.

Thanks again for your support.

  Best Regards
    Matthias

[PATCH] update to ALSA module

[John S. S. <to...@to...>]

Matthias,

Here is the much improved version of the ALSA module. I hope it addresses all
of your concerns.

cheers,

John

Index: module_alsamixer.c
===================================================================
RCS file: /cvsroot/pbbuttons/pbbuttons/pbbuttonsd/src/module_alsamixer.c,v
retrieving revision 1.3
diff -u -r1.3 module_alsamixer.c
--- module_alsamixer.c	12 Dec 2004 19:33:11 -0000	1.3
+++ module_alsamixer.c	23 Jan 2005 02:22:32 -0000
@@ -20,6 +20,7 @@
 #include <alsa/asoundlib.h>
 #include "pbbinput.h"
 
+#include <math.h>
 #include <pbb.h>
 
 #include "gettext_macros.h"
@@ -30,6 +31,7 @@
 
 struct moddata_alsamixer {
 	char *card;	 /* name of the soundcard (usually "default" is fine) */
+	char *channels;		/* the "MixerChannels" config string */
 	unsigned short keyvolup;
 	unsigned short modvolup;
 	unsigned short keyvoldn;
@@ -39,174 +41,255 @@
 	int init_complete;
 	int mixer_delayed;
 	int mute;
-	long vol_min;		/* minimum volume ALSA allows master */
-	long vol_max;		/* maximum volume ALSA allows master */
-	snd_mixer_elem_t * master; /* master sound element */
+	snd_mixer_t * mixer;
+	snd_mixer_elem_t ** elements; /* points to a NULL terminated list, first is master */
 } modbase_alsamixer;
 
+/* initialise the list of elements, based on the "channels" string */
 static int
-alsamixer_finish_init ()
+alsamixer_setup_elements_internal (char * channels, snd_mixer_selem_id_t *sid)
 {
 	struct moddata_alsamixer *base = &modbase_alsamixer;
-	snd_mixer_t *handle;
+        char *token;
+	char buffer[STDBUFFERLEN];
+	int n;
+
+	/* get rid of old list */
+	free (base->elements);
+	base->elements = NULL;
+	base->init_complete = 0;
+	n = 0;
+
+	/* this copy we save for returning in alsamixer_handle_tags */
+	strncpy (base->channels, channels, STDBUFFERLEN);
+
+	/* this copy we use for strtok */
+	strncpy (buffer, channels, STDBUFFERLEN);
+	cleanup_buffer (buffer);
+	if ((token = strtok(buffer, ",\n"))) {
+		do {
+			/* ensure we have enough memory to store pointer + terminator */
+			base->elements = realloc (base->elements, (n+2)*sizeof (snd_mixer_elem_t *));
+			if (base->elements == NULL) {
+				print_error (_("ERROR: memory allocation failed\n"));
+				return -1;
+			}
+
+			/* find element */
+			for (base->elements[n] = snd_mixer_first_elem (base->mixer);
+			     base->elements[n] != NULL;
+			     base->elements[n] = snd_mixer_elem_next (base->elements[n])) {
+				snd_mixer_selem_get_id (base->elements[n], sid);
+				if (strcasecmp (snd_mixer_selem_id_get_name (sid), token) == 0)
+					break;
+			}
+			
+			if (base->elements[n] == NULL) {
+				/* report any element we can't find */
+				print_error (_("WARNING: card \"%s\" has no \"%s\" element\n"), base->card, token);
+			} else {
+				n++;
+			}
+			
+		} while ((token = strtok(0,",\n")) != NULL);
+
+		/* terminate list */
+		base->elements[n] = NULL;
+
+		/* check that we have a master element */
+		if (!base->elements[0]) {
+			print_error (_("WARNING: MixerChannels contains no valid elements\n"));
+			return -1;
+		}
+
+		/* check that master element has a playback volume */
+		if (!snd_mixer_selem_has_playback_volume (base->elements[0])) {
+			snd_mixer_selem_get_id (base->elements[n], sid);
+			print_error (_("WARNING: mixer element \"%s\" has no playback volume control\n"), snd_mixer_selem_id_get_name (sid));
+			return -1;
+		}
+
+		base->init_complete = 1;
+		return 0;
+	} else {
+		/* no tokens? */
+		return -1;
+	}
+}
+
+/* wrapper arround alsamixer_setup_elements_internal so we can be sure to free the sid. */
+static int
+alsamixer_setup_elements (char * channels)
+{
 	snd_mixer_selem_id_t *sid;
-	int err;
+	int err, result;
 
-	/* setup mixer */
+	/* create a selem_id to extract names from */
 	if ((err = snd_mixer_selem_id_malloc (&sid))){
-		print_error (_("ERROR: failed memory allocation: %s"), snd_strerror (err));
+		print_error (_("ERROR: failed memory allocation: %s\n"), snd_strerror (err));
 		return -1;
 	}
-	if ((err = snd_mixer_open (&handle, 0)) < 0) {
-		print_error (_("ERROR: mixer open %s: %s"), base->card, snd_strerror (err));
+
+	result = alsamixer_setup_elements_internal (channels, sid);
+	snd_mixer_selem_id_free (sid);
+	return result;
+}
+
+static int
+alsamixer_finish_init ()
+{
+	struct moddata_alsamixer *base = &modbase_alsamixer;
+	int err;
+
+	/* setup mixer */
+	if ((err = snd_mixer_open (&base->mixer, 0)) < 0) {
+		print_error (_("ERROR: mixer open %s: %s\n"), base->card, snd_strerror (err));
 		return -1;
 	}
-	if ((err = snd_mixer_attach (handle, base->card)) < 0) {
-		print_error (_("ERROR: mixer attach %s: %s"), base->card, snd_strerror (err));
-		snd_mixer_close (handle);
+	if ((err = snd_mixer_attach (base->mixer, base->card)) < 0) {
+		print_error (_("ERROR: mixer attach %s: %s\n"), base->card, snd_strerror (err));
+		snd_mixer_free (base->mixer);
 		return -1;
 	}
-	if ((err = snd_mixer_selem_register (handle, NULL, NULL)) < 0) {
-		print_error (_("ERROR: mixer register: %s"), snd_strerror (err));
-		snd_mixer_close (handle);
+	if ((err = snd_mixer_selem_register (base->mixer, NULL, NULL)) < 0) {
+		print_error (_("ERROR: mixer register: %s\n"), snd_strerror (err));
+		snd_mixer_free (base->mixer);
 		return -1;
 	}
 	
-	if ((err = snd_mixer_load (handle)) < 0) {
-		print_error (_("ERROR: load %s: %s"), base->card, snd_strerror (err));
-		snd_mixer_close (handle);
+	if ((err = snd_mixer_load (base->mixer)) < 0) {
+		print_error (_("ERROR: load %s: %s\n"), base->card, snd_strerror (err));
+		snd_mixer_free (base->mixer);
 		return -1;
 	}
 
-	/* find "Master" element */
-	for (base->master = snd_mixer_first_elem (handle); base->master; base->master = snd_mixer_elem_next (base->master)) {
-		snd_mixer_selem_get_id (base->master, sid);
-		if(strcmp (snd_mixer_selem_id_get_name (sid), "Master") == 0) {
-			snd_mixer_selem_get_playback_volume_range (base->master, &base->vol_min, &base->vol_max);
-			base->init_complete = 1;
-			return 0;
-		}
-	}
-
-	/* couldn't find element */
-	print_error (_("ERROR: %s has no \"Master\" element"), base->card);
-	return -1;
+	return alsamixer_setup_elements (base->channels);
 }
 
 /* Converts an ALSA volume to a number in the range [0, +/-100]. This function
  * assumes that the mixer has already been setup successfully.
  */
 static inline int
-alsamixer_normalise_volume (long volume)
+alsamixer_vol_to_percentage (snd_mixer_elem_t * elem, long volume)
 {
-	long range;
-	struct moddata_alsamixer *base = &modbase_alsamixer;
+	long range, vol_min, vol_max;
+	int pct;
 
-	range = base->vol_max - base->vol_min;
-	return volume*100/range;
+	snd_mixer_selem_get_playback_volume_range (elem, &vol_min, &vol_max);
+	range = vol_max - vol_min;
+	pct = rint(volume*100.0/range);
+	return pct;
 }
 
 /* Converts a number in the range [0, +/-100] to an ALSA volume. This function
  * assumes that the mixer has already been setup successfully.
  */
-static inline long
-alsamixer_denormalise_volume (int volume)
+static long
+alsamixer_percentage_to_vol (snd_mixer_elem_t * elem, int volume)
 {
-	long range;
-	struct moddata_alsamixer *base = &modbase_alsamixer;
+	long range, vol_min, vol_max;
 	
-	range = base->vol_max - base->vol_min;
-	return volume*range/100;
+	snd_mixer_selem_get_playback_volume_range (elem, &vol_min, &vol_max);
+	range = vol_max - vol_min;
+	return rint(volume*range/100.0);
 }
 
-/* Returns the average normalised volume across all channels of an element, or -1. */
+/* Saves the current mute status in base->mute, and returns the volume of the first playback channel of the master element */
 static int
 alsamixer_get_volume ()
 {
-	snd_mixer_selem_channel_id_t chn;
-	long vol, vol_sum;
-	int num_chn;
+	long volume;
 	struct moddata_alsamixer *base = &modbase_alsamixer;
-	snd_mixer_elem_t * elem = base->master;
+	snd_mixer_elem_t * elem = base->elements[0];
 
-	vol_sum = num_chn = 0;
-	
 	if (base->init_complete == 0) /* is mixer setup already completed? */
 		if ((alsamixer_finish_init ()) != 0) /* no, then do it now */
 			return -1; /* Oops, try it again later */
-				   
-	if (snd_mixer_selem_has_playback_volume (elem)) {
-		/* cycle through each playback channel of element */
-		for (chn = 0; chn <= SND_MIXER_SCHN_LAST; chn++) {
-			if (!snd_mixer_selem_has_playback_channel (elem, chn))
-				continue;
-			
-			/* visit the channel */
-			snd_mixer_selem_get_playback_volume (elem, chn, &vol);
-			vol_sum += vol;
-			num_chn++;
-		}
-	}
 
-	return alsamixer_normalise_volume (vol_sum/num_chn);
+	/* force simple mixer to update its values from audio driver*/
+	snd_mixer_handle_events (base->mixer);
+
+	/* read values */
+	snd_mixer_selem_get_playback_switch (elem, 0, &base->mute);
+	base->mute = !base->mute;
+	snd_mixer_selem_get_playback_volume (elem, 0, &volume);
+	return volume;
+}
+
+/* same as alsamixer_get_volume, but returns the volume as number between 0 and 100 */
+static int 
+alsamixer_get_volume_as_percentage ()
+{
+	int volume;
+	struct moddata_alsamixer *base = &modbase_alsamixer;
+
+	volume = alsamixer_get_volume ();
+	
+	if (volume < 0)
+		return volume;
+	else
+		return alsamixer_vol_to_percentage (base->elements[0], volume);
 }
 
 static void
 alsamixer_set_and_send (int increment)
 {
-	snd_mixer_selem_channel_id_t chn;
-	long vol,  vol_sum, denormal_increment;
-	int num_chn;
+	long vol, vol_min, vol_max;
+	long master_percentage, master_vol, master_vol_max, master_vol_min;
+        snd_mixer_elem_t ** elem_ptr;
+	snd_mixer_elem_t * elem;
 	struct moddata_alsamixer *base = &modbase_alsamixer;
-	snd_mixer_elem_t * elem = base->master;
 
-	vol_sum = 0;
-	num_chn = 0;
-	denormal_increment = alsamixer_denormalise_volume (increment);
-	
 	if (base->init_complete == 0) /* is mixer setup already completed? */
 		if ((alsamixer_finish_init ()) != 0) /* no, then do it now */
 			return;	/* Oops, try it again later */
-				   
-	if (snd_mixer_selem_has_playback_volume (elem)) {
-		/* cycle through each playback channel of element */
-		for (chn = 0; chn <= SND_MIXER_SCHN_LAST; chn++) {
-			if (!snd_mixer_selem_has_playback_channel (elem, chn))
-				continue;
-			
-			/* visit the channel */
-			if (increment == 0) {
-				/* toggle mute status (pbs is opposite of mute) */
-				snd_mixer_selem_get_playback_switch (elem, chn, &base->mute);
-				snd_mixer_selem_set_playback_switch (elem, chn, !base->mute);
-				/* sum volume */
-				snd_mixer_selem_get_playback_volume (elem, chn, &vol);
-				vol_sum += vol;
-				num_chn++;
-			} else {
-				/* increment/decrement volume */
-				snd_mixer_selem_get_playback_volume (elem, chn, &vol);
-				
-				vol += denormal_increment;
-				if (vol > base->vol_max)
-					vol = base->vol_max;
-				else if (vol < base->vol_min)
-					vol = base->vol_min;
-				snd_mixer_selem_set_playback_volume (elem, chn, vol);
-
-				/* sum volume */
-				vol_sum += vol;
-				num_chn++;
-			}
-		}
-	}
 
-	if (base->mute) {
+
+        /* get initial values from master element */
+	elem_ptr = base->elements;
+	elem = *elem_ptr;
+	snd_mixer_selem_get_playback_volume_range (elem, &master_vol_min, &master_vol_max);
+	master_vol = alsamixer_get_volume ();
+	master_vol = master_vol + increment;
+	if (master_vol > master_vol_max)
+		master_vol = master_vol_max;
+	else if (master_vol < master_vol_min)
+		master_vol = master_vol_min;
+	master_percentage = alsamixer_vol_to_percentage (elem, master_vol);			
+	
+	if (increment == 0)
+		/* toggle mute status */
+		base->mute = !base->mute;
+	else
+		/* volume change releases the mute */
+		base->mute = 0;
+
+	/* cycle through elements */
+	for (elem_ptr = base->elements; *elem_ptr != NULL; elem_ptr++) {
+		elem = *elem_ptr;
+
+		/* sync playback switch */
+		snd_mixer_selem_set_playback_switch_all (elem, !base->mute);
+		
+		/* sync volume */
+		if (snd_mixer_selem_has_playback_volume (elem)) {
+			
+			/* compare volume limits with master */
+			snd_mixer_selem_get_playback_volume_range (elem, &vol_min, &vol_max);
+			if ((vol_min != master_vol_min) || (vol_max != master_vol_max))
+				vol = alsamixer_percentage_to_vol (elem, master_percentage);
+			else
+				vol = master_vol;
+		
+			/* sync volume with master element */
+			snd_mixer_selem_set_playback_volume_all (elem, vol);
+                }
+        }
+                
+	if (base->mute)
 		singletag_to_clients (CHANGEVALUE, TAG_MUTE, 0);
-	} else {
-		vol = alsamixer_normalise_volume (vol_sum/num_chn);
-		singletag_to_clients (CHANGEVALUE, TAG_VOLUME, vol);
-	}
+	else
+		singletag_to_clients (CHANGEVALUE, TAG_VOLUME, master_percentage);
 }
 
 static int
@@ -220,25 +303,30 @@
 		switch (taglist->tag) {
 		case TAG_VOLUME:
 			if (cfgure) {
-				volume_delta = taglist->data - alsamixer_get_volume ();
+				volume_delta = taglist->data - alsamixer_get_volume_as_percentage ();
 				if (volume_delta != 0)
 					alsamixer_set_and_send (volume_delta);
 			} else
-				taglist->data = alsamixer_get_volume ();
+				taglist->data = alsamixer_get_volume_as_percentage ();
 			break;
 		case TAG_MUTE:
 			if (cfgure) {
 				if (base->mute != taglist->data)
 					alsamixer_set_and_send (0);
-			} else
+			} else {
+				/* update mute status */
+				alsamixer_get_volume ();
 				taglist->data = base->mute;
+			}
 			break;
 		case TAG_MIXERDEVICE:
 			if (cfgure) {
 				char * cardname = (char *) taglist->data;
-				if (strlen (cardname) < STDBUFFERLEN)
+				if (strlen (cardname) < STDBUFFERLEN) {
 					strncpy (base->card, cardname, STDBUFFERLEN);
-				else
+					alsamixer_exit ();
+					alsamixer_finish_init ();
+				} else
 					rc = tagerror (taglist, E_BUFOVL);
 			} else
 				taglist->data = (long) base->card;
@@ -271,6 +359,10 @@
 			if (cfgure)	base->mixer_delayed  = taglist->data;
 			else		taglist->data = base->mixer_delayed;
 			break;
+		case TAG_MIXERCHANNELS:
+			if (cfgure)	alsamixer_setup_elements ((char *) taglist->data);
+			else		taglist->data = (long) base->channels;
+			break;
 		case TAG_SOUNDSYSTEM:
 			if (cfgure)	tagerror (taglist, E_NOWRITE);
 			else 		taglist->data = SOUNDSYSTEM_ALSA;
@@ -286,10 +378,14 @@
 {
 	struct moddata_alsamixer *base = &modbase_alsamixer;
 	static char devbuffer_mixer[STDBUFFERLEN];
+	static char mixer_channels[STDBUFFERLEN];
 	char *cardname;
+	char *channels;
 	int rc;
 
 	base->card		= devbuffer_mixer;
+	base->channels		= mixer_channels;
+	base->elements          = NULL;
 	base->keyvolup		= KEY_VOLUMEUP;
 	base->modvolup		= MOD_NONE;
 	base->keyvoldn		= KEY_VOLUMEDOWN;
@@ -310,6 +406,12 @@
 	else
 		return  E_BUFOVL;
 
+	channels = (char *) tagfind (taglist, TAG_MIXERCHANNELS, (long) "Master");
+	if (strlen (channels) < STDBUFFERLEN)
+		strncpy (base->channels, channels, STDBUFFERLEN);
+	else
+		return  E_BUFOVL;
+
 	if ((base->mixer_delayed = tagfind (taglist, TAG_MIXERINITDELAY, 0)) == 0) {
 		alsamixer_finish_init ();
 	} else {
@@ -330,6 +432,11 @@
 int
 alsamixer_exit ()
 {
+	struct moddata_alsamixer *base = &modbase_alsamixer;
+	free (base->elements);
+	base->elements = NULL;
+	snd_mixer_free (base->mixer);
+	base->init_complete = 0;
 	return 0;
 }
 

Re: suspend and resume

[<hu...@so...>]

On Sat, Jan 08, 2005 at 02:07:54PM +0100, Rein van den Boomgaard wrote:
> LS
> 
> I've installed Mandrake 10.1 on my iBook Dual USB. Works nicely!
> 
> I downloaded and installed pbbuttonsd en powerprefs and both seem to work (i
> can dim the lcd and adjust the volume). What i really need is the suspend mode
> and unfortunately that doesn't work.
> 
> Pressing the powerbutton seems to suspend the ibook but pressing it again
> shows a message complaining that it couldnt set the cpufreq.
> 
> It also prints something about unbalenced somethings... (i have to write it
> down if someone needs this to diagnose the problem).
> 
> Any ideas of what i'm doing wrong??
> 

try this one
http://honk.physik.uni-konstanz.de/~agx/linux-ppc/kernel/

-- 
Hu Gang       .-.
              /v\
             // \\ 
Linux User  /(   )\  [204016]
GPG Key ID   ^^-^^   http://soulinfo.com/~hugang/hugang.asc

suspend and resume

[Rein v. d. B. <re...@sc...>]

LS

I've installed Mandrake 10.1 on my iBook Dual USB. Works nicely!

I downloaded and installed pbbuttonsd en powerprefs and both seem to work (i
can dim the lcd and adjust the volume). What i really need is the suspend mode
and unfortunately that doesn't work.

Pressing the powerbutton seems to suspend the ibook but pressing it again
shows a message complaining that it couldnt set the cpufreq.

It also prints something about unbalenced somethings... (i have to write it
down if someone needs this to diagnose the problem).

Any ideas of what i'm doing wrong??

Regards
rein


Rein van den Boomgaard
Intelligent Sensory Information Systemse
Universiteit van Amsterdam
http://www.science.uva.nl/~rein/

Re: Update to ALSA module patch

[Matthias G. <mat...@us...>]

On Mon, 03 Jan 2005 10:32:13 +1030
John Steele Scott <to...@to...> wrote:

> > 1. A configuration error in dev_Mixer causes a segmentation fault. To fix this
> >    you need to check alsamixer_finish_init() in alsamixer_init() for errors and
> >    return with an error code if there was one.
> 
> I couldn't reproduce the segfault, but maybe my ALSA has debugging turned on, I
> don't know. What value of dev_Mixer were you using to break it?

I looked into the code again and the Segfault is still there but I wasn't right with
the cause. The implementation of alsamixer_finish_init() is ok and you don't need
to check for errors here in alsamixer_init(). The error mechanism is quite tricky.

If an error occurs and alsamixer_finish_init() fails the mixer module continues
normally and will try to initialize the mixer again later. So far so good. The next
function that will be called is alsamixer_handle_tags(). In this function all 
configuration options will be evaluated and the first might be 'TAG_VOLUME' (from
the option 'Volume'). This leads to a call of alsamixer_get_volume() and in this
function the line
	snd_mixer_elem_t * elem = base->elements[0];
will cause an Segfault, because base->elements = NULL so that you try to get
 NULL[0] -> Segfault.

To get this error use dev_Mixer = "/dev/mixer" in pbbuttonsd.conf for example.
I tried some existing or non existing names and both crashed.

The same could happen, if you set the option MixerInitDelay=yes. But in this
case the tags TAG_VOLUME and TAG_MUTE are removed from the configuration
list and the critical code won't be reached.

I think this bug could be fixed quite easily.

> Should alsamixer_init() return immediately when finish_init() fails, or should
> it run handle_tags() and register_function() first, then return with the error
> code at the end.

No. This was my fault. It should not return with error at all. A missing or faulty 
mixer device should not break pbbuttonsd. Those errors should be reported and
nothing else. 

> > 2. If I press the volume up key nothing happens. The volume level doesn't change.
> >     The problem here is the 'normalising' and 'denormalising' of volume levels.
> >     My sound hardware allow volume level from 0..15. That means that only volume
> >     steps of 8% or more have an effect. If you increase the volume by 1% this
> >     change will be 'denormalized' to 0% and nothing changed.
> >
> >     The aim should be that one trigger of the volume up key will increase the volume
> >     at least by one step (8% in my case, ALSAs max volume seems to be 128,
> >     16*8=128). I had no chance to do that with OSS but ALSA gives us all the
> >     information we need so we should do it right.
> >
> >     The 'shift' modifier should change volume in larger steps but this is only usefull
> >     with large volume ranges. With small ranges 'shift-volume-up' might jump near
> >     the end of scale but this doesn't matter.
> 
> I don't entirely follow this . . . if you run the "amixer" command line program,
> what does it say for "Limits" for the element you are talking about? 0 - 15? or
> 0 - 128?

0-15

> I suspect that for your card it would say 0-15. Using the very first patch I
> sent, the volume could be moved one step at a time. The problem was that the
> client programs did not reflect this value particularly well, because they
> expect a percentage. This was why I added the normalise/denormalise code. This
> was to mimic the behaviour of the OSS module, which changed the volume by +/-1%,
> and had a value in the range 0-100. If we are to follow the ALSA route, and use
> the "natural" range of the hardware, the client programs will need to take that
> into account somehow.

ALSA is a much better sound system as OSS and the alsamixer module should not
mimic the behaviour of OSS. OSS uses always 0-100 for volume and there was no
possibility to get the real supported volume range. ALSA is better here and the
module should use its features to comfort the user.

The module should ask ALSA for the real volume ranges of the master element
and uses this values for all volume calculations.

The clients uses 0-100% Volume because this is an system and hardware
independent range. If you send a message to clients you only have to calculate
your absolute volume level into an relative level in percent and that's it.
    percent = absolute*100/max_volume;

> On my iBook G4, the range of most elements is from 0-176, except there are a
> couple of exceptions. DRC is from 0-239 and Beep is from 0-100. If a user wants
> to change the volume on two channels with different ranges, what is the correct
> behaviour?

What do you think? What would you expect the program should behave? Imagine
you are the user (and you probably are a user) and have a look from his point of
view.

I would expect that all elements change there volume relativ to each other.
That means that a element with a larger range change its volume in larger steps
than an element with a smaller volume range. All channels should reach zero or
there maximum value at the same time. I would define the 'Master' as leader
and all other elements follow the leader.

> I could not find any place in the ALSA documentation where they discuss what the
> limits on the range are. So theoretically the range could include negative
> values, and it could also be very large.

Because you ask ALSA for the values this should not a problem.

> I think this is an artifact of trying to figure out how to handle different
> ranges for different channels/elements. Yes, it will simplify things if all
> channels are synchonised to the first one.

I think this is the best sollution. Most Powerbooks have only two keys for
volume control so we can't realize a full mixer control. There are enough
applications out there to do that.

> > 5. If there were no channels specified in the configuration file you would return
> >     an error without error message. The OSSMixer ignores unknown or empty
> >     channels. So you have the choice: Ignore empty channel string or return an
> >     error _with_ error message.
> 
> Now that I think about it, I would prefer to print a warning for unknown
> channels, but not return an error. As for the case where there are _no_ channels at
> all . . . I will think about that.

I would prefer to print a warning and (temporary) disable the mixer functionallity.
As I already said: A bad or missing mxer device should not break pbbuttonsd completly.

 Best Regards
   Matthias

Re: Update to ALSA module patch

[John S. S. <to...@to...>]

Matthias Grimm <mat...@ht...> writes:
> Thanks for your code again. I applied your latest patch and changing multiple
> channels/elements works now. Nevertheless there are some issues that make
> problems on my machine:

I would like to clarify some of these points before I get back to hacking on the
code. I knew nothing about ALSA before I started this, but I didn't like having
to load OSS emulation (couldn't get it to work with dmix properly). Yes, I would
like to get it working as well as possible before release.

> 1. A configuration error in dev_Mixer causes a segmentation fault. To fix this
>    you need to check alsamixer_finish_init() in alsamixer_init() for errors and
>    return with an error code if there was one.

I couldn't reproduce the segfault, but maybe my ALSA has debugging turned on, I
don't know. What value of dev_Mixer were you using to break it?

Should alsamixer_init() return immediately when finish_init() fails, or should
it run handle_tags() and register_function() first, then return with the error
code at the end.

I note that the OSS module does not return the value of ossmixer_finish_init().

> 2. If I press the volume up key nothing happens. The volume level doesn't change.
>     The problem here is the 'normalising' and 'denormalising' of volume levels.
>     My sound hardware allow volume level from 0..15. That means that only volume
>     steps of 8% or more have an effect. If you increase the volume by 1% this
>     change will be 'denormalized' to 0% and nothing changed.
>
>     The aim should be that one trigger of the volume up key will increase the volume
>     at least by one step (8% in my case, ALSAs max volume seems to be 128,
>     16*8=128). I had no chance to do that with OSS but ALSA gives us all the
>     information we need so we should do it right.
>
>     The 'shift' modifier should change volume in larger steps but this is only usefull
>     with large volume ranges. With small ranges 'shift-volume-up' might jump near
>     the end of scale but this doesn't matter.

I don't entirely follow this . . . if you run the "amixer" command line program,
what does it say for "Limits" for the element you are talking about? 0 - 15? or
0 - 128?

I suspect that for your card it would say 0-15. Using the very first patch I
sent, the volume could be moved one step at a time. The problem was that the
client programs did not reflect this value particularly well, because they
expect a percentage. This was why I added the normalise/denormalise code. This
was to mimic the behaviour of the OSS module, which changed the volume by +/-1%,
and had a value in the range 0-100. If we are to follow the ALSA route, and use
the "natural" range of the hardware, the client programs will need to take that
into account somehow.

On my iBook G4, the range of most elements is from 0-176, except there are a
couple of exceptions. DRC is from 0-239 and Beep is from 0-100. If a user wants
to change the volume on two channels with different ranges, what is the correct
behaviour?

I could not find any place in the ALSA documentation where they discuss what the
limits on the range are. So theoretically the range could include negative
values, and it could also be very large.

> 3. In your functions 'normalise' and 'denormalise' you calculate the volume range
>     like this: 
>        range = max - min;
>     I think the correct formula should be:
>        range = max - min + 1;
>
>     min = 0, max = 15 -> 16 volume levels - > max - min + 1 = 15 - 0 + 1 = 16

Ok.

> 4. You always sum up the volume levels of each channel of an element and divide
>      this through the channel count. Why do you do this? I would read the volume
>      level of the first channel and equalize all others accordingly.
>
>      A side effect of this summing and dividing is an calculation error due to
>      integer arithmetics that leads to different volume levels for different elements.
>      You could see this if you run alsamixer and increase volume at multiple
>      channels. At certain steps the levels are not equal as they always should be.

I think this is an artifact of trying to figure out how to handle different
ranges for different channels/elements. Yes, it will simplify things if all
channels are synchonised to the first one.

> 5. If there were no channels specified in the configuration file you would return
>     an error without error message. The OSSMixer ignores unknown or empty
>     channels. So you have the choice: Ignore empty channel string or return an
>     error _with_ error message.

Now that I think about it, I would prefer to print a warning for unknown
channels, but not return an error. As for the case where there are _no_ channels at
all . . . I will think about that.

> 6. Mute doesn't work correctly on my machine. You invert the mute status on
>     each channel independently. If this states are different (one channel muted,
>     the other not) the should could never muted with the hotkey. The second
>     problem: Only the mute status of the last channel of the last element would
>     be reported to the clients. This might be the wrong status.
>     Incresing or decreasing the volume must automazically unmute the speakers.
>     I can't test this because of the general problems with mute.

Okay, I will change it so that we toggle the mute status of the first channel,
and then set the other channels to have the same status. On my machine only the
master channel has a playback switch.

cheers,

John

Re: Update to ALSA module patch

[John S. S. <to...@to...>]

Sorry, there was a bug in the last version where the volume wasn't being
properly communicated to the clients when the sound was unmuted. This patch
should be used instead.

cheers,

John

diff -u -r1.3 module_alsamixer.c
--- module_alsamixer.c	12 Dec 2004 19:33:11 -0000	1.3
+++ module_alsamixer.c	2 Jan 2005 00:26:12 -0000
@@ -30,6 +30,7 @@
 
 struct moddata_alsamixer {
 	char *card;	 /* name of the soundcard (usually "default" is fine) */
+	char *channels;		/* the "MixerChannels" config string */
 	unsigned short keyvolup;
 	unsigned short modvolup;
 	unsigned short keyvoldn;
@@ -39,70 +40,140 @@
 	int init_complete;
 	int mixer_delayed;
 	int mute;
-	long vol_min;		/* minimum volume ALSA allows master */
-	long vol_max;		/* maximum volume ALSA allows master */
-	snd_mixer_elem_t * master; /* master sound element */
+	snd_mixer_t * mixer;
+	snd_mixer_elem_t ** elements; /* points to a NULL terminated list, first is master */
 } modbase_alsamixer;
 
+/* initialise the list of elements, based on the "channels" string */
 static int
-alsamixer_finish_init ()
+alsamixer_setup_elements_internal (char * channels, snd_mixer_selem_id_t *sid)
 {
 	struct moddata_alsamixer *base = &modbase_alsamixer;
-	snd_mixer_t *handle;
+        char *token;
+	char buffer[STDBUFFERLEN];
+	int n;
+
+	/* get rid of old list */
+	free (base->elements);
+	base->elements = NULL;
+	n = 0;
+
+	/* this copy we save for returning in alsamixer_handle_tags */
+	strncpy (base->channels, channels, STDBUFFERLEN);
+
+	/* this copy we use for strtok */
+	strncpy (buffer, channels, STDBUFFERLEN);
+	cleanup_buffer (buffer);
+	if ((token = strtok(buffer, ",\n"))) {
+		do {
+			/* ensure we have enough memory to store pointer + terminator */
+			base->elements = realloc (base->elements, (n+2)*sizeof (snd_mixer_elem_t *));
+			if (base->elements == NULL) {
+				print_error (_("ERROR: memory allocation failed\n"));
+				return -1;
+			}
+
+			/* find element */
+			for (base->elements[n] = snd_mixer_first_elem (base->mixer);
+			     base->elements[n];
+			     base->elements[n] = snd_mixer_elem_next (base->elements[n])) {
+				snd_mixer_selem_get_id (base->elements[n], sid);
+				if (strcasecmp (snd_mixer_selem_id_get_name (sid), token) == 0)
+					break;
+			}
+			
+			/* abort at first element we can't find */
+			if (base->elements[n] == NULL) {
+				print_error (_("ERROR: %s has no \"%s\" element\n"), base->card, token);
+				return -1;
+			} else {
+				n++;
+			}
+			
+		} while ((token = strtok(0,",\n")) != NULL);
+
+		/* terminate list */
+		base->elements[n] = NULL;
+
+		/* check that master has a playback volume */
+		if (!snd_mixer_selem_has_playback_volume (base->elements[0])) {
+			snd_mixer_selem_get_id (base->elements[n], sid);
+			print_error (_("ERROR: mixer element \"%s\" has no playback volume control\n"), snd_mixer_selem_id_get_name (sid));
+			return -1;
+		}
+
+		return 0;
+	} else {
+		/* no tokens? */
+		return -1;
+	}
+}
+
+/* wrapper arround alsamixer_setup_elements_internal so we can be sure to free the sid. */
+static int
+alsamixer_setup_elements (char * channels)
+{
 	snd_mixer_selem_id_t *sid;
-	int err;
+	int err, result;
 
-	/* setup mixer */
+	/* create a selem_id to extract names from */
 	if ((err = snd_mixer_selem_id_malloc (&sid))){
-		print_error (_("ERROR: failed memory allocation: %s"), snd_strerror (err));
+		print_error (_("ERROR: failed memory allocation: %s\n"), snd_strerror (err));
 		return -1;
 	}
-	if ((err = snd_mixer_open (&handle, 0)) < 0) {
-		print_error (_("ERROR: mixer open %s: %s"), base->card, snd_strerror (err));
+
+	result = alsamixer_setup_elements_internal (channels, sid);
+	snd_mixer_selem_id_free (sid);
+	return result;
+}
+
+static int
+alsamixer_finish_init ()
+{
+	struct moddata_alsamixer *base = &modbase_alsamixer;
+	int err;
+
+	/* setup mixer */
+	if ((err = snd_mixer_open (&base->mixer, 0)) < 0) {
+		print_error (_("ERROR: mixer open %s: %s\n"), base->card, snd_strerror (err));
 		return -1;
 	}
-	if ((err = snd_mixer_attach (handle, base->card)) < 0) {
-		print_error (_("ERROR: mixer attach %s: %s"), base->card, snd_strerror (err));
-		snd_mixer_close (handle);
+	if ((err = snd_mixer_attach (base->mixer, base->card)) < 0) {
+		print_error (_("ERROR: mixer attach %s: %s\n"), base->card, snd_strerror (err));
+		snd_mixer_free (base->mixer);
 		return -1;
 	}
-	if ((err = snd_mixer_selem_register (handle, NULL, NULL)) < 0) {
-		print_error (_("ERROR: mixer register: %s"), snd_strerror (err));
-		snd_mixer_close (handle);
+	if ((err = snd_mixer_selem_register (base->mixer, NULL, NULL)) < 0) {
+		print_error (_("ERROR: mixer register: %s\n"), snd_strerror (err));
+		snd_mixer_free (base->mixer);
 		return -1;
 	}
 	
-	if ((err = snd_mixer_load (handle)) < 0) {
-		print_error (_("ERROR: load %s: %s"), base->card, snd_strerror (err));
-		snd_mixer_close (handle);
+	if ((err = snd_mixer_load (base->mixer)) < 0) {
+		print_error (_("ERROR: load %s: %s\n"), base->card, snd_strerror (err));
+		snd_mixer_free (base->mixer);
 		return -1;
 	}
 
-	/* find "Master" element */
-	for (base->master = snd_mixer_first_elem (handle); base->master; base->master = snd_mixer_elem_next (base->master)) {
-		snd_mixer_selem_get_id (base->master, sid);
-		if(strcmp (snd_mixer_selem_id_get_name (sid), "Master") == 0) {
-			snd_mixer_selem_get_playback_volume_range (base->master, &base->vol_min, &base->vol_max);
-			base->init_complete = 1;
-			return 0;
-		}
+	if ((err = alsamixer_setup_elements (base->channels)) < 0) {
+		snd_mixer_free (base->mixer);
+		return -1;
+	} else {
+		base->init_complete = 1;
+		return 0;
 	}
-
-	/* couldn't find element */
-	print_error (_("ERROR: %s has no \"Master\" element"), base->card);
-	return -1;
 }
 
 /* Converts an ALSA volume to a number in the range [0, +/-100]. This function
  * assumes that the mixer has already been setup successfully.
  */
 static inline int
-alsamixer_normalise_volume (long volume)
+alsamixer_normalise_volume (snd_mixer_elem_t * elem, long volume)
 {
-	long range;
-	struct moddata_alsamixer *base = &modbase_alsamixer;
+	long range, vol_min, vol_max;
 
-	range = base->vol_max - base->vol_min;
+	snd_mixer_selem_get_playback_volume_range (elem, &vol_min, &vol_max);
+	range = vol_max - vol_min;
 	return volume*100/range;
 }
 
@@ -110,16 +181,16 @@
  * assumes that the mixer has already been setup successfully.
  */
 static inline long
-alsamixer_denormalise_volume (int volume)
+alsamixer_denormalise_volume (snd_mixer_elem_t * elem, int volume)
 {
-	long range;
-	struct moddata_alsamixer *base = &modbase_alsamixer;
+	long range, vol_min, vol_max;
 	
-	range = base->vol_max - base->vol_min;
+	snd_mixer_selem_get_playback_volume_range (elem, &vol_min, &vol_max);
+	range = vol_max - vol_min;
 	return volume*range/100;
 }
 
-/* Returns the average normalised volume across all channels of an element, or -1. */
+/* Returns the average normalised volume across all channels of the master element, or -1. */
 static int
 alsamixer_get_volume ()
 {
@@ -127,7 +198,7 @@
 	long vol, vol_sum;
 	int num_chn;
 	struct moddata_alsamixer *base = &modbase_alsamixer;
-	snd_mixer_elem_t * elem = base->master;
+	snd_mixer_elem_t * elem = base->elements[0];
 
 	vol_sum = num_chn = 0;
 	
@@ -148,65 +219,84 @@
 		}
 	}
 
-	return alsamixer_normalise_volume (vol_sum/num_chn);
+	return alsamixer_normalise_volume (elem, vol_sum/num_chn);
 }
 
 static void
 alsamixer_set_and_send (int increment)
 {
 	snd_mixer_selem_channel_id_t chn;
-	long vol,  vol_sum, denormal_increment;
-	int num_chn;
+	long vol,  vol_sum, denormal_increment, vol_min, vol_max, normalised_master_vol;
+	int num_master_channels;
+        snd_mixer_elem_t ** elem_ptr;
+	snd_mixer_elem_t * elem;
 	struct moddata_alsamixer *base = &modbase_alsamixer;
-	snd_mixer_elem_t * elem = base->master;
 
 	vol_sum = 0;
-	num_chn = 0;
-	denormal_increment = alsamixer_denormalise_volume (increment);
+	num_master_channels = 0;
+        normalised_master_vol = -1;
 	
 	if (base->init_complete == 0) /* is mixer setup already completed? */
 		if ((alsamixer_finish_init ()) != 0) /* no, then do it now */
 			return;	/* Oops, try it again later */
-				   
-	if (snd_mixer_selem_has_playback_volume (elem)) {
-		/* cycle through each playback channel of element */
-		for (chn = 0; chn <= SND_MIXER_SCHN_LAST; chn++) {
-			if (!snd_mixer_selem_has_playback_channel (elem, chn))
-				continue;
+	
+	/* cycle through each element */
+	for (elem_ptr = base->elements; *elem_ptr != NULL; elem_ptr++) {
+		elem = *elem_ptr;
+		
+		if (snd_mixer_selem_has_playback_volume (elem)) {
+
+			/* get range so we can saturate if changing volume */
+			snd_mixer_selem_get_playback_volume_range (elem, &vol_min, &vol_max);
+
+			/* cycle through each playback channel of element */
+			for (chn = 0; chn <= SND_MIXER_SCHN_LAST; chn++) {
+				if (!snd_mixer_selem_has_playback_channel (elem, chn))
+					continue;
 			
-			/* visit the channel */
-			if (increment == 0) {
-				/* toggle mute status (pbs is opposite of mute) */
-				snd_mixer_selem_get_playback_switch (elem, chn, &base->mute);
-				snd_mixer_selem_set_playback_switch (elem, chn, !base->mute);
-				/* sum volume */
-				snd_mixer_selem_get_playback_volume (elem, chn, &vol);
-				vol_sum += vol;
-				num_chn++;
-			} else {
-				/* increment/decrement volume */
-				snd_mixer_selem_get_playback_volume (elem, chn, &vol);
+				/* visit the channel */
+				if (increment == 0) {
+                                  /* toggle mute status (pbs is opposite of mute) */
+                                  snd_mixer_selem_get_playback_switch (elem, chn, &base->mute);
+                                  snd_mixer_selem_set_playback_switch (elem, chn, !base->mute);
+
+                                  /* sum volume for report to clients */
+                                  snd_mixer_selem_get_playback_volume (elem, chn, &vol);
+                                  vol_sum += alsamixer_normalise_volume (elem, vol);
+                                  num_master_channels++;
+				} else if (normalised_master_vol != -1) {
+					/* sync slave element with master element */
+					vol = alsamixer_denormalise_volume (elem, normalised_master_vol);
+					snd_mixer_selem_set_playback_volume (elem, chn, vol);
+				} else {
+					/* increment/decrement volume for master element */
+					snd_mixer_selem_get_playback_volume (elem, chn, &vol);
 				
-				vol += denormal_increment;
-				if (vol > base->vol_max)
-					vol = base->vol_max;
-				else if (vol < base->vol_min)
-					vol = base->vol_min;
-				snd_mixer_selem_set_playback_volume (elem, chn, vol);
-
-				/* sum volume */
-				vol_sum += vol;
-				num_chn++;
+					denormal_increment = alsamixer_denormalise_volume (elem, increment);
+					vol += denormal_increment;
+					if (vol > vol_max)
+						vol = vol_max;
+					else if (vol < vol_min)
+						vol = vol_min;
+					snd_mixer_selem_set_playback_volume (elem, chn, vol);
+
+					/* sum volume for report to clients */
+					vol_sum += alsamixer_normalise_volume (elem, vol);
+					num_master_channels++;
+				}
+			}
+			
+			if (normalised_master_vol == -1) {
+				/* compute average master volume */
+				normalised_master_vol = vol_sum/num_master_channels;
 			}
 		}
 	}
 
-	if (base->mute) {
+	if (base->mute)
 		singletag_to_clients (CHANGEVALUE, TAG_MUTE, 0);
-	} else {
-		vol = alsamixer_normalise_volume (vol_sum/num_chn);
-		singletag_to_clients (CHANGEVALUE, TAG_VOLUME, vol);
-	}
+	else
+		singletag_to_clients (CHANGEVALUE, TAG_VOLUME, vol_sum/num_master_channels);
 }
 
 static int
@@ -271,6 +361,10 @@
 			if (cfgure)	base->mixer_delayed  = taglist->data;
 			else		taglist->data = base->mixer_delayed;
 			break;
+		case TAG_MIXERCHANNELS:
+			if (cfgure)	alsamixer_setup_elements ((char *) taglist->data);
+			else		taglist->data = (long) base->channels;
+			break;
 		case TAG_SOUNDSYSTEM:
 			if (cfgure)	tagerror (taglist, E_NOWRITE);
 			else 		taglist->data = SOUNDSYSTEM_ALSA;
@@ -286,10 +380,14 @@
 {
 	struct moddata_alsamixer *base = &modbase_alsamixer;
 	static char devbuffer_mixer[STDBUFFERLEN];
+	static char mixer_channels[STDBUFFERLEN];
 	char *cardname;
+	char *channels;
 	int rc;
 
 	base->card		= devbuffer_mixer;
+	base->channels		= mixer_channels;
+	base->elements          = NULL;
 	base->keyvolup		= KEY_VOLUMEUP;
 	base->modvolup		= MOD_NONE;
 	base->keyvoldn		= KEY_VOLUMEDOWN;
@@ -310,6 +408,12 @@
 	else
 		return  E_BUFOVL;
 
+	channels = (char *) tagfind (taglist, TAG_MIXERCHANNELS, (long) "Master");
+	if (strlen (channels) < STDBUFFERLEN)
+		strncpy (base->channels, channels, STDBUFFERLEN);
+	else
+		return  E_BUFOVL;
+
 	if ((base->mixer_delayed = tagfind (taglist, TAG_MIXERINITDELAY, 0)) == 0) {
 		alsamixer_finish_init ();
 	} else {
@@ -330,6 +434,11 @@
 int
 alsamixer_exit ()
 {
+	struct moddata_alsamixer *base = &modbase_alsamixer;
+	free (base->elements);
+	base->elements = NULL;
+	snd_mixer_free (base->mixer);
+	base->init_complete = 0;
 	return 0;
 }
 

Update to ALSA module patch

[John S. S. <to...@to...>]

Finally I have finished off the patch for ALSA volume control. I would have
liked to have made a neater alsamixer_set_and_send function, but I'm not sure
that it is worth the effort.

You can now set multiple mixer channels, and volume/mute will be changed for
each one. This follows the same paradigm used in the OSS module, where the first
mixer channel specified is considered the "master", and the other channels are
synchronised to that. (By the way, what pbbuttonsd considers to be "channels",
ALSA calls "elements".)

I have tested changing mixer channels on the fly, which works fine, but I have
not tested switching between OSS and ALSA while pbbuttonsd is running. (I don't
have OSS emulation setup on my machine.)

This patch is against current CVS.

cheers,

John

Index: module_alsamixer.c
===================================================================
RCS file: /cvsroot/pbbuttons/pbbuttons/pbbuttonsd/src/module_alsamixer.c,v
retrieving revision 1.3
diff -u -r1.3 module_alsamixer.c
--- module_alsamixer.c	12 Dec 2004 19:33:11 -0000	1.3
+++ module_alsamixer.c	31 Dec 2004 01:24:03 -0000
@@ -30,6 +30,7 @@
 
 struct moddata_alsamixer {
 	char *card;	 /* name of the soundcard (usually "default" is fine) */
+	char *channels;		/* the "MixerChannels" config string */
 	unsigned short keyvolup;
 	unsigned short modvolup;
 	unsigned short keyvoldn;
@@ -39,70 +40,140 @@
 	int init_complete;
 	int mixer_delayed;
 	int mute;
-	long vol_min;		/* minimum volume ALSA allows master */
-	long vol_max;		/* maximum volume ALSA allows master */
-	snd_mixer_elem_t * master; /* master sound element */
+	snd_mixer_t * mixer;
+	snd_mixer_elem_t ** elements; /* points to a NULL terminated list, first is master */
 } modbase_alsamixer;
 
+/* initialise the list of elements, based on the "channels" string */
 static int
-alsamixer_finish_init ()
+alsamixer_setup_elements_internal (char * channels, snd_mixer_selem_id_t *sid)
 {
 	struct moddata_alsamixer *base = &modbase_alsamixer;
-	snd_mixer_t *handle;
+        char *token;
+	char buffer[STDBUFFERLEN];
+	int n;
+
+	/* get rid of old list */
+	free (base->elements);
+	base->elements = NULL;
+	n = 0;
+
+	/* this copy we save for returning in alsamixer_handle_tags */
+	strncpy (base->channels, channels, STDBUFFERLEN);
+
+	/* this copy we use for strtok */
+	strncpy (buffer, channels, STDBUFFERLEN);
+	cleanup_buffer (buffer);
+	if ((token = strtok(buffer, ",\n"))) {
+		do {
+			/* ensure we have enough memory to store pointer + terminator */
+			base->elements = realloc (base->elements, (n+2)*sizeof (snd_mixer_elem_t *));
+			if (base->elements == NULL) {
+				print_error (_("ERROR: memory allocation failed\n"));
+				return -1;
+			}
+
+			/* find element */
+			for (base->elements[n] = snd_mixer_first_elem (base->mixer);
+			     base->elements[n];
+			     base->elements[n] = snd_mixer_elem_next (base->elements[n])) {
+				snd_mixer_selem_get_id (base->elements[n], sid);
+				if (strcasecmp (snd_mixer_selem_id_get_name (sid), token) == 0)
+					break;
+			}
+			
+			/* abort at first element we can't find */
+			if (base->elements[n] == NULL) {
+				print_error (_("ERROR: %s has no \"%s\" element\n"), base->card, token);
+				return -1;
+			} else {
+				n++;
+			}
+			
+		} while ((token = strtok(0,",\n")) != NULL);
+
+		/* terminate list */
+		base->elements[n] = NULL;
+
+		/* check that master has a playback volume */
+		if (!snd_mixer_selem_has_playback_volume (base->elements[0])) {
+			snd_mixer_selem_get_id (base->elements[n], sid);
+			print_error (_("ERROR: mixer element \"%s\" has no playback volume control\n"), snd_mixer_selem_id_get_name (sid));
+			return -1;
+		}
+
+		return 0;
+	} else {
+		/* no tokens? */
+		return -1;
+	}
+}
+
+/* wrapper arround alsamixer_setup_elements_internal so we can be sure to free the sid. */
+static int
+alsamixer_setup_elements (char * channels)
+{
 	snd_mixer_selem_id_t *sid;
-	int err;
+	int err, result;
 
-	/* setup mixer */
+	/* create a selem_id to extract names from */
 	if ((err = snd_mixer_selem_id_malloc (&sid))){
-		print_error (_("ERROR: failed memory allocation: %s"), snd_strerror (err));
+		print_error (_("ERROR: failed memory allocation: %s\n"), snd_strerror (err));
 		return -1;
 	}
-	if ((err = snd_mixer_open (&handle, 0)) < 0) {
-		print_error (_("ERROR: mixer open %s: %s"), base->card, snd_strerror (err));
+
+	result = alsamixer_setup_elements_internal (channels, sid);
+	snd_mixer_selem_id_free (sid);
+	return result;
+}
+
+static int
+alsamixer_finish_init ()
+{
+	struct moddata_alsamixer *base = &modbase_alsamixer;
+	int err;
+
+	/* setup mixer */
+	if ((err = snd_mixer_open (&base->mixer, 0)) < 0) {
+		print_error (_("ERROR: mixer open %s: %s\n"), base->card, snd_strerror (err));
 		return -1;
 	}
-	if ((err = snd_mixer_attach (handle, base->card)) < 0) {
-		print_error (_("ERROR: mixer attach %s: %s"), base->card, snd_strerror (err));
-		snd_mixer_close (handle);
+	if ((err = snd_mixer_attach (base->mixer, base->card)) < 0) {
+		print_error (_("ERROR: mixer attach %s: %s\n"), base->card, snd_strerror (err));
+		snd_mixer_free (base->mixer);
 		return -1;
 	}
-	if ((err = snd_mixer_selem_register (handle, NULL, NULL)) < 0) {
-		print_error (_("ERROR: mixer register: %s"), snd_strerror (err));
-		snd_mixer_close (handle);
+	if ((err = snd_mixer_selem_register (base->mixer, NULL, NULL)) < 0) {
+		print_error (_("ERROR: mixer register: %s\n"), snd_strerror (err));
+		snd_mixer_free (base->mixer);
 		return -1;
 	}
 	
-	if ((err = snd_mixer_load (handle)) < 0) {
-		print_error (_("ERROR: load %s: %s"), base->card, snd_strerror (err));
-		snd_mixer_close (handle);
+	if ((err = snd_mixer_load (base->mixer)) < 0) {
+		print_error (_("ERROR: load %s: %s\n"), base->card, snd_strerror (err));
+		snd_mixer_free (base->mixer);
 		return -1;
 	}
 
-	/* find "Master" element */
-	for (base->master = snd_mixer_first_elem (handle); base->master; base->master = snd_mixer_elem_next (base->master)) {
-		snd_mixer_selem_get_id (base->master, sid);
-		if(strcmp (snd_mixer_selem_id_get_name (sid), "Master") == 0) {
-			snd_mixer_selem_get_playback_volume_range (base->master, &base->vol_min, &base->vol_max);
-			base->init_complete = 1;
-			return 0;
-		}
+	if ((err = alsamixer_setup_elements (base->channels)) < 0) {
+		snd_mixer_free (base->mixer);
+		return -1;
+	} else {
+		base->init_complete = 1;
+		return 0;
 	}
-
-	/* couldn't find element */
-	print_error (_("ERROR: %s has no \"Master\" element"), base->card);
-	return -1;
 }
 
 /* Converts an ALSA volume to a number in the range [0, +/-100]. This function
  * assumes that the mixer has already been setup successfully.
  */
 static inline int
-alsamixer_normalise_volume (long volume)
+alsamixer_normalise_volume (snd_mixer_elem_t * elem, long volume)
 {
-	long range;
-	struct moddata_alsamixer *base = &modbase_alsamixer;
+	long range, vol_min, vol_max;
 
-	range = base->vol_max - base->vol_min;
+	snd_mixer_selem_get_playback_volume_range (elem, &vol_min, &vol_max);
+	range = vol_max - vol_min;
 	return volume*100/range;
 }
 
@@ -110,16 +181,16 @@
  * assumes that the mixer has already been setup successfully.
  */
 static inline long
-alsamixer_denormalise_volume (int volume)
+alsamixer_denormalise_volume (snd_mixer_elem_t * elem, int volume)
 {
-	long range;
-	struct moddata_alsamixer *base = &modbase_alsamixer;
+	long range, vol_min, vol_max;
 	
-	range = base->vol_max - base->vol_min;
+	snd_mixer_selem_get_playback_volume_range (elem, &vol_min, &vol_max);
+	range = vol_max - vol_min;
 	return volume*range/100;
 }
 
-/* Returns the average normalised volume across all channels of an element, or -1. */
+/* Returns the average normalised volume across all channels of the master element, or -1. */
 static int
 alsamixer_get_volume ()
 {
@@ -127,7 +198,7 @@
 	long vol, vol_sum;
 	int num_chn;
 	struct moddata_alsamixer *base = &modbase_alsamixer;
-	snd_mixer_elem_t * elem = base->master;
+	snd_mixer_elem_t * elem = base->elements[0];
 
 	vol_sum = num_chn = 0;
 	
@@ -148,65 +219,86 @@
 		}
 	}
 
-	return alsamixer_normalise_volume (vol_sum/num_chn);
+	return alsamixer_normalise_volume (elem, vol_sum/num_chn);
 }
 
 static void
 alsamixer_set_and_send (int increment)
 {
 	snd_mixer_selem_channel_id_t chn;
-	long vol,  vol_sum, denormal_increment;
-	int num_chn;
+	long vol,  vol_sum, denormal_increment, vol_min, vol_max, normalised_master_vol;
+	int num_master_channels;
+        snd_mixer_elem_t ** elem_ptr;
+	snd_mixer_elem_t * elem;
 	struct moddata_alsamixer *base = &modbase_alsamixer;
-	snd_mixer_elem_t * elem = base->master;
 
 	vol_sum = 0;
-	num_chn = 0;
-	denormal_increment = alsamixer_denormalise_volume (increment);
+	num_master_channels = 0;
+        normalised_master_vol = -1;
 	
 	if (base->init_complete == 0) /* is mixer setup already completed? */
 		if ((alsamixer_finish_init ()) != 0) /* no, then do it now */
 			return;	/* Oops, try it again later */
-				   
-	if (snd_mixer_selem_has_playback_volume (elem)) {
-		/* cycle through each playback channel of element */
-		for (chn = 0; chn <= SND_MIXER_SCHN_LAST; chn++) {
-			if (!snd_mixer_selem_has_playback_channel (elem, chn))
-				continue;
+	
+	/* cycle through each element */
+	for (elem_ptr = base->elements; *elem_ptr != NULL; elem_ptr++) {
+		elem = *elem_ptr;
+		
+		if (snd_mixer_selem_has_playback_volume (elem)) {
+
+			/* get range so we can saturate if changing volume */
+			snd_mixer_selem_get_playback_volume_range (elem, &vol_min, &vol_max);
+
+			/* cycle through each playback channel of element */
+			for (chn = 0; chn <= SND_MIXER_SCHN_LAST; chn++) {
+				if (!snd_mixer_selem_has_playback_channel (elem, chn))
+					continue;
 			
-			/* visit the channel */
-			if (increment == 0) {
-				/* toggle mute status (pbs is opposite of mute) */
-				snd_mixer_selem_get_playback_switch (elem, chn, &base->mute);
-				snd_mixer_selem_set_playback_switch (elem, chn, !base->mute);
-				/* sum volume */
-				snd_mixer_selem_get_playback_volume (elem, chn, &vol);
-				vol_sum += vol;
-				num_chn++;
-			} else {
-				/* increment/decrement volume */
-				snd_mixer_selem_get_playback_volume (elem, chn, &vol);
+				/* visit the channel */
+				if (increment == 0) {
+					/* toggle mute status (pbs is opposite of mute) */
+					snd_mixer_selem_get_playback_switch (elem, chn, &base->mute);
+					snd_mixer_selem_set_playback_switch (elem, chn, !base->mute);
+
+					if (normalised_master_vol != -1) {
+						/* sum volume for report to clients */
+						snd_mixer_selem_get_playback_volume (elem, chn, &vol);
+						vol_sum += alsamixer_normalise_volume (elem, vol);
+						num_master_channels++;
+					}
+				} else if (normalised_master_vol != -1) {
+					/* sync slave element with master element */
+					vol = alsamixer_denormalise_volume (elem, normalised_master_vol);
+					snd_mixer_selem_set_playback_volume (elem, chn, vol);
+				} else {
+					/* increment/decrement volume for master element */
+					snd_mixer_selem_get_playback_volume (elem, chn, &vol);
 				
-				vol += denormal_increment;
-				if (vol > base->vol_max)
-					vol = base->vol_max;
-				else if (vol < base->vol_min)
-					vol = base->vol_min;
-				snd_mixer_selem_set_playback_volume (elem, chn, vol);
-
-				/* sum volume */
-				vol_sum += vol;
-				num_chn++;
+					denormal_increment = alsamixer_denormalise_volume (elem, increment);
+					vol += denormal_increment;
+					if (vol > vol_max)
+						vol = vol_max;
+					else if (vol < vol_min)
+						vol = vol_min;
+					snd_mixer_selem_set_playback_volume (elem, chn, vol);
+
+					/* sum volume for report to clients */
+					vol_sum += alsamixer_normalise_volume (elem, vol);
+					num_master_channels++;
+				}
+			}
+			
+			if (normalised_master_vol == -1) {
+				/* compute average master volume */
+				normalised_master_vol = vol_sum/num_master_channels;
 			}
 		}
 	}
 
-	if (base->mute) {
+	if (base->mute)
 		singletag_to_clients (CHANGEVALUE, TAG_MUTE, 0);
-	} else {
-		vol = alsamixer_normalise_volume (vol_sum/num_chn);
-		singletag_to_clients (CHANGEVALUE, TAG_VOLUME, vol);
-	}
+	else
+		singletag_to_clients (CHANGEVALUE, TAG_VOLUME, vol_sum/num_master_channels);
 }
 
 static int
@@ -271,6 +363,10 @@
 			if (cfgure)	base->mixer_delayed  = taglist->data;
 			else		taglist->data = base->mixer_delayed;
 			break;
+		case TAG_MIXERCHANNELS:
+			if (cfgure)	alsamixer_setup_elements ((char *) taglist->data);
+			else		taglist->data = (long) base->channels;
+			break;
 		case TAG_SOUNDSYSTEM:
 			if (cfgure)	tagerror (taglist, E_NOWRITE);
 			else 		taglist->data = SOUNDSYSTEM_ALSA;
@@ -286,10 +382,14 @@
 {
 	struct moddata_alsamixer *base = &modbase_alsamixer;
 	static char devbuffer_mixer[STDBUFFERLEN];
+	static char mixer_channels[STDBUFFERLEN];
 	char *cardname;
+	char *channels;
 	int rc;
 
 	base->card		= devbuffer_mixer;
+	base->channels		= mixer_channels;
+	base->elements          = NULL;
 	base->keyvolup		= KEY_VOLUMEUP;
 	base->modvolup		= MOD_NONE;
 	base->keyvoldn		= KEY_VOLUMEDOWN;
@@ -310,6 +410,12 @@
 	else
 		return  E_BUFOVL;
 
+	channels = (char *) tagfind (taglist, TAG_MIXERCHANNELS, (long) "Master");
+	if (strlen (channels) < STDBUFFERLEN)
+		strncpy (base->channels, channels, STDBUFFERLEN);
+	else
+		return  E_BUFOVL;
+
 	if ((base->mixer_delayed = tagfind (taglist, TAG_MIXERINITDELAY, 0)) == 0) {
 		alsamixer_finish_init ();
 	} else {
@@ -330,6 +436,11 @@
 int
 alsamixer_exit ()
 {
+	struct moddata_alsamixer *base = &modbase_alsamixer;
+	free (base->elements);
+	base->elements = NULL;
+	snd_mixer_free (base->mixer);
+	base->init_complete = 0;
 	return 0;
 }
 

Re: gtkpbbuttons session managment buggy

[Matthias G. <mat...@us...>]

On Wed, 8 Dec 2004 22:24:17 +0100
Matthias Grimm <mat...@us...> wrote:

Hi,

The work at GTKPBButtons is nearly finished. After founding a good and up-to-date
popt man page I have moved gtkpbbuttons command line handling to libpopt. As
consequence we got rid ot this ugly dummy command line args to please GNOME.
This will make life easier in future.

For everyone who want to test the current version is in CVS.

 Best Regards
    Matthias

PS: The MacOSX theme of the CVS version still misses one new image so it may
      not start out of the box.

Re: Trackpad Tap for Right Click?

[Wolfram Q. <wo...@mi...>]

On Thu, Dec 09, 2004 at 12:28:24AM +0000, Felix Oxley wrote:
>=20
> On 8 Dec 2004, at 21:33, Matthias Grimm wrote:
>=20
> >This might be possible but I'm not sure if pbbuttonsd is the right=20
> >place for that.
> >Have you checked mouse utilities like mousemu?
>=20
> Do you have a web address for this? I couldn't find the homepage on=20
> Google.

Mouseemu is written by Colin Leroy and can be found at=20
http://geekounet.org/powerbook/
Guido G=FCnther made a debian package out of it that is available at
http://honk.physik.uni-konstanz.de/~agx/linux-ppc/debian/mouseemu/
Probably this installs fine on ubuntu.

HTH,

Wolfi

Re: Trackpad Tap for Right Click?

[Dean H. <de...@bo...>]

most likely it would be an X config file question.

look for some per device setting to swap mouse buttons


Dean

Matthias Grimm wrote:

>On Tue, 7 Dec 2004 13:18:27 +0000
>Felix Oxley <fe...@ox...> wrote:
>
>Hi Felix,
> 
>  
>
>>I am just trying out Linux (various flavours but mainly Ubuntu) on my 
>>PowerBook.
>>This great utility seems to be pretty standard.
>>
>>But I am desperate to have track pad tapping emulate a RIGHT click. 
>>(Like SideTrack in Mac OS X).
>>
>>Can it do this?
>>If not, would it be easy/hard/impossible to implement?
>>    
>>
>
>This might be possible but I'm not sure if pbbuttonsd is the right place for that.
>Have you checked mouse utilities like mousemu? They might already do what you
>asked for. Pbbuttonsd only tells the trackpad what mode to use, not how trackpad
>actions should be interpreted.
>  
>  
>
>>(apologies to Matthias for mailing him directly)
>>    
>>
>
>No problem. I was very busy so that your mail has been forgotten to answer, sorry
>for that.
>
>  Best Regards
>    Matthias
>
>
>
>
>
>-------------------------------------------------------
>SF email is sponsored by - The IT Product Guide
>Read honest & candid reviews on hundreds of IT Products from real users.
>Discover which products truly live up to the hype. Start reading now. 
>http://productguide.itmanagersjournal.com/
>_______________________________________________
>pbbuttons-users mailing list
>pbb...@li...
>https://lists.sourceforge.net/lists/listinfo/pbbuttons-users
>
>  
>

-- 
WWW: http://dean.bong.com.au  LAN: http://www.bong.com.au
EMAIL: de...@bo...       or       dj...@bo...
ICQ: 16867613

Re: Trackpad Tap for Right Click?

[Felix O. <fe...@ox...>]

On 8 Dec 2004, at 21:33, Matthias Grimm wrote:

> This might be possible but I'm not sure if pbbuttonsd is the right 
> place for that.
> Have you checked mouse utilities like mousemu?

Do you have a web address for this? I couldn't find the homepage on 
Google.

Thanks

Re: gtkpbbuttons session managment buggy

[Frank L. <fr...@li...>]

On Wed, Dec 08, 2004 at 10:24:17PM +0100, Matthias Grimm wrote:
> As the first step I changed gtkpbbutton's 'evaluate_args()' in such a way that
> it will ignore unknown command line arguments. Getopt will complain about them
> but the program will be launched as desired. This is only temporary until I found a
> better solution.
> 
> I had some trouble with Popt. It's very badly documented (or I haven't found the
> right documents yet) and the man page is rather outdated. Using popt in gnome
> applications is also not documented at all so that I have to study a lot of source
> code. If anybody could tell me where I could get a good and up-to-date popt
> tutorial and a gnome application that uses popt in a nice way, you are very
> welcome.

I only know of http://developer.gnome.org/doc/guides/popt/ and it is not
particulary extensive. I just picked a random GNOME app and looked at
the source. It was gnome-gv and it looked not particulary complicated:

	static struct poptOption options[] = {
                { "windows", 'w', POPT_ARG_INT, &win_count, 0,
                  N_("Specify the number of empty windows to open."),
                  N_("Number of empty windows") },
                { "geometry", '\0', POPT_ARG_STRING, &win_geometry, 0,
                  N_("X geometry specification (see \"X\" man page)."),
                  N_("GEOMETRY") },
                { NULL, '\0', 0, NULL, 0 }
        };

	...

	program = gnome_program_init("ggv", VERSION,
                                     LIBGNOMEUI_MODULE,
                                     argc, argv,
                                     GNOME_PARAM_POPT_TABLE, options,
                                     GNOME_PARAM_APP_DATADIR, DATADIR,NULL)


Though I not yet understand the magic about the code following the
snippet above:

	g_value_init(&value, G_TYPE_POINTER);
        g_object_get_property(G_OBJECT(program), GNOME_PARAM_POPT_CONTEXT,
                              &value);
        ctx = g_value_get_pointer(&value);
        g_value_unset(&value);


Gruesse,
-- 
Frank Lichtenheld <fr...@li...>
www: http://www.djpig.de/

Re: gtkpbbuttons session managment buggy

[Matthias G. <mat...@us...>]

> On Mon, Dec 06, 2004 at 09:53:32PM +0100, Matthias Grimm wrote:
> > The program didn't crash but it seems that I can't test it very well under KDE.
> > Gtkpbbuttons didn't restart after relogin.

Ok, I found a method to test gnome session management even on KDE and it works
now (even on KDE, GNOME and KDE uses the same protokoll so it should have always
been worked, I think the problem was between chair and keyboard here ;-))

> The current code doesn't restart under GNOME either because GNOME always
> uses the --screen option. Adding screen as ARG_DUMMY fixes this. I would
> also suggest to only call gtk_init ifndef GNOME_ENABLED since
> gnome_program_init takes care of it otherwise. This leads to this patch:

I followed your advice and exclude gtk_init() and gnome_program_init() eachother.

> Problems that remain:
>  - gnome-program-init "captures" --help and so the produced output
>    doesn't make much sense currently in the gnome version

'--help' will call gnome help and '-h' will call gtkpbbuttons help. That '--help'
was captured doesn't matter here. Nevertheless its a ugly workaround to be
fixed in the future.

>  - To support the full range of GNOME and GTK options it still would be
>    best to move the argument parsing into gnome_program_init in the
>    gnome version. This would also solve the issue with --help.
>    One should consider to move the options parsing of the non-gnome
>    version from getopt to popt then, too. This would ensure minimal
>    code duplication.
>    Of course you still can add all supported options (gtkpbbuttons
>    --help will give it to you ;) as ARG_DUMMY, but this is ugly and
>    unreliable (in case new options get added).

This extra bogus options are indeed ugly and I don't want them either because they
don't belong to gtkpbbuttons and make it unreliable as you already said.

As the first step I changed gtkpbbutton's 'evaluate_args()' in such a way that
it will ignore unknown command line arguments. Getopt will complain about them
but the program will be launched as desired. This is only temporary until I found a
better solution.

I had some trouble with Popt. It's very badly documented (or I haven't found the
right documents yet) and the man page is rather outdated. Using popt in gnome
applications is also not documented at all so that I have to study a lot of source
code. If anybody could tell me where I could get a good and up-to-date popt
tutorial and a gnome application that uses popt in a nice way, you are very
welcome.

 The current and hopefully working version is in CVS

  Best Regards
    Matthias

Re: Trackpad Tap for Right Click?

[Matthias G. <mat...@us...>]

On Tue, 7 Dec 2004 13:18:27 +0000
Felix Oxley <fe...@ox...> wrote:

Hi Felix,
 
> I am just trying out Linux (various flavours but mainly Ubuntu) on my 
> PowerBook.
> This great utility seems to be pretty standard.
> 
> But I am desperate to have track pad tapping emulate a RIGHT click. 
> (Like SideTrack in Mac OS X).
>
> Can it do this?
> If not, would it be easy/hard/impossible to implement?

This might be possible but I'm not sure if pbbuttonsd is the right place for that.
Have you checked mouse utilities like mousemu? They might already do what you
asked for. Pbbuttonsd only tells the trackpad what mode to use, not how trackpad
actions should be interpreted.
  
> (apologies to Matthias for mailing him directly)

No problem. I was very busy so that your mail has been forgotten to answer, sorry
for that.

  Best Regards
    Matthias

Fw: Re: gtkpbbuttons session managment buggy

[Matthias G. <mat...@us...>]

Begin forwarded message:

Date: Tue, 7 Dec 2004 16:01:25 +0100
From: Frank Lichtenheld <dj...@de...>
To: Matthias Grimm <mat...@us...>
Cc: pbb...@li..., he...@gn...
Subject: Re: gtkpbbuttons session managment buggy


On Mon, Dec 06, 2004 at 09:53:32PM +0100, Matthias Grimm wrote:
> The program didn't crash but it seems that I can't test it very well under KDE.
> Gtkpbbuttons didn't restart after relogin.
> 
> All GNOME users are invited to test the session management of gtkpbbuttons,
> please. :-) You will find the code in CVS.

The current code doesn't restart under GNOME either because GNOME always
uses the --screen option. Adding screen as ARG_DUMMY fixes this. I would
also suggest to only call gtk_init ifndef GNOME_ENABLED since
gnome_program_init takes care of it otherwise. This leads to this patch:

Index: src/gtkpbbinit.c
===================================================================
RCS file: /cvsroot/pbbuttons/pbbuttons/gtkpbbuttons/src/gtkpbbinit.c,v
retrieving revision 1.2
diff -u -r1.2 gtkpbbinit.c
--- src/gtkpbbinit.c    6 Dec 2004 20:07:19 -0000       1.2
+++ src/gtkpbbinit.c    7 Dec 2004 14:53:09 -0000
@@ -116,6 +116,7 @@
                  {"theme", required_argument, 0, ARG_THEME},
                  {"sm-client-id", optional_argument, 0, ARG_DUMMY},
                  {"sm-config-prefix", optional_argument, 0, ARG_DUMMY},
+                 {"screen", optional_argument, 0, ARG_DUMMY},
                  {NULL, 0, NULL, 0}
        };
        int c, err = 0;
Index: src/gtkpbbuttons.c
===================================================================
RCS file: /cvsroot/pbbuttons/pbbuttons/gtkpbbuttons/src/gtkpbbuttons.c,v
retrieving revision 1.12
diff -u -r1.12 gtkpbbuttons.c
--- src/gtkpbbuttons.c  6 Dec 2004 20:07:20 -0000       1.12
+++ src/gtkpbbuttons.c  7 Dec 2004 14:53:09 -0000
@@ -48,10 +48,11 @@

        init_init(md, argc, argv);
        init_libpbb ();
-       gtk_init (&md->argc, &md->argv);

 #ifdef ENABLE_GNOME
        gnome_program_init (PACKAGE, VERSION, LIBGNOMEUI_MODULE, md->argc, md->argv, NULL);
+#else
+       gtk_init (&md->argc, &md->argv);
 #endif

        if((rc = evaluate_args(md)) == 0) {     /* evaluate command line */

Problems that remain:
 - gnome-program-init "captures" --help and so the produced output
   doesn't make much sense currently in the gnome version
 - To support the full range of GNOME and GTK options it still would be
   best to move the argument parsing into gnome_program_init in the
   gnome version. This would also solve the issue with --help.
   One should consider to move the options parsing of the non-gnome
   version from getopt to popt then, too. This would ensure minimal
   code duplication.
   Of course you still can add all supported options (gtkpbbuttons
   --help will give it to you ;) as ARG_DUMMY, but this is ugly and
   unreliable (in case new options get added).

Gruesse,
-- 
Frank Lichtenheld <fr...@li...>
www: http://www.djpig.de/

Re: gtkpbbuttons session managment buggy

[Frank L. <dj...@de...>]

On Mon, Dec 06, 2004 at 09:53:32PM +0100, Matthias Grimm wrote:
> The program didn't crash but it seems that I can't test it very well under KDE.
> Gtkpbbuttons didn't restart after relogin.
> 
> All GNOME users are invited to test the session management of gtkpbbuttons,
> please. :-) You will find the code in CVS.

The current code doesn't restart under GNOME either because GNOME always
uses the --screen option. Adding screen as ARG_DUMMY fixes this. I would
also suggest to only call gtk_init ifndef GNOME_ENABLED since
gnome_program_init takes care of it otherwise. This leads to this patch:

Index: src/gtkpbbinit.c
===================================================================
RCS file: /cvsroot/pbbuttons/pbbuttons/gtkpbbuttons/src/gtkpbbinit.c,v
retrieving revision 1.2
diff -u -r1.2 gtkpbbinit.c
--- src/gtkpbbinit.c    6 Dec 2004 20:07:19 -0000       1.2
+++ src/gtkpbbinit.c    7 Dec 2004 14:53:09 -0000
@@ -116,6 +116,7 @@
                  {"theme", required_argument, 0, ARG_THEME},
                  {"sm-client-id", optional_argument, 0, ARG_DUMMY},
                  {"sm-config-prefix", optional_argument, 0, ARG_DUMMY},
+                 {"screen", optional_argument, 0, ARG_DUMMY},
                  {NULL, 0, NULL, 0}
        };
        int c, err = 0;
Index: src/gtkpbbuttons.c
===================================================================
RCS file: /cvsroot/pbbuttons/pbbuttons/gtkpbbuttons/src/gtkpbbuttons.c,v
retrieving revision 1.12
diff -u -r1.12 gtkpbbuttons.c
--- src/gtkpbbuttons.c  6 Dec 2004 20:07:20 -0000       1.12
+++ src/gtkpbbuttons.c  7 Dec 2004 14:53:09 -0000
@@ -48,10 +48,11 @@

        init_init(md, argc, argv);
        init_libpbb ();
-       gtk_init (&md->argc, &md->argv);

 #ifdef ENABLE_GNOME
        gnome_program_init (PACKAGE, VERSION, LIBGNOMEUI_MODULE, md->argc, md->argv, NULL);
+#else
+       gtk_init (&md->argc, &md->argv);
 #endif

        if((rc = evaluate_args(md)) == 0) {     /* evaluate command line */

Problems that remain:
 - gnome-program-init "captures" --help and so the produced output
   doesn't make much sense currently in the gnome version
 - To support the full range of GNOME and GTK options it still would be
   best to move the argument parsing into gnome_program_init in the
   gnome version. This would also solve the issue with --help.
   One should consider to move the options parsing of the non-gnome
   version from getopt to popt then, too. This would ensure minimal
   code duplication.
   Of course you still can add all supported options (gtkpbbuttons
   --help will give it to you ;) as ARG_DUMMY, but this is ugly and
   unreliable (in case new options get added).

Gruesse,
-- 
Frank Lichtenheld <fr...@li...>
www: http://www.djpig.de/

Trackpad Tap for Right Click?

[Felix O. <fe...@ox...>]

I am just trying out Linux (various flavours but mainly Ubuntu) on my 
PowerBook.
This great utility seems to be pretty standard.

But I am desperate to have track pad tapping emulate a RIGHT click. 
(Like SideTrack in Mac OS X).

Can it do this?

If not, would it be easy/hard/impossible to implement?
Thanks.

(apologies to Matthias for mailing him directly)

Re: gtkpbbuttons session managment buggy

[Matthias G. <mat...@us...>]

On Mon, 6 Dec 2004 10:35:05 +0100
Frank Lichtenheld <dj...@de...> wrote:

> This one was completly bogus. gtk_init isn't the culprit here. The
> problem is, that you save the address of the _parameters_ of
> session_init in pb_argv. However, these addresses are invalidated
> as soon as the function session_init quits. What you need to to is to save
> the address of argc and argv from main.
> 
> Of course gtk_init should still called earlier because the the gtk
> option handling.

Excactly this was what I tried to commit to CVS, but something went wrong.
Let's try again :-)

gtk_init() will be called as one of the first functions in program so that gtk
option handling should be ok now.

According your patch I put argc and argc into the global structure. As consequence
some functions got a more simple interface and I get rid of the memory allocation 
in session_init().

The program didn't crash but it seems that I can't test it very well under KDE.
Gtkpbbuttons didn't restart after relogin.

All GNOME users are invited to test the session management of gtkpbbuttons,
please. :-) You will find the code in CVS.

  Best Regards
    Matthias

Re: gtkpbbuttons session managment buggy

[Frank L. <dj...@de...>]

On Fri, Dec 03, 2004 at 04:49:31AM +0100, Frank Lichtenheld wrote:
> Hi.
> 
> I just tested the gnome support of gtkpbbuttons and I'm afraid I've
> to say that it is not usable currently. gtkpbbuttons segfaults
> when trying to save the session. I've identified the cause but
> don't know yet how to fix it best.
> 
> gtkpbbuttons tries to save pointers to argc and argv. However, after
> calling session_init, gtk_init is called, which destroys these pointers.

This one was completly bogus. gtk_init isn't the culprit here. The
problem is, that you save the address of the _parameters_ of
session_init in pb_argv. However, these addresses are invalidated
as soon as the function session_init quits. What you need to to is to save
the address of argc and argv from main.

Of course gtk_init should still called earlier because the the gtk
option handling.

The following patch seems to work:
Index: src/gtkpbbuttons.c
===================================================================
RCS file: /cvsroot/pbbuttons/pbbuttons/gtkpbbuttons/src/gtkpbbuttons.c,v
retrieving revision 1.11
diff -u -r1.11 gtkpbbuttons.c
--- src/gtkpbbuttons.c  3 Dec 2004 14:45:17 -0000       1.11
+++ src/gtkpbbuttons.c  6 Dec 2004 09:31:20 -0000
@@ -56,7 +56,7 @@
        if((rc = evaluate_args(md, argc, argv)) == 0) { /* evaluate
command line */
                if ((create_daemon ()) == 0) {
 #ifdef ENABLE_GNOME
-                       session_init(argc, argv); /* connect to GNOME session management */
+                       session_init(&argc, &argv); /* connect to GNOME session management */
 #endif
                        gtk_init (&argc, &argv);
                        if ((rc = prg_init(md)) == 0)
Index: src/session.c
===================================================================
RCS file: /cvsroot/pbbuttons/pbbuttons/gtkpbbuttons/src/session.c,v
retrieving revision 1.1
diff -u -r1.1 session.c
--- src/session.c       4 May 2004 18:35:18 -0000       1.1
+++ src/session.c       6 Dec 2004 09:31:20 -0000
@@ -26,13 +26,13 @@
 #include "session.h"

 void
-session_init (int argc, char *argv[])
+session_init (int* argc, char*** argv)
 {
        GnomeClient *sm_client;
        GtkPbArgv *pb_argv = g_new0 (GtkPbArgv, 1);

-       pb_argv->argc = &argc;
-       pb_argv->argv = &argv;
+       pb_argv->argc = argc;
+       pb_argv->argv = argv;

        sm_client = gnome_master_client ();
        gnome_client_set_restart_style (sm_client, GNOME_RESTART_ANYWAY);
Index: src/session.h
===================================================================
RCS file: /cvsroot/pbbuttons/pbbuttons/gtkpbbuttons/src/session.h,v
retrieving revision 1.1
diff -u -r1.1 session.h
--- src/session.h       4 May 2004 18:35:18 -0000       1.1
+++ src/session.h       6 Dec 2004 09:31:20 -0000
@@ -29,7 +29,7 @@
 } GtkPbArgv;

 /* prototypes */
-void session_init (int argc, char *argv[]);
+void session_init (int* argc, char*** argv);
 void session_die (GnomeClient *client, gpointer user_data);
 gboolean session_save (GnomeClient *client, int phase, GnomeSaveStyle save_style, int end, GnomeInteractStyle interact_style, int fast, gpointer user_data);

Gruesse,
-- 
Frank Lichtenheld <fr...@li...>
www: http://www.djpig.de/

Re: gtkpbbuttons session managment buggy

[Frank L. <dj...@de...>]

On Fri, Dec 03, 2004 at 03:55:20PM +0100, Matthias Grimm wrote:
> Thanks for your report. I haven't developed gnome programs too but I
> see your point. For the first try I rearanged the functions at program
> start a little bit.
[...]
> A patched version is in CVS for anybody who want to test.

Could it be that there happened some mistake while committing?
The change under the log entry "gtk options were always ignored - fixed"
is
 #endif
-			gtk_set_locale ();
 			gtk_init (&argc, &argv);

which is clearly not the whole fix you were talking about in your mail.

Gruesse,
-- 
Frank Lichtenheld <fr...@li...>
www: http://www.djpig.de/

Re: gtkpbbuttons session managment buggy

[Frank L. <fr...@li...>]

On Fri, Dec 03, 2004 at 03:55:20PM +0100, Matthias Grimm wrote:
> A patched version is in CVS for anybody who want to test.

I will give it a shot.

> Could you tell me where I find the documentation of gnome_program_init()
> without the need of downloading the complete gnome SDK or source code?

http://developer.gnome.org/doc/ but don't ask me for more specific
pointers. I read the description of gnome_program_init in the libgnome
API documentation and then took a look at the source code of some
GNOME programs. And it was late at night, I tend to be a bit sloppy at
that time, so I will try to recheck my findings during the weekend ;)

Gruesse,
-- 
Frank Lichtenheld <fr...@li...>
www: http://www.djpig.de/

Re: gtkpbbuttons session managment buggy

[Matthias G. <mat...@us...>]

On Fri, 3 Dec 2004 04:49:31 +0100
Frank Lichtenheld <dj...@de...> wrote:

> gtkpbbuttons tries to save pointers to argc and argv. However, after
> calling session_init, gtk_init is called, which destroys these pointers.

Thanks for your report. I haven't developed gnome programs too but I
see your point. For the first try I rearanged the functions at program
start a little bit.

gtk_init() will be called first now so that gtk options can be used again.
gtk_init() filters known options out so that they won't disturb  anymore.

After that gnome_program_init() will be called and may save argc and
argv pointers that won't be changed in further program execution.

Last but not least evaluate_args() will be called to process gtkpbbuttons's
command line options.

I don't know if this is a satisfying and complete solution but this may prevent
the segfault. I will have a deeper look into this issue next week when I have
more time to do this.

A patched version is in CVS for anybody who want to test.

Could you tell me where I find the documentation of gnome_program_init()
without the need of downloading the complete gnome SDK or source code?

  Best Regards
   Matthias

gtkpbbuttons session managment buggy

[Frank L. <dj...@de...>]

Hi.

I just tested the gnome support of gtkpbbuttons and I'm afraid I've
to say that it is not usable currently. gtkpbbuttons segfaults
when trying to save the session. I've identified the cause but
don't know yet how to fix it best.

gtkpbbuttons tries to save pointers to argc and argv. However, after
calling session_init, gtk_init is called, which destroys these pointers.

Therefor session_save segfaults when trying to use these pointers. The
reason I don't have prepeared a patch yet is that there are more issues
with the command line parsing when compiled with GNOME support.
Apparently (I haven't deleopped gnome applications myself, yet)
gnome_program_init is supposed to do the commandline parsing for
the whole app. Therefor it doesn't remove common gnome options from
argv (unlike gtk_init does for gtk options). So one can use the gnome
options (like --disable-crash-assistent, you might guess why I tried
this one ;) but gtkpbbuttons is exiting immediatly in evaluate_args.

For similar reasons one can't use the gtk options either because
gtk_init is called _after_ evaluate_args, when it should called _before_
to be able to strip out and hande the gtk options.

Don't hesitate to ask if I left any details out that are important to
you for understanding the problems :)

Gruesse,
-- 
Frank Lichtenheld <fr...@li...>
www: http://www.djpig.de/

[PATCH] little fix to gtkpbbuttons Makefile.am

[Frank L. <dj...@de...>]

The install-data-local rule in the Makefile.am for gtkpbbuttons
has a flaw which prevents installation of the themes
if $(srcdir) != "". Patch follows (could still use a little
more testing):

Index: Makefile.am
===================================================================
RCS file: /home/djpig/cvs/Projekte/debian/gtkpbbuttons/Makefile.am,v
retrieving revision 1.2
retrieving revision 1.7
diff -u -r1.2 -r1.7
--- Makefile.am 28 Jun 2004 22:30:00 -0000      1.2
+++ Makefile.am 3 Dec 2004 01:09:25 -0000       1.7
@@ -8,12 +8,13 @@

 install-data-local:
        @$(NORMAL_INSTALL)
-       if test -d $(srcdir)/themes; then \
+       set -e; if test -d $(srcdir)/themes; then \
          $(mkinstalldirs) $(DESTDIR)$(pkgdatadir)/themes; \
-         for theme in $(srcdir)/themes/*; do \
-           if test -d $$theme; then \
+         for themedir in $(srcdir)/themes/*; do \
+           theme=themes/`basename $$themedir`; \
+           if test -d $$themedir; then \
              $(mkinstalldirs) $(DESTDIR)$(pkgdatadir)/$$theme; \
-             for file in $(srcdir)/$$theme/*; do \
+             for file in $$themedir/*; do \
                if test -f $$file; then \
                  $(INSTALL_DATA) $$file
$(DESTDIR)$(pkgdatadir)/$$theme; \
                fi; \
Index: Makefile.in
===================================================================
RCS file: /home/djpig/cvs/Projekte/debian/gtkpbbuttons/Makefile.in,v
retrieving revision 1.3
retrieving revision 1.8
diff -u -r1.3 -r1.8
--- Makefile.in 29 Jun 2004 10:37:41 -0000      1.3
+++ Makefile.in 3 Dec 2004 01:09:25 -0000       1.8
@@ -462,12 +462,13 @@

 install-data-local:
        @$(NORMAL_INSTALL)
-       if test -d $(srcdir)/themes; then \
+       set -e; if test -d $(srcdir)/themes; then \
          $(mkinstalldirs) $(DESTDIR)$(pkgdatadir)/themes; \
-         for theme in $(srcdir)/themes/*; do \
-           if test -d $$theme; then \
+         for themedir in $(srcdir)/themes/*; do \
+           theme=themes/`basename $$themedir`; \
+           if test -d $$themedir; then \
              $(mkinstalldirs) $(DESTDIR)$(pkgdatadir)/$$theme; \
-             for file in $(srcdir)/$$theme/*; do \
+             for file in $$themedir/*; do \
                if test -f $$file; then \
                  $(INSTALL_DATA) $$file
$(DESTDIR)$(pkgdatadir)/$$theme; \
                fi; \


(The CVS revision numbers are from my private Debian repository)

Gruesse,
-- 
Frank Lichtenheld <fr...@li...>
www: http://www.djpig.de/