[asterisk-dev] [Code Review] 3722: media_formats: Move format attribute modules over, tweak API, and fix some bugs. (Round 2!)

[Matt Jordan]

> On July 9, 2014, 9:43 a.m., Corey Farrell wrote:
> > /team/group/media_formats-reviewed-trunk/channels/chan_sip.c, lines 13461-13463
> > <https://reviewboard.asterisk.org/r/3722/diff/1/?file=62462#file62462line13461>
> >
> >     This may warrant mention in UPGRADE.txt.  Not sure we want to touch UPGRADE.txt itself, or maybe start an UPGRADE-MF.txt (to avoid automerge conflicts).

I think a note of what may have changed is appropriate. I'll add one in the commit.


- Matt


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3722/#review12523
-----------------------------------------------------------


On July 7, 2014, 8:40 p.m., Matt Jordan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3722/
> -----------------------------------------------------------
> 
> (Updated July 7, 2014, 8:40 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23957
>     https://issues.asterisk.org/jira/browse/ASTERISK-23957
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This patch is identical to r3703, except it tweaks up chan_sip such that the SDP_attribute_passthrough tests pass (more on that below).
> 
> The major changes to format_cap are:
> * format_cap now has a method to get the preferred codec out of a capabilities structure by some type. This prevents the situation where the 'preferred codec' is not an audio codec (which can happen when format capabilities structures are copied, moved around, and perturbed).
> * format_cap now explicitly prevents duplicate formats from being appended. A format is considered a duplicate if it has the same codec ID. This is a 'strict' interpretation of equivalence, as it means that a format that contains an attribute cannot coexist with another format of the same type. This was needed, however, to prevent capabilities structures from having duplicate video formats (some with attributes, some without), which the channel drivers cannot yet understand or process (and which is not strictly needed right now).
> 
> The chan_sip channel driver has been tweaked up a bit to make sure that when a capability is built out from an SDP, that capability (which is the jointcaps structure) is given preference when constructing an outgoing SDP. That helps ensure that adding a format to an outbound SDP gets the appropriate format w/ attributes, as opposed to the peer formats which would not have any attributes.
> 
> Note that since the jointcaps capability is guaranteed to be a subset of the peer's capabilities, this ends up being generally the same.
> 
> 
> Diffs
> -----
> 
>   /team/group/media_formats-reviewed-trunk/res/res_pjsip_sdp_rtp.c 418167 
>   /team/group/media_formats-reviewed-trunk/res/res_format_attr_silk.c 418167 
>   /team/group/media_formats-reviewed-trunk/res/res_format_attr_opus.c 418167 
>   /team/group/media_formats-reviewed-trunk/res/res_format_attr_h264.c 418167 
>   /team/group/media_formats-reviewed-trunk/res/res_format_attr_h263.c 418167 
>   /team/group/media_formats-reviewed-trunk/res/res_format_attr_celt.c 418167 
>   /team/group/media_formats-reviewed-trunk/main/rtp_engine.c 418167 
>   /team/group/media_formats-reviewed-trunk/main/format_cap.c 418167 
>   /team/group/media_formats-reviewed-trunk/main/format.c 418167 
>   /team/group/media_formats-reviewed-trunk/include/asterisk/rtp_engine.h 418167 
>   /team/group/media_formats-reviewed-trunk/include/asterisk/format_cap.h 418167 
>   /team/group/media_formats-reviewed-trunk/include/asterisk/format.h 418167 
>   /team/group/media_formats-reviewed-trunk/channels/chan_sip.c 418167 
> 
> Diff: https://reviewboard.asterisk.org/r/3722/diff/
> 
> 
> Testing
> -------
> 
> The SDP_attribute_passthrough test can now pass, with some tweaks. The attributes are now added in a different order, but all attributes for speex, h263, and h264 are present.
> 
> 
> Thanks,
> 
> Matt Jordan
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140709/be013246/attachment.html>