> On Jan. 17, 2014, 4:46 p.m., George Joseph wrote:
> > Not sure this is a good idea.  First, I wouldn't hardcode a test for 
> > "disallow" in the generic ast_sip_cli_print_sorcery_objset and given the 
> > meaning of "disallow", the "!" doesn't really make sense.  It actually 
> > negates the disallow. :)
> > 
> >
> 
> Scott Griepentrog wrote:
>     This change is strictly for visual output of the endpoint object, and 
> only in the case where it is displayed in response to cli command "pjsip show 
> endpoint foobar".  There is no change to the underlying sorcery code or the 
> meanings of the elements.
>     
>     The purpose is to change this:
>     
>     disallow                      : (ulaw|alaw)
>     allow                         : (ulaw|alaw)
>     
>     Into this:
>     
>     disallow                      : !(ulaw|alaw)
>     allow                         : (ulaw|alaw)
>     
>     The issue is that the interpretation of the disallow/allow can be 
> slightly more confusing by saying we're simultaneously disallowing and 
> allowing the same codecs, vs. saying that we are disallowing everything 
> except the listed codecs, and allowing those codecs.
>     
>     Since the list of codecs comes from the same data source underneath 
> sorcery, i.e. the pjsip endpoint element media.caps and/or media.prefs, it is 
> difficult although not impossible to recreate what was actually spelled out 
> on the pjsip.conf file for the disallow value.   This is because both allow 
> and disallow get merged together (obviously disallow is inverted) in setting 
> media.caps/prefs.
>     
>     This is a small and easy change, affects CLI output only, avoids a 
> possible point of confusion, and also duplicates how the function 
> PJSIP_ENDPOINT() already works.
>     
>     My apologies, I should have spelled out the details of this in the 
> description a little better (was almost 5 on a Friday ;-).
>

This is going to be very confusing.  Right now, allow and disallow both show 
the exact same string, which granted is incorrect.  Putting a ! in front of the 
string for disallow isn't going to make it any less confusing.  

Why not change the pjsip config to match media.caps rather than trying to 
perpetuate the allow/disallow construct?  One config param named "codecs".

If the ! really needs to go in, can you put it in pjsip_configuration where the 
endpoint code is?  I tried hard to refactor pjsip_cli so it doesn't have any 
object specific code in it.  Also if another object wants to use "disallow" for 
something, they're going to get the same ! behavior.


- George


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


On Jan. 17, 2014, 3:46 p.m., Scott Griepentrog wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3136/
> -----------------------------------------------------------
> 
> (Updated Jan. 17, 2014, 3:46 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23092
>     https://issues.asterisk.org/jira/browse/ASTERISK-23092
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Insert a ! prefix in the display of endpoint disallow value.  Result is:
> 
>  disallow                      : !(ulaw|alaw)
> 
> 
> Diffs
> -----
> 
>   /branches/12/res/res_pjsip/pjsip_cli.c 405882 
> 
> Diff: https://reviewboard.asterisk.org/r/3136/diff/
> 
> 
> Testing
> -------
> 
> Ran command and checked output.
> 
> 
> Thanks,
> 
> Scott Griepentrog
> 
>

-- 
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

Reply via email to