<p>Jenkins2 <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/6180">View Change</a></p><div style="white-space:pre-wrap">Approvals:
George Joseph: Looks good to me, approved
Jenkins2: Approved for Submit
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">bridge: Fix stream topology/participant locking and video misrouting.<br><br>This change fixes a few locking issues and some video misrouting.<br><br>1. When accessing the stream topology of a channel the channel lock<br>must be held to guarantee the topology remains valid.<br><br>2. When a channel was joined to a bridge the bridge specific<br>implementation for stream mapping was not invoked, causing video<br>to be misrouted for a brief period of time.<br><br>ASTERISK-27182<br><br>Change-Id: I5d2f779248b84d41c5bb3896bf22ba324b336b03<br>(cherry picked from commit 0e352ec5100331c6a32008acc88d69d0fc58ccdd)<br>---<br>M bridges/bridge_softmix.c<br>M main/bridge.c<br>M main/bridge_channel.c<br>3 files changed, 51 insertions(+), 22 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/bridges/bridge_softmix.c b/bridges/bridge_softmix.c<br>index c5428a8..b359948 100644<br>--- a/bridges/bridge_softmix.c<br>+++ b/bridges/bridge_softmix.c<br>@@ -577,13 +577,18 @@<br> struct ast_stream_topology *joiner_video = NULL;<br> struct ast_stream_topology *existing_video = NULL;<br> struct ast_bridge_channel *participant;<br>+ int res;<br> <br> joiner_video = ast_stream_topology_alloc();<br> if (!joiner_video) {<br> return;<br> }<br> <br>- if (append_source_streams(joiner_video, ast_channel_name(joiner->chan), ast_channel_get_stream_topology(joiner->chan))) {<br>+ ast_channel_lock(joiner->chan);<br>+ res = append_source_streams(joiner_video, ast_channel_name(joiner->chan), ast_channel_get_stream_topology(joiner->chan));<br>+ ast_channel_unlock(joiner->chan);<br>+<br>+ if (res) {<br> goto cleanup;<br> }<br> <br>@@ -596,13 +601,18 @@<br> if (participant == joiner) {<br> continue;<br> }<br>- if (append_source_streams(existing_video, ast_channel_name(participant->chan),<br>- ast_channel_get_stream_topology(participant->chan))) {<br>+ ast_channel_lock(participant->chan);<br>+ res = append_source_streams(existing_video, ast_channel_name(participant->chan),<br>+ ast_channel_get_stream_topology(participant->chan));<br>+ ast_channel_unlock(participant->chan);<br>+ if (res) {<br> goto cleanup;<br> }<br> }<br> <br>+ ast_channel_lock(joiner->chan);<br> joiner_topology = ast_stream_topology_clone(ast_channel_get_stream_topology(joiner->chan));<br>+ ast_channel_unlock(joiner->chan);<br> if (!joiner_topology) {<br> goto cleanup;<br> }<br>@@ -617,7 +627,9 @@<br> if (participant == joiner) {<br> continue;<br> }<br>+ ast_channel_lock(participant->chan);<br> participant_topology = ast_stream_topology_clone(ast_channel_get_stream_topology(participant->chan));<br>+ ast_channel_unlock(participant->chan);<br> if (!participant_topology) {<br> goto cleanup;<br> }<br>@@ -753,12 +765,16 @@<br> continue;<br> }<br> <br>+ ast_channel_lock(participant->chan);<br> remove_destination_streams(participant_topology, ast_channel_name(leaver->chan), ast_channel_get_stream_topology(participant->chan));<br>+ ast_channel_unlock(participant->chan);<br> ast_channel_request_stream_topology_change(participant->chan, participant_topology, NULL);<br> ast_stream_topology_free(participant_topology);<br> }<br> <br>+ ast_channel_lock(leaver->chan);<br> remove_destination_streams(leaver_topology, "", ast_channel_get_stream_topology(leaver->chan));<br>+ ast_channel_unlock(leaver->chan);<br> ast_channel_request_stream_topology_change(leaver->chan, leaver_topology, NULL);<br> ast_stream_topology_free(leaver_topology);<br> <br>@@ -1657,6 +1673,7 @@<br> }<br> <br> ast_bridge_channel_lock(participant);<br>+ ast_channel_lock(participant->chan);<br> topology = ast_channel_get_stream_topology(participant->chan);<br> <br> for (i = 0; i < ast_stream_topology_get_count(topology); ++i) {<br>@@ -1668,6 +1685,7 @@<br> break;<br> }<br> }<br>+ ast_channel_unlock(participant->chan);<br> ast_bridge_channel_unlock(participant);<br> }<br> }<br>@@ -1702,16 +1720,9 @@<br> <br> /* First traversal: re-initialize all of the participants' stream maps */<br> AST_LIST_TRAVERSE(&bridge->channels, participant, entry) {<br>- int size;<br>-<br> ast_bridge_channel_lock(participant);<br>- size = ast_stream_topology_get_count(ast_channel_get_stream_topology(participant->chan));<br>-<br>- AST_VECTOR_FREE(&participant->stream_map.to_channel);<br>- AST_VECTOR_FREE(&participant->stream_map.to_bridge);<br>-<br>- AST_VECTOR_INIT(&participant->stream_map.to_channel, size);<br>- AST_VECTOR_INIT(&participant->stream_map.to_bridge, size);<br>+ AST_VECTOR_RESET(&participant->stream_map.to_channel, AST_VECTOR_ELEM_CLEANUP_NOOP);<br>+ AST_VECTOR_RESET(&participant->stream_map.to_bridge, AST_VECTOR_ELEM_CLEANUP_NOOP);<br> ast_bridge_channel_unlock(participant);<br> }<br> <br>@@ -1726,6 +1737,8 @@<br> AST_LIST_TRAVERSE(&bridge->channels, participant, entry) {<br> int i;<br> struct ast_stream_topology *topology;<br>+<br>+ ast_channel_lock(participant->chan);<br> <br> topology = ast_channel_get_stream_topology(participant->chan);<br> <br>@@ -1766,6 +1779,8 @@<br> }<br> ast_bridge_channel_unlock(participant);<br> }<br>+<br>+ ast_channel_unlock(participant->chan);<br> }<br> }<br> <br>diff --git a/main/bridge.c b/main/bridge.c<br>index a1a1a6f..b732d5f 100644<br>--- a/main/bridge.c<br>+++ b/main/bridge.c<br>@@ -446,7 +446,12 @@<br> * media types vector. This way all streams map to the same media type index for<br> * a given channel.<br> */<br>- ast_bridge_channel_stream_map(bridge_channel);<br>+ if (bridge_channel->bridge->technology->stream_topology_changed) {<br>+ bridge_channel->bridge->technology->stream_topology_changed(<br>+ bridge_channel->bridge, bridge_channel);<br>+ } else {<br>+ ast_bridge_channel_stream_map(bridge_channel);<br>+ }<br> }<br> <br> /*!<br>diff --git a/main/bridge_channel.c b/main/bridge_channel.c<br>index 2e94300..1427fd0 100644<br>--- a/main/bridge_channel.c<br>+++ b/main/bridge_channel.c<br>@@ -2344,16 +2344,23 @@<br> case AST_FRAME_NULL:<br> break;<br> default:<br>- if (fr->stream_num > 0 &&<br>- (fr->stream_num >= (int)AST_VECTOR_SIZE(&bridge_channel->stream_map.to_channel) ||<br>- AST_VECTOR_GET(&bridge_channel->stream_map.to_channel, fr->stream_num) == -1)) {<br>- /* Nowhere to write to, so drop it */<br>- break;<br>- }<br>+ /* Assume that there is no mapped stream for this */<br>+ num = -1;<br> <br>- /* Find what stream number to write to for the channel */<br>- num = fr->stream_num < 0 ? -1 :<br>- AST_VECTOR_GET(&bridge_channel->stream_map.to_channel, fr->stream_num);<br>+ if (fr->stream_num > -1) {<br>+ ast_bridge_channel_lock(bridge_channel);<br>+ if (fr->stream_num < (int)AST_VECTOR_SIZE(&bridge_channel->stream_map.to_channel)) {<br>+ num = AST_VECTOR_GET(&bridge_channel->stream_map.to_channel, fr->stream_num);<br>+ }<br>+ ast_bridge_channel_unlock(bridge_channel);<br>+<br>+ /* If there is no mapped stream after checking the mapping then there is nowhere<br>+ * to write this frame to, so drop it.<br>+ */<br>+ if (num == -1) {<br>+ break;<br>+ }<br>+ }<br> <br> /* Write the frame to the channel. */<br> bridge_channel->activity = BRIDGE_CHANNEL_THREAD_SIMPLE;<br>@@ -2983,7 +2990,9 @@<br> <br> void ast_bridge_channel_stream_map(struct ast_bridge_channel *bridge_channel)<br> {<br>+ ast_channel_lock(bridge_channel->chan);<br> ast_stream_topology_map(ast_channel_get_stream_topology(bridge_channel->chan),<br> &bridge_channel->bridge->media_types, &bridge_channel->stream_map.to_bridge,<br> &bridge_channel->stream_map.to_channel);<br>+ ast_channel_unlock(bridge_channel->chan);<br> }<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/6180">change 6180</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/6180"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 15.0 </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: I5d2f779248b84d41c5bb3896bf22ba324b336b03 </div>
<div style="display:none"> Gerrit-Change-Number: 6180 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Joshua Colp <jcolp@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>