[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