<p>Richard Mudgett has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/8308">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">bridge_simple.c: Fix stream topology handling.<br><br>The handling of stream topologies was not protected by channel locks in<br>simple_bridge_request_stream_topology_change().<br><br>* Fixed topology handling to be protected by channel locks where needed in<br>simple_bridge_request_stream_topology_change().<br><br>ASTERISK-27692<br><br>Change-Id: Ica5d78a6c7ecf4f0b95fb16de28d3889b32c4776<br>---<br>M bridges/bridge_simple.c<br>1 file changed, 53 insertions(+), 33 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/08/8308/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/bridges/bridge_simple.c b/bridges/bridge_simple.c<br>index 7ee1966..be1ec43 100644<br>--- a/bridges/bridge_simple.c<br>+++ b/bridges/bridge_simple.c<br>@@ -114,18 +114,34 @@<br> };<br> <br> static void simple_bridge_request_stream_topology_change(struct ast_channel *chan,<br>-   struct ast_stream_topology *requested_topology)<br>+      struct ast_channel *requested_chan)<br> {<br>-      struct ast_stream_topology *existing_topology = ast_channel_get_stream_topology(chan);<br>+       struct ast_stream_topology *existing_topology;<br>+       struct ast_stream_topology *requested_topology;<br>       struct ast_stream *stream;<br>    struct ast_format_cap *audio_formats = NULL;<br>  struct ast_stream_topology *new_topology;<br>     int i;<br> <br>+    /*<br>+    * Sigh.  We have to clone to avoid the request_chan's current topology<br>+   * going away on us because we have to give up the lock and topology<br>+  * is not an ao2 object.<br>+      */<br>+  requested_topology = ast_channel_get_stream_topology(requested_chan);<br>+        new_topology = ast_stream_topology_clone(requested_topology);<br>+        ast_channel_unlock(requested_chan);<br>+  if (!new_topology) {<br>+         /* Complete failure.  We'll just have to live with it. */<br>+                ast_channel_unlock(chan);<br>+            return;<br>+      }<br>+<br>  /* We find an existing stream with negotiated audio formats that we can place into<br>     * any audio streams in the new topology to ensure that negotiation succeeds. Some<br>     * endpoints incorrectly terminate the call if SDP negotiation fails.<br>          */<br>+  existing_topology = ast_channel_get_stream_topology(chan);<br>    for (i = 0; i < ast_stream_topology_get_count(existing_topology); ++i) {<br>           stream = ast_stream_topology_get_stream(existing_topology, i);<br> <br>@@ -134,63 +150,67 @@<br>                      continue;<br>             }<br> <br>-         audio_formats = ast_stream_get_formats(stream);<br>+              audio_formats = ao2_bump(ast_stream_get_formats(stream));<br>             break;<br>        }<br> <br>- if (!audio_formats) {<br>-                ast_channel_request_stream_topology_change(chan, requested_topology, &simple_bridge);<br>-            return;<br>-      }<br>+    ast_channel_unlock(chan);<br> <br>- new_topology = ast_stream_topology_clone(requested_topology);<br>-        if (!new_topology) {<br>-         ast_channel_request_stream_topology_change(chan, requested_topology, &simple_bridge);<br>-            return;<br>-      }<br>+    if (audio_formats) {<br>+         for (i = 0; i < ast_stream_topology_get_count(new_topology); ++i) {<br>+                       stream = ast_stream_topology_get_stream(new_topology, i);<br> <br>- for (i = 0; i < ast_stream_topology_get_count(new_topology); ++i) {<br>-               stream = ast_stream_topology_get_stream(new_topology, i);<br>+                    if (ast_stream_get_type(stream) != AST_MEDIA_TYPE_AUDIO ||<br>+                           ast_stream_get_state(stream) == AST_STREAM_STATE_REMOVED) {<br>+                          continue;<br>+                    }<br> <br>-         if (ast_stream_get_type(stream) != AST_MEDIA_TYPE_AUDIO ||<br>-                   ast_stream_get_state(stream) == AST_STREAM_STATE_REMOVED) {<br>-                  continue;<br>+                    ast_format_cap_append_from_cap(ast_stream_get_formats(stream), audio_formats,<br>+                                AST_MEDIA_TYPE_AUDIO);<br>                }<br>-<br>-         ast_format_cap_append_from_cap(ast_stream_get_formats(stream), audio_formats, AST_MEDIA_TYPE_AUDIO);<br>+         ao2_ref(audio_formats, -1);<br>   }<br> <br>  ast_channel_request_stream_topology_change(chan, new_topology, &simple_bridge);<br>-<br>        ast_stream_topology_free(new_topology);<br> }<br> <br> static void simple_bridge_stream_topology_changed(struct ast_bridge *bridge,<br>                 struct ast_bridge_channel *bridge_channel)<br> {<br>-       struct ast_channel *c0 = AST_LIST_FIRST(&bridge->channels)->chan;<br>-  struct ast_channel *c1 = AST_LIST_LAST(&bridge->channels)->chan;<br>-   struct ast_stream_topology *t0 = ast_channel_get_stream_topology(c0);<br>-        struct ast_stream_topology *t1 = ast_channel_get_stream_topology(c1);<br>+        struct ast_channel *c0;<br>+      struct ast_channel *c1;<br>+      struct ast_stream_topology *t0;<br>+      struct ast_stream_topology *t1;<br> <br>    if (bridge_channel) {<br>                 ast_bridge_channel_stream_map(bridge_channel);<br>+<br>+            if (ast_channel_get_stream_topology_change_source(bridge_channel->chan)<br>+                   == &simple_bridge) {<br>+                     return;<br>+              }<br>     }<br>-    /*<br>-    * The bridge_channel should only be NULL after both channels join<br>-    * the bridge and their topologies are being aligned.<br>-         */<br>-  if (bridge_channel && ast_channel_get_stream_topology_change_source(<br>-             bridge_channel->chan) == &simple_bridge) {<br>+<br>+     c0 = AST_LIST_FIRST(&bridge->channels)->chan;<br>+      c1 = AST_LIST_LAST(&bridge->channels)->chan;<br>+       if (c0 == c1) {<br>+              /*<br>+            * Wait until there is another channel in the bridge<br>+          * to align topologies.<br>+               */<br>           return;<br>       }<br> <br>  /* Align topologies according to size or first channel to join */<br>+    ast_channel_lock_both(c0, c1);<br>+       t0 = ast_channel_get_stream_topology(c0);<br>+    t1 = ast_channel_get_stream_topology(c1);<br>     if (ast_stream_topology_get_count(t0) < ast_stream_topology_get_count(t1)) {<br>-              simple_bridge_request_stream_topology_change(c0, t1);<br>-        } else {<br>-             simple_bridge_request_stream_topology_change(c1, t0);<br>+                SWAP(c0, c1);<br>         }<br>+    simple_bridge_request_stream_topology_change(c1, c0);<br>+        /* c0 and c1 are unlocked by simple_bridge_request_stream_topology_change() */<br> }<br> <br> static int unload_module(void)<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/8308">change 8308</a>. To unsubscribe, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/8308"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 15 </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: Ica5d78a6c7ecf4f0b95fb16de28d3889b32c4776 </div>
<div style="display:none"> Gerrit-Change-Number: 8308 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Richard Mudgett <rmudgett@digium.com> </div>