<p>Joshua Colp has uploaded this change for <strong>review</strong>.</p><p><a href="https://gerrit.asterisk.org/5994">View Change</a></p><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;">git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/94/5994/1</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: newchange </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>