merge rizzo's codec module rework (very similar to the format module rework)

git-svn-id: http://svn.digium.com/svn/asterisk/trunk@18541 f38db490-d61c-443f-a65b-d21fe96a405b

1 files changed, 369 insertions, 221 deletions

diff --git a/translate.c b/translate.c
index 69a2a2b59..b3af17d50 100644
--- a/translate.c
+++ b/translate.c
@@ -47,118 +47,243 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
 
 #define MAX_RECALC 200 /* max sample recalc */
 
-/*! \note
-   This could all be done more efficiently *IF* we chained packets together
-   by default, but it would also complicate virtually every application. */
-   
+/*! \brief the list of translators */
 static AST_LIST_HEAD_STATIC(translators, ast_translator);
 
-struct ast_translator_dir {
+struct translator_path {
 	struct ast_translator *step;	/*!< Next step translator */
 	unsigned int cost;		/*!< Complete cost to destination */
 	unsigned int multistep;		/*!< Multiple conversions required for this translation */
 };
 
-struct ast_frame_delivery {
-	struct ast_frame *f;
-	struct ast_channel *chan;
-	int fd;
-	struct translator_pvt *owner;
-	struct ast_frame_delivery *prev;
-	struct ast_frame_delivery *next;
-};
-
-static struct ast_translator_dir tr_matrix[MAX_FORMAT][MAX_FORMAT];
-
-struct ast_trans_pvt {
-	struct ast_translator *step;
-	struct ast_translator_pvt *state;
-	struct ast_trans_pvt *next;
-	struct timeval nextin;
-	struct timeval nextout;
-};
+/*! \brief a matrix that, for any pair of supported formats,
+ * indicates the total cost of translation and the first step.
+ * The full path can be reconstricted iterating on the matrix
+ * until step->dstfmt == desired_format.
+ */
+static struct translator_path tr_matrix[MAX_FORMAT][MAX_FORMAT];
 
+/*
+ * TODO: sample frames for each supported input format.
+ * We build this on the fly, by taking an SLIN frame and using
+ * the existing converter to play with it.
+ */
 
+/* returns the index of the lowest bit set */
 static int powerof(int d)
 {
 	int x;
-	for (x = 0; x < 32; x++)
+	for (x = 0; x < MAX_FORMAT; x++)
 		if ((1 << x) & d)
 			return x;
 	ast_log(LOG_WARNING, "Powerof %d: No power??\n", d);
 	return -1;
 }
 
+/*
+ * wrappers around the translator routines.
+ */
+
+/*!
+ * \brief Allocate the descriptor, required outbuf space,
+ * and possibly also plc and desc.
+ */
+static void *newpvt(struct ast_translator *t)
+{
+	struct ast_trans_pvt *pvt;
+	int len;
+	int useplc = t->plc_samples > 0 && t->useplc;	/* cache, because it can change on the fly */
+	char *ofs;
+	/*
+	 * compute the required size adding private descriptor,
+	 * plc, buffer, AST_FRIENDLY_OFFSET.
+	 */
+	len = sizeof(*pvt) + t->desc_size;
+	if (useplc)
+		len += sizeof(plc_state_t);
+	if (t->buf_size)
+		len += AST_FRIENDLY_OFFSET + t->buf_size;
+	pvt = ast_calloc(1, len);
+	if (!pvt)
+		return NULL;
+	pvt->t = t;
+	ofs = (char *)(pvt + 1);	/* pointer to data space */
+	if (t->desc_size) {		/* first comes the descriptor */
+		pvt->pvt = ofs;
+		ofs += t->desc_size;
+	}
+	if (useplc) {			/* then plc state */
+		pvt->plc = (plc_state_t *)ofs;
+		ofs += sizeof(plc_state_t);
+	}
+	if (t->buf_size)		/* finally buffer and header */
+		pvt->outbuf = ofs + AST_FRIENDLY_OFFSET;
+	/* call local init routine, if present */
+	if (t->newpvt && t->newpvt(pvt) == NULL) {
+		free(pvt);
+		return NULL;
+	}
+	ast_mutex_lock(&t->lockp->lock);
+	t->lockp->usecnt++;
+	ast_mutex_unlock(&t->lockp->lock);
+	ast_update_use_count();
+	return pvt;
+}
+
+static void destroy(struct ast_trans_pvt *pvt)
+{
+	struct ast_translator *t = pvt->t;
+
+	if (t->destroy)
+		t->destroy(pvt);
+	free(pvt);
+	ast_mutex_lock(&t->lockp->lock);
+	t->lockp->usecnt--;
+	ast_mutex_unlock(&t->lockp->lock);
+	ast_update_use_count();
+}
+
+/*
+ * framein wrapper, deals with plc and bound checks.
+ */
+static int framein(struct ast_trans_pvt *pvt, struct ast_frame *f)
+{
+	int16_t *dst = (int16_t *)pvt->outbuf;
+	int ret;
+	int samples = pvt->samples;	/* initial value */
+
+	if (f->samples == 0) {
+		ast_log(LOG_WARNING, "no samples for %s\n", pvt->t->name);
+	}
+	if (pvt->t->buffer_samples) {	/* do not pass empty frames to callback */
+		if (f->datalen == 0) { /* perform PLC with nominal framesize of 20ms/160 samples */
+			if (pvt->plc) {
+				int l = pvt->t->plc_samples;
+				if (pvt->samples + l > pvt->t->buffer_samples) {
+					ast_log(LOG_WARNING, "Out of buffer space\n");
+					return -1;
+				}
+				l = plc_fillin(pvt->plc, dst + pvt->samples, l);
+				pvt->samples += l;
+			}
+			return 0;
+		}
+		if (pvt->samples + f->samples > pvt->t->buffer_samples) {
+			ast_log(LOG_WARNING, "Out of buffer space\n");
+			return -1;
+		}
+	}
+	/* we require a framein routine, wouldn't know how to do
+	 * it otherwise.
+	 */
+	ret = pvt->t->framein(pvt, f);
+	/* possibly store data for plc */
+	if (!ret && pvt->plc) {
+		int l = pvt->t->plc_samples;
+		if (pvt->samples < l)
+			l = pvt->samples;
+		plc_rx(pvt->plc, dst + pvt->samples - l, l);
+	}
+	/* diagnostic ... */
+	if (pvt->samples == samples)
+		ast_log(LOG_WARNING, "%s did not update samples %d\n",
+			pvt->t->name, pvt->samples);
+        return ret;
+}
+
+/*
+ * generic frameout routine.
+ * If samples and datalen are 0, take whatever is in pvt
+ * and reset them, otherwise take the values in the caller and
+ * leave alone the pvt values.
+ */
+struct ast_frame *ast_trans_frameout(struct ast_trans_pvt *pvt,
+	int datalen, int samples)
+{
+	struct ast_frame *f = &pvt->f;
+
+        if (samples)
+		f->samples = samples;
+	else {
+		if (pvt->samples == 0)
+			return NULL;
+		f->samples = pvt->samples;
+		pvt->samples = 0;
+	}
+	if (datalen)
+		f->datalen = datalen;
+	else {
+		f->datalen = pvt->datalen;
+		pvt->datalen = 0;
+	}
+
+	f->frametype = AST_FRAME_VOICE;
+	f->subclass = 1 << (pvt->t->dstfmt);
+	f->mallocd = 0;
+	f->offset = AST_FRIENDLY_OFFSET;
+	f->src = pvt->t->name;
+	f->data = pvt->outbuf;
+	return f;
+}
+
+static struct ast_frame *default_frameout(struct ast_trans_pvt *pvt)
+{
+	return ast_trans_frameout(pvt, 0, 0);
+}
+
+/* end of callback wrappers and helpers */
+
 void ast_translator_free_path(struct ast_trans_pvt *p)
 {
-	struct ast_trans_pvt *pl, *pn;
-	pn = p;
-	while(pn) {
-		pl = pn;
-		pn = pn->next;
-		if (pl->state && pl->step->destroy)
-			pl->step->destroy(pl->state);
-		free(pl);
+	struct ast_trans_pvt *pn = p;
+	while ( (p = pn) ) {
+		pn = p->next;
+		destroy(p);
 	}
 }
 
-/*! Build a set of translators based upon the given source and destination formats */
+/*! Build a chain of translators based upon the given source and dest formats */
 struct ast_trans_pvt *ast_translator_build_path(int dest, int source)
 {
-	struct ast_trans_pvt *tmpr = NULL, *tmp = NULL;
+	struct ast_trans_pvt *head = NULL, *tail = NULL;
 	
 	source = powerof(source);
 	dest = powerof(dest);
 	
 	while (source != dest) {
-		if (!tr_matrix[source][dest].step) {
-			/* We shouldn't have allocated any memory */
+		struct ast_trans_pvt *cur;
+		struct ast_translator *t = tr_matrix[source][dest].step;
+		if (!t) {
 			ast_log(LOG_WARNING, "No translator path from %s to %s\n", 
 				ast_getformatname(source), ast_getformatname(dest));
 			return NULL;
 		}
-
-		if (tmp) {
-			tmp->next = ast_malloc(sizeof(*tmp));
-			tmp = tmp->next;
-		} else
-			tmp = ast_malloc(sizeof(*tmp));
-			
-		if (!tmp) {
-			if (tmpr)
-				ast_translator_free_path(tmpr);	
-			return NULL;
-		}
-
-		/* Set the root, if it doesn't exist yet... */
-		if (!tmpr)
-			tmpr = tmp;
-
-		tmp->next = NULL;
-		tmp->nextin = tmp->nextout = ast_tv(0, 0);
-		tmp->step = tr_matrix[source][dest].step;
-		tmp->state = tmp->step->newpvt();
-		
-		if (!tmp->state) {
+		if (!(cur = newpvt(t))) {
 			ast_log(LOG_WARNING, "Failed to build translator step from %d to %d\n", source, dest);
-			ast_translator_free_path(tmpr);	
+			if (head)
+				ast_translator_free_path(head);	
 			return NULL;
 		}
-		
+		if (!head)
+			head = cur;
+		else
+			tail->next = cur;
+		tail = cur;
+		cur->nextin = cur->nextout = ast_tv(0, 0);
 		/* Keep going if this isn't the final destination */
-		source = tmp->step->dstfmt;
+		source = cur->t->dstfmt;
 	}
-	return tmpr;
+	return head;
 }
 
+/*! \brief do the actual translation */
 struct ast_frame *ast_translate(struct ast_trans_pvt *path, struct ast_frame *f, int consume)
 {
-	struct ast_trans_pvt *p;
-	struct ast_frame *out;
+	struct ast_trans_pvt *p = path;
+	struct ast_frame *out = f;
 	struct timeval delivery;
-	p = path;
-	/* Feed the first frame into the first translator */
-	p->step->framein(p->state, f);
+
+	/* XXX hmmm... check this below */
 	if (!ast_tvzero(f->delivery)) {
 		if (!ast_tvzero(path->nextin)) {
 			/* Make sure this is in line with what we were expecting */
@@ -181,54 +306,45 @@ struct ast_frame *ast_translate(struct ast_trans_pvt *path, struct ast_frame *f,
 		path->nextin = ast_tvadd(path->nextin, ast_samp2tv(f->samples, 8000));
 	}
 	delivery = f->delivery;
+	for ( ; out && p ; p = p->next) {
+		framein(p, out);
+		out = p->t->frameout(p);
+	}
 	if (consume)
 		ast_frfree(f);
-	while(p) {
-		out = p->step->frameout(p->state);
-		/* If we get nothing out, return NULL */
-		if (!out)
-			return NULL;
-		/* If there is a next state, feed it in there.  If not,
-		   return this frame  */
-		if (p->next) 
-			p->next->step->framein(p->next->state, out);
-		else {
-			if (!ast_tvzero(delivery)) {
-				/* Regenerate prediction after a discontinuity */
-				if (ast_tvzero(path->nextout))
-					path->nextout = ast_tvnow();
-
-				/* Use next predicted outgoing timestamp */
-				out->delivery = path->nextout;
-				
-				/* Predict next outgoing timestamp from samples in this
-				   frame. */
-				path->nextout = ast_tvadd(path->nextout, ast_samp2tv( out->samples, 8000));
-			} else {
-				out->delivery = ast_tv(0, 0);
-			}
-			/* Invalidate prediction if we're entering a silence period */
-			if (out->frametype == AST_FRAME_CNG)
-				path->nextout = ast_tv(0, 0);
-			return out;
-		}
-		p = p->next;
+	if (out == NULL)
+		return NULL;
+	/* we have a frame, play with times */
+	if (!ast_tvzero(delivery)) {
+		/* Regenerate prediction after a discontinuity */
+		if (ast_tvzero(path->nextout))
+			path->nextout = ast_tvnow();
+
+		/* Use next predicted outgoing timestamp */
+		out->delivery = path->nextout;
+		
+		/* Predict next outgoing timestamp from samples in this
+		   frame. */
+		path->nextout = ast_tvadd(path->nextout, ast_samp2tv( out->samples, 8000));
+	} else {
+		out->delivery = ast_tv(0, 0);
 	}
-	ast_log(LOG_WARNING, "I should never get here...\n");
-	return NULL;
+	/* Invalidate prediction if we're entering a silence period */
+	if (out->frametype == AST_FRAME_CNG)
+		path->nextout = ast_tv(0, 0);
+	return out;
 }
 
-
-static void calc_cost(struct ast_translator *t, int samples)
+/*! \brief compute the cost of a single translation step */
+static void calc_cost(struct ast_translator *t, int seconds)
 {
 	int sofar=0;
-	struct ast_translator_pvt *pvt;
-	struct ast_frame *f, *out;
+	struct ast_trans_pvt *pvt;
 	struct timeval start;
 	int cost;
 
-	if(!samples)
-		samples = 1;
+	if (!seconds)
+		seconds = 1;
 	
 	/* If they don't make samples, give them a terrible score */
 	if (!t->sample) {
@@ -236,108 +352,114 @@ static void calc_cost(struct ast_translator *t, int samples)
 		t->cost = 99999;
 		return;
 	}
-	pvt = t->newpvt();
+	pvt = newpvt(t);
 	if (!pvt) {
 		ast_log(LOG_WARNING, "Translator '%s' appears to be broken and will probably fail.\n", t->name);
 		t->cost = 99999;
 		return;
 	}
 	start = ast_tvnow();
-	/* Call the encoder until we've processed one second of time */
-	while(sofar < samples * 8000) {
-		f = t->sample();
+	/* Call the encoder until we've processed the required number of samples */
+	while (sofar < seconds * 8000) {
+		struct ast_frame *f = t->sample();
 		if (!f) {
 			ast_log(LOG_WARNING, "Translator '%s' failed to produce a sample frame.\n", t->name);
-			t->destroy(pvt);
+			destroy(pvt);
 			t->cost = 99999;
 			return;
 		}
-		t->framein(pvt, f);
+		framein(pvt, f);
 		ast_frfree(f);
-		while((out = t->frameout(pvt))) {
-			sofar += out->samples;
-			ast_frfree(out);
+		while( (f = t->frameout(pvt))) {
+			sofar += f->samples;
+			ast_frfree(f);
 		}
 	}
 	cost = ast_tvdiff_ms(ast_tvnow(), start);
-	t->destroy(pvt);
-	t->cost = cost / samples;
+	destroy(pvt);
+	t->cost = cost / seconds;
 	if (!t->cost)
 		t->cost = 1;
 }
 
-/*! 
-  \brief Use the list of translators to build a translation matrix
-
-  \note This function expects the list of translators to be locked
+/*!
+ * \brief rebuild a translation matrix.
+ * \note This function expects the list of translators to be locked
 */
 static void rebuild_matrix(int samples)
 {
 	struct ast_translator *t;
-	int changed;
-	int x, y, z;
+	int x;	/* source format index */
+	int y;	/* intermediate format index */
+	int z;	/* destination format index */
 
 	if (option_debug)
 		ast_log(LOG_DEBUG, "Resetting translation matrix\n");
 
 	bzero(tr_matrix, sizeof(tr_matrix));
-
+	/* first, compute all direct costs */
 	AST_LIST_TRAVERSE(&translators, t, list) {
+		x = t->srcfmt;
+		z = t->dstfmt;
+
 		if (samples)
 			calc_cost(t, samples);
 	  
-		if (!tr_matrix[t->srcfmt][t->dstfmt].step ||
-		    tr_matrix[t->srcfmt][t->dstfmt].cost > t->cost) {
-			tr_matrix[t->srcfmt][t->dstfmt].step = t;
-			tr_matrix[t->srcfmt][t->dstfmt].cost = t->cost;
+		if (!tr_matrix[x][z].step || t->cost < tr_matrix[x][z].cost) {
+			tr_matrix[x][z].step = t;
+			tr_matrix[x][z].cost = t->cost;
 		}
 	}
-
-	do {
-		changed = 0;
-
-		/* Don't you just love O(N^3) operations? */
-		for (x = 0; x< MAX_FORMAT; x++) {			/* For each source format */
-			for (y = 0; y < MAX_FORMAT; y++) {		/* And each destination format */
-				if (x == y)				/* Except ourselves, of course */
+	/*
+	 * For each triple x, y, z of distinct formats, check if there is
+	 * a path from x to z through y which is cheaper than what is
+	 * currently known, and in case, update the matrix.
+	 * Repeat until the matrix is stable.
+	 */
+	for (;;) {
+		int changed = 0;
+		for (x=0; x < MAX_FORMAT; x++) {			/* source format */
+			for (y=0; y < MAX_FORMAT; y++) {	/* intermediate format */
+				if (x == y) 			/* skip ourselves */
 					continue;
 
-				for (z=0; z < MAX_FORMAT; z++) { 	/* And each format it might convert to */
-					if ((x == z) || (y == z))	/* Don't ever convert back to us */
-						continue;
+				for (z=0; z<MAX_FORMAT; z++) {	/* dst format */
+					int newcost;
 
-					if (tr_matrix[x][y].step &&	/* We can convert from x to y */
-					    tr_matrix[y][z].step &&	/* And from y to z and... */
-					    (!tr_matrix[x][z].step || 	/* Either there isn't an x->z conversion */
-					     (tr_matrix[x][y].cost + 
-					      tr_matrix[y][z].cost <	/* Or we're cheaper than the existing */
-					      tr_matrix[x][z].cost)	/* solution */
-						    )) {
-						/* We can get from x to z via y with a cost that
-						   is the sum of the transition from x to y and
-						   from y to z */
-						
-						tr_matrix[x][z].step = tr_matrix[x][y].step;
-						tr_matrix[x][z].cost = tr_matrix[x][y].cost + 
-							tr_matrix[y][z].cost;
-						tr_matrix[x][z].multistep = 1;
-						if (option_debug)
-							ast_log(LOG_DEBUG, "Discovered %d cost path from %s to %s, via %d\n", tr_matrix[x][z].cost, ast_getformatname(x), ast_getformatname(z), y);
-						changed++;
-					}
+					if (z == x || z == y)	/* skip null conversions */
+						continue;
+					if (!tr_matrix[x][y].step)	/* no path from x to y */
+						continue;
+					if (!tr_matrix[y][z].step)	/* no path from y to z */
+						continue;
+					newcost = tr_matrix[x][y].cost + tr_matrix[y][z].cost;
+					if (tr_matrix[x][z].step && newcost >= tr_matrix[x][z].cost)
+						continue;	/* x->y->z is more expensive than
+								 * the existing path */
+					/* ok, we can get from x to z via y with a cost that
+					   is the sum of the transition from x to y and
+					   from y to z */
+						 
+					tr_matrix[x][z].step = tr_matrix[x][y].step;
+					tr_matrix[x][z].cost = newcost;
+					tr_matrix[x][z].multistep = 1;
+					if (option_debug)
+						ast_log(LOG_DEBUG, "Discovered %d cost path from %s to %s, via %d\n", tr_matrix[x][z].cost, ast_getformatname(x), ast_getformatname(z), y);
+					changed++;
 				}
 			}
 		}
-	} while (changed);
+		if (!changed)
+			break;
+	}
 }
 
-
 /*! \brief CLI "show translation" command handler */
 static int show_translation(int fd, int argc, char *argv[])
 {
 #define SHOW_TRANS 11
 	int x, y, z;
-	char line[80];
+
 	if (argc > 4) 
 		return RESULT_SHOWUSAGE;
 
@@ -362,30 +484,30 @@ static int show_translation(int fd, int argc, char *argv[])
 	ast_cli(fd, "         Translation times between formats (in milliseconds)\n");
 	ast_cli(fd, "          Source Format (Rows) Destination Format(Columns)\n\n");
 	for (x = -1; x < SHOW_TRANS; x++) {
-		/* next 2 lines run faster than using strcpy() */
-		line[0] = ' ';
-		line[1] = '\0';
+		char line[80];
+		char *buf = line;
+		int left = sizeof(line) - 1;	/* one initial space */
+		/* next 2 lines run faster than using ast_build_string() */
+		*buf++ = ' ';
+		*buf = '\0';
 		for (y=-1;y<SHOW_TRANS;y++) {
-			if (x >= 0 && y >= 0 && tr_matrix[x][y].step)
-				snprintf(line + strlen(line), sizeof(line) - strlen(line), " %5d", tr_matrix[x][y].cost >= 99999 ? tr_matrix[x][y].cost-99999 : tr_matrix[x][y].cost);
-			else
-				if (((x == -1 && y >= 0) || (y == -1 && x >= 0))) {
-					snprintf(line + strlen(line), sizeof(line) - strlen(line), 
-						" %5s", ast_getformatname(1<<(x+y+1)) );
-				} else if (x != -1 && y != -1) {
-					snprintf(line + strlen(line), sizeof(line) - strlen(line), "     -");
-				} else {
-					snprintf(line + strlen(line), sizeof(line) - strlen(line), "      ");
-				}
+			if (x >= 0 && y >= 0 && tr_matrix[x][y].step)	/* XXX what is 99999 ? */
+				ast_build_string(&buf, &left, " %5d", tr_matrix[x][y].cost >= 99999 ? 0 : tr_matrix[x][y].cost);
+			else if (((x == -1 && y >= 0) || (y == -1 && x >= 0))) {
+				ast_build_string(&buf, &left, " %5s", ast_getformatname(1<<(x+y+1)) );
+			} else if (x != -1 && y != -1) {
+				ast_build_string(&buf, &left, "     -");
+			} else {
+				ast_build_string(&buf, &left, "      ");
+			}
 		}
-		snprintf(line + strlen(line), sizeof(line) - strlen(line), "\n");
+		ast_build_string(&buf, &left, "\n");
 		ast_cli(fd, line);			
 	}
 	AST_LIST_UNLOCK(&translators);
 	return RESULT_SUCCESS;
 }
 
-static int added_cli = 0;
 
 static char show_trans_usage[] =
 "Usage: show translation [recalc] [<recalc seconds>]\n"
@@ -399,9 +521,29 @@ static struct ast_cli_entry show_trans =
 
 int ast_register_translator(struct ast_translator *t)
 {
-	char tmp[80];
+	static int added_cli = 0;
+
+	if (t->lockp == NULL) {
+		ast_log(LOG_WARNING, "Missing lock pointer, you need to supply one\n");
+		return -1;
+	}
+	if (t->buf_size == 0) {
+		ast_log(LOG_WARNING, "empty buf size, you need to supply one\n");
+		return -1;
+	}
+	if (t->plc_samples) {
+		if (t->buffer_samples < t->plc_samples) {
+			ast_log(LOG_WARNING, "plc_samples %d buffer_samples %d\n",
+				t->plc_samples, t->buffer_samples);
+			return -1;
+		}
+		if (t->dstfmt != AST_FORMAT_SLINEAR)
+			ast_log(LOG_WARNING, "plc_samples %d format %x\n",
+				t->plc_samples, t->dstfmt);
+	}
 	t->srcfmt = powerof(t->srcfmt);
 	t->dstfmt = powerof(t->dstfmt);
+	/* XXX maybe check that it is not existing yet ? */
 	if (t->srcfmt >= MAX_FORMAT) {
 		ast_log(LOG_WARNING, "Source format %s is larger than MAX_FORMAT\n", ast_getformatname(t->srcfmt));
 		return -1;
@@ -410,9 +552,29 @@ int ast_register_translator(struct ast_translator *t)
 		ast_log(LOG_WARNING, "Destination format %s is larger than MAX_FORMAT\n", ast_getformatname(t->dstfmt));
 		return -1;
 	}
+	if (t->buf_size) {
+               /*
+		* Align buf_size properly, rounding up to the machine-specific
+		* alignment for pointers.
+		*/
+		struct _test_align { void *a, *b; } p;
+		int align = (char *)&p.b - (char *)&p.a;
+		t->buf_size = ((t->buf_size + align - 1)/align)*align;
+	}
+	if (t->lockp->usecnt < 0) {	/* XXX need to initialize the lock */
+		ast_mutex_init(&t->lockp->lock);
+		t->lockp->usecnt = 0;
+	}
+	if (t->frameout == NULL)
+		t->frameout = default_frameout;
+  
 	calc_cost(t,1);
-	if (option_verbose > 1)
-		ast_verbose(VERBOSE_PREFIX_2 "Registered translator '%s' from format %s to %s, cost %d\n", term_color(tmp, t->name, COLOR_MAGENTA, COLOR_BLACK, sizeof(tmp)), ast_getformatname(1 << t->srcfmt), ast_getformatname(1 << t->dstfmt), t->cost);
+	if (option_verbose > 1) {
+		char tmp[80];
+		ast_verbose(VERBOSE_PREFIX_2 "Registered translator '%s' from format %s to %s, cost %d\n",
+			term_color(tmp, t->name, COLOR_MAGENTA, COLOR_BLACK, sizeof(tmp)),
+			ast_getformatname(1 << t->srcfmt), ast_getformatname(1 << t->dstfmt), t->cost);
+	}
 	AST_LIST_LOCK(&translators);
 	if (!added_cli) {
 		ast_cli_register(&show_trans);
@@ -450,58 +612,44 @@ int ast_translator_best_choice(int *dst, int *srcs)
 	int x,y;
 	int best = -1;
 	int bestdst = 0;
-	int cur = 1;
+	int cur, cursrc;
 	int besttime = INT_MAX;
 	int beststeps = INT_MAX;
-	int common;
-
-	if ((common = (*dst) & (*srcs))) {
-		/* We have a format in common */
-		for (y = 0; y < MAX_FORMAT; y++) {
-			if (cur & common) {
-				/* This is a common format to both.  Pick it if we don't have one already */
-				bestdst = cur;
-				best = cur;
-			}
-			cur = cur << 1;
+	int common = (*dst) & (*srcs);	/* are there common formats ? */
+
+	if (common) { /* yes, pick one and return */
+		for (cur = 1, y=0; y < MAX_FORMAT; cur <<=1, y++) {
+			if (cur & common)	/* guaranteed to find one */
+				break;
 		}
-	} else {
-		/* We will need to translate */
+		/* We are done, this is a common format to both. */
+		*srcs = *dst  = cur;
+		return 0;
+	} else {	/* No, we will need to translate */
 		AST_LIST_LOCK(&translators);
-		for (y = 0; y < MAX_FORMAT; y++) {
-			if (!(cur & *dst)) {
-				cur = cur << 1;
+		for (cur = 1, y=0; y < MAX_FORMAT; cur <<=1, y++) {
+			if (! (cur & *dst))
 				continue;
-			}
-
-			for (x = 0; x < MAX_FORMAT; x++) {
-				if ((*srcs & (1 << x)) &&			/* x is a valid source format */
-				    tr_matrix[x][y].step) {			/* There's a step */
-					if (tr_matrix[x][y].cost > besttime)
-						continue;			/* It's more expensive, skip it */
-					
-					if (tr_matrix[x][y].cost == besttime &&
-					    tr_matrix[x][y].multistep >= beststeps)
-						continue; 			/* It requires the same (or more) steps,
-										   skip it */
-
-					/* It's better than what we have so far */
-					best = 1 << x;
+			for (cursrc = 1, x=0; x < MAX_FORMAT; cursrc <<= 1, x++) {
+				if (!(*srcs & cursrc) || !tr_matrix[x][y].step ||
+				    tr_matrix[x][y].cost >  besttime)
+					continue;	/* not existing or no better */
+				if (tr_matrix[x][y].cost < besttime ||
+				    tr_matrix[x][y].multistep < beststeps) {
+					/* better than what we have so far */
+					best = cursrc;
 					bestdst = cur;
 					besttime = tr_matrix[x][y].cost;
 					beststeps = tr_matrix[x][y].multistep;
 				}
 			}
-			cur = cur << 1;
 		}
 		AST_LIST_UNLOCK(&translators);
+		if (best > -1) {
+			*srcs = best;
+			*dst = bestdst;
+			best = 0;
+		}
+		return best;
 	}
-
-	if (best > -1) {
-		*srcs = best;
-		*dst = bestdst;
-		best = 0;
-	}
-
-	return best;
 }