<p>Corey Farrell <strong>posted comments</strong> on this change.</p><p><a href="https://gerrit.asterisk.org/8209">View Change</a></p><p>Patch set 4:</p><p>(1 comment)</p><ul style="list-style: none; padding-left: 20px;"><li><p><a href="https://gerrit.asterisk.org/#/c/8209/4/include/asterisk/mod_format.h">File include/asterisk/mod_format.h:</a></p><ul style="list-style: none; padding-left: 20px;"><li><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/8209/4/include/asterisk/mod_format.h@47">Patch Set #4, Line 47:</a> <code style="font-family:monospace,monospace">    char mime_types[80]; /*!< MIME Types related to the format (separated by | if more than one)*/</code></p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">As for repeatedly parsing the string, I don't think the extra CPU<br>needed for parsing the string is much of a problem to consider<br>alternate encodings.</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">If we were to look at optimizing that function (exts_compare) then I think we would need some kind of hashtab lookup to avoid running strcmp on every item.  This would likely require using a private structure to link key to ast_format_def so we can lookup by extension or mime_type.  I think that would be a separate change and would not require ABI/API changes.  Using a delimited string in the ast_format_def structure keeps the API simpler than using any kind of list.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Regarding the character to be used for the mime_type delimiter, RFC1341/RFC1521 define the "Content-Type" header.  They do not exclude the possibility of "|" being part of a mime type (granted I've never seen this).  So ":" might be a better option for the mime-type delimiter as it is explicitly forbidden by the MIME specification.  This is a pedantic nitpick, not sure if it's worth acting on.</p><p style="white-space: pre-wrap; word-wrap: break-word;">https://www.w3.org/Protocols/rfc1341/4_Content-Type.html<br>https://datatracker.ietf.org/doc/rfc1521/?include_text=1 (page 9, sorry no direct link)</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/8209">change 8209</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/8209"/><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: I1b14692a49b2c1ac67688f58757184122e92ba89 </div>
<div style="display:none"> Gerrit-Change-Number: 8209 </div>
<div style="display:none"> Gerrit-PatchSet: 4 </div>
<div style="display:none"> Gerrit-Owner: Sean Bright <sean.bright@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: Corey Farrell <git@cfware.com> </div>
<div style="display:none"> Gerrit-Reviewer: Gaurav Khurana <gkhurana@godaddy.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Matt Jordan <mjordan@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Sean Bright <sean.bright@gmail.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Tue, 27 Feb 2018 16:38:56 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>