[asterisk-dev] [Code Review] 3131: PJSIP: support 'allow=all'

[Mark Michelson]

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

Ship it!


Because my comments are so minor, I'm going ahead with a "ship it!" on the review.


/branches/12/main/format_pref.c
<https://reviewboard.asterisk.org/r/3131/#comment20123>

    And the award for most nit-picky critique on a code review goes to: <drumroll>
    
    Mark Michelson! For his comment: "The i.e. here should really be an e.g. since you are providing an example of an entry that could result in duplicates".



/branches/12/main/format_pref.c
<https://reviewboard.asterisk.org/r/3131/#comment20121>

    Coding guidelines: add spaces around the plus sign.



/branches/12/main/format_pref.c
<https://reviewboard.asterisk.org/r/3131/#comment20122>

    Coding guidelines: add spaces around the plus sign.


- Mark Michelson


On Jan. 16, 2014, 10:13 p.m., Scott Griepentrog wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3131/
> -----------------------------------------------------------
> 
> (Updated Jan. 16, 2014, 10:13 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23018
>     https://issues.asterisk.org/jira/browse/ASTERISK-23018
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Changes to support allow=all in PJSIP.conf: 
> 
> * added ast_codec_pref_append_all() that intelligently adds all yet unspecified codecs to the preference list
> 
> * Sorcery function ast_parse_allow_disallow() now uses codec_pref_append_all on pref list when all is supplied
> 
> * in create_outgoing_sdp_stream(), the loop checking codecs to add will now skip a codec that doesn't have a payload code instead of bailing (fixes issue with deferred sdp bombing on testlaw)
> 
> * switched display of codecs (reverse codec_handler_fn in sorcery.c) from caps to prefs, so that cli 'pjsip show endpoint x' shows allowed codec list in preference order matching the configuration
> 
> Note that it is now possible to put all at the end of an allow list, such as allow=ulaw,all which will set ulaw as preferred (first in list) but still have every other available codec listed.  Also possible is to remove specific codecs after all, such as allow=ulaw,all,!g729 which puts ulaw first, then every other codec, but removes g729.
> 
> The codec order otherwise is currently set by the internal codec table list order, which is arbitrary but functional.   
> 
> 
> Diffs
> -----
> 
>   /branches/12/res/res_pjsip_sdp_rtp.c 405637 
>   /branches/12/main/sorcery.c 405637 
>   /branches/12/main/frame.c 405637 
>   /branches/12/main/format_pref.c 405637 
>   /branches/12/include/asterisk/format_pref.h 405637 
> 
> Diff: https://reviewboard.asterisk.org/r/3131/diff/
> 
> 
> Testing
> -------
> 
> Tested with new test allow_all_sdp (https://reviewboard.asterisk.org/r/3132/) with success.  Also ran pjsip basic_calls. 
> 
> 
> Thanks,
> 
> Scott Griepentrog
> 
>

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