<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Apr 30, 2014 at 8:00 AM, Joshua Colp <span dir="ltr"><<a href="mailto:jcolp@digium.com" target="_blank">jcolp@digium.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Matthew Jordan wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">
<br>
On Wed, Apr 30, 2014 at 5:59 AM, Joshua Colp <<a href="mailto:jcolp@digium.com" target="_blank">jcolp@digium.com</a><br></div><div class="">
<mailto:<a href="mailto:jcolp@digium.com" target="_blank">jcolp@digium.com</a>>> wrote:<br>
<br>
Matthew Jordan wrote:<br>
<br>
<br>
</div></blockquote>
<br>
<snip><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br><div class="">
If we allow attributes to be set after format creation, we have two options:<br>
<br>
(1) We make formats mutable, and all the badness that entails.<br>
<br>
(2) Setting a format implicitly dupes the existing format, adds the new<br>
attribute, then returns a new format.<br>
<br>
Going with option #2 (mutable == bad), I'm not sure how useful this is<br>
as a public operation. When you get an SDP, the attributes will be<br>
presented to you as a big ol' blob of text. You don't want to parse<br>
those outside of something that is creating the format - otherwise<br>
you'll end up with this incredibly bad code:<br>
<br>
format1 = ast_format_attribute_set(<u></u>format, "foo", "bar");<br>
format2 = ast_format_attribute_set(<u></u>format1, "yackity", "schmackity");<br>
format3 = ast_format_attribute_set(<u></u>format2, "oh_nos", "the_pain");<br>
ao2_cleanup(format2);<br>
ao2_cleanup(format1)<br>
ao2_cleanup(format);<br>
format = format3;<br>
</div></blockquote>
<br>
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.<div class=""><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Ew.<br>
<br>
This, on the other hand, is slightly cleaner:<br>
<br>
new_format = ast_format_sdp_parse(format, my_sdp_with_attributes);<br>
ao2_cleanup(format);<br>
format = new_format;<br>
<br>
And is how both chan_sip and res_pjsip_sdp_rtp operate.<br>
<br>
While we may some day want to add a public function that adds<br>
attributes... YAGNI?<br>
</blockquote>
<br></div>
If you want to ditch it now, fine. It'll probably just end up getting resurrected in the future. :D<div class=""><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
:: Getting Attributes ::<br>
<br>
Since the format is read-only, it's safe to get the format<br>
attributes<br>
from a format object. There is one place where this is done using<br>
ast_format_isset. ast_format_get_value is never used. As the<br>
object is<br>
immutable, a public accessor for an attribute key could be used that<br>
returns a const char *, which would fulfil both functions.<br>
<br>
<br>
The design currently gets away from the whole attribute key thing.<br>
Attributes are stored in whatever way the format interface has<br>
decided via an AO2 structure associated with the format itself. It<br>
needs a way to set/get that.<br>
<br>
<br>
<br>
How so? The current design calls for a public function that accepts a<br>
const char *key and a const char *value. The key and value are always<br>
going to be presented to the core as text (inbound SDP), and typically<br>
will be generated as text (outgoing SDP). Yes, sometimes we may need to<br>
interpret the values as integers, or floats, or something - but that<br>
will typically be the domain of the resource modules. I'm not sure<br>
storing them as heterogeneous types is all that advantageous.<br>
</blockquote>
<br></div>
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?<div class="HOEnZb">
<div class="h5"><br clear="all"></div></div></blockquote><div><br></div><div>True. OTOH, there is at least one place in the code (and maybe it should go away? I don't know) outside of the format core that does check to see if an attribute is set. We will have to 'deal with that', in some form or fashion. <br>
</div></div><br>-- <br><div dir="ltr"><div>Matthew Jordan<br></div><div>Digium, Inc. | Engineering Manager</div><div>445 Jan Davis Drive NW - Huntsville, AL 35806 - USA</div><div>Check us out at: <a href="http://digium.com" target="_blank">http://digium.com</a> & <a href="http://asterisk.org" target="_blank">http://asterisk.org</a></div>
</div>
</div></div>