<p>Jenkins2 <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/6451">View Change</a></p><div style="white-space:pre-wrap">Approvals:
  Joshua Colp: Looks good to me, but someone else must approve
  Kevin Harwell: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved
  Jenkins2: Approved for Submit

</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">app_waitforsilence: Cleanup & don't treat missing frames as 'noise'<br><br>* WaitForSilence completes successfully if it receives no media in the<br>  specified timeout, but when acting as WaitForNoise that logic needs<br>  to be reversed.<br><br>* Use standard argument parsing macros and add some error checking for<br>  invalid values.<br><br>* The documentation indicated that the first argument to both<br>  WaitForSilence and WaitForNoise was required when it was not. Update<br>  the documentation to reflect that.<br><br>* Wrap up some behavior in structs to avoid boolean checks all over the<br>  place.<br><br>ASTERISK-24066 #close<br>Reported by: M vd S<br><br>Change-Id: I01d40adc5b63342bb5018a1bea2081a0aa191ef9<br>---<br>M apps/app_waitforsilence.c<br>1 file changed, 91 insertions(+), 48 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/apps/app_waitforsilence.c b/apps/app_waitforsilence.c<br>index 07f2824..e56e49f 100644<br>--- a/apps/app_waitforsilence.c<br>+++ b/apps/app_waitforsilence.c<br>@@ -46,6 +46,7 @@<br> <br> ASTERISK_FILE_VERSION(__FILE__, "$Revision$")<br> <br>+#include "asterisk/app.h"<br> #include "asterisk/file.h"<br> #include "asterisk/channel.h"<br> #include "asterisk/pbx.h"<br>@@ -59,7 +60,9 @@<br>                     Waits for a specified amount of silence.<br>              </synopsis><br>             <syntax><br>-                       <parameter name="silencerequired" required="true" /><br>+                       <parameter name="silencerequired"><br>+                           <para>If not specified, defaults to <literal>1000</literal> milliseconds.</para><br>+                     </parameter><br>                    <parameter name="iterations"><br>                                 <para>If not specified, defaults to <literal>1</literal>.</para><br>                      </parameter><br>@@ -103,7 +106,9 @@<br>                       Waits for a specified amount of noise.<br>                </synopsis><br>             <syntax><br>-                       <parameter name="noiserequired" required="true" /><br>+                 <parameter name="noiserequired"><br>+                             <para>If not specified, defaults to <literal>1000</literal> milliseconds.</para><br>+                     </parameter><br>                    <parameter name="iterations"><br>                                 <para>If not specified, defaults to <literal>1</literal>.</para><br>                      </parameter><br>@@ -127,17 +132,32 @@<br> static char *app_silence = "WaitForSilence";<br> static char *app_noise = "WaitForNoise";<br> <br>-static int do_waiting(struct ast_channel *chan, int timereqd, time_t waitstart, int timeout, int wait_for_silence) {<br>-    struct ast_frame *f = NULL;<br>-  int dsptime = 0;<br>-     RAII_VAR(struct ast_format *, rfmt, NULL, ao2_cleanup);<br>-      int res = 0;<br>- struct ast_dsp *sildet;  /* silence detector dsp */<br>-  time_t now;<br>+struct wait_type {<br>+     const char *name;<br>+    const char *status;<br>+  int stop_on_frame_timeout;<br>+   int (*func)(struct ast_dsp *, struct ast_frame *, int *);<br>+};<br> <br>-    /*Either silence or noise calc depending on wait_for_silence flag*/<br>-  int (*ast_dsp_func)(struct ast_dsp*, struct ast_frame*, int*) =<br>-                              wait_for_silence ? ast_dsp_silence : ast_dsp_noise;<br>+static const struct wait_type wait_for_silence = {<br>+     .name = "silence",<br>+ .status = "SILENCE",<br>+       .stop_on_frame_timeout = 1,<br>+  .func = ast_dsp_silence,<br>+};<br>+<br>+static const struct wait_type wait_for_noise = {<br>+  .name = "noise",<br>+   .status = "NOISE",<br>+ .stop_on_frame_timeout = 0,<br>+  .func = ast_dsp_noise,<br>+};<br>+<br>+static int do_waiting(struct ast_channel *chan, int timereqd, time_t waitstart, int timeout, const struct wait_type *wait_for)<br>+{<br>+  RAII_VAR(struct ast_format *, rfmt, NULL, ao2_cleanup);<br>+      int res;<br>+     struct ast_dsp *sildet;<br> <br>    rfmt = ao2_bump(ast_channel_readformat(chan));<br>        if ((res = ast_set_read_format(chan, ast_format_slin)) < 0) {<br>@@ -147,15 +167,13 @@<br> <br>    /* Create the silence detector */<br>     if (!(sildet = ast_dsp_new())) {<br>-             ast_log(LOG_WARNING, "Unable to create silence detector :(\n");<br>+            ast_log(LOG_WARNING, "Unable to create silence detector\n");<br>                return -1;<br>    }<br>     ast_dsp_set_threshold(sildet, ast_dsp_get_threshold_from_settings(THRESHOLD_SILENCE));<br> <br>-    /* Await silence... */<br>        for (;;) {<br>-           /* Start with no silence received */<br>-         dsptime = 0;<br>+         int dsptime = 0;<br> <br>           res = ast_waitfor(chan, timereqd);<br> <br>@@ -164,34 +182,36 @@<br>                  pbx_builtin_setvar_helper(chan, "WAITSTATUS", "HANGUP");<br>                  break;<br>                }<br>-            <br>+<br>           /* We waited and got no frame; sounds like digital silence or a muted digital channel */<br>              if (res == 0) {<br>-                      dsptime = timereqd;<br>+                  if (wait_for->stop_on_frame_timeout) {<br>+                            dsptime = timereqd;<br>+                  }<br>             } else {<br>                      /* Looks like we did get a frame, so let's check it out */<br>-                       if (!(f = ast_read(chan))) {<br>+                 struct ast_frame *f = ast_read(chan);<br>+                        if (!f) {<br>                             pbx_builtin_setvar_helper(chan, "WAITSTATUS", "HANGUP");<br>                          break;<br>                        }<br>                     if (f->frametype == AST_FRAME_VOICE) {<br>-                            ast_dsp_func(sildet, f, &dsptime);<br>+                               wait_for->func(sildet, f, &dsptime);<br>                   }<br>                     ast_frfree(f);<br>                }<br> <br>-         ast_debug(1, "Got %dms %s < %dms required\n", dsptime, wait_for_silence ? "silence" : "noise", timereqd);<br>+           ast_debug(1, "Got %dms of %s < %dms required\n", dsptime, wait_for->name, timereqd);<br> <br>               if (dsptime >= timereqd) {<br>-                        ast_verb(3, "Exiting with %dms %s >= %dms required\n", dsptime, wait_for_silence ? "silence" : "noise", timereqd);<br>-                  /* Ended happily with silence */<br>+                     ast_verb(3, "Exiting with %dms of %s >= %dms required\n", dsptime, wait_for->name, timereqd);<br>+                        pbx_builtin_setvar_helper(chan, "WAITSTATUS", wait_for->status);<br>+                        ast_debug(1, "WAITSTATUS was set to %s\n", wait_for->status);<br>                    res = 1;<br>-                     pbx_builtin_setvar_helper(chan, "WAITSTATUS", wait_for_silence ? "SILENCE" : "NOISE");<br>-                 ast_debug(1, "WAITSTATUS was set to %s\n", wait_for_silence ? "SILENCE" : "NOISE");<br>                     break;<br>                }<br> <br>-         if (timeout && (difftime(time(&now), waitstart) >= timeout)) {<br>+                if (timeout && difftime(time(NULL), waitstart) >= timeout) {<br>                       pbx_builtin_setvar_helper(chan, "WAITSTATUS", "TIMEOUT");<br>                         ast_debug(1, "WAITSTATUS was set to TIMEOUT\n");<br>                    res = 0;<br>@@ -199,61 +219,86 @@<br>               }<br>     }<br> <br>-<br>       if (rfmt && ast_set_read_format(chan, rfmt)) {<br>                ast_log(LOG_WARNING, "Unable to restore format %s to channel '%s'\n", ast_format_get_name(rfmt), ast_channel_name(chan));<br>   }<br>+<br>  ast_dsp_free(sildet);<br>         return res;<br> }<br> <br>-static int waitfor_exec(struct ast_channel *chan, const char *data, int wait_for_silence)<br>+static int waitfor_exec(struct ast_channel *chan, const char *data, const struct wait_type *wait_for)<br> {<br>    int res = 1;<br>  int timereqd = 1000;<br>  int timeout = 0;<br>      int iterations = 1, i;<br>        time_t waitstart;<br>+    char *parse;<br>  struct ast_silence_generator *silgen = NULL;<br> <br>+      AST_DECLARE_APP_ARGS(args,<br>+           AST_APP_ARG(timereqd);<br>+               AST_APP_ARG(iterations);<br>+             AST_APP_ARG(timeout);<br>+        );<br>+<br>+        parse = ast_strdupa(data);<br>+   AST_STANDARD_APP_ARGS(args, parse);<br>+<br>+       if (!ast_strlen_zero(args.timereqd)) {<br>+               if (sscanf(args.timereqd, "%30d", &timereqd) != 1 || timereqd < 0) {<br>+                        ast_log(LOG_ERROR, "Argument '%srequired' must be an integer greater than or equal to zero.\n",<br>+                                    wait_for->name);<br>+                  return -1;<br>+           }<br>+    }<br>+<br>+ if (!ast_strlen_zero(args.iterations)) {<br>+             if (sscanf(args.iterations, "%30d", &iterations) != 1 || iterations < 1) {<br>+                  ast_log(LOG_ERROR, "Argument 'iterations' must be an integer greater than 0.\n");<br>+                  return -1;<br>+           }<br>+    }<br>+<br>+ if (!ast_strlen_zero(args.timeout)) {<br>+                if (sscanf(args.timeout, "%30d", &timeout) != 1 || timeout < 0) {<br>+                   ast_log(LOG_ERROR, "Argument 'timeout' must be an integer greater than or equal to zero.\n");<br>+                      return -1;<br>+           }<br>+    }<br>+<br>  if (ast_channel_state(chan) != AST_STATE_UP) {<br>-               res = ast_answer(chan); /* Answer the channel */<br>+             ast_answer(chan); /* Answer the channel */<br>    }<br> <br>- if (!data || ( (sscanf(data, "%30d,%30d,%30d", &timereqd, &iterations, &timeout) != 3) &&<br>-              (sscanf(data, "%30d,%30d", &timereqd, &iterations) != 2) &&<br>-                (sscanf(data, "%30d", &timereqd) != 1) ) ) {<br>-           ast_log(LOG_WARNING, "Using default value of 1000ms, 1 iteration, no timeout\n");<br>-  }<br>-<br>- ast_verb(3, "Waiting %d time(s) for %d ms silence with %d timeout\n", iterations, timereqd, timeout);<br>+      ast_verb(3, "Waiting %d time(s) for %dms of %s with %ds timeout\n",<br>+                         iterations, timereqd, wait_for->name, timeout);<br> <br>        if (ast_opt_transmit_silence) {<br>               silgen = ast_channel_start_silence_generator(chan);<br>   }<br>+<br>  time(&waitstart);<br>-        res = 1;<br>-     for (i=0; (i<iterations) && (res == 1); i++) {<br>-            res = do_waiting(chan, timereqd, waitstart, timeout, wait_for_silence);<br>+      for (i = 0; i < iterations && res == 1; i++) {<br>+            res = do_waiting(chan, timereqd, waitstart, timeout, wait_for);<br>       }<br>+<br>  if (silgen) {<br>                 ast_channel_stop_silence_generator(chan, silgen);<br>     }<br> <br>-<br>-      if (res > 0)<br>-              res = 0;<br>-     return res;<br>+  return res > 0 ? 0 : res;<br> }<br> <br> static int waitforsilence_exec(struct ast_channel *chan, const char *data)<br> {<br>- return waitfor_exec(chan, data, 1);<br>+  return waitfor_exec(chan, data, &wait_for_silence);<br> }<br> <br> static int waitfornoise_exec(struct ast_channel *chan, const char *data)<br> {<br>-        return waitfor_exec(chan, data, 0);<br>+  return waitfor_exec(chan, data, &wait_for_noise);<br> }<br> <br> static int unload_module(void)<br>@@ -274,6 +319,4 @@<br>    return res;<br> }<br> <br>-AST_MODULE_INFO_STANDARD_EXTENDED(ASTERISK_GPL_KEY, "Wait For Silence");<br>-<br>-<br>+AST_MODULE_INFO_STANDARD_EXTENDED(ASTERISK_GPL_KEY, "Wait For Silence/Noise");<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/6451">change 6451</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/6451"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 13 </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: I01d40adc5b63342bb5018a1bea2081a0aa191ef9 </div>
<div style="display:none"> Gerrit-Change-Number: 6451 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Sean Bright <sean.bright@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>