[asterisk-commits] jrose: branch jrose/mix-monitor-branch r309586 - in /team/jrose/mix-monitor-b...
SVN commits to the Asterisk project
asterisk-commits at lists.digium.com
Fri Mar 4 13:38:52 CST 2011
Author: jrose
Date: Fri Mar 4 13:38:50 2011
New Revision: 309586
URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=309586
Log:
all recent changes from code review
Modified:
team/jrose/mix-monitor-branch/apps/app_mixmonitor.c
team/jrose/mix-monitor-branch/main/audiohook.c
Modified: team/jrose/mix-monitor-branch/apps/app_mixmonitor.c
URL: http://svnview.digium.com/svn/asterisk/team/jrose/mix-monitor-branch/apps/app_mixmonitor.c?view=diff&rev=309586&r1=309585&r2=309586
==============================================================================
--- team/jrose/mix-monitor-branch/apps/app_mixmonitor.c (original)
+++ team/jrose/mix-monitor-branch/apps/app_mixmonitor.c Fri Mar 4 13:38:50 2011
@@ -116,7 +116,7 @@
<variable name="MIXMONITOR_FILENAME">
<para>Will contain the filename used to record.</para>
</variable>
- </variablelist>
+ </variablelist>
</description>
<see-also>
<ref type="application">Monitor</ref>
@@ -179,7 +179,7 @@
unsigned int flags;
struct ast_autochan *autochan;
struct mixmonitor_ds *mixmonitor_ds;
- };
+};
enum mixmonitor_flags {
MUXFLAG_APPEND = (1 << 1),
@@ -190,7 +190,7 @@
MUXFLAG_READ = (1 << 6),
MUXFLAG_WRITE = (1 << 7),
MUXFLAG_COMBINED = (1 << 8),
- };
+};
enum mixmonitor_args {
OPT_ARG_READVOLUME = 0,
@@ -199,10 +199,10 @@
OPT_ARG_WRITENAME,
OPT_ARG_READNAME,
OPT_ARG_ARRAY_SIZE, /* Always last element of the enum */
- };
+};
AST_APP_OPTIONS(mixmonitor_opts, {
- AST_APP_OPTION('a', MUXFLAG_APPEND),
+ AST_APP_OPTION('a', MUXFLAG_APPEND),
AST_APP_OPTION('b', MUXFLAG_BRIDGED),
AST_APP_OPTION_ARG('v', MUXFLAG_READVOLUME, OPT_ARG_READVOLUME),
AST_APP_OPTION_ARG('V', MUXFLAG_WRITEVOLUME, OPT_ARG_WRITEVOLUME),
@@ -256,7 +256,9 @@
ast_verb(2, "MixMonitor close filestream (write)\n");
}
- if (quitting) { mixmonitor_ds->fs_quit = 1; }
+ if (quitting) {
+ mixmonitor_ds->fs_quit = 1;
+ }
}
static void mixmonitor_ds_destroy(void *data)
@@ -289,7 +291,7 @@
ast_audiohook_destroy(&mixmonitor->audiohook);
}
-static int startmon(struct ast_channel *chan, struct ast_audiohook *audiohook)
+static int startmon(struct ast_channel *chan, struct ast_audiohook *audiohook)
{
struct ast_channel *peer = NULL;
int res = 0;
@@ -300,7 +302,7 @@
ast_audiohook_attach(chan, audiohook);
if (!res && ast_test_flag(chan, AST_FLAG_NBRIDGE) && (peer = ast_bridged_channel(chan)))
- ast_softhangup(peer, AST_SOFTHANGUP_UNBRIDGE);
+ ast_softhangup(peer, AST_SOFTHANGUP_UNBRIDGE);
return res;
}
@@ -316,6 +318,8 @@
ast_free(mixmonitor->filename_write);
ast_free(mixmonitor->filename_read);
ast_free(mixmonitor->mixmonitor_ds);
+ ast_free(mixmonitor->name);
+ ast_free(mixmonitor->post_process);
}
ast_free(mixmonitor);
}
@@ -323,32 +327,27 @@
static void mixmonitor_save_prep(struct mixmonitor *mixmonitor, char *filename, struct ast_filestream **fs, unsigned int *oflags, int *errflag)
{
- /* Initialize the file if not already done so */
- char *ext = NULL;
- if (!ast_strlen_zero(filename)) {
-
-
+ /* Initialize the file if not already done so */
+ char *ext = NULL;
+ if (!ast_strlen_zero(filename)) {
if (!*fs && !*errflag && !mixmonitor->mixmonitor_ds->fs_quit) {
*oflags = O_CREAT | O_WRONLY;
- *oflags |=
- ast_test_flag(mixmonitor, MUXFLAG_APPEND) ? O_APPEND : O_TRUNC;
+ *oflags |= ast_test_flag(mixmonitor, MUXFLAG_APPEND) ? O_APPEND : O_TRUNC;
if ((ext = strrchr(filename, '.')))
*(ext++) = '\0';
else
ext = "raw";
- if (!
- (*fs =
- ast_writefile(filename, ext, NULL, *oflags, 0, 0666))) {
- ast_log(LOG_ERROR, "Cannot open %s.%s\n", filename, ext);
- *errflag = 1;
- }
- }
- }
-}
-
-static void *mixmonitor_thread(void *obj)
+ if (!(*fs = ast_writefile(filename, ext, NULL, *oflags, 0, 0666))) {
+ ast_log(LOG_ERROR, "Cannot open %s.%s\n", filename, ext);
+ *errflag = 1;
+ }
+ }
+ }
+}
+
+static void *mixmonitor_thread(void *obj)
{
struct mixmonitor *mixmonitor = obj;
@@ -369,16 +368,13 @@
/* The audiohook must enter and exit the loop locked */
ast_audiohook_lock(&mixmonitor->audiohook);
-
while (mixmonitor->audiohook.status == AST_AUDIOHOOK_STATUS_RUNNING && !mixmonitor->mixmonitor_ds->fs_quit) {
struct ast_frame *fr = NULL;
struct ast_frame *fr_read = NULL;
struct ast_frame *fr_write = NULL;
- /* here we need to use a modified version of the ast_audiohook_read_frame function
- * designed to take our read and write frames as arguments. */
if (!(fr = ast_audiohook_read_frame_all(&mixmonitor->audiohook, SAMPLES_PER_FRAME, &format_slin,
- &fr_read, &fr_write))) {
+ &fr_read, &fr_write))) {
ast_audiohook_trigger_wait(&mixmonitor->audiohook);
if (mixmonitor->audiohook.status != AST_AUDIOHOOK_STATUS_RUNNING) {
@@ -415,17 +411,15 @@
}
}
- if ((*fs) && (fr)) {
- struct ast_frame *cur;
+ if ((*fs) && (fr)) {
+ struct ast_frame *cur;
for (cur = fr; cur; cur = AST_LIST_NEXT(cur, frame_list)) {
ast_writestream(*fs, cur);
}
}
-
ast_mutex_unlock(&mixmonitor->mixmonitor_ds->lock);
}
-
/* All done! free it. */
if (fr) {
ast_frame_free(fr, 0);
@@ -443,7 +437,6 @@
ast_audiohook_lock(&mixmonitor->audiohook);
}
-
ast_audiohook_unlock(&mixmonitor->audiohook);
ast_autochan_destroy(mixmonitor->autochan);
@@ -507,9 +500,6 @@
pthread_t thread;
struct mixmonitor *mixmonitor;
char postprocess2[1024] = "";
- size_t len;
-
- len = sizeof(*mixmonitor) + strlen(chan->name) + strlen(filename) + 2;
postprocess2[0] = 0;
/* If a post process system command is given attach it to the structure */
@@ -523,12 +513,10 @@
}
}
pbx_substitute_variables_helper(chan, p1, postprocess2, sizeof(postprocess2) - 1);
- if (!ast_strlen_zero(postprocess2))
- len += strlen(postprocess2) + 1;
}
/* Pre-allocate mixmonitor structure and spy */
- if (!(mixmonitor = ast_calloc(1, len))) {
+ if (!(mixmonitor = ast_calloc(1, sizeof(*mixmonitor)))) {
return;
}
@@ -550,14 +538,12 @@
mixmonitor_free(mixmonitor);
return;
}
- mixmonitor->name = (char *) mixmonitor + sizeof(*mixmonitor);
- strcpy(mixmonitor->name, chan->name);
+
+ mixmonitor->name = ast_strdup(chan->name);
+
if (!ast_strlen_zero(postprocess2)) {
- mixmonitor->post_process = mixmonitor->name + strlen(mixmonitor->name) + strlen(filename) + 2;
- strcpy(mixmonitor->post_process, postprocess2);
- }
-
- mixmonitor->filename = (char *) mixmonitor + sizeof(*mixmonitor) + strlen(chan->name) + 1;
+ mixmonitor->post_process = ast_strdup(postprocess2);
+ }
mixmonitor->filename = ast_strdup(filename);
mixmonitor->filename_write = ast_strdup(filename_write);
@@ -572,7 +558,7 @@
if (startmon(chan, &mixmonitor->audiohook)) {
ast_log(LOG_WARNING, "Unable to add '%s' spy to channel '%s'\n",
- mixmonitor_spy_type, chan->name);
+ mixmonitor_spy_type, chan->name);
ast_audiohook_destroy(&mixmonitor->audiohook);
mixmonitor_free(mixmonitor);
return;
@@ -581,7 +567,8 @@
ast_pthread_create_detached_background(&thread, NULL, mixmonitor_thread, mixmonitor);
}
-static char *filename_parse(char *filename)
+/* a note on filename_parse: creates directory structure and assigns absolute path from relative paths for filenames */
+static char *filename_parse(char *filename, char buffer[], int len)
{
char *tmp, *slash;
if (ast_strlen_zero(filename)) {
@@ -593,12 +580,14 @@
filename = build;
}
+ strncpy (buffer, filename, len - 1);
+
tmp = ast_strdupa(filename);
if ((slash = strrchr(tmp, '/')))
*slash = '\0';
ast_mkdir(tmp, 0777);
- return filename;
+ return buffer;
}
static int mixmonitor_exec(struct ast_channel *chan, const char *data)
@@ -607,12 +596,15 @@
char *filename_read = NULL;
char *filename_write = NULL;
+ const int buffer_len = 1024;
+ char filename_buffer[1024] = "";
+
struct ast_flags flags = { 0 };
char *parse;
AST_DECLARE_APP_ARGS(args,
- AST_APP_ARG(filename);
- AST_APP_ARG(options);
- AST_APP_ARG(post_process);
+ AST_APP_ARG(filename);
+ AST_APP_ARG(options);
+ AST_APP_ARG(post_process);
);
if (ast_strlen_zero(data)) {
@@ -653,36 +645,36 @@
if (ast_strlen_zero(opts[OPT_ARG_VOLUME])) {
ast_log(LOG_WARNING, "No volume level was provided for the combined volume ('W') option.\n");
} else if ((sscanf(opts[OPT_ARG_VOLUME], "%2d", &x) != 1) || (x < -4) || (x > 4)) {
- ast_log(LOG_NOTICE, "Combined volume must be a number between -4 and 4, not '%s'\n", opts[OPT_ARG_VOLUME]);
+ ast_log(LOG_NOTICE, "Combined volume must be a number between -4 and 4, not '%s'\n", opts[OPT_ARG_VOLUME]);
} else {
readvol = writevol = get_volfactor(x);
}
}
if (ast_test_flag(&flags, MUXFLAG_WRITE)) {
- filename_write = ast_strdupa(filename_parse(opts[OPT_ARG_WRITENAME]));
+ filename_write = ast_strdupa(filename_parse(opts[OPT_ARG_WRITENAME], filename_buffer, buffer_len));
}
if (ast_test_flag(&flags, MUXFLAG_READ)) {
- filename_read = ast_strdupa(filename_parse(opts[OPT_ARG_READNAME]));
- }
- }
-
+ filename_read = ast_strdupa(filename_parse(opts[OPT_ARG_READNAME], filename_buffer, buffer_len));
+ }
+ }
+
/* If there are no file writing arguments/options for the mix monitor, send a warning message and return -1 */
-
+
if (!ast_test_flag(&flags, MUXFLAG_WRITE) && !ast_test_flag(&flags, MUXFLAG_READ) && ast_strlen_zero(args.filename)) {
ast_log(LOG_WARNING, "MixMonitor requires an argument (filename)\n");
return -1;
}
-
+
/* If filename exists, try to create directories for it */
if (!(ast_strlen_zero(args.filename))) {
- args.filename = ast_strdupa(filename_parse(args.filename));
- }
-
+ args.filename = ast_strdupa(filename_parse(args.filename, filename_buffer, buffer_len));
+ }
+
pbx_builtin_setvar_helper(chan, "MIXMONITOR_FILENAME", args.filename);
launch_monitor_thread(chan, args.filename, flags.flags, readvol, writevol, args.post_process, filename_write, filename_read);
-
+
return 0;
}
@@ -698,7 +690,7 @@
ast_mutex_lock(&mixmonitor_ds->lock);
/* closing the filestream here guarantees the file is avaliable to the dialplan
- * after calling StopMixMonitor */
+ * after calling StopMixMonitor */
mixmonitor_ds_close_fs(mixmonitor_ds);
/* The mixmonitor thread may be waiting on the audiohook trigger.
@@ -771,7 +763,7 @@
const char *name = astman_get_header(m, "Channel");
const char *id = astman_get_header(m, "ActionID");
const char *state = astman_get_header(m, "State");
- const char *direction = astman_get_header(m, "Direction");
+ const char *direction = astman_get_header(m,"Direction");
int clearmute = 1;
@@ -784,9 +776,9 @@
if (!strcasecmp(direction, "read")) {
flag = AST_AUDIOHOOK_MUTE_READ;
- } else if (!strcasecmp(direction, "write")) {
+ } else if (!strcasecmp(direction, "write")) {
flag = AST_AUDIOHOOK_MUTE_WRITE;
- } else if (!strcasecmp(direction, "both")) {
+ } else if (!strcasecmp(direction, "both")) {
flag = AST_AUDIOHOOK_MUTE_READ | AST_AUDIOHOOK_MUTE_WRITE;
} else {
astman_send_error(s, m, "Invalid direction specified. Must be read, write or both");
@@ -858,4 +850,4 @@
return res;
}
-AST_MODULE_INFO_STANDARD(ASTERISK_GPL_KEY, "Mixed Audio Monitoring Application");
+AST_MODULE_INFO_STANDARD(ASTERISK_GPL_KEY, "Mixed Audio Monitoring Application");
Modified: team/jrose/mix-monitor-branch/main/audiohook.c
URL: http://svnview.digium.com/svn/asterisk/team/jrose/mix-monitor-branch/main/audiohook.c?view=diff&rev=309586&r1=309585&r2=309586
==============================================================================
--- team/jrose/mix-monitor-branch/main/audiohook.c (original)
+++ team/jrose/mix-monitor-branch/main/audiohook.c Fri Mar 4 13:38:50 2011
@@ -226,7 +226,7 @@
/* Ensure the factory is able to give us the samples we want */
if (samples > ast_slinfactory_available(factory))
return NULL;
-
+
/* Read data in from factory */
if (!ast_slinfactory_read(factory, buf, samples))
return NULL;
@@ -248,7 +248,7 @@
.datalen = sizeof(buf1),
.samples = samples,
};
- ast_format_set(&frame.subclass.format, AST_FORMAT_SLINEAR, 0);
+ ast_format_set(&frame.subclass.format, ast_format_slin_by_rate(audiohook->hook_internal_samp_rate), 0);
/* Make sure both factories have the required samples */
usable_read = (ast_slinfactory_available(&audiohook->read_factory) >= samples ? 1 : 0);
@@ -340,26 +340,45 @@
return ast_frdup(&frame);
}
-static struct ast_frame *ast_audiohook_read_frame_helper(struct ast_audiohook *audiohook, size_t samples, enum ast_audiohook_direction direction, struct ast_format *format, struct ast_frame **read_reference, struct ast_frame **write_reference)
+static struct ast_frame *audiohook_read_frame_helper(struct ast_audiohook *audiohook, size_t samples, enum ast_audiohook_direction direction, struct ast_format *format, struct ast_frame **read_reference, struct ast_frame **write_reference)
{
struct ast_frame *read_frame = NULL, *final_frame = NULL;
struct ast_format tmp_fmt;
-
- if (!(read_frame = (direction == AST_AUDIOHOOK_DIRECTION_BOTH ? audiohook_read_frame_both(audiohook, samples, read_reference, write_reference) : audiohook_read_frame_single(audiohook, samples, direction)))) { return NULL; }
+ int samples_converted;
+
+ /* the number of samples requested is based on the format they are requesting. Inorder
+ * to process this correctly samples must be converted to our internal sample rate */
+
+ if (audiohook->hook_internal_samp_rate == ast_format_rate(format)) {
+ samples_converted = samples;
+ } else if (audiohook->hook_internal_samp_rate > ast_format_rate(format)) {
+ samples_converted = samples * (audiohook->hook_internal_samp_rate / (float) ast_format_rate(format));
+ } else {
+ samples_converted = samples * (ast_format_rate(format) / (float) audiohook->hook_internal_samp_rate);
+ }
+
+ if (!(read_frame = (direction == AST_AUDIOHOOK_DIRECTION_BOTH ?
+ audiohook_read_frame_both(audiohook, samples_converted, read_reference, write_reference) :
+ audiohook_read_frame_single(audiohook, samples_converted, direction)))) {
+ return NULL;
+ }
+
/* If they don't want signed linear back out, we'll have to send it through the translation path */
- if (format->id != AST_FORMAT_SLINEAR) {
+ if (format->id != ast_format_slin_by_rate(audiohook->hook_internal_samp_rate)) {
/* Rebuild translation path if different format then previously */
if (ast_format_cmp(format, &audiohook->format) == AST_FORMAT_CMP_NOT_EQUAL) {
if (audiohook->trans_pvt) {
ast_translator_free_path(audiohook->trans_pvt);
audiohook->trans_pvt = NULL;
}
+
/* Setup new translation path for this format... if we fail we can't very well return signed linear so free the frame and return nothing */
- if (!(audiohook->trans_pvt = ast_translator_build_path(format, ast_format_set(&tmp_fmt, AST_FORMAT_SLINEAR, 0)))) {
+ if (!(audiohook->trans_pvt = ast_translator_build_path(format, ast_format_set(&tmp_fmt, ast_format_slin_by_rate(audiohook->hook_internal_samp_rate), 0)))) {
ast_frfree(read_frame);
return NULL;
}
+ ast_format_copy(&audiohook->format, format);
}
/* Convert to requested format, and allow the read in frame to be freed */
final_frame = ast_translate(audiohook->trans_pvt, read_frame, 1);
@@ -379,7 +398,7 @@
*/
struct ast_frame *ast_audiohook_read_frame(struct ast_audiohook *audiohook, size_t samples, enum ast_audiohook_direction direction, struct ast_format *format)
{
- return ast_audiohook_read_frame_helper(audiohook, samples, direction, format, NULL, NULL);
+ return audiohook_read_frame_helper(audiohook, samples, direction, format, NULL, NULL);
}
/*! \brief Reads a frame in from the audiohook structure
@@ -387,12 +406,13 @@
* \param samples Number of samples wanted
* \param direction Direction the audio frame came from
* \param format Format of frame remote side wants back
- * \param read_frame
+ * \param read_frame frame pointer for copying read frame data
+ * \param write_frame frame pointer for copying write frame data
* \return Returns frame on success, NULL on failure
*/
struct ast_frame *ast_audiohook_read_frame_all(struct ast_audiohook *audiohook, size_t samples, struct ast_format *format, struct ast_frame **read_frame, struct ast_frame **write_frame)
{
- return ast_audiohook_read_frame_helper(audiohook, samples, AST_AUDIOHOOK_DIRECTION_BOTH, format, read_frame, write_frame);
+ return audiohook_read_frame_helper(audiohook, samples, AST_AUDIOHOOK_DIRECTION_BOTH, format, read_frame, write_frame);
}
static void audiohook_list_set_samplerate_compatibility(struct ast_audiohook_list *audiohook_list)
@@ -516,7 +536,7 @@
if (audiohook_list->out_translate[i].trans_pvt)
ast_translator_free_path(audiohook_list->out_translate[i].trans_pvt);
}
-
+
/* Free ourselves */
ast_free(audiohook_list);
@@ -750,7 +770,7 @@
* because no translation to SLINEAR audio was required.
* Part_3: Translate end_frame's audio back into the format of start frame if necessary. This
* is only necessary if manipulation of middle_frame occurred.
- *
+ *
* \param chan Channel that the list is coming off of
* \param audiohook_list List of audiohooks
* \param direction Direction frame is coming in from
More information about the asterisk-commits
mailing list