[pulseaudio-discuss] [PATCH] alsa-mixer: Fix element_is_subset().

Tanu Kaskinen tanuk at iki.fi
Fri Jul 22 10:20:20 PDT 2011


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



More information about the pulseaudio-discuss mailing list