<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>