<p>Jenkins2 <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/6457">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 5eaeace..52248ac 100644<br>--- a/apps/app_waitforsilence.c<br>+++ b/apps/app_waitforsilence.c<br>@@ -44,6 +44,7 @@<br> <br> #include "asterisk.h"<br> <br>+#include "asterisk/app.h"<br> #include "asterisk/file.h"<br> #include "asterisk/channel.h"<br> #include "asterisk/pbx.h"<br>@@ -57,7 +58,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>@@ -101,7 +104,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>@@ -125,17 +130,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>@@ -145,15 +165,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>@@ -162,34 +180,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>@@ -197,61 +217,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>@@ -272,6 +317,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/6457">change 6457</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/6457"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </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: 6457 </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>