[asterisk-dev] A thread for format work

[Matthew Jordan]

bridge_native_rtp up for review. One down, three hundred to go...

But onward anyway. Next on the "list": format attributes!

The new specification calls for setting attributes using
ast_format_set_attr, which takes in a key/value pair along with the format
to set the attribute on. While this seems like the appropriate place to
store the attribute (since the attribute describes the format), there's
some problems with setting the attribute after format creation. Namely:

(1) A format object is immutable by convention. However, setting an
attribute on a format is a mutable operation, which means we either (a)
should set all attributes on format creation or (b) setting an attribute
actually returns a copy of the format. Both of these approaches have
drawbacks.

(2) Setting an attribute technically gets mapped back to a format attribute
interface. This interface shouldn't require massive changes, but does need
to be exposed through the format API as it generates the SDP attributes.
How we expose that may need some tweaking, as ideally something using the
format API won't care too much how it gets the SDP - just that it gets it.

:: Setting Attributes ::

The biggest sticky point is clearly #1. I don't think we should lose the
immutability convention - that feels bad, since there's a lot of benefit to
treating a format as a read-only object (no locking!) - and a format
shouldn't change very often. In order to maintain mutability, I'd remove
the public format attribute setting function and instead do the following:

(1) We set attributes only as the result of parsing an SDP. Currently, this
is the only time we set attributes. ast_format_sdp_parse would be modified
to return an ast_format. Calling that function should result in either NULL
(failure) or a new ast_format, which should replace the format passed to
the function (which would be the burden of the channel driver). Most of the
work of duplicating the existing format and safely putting formats onto it
could happen within format.c. In that case, there isn't much need for an
individual 'add this format' function.

(2) Since the only time we add attributes is during SDP generation, there
isn't any need for a public API that sets formats. The variable parameters
that can be passed to ast_format_set are - from what I can tell - never
passed.

(3) Producing the attributes is really just a matter of generating the SDP.
This should be a public function in format.h that maps down to the
attribute interface.

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

Thoughts? Flames?

-- 
Matthew Jordan
Digium, Inc. | Engineering Manager
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
Check us out at: http://digium.com & http://asterisk.org
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140429/47d444e9/attachment-0001.html>