Re: [asterisk-dev] SIP/SDP: ptime in translation module?

[Alexander Traud]

> 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.

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.

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.

> 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,
see  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.

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?



-- 
_
-- 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

Re: [asterisk-dev] SIP/SDP: ptime in translation module?

[Matthew Jordan]

On Sun, Oct 11, 2015 at 3:09 PM, Alexander Traud
 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  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 

Re: [asterisk-dev] SIP/SDP: ptime in translation module?

[Joshua Colp]

On 15-10-04 11:06 PM, Matthew Jordan wrote:





2) Another option would be to just store the framing on the frame
itself. Frames are not immutable; they are created by the RTP instance
(which has the framing for that instance available on it), and could
easily be set in ast_rtp_read. Since the framing negotiated is not
dependent on the format but on the media session itself, it seems more
appropriate to store it on the frames generated by that media session.
What's nice about this is that codec modules are fed frames already;
that means there's no additional lookups you'll have to do in your
codec module.

Of the two approaches I like #2 best, but some others may want to
chime in on this as well.


I also like #2 best.

--
Joshua Colp
Digium, Inc. | Senior Software Developer
445 Jan Davis Drive NW - Huntsville, AL 35806 - US
Check us out at: www.digium.com & www.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

Re: [asterisk-dev] SIP/SDP: ptime in translation module?

[Matthew Jordan]

On Fri, Oct 2, 2015 at 2:13 PM, Alexander Traud
 wrote:
> I am creating a translation module for AMR-WB. In one scenario on the
> SIP/SDP layer, a higher ptime was negotiated than the default one. For
> example, 60ms were negotiated instead of the AMR default 20ms. Now, Asterisk
> should send three frames per RTP packet. I try to play one of the recorded
> voices (slin8). Asterisk sends 320 samples to my translation module; the
> default for 20ms packetization. My translation module has to wait 960
> samples to create the frames.
>
> Which structure do I have to query: How do know the ptime, there in such a
> transcoding module? This was available in Asterisk 11 via
> ast_format->cur_ms. How do I access this information in Asterisk 13|master?
>
> I am asking asterisk-dev, because all transcoding modules might be affected.
> At least with AMR-WB, I have to code/build those frames within the module
> because RFC 4867 mandates a special header (section 4.3.5.2 and 4.4.5.1). At
> first glance, this could be an architectural issue. Therefore, I am asking
> for advice how to approach this: Simply, re-adding "cur_ms" to ast_format?
>

I don't think we want to add cur_ms back to ast_format, for two
reasons. The first is simply that packetization is negotiated at the
media level, and applies to all formats negotiated for that media
session. Hence why it was removed from the formats themselves - it
exists at a "higher" level.

Second, formats in 13+ are immutable by convention; this gave Asterisk
a big performance improvement as it didn't have to copy the format
structs all over the place. Unfortunately, that also means you don't
want to change them too often. Generally, once created for a channel,
you don't want to put more information in them or otherwise muck with
them. Modifying a format during negotiation can be done, but there's a
hit you pay for doing it at that point.

Currently, packetization is only stored on the RTP instance, as that
represents the media session that was negotiated along with all
formats in that media session.

If you need to get that information into a codec module, there's a few options:

1) Implement an attribute module for the format, such as
res_format_amr_wb. You can set arbitrary attributes on any given
format, and it will set those attributes in various callbacks into the
module. The callbacks are responsible for creating a new cloned
format, setting attributes on it in some private data structure, and
returning the clone. The format core is smart enough to not impact
"normal" formats using this approach. You can look at the existing
implementations, such as res_format_attr_opus, to get a hint on how to
do that. There are, however, a few issues with this approach:

a) By default, the format attribute modules expect to get their
attributes out of an 'fmtp' line in an SDP. The ptime attribute won't
be passed along with that. That means that you'll have to update
parsers of 'ptime' in the various channel drivers to call
ast_format_attribute_set with the ptime value, and handle the cloned
format appropriately.

b) You'll need to pull the format for the RTP instance's codecs out
using ast_rtp_codecs_get_payload_format, which may be tricky. Another
way of handling this approach would be to implement the call to
ast_format_attribute_set in ast_rtp_codecs_set_framing, as it knows
how to get at the formats stored in the RTP instance.

2) Another option would be to just store the framing on the frame
itself. Frames are not immutable; they are created by the RTP instance
(which has the framing for that instance available on it), and could
easily be set in ast_rtp_read. Since the framing negotiated is not
dependent on the format but on the media session itself, it seems more
appropriate to store it on the frames generated by that media session.
What's nice about this is that codec modules are fed frames already;
that means there's no additional lookups you'll have to do in your
codec module.

Of the two approaches I like #2 best, but some others may want to
chime in on this as well.

Matt

-- 
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