<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Dec 8, 2015 at 4:17 AM, Alexander Traud <span dir="ltr"><<a href="mailto:pabstraud@compuserve.com" target="_blank">pabstraud@compuserve.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">>> Question #1: Level of Integration?<br>
</span><span class="">>> [x] [x] [ ] GSM-EFR; GSM-FR is available already<br>
</span><span class="">> More analysis would have to be done for a codec module.<br>
<br>
</span>GSM-EFR uses the same library as AMR. Therefore, it is the same situation.<br>
<span class=""><br>
>> Question #2: Format-attribute Keys as Header Files?<br>
</span><span class="">> format_attribute_set was implemented as a way to set arbitrary attributes<br>
> in a format module. I wouldn't remove that API call,<br>
<br>
</span>I am not about removing that API, I am more about: Do I have to and for whom<br>
should I implement that API? Currently, that API is not implemented in my<br>
modules. I am just asking before it gets rejected in code-review just<br>
because of that.<br>
<span class=""><br></span></blockquote><div><br></div><div>You do not need to implement that callback if you module does not use it.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
>> Question #3: Module Loading Priority<br>
>> H.26x modules load very late (AST_MODPRI_DEFAULT). The Opus Codec module<br>
>> loads very early (AST_MODPRI_CHANNEL_DEPEND). Which one is correct? Or are<br>
>> both and there is a reason why video modules load later than audio modules?<br>
> So long as the core 'understand' that the format may exist,<br>
> AST_MODPRI_DEFAULT is fine. I would imagine that AST_MODPRI_CHANNEL_DEPEND<br>
> would only be necessary if it needed to register itself with the codec core.<br>
<br>
</span>Channels do create copies of formats. When a format-attribute module is not<br>
loaded, then copies of the format do not have an attribute attached. This is<br>
fixed later on in the channels, by searching each time. However, I wonder<br>
why some format-attribute modules are loaded (too) late and some not. Or<br>
should I just change all the existing module to AST_MODPRI_CHANNEL_DEPEND,<br>
create a review and wait what happens (abandoned, rejected, or committed)? I<br>
am still not sure what is easier for the team: Pre-ask on the mailing list<br>
or going through issue-report/code-review.<br>
<span class=""><br></span></blockquote><div><br></div><div>Given that explanation, I would probably lean towards having the format attribute modules use a module priority of AST_MODPRI_CHANNEL_DEPEND. You would avoid having a very small window of time in which a channel could be created without using the format attribute module to aid in negotiation.<br><br></div><div>As far as mailing list versus code review:<br></div><div>(1) Mailing list is used to discuss anything related to the development of the project.<br></div><div>(2) Code review is more formal - you've submitted a patch, subject to the CLA, for inclusion into the project. As a formal peer review, acceptance of the patch would immediately move towards inclusion in the project.<br><br></div><div>So I think you've got it right. Using the mailing list to discuss potential approaches to a patch is perfectly legitimate, and can save a lot of time and energy if a patch is written and rejected due to a fundamental design issue. On the other hand, using a code review tool is quite obviously handy for in depth discussion and analysis of a proposed patch.<br></div><div><br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
>> Question #4: Orphan Format-attribute module CELT<br>
</span><span class="">> you can have pass through support with the format modules - you don't need<br>
> necessarily need a format module to pass media from one channel through a<br>
> bridge to another, the core merely has to know that the format exists. That<br>
> also explains why the media format attribute modules exist - they provide<br>
> assistance during negotiation.<br>
<br>
</span>Yes, format-attribute modules are required only for supporting the line<br>
'fmtp' in SDP negotiation. However since Asterisk 13, both SILK and CELT are<br>
not available, even not as pass-through. Therefore currently, these<br>
format-attribute modules are orphan modules.<br>
<span class=""><br></span></blockquote><div><br></div><div>Whoops. That does look to be an overight from the format overhaul done in 13 - they should at least be supported as pass through. We should probably make an issue for that :-)<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
> SILK and CELT have commercial but free modules offered by Digium - although<br>
> due to time constraints, versions of those modules have not been released<br>
> for Asterisk 13.<br>
<br>
</span>Thanks for the stars given on GitHub. It is most appreciated.<br>
You have not given a star for my SILK modules:<br>
<<a href="https://github.com/traud/asterisk-silk" rel="noreferrer" target="_blank">https://github.com/traud/asterisk-silk</a>>. Perhaps it got overlooked. Because<br>
you mention time constraints, SILK was not dropped on purpose, I guess.<br>
Therefore, is the Asterisk Team interested in pass-through support for SILK<br>
as well? By the way, are all these new modules just for master, or is there<br>
any chance to submit them for branch 13, too?<br>
<div class="HOEnZb"><div class="h5"><br></div></div></blockquote><div><br></div><div>New modules for 13 must have accompanying tests. For the purposes of new codec modules, I would suspect we would want to test at a minimum:<br></div><div> * Basic pass through negotiation<br></div><div> * Format attribute negotiation, if appropriate<br><br></div><div>There's some good examples of these tests currently in the Asterisk Test Suite. Most of these tests simply use SIPp to send/receive an offer from Asterisk and receive/send a corresponding answer:<br></div><div> * tests/channels/SIP/SDP_offer_answer<br></div><div> * tests/channels/SIP/SDP_attribute_passthrough<br></div><div> * tests/channels/pjsip/sdp_offer_answer/incoming/nominal/single-media-stream/audio/basic<br></div><div> * tests/channels/pjsip/sdp_offer_answer/attribute_passthrough/speex_h263_h264<br></div><div><br></div><div>Clearly, additional tests could be written beyond pass through/negotiation, but it's a good starting point.<br></div></div><br>-- <br><div class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div>Matthew Jordan<br></div><div>Digium, Inc. | Director of Technology<br></div><div>445 Jan Davis Drive NW - Huntsville, AL 35806 - USA</div><div>Check us out at: <a href="http://digium.com" target="_blank">http://digium.com</a> & <a href="http://asterisk.org" target="_blank">http://asterisk.org</a></div></div></div></div></div>
</div></div>