Remove ability to have no active switch option and fix OSS to correctly handle such case

4 files changed, 91 insertions, 36 deletions

diff --git a/backends/oss/oss-switch.c b/backends/oss/oss-switch.c
index b138051..8e32281 100644
--- a/backends/oss/oss-switch.c
+++ b/backends/oss/oss-switch.c
@@ -38,10 +38,12 @@ static void oss_switch_finalize        (GObject             *object);
 
 G_DEFINE_TYPE (OssSwitch, oss_switch, MATE_MIXER_TYPE_SWITCH)
 
-static gboolean     oss_switch_set_active_option (MateMixerSwitch       *mms,
-                                                  MateMixerSwitchOption *mmso);
+static gboolean         oss_switch_set_active_option (MateMixerSwitch       *mms,
+                                                      MateMixerSwitchOption *mmso);
 
-static const GList *oss_switch_list_options      (MateMixerSwitch       *mms);
+static const GList *    oss_switch_list_options      (MateMixerSwitch       *mms);
+
+static OssSwitchOption *choose_default_option        (OssSwitch             *swtch);
 
 static void
 oss_switch_class_init (OssSwitchClass *klass)
@@ -104,10 +106,14 @@ oss_switch_new (const gchar *name,
 {
     OssSwitch *swtch;
 
+    g_return_val_if_fail (name != NULL, NULL);
+    g_return_val_if_fail (label != NULL, NULL);
+    g_return_val_if_fail (fd != -1, NULL);
+    g_return_val_if_fail (options != NULL, NULL);
+
     swtch = g_object_new (OSS_TYPE_SWITCH,
                           "name", name,
                           "label", label,
-                          "flags", MATE_MIXER_SWITCH_ALLOWS_NO_ACTIVE_OPTION,
                           "role", MATE_MIXER_SWITCH_ROLE_PORT,
                           NULL);
 
@@ -121,39 +127,70 @@ oss_switch_new (const gchar *name,
 void
 oss_switch_load (OssSwitch *swtch)
 {
-    GList *list;
-    gint   recsrc;
-    gint   ret;
+    OssSwitchOption *option;
+    gint             recsrc;
+    gint             devnum;
+    gint             ret;
 
     g_return_if_fail (OSS_IS_SWITCH (swtch));
 
     if G_UNLIKELY (swtch->priv->fd == -1)
         return;
 
+    /* Recsrc contains a bitmask of currently enabled recording sources */
     ret = ioctl (swtch->priv->fd, MIXER_READ (SOUND_MIXER_RECSRC), &recsrc);
     if (ret == -1)
         return;
-    if (recsrc == 0) {
-        _mate_mixer_switch_set_active_option (MATE_MIXER_SWITCH (swtch), NULL);
-        return;
-    }
 
-    list = swtch->priv->options;
-    while (list != NULL) {
-        OssSwitchOption *option = OSS_SWITCH_OPTION (list->data);
-        gint             devnum = oss_switch_option_get_devnum (option);
-
-        /* Mark the selected option when we find it */
-        if (recsrc & (1 << devnum)) {
-            _mate_mixer_switch_set_active_option (MATE_MIXER_SWITCH (swtch),
-                                                  MATE_MIXER_SWITCH_OPTION (option));
-            return;
+    if (recsrc != 0) {
+        GList *list = swtch->priv->options;
+
+        /* Find out which option is currently selected */
+        while (list != NULL) {
+            gint devnum;
+
+            option = OSS_SWITCH_OPTION (list->data);
+            devnum = oss_switch_option_get_devnum (option);
+
+            if (recsrc & (1 << devnum)) {
+                /* It is possible that some hardware might allow and have more recording
+                 * sources active at the same time, but we only support one active
+                 * source at a time */
+                _mate_mixer_switch_set_active_option (MATE_MIXER_SWITCH (swtch),
+                                                      MATE_MIXER_SWITCH_OPTION (option));
+                return;
+            }
+            list = list->next;
         }
-        list = list->next;
+
+        g_debug ("Switch %s has unknown device %d as the active option",
+                 mate_mixer_switch_get_name (MATE_MIXER_SWITCH (swtch)),
+                 devnum);
+
+        /* OSS shouldn't let a non-record device be selected, let's step in and select
+         * something reasonable instead... */
+    } else {
+         g_debug ("Switch %s has no active device",
+                  mate_mixer_switch_get_name (MATE_MIXER_SWITCH (swtch)));
+
+        /* According to the OSS Programmer's Guide, if the recsrc value is 0, the
+         * microphone will be selected implicitly.
+         * Let's not assume that's true everywhere and select something explicitly... */
     }
 
-    g_warning ("Unknown active option of switch %s",
-               mate_mixer_switch_get_name (MATE_MIXER_SWITCH (swtch)));
+    option = choose_default_option (swtch);
+
+    g_debug ("Selecting default device %s as active for switch %s",
+             mate_mixer_switch_option_get_name (MATE_MIXER_SWITCH_OPTION (option)),
+             mate_mixer_switch_get_name (MATE_MIXER_SWITCH (swtch)));
+
+    if (mate_mixer_switch_set_active_option (MATE_MIXER_SWITCH (swtch),
+                                             MATE_MIXER_SWITCH_OPTION (option)) == FALSE) {
+        g_debug ("Failed to set the default device, assuming it is selected anyway");
+
+        _mate_mixer_switch_set_active_option (MATE_MIXER_SWITCH (swtch),
+                                              MATE_MIXER_SWITCH_OPTION (option));
+    }
 }
 
 void
@@ -174,7 +211,6 @@ oss_switch_set_active_option (MateMixerSwitch *mms, MateMixerSwitchOption *mmso)
     OssSwitch *swtch;
     gint       ret;
     gint       recsrc;
-    gint       devnum;
 
     g_return_val_if_fail (OSS_IS_SWITCH (mms), FALSE);
     g_return_val_if_fail (OSS_IS_SWITCH_OPTION (mmso), FALSE);
@@ -184,8 +220,7 @@ oss_switch_set_active_option (MateMixerSwitch *mms, MateMixerSwitchOption *mmso)
     if G_UNLIKELY (swtch->priv->fd == -1)
         return FALSE;
 
-    devnum = oss_switch_option_get_devnum (OSS_SWITCH_OPTION (mmso));
-    recsrc = 1 << devnum;
+    recsrc = 1 << oss_switch_option_get_devnum (OSS_SWITCH_OPTION (mmso));
 
     ret = ioctl (swtch->priv->fd, MIXER_WRITE (SOUND_MIXER_RECSRC), &recsrc);
     if (ret == -1)
@@ -201,3 +236,27 @@ oss_switch_list_options (MateMixerSwitch *mms)
 
     return OSS_SWITCH (mms)->priv->options;
 }
+
+#define OSS_SWITCH_PREFERRED_DEFAULT_DEVNUM 7 /* Microphone */
+
+static OssSwitchOption *
+choose_default_option (OssSwitch *swtch)
+{
+    GList *list = swtch->priv->options;
+
+    /* Search for the preferred device */
+    while (list != NULL) {
+        OssSwitchOption *option;
+        gint             devnum;
+
+        option = OSS_SWITCH_OPTION (list->data);
+        devnum = oss_switch_option_get_devnum (option);
+
+        if (devnum == OSS_SWITCH_PREFERRED_DEFAULT_DEVNUM)
+            return option;
+
+        list = list->next;
+    }
+    /* If the preferred device is not present, use the first available one */
+    return OSS_SWITCH_OPTION (swtch->priv->options->data);
+}
diff --git a/libmatemixer/matemixer-enum-types.c b/libmatemixer/matemixer-enum-types.c
index cbc4ea8..6d88a83 100644
--- a/libmatemixer/matemixer-enum-types.c
+++ b/libmatemixer/matemixer-enum-types.c
@@ -198,7 +198,6 @@ mate_mixer_switch_flags_get_type (void)
         static const GFlagsValue values[] = {
             { MATE_MIXER_SWITCH_NO_FLAGS, "MATE_MIXER_SWITCH_NO_FLAGS", "no-flags" },
             { MATE_MIXER_SWITCH_TOGGLE, "MATE_MIXER_SWITCH_TOGGLE", "toggle" },
-            { MATE_MIXER_SWITCH_ALLOWS_NO_ACTIVE_OPTION, "MATE_MIXER_SWITCH_ALLOWS_NO_ACTIVE_OPTION", "allows-no-active-option" },
             { 0, NULL, NULL }
         };
         etype = g_flags_register_static (
diff --git a/libmatemixer/matemixer-enums.h b/libmatemixer/matemixer-enums.h
index 96176fa..befc651 100644
--- a/libmatemixer/matemixer-enums.h
+++ b/libmatemixer/matemixer-enums.h
@@ -266,9 +266,8 @@ typedef enum {
  *     The switch is a #MateMixerToggle.
  */
 typedef enum { /*< flags >*/
-    MATE_MIXER_SWITCH_NO_FLAGS                = 0,
-    MATE_MIXER_SWITCH_TOGGLE                  = 1 << 0,
-    MATE_MIXER_SWITCH_ALLOWS_NO_ACTIVE_OPTION = 1 << 1
+    MATE_MIXER_SWITCH_NO_FLAGS = 0,
+    MATE_MIXER_SWITCH_TOGGLE   = 1 << 0,
 } MateMixerSwitchFlags;
 
 /**
diff --git a/libmatemixer/matemixer-switch.c b/libmatemixer/matemixer-switch.c
index 87580dd..6466838 100644
--- a/libmatemixer/matemixer-switch.c
+++ b/libmatemixer/matemixer-switch.c
@@ -359,6 +359,7 @@ mate_mixer_switch_set_active_option (MateMixerSwitch       *swtch,
     MateMixerSwitchClass *klass;
 
     g_return_val_if_fail (MATE_MIXER_IS_SWITCH (swtch), FALSE);
+    g_return_val_if_fail (MATE_MIXER_IS_SWITCH_OPTION (option), FALSE);
 
     klass = MATE_MIXER_SWITCH_GET_CLASS (swtch);
     if (klass->set_active_option == NULL)
@@ -398,7 +399,7 @@ _mate_mixer_switch_set_active_option (MateMixerSwitch       *swtch,
                                       MateMixerSwitchOption *option)
 {
     g_return_if_fail (MATE_MIXER_IS_SWITCH (swtch));
-    g_return_if_fail (option == NULL || MATE_MIXER_IS_SWITCH_OPTION (option));
+    g_return_if_fail (MATE_MIXER_IS_SWITCH_OPTION (option));
 
     if (swtch->priv->active == option)
         return;
@@ -406,10 +407,7 @@ _mate_mixer_switch_set_active_option (MateMixerSwitch       *swtch,
     if (swtch->priv->active != NULL)
         g_object_unref (swtch->priv->active);
 
-    if (option != NULL)
-        swtch->priv->active = g_object_ref (option);
-    else
-        swtch->priv->active = NULL;
+    swtch->priv->active = g_object_ref (option);
 
     g_object_notify_by_pspec (G_OBJECT (swtch), properties[PROP_ACTIVE_OPTION]);
 }