<p>Jenkins2 <strong>merged</strong> this change.</p><p><a href="https://gerrit.asterisk.org/5994">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Joshua Colp: Verified
Kevin Harwell: Looks good to me, but someone else must approve
Matthew Fredrickson: Looks good to me, approved
Jenkins2: Approved for Submit
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">bridge/core_unreal: Fix SFU bugs with forwarding frames.<br><br>This change fixes a few things uncovered during SFU testing.<br><br>1. Unreal channels incorrectly forwarded video frames when<br>no video stream was present on them. This caused a crash when<br>they were read as the core requires a stream to exist for the<br>underlying media type. The Unreal channel will now ensure a<br>stream exists for the media type before forwarding the frame<br>and if no stream exists then the frame is dropped.<br><br>2. Mapping of frames during bridging from the stream number of<br>the underlying channel to the stream number of the bridge was<br>done in the wrong location. This resulted in the frame getting<br>dropped. This mapping now occurs on reading of the frame from<br>the channel.<br><br>3. Bridging was using the wrong ast_read function resulting in<br>it living in a non-multistream world.<br><br>4. In bridge_softmix when adding new streams to existing channels<br>the wrong stream topology was copied resulting in no streams<br>being added.<br><br>Change-Id: Ib7445722c3219951d6740802a0feddf2908c18c8<br>---<br>M bridges/bridge_softmix.c<br>M include/asterisk/channel.h<br>M main/bridge_channel.c<br>M main/channel.c<br>M main/core_unreal.c<br>5 files changed, 52 insertions(+), 18 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 ae877eb..21f5190 100644<br>--- a/bridges/bridge_softmix.c<br>+++ b/bridges/bridge_softmix.c<br>@@ -600,7 +600,7 @@<br> if (participant == joiner) {<br> continue;<br> }<br>- participant_topology = ast_stream_topology_clone(ast_channel_get_stream_topology(joiner->chan));<br>+ participant_topology = ast_stream_topology_clone(ast_channel_get_stream_topology(participant->chan));<br> if (!participant_topology) {<br> goto cleanup;<br> }<br>diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h<br>index 197cc99..55126b4 100644<br>--- a/include/asterisk/channel.h<br>+++ b/include/asterisk/channel.h<br>@@ -2030,6 +2030,26 @@<br> struct ast_frame *ast_read_noaudio(struct ast_channel *chan);<br> <br> /*!<br>+ * \brief Reads a frame, but does not filter to just the default streams,<br>+ * returning AST_FRAME_NULL frame if audio.<br>+ *<br>+ * \param chan channel to read a frame from<br>+ *<br>+ * \return Returns a frame, or NULL on error. If it returns NULL, you<br>+ * best just stop reading frames and assume the channel has been<br>+ * disconnected.<br>+ *<br>+ * \note This function will not perform any filtering and will return<br>+ * media frames from all streams on the channel. To determine which<br>+ * stream a frame originated from the stream_num on it can be<br>+ * examined.<br>+ *<br>+ * \note Audio is replaced with AST_FRAME_NULL to avoid<br>+ * transcode when the resulting audio is not necessary.<br>+ */<br>+struct ast_frame *ast_read_stream_noaudio(struct ast_channel *chan);<br>+<br>+/*!<br> * \brief Write a frame to a channel<br> * This function writes the given frame to the indicated channel.<br> * \param chan destination channel of the frame<br>diff --git a/main/bridge_channel.c b/main/bridge_channel.c<br>index e8ab8a8..2e94300 100644<br>--- a/main/bridge_channel.c<br>+++ b/main/bridge_channel.c<br>@@ -998,21 +998,6 @@<br> return 0;<br> }<br> <br>- if (ast_channel_is_multistream(bridge_channel->chan) &&<br>- (fr->frametype == AST_FRAME_IMAGE || fr->frametype == AST_FRAME_TEXT ||<br>- fr->frametype == AST_FRAME_VIDEO || fr->frametype == AST_FRAME_VOICE)) {<br>- /* Media frames need to be mapped to an appropriate write stream */<br>- dup->stream_num = AST_VECTOR_GET(<br>- &bridge_channel->stream_map.to_bridge, fr->stream_num);<br>- if (dup->stream_num == -1) {<br>- ast_bridge_channel_unlock(bridge_channel);<br>- bridge_frame_free(dup);<br>- return 0;<br>- }<br>- } else {<br>- dup->stream_num = -1;<br>- }<br>-<br> AST_LIST_INSERT_TAIL(&bridge_channel->wr_queue, dup, frame_list);<br> if (ast_alertpipe_write(bridge_channel->alert_pipe)) {<br> ast_log(LOG_ERROR, "We couldn't write alert pipe for %p(%s)... something is VERY wrong\n",<br>@@ -2455,15 +2440,26 @@<br> }<br> <br> if (bridge_channel->features->mute) {<br>- frame = ast_read_noaudio(bridge_channel->chan);<br>+ frame = ast_read_stream_noaudio(bridge_channel->chan);<br> } else {<br>- frame = ast_read(bridge_channel->chan);<br>+ frame = ast_read_stream(bridge_channel->chan);<br> }<br> <br> if (!frame) {<br> ast_bridge_channel_kick(bridge_channel, 0);<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>+ frame->stream_num = -1;<br>+ }<br>+<br> switch (frame->frametype) {<br> case AST_FRAME_CONTROL:<br> switch (frame->subclass.integer) {<br>diff --git a/main/channel.c b/main/channel.c<br>index 811826f..23bb74f 100644<br>--- a/main/channel.c<br>+++ b/main/channel.c<br>@@ -4180,6 +4180,11 @@<br> return __ast_read(chan, 1, 1);<br> }<br> <br>+struct ast_frame *ast_read_stream_noaudio(struct ast_channel *chan)<br>+{<br>+ return __ast_read(chan, 1, 0);<br>+}<br>+<br> int ast_indicate(struct ast_channel *chan, int condition)<br> {<br> return ast_indicate_data(chan, condition, NULL, 0);<br>diff --git a/main/core_unreal.c b/main/core_unreal.c<br>index 5da7408..3db6a4d 100644<br>--- a/main/core_unreal.c<br>+++ b/main/core_unreal.c<br>@@ -323,6 +323,19 @@<br> return -1;<br> }<br> <br>+ /* If we are told to write a frame with a type that has no corresponding<br>+ * stream on the channel then drop it.<br>+ */<br>+ if (f->frametype == AST_FRAME_VOICE) {<br>+ if (!ast_channel_get_default_stream(ast, AST_MEDIA_TYPE_AUDIO)) {<br>+ return 0;<br>+ }<br>+ } else if (f->frametype == AST_FRAME_VIDEO) {<br>+ if (!ast_channel_get_default_stream(ast, AST_MEDIA_TYPE_VIDEO)) {<br>+ return 0;<br>+ }<br>+ }<br>+<br> /* Just queue for delivery to the other side */<br> ao2_ref(p, 1);<br> ao2_lock(p);<br></pre><p>To view, visit <a href="https://gerrit.asterisk.org/5994">change 5994</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/5994"/><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: Ib7445722c3219951d6740802a0feddf2908c18c8 </div>
<div style="display:none"> Gerrit-Change-Number: 5994 </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: 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>
<div style="display:none"> Gerrit-Reviewer: Matthew Fredrickson <creslin@digium.com> </div>