Improve the Audio ALSA backend

Guard the ALSA backend against some undefined values,
error codes, and remove a memory leak.

Signed-off-by: Manuel Bachmann <manuel.bachmann@iot.bzh>

2 files changed, 61 insertions, 25 deletions

diff --git a/plugins/audio/audio-alsa.c b/plugins/audio/audio-alsa.c
index 28fe3258..45ff2edb 100644
--- a/plugins/audio/audio-alsa.c
+++ b/plugins/audio/audio-alsa.c
@@ -44,8 +44,10 @@ unsigned char _alsa_init (const char *name, audioCtxHandleT *ctx) {
     long vol, vol_min, vol_max;
     int num, i;
 
-    if (snd_pcm_open (&dev, name, SND_PCM_STREAM_PLAYBACK, 0) < 0)
+    if (snd_pcm_open (&dev, name, SND_PCM_STREAM_PLAYBACK, 0) < 0) {
+        fprintf (stderr, "ALSA backend could not open card '%s'\n", name);
         return 0;
+    }
 
     snd_pcm_hw_params_malloc (&params);
     snd_pcm_hw_params_any (dev, params);
@@ -55,6 +57,7 @@ unsigned char _alsa_init (const char *name, audioCtxHandleT *ctx) {
     snd_pcm_hw_params_set_channels (dev, params, ctx->channels);
     if (snd_pcm_hw_params (dev, params) < 0) {
         snd_pcm_hw_params_free (params);
+        fprintf (stderr, "ALSA backend could set channels on card '%s'\n", name);
         return 0;
     }
     snd_pcm_prepare (dev);
@@ -62,6 +65,7 @@ unsigned char _alsa_init (const char *name, audioCtxHandleT *ctx) {
     snd_mixer_open (&mixer, 0);
     if (snd_mixer_attach (mixer, name) < 0) {
         snd_pcm_hw_params_free (params);
+        fprintf (stderr, "ALSA backend could not open mixer for card '%s'\n", name);
         return 0;
     }
     snd_mixer_selem_register (mixer, NULL, NULL);
@@ -109,14 +113,17 @@ unsigned char _alsa_init (const char *name, audioCtxHandleT *ctx) {
     /* is a card with similar name already opened ? */
     for (num = 0; num < (sizeof(dev_ctx_a)/sizeof(dev_ctx_alsa_T*)); num++) {
         if (dev_ctx_a[num]->name &&
-           !strcmp (dev_ctx_a[num]->name, name))
+           !strcmp (dev_ctx_a[num]->name, name)) {
+            fprintf (stderr, "Card '%s' already locked by other ALSA backend session\n", name);
             return 0;
+        }
     }
-    num++;
+    num--;
 
     /* it's not... let us add it to the global array */
     dev_ctx_a = (dev_ctx_alsa_T**) realloc (dev_ctx_a, (num+1)*sizeof(dev_ctx_alsa_T*));
-    dev_ctx_a[num] = (dev_ctx_alsa_T*) malloc (sizeof(dev_ctx_alsa_T));
+    if (!dev_ctx_a[num])
+        dev_ctx_a[num] = (dev_ctx_alsa_T*) malloc (sizeof(dev_ctx_alsa_T));
     dev_ctx_a[num]->name = strdup (name);
     dev_ctx_a[num]->dev = dev;
     dev_ctx_a[num]->params = params;
@@ -206,6 +213,7 @@ void _alsa_set_volume_all (int num, unsigned int vol) {
     if (!dev_ctx_a || !dev_ctx_a[num] || !dev_ctx_a[num]->mixer_elm ||
         vol > 100)
 
+    fflush (stdout); /* seems to force this logic to apply quickly */
     snd_mixer_selem_set_playback_volume_all (dev_ctx_a[num]->mixer_elm, (vol*dev_ctx_a[num]->vol_max)/100);
 }
 
diff --git a/plugins/audio/audio-api.c b/plugins/audio/audio-api.c
index 8956f809..1ba9126c 100644
--- a/plugins/audio/audio-api.c
+++ b/plugins/audio/audio-api.c
@@ -161,7 +161,7 @@ static void releaseAudioCtx (void *context) {
 
 static void init (struct afb_req request) {        /* AFB_SESSION_CHECK */
 
-    audioCtxHandleT *ctx = (audioCtxHandleT*) afb_req_context_get(request);
+    audioCtxHandleT *ctx = afb_req_context_get (request);
     json_object *jresp;
 
     /* create a private client context */
@@ -170,9 +170,8 @@ static void init (struct afb_req request) {        /* AFB_SESSION_CHECK */
         afb_req_context_set (request, ctx, releaseAudioCtx);
     }
 
-    if (!_backend_init ("default", ctx)) {
+    if (!_backend_init ("default", ctx))
         afb_req_fail (request, "failed", "backend initialization failed");
-    }
 
     jresp = json_object_new_object();
     json_object_object_add (jresp, "init", json_object_new_string ("success"));
@@ -181,7 +180,7 @@ static void init (struct afb_req request) {        /* AFB_SESSION_CHECK */
 
 static void volume (struct afb_req request) {      /* AFB_SESSION_CHECK */
 
-    audioCtxHandleT *ctx = (audioCtxHandleT*) afb_req_context_get(request);
+    audioCtxHandleT *ctx = afb_req_context_get (request);
     const char *value = afb_req_value (request, "value");
     json_object *jresp;
     unsigned int volume[8], i;
@@ -189,15 +188,22 @@ static void volume (struct afb_req request) {      /* AFB_SESSION_CHECK */
     char volume_str[256];
     size_t len_str = 0;
 
+	if (!ctx) {
+        afb_req_fail (request, "failed", "you must call 'init' first");
+        return;
+    }
+    jresp = json_object_new_object();
+
     /* no "?value=" parameter : return current state */
     if (!value) {
         for (i = 0; i < 8; i++) {
             ctx->volume[i] = _backend_get_volume (ctx, i);
             snprintf (volume_str+len_str, sizeof(volume_str)-len_str, "%d,", ctx->volume[i]);
-            len_str = strlen(volume_str);
+            len_str = strlen (volume_str);
         }
-        jresp = json_object_new_object();
         json_object_object_add (jresp, "volume", json_object_new_string(volume_str));
+        afb_req_success (request, jresp, "Audio - Volume obtained");
+        return;
     }
 
     /* "?value=" parameter, set volume */
@@ -220,16 +226,16 @@ static void volume (struct afb_req request) {      /* AFB_SESSION_CHECK */
             /* if there is only one value, set all channels to this one */
             if (!volume_i && i == 1)
                _backend_set_volume_all (ctx, ctx->volume[0]);
-            if (!volume_i || 100 < atoi(volume_i) || atoi(volume_i) < 0) {
+            if (!volume_i || 100 < atoi(volume_i) || atoi(volume_i) < 0)
                ctx->volume[i] = _backend_get_volume (ctx, i);
-            } else {
+            else {
                ctx->volume[i] = (unsigned int) atoi(volume_i);
                _backend_set_volume (ctx, i, ctx->volume[i]);
             }
             len_str = strlen(volume_str);
             snprintf (volume_str+len_str, sizeof(volume_str)-len_str, "%d,", ctx->volume[i]);
         }
-        jresp = json_object_new_object();
+        free (volume_i);
         json_object_object_add (jresp, "volume", json_object_new_string(volume_str));
     }
 
@@ -238,23 +244,33 @@ static void volume (struct afb_req request) {      /* AFB_SESSION_CHECK */
 
 static void channels (struct afb_req request) {    /* AFB_SESSION_CHECK */
 
-    audioCtxHandleT *ctx = (audioCtxHandleT*) afb_req_context_get(request);
+    audioCtxHandleT *ctx = afb_req_context_get (request);
     const char *value = afb_req_value (request, "value");
-    json_object *jresp = json_object_new_object();
+    json_object *jresp;
     char channels_str[256];
 
+	if (!ctx) {
+        afb_req_fail (request, "failed", "you must call 'init' first");
+        return;
+    }
+    jresp = json_object_new_object();
+
     /* no "?value=" parameter : return current state */
     if (!value) {
         snprintf (channels_str, sizeof(channels_str), "%d", ctx->channels);
+
         json_object_object_add (jresp, "channels", json_object_new_string (channels_str));
+        afb_req_success (request, jresp, "Audio - Channels obtained");
+        return;
     }
 
     /* "?value=" parameter, set channels */
     else {
         ctx->channels = (unsigned int) atoi (value);
         _backend_set_channels (ctx, ctx->channels);
-
         snprintf (channels_str, sizeof(channels_str), "%d", ctx->channels);
+
+        jresp = json_object_new_object();
         json_object_object_add (jresp, "channels", json_object_new_string (channels_str));
     }
 
@@ -263,9 +279,15 @@ static void channels (struct afb_req request) {    /* AFB_SESSION_CHECK */
 
 static void mute (struct afb_req request) {        /* AFB_SESSION_CHECK */
 
-    audioCtxHandleT *ctx = (audioCtxHandleT*) afb_req_context_get(request);
+    audioCtxHandleT *ctx = afb_req_context_get (request);
     const char *value = afb_req_value (request, "value");
-    json_object *jresp = json_object_new_object();
+    json_object *jresp;
+
+	if (!ctx) {
+        afb_req_fail (request, "failed", "you must call 'init' first");
+        return;
+    }
+    jresp = json_object_new_object();
 
     /* no "?value=" parameter : return current state */
     if (!value) {
@@ -273,13 +295,14 @@ static void mute (struct afb_req request) {        /* AFB_SESSION_CHECK */
         ctx->mute ?
             json_object_object_add (jresp, "mute", json_object_new_string ("on"))
           : json_object_object_add (jresp, "mute", json_object_new_string ("off"));
+        afb_req_success (request, jresp, "Audio - Mute status obtained");
+        return;
     }
 
     /* "?value=" parameter is "1" or "true" */
     else if ( atoi(value) == 1 || !strcasecmp(value, "true") ) {
         ctx->mute = 1;
         _backend_set_mute (ctx, ctx->mute);
-
         json_object_object_add (jresp, "mute", json_object_new_string ("on"));
     }
 
@@ -287,7 +310,6 @@ static void mute (struct afb_req request) {        /* AFB_SESSION_CHECK */
     else if ( atoi(value) == 0 || !strcasecmp(value, "false") ) {
         ctx->mute = 0;
         _backend_set_mute (ctx, ctx->mute);
-
         json_object_object_add (jresp, "mute", json_object_new_string ("off"));
     }
 
@@ -296,22 +318,29 @@ static void mute (struct afb_req request) {        /* AFB_SESSION_CHECK */
 
 static void play (struct afb_req request) {        /* AFB_SESSION_CHECK */
 
-    audioCtxHandleT *ctx = (audioCtxHandleT*) afb_req_context_get(request);
+    audioCtxHandleT *ctx = afb_req_context_get (request);
     const char *value = afb_req_value (request, "value");
-    json_object *jresp = json_object_new_object();
+    json_object *jresp;
+
+	if (!ctx) {
+        afb_req_fail (request, "failed", "you must call 'init' first");
+        return;
+    }
+    jresp = json_object_new_object();
 
     /* no "?value=" parameter : return current state */
     if (!value) {
         ctx->is_playing ?
             json_object_object_add (jresp, "play", json_object_new_string ("on"))
           : json_object_object_add (jresp, "play", json_object_new_string ("off"));
+        afb_req_success (request, jresp, "Audio - Playing status obtained");
+        return;
     }
 
     /* "?value=" parameter is "1" or "true" */
     else if ( atoi(value) == 1 || !strcasecmp(value, "true") ) {
         ctx->is_playing = 1;
         _backend_play (ctx);
-
         json_object_object_add (jresp, "play", json_object_new_string ("on"));
     }
 
@@ -319,7 +348,6 @@ static void play (struct afb_req request) {        /* AFB_SESSION_CHECK */
     else if ( atoi(value) == 0 || !strcasecmp(value, "false") ) {
         ctx->is_playing = 0;
         _backend_stop (ctx);
-
         json_object_object_add (jresp, "play", json_object_new_string ("off"));
     }
 
@@ -330,7 +358,7 @@ static void ping (struct afb_req request) {         /* AFB_SESSION_NONE */
     afb_req_success (request, NULL, "Audio - Ping success");
 }
 
-static const struct AFB_verb_desc_v1 verbs[]= {
+static const struct AFB_verb_desc_v1 verbs[] = {
   {"init"    , AFB_SESSION_CHECK,  init      , "Audio API - init"},
   {"volume"  , AFB_SESSION_CHECK,  volume    , "Audio API - volume"},
   {"channels", AFB_SESSION_CHECK,  channels  , "Audio API - channels"},