[alsa-devel] [PATCH] modem.conf Off-hook improve behavior
From: David Fries david@fries.net
Only restore the old value if it differs from the requested value, because if it has changed restoring the old value overrides the change. Take for example, a voice modem with a .conf that sets preserve off-hook. Start playback (on-hook to off-hook), start record (off-hook to off-hook), stop playback (off-hook to restore on-hook), stop record (on-hook to restore off-hook), Clearly you don't want to leave the modem "on the phone" now that there isn't any playback or recording active.
Signed-off-by: David Fries david@fries.net --- Comments welcome.
The default modem.conf has, hooks.0 { type ctl_elems hook_args [ { name "Off-hook Switch" preserve true value "on" lock false optional true } ] } I wrote the patch to be, if you didn't modify it, don't restore the value. I also thought that it might work to check to see if the current mixer value matches the requested value (which is the value the program set on open), and only set the old value if it hasn't been externally modified. The theory being if something else changed the mixer value, we don't want to by default overwrite that change.
Some additional flags could also be added to leave the current behavior alone and add the if you didn't modify it, don't restore it, or if it changed after you set it, don't restore it. Here are some flag names if that would be preferred, preserve_if_different restore_if_unmodified no_squash restore_overrides preserve_if_changed.
Setting the lock isn't really an option, because then you can't manually "hang up" the modem if you needed to.
On a side note, do any of the alsa voice/software modems support notifying userspace when the line is ringing? I'm using intel8x0m, not that I think it matters here.
diff --git a/include/control.h b/include/control.h index 2361dc3..ded884a 100644 --- a/include/control.h +++ b/include/control.h @@ -419,6 +419,7 @@ int snd_ctl_elem_value_malloc(snd_ctl_elem_value_t **ptr); void snd_ctl_elem_value_free(snd_ctl_elem_value_t *obj); void snd_ctl_elem_value_clear(snd_ctl_elem_value_t *obj); void snd_ctl_elem_value_copy(snd_ctl_elem_value_t *dst, const snd_ctl_elem_value_t *src); +int snd_ctl_elem_value_compare(snd_ctl_elem_value_t *left, const snd_ctl_elem_value_t *right); void snd_ctl_elem_value_get_id(const snd_ctl_elem_value_t *obj, snd_ctl_elem_id_t *ptr); unsigned int snd_ctl_elem_value_get_numid(const snd_ctl_elem_value_t *obj); snd_ctl_elem_iface_t snd_ctl_elem_value_get_interface(const snd_ctl_elem_value_t *obj); diff --git a/src/control/control.c b/src/control/control.c index df249dd..159b09a 100644 --- a/src/control/control.c +++ b/src/control/control.c @@ -2256,6 +2256,18 @@ void snd_ctl_elem_value_copy(snd_ctl_elem_value_t *dst, const snd_ctl_elem_value }
/** + * \brief compare one #snd_ctl_elem_value_t to another + * \param dst pointer to destination + * \param src pointer to source + * \return 0 on match, less than or greater than otherwise, see memcmp + */ +int snd_ctl_elem_value_compare(snd_ctl_elem_value_t *left, const snd_ctl_elem_value_t *right) +{ + assert(left && right); + return memcmp(left, right, sizeof(*left)); +} + +/** * \brief Get CTL element identifier of a CTL element id/value * \param obj CTL element id/value * \param ptr Pointer to returned CTL element identifier diff --git a/src/control/setup.c b/src/control/setup.c index 408244e..3606daf 100644 --- a/src/control/setup.c +++ b/src/control/setup.c @@ -192,7 +192,17 @@ int snd_sctl_remove(snd_sctl_t *h) return err; } } - if (elem->preserve) { + /* Only restore the old value if it differs from the requested + * value, because if it has changed restoring the old value + * overrides the change. Take for example, a voice modem with + * a .conf that sets preserve off-hook. Start playback (on-hook + * to off-hook), start record (off-hook to off-hook), stop + * playback (off-hook to restore on-hook), stop record (on-hook + * to restore off-hook), Clearly you don't want to leave the + * modem "on the phone" now that there isn't any playback or + * recording active. + */ + if (elem->preserve && snd_ctl_elem_value_compare(elem->val, elem->old)) { err = snd_ctl_elem_write(h->ctl, elem->old); if (err < 0) { SNDERR("Cannot restore ctl elem");
At Fri, 25 Dec 2009 14:22:38 -0600, David Fries wrote:
From: David Fries david@fries.net
Only restore the old value if it differs from the requested value, because if it has changed restoring the old value overrides the change. Take for example, a voice modem with a .conf that sets preserve off-hook. Start playback (on-hook to off-hook), start record (off-hook to off-hook), stop playback (off-hook to restore on-hook), stop record (on-hook to restore off-hook), Clearly you don't want to leave the modem "on the phone" now that there isn't any playback or recording active.
Signed-off-by: David Fries david@fries.net
Thanks for the patch (and sorry for overlooking for a long time!) Applied now to GIT tree. It's a nice improvement.
Comments welcome.
The default modem.conf has, hooks.0 { type ctl_elems hook_args [ { name "Off-hook Switch" preserve true value "on" lock false optional true } ] } I wrote the patch to be, if you didn't modify it, don't restore the value. I also thought that it might work to check to see if the current mixer value matches the requested value (which is the value the program set on open), and only set the old value if it hasn't been externally modified. The theory being if something else changed the mixer value, we don't want to by default overwrite that change.
Some additional flags could also be added to leave the current behavior alone and add the if you didn't modify it, don't restore it, or if it changed after you set it, don't restore it. Here are some flag names if that would be preferred, preserve_if_different restore_if_unmodified no_squash restore_overrides preserve_if_changed.
Well, this would be just a feature no one will ever use :) If anyone stumbles the problem regarding this, we can implement such flags.
Setting the lock isn't really an option, because then you can't manually "hang up" the modem if you needed to.
On a side note, do any of the alsa voice/software modems support notifying userspace when the line is ringing? I'm using intel8x0m, not that I think it matters here.
I don't know of any, too.
Takashi
On Fri, Jan 22, 2010 at 11:51:59AM +0100, Takashi Iwai wrote:
At Fri, 25 Dec 2009 14:22:38 -0600, David Fries wrote:
On a side note, do any of the alsa voice/software modems support notifying userspace when the line is ringing? I'm using intel8x0m, not that I think it matters here.
I don't know of any, too.
I have a patch adding a mixer control to provide Ring Detection notification when the ring GPIO pin is going crazy. That's used to indicate when a local phone went off hook, but only the first time after being on hook for an extended time, or when a local phone goes back on hook, when the modem goes on hook or off hook, when a local user dials pulse (not tone), or if going crazy for an extended period of time the line might actually be ringing. In practice it has been very reliable as long as you ignore the short pulses, other than when someone is pulse dialing that is.
I also hooked up Caller ID and Loop Current Sense which is used to detect remote hangup (or a local phone going off hook when Caller ID is enabled). It is working, but it needs some cleanup before it is ready to be submitted. As dealing with the interrupts is more hardware specific I'm also thinking of moving setting up the sticky and interrupt mask setup from snd_ac97_modem_build to someplace in the intel8x0m.c file. Dealing with the hardware to get a usable ring indication out makes me long for the days when you could just wait for RING and the modem hid he details.
I have some changes to Asterisk as well to make use of it, though it is suffering from some audio glitches and isn't as far along.
participants (2)
-
David Fries
-
Takashi Iwai