[asterisk-dev] A thread for format work

[Joshua Colp]

Matthew Jordan wrote:
>
> On Wed, Apr 30, 2014 at 5:59 AM, Joshua Colp <jcolp at digium.com
> <mailto:jcolp at digium.com>> wrote:
>
>     Matthew Jordan wrote:
>
>

<snip>

>
>
> If we allow attributes to be set after format creation, we have two options:
>
> (1) We make formats mutable, and all the badness that entails.
>
> (2) Setting a format implicitly dupes the existing format, adds the new
> attribute, then returns a new format.
>
> Going with option #2 (mutable == bad), I'm not sure how useful this is
> as a public operation. When you get an SDP, the attributes will be
> presented to you as a big ol' blob of text. You don't want to parse
> those outside of something that is creating the format - otherwise
> you'll end up with this incredibly bad code:
>
> format1 = ast_format_attribute_set(format, "foo", "bar");
> format2 = ast_format_attribute_set(format1, "yackity", "schmackity");
> format3 = ast_format_attribute_set(format2, "oh_nos", "the_pain");
> ao2_cleanup(format2);
> ao2_cleanup(format1)
> ao2_cleanup(format);
> format = format3;

I didn't mean replacing the SDP stuff with this. They are not 
equivalent. They are both different interfaces to the same thing, for 
different purposes.

> Ew.
>
> This, on the other hand, is slightly cleaner:
>
> new_format = ast_format_sdp_parse(format, my_sdp_with_attributes);
> ao2_cleanup(format);
> format = new_format;
>
> And is how both chan_sip and res_pjsip_sdp_rtp operate.
>
> While we may some day want to add a public function that adds
> attributes... YAGNI?

If you want to ditch it now, fine. It'll probably just end up getting 
resurrected in the future. :D

>
>
>         :: Getting Attributes ::
>
>         Since the format is read-only, it's safe to get the format
>         attributes
>         from a format object. There is one place where this is done using
>         ast_format_isset. ast_format_get_value is never used. As the
>         object is
>         immutable, a public accessor for an attribute key could be used that
>         returns a const char *, which would fulfil both functions.
>
>
>     The design currently gets away from the whole attribute key thing.
>     Attributes are stored in whatever way the format interface has
>     decided via an AO2 structure associated with the format itself. It
>     needs a way to set/get that.
>
>
>
> How so? The current design calls for a public function that accepts a
> const char *key and a const char *value. The key and value are always
> going to be presented to the core as text (inbound SDP), and typically
> will be generated as text (outgoing SDP). Yes, sometimes we may need to
> interpret the values as integers, or floats, or something - but that
> will typically be the domain of the resource modules. I'm not sure
> storing them as heterogeneous types is all that advantageous.

Two words: Format comparison. The more complex codecs aren't just a 
string comparison to know if they are equal, they get turned into a 
different type and then a comparison done on that. The amount of times a 
format comparison is done throughout the lifetime will also be greater 
than that of just interpreting/generating, I wager. The question is... 
which one do we want to be fast?

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