[Asterisk-code-review] codecs: Add Codec 2 mode 2400. (asterisk[master])

[Alexander Traud]

Alexander Traud has posted comments on this change.

Change subject: codecs: Add Codec 2 mode 2400.
......................................................................


Patch Set 3:

(1 comment)

I did not know about ast_rtp_engine_(un)load_format before. Handy part of the API. Is that documented somewhere or used in other open-source modules? I wonder why not more codecs use that, especially Siren.

Currently in the dynamic range, one payload type (120) is still available. Four types (108, 109, 113, 114) get occupied, when the SILK codec module from Digium is used. That module was updated for Asterisk 13.11 and newer. Since <https://gerrit.asterisk.org/3398>, the SILK module registers dynamically via ast_rtp_engine_(un)load_format, I guess.

Richard, I went for your proposed resolution. Historically, ast_rtp_engine_(un)load_format should be used by codecs which support the codec.conf file only. This were CELT and SILK. With my new patch-set, I removed all changes to main/rtp_engine.c. Therefore, passthrough support is gone in case -lcodec2 is not available as shared library. Therefore, moving to ast_rtp_engine_(un)load_format within codecs/codec_codec2.so is not correct technically. Anyway with Codec 2, that anyone on this earth uses just passthrough and does not want transcoding, that is not expected. Consequently, I am OK with that.

If you are not OK with that anymore, I would go for payload type 120.

By the way, looking at the SILK patch <https://gerrit.asterisk.org/3136>, I wonder whether I have to change main/format_compatibility.c/.h at all.

https://gerrit.asterisk.org/#/c/3244/3/main/rtp_engine.c
File main/rtp_engine.c:

Line 2705: 	add_static_payload(126, ast_format_codec2, 0);
> We already have a 126 declared below, you'll have to pick something differe
Done


-- 
To view, visit https://gerrit.asterisk.org/3244
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I1e45d8084683fab5f2b272bf35f4a149cea8b8d6
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Alexander Traud <pabstraud at compuserve.com>
Gerrit-Reviewer: Alexander Traud <pabstraud at compuserve.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Matthew Fredrickson <creslin at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes