[asterisk-commits] kharwell: branch 11 r432174 - /branches/11/bridges/bridge_softmix.c

[SVN commits to the Asterisk project]

Author: kharwell
Date: Tue Feb 24 12:22:58 2015
New Revision: 432174

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=432174
Log:
bridge_softmix: G.729 codec license held

When more than one call using the same codec type enters into a softmix bridge
and no audio is present for a channel the bridge optimizes the out frame by
using the same one for all channels with the same codec type. Unfortunately,
when that number (channels with same codec type) dropped to <= 1 the codec
was not dereferenced. At least not until all parties left the bridge. Thus in
the case of G.729 the license was not released. This patch ensures that the
codec is dereferenced immediately when the optimization no longer applies.

ASTERISK-24797 #close
Reported by: Luke Hulsey
Review: https://reviewboard.asterisk.org/r/4429/

Modified:
    branches/11/bridges/bridge_softmix.c

Modified: branches/11/bridges/bridge_softmix.c
URL: http://svnview.digium.com/svn/asterisk/branches/11/bridges/bridge_softmix.c?view=diff&rev=432174&r1=432173&r2=432174
==============================================================================
--- branches/11/bridges/bridge_softmix.c (original)
+++ branches/11/bridges/bridge_softmix.c Tue Feb 24 12:22:58 2015
@@ -163,6 +163,9 @@
 		return NULL;
 	}
 	ast_format_copy(&entry->dst_format, dst);
+	/* initialize this to one so that the first time through the cleanup code after
+	   allocation it won't be removed from the entry list */
+	entry->num_times_requested = 1;
 	return entry;
 }
 
@@ -250,11 +253,24 @@
 		for (i = 0; i < sc->write_frame.samples; i++) {
 			ast_slinear_saturated_subtract(&sc->final_buf[i], &sc->our_buf[i]);
 		}
+		/* check to see if any entries exist for the format. if not we'll want
+		   to remove it during cleanup */
+		AST_LIST_TRAVERSE(&trans_helper->entries, entry, entry) {
+			if (ast_format_cmp(&entry->dst_format, raw_write_fmt) == AST_FORMAT_CMP_EQUAL) {
+				++entry->num_times_requested;
+				break;
+			}
+		}
 		/* do not do any special write translate optimization if we had to make
 		 * a special mix for them to remove their own audio. */
 		return;
 	}
 
+	/* Attempt to optimize channels using the same translation path/codec. Build a list of entries
+	   of translation paths and track the number of references for each type. Each one of the same
+	   type should be able to use the same out_frame. Since the optimization is only necessary for
+	   multiple channels (>=2) using the same codec make sure resources are allocated only when
+	   needed and released when not (see also softmix_translate_helper_cleanup */
 	AST_LIST_TRAVERSE(&trans_helper->entries, entry, entry) {
 		if (ast_format_cmp(&entry->dst_format, raw_write_fmt) == AST_FORMAT_CMP_EQUAL) {
 			entry->num_times_requested++;
@@ -285,13 +301,32 @@
 static void softmix_translate_helper_cleanup(struct softmix_translate_helper *trans_helper)
 {
 	struct softmix_translate_helper_entry *entry = NULL;
-	AST_LIST_TRAVERSE(&trans_helper->entries, entry, entry) {
+	AST_LIST_TRAVERSE_SAFE_BEGIN(&trans_helper->entries, entry, entry) {
+		/* if it hasn't been requested then remove it */
+		if (!entry->num_times_requested) {
+			AST_LIST_REMOVE_CURRENT(entry);
+			softmix_translate_helper_free_entry(entry);
+			continue;
+		}
+
 		if (entry->out_frame) {
 			ast_frfree(entry->out_frame);
 			entry->out_frame = NULL;
 		}
+
+		/* nothing is optimized for a single path reference, so there is
+		   no reason to continue to hold onto the codec */
+		if (entry->num_times_requested == 1 && entry->trans_pvt) {
+			ast_translator_free_path(entry->trans_pvt);
+			entry->trans_pvt = NULL;
+		}
+
+		/* for each iteration (a mixing run) in the bridge softmix thread the number
+		   of references to a given entry is recalculated, so reset the number of
+		   times requested */
 		entry->num_times_requested = 0;
 	}
+	AST_LIST_TRAVERSE_SAFE_END;
 }
 
 static void softmix_bridge_data_destroy(void *obj)