[asterisk-commits] dvossel: branch dvossel/hd_confbridge r310239 - /team/dvossel/hd_confbridge/b...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Thu Mar 10 09:35:48 CST 2011


Author: dvossel
Date: Thu Mar 10 09:35:44 2011
New Revision: 310239

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=310239
Log:
Improving comments and code clarity in the softmix thread code.

Modified:
    team/dvossel/hd_confbridge/bridges/bridge_softmix.c

Modified: team/dvossel/hd_confbridge/bridges/bridge_softmix.c
URL: http://svnview.digium.com/svn/asterisk/team/dvossel/hd_confbridge/bridges/bridge_softmix.c?view=diff&rev=310239&r1=310238&r2=310239
==============================================================================
--- team/dvossel/hd_confbridge/bridges/bridge_softmix.c (original)
+++ team/dvossel/hd_confbridge/bridges/bridge_softmix.c Thu Mar 10 09:35:44 2011
@@ -461,17 +461,32 @@
 		unsigned int best_rate = stats->highest_supported_rate;
 		int best_index = -1;
 
-		/* 1. pick the best sample rate two or more channels support
-		 * 2. if two or more channels do not support the same rate, pick the
-		 * lowest sample rate that is still above the internal rate. */
-		for (i = 0; ((i < ARRAY_LEN(stats->num_channels)) && stats->num_channels[i]); i++) {
-			if ((stats->num_channels[i] >= 2 && (best_index == -1)) ||
-				((best_index != -1) &&
+		for (i = 0; i < ARRAY_LEN(stats->num_channels); i++) {
+			if (stats->num_channels[i]) {
+				break;
+			}
+			/* best_rate starts out being the first sample rate
+			 * greater than the internal sample rate that 2 or
+			 * more channels support. */
+			if (stats->num_channels[i] >= 2 && (best_index == -1)) {
+				best_rate = stats->sample_rates[i];
+				best_index = i;
+			/* If it has been detected that multiple rates above
+			 * the internal rate are present, compare those rates
+			 * to each other and pick the highest one two or more
+			 * channels support. */
+			} else if (((best_index != -1) &&
 				(stats->num_channels[i] >= 2) &&
 				(stats->sample_rates[best_index] < stats->sample_rates[i]))) {
-
 				best_rate = stats->sample_rates[i];
 				best_index = i;
+			/* It is possible that multiple channels exist with native sample
+			 * rates above the internal sample rate, but none of those channels
+			 * have the same rate in common.  In this case, the lowest sample
+			 * rate among those channels is picked. Over time as additional
+			 * statistic runs are made the internal sample rate number will
+			 * adjust to the most optimal sample rate, but it may take multiple
+			 * iterations. */
 			} else if (best_index == -1) {
 				best_rate = MIN(best_rate, stats->sample_rates[i]);
 			}
@@ -481,7 +496,7 @@
 		bridge_data->internal_rate = best_rate;
 		return 1;
 	} else if (!stats->num_at_internal_rate && !stats->num_above_internal_rate) {
-		/* in this case, the highest supported rate is actually lower than the internal rate */
+		/* In this case, the highest supported rate is actually lower than the internal rate */
 		bridge_data->internal_rate = stats->highest_supported_rate;
 		ast_debug(1, " Bridge changed from %d to %d\n", bridge_data->internal_rate, stats->highest_supported_rate);
 		return 1;
@@ -540,16 +555,14 @@
 	ao2_ref(bridge_data, 1);
 	timer = bridge_data->timer;
 	timingfd = ast_timer_fd(timer);
-
-	/* give it room to grow, memory is cheap but allocations are expensive */
+	softmix_translate_helper_init(&trans_helper, bridge_data->internal_rate);
+	ast_timer_set_rate(timer, (1000 / SOFTMIX_INTERVAL));
+
+	/* Give the mixing array room to grow, memory is cheap but allocations are expensive. */
 	if (softmix_mixing_array_init(&mixing_array, bridge->num + 10)) {
 		ast_log(LOG_NOTICE, "Failed to allocate softmix mixing structure. \n");
 		goto softmix_cleanup;
 	}
-
-	softmix_translate_helper_init(&trans_helper, bridge_data->internal_rate);
-
-	ast_timer_set_rate(timer, (1000 / SOFTMIX_INTERVAL));
 
 	while (!bridge->stop && !bridge->refresh && bridge->array_num) {
 		struct ast_bridge_channel *bridge_channel = NULL;
@@ -559,25 +572,30 @@
 		unsigned int softmix_datalen = SOFTMIX_DATALEN(bridge_data->internal_rate);
 
 		if (softmix_datalen > MAX_DATALEN) {
-			/* This should NEVER happen, but if it does we need to know about it. */
+			/* This should NEVER happen, but if it does we need to know about it. Almost
+			 * all the memcpys used during this process depend on this assumption.  Rather
+			 * than checking this over and over again through out the code, this single
+			 * verification is done on each iteration. */
 			ast_log(LOG_WARNING, "Conference mixing error, requested mixing length greater than mixing buffer.\n");
 			goto softmix_cleanup;
 		}
-		/* If the number of participants in the bridge has changed, the mixing array
-		 * buffer must realloc */
+
+		/* Grow the mixing array buffer as participants are added. */
 		if (mixing_array.max_num_entries < bridge->num && softmix_mixing_array_grow(&mixing_array, bridge->num + 5)) {
 			goto softmix_cleanup;
 		}
+
+		/* init the number of buffers stored in the mixing array to 0.
+		 * As buffers are added for mixing, this number is incremented. */
 		mixing_array.used_entries = 0;
 
-		/* these variables help determine if a rate change is required */
+		/* These variables help determine if a rate change is required */
 		if (!stat_iteration_counter) {
 			memset(&stats, 0, sizeof(stats));
-			/* If the bridge has an internal sample rate set, we must lock in on that rate */
 			stats.locked_rate = bridge->internal_sample_rate;
 		}
 
-		/* if the sample rate has changed, update the translator helper */
+		/* If the sample rate has changed, update the translator helper */
 		if (update_all_rates) {
 			softmix_translate_helper_change_rate(&trans_helper, bridge_data->internal_rate);
 		}
@@ -591,24 +609,23 @@
 				set_softmix_bridge_data(bridge_data->internal_rate, bridge_channel, 1);
 			}
 
+			/* If stat_iteration_counter is 0, then collect statistics during this mixing interation */
 			if (!stat_iteration_counter) {
 				gather_softmix_stats(&stats, bridge_data, bridge_channel);
 			}
 
-			/* if the channel is suspended, don't check for audio, but
-			 * do continue to collect statistics */
+			/* if the channel is suspended, don't check for audio, but still gather stats */
 			if (bridge_channel->suspended) {
 				continue;
 			}
 
+			/* Try to get audio from the factory if available */
 			ast_mutex_lock(&sc->lock);
 			sc->have_audio = 0;
-			/* Try to get audio from the factory if available */
 			if ((ast_slinfactory_available(&sc->factory) >= softmix_samples) &&
 				ast_slinfactory_read(&sc->factory, sc->our_buf, softmix_samples)) {
 				mixing_array.buffers[mixing_array.used_entries] = sc->our_buf;
 				mixing_array.used_entries++;
-				/* Yay we have our own audio */
 				sc->have_audio = 1;
 			}
 			ast_mutex_unlock(&sc->lock);
@@ -642,8 +659,10 @@
 
 			/* process the softmix channel's new write audio */
 			softmix_process_write_audio(&trans_helper, &bridge_channel->chan->rawwriteformat, sc);
+
 			/* The frame is now ready for use... */
 			sc->have_frame = 1;
+
 			ast_mutex_unlock(&sc->lock);
 
 			/* Poke bridged channel thread just in case */




More information about the asterisk-commits mailing list