<p>Joshua Colp <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/6281">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Joshua Colp: Looks good to me, but someone else must approve
George Joseph: Looks good to me, approved
Jenkins2: Approved for Submit
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">bridge_channel.c: Fix FRACK when mapping frames to the bridge.<br><br>* Add protection checks when mapping streams to the bridge. The channel<br>and bridge may be in the process of updating the stream mapping when a<br>media frame comes in so we may not be able to map the frame at the time.<br><br>* We need to map the streams to the bridge's stream numbers right before<br>they are written into the bridge. That way we don't have to keep<br>locking/unlocking the bridge and we won't have any synchronization<br>problems before the frames actually go into the bridge.<br><br>* Protect the deferred queue with the bridge_channel lock.<br><br>ASTERISK-27212<br><br>Change-Id: Id6860dd61b594b90c8395f6e2c0150219094c21a<br>---<br>M main/bridge_channel.c<br>1 file changed, 61 insertions(+), 10 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/main/bridge_channel.c b/main/bridge_channel.c<br>index c465ec1..66292bf 100644<br>--- a/main/bridge_channel.c<br>+++ b/main/bridge_channel.c<br>@@ -626,11 +626,11 @@<br> <br> /*!<br> * \internal<br>- * \brief Write an \ref ast_frame onto the bridge channel<br>+ * \brief Write an \ref ast_frame into the bridge<br> * \since 12.0.0<br> *<br>- * \param bridge_channel Which channel to queue the frame onto.<br>- * \param frame The frame to write onto the bridge_channel<br>+ * \param bridge_channel Which channel is queueing the frame.<br>+ * \param frame The frame to write into the bridge<br> *<br> * \retval 0 on success.<br> * \retval -1 on error.<br>@@ -638,11 +638,58 @@<br> static int bridge_channel_write_frame(struct ast_bridge_channel *bridge_channel, struct ast_frame *frame)<br> {<br> const struct ast_control_t38_parameters *t38_parameters;<br>+ int unmapped_stream_num;<br> int deferred;<br> <br> ast_assert(frame->frametype != AST_FRAME_BRIDGE_ACTION_SYNC);<br> <br> ast_bridge_channel_lock_bridge(bridge_channel);<br>+<br>+ /* Map the frame to the bridge. */<br>+ if (ast_channel_is_multistream(bridge_channel->chan)) {<br>+ unmapped_stream_num = frame->stream_num;<br>+ switch (frame->frametype) {<br>+ case AST_FRAME_VOICE:<br>+ case AST_FRAME_VIDEO:<br>+ case AST_FRAME_TEXT:<br>+ case AST_FRAME_IMAGE:<br>+ /* Media frames need to be mapped to an appropriate write stream */<br>+ if (frame->stream_num < 0) {<br>+ /* Map to default stream */<br>+ frame->stream_num = -1;<br>+ break;<br>+ }<br>+ ast_bridge_channel_lock(bridge_channel);<br>+ if (frame->stream_num < (int)AST_VECTOR_SIZE(&bridge_channel->stream_map.to_bridge)) {<br>+ frame->stream_num = AST_VECTOR_GET(<br>+ &bridge_channel->stream_map.to_bridge, frame->stream_num);<br>+ if (0 <= frame->stream_num) {<br>+ ast_bridge_channel_unlock(bridge_channel);<br>+ break;<br>+ }<br>+ }<br>+ ast_bridge_channel_unlock(bridge_channel);<br>+ ast_bridge_unlock(bridge_channel->bridge);<br>+ /*<br>+ * Ignore frame because we don't know how to map the frame<br>+ * or the bridge is not expecting any media from that<br>+ * stream.<br>+ */<br>+ return 0;<br>+ case AST_FRAME_DTMF_BEGIN:<br>+ case AST_FRAME_DTMF_END:<br>+ /*<br>+ * XXX It makes sense that DTMF could be on any audio stream.<br>+ * For now we will only put it on the default audio stream.<br>+ */<br>+ default:<br>+ frame->stream_num = -1;<br>+ break;<br>+ }<br>+ } else {<br>+ unmapped_stream_num = -1;<br>+ frame->stream_num = -1;<br>+ }<br> <br> deferred = bridge_channel->bridge->technology->write(bridge_channel->bridge, bridge_channel, frame);<br> if (deferred) {<br>@@ -650,7 +697,14 @@<br> <br> dup = ast_frdup(frame);<br> if (dup) {<br>+ /*<br>+ * We have to unmap the deferred frame so it comes back<br>+ * in like a new frame.<br>+ */<br>+ dup->stream_num = unmapped_stream_num;<br>+ ast_bridge_channel_lock(bridge_channel);<br> AST_LIST_INSERT_HEAD(&bridge_channel->deferred_queue, dup, frame_list);<br>+ ast_bridge_channel_unlock(bridge_channel);<br> }<br> }<br> <br>@@ -760,12 +814,14 @@<br> {<br> struct ast_frame *frame;<br> <br>+ ast_bridge_channel_lock(bridge_channel);<br> ast_channel_lock(bridge_channel->chan);<br> while ((frame = AST_LIST_REMOVE_HEAD(&bridge_channel->deferred_queue, frame_list))) {<br> ast_queue_frame_head(bridge_channel->chan, frame);<br> ast_frfree(frame);<br> }<br> ast_channel_unlock(bridge_channel->chan);<br>+ ast_bridge_channel_unlock(bridge_channel);<br> }<br> <br> /*!<br>@@ -2458,13 +2514,8 @@<br> return;<br> }<br> <br>- if (ast_channel_is_multistream(bridge_channel->chan) &&<br>- (frame->frametype == AST_FRAME_IMAGE || frame->frametype == AST_FRAME_TEXT ||<br>- frame->frametype == AST_FRAME_VIDEO || frame->frametype == AST_FRAME_VOICE)) {<br>- /* Media frames need to be mapped to an appropriate write stream */<br>- frame->stream_num = AST_VECTOR_GET(<br>- &bridge_channel->stream_map.to_bridge, frame->stream_num);<br>- } else {<br>+ if (!ast_channel_is_multistream(bridge_channel->chan)) {<br>+ /* This may not be initialized by non-multistream channel drivers */<br> frame->stream_num = -1;<br> }<br> <br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/6281">change 6281</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/6281"/><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: Id6860dd61b594b90c8395f6e2c0150219094c21a </div>
<div style="display:none"> Gerrit-Change-Number: 6281 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Joshua Colp <jcolp@digium.com> </div>