On Tue, Dec 8, 2015 at 4:17 AM, Alexander Traud <pabstr...@compuserve.com> wrote:
> >> Question #1: Level of Integration? > >> [x] [x] [ ] GSM-EFR; GSM-FR is available already > > More analysis would have to be done for a codec module. > > GSM-EFR uses the same library as AMR. Therefore, it is the same situation. > > >> Question #2: Format-attribute Keys as Header Files? > > format_attribute_set was implemented as a way to set arbitrary attributes > > in a format module. I wouldn't remove that API call, > > I am not about removing that API, I am more about: Do I have to and for > whom > should I implement that API? Currently, that API is not implemented in my > modules. I am just asking before it gets rejected in code-review just > because of that. > > You do not need to implement that callback if you module does not use it. > >> Question #3: Module Loading Priority > >> H.26x modules load very late (AST_MODPRI_DEFAULT). The Opus Codec module > >> loads very early (AST_MODPRI_CHANNEL_DEPEND). Which one is correct? Or > are > >> both and there is a reason why video modules load later than audio > modules? > > So long as the core 'understand' that the format may exist, > > AST_MODPRI_DEFAULT is fine. I would imagine that > AST_MODPRI_CHANNEL_DEPEND > > would only be necessary if it needed to register itself with the codec > core. > > Channels do create copies of formats. When a format-attribute module is not > loaded, then copies of the format do not have an attribute attached. This > is > fixed later on in the channels, by searching each time. However, I wonder > why some format-attribute modules are loaded (too) late and some not. Or > should I just change all the existing module to AST_MODPRI_CHANNEL_DEPEND, > create a review and wait what happens (abandoned, rejected, or committed)? > I > am still not sure what is easier for the team: Pre-ask on the mailing list > or going through issue-report/code-review. > > 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. As far as mailing list versus code review: (1) Mailing list is used to discuss anything related to the development of the project. (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. 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. > >> Question #4: Orphan Format-attribute module CELT > > you can have pass through support with the format modules - you don't > need > > necessarily need a format module to pass media from one channel through a > > bridge to another, the core merely has to know that the format exists. > That > > also explains why the media format attribute modules exist - they provide > > assistance during negotiation. > > Yes, format-attribute modules are required only for supporting the line > 'fmtp' in SDP negotiation. However since Asterisk 13, both SILK and CELT > are > not available, even not as pass-through. Therefore currently, these > format-attribute modules are orphan modules. > > 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 :-) > > SILK and CELT have commercial but free modules offered by Digium - > although > > due to time constraints, versions of those modules have not been released > > for Asterisk 13. > > Thanks for the stars given on GitHub. It is most appreciated. > You have not given a star for my SILK modules: > <https://github.com/traud/asterisk-silk>. Perhaps it got overlooked. > Because > you mention time constraints, SILK was not dropped on purpose, I guess. > Therefore, is the Asterisk Team interested in pass-through support for SILK > as well? By the way, are all these new modules just for master, or is there > any chance to submit them for branch 13, too? > > 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: * Basic pass through negotiation * Format attribute negotiation, if appropriate 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: * tests/channels/SIP/SDP_offer_answer * tests/channels/SIP/SDP_attribute_passthrough * tests/channels/pjsip/sdp_offer_answer/incoming/nominal/single-media-stream/audio/basic * tests/channels/pjsip/sdp_offer_answer/attribute_passthrough/speex_h263_h264 Clearly, additional tests could be written beyond pass through/negotiation, but it's a good starting point. -- Matthew Jordan Digium, Inc. | Director of Technology 445 Jan Davis Drive NW - Huntsville, AL 35806 - USA Check us out at: http://digium.com & http://asterisk.org
-- _____________________________________________________________________ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
