<p>Jenkins2 <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/8309">View Change</a></p><div style="white-space:pre-wrap">Approvals:
  Jasper van der Neut: Looks good to me, but someone else must approve
  Joshua Colp: Looks good to me, but someone else must approve
  Kevin Harwell: Looks good to me, approved
  Jenkins2: Approved for Submit

</div><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, 49 insertions(+), 36 deletions(-)<br><br></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..40f7ddc 100644<br>--- a/bridges/bridge_simple.c<br>+++ b/bridges/bridge_simple.c<br>@@ -113,14 +113,19 @@<br>      .stream_topology_changed = simple_bridge_stream_topology_changed,<br> };<br> <br>-static void simple_bridge_request_stream_topology_change(struct ast_channel *chan,<br>+static struct ast_stream_topology *simple_bridge_request_stream_topology_update(<br>+    struct ast_stream_topology *existing_topology,<br>        struct ast_stream_topology *requested_topology)<br> {<br>-  struct ast_stream_topology *existing_topology = ast_channel_get_stream_topology(chan);<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>+    new_topology = ast_stream_topology_clone(requested_topology);<br>+        if (!new_topology) {<br>+         return NULL;<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>@@ -138,59 +143,67 @@<br>             break;<br>        }<br> <br>- if (!audio_formats) {<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>- 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 (ast_stream_get_type(stream) != AST_MEDIA_TYPE_AUDIO ||<br>+                           ast_stream_get_state(stream) == AST_STREAM_STATE_REMOVED) {<br>+                          continue;<br>+                    }<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>-<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>  }<br> <br>- ast_channel_request_stream_topology_change(chan, new_topology, &simple_bridge);<br>-<br>-       ast_stream_topology_free(new_topology);<br>+      return 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 *req_chan;<br>+        struct ast_channel *existing_chan;<br>+   struct ast_stream_topology *req_top;<br>+ struct ast_stream_topology *existing_top;<br>+    struct ast_stream_topology *new_top;<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>+     req_chan = AST_LIST_FIRST(&bridge->channels)->chan;<br>+        existing_chan = AST_LIST_LAST(&bridge->channels)->chan;<br>+    if (req_chan == existing_chan) {<br>+             /* Wait until both channels are in the bridge to align topologies. */<br>                 return;<br>       }<br> <br>  /* Align topologies according to size or first channel to join */<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>+        ast_channel_lock_both(req_chan, existing_chan);<br>+      req_top = ast_channel_get_stream_topology(req_chan);<br>+ existing_top = ast_channel_get_stream_topology(existing_chan);<br>+       if (ast_stream_topology_get_count(req_top) < ast_stream_topology_get_count(existing_top)) {<br>+               SWAP(req_top, existing_top);<br>+         SWAP(req_chan, existing_chan);<br>        }<br>+    new_top = simple_bridge_request_stream_topology_update(existing_top, req_top);<br>+       ast_channel_unlock(req_chan);<br>+        ast_channel_unlock(existing_chan);<br>+<br>+        if (!new_top) {<br>+              /* Failure.  We'll just have to live with the current topology. */<br>+               return;<br>+      }<br>+<br>+ ast_channel_request_stream_topology_change(existing_chan, new_top, &simple_bridge);<br>+      ast_stream_topology_free(new_top);<br> }<br> <br> static int unload_module(void)<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/8309">change 8309</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/8309"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: Ica5d78a6c7ecf4f0b95fb16de28d3889b32c4776 </div>
<div style="display:none"> Gerrit-Change-Number: 8309 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jasper van der Neut <jasper@isotopic.nl> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Kevin Harwell <kharwell@digium.com> </div>