app_waitforsilence: Cleanup & don't treat missing frames as 'noise'
authorSean Bright <sean.bright@gmail.com>
Wed, 6 Sep 2017 15:50:53 +0000 (11:50 -0400)
committerSean Bright <sean.bright@gmail.com>
Wed, 6 Sep 2017 21:16:19 +0000 (16:16 -0500)
* WaitForSilence completes successfully if it receives no media in the
  specified timeout, but when acting as WaitForNoise that logic needs
  to be reversed.

* Use standard argument parsing macros and add some error checking for
  invalid values.

* The documentation indicated that the first argument to both
  WaitForSilence and WaitForNoise was required when it was not. Update
  the documentation to reflect that.

* Wrap up some behavior in structs to avoid boolean checks all over the
  place.

ASTERISK-24066 #close
Reported by: M vd S

Change-Id: I01d40adc5b63342bb5018a1bea2081a0aa191ef9

apps/app_waitforsilence.c

index 5eaeace..52248ac 100644 (file)
@@ -44,6 +44,7 @@
 
 #include "asterisk.h"
 
+#include "asterisk/app.h"
 #include "asterisk/file.h"
 #include "asterisk/channel.h"
 #include "asterisk/pbx.h"
@@ -57,7 +58,9 @@
                        Waits for a specified amount of silence.
                </synopsis>
                <syntax>
-                       <parameter name="silencerequired" required="true" />
+                       <parameter name="silencerequired">
+                               <para>If not specified, defaults to <literal>1000</literal> milliseconds.</para>
+                       </parameter>
                        <parameter name="iterations">
                                <para>If not specified, defaults to <literal>1</literal>.</para>
                        </parameter>
                        Waits for a specified amount of noise.
                </synopsis>
                <syntax>
-                       <parameter name="noiserequired" required="true" />
+                       <parameter name="noiserequired">
+                               <para>If not specified, defaults to <literal>1000</literal> milliseconds.</para>
+                       </parameter>
                        <parameter name="iterations">
                                <para>If not specified, defaults to <literal>1</literal>.</para>
                        </parameter>
 static char *app_silence = "WaitForSilence";
 static char *app_noise = "WaitForNoise";
 
-static int do_waiting(struct ast_channel *chan, int timereqd, time_t waitstart, int timeout, int wait_for_silence) {
-       struct ast_frame *f = NULL;
-       int dsptime = 0;
+struct wait_type {
+       const char *name;
+       const char *status;
+       int stop_on_frame_timeout;
+       int (*func)(struct ast_dsp *, struct ast_frame *, int *);
+};
+
+static const struct wait_type wait_for_silence = {
+       .name = "silence",
+       .status = "SILENCE",
+       .stop_on_frame_timeout = 1,
+       .func = ast_dsp_silence,
+};
+
+static const struct wait_type wait_for_noise = {
+       .name = "noise",
+       .status = "NOISE",
+       .stop_on_frame_timeout = 0,
+       .func = ast_dsp_noise,
+};
+
+static int do_waiting(struct ast_channel *chan, int timereqd, time_t waitstart, int timeout, const struct wait_type *wait_for)
+{
        RAII_VAR(struct ast_format *, rfmt, NULL, ao2_cleanup);
-       int res = 0;
-       struct ast_dsp *sildet;  /* silence detector dsp */
-       time_t now;
-
-       /*Either silence or noise calc depending on wait_for_silence flag*/
-       int (*ast_dsp_func)(struct ast_dsp*, struct ast_frame*, int*) =
-                               wait_for_silence ? ast_dsp_silence : ast_dsp_noise;
+       int res;
+       struct ast_dsp *sildet;
 
        rfmt = ao2_bump(ast_channel_readformat(chan));
        if ((res = ast_set_read_format(chan, ast_format_slin)) < 0) {
@@ -145,15 +165,13 @@ static int do_waiting(struct ast_channel *chan, int timereqd, time_t waitstart,
 
        /* Create the silence detector */
        if (!(sildet = ast_dsp_new())) {
-               ast_log(LOG_WARNING, "Unable to create silence detector :(\n");
+               ast_log(LOG_WARNING, "Unable to create silence detector\n");
                return -1;
        }
        ast_dsp_set_threshold(sildet, ast_dsp_get_threshold_from_settings(THRESHOLD_SILENCE));
 
-       /* Await silence... */
        for (;;) {
-               /* Start with no silence received */
-               dsptime = 0;
+               int dsptime = 0;
 
                res = ast_waitfor(chan, timereqd);
 
@@ -162,34 +180,36 @@ static int do_waiting(struct ast_channel *chan, int timereqd, time_t waitstart,
                        pbx_builtin_setvar_helper(chan, "WAITSTATUS", "HANGUP");
                        break;
                }
-               
+
                /* We waited and got no frame; sounds like digital silence or a muted digital channel */
                if (res == 0) {
-                       dsptime = timereqd;
+                       if (wait_for->stop_on_frame_timeout) {
+                               dsptime = timereqd;
+                       }
                } else {
                        /* Looks like we did get a frame, so let's check it out */
-                       if (!(f = ast_read(chan))) {
+                       struct ast_frame *f = ast_read(chan);
+                       if (!f) {
                                pbx_builtin_setvar_helper(chan, "WAITSTATUS", "HANGUP");
                                break;
                        }
                        if (f->frametype == AST_FRAME_VOICE) {
-                               ast_dsp_func(sildet, f, &dsptime);
+                               wait_for->func(sildet, f, &dsptime);
                        }
                        ast_frfree(f);
                }
 
-               ast_debug(1, "Got %dms %s < %dms required\n", dsptime, wait_for_silence ? "silence" : "noise", timereqd);
+               ast_debug(1, "Got %dms of %s < %dms required\n", dsptime, wait_for->name, timereqd);
 
                if (dsptime >= timereqd) {
-                       ast_verb(3, "Exiting with %dms %s >= %dms required\n", dsptime, wait_for_silence ? "silence" : "noise", timereqd);
-                       /* Ended happily with silence */
+                       ast_verb(3, "Exiting with %dms of %s >= %dms required\n", dsptime, wait_for->name, timereqd);
+                       pbx_builtin_setvar_helper(chan, "WAITSTATUS", wait_for->status);
+                       ast_debug(1, "WAITSTATUS was set to %s\n", wait_for->status);
                        res = 1;
-                       pbx_builtin_setvar_helper(chan, "WAITSTATUS", wait_for_silence ? "SILENCE" : "NOISE");
-                       ast_debug(1, "WAITSTATUS was set to %s\n", wait_for_silence ? "SILENCE" : "NOISE");
                        break;
                }
 
-               if (timeout && (difftime(time(&now), waitstart) >= timeout)) {
+               if (timeout && difftime(time(NULL), waitstart) >= timeout) {
                        pbx_builtin_setvar_helper(chan, "WAITSTATUS", "TIMEOUT");
                        ast_debug(1, "WAITSTATUS was set to TIMEOUT\n");
                        res = 0;
@@ -197,61 +217,86 @@ static int do_waiting(struct ast_channel *chan, int timereqd, time_t waitstart,
                }
        }
 
-
        if (rfmt && ast_set_read_format(chan, rfmt)) {
                ast_log(LOG_WARNING, "Unable to restore format %s to channel '%s'\n", ast_format_get_name(rfmt), ast_channel_name(chan));
        }
+
        ast_dsp_free(sildet);
        return res;
 }
 
-static int waitfor_exec(struct ast_channel *chan, const char *data, int wait_for_silence)
+static int waitfor_exec(struct ast_channel *chan, const char *data, const struct wait_type *wait_for)
 {
        int res = 1;
        int timereqd = 1000;
        int timeout = 0;
        int iterations = 1, i;
        time_t waitstart;
+       char *parse;
        struct ast_silence_generator *silgen = NULL;
 
-       if (ast_channel_state(chan) != AST_STATE_UP) {
-               res = ast_answer(chan); /* Answer the channel */
+       AST_DECLARE_APP_ARGS(args,
+               AST_APP_ARG(timereqd);
+               AST_APP_ARG(iterations);
+               AST_APP_ARG(timeout);
+       );
+
+       parse = ast_strdupa(data);
+       AST_STANDARD_APP_ARGS(args, parse);
+
+       if (!ast_strlen_zero(args.timereqd)) {
+               if (sscanf(args.timereqd, "%30d", &timereqd) != 1 || timereqd < 0) {
+                       ast_log(LOG_ERROR, "Argument '%srequired' must be an integer greater than or equal to zero.\n",
+                                       wait_for->name);
+                       return -1;
+               }
        }
 
-       if (!data || ( (sscanf(data, "%30d,%30d,%30d", &timereqd, &iterations, &timeout) != 3) &&
-               (sscanf(data, "%30d,%30d", &timereqd, &iterations) != 2) &&
-               (sscanf(data, "%30d", &timereqd) != 1) ) ) {
-               ast_log(LOG_WARNING, "Using default value of 1000ms, 1 iteration, no timeout\n");
+       if (!ast_strlen_zero(args.iterations)) {
+               if (sscanf(args.iterations, "%30d", &iterations) != 1 || iterations < 1) {
+                       ast_log(LOG_ERROR, "Argument 'iterations' must be an integer greater than 0.\n");
+                       return -1;
+               }
+       }
+
+       if (!ast_strlen_zero(args.timeout)) {
+               if (sscanf(args.timeout, "%30d", &timeout) != 1 || timeout < 0) {
+                       ast_log(LOG_ERROR, "Argument 'timeout' must be an integer greater than or equal to zero.\n");
+                       return -1;
+               }
+       }
+
+       if (ast_channel_state(chan) != AST_STATE_UP) {
+               ast_answer(chan); /* Answer the channel */
        }
 
-       ast_verb(3, "Waiting %d time(s) for %d ms silence with %d timeout\n", iterations, timereqd, timeout);
+       ast_verb(3, "Waiting %d time(s) for %dms of %s with %ds timeout\n",
+                        iterations, timereqd, wait_for->name, timeout);
 
        if (ast_opt_transmit_silence) {
                silgen = ast_channel_start_silence_generator(chan);
        }
+
        time(&waitstart);
-       res = 1;
-       for (i=0; (i<iterations) && (res == 1); i++) {
-               res = do_waiting(chan, timereqd, waitstart, timeout, wait_for_silence);
+       for (i = 0; i < iterations && res == 1; i++) {
+               res = do_waiting(chan, timereqd, waitstart, timeout, wait_for);
        }
+
        if (silgen) {
                ast_channel_stop_silence_generator(chan, silgen);
        }
 
-
-       if (res > 0)
-               res = 0;
-       return res;
+       return res > 0 ? 0 : res;
 }
 
 static int waitforsilence_exec(struct ast_channel *chan, const char *data)
 {
-       return waitfor_exec(chan, data, 1);
+       return waitfor_exec(chan, data, &wait_for_silence);
 }
 
 static int waitfornoise_exec(struct ast_channel *chan, const char *data)
 {
-       return waitfor_exec(chan, data, 0);
+       return waitfor_exec(chan, data, &wait_for_noise);
 }
 
 static int unload_module(void)
@@ -272,6 +317,4 @@ static int load_module(void)
        return res;
 }
 
-AST_MODULE_INFO_STANDARD_EXTENDED(ASTERISK_GPL_KEY, "Wait For Silence");
-
-
+AST_MODULE_INFO_STANDARD_EXTENDED(ASTERISK_GPL_KEY, "Wait For Silence/Noise");