<p>Richard Mudgett <strong>posted comments</strong> on this change.</p><p><a href="https://gerrit.asterisk.org/6759">View Change</a></p><p>Patch set 1:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4;">Code-Review -1</span></p><p>(3 comments)</p><ul style="list-style: none; padding-left: 20px;"><li><p><a href="https://gerrit.asterisk.org/#/c/6759/1/bridges/bridge_simple.c">File bridges/bridge_simple.c:</a></p><ul style="list-style: none; padding-left: 20px;"><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/6759/1/bridges/bridge_simple.c@141">Patch Set #1, Line 141:</a> <code style="font-family:monospace,monospace">      new_topology = ast_stream_topology_clone(requested_topology);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This can fail.</p></li><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/6759/1/bridges/bridge_simple.c@146">Patch Set #1, Line 146:</a> <code style="font-family:monospace,monospace">  if (audio_formats) {</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Invert the logic to reduce indention and avoid cloning the requested_topology.</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">if (!audio_formats) {<br>   ast_channel_request_stream_toology_change(chan, requested_topology, &simple_bridge);<br>   return;<br>}</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">new_topology = ast_stream_topology_clone();<br>if (!new_topology) {<br>   Don't bother requesting a change?<br>   Or just request a change without adding the audio formats?<br>   return;<br>}</pre><p style="white-space: pre-wrap; word-wrap: break-word;">add the audio_formats as needed.</p></li><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/6759/1/bridges/bridge_simple.c@157">Patch Set #1, Line 157:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">                     /* We go through the formats individually to ensure we don't have<br>                  * duplicates.<br>                         */<br>                   for (format_index = 0; format_index < ast_format_cap_count(audio_formats); ++format_index) {<br>                               struct ast_format *format = ast_format_cap_get_format(audio_formats, format_index);<br><br>                         if (ast_format_cap_iscompatible_format(ast_stream_get_formats(stream), format) != AST_FORMAT_CMP_NOT_EQUAL) {<br>                                 ao2_ref(format, -1);<br>                                  continue;<br>                             }<br><br>                           ast_format_cap_append(ast_stream_get_formats(stream), format, ast_format_cap_get_framing(audio_formats));<br>                             ao2_ref(format, -1);<br>                  }<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Why not use:<br>ast_format_cap_append_from_cap(ast_stream_get_formats(stream), audio_formats, AST_MEDIA_TYPE_AUDIO);</p><p style="white-space: pre-wrap; word-wrap: break-word;">ast_format_cap_append() checks if the format is already in the cap.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/6759">change 6759</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/6759"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 15 </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: I8fc0cc03e8bcfd0be8302f13b9f32d8268977f43 </div>
<div style="display:none"> Gerrit-Change-Number: 6759 </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: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Fri, 13 Oct 2017 22:16:08 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>