From e6d40a8447eff5e1be00ea35715092876e0520fa Mon Sep 17 00:00:00 2001 From: Manuel Bachmann Date: Mon, 23 May 2016 14:23:06 +0200 Subject: [PATCH] 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 --- plugins/audio/audio-alsa.c | 16 ++++++++--- plugins/audio/audio-api.c | 70 ++++++++++++++++++++++++++++++++-------------- 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 (¶ms); 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"}, -- 2.16.6