[asterisk-dev] Transcoding: Codec 2, iLBC 20, SILK, GSM-EFR, AMR(-WB)

[Matthew Jordan]

On Tue, Dec 8, 2015 at 4:17 AM, Alexander Traud <pabstraud at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20151208/e5079928/attachment.html>