bridge_softmix: G.729 codec license held
authorKevin Harwell <kharwell@digium.com>
Tue, 24 Feb 2015 18:38:03 +0000 (18:38 +0000)
committerKevin Harwell <kharwell@digium.com>
Tue, 24 Feb 2015 18:38:03 +0000 (18:38 +0000)
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/
........

Merged revisions 432174 from http://svn.asterisk.org/svn/asterisk/branches/11
........

Merged revisions 432175 from http://svn.asterisk.org/svn/asterisk/branches/13

git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@432176 65c4cc65-6c06-0410-ace0-fbb531ad65f3

bridges/bridge_softmix.c

index 4d20b36..53be7d9 100644 (file)
@@ -181,6 +181,9 @@ static struct softmix_translate_helper_entry *softmix_translate_helper_entry_all
                return NULL;
        }
        entry->dst_format = ao2_bump(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;
 }
 
@@ -270,11 +273,24 @@ static void softmix_process_write_audio(struct softmix_translate_helper *trans_h
                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++;
@@ -306,13 +322,32 @@ static void softmix_translate_helper_cleanup(struct softmix_translate_helper *tr
 {
        struct softmix_translate_helper_entry *entry;
 
-       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 set_softmix_bridge_data(int rate, int interval, struct ast_bridge_channel *bridge_channel, int reset)