On Sun, Oct 11, 2015 at 3:09 PM, Alexander Traud <pabstr...@compuserve.com> wrote: >> 2) ... codec modules are fed [with] frames already ... > > Uh. That is a bit too abstract, yet: When do I tag the frames exactly? > > ast_rtp_codecs_get_framing(ast_rtp_instance_get_codecs(instance)) > provides the expected packetization value in res/res_rtp_asterisk.c, yes. > However, the transcoding module needs that value while creating the frame, > somewhere in the call chain: ast_write -> ast_translate -> *frameout.
You can only get the packetization in one of two ways: * The channel drivers inform the codec instance during negotiation and/or when the channel is created. I'm not sure of any way to do this without modifying the codec core/channel core in some fashion. I will say that a format is still not the appropriate place for it. (1) Formats are immutable, more on that below. (2) Packetization is a session level attribute, not a media format attribute. It is semantically wrong to store it in that location. * The RTP instance knows of the packetization. It can request the packetization via ast_rtp_codecs_get_framing; it can add that to the frames it creates in ast_rtp_read. A read frame will be passed up to the codec translator with the correct packetization; that should set the packetization in the codec "on the fly". Realistically, this will result in one generated frame with the "default" packetization. Practically, I would expect that to not matter. > res/res_rtp_asterisk.c:ast_rtp_write is called after the above chain. > Therefore, the frame would be tagged with the current packetization value > too late. By the way, I might have created a misunderstanding: While reading > (ast_rtp_read, ast_read), the AMR transcoding module does not care about > that value because the remote device could send/use a different value > anytime - actually the module does *not* need to know the value while > reading AMR, because the module able to extract the amount of frames from > the payload data itself. It is just about writing. Hm. I would expect packetization to be negotiated the same on both sides. If not, then yes, the frame approach won't work. > My second issue with solution #2: Because ptime is needed in *frameout, the > place where the frames are created, I do not know, which frames to tag. > There is one frame in the private structure of the translation itself > (ast_trans_pvt). Again yet, I was not able to find a place in code where > that meets the RTP packetization value. Mhm. I don't understand this point; you'll need to point more closely to where in particular you are talking about. >> formats in 13+ are immutable by convention > > Since Asterisk 13, there is a format cache, which is used heavily, yes. I > guess, this is meant by immutable. Or? However, this is not used everywhere, Immutable means you MUST NOT change a format instance once it is created. You *will* cause something horrible to happen. Formats have no lock; they are accessed by multiple threads concurrently. If you modify one, you *may* modify it for multiple channels. You may crash. You may have monkeys fly out of your Asterisk system. Please don't do it. :-) There are safe ways to duplicate a format, modifying it safely in the process of duplication. This, however, is generally not a great solution, as it requires a reasonable amount of overhead. > see <http://github.com/traud/asterisk-amr> capability_cached_format.patch > (Is that a patch not for a new feature but a bug?). At the end, I saw a lot > of different ast_formats pointers in my res/res_format_attr_*.c, which had > to be reduced because: > I) The AMR decoder and the encoder require access to the fmtp line(s). > Actually, the negotiated result of both lines is required. > II) The decoder has to talk to the encoder (only that way is a must) because > the remote device is able to request a AMR-mode change for the encoder. I haven't looked at the patch, but if you're changing the contract of what occurs in the format modules and the formats, I would suspect that you've introduced a major bug in the format core. You can't modify formats that are shared between channels. > Therefore, ast_format_interface->format_get_joint creates a new ast_format. > Consequently, I meet a unique entity when > ast_format_cap_get_format_framing() is called. That is the place where I tag > "my" ast_format/ast_format_interface with the current packetization value > (possibility negotiated; sip.conf:autoframing=yes). That is very much your > solution #1. Any comments? > I think any modification to ast_format is the wrong approach. (1) It is semantically incorrect, storing a media session level attribute on a specific format within that session, while the packetization applies to all formats within that session. (2) It goes against the contract ast_format tries hard to enforce. You'll either kill performance by requiring a large amount of new ast_format structures when you don't want them, or you'll cause major threading issues. Either channels will be running with a packetization they didn't want, or you may just crash. Or both. If setting it on the ast_frame doesn't work - and that's fair - and packetization is a media session attribute - why not just store it on the ast_translator on the channel? Frankly, it is the closest to something keeping track of the state of the media session that I can think of, and that's really where packetization should probably go. -- 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