[asterisk-dev] A thread for format work

[Joshua Colp]

Matthew Jordan wrote:
> Like a normal person, I decided to poke at the format work while
> watching tornadoes track North and South of Huntsville (because really,
> what else are you going to do?) After looking through Josh's notes on
> the wiki [1] and the code in the team branch [2], I decided to tackle
> bridge_native_rtp (as it was the first thing that didn't compile). After
> hacking through it a bit, I had a few questions. Since this is going to
> be a recurring theme while working on this, a thread seemed in order.
>
> So: if you decide to work on this or take a poke at it and have some
> questions about things, feel free to respond to this thread.
>
> Anyway, without further ado:
>
> There is a portion of bridge_native_rtp that attempts to check if the
> channels are still compatible. If not, it breaks the native bridge so
> that the bridging framework can attempt a more suitable mixing
> technology. Part of the check looks at the packetization attribute of
> the formats in use:
>
>      read_ptime0 =
> (ast_codec_pref_getsize(&ast_rtp_instance_get_codecs(instance0)->pref,
> ast_channel_rawreadformat(c0->chan))).cur_ms;
>      read_ptime1 =
> (ast_codec_pref_getsize(&ast_rtp_instance_get_codecs(instance1)->pref,
> ast_channel_rawreadformat(c1->chan))).cur_ms;
>      write_ptime0 =
> (ast_codec_pref_getsize(&ast_rtp_instance_get_codecs(instance0)->pref,
> ast_channel_rawwriteformat(c0->chan))).cur_ms;
>      write_ptime1 =
> (ast_codec_pref_getsize(&ast_rtp_instance_get_codecs(instance1)->pref,
> ast_channel_rawwriteformat(c1->chan))).cur_ms;
>
>      if (read_ptime0 != write_ptime1 || read_ptime1 != write_ptime0) {
>          ast_debug(1, "Packetization differs between RTP streams (%d !=
> %d or %d != %d). Cannot native bridge in RTP\n",
>                  read_ptime0, write_ptime1, read_ptime1, write_ptime0);
>          return 0;
>      }
>
> That is, if the packetization between the read/write of each channel
> pair is not the same, break the bridge.
>
> Right now, there aren't compatible calls with this code in the branch.
> So... questions!
>
> (1) Where will the packetization of a format on a channel reside? With
> the format on the channel? Or with the capability of the channel?

It "has" to reside in the RTP engine layer and optionally can (but I've 
written it so it is) in the format capabilities layer.

It has to be in the RTP layer so that the underlying stack can know how 
to set up the smoother so packets contain the right amount of media.

It can optionally be in format capabilities so that further down the 
road the smoothing logic can be moved out of RTP and into the core.

> (2) Should we even be bothering with the codec's preferred packetization
> here? It doesn't really matter what a codec prefers at this point - if
> an SDP negotiated a different packetization rate, that's what we care
> about...

The naming of the structure above is misleading because it was extended. 
It contains negotiated codec preferences and packetization.

> (3) Are all these questions moot, and should I just go ahead and add the
> proposed framing size functions from the wiki:
>
> {quote}
> Framing size
>
> The framing size controls the length of media frames (in milliseconds).
> Previously this was stored in a separate structure but has now been
> rolled into ast_format_cap. To allow control two API calls will be added.
>
> void ast_format_cap_framing_set(struct ast_format_cap *cap, const struct
> ast_format *format, unsigned int framing);
> unsigned int ast_format_cap_framing_get(const struct ast_format_cap
> *cap, const struct ast_format *format);
> {quote}

The ability to set the framing per-format, globally in a capabilities 
structure, and to get the framing already exists. What doesn't exist is 
adding a format and then setting the framing as two separate API calls. 
That is something that could be added but to make sure you set the right 
format (format capabilities can contain multiple formats of the same 
codec) you have to keep that format around, in which case why not just 
add/set in the same API call?

Cheers,

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