[Asterisk-code-review] app waitforsilence: Cleanup & don't treat missing frames as ... (asterisk[15])
Sean Bright
asteriskteam at digium.com
Wed Sep 6 16:16:02 CDT 2017
Sean Bright has uploaded this change for review. ( https://gerrit.asterisk.org/6456
Change subject: app_waitforsilence: Cleanup & don't treat missing frames as 'noise'
......................................................................
app_waitforsilence: Cleanup & don't treat missing frames as 'noise'
* 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
---
M apps/app_waitforsilence.c
1 file changed, 91 insertions(+), 48 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/56/6456/1
diff --git a/apps/app_waitforsilence.c b/apps/app_waitforsilence.c
index 5eaeace..52248ac 100644
--- a/apps/app_waitforsilence.c
+++ b/apps/app_waitforsilence.c
@@ -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>
@@ -101,7 +104,9 @@
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>
@@ -125,17 +130,32 @@
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;
- RAII_VAR(struct ast_format *, rfmt, NULL, ao2_cleanup);
- int res = 0;
- struct ast_dsp *sildet; /* silence detector dsp */
- time_t now;
+struct wait_type {
+ const char *name;
+ const char *status;
+ int stop_on_frame_timeout;
+ int (*func)(struct ast_dsp *, struct ast_frame *, int *);
+};
- /*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;
+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;
+ 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 @@
/* 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 @@
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 @@
}
}
-
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;
+ 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 (!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) {
- res = ast_answer(chan); /* Answer the channel */
+ ast_answer(chan); /* Answer the channel */
}
- 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");
- }
-
- 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 @@
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");
--
To view, visit https://gerrit.asterisk.org/6456
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 15
Gerrit-MessageType: newchange
Gerrit-Change-Id: I01d40adc5b63342bb5018a1bea2081a0aa191ef9
Gerrit-Change-Number: 6456
Gerrit-PatchSet: 1
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20170906/2f5bb792/attachment-0001.html>
More information about the asterisk-code-review
mailing list