Untested code, don't commit willy-nilly... I'll be away for the weekend, and I thought I'd already send this patch for review. I'll test this some day next week. element_change_off_and_zero_to_constant() is introduced to make the volume subset checking easier. element_dB_to_step and element_alsa_dB_to_step help with getting the step for 0 dB without tons of annoying ifs. They could probably be used also elsewhere in the code, but I'll do that cleanup later. Colin, there are also a couple of questions for you in the comments. --- src/modules/alsa/alsa-mixer.c | 132 ++++++++++++++++++++++++++++++----------- 1 files changed, 96 insertions(+), 36 deletions(-) diff --git a/src/modules/alsa/alsa-mixer.c b/src/modules/alsa/alsa-mixer.c index f6a2a20..8f1fb4a 100644 --- a/src/modules/alsa/alsa-mixer.c +++ b/src/modules/alsa/alsa-mixer.c @@ -1344,6 +1344,82 @@ static int check_required(pa_alsa_element *e, snd_mixer_elem_t *me) { return 0; } +static int element_alsa_dB_to_step(pa_alsa_element *e, snd_mixer_elem_t *me, long *dB, long *step) { + int r; + + pa_assert(e); + pa_assert(me); + pa_assert(dB); + pa_assert(step); + + if (e->direction == PA_ALSA_DIRECTION_OUTPUT) { + if ((r = snd_mixer_selem_ask_playback_dB_vol(me, *dB, +1, step)) >= 0) { + *step = (e->volume_limit >= 0) ? PA_MIN(*step, e->volume_limit) : *step; + + if ((r = snd_mixer_selem_ask_playback_vol_dB(me, *step, dB) < 0)) { + pa_log("snd_mixer_selem_ask_playback_vol_dB() failed: %s", pa_alsa_strerror(r)); + return -1; + } + } else { + pa_log("snd_mixer_selem_ask_playback_dB_vol() failed: %s", pa_alsa_strerror(r)); + return -1; + } + } else { + if ((r = snd_mixer_selem_ask_capture_dB_vol(me, *dB, -1, step)) >= 0) { + *step = (e->volume_limit >= 0) ? PA_MIN(*step, e->volume_limit) : *step; + + if ((r = snd_mixer_selem_ask_capture_vol_dB(me, *step, dB) < 0)) { + pa_log("snd_mixer_selem_ask_capture_vol_dB() failed: %s", pa_alsa_strerror(r)); + return -1; + } + } else { + pa_log("snd_mixer_selem_ask_capture_dB_vol() failed: %s", pa_alsa_strerror(r)); + return -1; + } + } + + return 0; +} + +static int element_dB_to_step(pa_alsa_element *e, snd_mixer_elem_t *me, long *dB, long *step) { + pa_assert(e); + pa_assert(me); + pa_assert(dB); + pa_assert(step); + + if (e->db_fix) { + int rounding = (e->direction == PA_ALSA_DIRECTION_OUTPUT) ? +1 : -1; + + *step = decibel_fix_get_step(e->db_fix, dB, rounding); + return 0; + } + + return element_alsa_dB_to_step(e, me, dB, step); +} + +/* If element's volume_use is OFF or ZERO, it's changed to CONSTANT. + * If a problem occurs with alsa while doing this, volume_use will be + * changed to IGNORE. */ +static void element_change_off_and_zero_to_constant(pa_alsa_element *e, snd_mixer_elem_t *me) { + pa_assert(e); + pa_assert(me); + + if (e->volume_use == PA_ALSA_VOLUME_OFF) { + e->volume_use = PA_ALSA_VOLUME_CONSTANT; + e->constant_volume = e->min_volume; + return; + } + + if (e->volume_use == PA_ALSA_VOLUME_ZERO) { + long dB = 0; + if (element_dB_to_step(e, me, &dB, &e->constant_volume) < 0) { + pa_log("Failed to query element's volume step for 0 dB. Ignoring volume for element %s.", e->alsa_name); + e->volume_use = PA_ALSA_VOLUME_IGNORE; + } else + e->volume_use = PA_ALSA_VOLUME_CONSTANT; + } +} + static int element_probe(pa_alsa_element *e, snd_mixer_t *m) { snd_mixer_selem_id_t *sid; snd_mixer_elem_t *me; @@ -1627,9 +1703,10 @@ static int element_probe(pa_alsa_element *e, snd_mixer_t *m) { } } } - } + element_change_off_and_zero_to_constant(e, me); + if (e->switch_use == PA_ALSA_SWITCH_SELECT) { pa_alsa_option *o; @@ -2922,54 +2999,31 @@ static pa_bool_t element_is_subset(pa_alsa_element *a, pa_alsa_element *b, snd_m /* Check the volume_use */ if (a->volume_use != PA_ALSA_VOLUME_IGNORE) { + if (b->volume_use == PA_ALSA_VOLUME_IGNORE) + return FALSE; + + /* ZERO and OFF should have been changed into CONSTANT + * while probing the elements. */ + pa_assert(a->volume_use == PA_ALSA_VOLUME_MERGE || a->volume_use == PA_ALSA_VOLUME_CONSTANT); + pa_assert(b->volume_use == PA_ALSA_VOLUME_MERGE || b->volume_use == PA_ALSA_VOLUME_CONSTANT); /* "Constant" is subset of "Constant" only when their constant values are equal */ if (a->volume_use == PA_ALSA_VOLUME_CONSTANT && b->volume_use == PA_ALSA_VOLUME_CONSTANT && a->constant_volume != b->constant_volume) return FALSE; - /* Different volume uses when b is not "Merge" means we are definitely not a subset */ - if (a->volume_use != b->volume_use && b->volume_use != PA_ALSA_VOLUME_MERGE) + /* "Merge" is never a subset of "Constant" */ + if (a->volume_use == PA_ALSA_VOLUME_MERGE && b->volume_use == PA_ALSA_VOLUME_CONSTANT) return FALSE; /* "Constant" is a subset of "Merge", if there is not a "volume-limit" in "Merge" below the actual constant. - * "Zero" and "Off" are just special cases of "Constant" when comparing to "Merge" * "Merge" with a "volume-limit" is a subset of "Merge" without a "volume-limit" or with a higher "volume-limit" */ if (b->volume_use == PA_ALSA_VOLUME_MERGE && b->volume_limit >= 0) { long a_limit; if (a->volume_use == PA_ALSA_VOLUME_CONSTANT) a_limit = a->constant_volume; - else if (a->volume_use == PA_ALSA_VOLUME_ZERO) { - long dB = 0; - - if (a->db_fix) { - int rounding = (a->direction == PA_ALSA_DIRECTION_OUTPUT ? +1 : -1); - a_limit = decibel_fix_get_step(a->db_fix, &dB, rounding); - } else { - snd_mixer_selem_id_t *sid; - snd_mixer_elem_t *me; - - SELEM_INIT(sid, a->alsa_name); - if (!(me = snd_mixer_find_selem(m, sid))) { - pa_log_warn("Element %s seems to have disappeared.", a->alsa_name); - return FALSE; - } - - if (a->direction == PA_ALSA_DIRECTION_OUTPUT) { - if (snd_mixer_selem_ask_playback_dB_vol(me, dB, +1, &a_limit) < 0) - return FALSE; - } else { - if (snd_mixer_selem_ask_capture_dB_vol(me, dB, -1, &a_limit) < 0) - return FALSE; - } - } - } else if (a->volume_use == PA_ALSA_VOLUME_OFF) - a_limit = a->min_volume; - else if (a->volume_use == PA_ALSA_VOLUME_MERGE) - a_limit = a->volume_limit; else - /* This should never be reached */ - pa_assert(FALSE); + a_limit = a->volume_limit; if (a_limit > b->volume_limit) return FALSE; @@ -2977,6 +3031,9 @@ static pa_bool_t element_is_subset(pa_alsa_element *a, pa_alsa_element *b, snd_m } if (a->switch_use != PA_ALSA_SWITCH_IGNORE) { + if (b->switch_use == PA_ALSA_SWITCH_IGNORE) + return FALSE; + /* "On" is a subset of "Mute". * "Off" is a subset of "Mute". * "On" is a subset of "Select", if there is an "Option:On" in B. @@ -3022,9 +3079,8 @@ static void path_set_condense(pa_alsa_path_set *ps, snd_mixer_t *m) { if (!ps->paths || !ps->paths->next) return; - for (p = ps->paths; p; p = np) { + PA_LLIST_FOREACH_SAFE(p, np, ps->paths) { pa_alsa_path *p2; - np = p->next; PA_LLIST_FOREACH(p2, ps->paths) { pa_alsa_element *ea, *eb; @@ -3043,6 +3099,8 @@ static void path_set_condense(pa_alsa_path_set *ps, snd_mixer_t *m) { ea = ea->next; eb = eb->next; if ((ea && !eb) || (!ea && eb)) + /* Question to Colin: Do we really want to require that p and p2 contain an equal amount of elements to consider p a subset of p2? + * Isn't it enough if p2 contains all elements of p? */ is_subset = FALSE; else if (!ea && !eb) break; @@ -3050,6 +3108,8 @@ static void path_set_condense(pa_alsa_path_set *ps, snd_mixer_t *m) { is_subset = FALSE; } else + /* Question to Colin: Do we really want to require that p and p2 contain their elements in the exact same order? + * Is there a guarantee for that? (I haven't inspected the code that closely.) */ is_subset = FALSE; } -- 1.7.5.4