Re: [asterisk-dev] [Code Review] 3184: Create sorcery instance registry

[George Joseph]

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

(Updated Feb. 6, 2014, 11:30 p.m.)


Review request for Asterisk Developers.


Changes
---

Moved lock/unlock.


Bugs: ASTERISK-22537
https://issues.asterisk.org/jira/browse/ASTERISK-22537


Repository: Asterisk


Description
---

Create sorcery instance registry as a precursor to creating a generic dialplan 
function that can retrieve parameters from a sorcery-based config file.

ast_sorcery_init now creates a hashtab as a registry.
ast_sorcery_open now checks the hashtab for an existing sorcery instance 
matching the caller's module name.  If it finds one, it bumps the refcount and 
returns it.  If not, it creates a new sorcery instance, adds it to the hashtab, 
then returns it.
ast_sorcery_retrieve_by_module_name is a new function that does a hashtab 
lookup by module name.  It can be called by the future dialplan function.

A side effect of this patch is that a module can only have 1 sorcery instance 
(because it's the key for the hashtab).  res_pjsip/config_system needed a small 
change to share the main res_pjsip sorcery instance.


Diffs (updated)
-

  branches/12/tests/test_sorcery.c 407566 
  branches/12/res/res_pjsip/include/res_pjsip_private.h 407566 
  branches/12/res/res_pjsip/config_system.c 407566 
  branches/12/res/res_pjsip.c 407566 
  branches/12/main/sorcery.c 407566 
  branches/12/include/asterisk/sorcery.h 407566 

Diff: https://reviewboard.asterisk.org/r/3184/diff/


Testing
---

Made sure that users of sorcery (mostly res_pjsip) continued to load their 
configs correctly.
Made sure there were no ill effects on res_pjsip from config_system sharing the 
same sorcery instance as the rest of the pjsip infrastructure.
Made sure that config_system was properly marked as 'not reloadable' and that 
it was maintaining it's original values when res_pjsip was reloaded.


ernie*CLI> test execute category /main/sorcery/
Running all available tests matching category /main/sorcery/

START  /main/sorcery/ - configuration_file_wizard_retrieve_multiple_all 
END/main/sorcery/ - configuration_file_wizard_retrieve_multiple_all Time: 
<1ms Result: PASS
[snip]
START  /main/sorcery/ - open 
END/main/sorcery/ - open Time: <1ms Result: PASS
START  /main/sorcery/ - wizard_registration 
END/main/sorcery/ - wizard_registration Time: <1ms Result: PASS

43 Test(s) Executed  43 Passed  0 Failed


Thanks,

George Joseph

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3178: media_formats: Moving of existing code around, implementation, and unit tests

[rmudgett]

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



/branches/12/include/asterisk/codec.h


These should be documented like a function since they are templates for the 
functions.



/branches/12/include/asterisk/format_ng.h


Same here for the callback functions.



/branches/12/include/asterisk/vector.h


This macro also needs a warning that it doesn't handle the case where the 
position you are inserting is already in use.



/branches/12/main/codec.c


You need to use the AO2_ITERATOR_DONTLOCK flag since you have the container 
locked.



/branches/12/main/codec.c


add
codecs = NULL;



/branches/12/main/codec_builtin.c


Is there a
lpc10_lenght()
needed?



/branches/12/main/codec_builtin.c


get_length missing?



/branches/12/main/codec_builtin.c


get_length missing?



/branches/12/main/codec_builtin.c


get_length missing?



/branches/12/main/codec_builtin.c


get_length missing?



/branches/12/main/codec_builtin.c


Is this to be a passthrough codec or is there missing get_samples and 
get_lenght functions?



/branches/12/main/format_cache.c


add
formats = NULL;


- rmudgett


On Feb. 4, 2014, 9:57 a.m., Joshua Colp wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3178/
> ---
> 
> (Updated Feb. 4, 2014, 9:57 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> This change has a few things in it:
> 
> 1. Some media related things have been moved around to more logical places or 
> their own parts (smoothers).
> 
> 2. A new implementation of media formats according to 
> https://wiki.asterisk.org/wiki/display/AST/Media+Format+Rewrite. The 
> implementation doesn't completely adhere to the design since I tweaked things 
> but it mostly conforms.
> 
> 3. Unit tests for the above implementation.
> 
> What I'd like feedback on is the actual media formats implementation and the 
> API design itself. Is this something you would be comfortable using?
> 
> 
> Diffs
> -
> 
>   /branches/12/tests/test_format_cap.c PRE-CREATION 
>   /branches/12/tests/test_format_cache.c PRE-CREATION 
>   /branches/12/tests/test_core_format.c PRE-CREATION 
>   /branches/12/tests/test_core_codec.c PRE-CREATION 
>   /branches/12/res/res_rtp_asterisk.c 406006 
>   /branches/12/res/res_fax.c 406006 
>   /branches/12/main/smoother.c PRE-CREATION 
>   /branches/12/main/frame.c 406006 
>   /branches/12/main/format_ng.c PRE-CREATION 
>   /branches/12/main/format_cap_ng.c PRE-CREATION 
>   /branches/12/main/format_cache.c PRE-CREATION 
>   /branches/12/main/format.c 406006 
>   /branches/12/main/codec_builtin.c PRE-CREATION 
>   /branches/12/main/codec.c PRE-CREATION 
>   /branches/12/main/asterisk.c 406006 
>   /branches/12/include/asterisk/vector.h 406006 
>   /branches/12/include/asterisk/smoother.h PRE-CREATION 
>   /branches/12/include/asterisk/frame.h 406006 
>   /branches/12/include/asterisk/format_ng.h PRE-CREATION 
>   /branches/12/include/asterisk/format_cap_ng.h PRE-CREATION 
>   /branches/12/include/asterisk/format_cache.h PRE-CREATION 
>   /branches/12/include/asterisk/format.h 406006 
>   /branches/12/include/asterisk/codec.h PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/3178/diff/
> 
> 
> Testing
> ---
> 
> Ran unit tests, all passed.
> 
> Note: I know AO2 throws a fit and it's because a container isn't getting 
> initialized. Getting said container initialized requires beginning the 
> hacking apart process.
> 
> 
> Thanks,
> 
> Joshua Colp
> 
>

-- 
_
-- 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

Re: [asterisk-dev] [r400723-400741] ConfBridge now has the ability to set the language of announcements

[Richard Mudgett]

On Thu, Feb 6, 2014 at 6:40 PM, Jonathan White  wrote:

>   Good afternoon. Thanks for adding this feature. I have been testing it 
> today and notice some unexpected behaviour.
>
> When multiple users call in and set different languages they will only hear 
> the language set by the first caller to join the conference.
>
> Is this the expected and desired behaviour?
>
> I would have expected the desired result would be for each caller who sets 
> their own language to hear the prompts they selected not the language of the 
> first caller.
>
> That option sets the language of the conference bridge itself so any
prompts
played to the bridge get played in the selected language of the bridge.
Prompts
played to the bridge are heard by everyone in the bridge at the same time.
Prompts played to a specific user would be in that users language and are
heard by that user only.

Richard
-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3184: Create sorcery instance registry

[rmudgett]

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



branches/12/main/sorcery.c


Unfortunately, to avoid the race you have to lock the container before 
unrefing sorcery.
ao2_wrlock()
if (ao2_ref()) {
  ao2_unlink()
}
ao2_unlock()


- rmudgett


On Feb. 6, 2014, 6:59 p.m., George Joseph wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3184/
> ---
> 
> (Updated Feb. 6, 2014, 6:59 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22537
> https://issues.asterisk.org/jira/browse/ASTERISK-22537
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Create sorcery instance registry as a precursor to creating a generic 
> dialplan function that can retrieve parameters from a sorcery-based config 
> file.
> 
> ast_sorcery_init now creates a hashtab as a registry.
> ast_sorcery_open now checks the hashtab for an existing sorcery instance 
> matching the caller's module name.  If it finds one, it bumps the refcount 
> and returns it.  If not, it creates a new sorcery instance, adds it to the 
> hashtab, then returns it.
> ast_sorcery_retrieve_by_module_name is a new function that does a hashtab 
> lookup by module name.  It can be called by the future dialplan function.
> 
> A side effect of this patch is that a module can only have 1 sorcery instance 
> (because it's the key for the hashtab).  res_pjsip/config_system needed a 
> small change to share the main res_pjsip sorcery instance.
> 
> 
> Diffs
> -
> 
>   branches/12/tests/test_sorcery.c 407566 
>   branches/12/res/res_pjsip/include/res_pjsip_private.h 407566 
>   branches/12/res/res_pjsip/config_system.c 407566 
>   branches/12/res/res_pjsip.c 407566 
>   branches/12/main/sorcery.c 407566 
>   branches/12/include/asterisk/sorcery.h 407566 
> 
> Diff: https://reviewboard.asterisk.org/r/3184/diff/
> 
> 
> Testing
> ---
> 
> Made sure that users of sorcery (mostly res_pjsip) continued to load their 
> configs correctly.
> Made sure there were no ill effects on res_pjsip from config_system sharing 
> the same sorcery instance as the rest of the pjsip infrastructure.
> Made sure that config_system was properly marked as 'not reloadable' and that 
> it was maintaining it's original values when res_pjsip was reloaded.
> 
> 
> ernie*CLI> test execute category /main/sorcery/
> Running all available tests matching category /main/sorcery/
> 
> START  /main/sorcery/ - configuration_file_wizard_retrieve_multiple_all 
> END/main/sorcery/ - configuration_file_wizard_retrieve_multiple_all Time: 
> <1ms Result: PASS
> [snip]
> START  /main/sorcery/ - open 
> END/main/sorcery/ - open Time: <1ms Result: PASS
> START  /main/sorcery/ - wizard_registration 
> END/main/sorcery/ - wizard_registration Time: <1ms Result: PASS
> 
> 43 Test(s) Executed  43 Passed  0 Failed
> 
> 
> Thanks,
> 
> George Joseph
> 
>

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3184: Create sorcery instance registry

[George Joseph]

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

(Updated Feb. 6, 2014, 5:59 p.m.)


Review request for Asterisk Developers.


Changes
---

Latest corrections...
Locking around the unlink in ast_sorcery_unref and test correction.


Bugs: ASTERISK-22537
https://issues.asterisk.org/jira/browse/ASTERISK-22537


Repository: Asterisk


Description
---

Create sorcery instance registry as a precursor to creating a generic dialplan 
function that can retrieve parameters from a sorcery-based config file.

ast_sorcery_init now creates a hashtab as a registry.
ast_sorcery_open now checks the hashtab for an existing sorcery instance 
matching the caller's module name.  If it finds one, it bumps the refcount and 
returns it.  If not, it creates a new sorcery instance, adds it to the hashtab, 
then returns it.
ast_sorcery_retrieve_by_module_name is a new function that does a hashtab 
lookup by module name.  It can be called by the future dialplan function.

A side effect of this patch is that a module can only have 1 sorcery instance 
(because it's the key for the hashtab).  res_pjsip/config_system needed a small 
change to share the main res_pjsip sorcery instance.


Diffs (updated)
-

  branches/12/tests/test_sorcery.c 407566 
  branches/12/res/res_pjsip/include/res_pjsip_private.h 407566 
  branches/12/res/res_pjsip/config_system.c 407566 
  branches/12/res/res_pjsip.c 407566 
  branches/12/main/sorcery.c 407566 
  branches/12/include/asterisk/sorcery.h 407566 

Diff: https://reviewboard.asterisk.org/r/3184/diff/


Testing
---

Made sure that users of sorcery (mostly res_pjsip) continued to load their 
configs correctly.
Made sure there were no ill effects on res_pjsip from config_system sharing the 
same sorcery instance as the rest of the pjsip infrastructure.
Made sure that config_system was properly marked as 'not reloadable' and that 
it was maintaining it's original values when res_pjsip was reloaded.


ernie*CLI> test execute category /main/sorcery/
Running all available tests matching category /main/sorcery/

START  /main/sorcery/ - configuration_file_wizard_retrieve_multiple_all 
END/main/sorcery/ - configuration_file_wizard_retrieve_multiple_all Time: 
<1ms Result: PASS
[snip]
START  /main/sorcery/ - open 
END/main/sorcery/ - open Time: <1ms Result: PASS
START  /main/sorcery/ - wizard_registration 
END/main/sorcery/ - wizard_registration Time: <1ms Result: PASS

43 Test(s) Executed  43 Passed  0 Failed


Thanks,

George Joseph

-- 
_
-- 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

[asterisk-dev] [r400723-400741] ConfBridge now has the ability to set the language of announcements

[Jonathan White]

Good afternoon. Thanks for adding this feature. I have been testing it today 
and notice some unexpected behaviour.When multiple users call in and set 
different languages they will only hear the language set by the first caller to 
join the conference.Is this the expected and desired behaviour?I would have 
expected the desired result would be for each caller who sets their own 
language to hear the prompts they selected not the language of the first 
caller.Below is are a copy of the relating release notesMany thanks 2013-10-08 
20:14 + [r400723-400741]  Richard Mudgett 

* UPGRADE.txt, apps/app_confbridge.c,
  apps/confbridge/conf_config_parser.c,
  configs/confbridge.conf.sample,
  apps/confbridge/include/confbridge.h: app_confbridge: Can now set
  the language used for announcements to the conference. ConfBridge
  now has the ability to set the language of announcements to the
  conference. The language can be set on a bridge profile in
  confbridge.conf or by the dialplan function
  CONFBRIDGE(bridge,language)=en. (closes issue ASTERISK-19983)
  Reported by: Jonathan White Patches: M19983_rev2.diff (license
  #5138) patch uploaded by junky (modified) Tested by: rmudgett

* apps/confbridge/conf_config_parser.c: app_confbridge: Fix
  duplicate default_user profile. * Fixed looking in the wrong
  profiles container to see if the default_user profile is already
  created in verify_default_profiles(). The bridge profile
  container is never going to hold user profiles. :)

---
This email is free from viruses and malware because avast! Antivirus protection 
is active.
http://www.avast.com
-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3184: Create sorcery instance registry

[rmudgett]

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



branches/12/main/sorcery.c


Oops a bit of a race condition between ast_sorcery_unref() and 
ast_sorcery_open()/ast_sorcery_retrieve_by_module_name().

Need to ao2_wrlock() before unref/unlink
ao2_unlink_flags(instances, sorcery, OBJ_NOLOCK)
then unlock



branches/12/tests/test_sorcery.c


Setting sorcery and sorcery2 to NULL should be done after unrefing them.

Though you should assign sorcery the returned value of 
ast_sorcery_retrive_by_module_name() in case it actually returns a sorcery 
object.


- rmudgett


On Feb. 6, 2014, 5:45 p.m., George Joseph wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3184/
> ---
> 
> (Updated Feb. 6, 2014, 5:45 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22537
> https://issues.asterisk.org/jira/browse/ASTERISK-22537
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Create sorcery instance registry as a precursor to creating a generic 
> dialplan function that can retrieve parameters from a sorcery-based config 
> file.
> 
> ast_sorcery_init now creates a hashtab as a registry.
> ast_sorcery_open now checks the hashtab for an existing sorcery instance 
> matching the caller's module name.  If it finds one, it bumps the refcount 
> and returns it.  If not, it creates a new sorcery instance, adds it to the 
> hashtab, then returns it.
> ast_sorcery_retrieve_by_module_name is a new function that does a hashtab 
> lookup by module name.  It can be called by the future dialplan function.
> 
> A side effect of this patch is that a module can only have 1 sorcery instance 
> (because it's the key for the hashtab).  res_pjsip/config_system needed a 
> small change to share the main res_pjsip sorcery instance.
> 
> 
> Diffs
> -
> 
>   branches/12/tests/test_sorcery.c 407566 
>   branches/12/res/res_pjsip/include/res_pjsip_private.h 407566 
>   branches/12/res/res_pjsip/config_system.c 407566 
>   branches/12/res/res_pjsip.c 407566 
>   branches/12/main/sorcery.c 407566 
>   branches/12/include/asterisk/sorcery.h 407566 
> 
> Diff: https://reviewboard.asterisk.org/r/3184/diff/
> 
> 
> Testing
> ---
> 
> Made sure that users of sorcery (mostly res_pjsip) continued to load their 
> configs correctly.
> Made sure there were no ill effects on res_pjsip from config_system sharing 
> the same sorcery instance as the rest of the pjsip infrastructure.
> Made sure that config_system was properly marked as 'not reloadable' and that 
> it was maintaining it's original values when res_pjsip was reloaded.
> 
> 
> ernie*CLI> test execute category /main/sorcery/
> Running all available tests matching category /main/sorcery/
> 
> START  /main/sorcery/ - configuration_file_wizard_retrieve_multiple_all 
> END/main/sorcery/ - configuration_file_wizard_retrieve_multiple_all Time: 
> <1ms Result: PASS
> [snip]
> START  /main/sorcery/ - open 
> END/main/sorcery/ - open Time: <1ms Result: PASS
> START  /main/sorcery/ - wizard_registration 
> END/main/sorcery/ - wizard_registration Time: <1ms Result: PASS
> 
> 43 Test(s) Executed  43 Passed  0 Failed
> 
> 
> Thanks,
> 
> George Joseph
> 
>

-- 
_
-- 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

Re: [asterisk-dev] Asterisk 12.1.0-rc1 Now Available

[Paul Belanger]

On Thu, Feb 6, 2014 at 6:07 PM, Asterisk Development Team
 wrote:
> The Asterisk Development Team has announced the first release candidate of
> Asterisk 12.1.0. This release candidate is available for immediate
> download at http://downloads.asterisk.org/pub/telephony/asterisk
>
> The release of Asterisk 12.1.0-rc1 resolves several issues reported by the
> community and would have not been possible without your participation.
> Thank you!
>
> The following is a sample of the issues resolved in this release candidate:
>
> * --- pjsip: fix support for allow=all
>   (Closes issue ASTERISK-23018. Reported by xrobau)
>
> * --- res_pjsip_session: Be less strict with core requested outgoing
>   capabilities.
>   (Closes issue ASTERISK-23082. Reported by xrobau)
>
> * --- res_stasis: Enable transfers and provide events when they occur.
>   (Closes issue ASTERISK-22984. Reported by David M. Lee)
>
> * --- ARI: Support channel variables in originate
>   (Closes issue ASTERISK-23051. Reported by Matt Jordan)
>
Have we done anything to bump the ARI versioning for the channel
variable changes?  What is the best way to detect this change from an
library POV?

> * --- PJSIP: Fix address for ACK in NAT situations
>   (Closes issue ASTERISK-23106. Reported by Matt Jordan)
>
> For a full list of changes in this release candidate, please see the 
> ChangeLog:
>
> http://downloads.asterisk.org/pub/telephony/Asterisk/ChangeLog-12.1.0-rc1
>
> Thank you for your continued support of Asterisk!
>
> --
> _
> -- 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



-- 
Paul Belanger | PolyBeacon, Inc.
Jabber: paul.belan...@polybeacon.com | IRC: pabelanger (Freenode)
Github: https://github.com/pabelanger | Twitter: https://twitter.com/pabelanger

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3184: Create sorcery instance registry

[George Joseph]

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

(Updated Feb. 6, 2014, 4:45 p.m.)


Review request for Asterisk Developers.


Changes
---

This one actually compiles. :)


Bugs: ASTERISK-22537
https://issues.asterisk.org/jira/browse/ASTERISK-22537


Repository: Asterisk


Description
---

Create sorcery instance registry as a precursor to creating a generic dialplan 
function that can retrieve parameters from a sorcery-based config file.

ast_sorcery_init now creates a hashtab as a registry.
ast_sorcery_open now checks the hashtab for an existing sorcery instance 
matching the caller's module name.  If it finds one, it bumps the refcount and 
returns it.  If not, it creates a new sorcery instance, adds it to the hashtab, 
then returns it.
ast_sorcery_retrieve_by_module_name is a new function that does a hashtab 
lookup by module name.  It can be called by the future dialplan function.

A side effect of this patch is that a module can only have 1 sorcery instance 
(because it's the key for the hashtab).  res_pjsip/config_system needed a small 
change to share the main res_pjsip sorcery instance.


Diffs (updated)
-

  branches/12/tests/test_sorcery.c 407566 
  branches/12/res/res_pjsip/include/res_pjsip_private.h 407566 
  branches/12/res/res_pjsip/config_system.c 407566 
  branches/12/res/res_pjsip.c 407566 
  branches/12/main/sorcery.c 407566 
  branches/12/include/asterisk/sorcery.h 407566 

Diff: https://reviewboard.asterisk.org/r/3184/diff/


Testing
---

Made sure that users of sorcery (mostly res_pjsip) continued to load their 
configs correctly.
Made sure there were no ill effects on res_pjsip from config_system sharing the 
same sorcery instance as the rest of the pjsip infrastructure.
Made sure that config_system was properly marked as 'not reloadable' and that 
it was maintaining it's original values when res_pjsip was reloaded.


ernie*CLI> test execute category /main/sorcery/
Running all available tests matching category /main/sorcery/

START  /main/sorcery/ - configuration_file_wizard_retrieve_multiple_all 
END/main/sorcery/ - configuration_file_wizard_retrieve_multiple_all Time: 
<1ms Result: PASS
[snip]
START  /main/sorcery/ - open 
END/main/sorcery/ - open Time: <1ms Result: PASS
START  /main/sorcery/ - wizard_registration 
END/main/sorcery/ - wizard_registration Time: <1ms Result: PASS

43 Test(s) Executed  43 Passed  0 Failed


Thanks,

George Joseph

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3184: Create sorcery instance registry

[George Joseph]

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

(Updated Feb. 6, 2014, 4:42 p.m.)


Review request for Asterisk Developers.


Changes
---

Incorporated comments from rmudgett.


Bugs: ASTERISK-22537
https://issues.asterisk.org/jira/browse/ASTERISK-22537


Repository: Asterisk


Description
---

Create sorcery instance registry as a precursor to creating a generic dialplan 
function that can retrieve parameters from a sorcery-based config file.

ast_sorcery_init now creates a hashtab as a registry.
ast_sorcery_open now checks the hashtab for an existing sorcery instance 
matching the caller's module name.  If it finds one, it bumps the refcount and 
returns it.  If not, it creates a new sorcery instance, adds it to the hashtab, 
then returns it.
ast_sorcery_retrieve_by_module_name is a new function that does a hashtab 
lookup by module name.  It can be called by the future dialplan function.

A side effect of this patch is that a module can only have 1 sorcery instance 
(because it's the key for the hashtab).  res_pjsip/config_system needed a small 
change to share the main res_pjsip sorcery instance.


Diffs (updated)
-

  branches/12/tests/test_sorcery.c 407566 
  branches/12/res/res_pjsip/include/res_pjsip_private.h 407566 
  branches/12/res/res_pjsip/config_system.c 407566 
  branches/12/res/res_pjsip.c 407566 
  branches/12/main/sorcery.c 407566 
  branches/12/include/asterisk/sorcery.h 407566 

Diff: https://reviewboard.asterisk.org/r/3184/diff/


Testing
---

Made sure that users of sorcery (mostly res_pjsip) continued to load their 
configs correctly.
Made sure there were no ill effects on res_pjsip from config_system sharing the 
same sorcery instance as the rest of the pjsip infrastructure.
Made sure that config_system was properly marked as 'not reloadable' and that 
it was maintaining it's original values when res_pjsip was reloaded.


ernie*CLI> test execute category /main/sorcery/
Running all available tests matching category /main/sorcery/

START  /main/sorcery/ - configuration_file_wizard_retrieve_multiple_all 
END/main/sorcery/ - configuration_file_wizard_retrieve_multiple_all Time: 
<1ms Result: PASS
[snip]
START  /main/sorcery/ - open 
END/main/sorcery/ - open Time: <1ms Result: PASS
START  /main/sorcery/ - wizard_registration 
END/main/sorcery/ - wizard_registration Time: <1ms Result: PASS

43 Test(s) Executed  43 Passed  0 Failed


Thanks,

George Joseph

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3104: PJSIP CLI Part 2

[George Joseph]

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

(Updated Feb. 6, 2014, 5:33 p.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Repository: Asterisk


Description
---

Adds identify, transport and registration support to the CLI.
Creates 3 additional callbacks, one for an iterator, one for a comparator and 
one for a container.  This eliminates the link dependency from higher level 
modules to lower level ones.
Eliminates duplicate sorting.
Cleans up output formatting.
Pushes cli registration down to the implementing source file.
Adds several ast_sip_destroy_sorcery functions to complement existing 
ast_sip_sorcery_initialize functions.  The destroy functions unregister cli 
commands and cli formatters.


Diffs
-

  branches/12/res/res_pjsip_outbound_registration.c 406802 
  branches/12/res/res_pjsip_endpoint_identifier_ip.c 406802 
  branches/12/res/res_pjsip/pjsip_options.c 406802 
  branches/12/res/res_pjsip/pjsip_configuration.c 406802 
  branches/12/res/res_pjsip/pjsip_cli.c 406802 
  branches/12/res/res_pjsip/location.c 406802 
  branches/12/res/res_pjsip/include/res_pjsip_private.h 406802 
  branches/12/res/res_pjsip/config_transport.c 406802 
  branches/12/res/res_pjsip/config_global.c 406802 
  branches/12/res/res_pjsip/config_domain_aliases.c 406802 
  branches/12/res/res_pjsip/config_auth.c 406802 
  branches/12/include/asterisk/res_pjsip_cli.h 406802 
  branches/12/include/asterisk/res_pjsip.h 406802 

Diff: https://reviewboard.asterisk.org/r/3104/diff/


Testing
---


Thanks,

George Joseph

-- 
_
-- 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

Re: [asterisk-dev] Asterisk 12.1.0-rc1 Now Available

[Daniel Jenkins]

On Feb 6, 2014 11:26 PM, "Daniel Jenkins"  wrote:
>
> The change log link doesn't seem to be right,
>
> This seems to be the correct one
>
> http://downloads.asterisk.org/pub/telephony/asterisk/ChangeLog-12.1.0-rc1
>
> On Feb 6, 2014 11:15 PM, "Asterisk Development Team" <
asteriskt...@digium.com> wrote:
>>
>> The Asterisk Development Team has announced the first release candidate
of
>> Asterisk 12.1.0. This release candidate is available for immediate
>> download at http://downloads.asterisk.org/pub/telephony/asterisk
>>
>> The release of Asterisk 12.1.0-rc1 resolves several issues reported by
the
>> community and would have not been possible without your participation.
>> Thank you!
>>
>> The following is a sample of the issues resolved in this release
candidate:
>>
>> * --- pjsip: fix support for allow=all
>>   (Closes issue ASTERISK-23018. Reported by xrobau)
>>
>> * --- res_pjsip_session: Be less strict with core requested outgoing
>>   capabilities.
>>   (Closes issue ASTERISK-23082. Reported by xrobau)
>>
>> * --- res_stasis: Enable transfers and provide events when they occur.
>>   (Closes issue ASTERISK-22984. Reported by David M. Lee)
>>
>> * --- ARI: Support channel variables in originate
>>   (Closes issue ASTERISK-23051. Reported by Matt Jordan)
>>
>> * --- PJSIP: Fix address for ACK in NAT situations
>>   (Closes issue ASTERISK-23106. Reported by Matt Jordan)
>>
>> For a full list of changes in this release candidate, please see the
ChangeLog:
>>
>> http://downloads.asterisk.org/pub/telephony/Asterisk/ChangeLog-12.1.0-rc1
>>
>> Thank you for your continued support of Asterisk!
>>
>> --
>> _
>> -- 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

Sorry! Stupid phone top posted ( workman blames his tools!)
-- 
_
-- 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

Re: [asterisk-dev] Asterisk 12.1.0-rc1 Now Available

[Daniel Jenkins]

The change log link doesn't seem to be right,

This seems to be the correct one

http://downloads.asterisk.org/pub/telephony/asterisk/ChangeLog-12.1.0-rc1
On Feb 6, 2014 11:15 PM, "Asterisk Development Team" <
asteriskt...@digium.com> wrote:

> The Asterisk Development Team has announced the first release candidate of
> Asterisk 12.1.0. This release candidate is available for immediate
> download at http://downloads.asterisk.org/pub/telephony/asterisk
>
> The release of Asterisk 12.1.0-rc1 resolves several issues reported by the
> community and would have not been possible without your participation.
> Thank you!
>
> The following is a sample of the issues resolved in this release candidate:
>
> * --- pjsip: fix support for allow=all
>   (Closes issue ASTERISK-23018. Reported by xrobau)
>
> * --- res_pjsip_session: Be less strict with core requested outgoing
>   capabilities.
>   (Closes issue ASTERISK-23082. Reported by xrobau)
>
> * --- res_stasis: Enable transfers and provide events when they occur.
>   (Closes issue ASTERISK-22984. Reported by David M. Lee)
>
> * --- ARI: Support channel variables in originate
>   (Closes issue ASTERISK-23051. Reported by Matt Jordan)
>
> * --- PJSIP: Fix address for ACK in NAT situations
>   (Closes issue ASTERISK-23106. Reported by Matt Jordan)
>
> For a full list of changes in this release candidate, please see the
> ChangeLog:
>
> http://downloads.asterisk.org/pub/telephony/Asterisk/ChangeLog-12.1.0-rc1
>
> Thank you for your continued support of Asterisk!
>
> --
> _
> -- 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
>
-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3184: Create sorcery instance registry

[rmudgett]

> On Feb. 6, 2014, 2:21 p.m., rmudgett wrote:
> > branches/12/main/sorcery.c, lines 1630-1633
> > 
> >
> > There are two red flags here:
> > 1) You are using sorcery after you have released your ref to it.
> > 
> > 2) Any time you are looking at the number of references left, there is 
> > a design problem.  In this case, only the owner of the sorcery object 
> > should close/destroy it which will remove it from the instances container.  
> > Only the owner of the sorcery object should open/create it which will add 
> > it to the instances container.  Everyone else will search the instances 
> > container for the object to get a reference.
> 
> George Joseph wrote:
> There's no concept of weak references so when the sorcery instance is 
> linked to the registry container, the refcount gets bumped without the actual 
> consumer knowing.  When the last consumer releases the instance, the 
> instance's refcount is still 1 so the destructor will never run and and 
> remove the entry.  Instead of checking the refcount I could simply decrement 
> it automatically after inserting the instance to the container.  This way the 
> last consumer to release it will trigger the cleanup.
> 
> Now, if you're saying that only 1 consumer should ever call 
> ast_sorcery_open then I need to rethink.  The original idea was that multiple 
> consumers could simply call ast_sorcery_open and let it figure out if it 
> needed to create a new one or return the existing one.
> 
>
> 
> rmudgett wrote:
> You cannot decrement the ref that is held by the container without 
> causing problems for the container when it decrements the the ref on an 
> unlink.  You would get a negative ref count.
> 
> I was thinking only one creator/owner should ever call the open/destroy.
> 
> Bridges and channels have the same kind of problem since they are also 
> put into a global container.  They have trigger events to know when they need 
> to be destroyed so they can unlink from the global container.
> 
> George Joseph wrote:
> Yeah, -1 to the auto -1. :)
> 
> So I'd have to create a balancing ast_sorcery_close() that did a complete 
> clean of the sorcery instance.  Should it clean and destroy (possibly pulling 
> it out from under someone who did a retrieve on it but never -1'd it) or just 
> remove it from the registry and let the last -1 clean it up?
> 
> What should happen if the same module (hence the same key) tries to open 
> twice?
> 
> BTW, the sorcery instance opened by res_pjsip/config_system gets passed 
> all over pjsip-land, even to other modules, without anyone ever doing a +1 on 
> it.I'd want to go clean those up as well but don't think it should be 
> part of this patch.
>
> 
> Joshua Colp wrote:
> This may be a time where it is near impossible to do this in a clean 
> fashion that is safe...

If that's the case then the following will have to do:
ast_sorcery_unref(sorcery)
{
  if (sorcery) {
/* One ref for what we just released, the other for the instances 
container. */
if (ao2_ref(sorcery, -1) == 2) {
  ao2_unlink(instances, sorcery);
}
  }
}


- rmudgett


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


On Feb. 6, 2014, 2:07 p.m., George Joseph wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3184/
> ---
> 
> (Updated Feb. 6, 2014, 2:07 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22537
> https://issues.asterisk.org/jira/browse/ASTERISK-22537
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Create sorcery instance registry as a precursor to creating a generic 
> dialplan function that can retrieve parameters from a sorcery-based config 
> file.
> 
> ast_sorcery_init now creates a hashtab as a registry.
> ast_sorcery_open now checks the hashtab for an existing sorcery instance 
> matching the caller's module name.  If it finds one, it bumps the refcount 
> and returns it.  If not, it creates a new sorcery instance, adds it to the 
> hashtab, then returns it.
> ast_sorcery_retrieve_by_module_name is a new function that does a hashtab 
> lookup by module name.  It can be called by the future dialplan function.
> 
> A side effect of this patch is that a module can only have 1 sorcery instance 
> (because it's the key for the hashtab).  res_pjsip/config_system needed a 
> small change to share the main res_pjsip sorcery instance.
> 
> 
> Diffs
> -
> 
>   branches/12/tests/test_sorcery.c 407566 
>   branches/12/res/res_pjsip/include/res_pjsip

[asterisk-dev] Asterisk 12.1.0-rc1 Now Available

[Asterisk Development Team]

The Asterisk Development Team has announced the first release candidate of
Asterisk 12.1.0. This release candidate is available for immediate
download at http://downloads.asterisk.org/pub/telephony/asterisk

The release of Asterisk 12.1.0-rc1 resolves several issues reported by the
community and would have not been possible without your participation.
Thank you!

The following is a sample of the issues resolved in this release candidate:

* --- pjsip: fix support for allow=all
  (Closes issue ASTERISK-23018. Reported by xrobau)

* --- res_pjsip_session: Be less strict with core requested outgoing
  capabilities.
  (Closes issue ASTERISK-23082. Reported by xrobau)

* --- res_stasis: Enable transfers and provide events when they occur.
  (Closes issue ASTERISK-22984. Reported by David M. Lee)

* --- ARI: Support channel variables in originate
  (Closes issue ASTERISK-23051. Reported by Matt Jordan)

* --- PJSIP: Fix address for ACK in NAT situations
  (Closes issue ASTERISK-23106. Reported by Matt Jordan)

For a full list of changes in this release candidate, please see the ChangeLog:

http://downloads.asterisk.org/pub/telephony/Asterisk/ChangeLog-12.1.0-rc1

Thank you for your continued support of Asterisk!

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3184: Create sorcery instance registry

[Joshua Colp]

> On Feb. 6, 2014, 8:21 p.m., rmudgett wrote:
> > branches/12/main/sorcery.c, lines 1630-1633
> > 
> >
> > There are two red flags here:
> > 1) You are using sorcery after you have released your ref to it.
> > 
> > 2) Any time you are looking at the number of references left, there is 
> > a design problem.  In this case, only the owner of the sorcery object 
> > should close/destroy it which will remove it from the instances container.  
> > Only the owner of the sorcery object should open/create it which will add 
> > it to the instances container.  Everyone else will search the instances 
> > container for the object to get a reference.
> 
> George Joseph wrote:
> There's no concept of weak references so when the sorcery instance is 
> linked to the registry container, the refcount gets bumped without the actual 
> consumer knowing.  When the last consumer releases the instance, the 
> instance's refcount is still 1 so the destructor will never run and and 
> remove the entry.  Instead of checking the refcount I could simply decrement 
> it automatically after inserting the instance to the container.  This way the 
> last consumer to release it will trigger the cleanup.
> 
> Now, if you're saying that only 1 consumer should ever call 
> ast_sorcery_open then I need to rethink.  The original idea was that multiple 
> consumers could simply call ast_sorcery_open and let it figure out if it 
> needed to create a new one or return the existing one.
> 
>
> 
> rmudgett wrote:
> You cannot decrement the ref that is held by the container without 
> causing problems for the container when it decrements the the ref on an 
> unlink.  You would get a negative ref count.
> 
> I was thinking only one creator/owner should ever call the open/destroy.
> 
> Bridges and channels have the same kind of problem since they are also 
> put into a global container.  They have trigger events to know when they need 
> to be destroyed so they can unlink from the global container.
> 
> George Joseph wrote:
> Yeah, -1 to the auto -1. :)
> 
> So I'd have to create a balancing ast_sorcery_close() that did a complete 
> clean of the sorcery instance.  Should it clean and destroy (possibly pulling 
> it out from under someone who did a retrieve on it but never -1'd it) or just 
> remove it from the registry and let the last -1 clean it up?
> 
> What should happen if the same module (hence the same key) tries to open 
> twice?
> 
> BTW, the sorcery instance opened by res_pjsip/config_system gets passed 
> all over pjsip-land, even to other modules, without anyone ever doing a +1 on 
> it.I'd want to go clean those up as well but don't think it should be 
> part of this patch.
>

This may be a time where it is near impossible to do this in a clean fashion 
that is safe...


- Joshua


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


On Feb. 6, 2014, 8:07 p.m., George Joseph wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3184/
> ---
> 
> (Updated Feb. 6, 2014, 8:07 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22537
> https://issues.asterisk.org/jira/browse/ASTERISK-22537
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Create sorcery instance registry as a precursor to creating a generic 
> dialplan function that can retrieve parameters from a sorcery-based config 
> file.
> 
> ast_sorcery_init now creates a hashtab as a registry.
> ast_sorcery_open now checks the hashtab for an existing sorcery instance 
> matching the caller's module name.  If it finds one, it bumps the refcount 
> and returns it.  If not, it creates a new sorcery instance, adds it to the 
> hashtab, then returns it.
> ast_sorcery_retrieve_by_module_name is a new function that does a hashtab 
> lookup by module name.  It can be called by the future dialplan function.
> 
> A side effect of this patch is that a module can only have 1 sorcery instance 
> (because it's the key for the hashtab).  res_pjsip/config_system needed a 
> small change to share the main res_pjsip sorcery instance.
> 
> 
> Diffs
> -
> 
>   branches/12/tests/test_sorcery.c 407566 
>   branches/12/res/res_pjsip/include/res_pjsip_private.h 407566 
>   branches/12/res/res_pjsip/config_system.c 407566 
>   branches/12/res/res_pjsip.c 407566 
>   branches/12/main/sorcery.c 407566 
>   branches/12/include/asterisk/sorcery.h 407566 
> 
> Diff: https://reviewboard.asterisk.org/r/3184/diff/
> 
> 
> Testing
> ---
> 
> Made sure that u

Re: [asterisk-dev] [Code Review] 3184: Create sorcery instance registry

[George Joseph]

> On Feb. 6, 2014, 1:21 p.m., rmudgett wrote:
> > branches/12/main/sorcery.c, lines 1630-1633
> > 
> >
> > There are two red flags here:
> > 1) You are using sorcery after you have released your ref to it.
> > 
> > 2) Any time you are looking at the number of references left, there is 
> > a design problem.  In this case, only the owner of the sorcery object 
> > should close/destroy it which will remove it from the instances container.  
> > Only the owner of the sorcery object should open/create it which will add 
> > it to the instances container.  Everyone else will search the instances 
> > container for the object to get a reference.
> 
> George Joseph wrote:
> There's no concept of weak references so when the sorcery instance is 
> linked to the registry container, the refcount gets bumped without the actual 
> consumer knowing.  When the last consumer releases the instance, the 
> instance's refcount is still 1 so the destructor will never run and and 
> remove the entry.  Instead of checking the refcount I could simply decrement 
> it automatically after inserting the instance to the container.  This way the 
> last consumer to release it will trigger the cleanup.
> 
> Now, if you're saying that only 1 consumer should ever call 
> ast_sorcery_open then I need to rethink.  The original idea was that multiple 
> consumers could simply call ast_sorcery_open and let it figure out if it 
> needed to create a new one or return the existing one.
> 
>
> 
> rmudgett wrote:
> You cannot decrement the ref that is held by the container without 
> causing problems for the container when it decrements the the ref on an 
> unlink.  You would get a negative ref count.
> 
> I was thinking only one creator/owner should ever call the open/destroy.
> 
> Bridges and channels have the same kind of problem since they are also 
> put into a global container.  They have trigger events to know when they need 
> to be destroyed so they can unlink from the global container.

Yeah, -1 to the auto -1. :)

So I'd have to create a balancing ast_sorcery_close() that did a complete clean 
of the sorcery instance.  Should it clean and destroy (possibly pulling it out 
from under someone who did a retrieve on it but never -1'd it) or just remove 
it from the registry and let the last -1 clean it up?

What should happen if the same module (hence the same key) tries to open twice?

BTW, the sorcery instance opened by res_pjsip/config_system gets passed all 
over pjsip-land, even to other modules, without anyone ever doing a +1 on it.   
 I'd want to go clean those up as well but don't think it should be part of 
this patch.


- George


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


On Feb. 6, 2014, 1:07 p.m., George Joseph wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3184/
> ---
> 
> (Updated Feb. 6, 2014, 1:07 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22537
> https://issues.asterisk.org/jira/browse/ASTERISK-22537
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Create sorcery instance registry as a precursor to creating a generic 
> dialplan function that can retrieve parameters from a sorcery-based config 
> file.
> 
> ast_sorcery_init now creates a hashtab as a registry.
> ast_sorcery_open now checks the hashtab for an existing sorcery instance 
> matching the caller's module name.  If it finds one, it bumps the refcount 
> and returns it.  If not, it creates a new sorcery instance, adds it to the 
> hashtab, then returns it.
> ast_sorcery_retrieve_by_module_name is a new function that does a hashtab 
> lookup by module name.  It can be called by the future dialplan function.
> 
> A side effect of this patch is that a module can only have 1 sorcery instance 
> (because it's the key for the hashtab).  res_pjsip/config_system needed a 
> small change to share the main res_pjsip sorcery instance.
> 
> 
> Diffs
> -
> 
>   branches/12/tests/test_sorcery.c 407566 
>   branches/12/res/res_pjsip/include/res_pjsip_private.h 407566 
>   branches/12/res/res_pjsip/config_system.c 407566 
>   branches/12/res/res_pjsip.c 407566 
>   branches/12/main/sorcery.c 407566 
>   branches/12/include/asterisk/sorcery.h 407566 
> 
> Diff: https://reviewboard.asterisk.org/r/3184/diff/
> 
> 
> Testing
> ---
> 
> Made sure that users of sorcery (mostly res_pjsip) continued to load their 
> configs correctly.
> Made sure there were no ill effects on res_pjsip from config_system sharing 
> the same sorcery 

Re: [asterisk-dev] [Code Review] 3183: ARI: pass channel variables into originate as opposed to assigning after originate

[Corey Farrell]

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

Ship it!


Ship It!

- Corey Farrell


On Feb. 6, 2014, 4:18 p.m., Matt Jordan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3183/
> ---
> 
> (Updated Feb. 6, 2014, 4:18 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> This patch tweaks the behaviour of POST /channels with channel variables such 
> that the variables are passed into the pbx.c routines that perform the 
> origination. This allows the variables to be assigned to the newly created 
> channels immediately upon their construction, as opposed to be assigned after 
> the originate has completed.
> 
> The upshot of this is that the variables are available on the channels if 
> they execute in the dialplan, as opposed to only being available once the 
> channels are answered.
> 
> 
> Diffs
> -
> 
>   /branches/12/res/ari/resource_channels.c 407562 
> 
> Diff: https://reviewboard.asterisk.org/r/3183/diff/
> 
> 
> Testing
> ---
> 
> Both testsuite originate tests still pass.
> 
> 
> Thanks,
> 
> Matt Jordan
> 
>

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3184: Create sorcery instance registry

[rmudgett]

> On Feb. 6, 2014, 2:21 p.m., rmudgett wrote:
> > branches/12/main/sorcery.c, lines 1630-1633
> > 
> >
> > There are two red flags here:
> > 1) You are using sorcery after you have released your ref to it.
> > 
> > 2) Any time you are looking at the number of references left, there is 
> > a design problem.  In this case, only the owner of the sorcery object 
> > should close/destroy it which will remove it from the instances container.  
> > Only the owner of the sorcery object should open/create it which will add 
> > it to the instances container.  Everyone else will search the instances 
> > container for the object to get a reference.
> 
> George Joseph wrote:
> There's no concept of weak references so when the sorcery instance is 
> linked to the registry container, the refcount gets bumped without the actual 
> consumer knowing.  When the last consumer releases the instance, the 
> instance's refcount is still 1 so the destructor will never run and and 
> remove the entry.  Instead of checking the refcount I could simply decrement 
> it automatically after inserting the instance to the container.  This way the 
> last consumer to release it will trigger the cleanup.
> 
> Now, if you're saying that only 1 consumer should ever call 
> ast_sorcery_open then I need to rethink.  The original idea was that multiple 
> consumers could simply call ast_sorcery_open and let it figure out if it 
> needed to create a new one or return the existing one.
> 
>

You cannot decrement the ref that is held by the container without causing 
problems for the container when it decrements the the ref on an unlink.  You 
would get a negative ref count.

I was thinking only one creator/owner should ever call the open/destroy.

Bridges and channels have the same kind of problem since they are also put into 
a global container.  They have trigger events to know when they need to be 
destroyed so they can unlink from the global container.


- rmudgett


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


On Feb. 6, 2014, 2:07 p.m., George Joseph wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3184/
> ---
> 
> (Updated Feb. 6, 2014, 2:07 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22537
> https://issues.asterisk.org/jira/browse/ASTERISK-22537
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Create sorcery instance registry as a precursor to creating a generic 
> dialplan function that can retrieve parameters from a sorcery-based config 
> file.
> 
> ast_sorcery_init now creates a hashtab as a registry.
> ast_sorcery_open now checks the hashtab for an existing sorcery instance 
> matching the caller's module name.  If it finds one, it bumps the refcount 
> and returns it.  If not, it creates a new sorcery instance, adds it to the 
> hashtab, then returns it.
> ast_sorcery_retrieve_by_module_name is a new function that does a hashtab 
> lookup by module name.  It can be called by the future dialplan function.
> 
> A side effect of this patch is that a module can only have 1 sorcery instance 
> (because it's the key for the hashtab).  res_pjsip/config_system needed a 
> small change to share the main res_pjsip sorcery instance.
> 
> 
> Diffs
> -
> 
>   branches/12/tests/test_sorcery.c 407566 
>   branches/12/res/res_pjsip/include/res_pjsip_private.h 407566 
>   branches/12/res/res_pjsip/config_system.c 407566 
>   branches/12/res/res_pjsip.c 407566 
>   branches/12/main/sorcery.c 407566 
>   branches/12/include/asterisk/sorcery.h 407566 
> 
> Diff: https://reviewboard.asterisk.org/r/3184/diff/
> 
> 
> Testing
> ---
> 
> Made sure that users of sorcery (mostly res_pjsip) continued to load their 
> configs correctly.
> Made sure there were no ill effects on res_pjsip from config_system sharing 
> the same sorcery instance as the rest of the pjsip infrastructure.
> Made sure that config_system was properly marked as 'not reloadable' and that 
> it was maintaining it's original values when res_pjsip was reloaded.
> 
> 
> ernie*CLI> test execute category /main/sorcery/
> Running all available tests matching category /main/sorcery/
> 
> START  /main/sorcery/ - configuration_file_wizard_retrieve_multiple_all 
> END/main/sorcery/ - configuration_file_wizard_retrieve_multiple_all Time: 
> <1ms Result: PASS
> [snip]
> START  /main/sorcery/ - open 
> END/main/sorcery/ - open Time: <1ms Result: PASS
> START  /main/sorcery/ - wizard_registration 
> END/main/sorcery/ - wizard_registration Time: <1ms Result: P

Re: [asterisk-dev] [Code Review] 3184: Create sorcery instance registry

[George Joseph]

> On Feb. 6, 2014, 1:21 p.m., rmudgett wrote:
> > branches/12/main/sorcery.c, lines 1630-1633
> > 
> >
> > There are two red flags here:
> > 1) You are using sorcery after you have released your ref to it.
> > 
> > 2) Any time you are looking at the number of references left, there is 
> > a design problem.  In this case, only the owner of the sorcery object 
> > should close/destroy it which will remove it from the instances container.  
> > Only the owner of the sorcery object should open/create it which will add 
> > it to the instances container.  Everyone else will search the instances 
> > container for the object to get a reference.

There's no concept of weak references so when the sorcery instance is linked to 
the registry container, the refcount gets bumped without the actual consumer 
knowing.  When the last consumer releases the instance, the instance's refcount 
is still 1 so the destructor will never run and and remove the entry.  Instead 
of checking the refcount I could simply decrement it automatically after 
inserting the instance to the container.  This way the last consumer to release 
it will trigger the cleanup.

Now, if you're saying that only 1 consumer should ever call ast_sorcery_open 
then I need to rethink.  The original idea was that multiple consumers could 
simply call ast_sorcery_open and let it figure out if it needed to create a new 
one or return the existing one.


- George


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


On Feb. 6, 2014, 1:07 p.m., George Joseph wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3184/
> ---
> 
> (Updated Feb. 6, 2014, 1:07 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22537
> https://issues.asterisk.org/jira/browse/ASTERISK-22537
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Create sorcery instance registry as a precursor to creating a generic 
> dialplan function that can retrieve parameters from a sorcery-based config 
> file.
> 
> ast_sorcery_init now creates a hashtab as a registry.
> ast_sorcery_open now checks the hashtab for an existing sorcery instance 
> matching the caller's module name.  If it finds one, it bumps the refcount 
> and returns it.  If not, it creates a new sorcery instance, adds it to the 
> hashtab, then returns it.
> ast_sorcery_retrieve_by_module_name is a new function that does a hashtab 
> lookup by module name.  It can be called by the future dialplan function.
> 
> A side effect of this patch is that a module can only have 1 sorcery instance 
> (because it's the key for the hashtab).  res_pjsip/config_system needed a 
> small change to share the main res_pjsip sorcery instance.
> 
> 
> Diffs
> -
> 
>   branches/12/tests/test_sorcery.c 407566 
>   branches/12/res/res_pjsip/include/res_pjsip_private.h 407566 
>   branches/12/res/res_pjsip/config_system.c 407566 
>   branches/12/res/res_pjsip.c 407566 
>   branches/12/main/sorcery.c 407566 
>   branches/12/include/asterisk/sorcery.h 407566 
> 
> Diff: https://reviewboard.asterisk.org/r/3184/diff/
> 
> 
> Testing
> ---
> 
> Made sure that users of sorcery (mostly res_pjsip) continued to load their 
> configs correctly.
> Made sure there were no ill effects on res_pjsip from config_system sharing 
> the same sorcery instance as the rest of the pjsip infrastructure.
> Made sure that config_system was properly marked as 'not reloadable' and that 
> it was maintaining it's original values when res_pjsip was reloaded.
> 
> 
> ernie*CLI> test execute category /main/sorcery/
> Running all available tests matching category /main/sorcery/
> 
> START  /main/sorcery/ - configuration_file_wizard_retrieve_multiple_all 
> END/main/sorcery/ - configuration_file_wizard_retrieve_multiple_all Time: 
> <1ms Result: PASS
> [snip]
> START  /main/sorcery/ - open 
> END/main/sorcery/ - open Time: <1ms Result: PASS
> START  /main/sorcery/ - wizard_registration 
> END/main/sorcery/ - wizard_registration Time: <1ms Result: PASS
> 
> 43 Test(s) Executed  43 Passed  0 Failed
> 
> 
> Thanks,
> 
> George Joseph
> 
>

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3174: chan_iax2: Block unnecessary control frames to/from the wire.

[Matt Jordan]

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

Ship it!


Ship It!

- Matt Jordan


On Feb. 5, 2014, 6:13 p.m., rmudgett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3174/
> ---
> 
> (Updated Feb. 5, 2014, 6:13 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: AST-1302
> https://issues.asterisk.org/jira/browse/AST-1302
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Establishing an IAX2 call between Asterisk v1.4 and v1.8 (or later) results 
> in an unexpected call disconnect.  The problem happens because newer values 
> in the enum ast_control_frame_type are not consistent between the branch 
> versions of Asterisk.
> 
> For example:
> 1) v1.4 calls v1.8 (or later) using IAX2
> 2) v1.8 answers and sends a connected line update control frame. (on v1.8 
> AST_CONTROL_CONNECTED_LINE = 22)
> 3) v1.4 receives the control frame as an end-of-q (on v1.4 
> AST_CONTROL_END_OF_Q = 22)
> 4) v1.4 disconnects the call once the receive queue becomes empty.
> 
> Several things are done by this patch to fix the problem and attempt to 
> prevent it from happening again in the future:
> * Added a warning at the definition of enum ast_control_frame_type about how 
> to add new control frame values.
> 
> * Made block sending and receiving control frames that have no reason to go 
> over the wire.
> 
> * Extended the connectedline iax.conf parameter to also include the 
> redirecting information updates.
> 
> * Updated the connectedline iax.conf parameter documentation to include a 
> notice that the parameter must be "no" when the peer is an Asterisk v1.4 
> instance.
> 
> 
> Diffs
> -
> 
>   /branches/1.8/include/asterisk/frame.h 407562 
>   /branches/1.8/configs/iax.conf.sample 407562 
>   /branches/1.8/channels/chan_iax2.c 407562 
> 
> Diff: https://reviewboard.asterisk.org/r/3174/diff/
> 
> 
> Testing
> ---
> 
> Without the patch the IAX2 call gets disconnected when the call initially 
> connects.
> With the patch the call stays up.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3183: ARI: pass channel variables into originate as opposed to assigning after originate

[Matt Jordan]

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

(Updated Feb. 6, 2014, 3:18 p.m.)


Review request for Asterisk Developers.


Changes
---

Promoted variable failure to an error


Repository: Asterisk


Description
---

This patch tweaks the behaviour of POST /channels with channel variables such 
that the variables are passed into the pbx.c routines that perform the 
origination. This allows the variables to be assigned to the newly created 
channels immediately upon their construction, as opposed to be assigned after 
the originate has completed.

The upshot of this is that the variables are available on the channels if they 
execute in the dialplan, as opposed to only being available once the channels 
are answered.


Diffs (updated)
-

  /branches/12/res/ari/resource_channels.c 407562 

Diff: https://reviewboard.asterisk.org/r/3183/diff/


Testing
---

Both testsuite originate tests still pass.


Thanks,

Matt Jordan

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3183: ARI: pass channel variables into originate as opposed to assigning after originate

[Matt Jordan]

> On Feb. 6, 2014, 3:02 p.m., Corey Farrell wrote:
> > /branches/12/res/ari/resource_channels.c, lines 772-774
> > 
> >
> > Why not ast_ari_response_alloc_failed?

I didn't go with that originally, as the caller doesn't really know why it 
failed, just that it did. As it turns out, the only way it *will* actually fail 
is due to an allocation error, so that's fine.


- Matt


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


On Feb. 6, 2014, 1:57 p.m., Matt Jordan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3183/
> ---
> 
> (Updated Feb. 6, 2014, 1:57 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> This patch tweaks the behaviour of POST /channels with channel variables such 
> that the variables are passed into the pbx.c routines that perform the 
> origination. This allows the variables to be assigned to the newly created 
> channels immediately upon their construction, as opposed to be assigned after 
> the originate has completed.
> 
> The upshot of this is that the variables are available on the channels if 
> they execute in the dialplan, as opposed to only being available once the 
> channels are answered.
> 
> 
> Diffs
> -
> 
>   /branches/12/res/ari/resource_channels.c 407562 
> 
> Diff: https://reviewboard.asterisk.org/r/3183/diff/
> 
> 
> Testing
> ---
> 
> Both testsuite originate tests still pass.
> 
> 
> Thanks,
> 
> Matt Jordan
> 
>

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3183: ARI: pass channel variables into originate as opposed to assigning after originate

[Corey Farrell]

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



/branches/12/res/ari/resource_channels.c


This should log an error not warning.  The message seems to indicate that 
there was something wrong with the input, while we actually had a memory 
allocation failure.  Might also be useful for this to mention that Originate is 
being aborted.



/branches/12/res/ari/resource_channels.c


Why not ast_ari_response_alloc_failed?


- Corey Farrell


On Feb. 6, 2014, 2:57 p.m., Matt Jordan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3183/
> ---
> 
> (Updated Feb. 6, 2014, 2:57 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> This patch tweaks the behaviour of POST /channels with channel variables such 
> that the variables are passed into the pbx.c routines that perform the 
> origination. This allows the variables to be assigned to the newly created 
> channels immediately upon their construction, as opposed to be assigned after 
> the originate has completed.
> 
> The upshot of this is that the variables are available on the channels if 
> they execute in the dialplan, as opposed to only being available once the 
> channels are answered.
> 
> 
> Diffs
> -
> 
>   /branches/12/res/ari/resource_channels.c 407562 
> 
> Diff: https://reviewboard.asterisk.org/r/3183/diff/
> 
> 
> Testing
> ---
> 
> Both testsuite originate tests still pass.
> 
> 
> Thanks,
> 
> Matt Jordan
> 
>

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3184: Create sorcery instance registry

[rmudgett]

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



branches/12/main/sorcery.c


The usual way in the code is to do:
strcpy(); /* Safe */


- rmudgett


On Feb. 6, 2014, 2:07 p.m., George Joseph wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3184/
> ---
> 
> (Updated Feb. 6, 2014, 2:07 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22537
> https://issues.asterisk.org/jira/browse/ASTERISK-22537
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Create sorcery instance registry as a precursor to creating a generic 
> dialplan function that can retrieve parameters from a sorcery-based config 
> file.
> 
> ast_sorcery_init now creates a hashtab as a registry.
> ast_sorcery_open now checks the hashtab for an existing sorcery instance 
> matching the caller's module name.  If it finds one, it bumps the refcount 
> and returns it.  If not, it creates a new sorcery instance, adds it to the 
> hashtab, then returns it.
> ast_sorcery_retrieve_by_module_name is a new function that does a hashtab 
> lookup by module name.  It can be called by the future dialplan function.
> 
> A side effect of this patch is that a module can only have 1 sorcery instance 
> (because it's the key for the hashtab).  res_pjsip/config_system needed a 
> small change to share the main res_pjsip sorcery instance.
> 
> 
> Diffs
> -
> 
>   branches/12/tests/test_sorcery.c 407566 
>   branches/12/res/res_pjsip/include/res_pjsip_private.h 407566 
>   branches/12/res/res_pjsip/config_system.c 407566 
>   branches/12/res/res_pjsip.c 407566 
>   branches/12/main/sorcery.c 407566 
>   branches/12/include/asterisk/sorcery.h 407566 
> 
> Diff: https://reviewboard.asterisk.org/r/3184/diff/
> 
> 
> Testing
> ---
> 
> Made sure that users of sorcery (mostly res_pjsip) continued to load their 
> configs correctly.
> Made sure there were no ill effects on res_pjsip from config_system sharing 
> the same sorcery instance as the rest of the pjsip infrastructure.
> Made sure that config_system was properly marked as 'not reloadable' and that 
> it was maintaining it's original values when res_pjsip was reloaded.
> 
> 
> ernie*CLI> test execute category /main/sorcery/
> Running all available tests matching category /main/sorcery/
> 
> START  /main/sorcery/ - configuration_file_wizard_retrieve_multiple_all 
> END/main/sorcery/ - configuration_file_wizard_retrieve_multiple_all Time: 
> <1ms Result: PASS
> [snip]
> START  /main/sorcery/ - open 
> END/main/sorcery/ - open Time: <1ms Result: PASS
> START  /main/sorcery/ - wizard_registration 
> END/main/sorcery/ - wizard_registration Time: <1ms Result: PASS
> 
> 43 Test(s) Executed  43 Passed  0 Failed
> 
> 
> Thanks,
> 
> George Joseph
> 
>

-- 
_
-- 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

Re: [asterisk-dev] res_fax_spandsp segfaults during fax detection - FIXED?

[Michal Rybárik]

  
  
Hi Jaco,

I reviewed spandsp sources at the places where your segfaults
happened, and this is very different situation than mine. But I see
that span_log function (spandsp logging) is called frequently from
this code, you should find some more information in spandsp log
probably, to discover what happened before your segfault.

Regards,
Michal Rybarik

On 02/06/2014 06:53 PM, Michal Rybarik wrote:

  
  Hi Jaco,
  
  if I understand correctly, your segfault did not happen during in
  T38gateway, but while receiving fax to tiff file (ReceiveFax), am
  I right?
  I haven't checked neither patched this (because my Asterisks are
  only relaying faxes, not terminating/originating to/from tiff
  file), but if your segfault happen when data are passed to
  libspandsp, it should be the same situation as mine was. Code in
  res_fax allows slinear/alaw/ulaw frames to be passed to
  res_fax_spandsp and then to libspandsp, but libspandsp accepts
  only slinear. When ulaw/alaw frames are passed here, bad things
  can happen.
  -- 
Regards,
Michal Rybarik
  
  
  Dňa 6. 2. 2014 12:07, Jaco Kroon  wrote / napísal(a):
  

Hi All,

Could this backtrace possibly be related?

#0  process_rx_data (t=0x7fae54c698a8, user_data=0x2,
data_type=1, field_type=,
buf=0x7fae11c58cda "cng", len=0) at t38_terminal.c:314
#1  0x7fae11c22c7d in t38_core_rx_ifp_packet
(s=0x7fae54c698a8, buf=0x7fae54c8475b "\002", len=1,
seq_no=) at t38_core.c:459
#2  0x7fae50ea96c5 in generic_fax_exec
(chan=chan@entry=0x7fadc4548c18,
details=details@entry=0x7fad50602c28,
reserved=reserved@entry=0x7fad50155478, token=) at res_fax.c:1498
#3  0x7fae50eaea9e in receivefax_exec
(chan=0x7fadc4548c18, data="" out>) at
res_fax.c:1932
#4  0x00530fdd in pbx_exec
(c=c@entry=0x7fadc4548c18, app=app@entry=0x2ddca60,
data=""
"/tmp/morpheus-1391681512.850.tiff") at pbx.c:1622
#5  0x0053656f in pbx_extension_helper
(c=c@entry=0x7fadc4548c18, context=,
exten=exten@entry=0x7fadc4549ab8 "0123489251",
priority=priority@entry=6, label=label@entry=0x0,
callerid=callerid@entry=0x7fadc44757b0 "0126413300",
action=""
found=found@entry=0x7fad838bad60, 
    combined_find_spawn=combined_find_spawn@entry=1,
con=0x0) at pbx.c:4922
#6  0x005404a4 in ast_spawn_extension
(found=0x7fad838bad60, callerid=0x7fadc44757b0 "0126413300",
priority=6, exten=0x7fadc4549ab8 "0123489251",
context=, c=0x7fadc4548c18,
combined_find_spawn=) at pbx.c:6038
#7  __ast_pbx_run (c=c@entry=0x7fadc4548c18,
args=args@entry=0x0) at pbx.c:6513
#8  0x00541c0b in pbx_thread
(data="" at pbx.c:6843
#9  0x00587c5a in dummy_start (data=""
out>) at utils.c:1162
#10 0x7fae530f2f3a in start_thread (arg=0x7fad838bb700)
at pthread_create.c:308
#11 0x7fae54754dad in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:113

Had about 11 of those this morning on asterisk 11.7.0. 
Codec's that's allowed on SIP though is g729 and gsm only,
so no ulaw/alaw allowed.  Actually, just double checked,
ulaw/alaw is (was now) allowed, so someone is possibly
trying to run in bypass mode, resulting in the t38 gateway
instead of t38 pass through.  I downgraded to 11.6.0 and
hadn't had a crash since but I opted to disable ulaw+alaw in
any case, just to be on the safer side.

  
  Kind Regards,
Jaco Kroon
 
  
  
  

  
  On 01/02/2014 06:49, Michal Rybárik wrote:

Hello


  Pavel, 
  
  On 01/31/2014 07:59 AM, Pavel Troller wrote: 
  
This code will translate non-slinear
  frames to slinear, just before they 
  are sent to libspandsp for v21detection. With this patch
  applied, v21 
  detection is done also for RTP (SIP) alaw/ulaw frames, so
  maybe SIP/G711 
  <->  SIP/T38 gateway will work too. I tested
  DAHDI<->  SIP/T38, gateway 
  works both ways, voice calls too. Is it better now? :o) 

I fully understand the code, but I'm not trained enough in
the Asterisk 
internals to respond to questio

Re: [asterisk-dev] [Code Review] 3184: Create sorcery instance registry

[rmudgett]

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



branches/12/include/asterisk/sorcery.h


Spacing
const char *module



branches/12/main/sorcery.c


These should be set NULL after cleanup so the pointer is not dangling on 
destroyed data.



branches/12/main/sorcery.c


instance = alloc();
if (!instance) {
}

The above is a better format because there are more and better line break 
opportunities available with the stand alone assignment statement than when 
combining the assignment into the if test.

Also you don't need the error message here because the alloc has already 
given a memory allocation failure error message.



branches/12/main/sorcery.c


It is best to have a blank line between declarations and code.



branches/12/main/sorcery.c


spacing
const char *module_name



branches/12/main/sorcery.c


Use ao2_wrlock() instead since the instances container has a rwlock.  
ao2_lock() is a synonym for ao2_wrlock() when an object has a rwlock.



branches/12/main/sorcery.c


Use OBJ_SEARCH_KEY not OBJ_KEY.



branches/12/main/sorcery.c


OBJ_SEARCH_KEY



branches/12/main/sorcery.c


There are two red flags here:
1) You are using sorcery after you have released your ref to it.

2) Any time you are looking at the number of references left, there is a 
design problem.  In this case, only the owner of the sorcery object should 
close/destroy it which will remove it from the instances container.  Only the 
owner of the sorcery object should open/create it which will add it to the 
instances container.  Everyone else will search the instances container for the 
object to get a reference.



branches/12/main/sorcery.c


This ao2_find is really an ao2_unlink().


- rmudgett


On Feb. 6, 2014, 2:07 p.m., George Joseph wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3184/
> ---
> 
> (Updated Feb. 6, 2014, 2:07 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22537
> https://issues.asterisk.org/jira/browse/ASTERISK-22537
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Create sorcery instance registry as a precursor to creating a generic 
> dialplan function that can retrieve parameters from a sorcery-based config 
> file.
> 
> ast_sorcery_init now creates a hashtab as a registry.
> ast_sorcery_open now checks the hashtab for an existing sorcery instance 
> matching the caller's module name.  If it finds one, it bumps the refcount 
> and returns it.  If not, it creates a new sorcery instance, adds it to the 
> hashtab, then returns it.
> ast_sorcery_retrieve_by_module_name is a new function that does a hashtab 
> lookup by module name.  It can be called by the future dialplan function.
> 
> A side effect of this patch is that a module can only have 1 sorcery instance 
> (because it's the key for the hashtab).  res_pjsip/config_system needed a 
> small change to share the main res_pjsip sorcery instance.
> 
> 
> Diffs
> -
> 
>   branches/12/tests/test_sorcery.c 407566 
>   branches/12/res/res_pjsip/include/res_pjsip_private.h 407566 
>   branches/12/res/res_pjsip/config_system.c 407566 
>   branches/12/res/res_pjsip.c 407566 
>   branches/12/main/sorcery.c 407566 
>   branches/12/include/asterisk/sorcery.h 407566 
> 
> Diff: https://reviewboard.asterisk.org/r/3184/diff/
> 
> 
> Testing
> ---
> 
> Made sure that users of sorcery (mostly res_pjsip) continued to load their 
> configs correctly.
> Made sure there were no ill effects on res_pjsip from config_system sharing 
> the same sorcery instance as the rest of the pjsip infrastructure.
> Made sure that config_system was properly marked as 'not reloadable' and that 
> it was maintaining it's original values when res_pjsip was reloaded.
> 
> 
> ernie*CLI> test execute category /main/sorcery/
> Running all available tests matching category /main/sorcery/
> 
> START  /main/sorcery/ - configuration_file_wizard_retrieve_multiple_all 
> END/main/sorcery/ - configuration_file_wizard_retrieve_multiple_all Time: 
> <1ms Result: PASS

Re: [asterisk-dev] [Code Review] 3136: cli: pjsip show endpoint shows allow/disallow codecs the same - OPTION 1

[Matt Jordan]

> On Feb. 6, 2014, 9:52 a.m., Joshua Colp wrote:
> > While this works I'm not happy with pushing this down to such a low level. 
> > What if in the future I want to filter something else?
> > 
> > What I'd really like to see is something on top which allows you to 
> > arbitrarily filter anything. If something similar came up in the future 
> > then we'd have an immediate easy solution with no core changes and it would 
> > also allow the information to still exist for cases where you do want to 
> > get it.
> 
> Scott Griepentrog wrote:
> The other option would be to add another parameter to the 
> ast_sorcery_object_field_register() which would indicate a "hidden" option.  
> I can write that up as a separate review and see which one is preferred.  
> Which is now up for review: https://reviewboard.asterisk.org/r/3193/
>
> 
> Joshua Colp wrote:
> I disagree that that is the only other option. You don't have to consume 
> the information provided from sorcery as-is, and you don't have to push 
> filtering down to that level. Something that sits between sorcery and the 
> consumer can easily do the filtering based on information the consumer 
> provides.
> 
> Joshua Colp wrote:
> To go a step further: Whether an object field should be hidden is not a 
> property of the object field itself, it is a constraint within the consumer.

I'm in 100% agreement with Josh here. I don't think this or option 2 are the 
right approach.

A framework should provide information to consumers. It is up to the consumer 
to decide how that information is displayed.

What do we mean by consumer?

Today, that is the CLI and AMI. But it doesn't have to be.

I could just as easily have a module that extracts information about SIP 
endpoints via a sorcery object and transmits that data over a JSON websocket 
(why, I don't know, but think of the integration!) Since it is not displaying 
information to an end user, it might want to see the disallow field. It might 
not: but the answer is, I don't know what it wants to send. If this becomes 
part of the framework, then it precludes this module from ever existing. You 
can't go back and simply 'undo' not showing the option - you've now broken the 
CLI/AMI. Your only option is to undo it *and* modify the CLI/AMI, and now that 
module has become intrusive.

I'd rather keep this sorcery agnostic of how things use it. Whenever possible, 
let the consumers decide what and how to show data. A filter is not a bad 
thing; it is appropriate for a consumer to filter out what it doesn't want to 
show.


- Matt


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


On Feb. 6, 2014, 11:40 a.m., Scott Griepentrog wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3136/
> ---
> 
> (Updated Feb. 6, 2014, 11:40 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23092
> https://issues.asterisk.org/jira/browse/ASTERISK-23092
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> WAS:
> 
> Insert a ! prefix in the display of endpoint disallow value.  Result is:
> 
>  disallow  : !(ulaw|alaw)
> 
> NOW:
> 
> Remove the disallow option from generated lists, while still accepting it 
> from a configuration file.
> 
> 
> Diffs
> -
> 
>   /branches/12/res/res_pjsip/pjsip_configuration.c 407196 
>   /branches/12/main/sorcery.c 407196 
>   /branches/12/main/config_options.c 407196 
>   /branches/12/include/asterisk/config_options.h 407196 
> 
> 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

Re: [asterisk-dev] [Code Review] 3193: cli: pjsip show endpoint shows allow/disallow codecs the same - OPTION 2

[Matt Jordan]

> On Feb. 6, 2014, 1:50 p.m., Joshua Colp wrote:
> > I like the idea of being able to mark stuff as deprecated and get a 
> > warning, but I still dislike the idea of hidden fields. ^_^

Two things here:

(1) There is already support for deprecated fields and information hiding in 
config_options. As sorcery is, for the most part, built on that framework, I 
would have expected to see those APIs being used, and not a re-creation of 
them. I suspect that there is a large amount of the implementation in here that 
could be simplified as a result.

(2) For the problem being solved here, this feels like overkill. If a consumer 
wants to hide information from the user, then the consumer should choose to 
hide that information. A framework should only hide that information when it is 
absolutely sure that no consumer, ever, in any module ever created (or that 
ever will be created), should have access to that information. I don't think 
that is the case here with 'disallow'.


- Matt


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


On Feb. 6, 2014, 12:50 p.m., Scott Griepentrog wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3193/
> ---
> 
> (Updated Feb. 6, 2014, 12:50 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23092
> https://issues.asterisk.org/jira/browse/ASTERISK-23092
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> WAS:
> 
> Insert a ! prefix in the display of endpoint disallow value.  Result is:
> 
>  disallow  : !(ulaw|alaw)
> 
> NOW:
> 
> Remove the disallow option from generated lists, while still accepting it 
> from a configuration file.
> 
> This is OPTION 2 - option 1 is https://reviewboard.asterisk.org/r/3136/
> 
> 
> Diffs
> -
> 
>   /branches/12/res/res_pjsip_outbound_registration.c 407566 
>   /branches/12/res/res_pjsip_endpoint_identifier_ip.c 407566 
>   /branches/12/res/res_pjsip_acl.c 407566 
>   /branches/12/res/res_pjsip/pjsip_options.c 407566 
>   /branches/12/res/res_pjsip/pjsip_configuration.c 407566 
>   /branches/12/res/res_pjsip/location.c 407566 
>   /branches/12/res/res_pjsip/config_transport.c 407566 
>   /branches/12/res/res_pjsip/config_system.c 407566 
>   /branches/12/res/res_pjsip/config_global.c 407566 
>   /branches/12/res/res_pjsip/config_domain_aliases.c 407566 
>   /branches/12/res/res_pjsip/config_auth.c 407566 
>   /branches/12/main/sorcery.c 407566 
>   /branches/12/main/bucket.c 407566 
>   /branches/12/include/asterisk/sorcery.h 407566 
> 
> Diff: https://reviewboard.asterisk.org/r/3193/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

Re: [asterisk-dev] [Code Review] 3184: Create sorcery instance registry

[Joshua Colp]

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

Ship it!


Ship It!

- Joshua Colp


On Feb. 6, 2014, 8:07 p.m., George Joseph wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3184/
> ---
> 
> (Updated Feb. 6, 2014, 8:07 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22537
> https://issues.asterisk.org/jira/browse/ASTERISK-22537
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Create sorcery instance registry as a precursor to creating a generic 
> dialplan function that can retrieve parameters from a sorcery-based config 
> file.
> 
> ast_sorcery_init now creates a hashtab as a registry.
> ast_sorcery_open now checks the hashtab for an existing sorcery instance 
> matching the caller's module name.  If it finds one, it bumps the refcount 
> and returns it.  If not, it creates a new sorcery instance, adds it to the 
> hashtab, then returns it.
> ast_sorcery_retrieve_by_module_name is a new function that does a hashtab 
> lookup by module name.  It can be called by the future dialplan function.
> 
> A side effect of this patch is that a module can only have 1 sorcery instance 
> (because it's the key for the hashtab).  res_pjsip/config_system needed a 
> small change to share the main res_pjsip sorcery instance.
> 
> 
> Diffs
> -
> 
>   branches/12/tests/test_sorcery.c 407566 
>   branches/12/res/res_pjsip/include/res_pjsip_private.h 407566 
>   branches/12/res/res_pjsip/config_system.c 407566 
>   branches/12/res/res_pjsip.c 407566 
>   branches/12/main/sorcery.c 407566 
>   branches/12/include/asterisk/sorcery.h 407566 
> 
> Diff: https://reviewboard.asterisk.org/r/3184/diff/
> 
> 
> Testing
> ---
> 
> Made sure that users of sorcery (mostly res_pjsip) continued to load their 
> configs correctly.
> Made sure there were no ill effects on res_pjsip from config_system sharing 
> the same sorcery instance as the rest of the pjsip infrastructure.
> Made sure that config_system was properly marked as 'not reloadable' and that 
> it was maintaining it's original values when res_pjsip was reloaded.
> 
> 
> ernie*CLI> test execute category /main/sorcery/
> Running all available tests matching category /main/sorcery/
> 
> START  /main/sorcery/ - configuration_file_wizard_retrieve_multiple_all 
> END/main/sorcery/ - configuration_file_wizard_retrieve_multiple_all Time: 
> <1ms Result: PASS
> [snip]
> START  /main/sorcery/ - open 
> END/main/sorcery/ - open Time: <1ms Result: PASS
> START  /main/sorcery/ - wizard_registration 
> END/main/sorcery/ - wizard_registration Time: <1ms Result: PASS
> 
> 43 Test(s) Executed  43 Passed  0 Failed
> 
> 
> Thanks,
> 
> George Joseph
> 
>

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3186: AMI Security Events: document the events; add optional IEs to the events

[Matt Jordan]

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

(Updated Feb. 6, 2014, 2:07 p.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Repository: Asterisk


Description
---

This patch:
(1) Adds optional IEs to the AMI events. The optional fields are documented as 
not being required.
(2) Adds documentation for all of the Security Events.


Diffs
-

  /branches/12/main/security_events.c 407401 
  /branches/12/UPGRADE.txt 407402 
  /branches/12/CHANGES 407402 

Diff: https://reviewboard.asterisk.org/r/3186/diff/


Testing
---

Tests that use the AMI events in the testsuite continue to pass.

Tested documentation through the CLI.


Thanks,

Matt Jordan

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3184: Create sorcery instance registry

[George Joseph]

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

(Updated Feb. 6, 2014, 1:07 p.m.)


Review request for Asterisk Developers.


Changes
---

NULLed the sorcery pointers before test exit.


Bugs: ASTERISK-22537
https://issues.asterisk.org/jira/browse/ASTERISK-22537


Repository: Asterisk


Description
---

Create sorcery instance registry as a precursor to creating a generic dialplan 
function that can retrieve parameters from a sorcery-based config file.

ast_sorcery_init now creates a hashtab as a registry.
ast_sorcery_open now checks the hashtab for an existing sorcery instance 
matching the caller's module name.  If it finds one, it bumps the refcount and 
returns it.  If not, it creates a new sorcery instance, adds it to the hashtab, 
then returns it.
ast_sorcery_retrieve_by_module_name is a new function that does a hashtab 
lookup by module name.  It can be called by the future dialplan function.

A side effect of this patch is that a module can only have 1 sorcery instance 
(because it's the key for the hashtab).  res_pjsip/config_system needed a small 
change to share the main res_pjsip sorcery instance.


Diffs (updated)
-

  branches/12/tests/test_sorcery.c 407566 
  branches/12/res/res_pjsip/include/res_pjsip_private.h 407566 
  branches/12/res/res_pjsip/config_system.c 407566 
  branches/12/res/res_pjsip.c 407566 
  branches/12/main/sorcery.c 407566 
  branches/12/include/asterisk/sorcery.h 407566 

Diff: https://reviewboard.asterisk.org/r/3184/diff/


Testing
---

Made sure that users of sorcery (mostly res_pjsip) continued to load their 
configs correctly.
Made sure there were no ill effects on res_pjsip from config_system sharing the 
same sorcery instance as the rest of the pjsip infrastructure.
Made sure that config_system was properly marked as 'not reloadable' and that 
it was maintaining it's original values when res_pjsip was reloaded.


ernie*CLI> test execute category /main/sorcery/
Running all available tests matching category /main/sorcery/

START  /main/sorcery/ - configuration_file_wizard_retrieve_multiple_all 
END/main/sorcery/ - configuration_file_wizard_retrieve_multiple_all Time: 
<1ms Result: PASS
[snip]
START  /main/sorcery/ - open 
END/main/sorcery/ - open Time: <1ms Result: PASS
START  /main/sorcery/ - wizard_registration 
END/main/sorcery/ - wizard_registration Time: <1ms Result: PASS

43 Test(s) Executed  43 Passed  0 Failed


Thanks,

George Joseph

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3184: Create sorcery instance registry

[George Joseph]

> On Feb. 6, 2014, 1:01 p.m., Joshua Colp wrote:
> > branches/12/tests/test_sorcery.c, line 362
> > 
> >
> > Er, do you mean you are unreffing an object that is already destroyed? 
> > If so setting the variables to NULL will prevent that. Once this 
> > clarification is provided this'll be good!

Duh.  Low blood sugar.


- George


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


On Feb. 6, 2014, 12:55 p.m., George Joseph wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3184/
> ---
> 
> (Updated Feb. 6, 2014, 12:55 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22537
> https://issues.asterisk.org/jira/browse/ASTERISK-22537
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Create sorcery instance registry as a precursor to creating a generic 
> dialplan function that can retrieve parameters from a sorcery-based config 
> file.
> 
> ast_sorcery_init now creates a hashtab as a registry.
> ast_sorcery_open now checks the hashtab for an existing sorcery instance 
> matching the caller's module name.  If it finds one, it bumps the refcount 
> and returns it.  If not, it creates a new sorcery instance, adds it to the 
> hashtab, then returns it.
> ast_sorcery_retrieve_by_module_name is a new function that does a hashtab 
> lookup by module name.  It can be called by the future dialplan function.
> 
> A side effect of this patch is that a module can only have 1 sorcery instance 
> (because it's the key for the hashtab).  res_pjsip/config_system needed a 
> small change to share the main res_pjsip sorcery instance.
> 
> 
> Diffs
> -
> 
>   branches/12/tests/test_sorcery.c 407566 
>   branches/12/res/res_pjsip/include/res_pjsip_private.h 407566 
>   branches/12/res/res_pjsip/config_system.c 407566 
>   branches/12/res/res_pjsip.c 407566 
>   branches/12/main/sorcery.c 407566 
>   branches/12/include/asterisk/sorcery.h 407566 
> 
> Diff: https://reviewboard.asterisk.org/r/3184/diff/
> 
> 
> Testing
> ---
> 
> Made sure that users of sorcery (mostly res_pjsip) continued to load their 
> configs correctly.
> Made sure there were no ill effects on res_pjsip from config_system sharing 
> the same sorcery instance as the rest of the pjsip infrastructure.
> Made sure that config_system was properly marked as 'not reloadable' and that 
> it was maintaining it's original values when res_pjsip was reloaded.
> 
> 
> ernie*CLI> test execute category /main/sorcery/
> Running all available tests matching category /main/sorcery/
> 
> START  /main/sorcery/ - configuration_file_wizard_retrieve_multiple_all 
> END/main/sorcery/ - configuration_file_wizard_retrieve_multiple_all Time: 
> <1ms Result: PASS
> [snip]
> START  /main/sorcery/ - open 
> END/main/sorcery/ - open Time: <1ms Result: PASS
> START  /main/sorcery/ - wizard_registration 
> END/main/sorcery/ - wizard_registration Time: <1ms Result: PASS
> 
> 43 Test(s) Executed  43 Passed  0 Failed
> 
> 
> Thanks,
> 
> George Joseph
> 
>

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3184: Create sorcery instance registry

[Joshua Colp]

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



branches/12/tests/test_sorcery.c


Er, do you mean you are unreffing an object that is already destroyed? If 
so setting the variables to NULL will prevent that. Once this clarification is 
provided this'll be good!


- Joshua Colp


On Feb. 6, 2014, 7:55 p.m., George Joseph wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3184/
> ---
> 
> (Updated Feb. 6, 2014, 7:55 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22537
> https://issues.asterisk.org/jira/browse/ASTERISK-22537
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Create sorcery instance registry as a precursor to creating a generic 
> dialplan function that can retrieve parameters from a sorcery-based config 
> file.
> 
> ast_sorcery_init now creates a hashtab as a registry.
> ast_sorcery_open now checks the hashtab for an existing sorcery instance 
> matching the caller's module name.  If it finds one, it bumps the refcount 
> and returns it.  If not, it creates a new sorcery instance, adds it to the 
> hashtab, then returns it.
> ast_sorcery_retrieve_by_module_name is a new function that does a hashtab 
> lookup by module name.  It can be called by the future dialplan function.
> 
> A side effect of this patch is that a module can only have 1 sorcery instance 
> (because it's the key for the hashtab).  res_pjsip/config_system needed a 
> small change to share the main res_pjsip sorcery instance.
> 
> 
> Diffs
> -
> 
>   branches/12/tests/test_sorcery.c 407566 
>   branches/12/res/res_pjsip/include/res_pjsip_private.h 407566 
>   branches/12/res/res_pjsip/config_system.c 407566 
>   branches/12/res/res_pjsip.c 407566 
>   branches/12/main/sorcery.c 407566 
>   branches/12/include/asterisk/sorcery.h 407566 
> 
> Diff: https://reviewboard.asterisk.org/r/3184/diff/
> 
> 
> Testing
> ---
> 
> Made sure that users of sorcery (mostly res_pjsip) continued to load their 
> configs correctly.
> Made sure there were no ill effects on res_pjsip from config_system sharing 
> the same sorcery instance as the rest of the pjsip infrastructure.
> Made sure that config_system was properly marked as 'not reloadable' and that 
> it was maintaining it's original values when res_pjsip was reloaded.
> 
> 
> ernie*CLI> test execute category /main/sorcery/
> Running all available tests matching category /main/sorcery/
> 
> START  /main/sorcery/ - configuration_file_wizard_retrieve_multiple_all 
> END/main/sorcery/ - configuration_file_wizard_retrieve_multiple_all Time: 
> <1ms Result: PASS
> [snip]
> START  /main/sorcery/ - open 
> END/main/sorcery/ - open Time: <1ms Result: PASS
> START  /main/sorcery/ - wizard_registration 
> END/main/sorcery/ - wizard_registration Time: <1ms Result: PASS
> 
> 43 Test(s) Executed  43 Passed  0 Failed
> 
> 
> Thanks,
> 
> George Joseph
> 
>

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3180: Documenation: Configuration section naming in pjsip.conf.sample needs a little clarification

[rnewton]

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

(Updated Feb. 6, 2014, 8 p.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Repository: Asterisk


Description
---

There is some nuance to naming configuration sections. In the long term I'll 
hope that configuration section names become a bit more arbitrary; in the short 
term help me make sure this documentation patch clarifies things.


Diffs
-

  /branches/12/configs/pjsip.conf.sample 407338 

Diff: https://reviewboard.asterisk.org/r/3180/diff/


Testing
---

Only adding informational text to the pjsip.conf.sample file.


Thanks,

rnewton

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3183: ARI: pass channel variables into originate as opposed to assigning after originate

[Matt Jordan]

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

(Updated Feb. 6, 2014, 1:57 p.m.)


Review request for Asterisk Developers.


Changes
---

Addressed Corey's findings.


Repository: Asterisk


Description
---

This patch tweaks the behaviour of POST /channels with channel variables such 
that the variables are passed into the pbx.c routines that perform the 
origination. This allows the variables to be assigned to the newly created 
channels immediately upon their construction, as opposed to be assigned after 
the originate has completed.

The upshot of this is that the variables are available on the channels if they 
execute in the dialplan, as opposed to only being available once the channels 
are answered.


Diffs (updated)
-

  /branches/12/res/ari/resource_channels.c 407562 

Diff: https://reviewboard.asterisk.org/r/3183/diff/


Testing
---

Both testsuite originate tests still pass.


Thanks,

Matt Jordan

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3184: Create sorcery instance registry

[George Joseph]

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

(Updated Feb. 6, 2014, 12:55 p.m.)


Review request for Asterisk Developers.


Changes
---

Added suggested doc and added unit tests to test_sorcery/open


Bugs: ASTERISK-22537
https://issues.asterisk.org/jira/browse/ASTERISK-22537


Repository: Asterisk


Description
---

Create sorcery instance registry as a precursor to creating a generic dialplan 
function that can retrieve parameters from a sorcery-based config file.

ast_sorcery_init now creates a hashtab as a registry.
ast_sorcery_open now checks the hashtab for an existing sorcery instance 
matching the caller's module name.  If it finds one, it bumps the refcount and 
returns it.  If not, it creates a new sorcery instance, adds it to the hashtab, 
then returns it.
ast_sorcery_retrieve_by_module_name is a new function that does a hashtab 
lookup by module name.  It can be called by the future dialplan function.

A side effect of this patch is that a module can only have 1 sorcery instance 
(because it's the key for the hashtab).  res_pjsip/config_system needed a small 
change to share the main res_pjsip sorcery instance.


Diffs (updated)
-

  branches/12/tests/test_sorcery.c 407566 
  branches/12/res/res_pjsip/include/res_pjsip_private.h 407566 
  branches/12/res/res_pjsip/config_system.c 407566 
  branches/12/res/res_pjsip.c 407566 
  branches/12/main/sorcery.c 407566 
  branches/12/include/asterisk/sorcery.h 407566 

Diff: https://reviewboard.asterisk.org/r/3184/diff/


Testing (updated)
---

Made sure that users of sorcery (mostly res_pjsip) continued to load their 
configs correctly.
Made sure there were no ill effects on res_pjsip from config_system sharing the 
same sorcery instance as the rest of the pjsip infrastructure.
Made sure that config_system was properly marked as 'not reloadable' and that 
it was maintaining it's original values when res_pjsip was reloaded.


ernie*CLI> test execute category /main/sorcery/
Running all available tests matching category /main/sorcery/

START  /main/sorcery/ - configuration_file_wizard_retrieve_multiple_all 
END/main/sorcery/ - configuration_file_wizard_retrieve_multiple_all Time: 
<1ms Result: PASS
[snip]
START  /main/sorcery/ - open 
END/main/sorcery/ - open Time: <1ms Result: PASS
START  /main/sorcery/ - wizard_registration 
END/main/sorcery/ - wizard_registration Time: <1ms Result: PASS

43 Test(s) Executed  43 Passed  0 Failed


Thanks,

George Joseph

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3193: cli: pjsip show endpoint shows allow/disallow codecs the same - OPTION 2

[Joshua Colp]

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


I like the idea of being able to mark stuff as deprecated and get a warning, 
but I still dislike the idea of hidden fields. ^_^

- Joshua Colp


On Feb. 6, 2014, 6:50 p.m., Scott Griepentrog wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3193/
> ---
> 
> (Updated Feb. 6, 2014, 6:50 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23092
> https://issues.asterisk.org/jira/browse/ASTERISK-23092
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> WAS:
> 
> Insert a ! prefix in the display of endpoint disallow value.  Result is:
> 
>  disallow  : !(ulaw|alaw)
> 
> NOW:
> 
> Remove the disallow option from generated lists, while still accepting it 
> from a configuration file.
> 
> This is OPTION 2 - option 1 is https://reviewboard.asterisk.org/r/3136/
> 
> 
> Diffs
> -
> 
>   /branches/12/res/res_pjsip_outbound_registration.c 407566 
>   /branches/12/res/res_pjsip_endpoint_identifier_ip.c 407566 
>   /branches/12/res/res_pjsip_acl.c 407566 
>   /branches/12/res/res_pjsip/pjsip_options.c 407566 
>   /branches/12/res/res_pjsip/pjsip_configuration.c 407566 
>   /branches/12/res/res_pjsip/location.c 407566 
>   /branches/12/res/res_pjsip/config_transport.c 407566 
>   /branches/12/res/res_pjsip/config_system.c 407566 
>   /branches/12/res/res_pjsip/config_global.c 407566 
>   /branches/12/res/res_pjsip/config_domain_aliases.c 407566 
>   /branches/12/res/res_pjsip/config_auth.c 407566 
>   /branches/12/main/sorcery.c 407566 
>   /branches/12/main/bucket.c 407566 
>   /branches/12/include/asterisk/sorcery.h 407566 
> 
> Diff: https://reviewboard.asterisk.org/r/3193/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

Re: [asterisk-dev] [Code Review] 3183: ARI: pass channel variables into originate as opposed to assigning after originate

[Matt Jordan]

> On Feb. 6, 2014, 9:42 a.m., Corey Farrell wrote:
> > /branches/12/res/ari/resource_channels.c, line 761
> > 
> >
> > If we actually failed to create variables that were requested, we 
> > should abort the Originate.  Half-success is worse than total failure.

Agreed


> On Feb. 6, 2014, 9:42 a.m., Corey Farrell wrote:
> > /branches/12/res/ari/resource_channels.c, line 690
> > 
> >
> > I feel the following prototype would be better for the caller:
> > static int json_to_ast_variables(struct ast_json *src, struct 
> > ast_variable **dest);
> > Return 0 or -1 for success/failure.
> > 
> > This way the caller can tell between a failure and an empty 
> > json_variables object.

I'm usually less of a fan of 'out' parameters than just returning what has been 
created, I can see how this is useful in this case - it would at least prevent 
the case of { variables: { } } from causing an error response. Changed.


- Matt


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


On Feb. 6, 2014, 9:05 a.m., Matt Jordan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3183/
> ---
> 
> (Updated Feb. 6, 2014, 9:05 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> This patch tweaks the behaviour of POST /channels with channel variables such 
> that the variables are passed into the pbx.c routines that perform the 
> origination. This allows the variables to be assigned to the newly created 
> channels immediately upon their construction, as opposed to be assigned after 
> the originate has completed.
> 
> The upshot of this is that the variables are available on the channels if 
> they execute in the dialplan, as opposed to only being available once the 
> channels are answered.
> 
> 
> Diffs
> -
> 
>   /branches/12/res/ari/resource_channels.c 407562 
> 
> Diff: https://reviewboard.asterisk.org/r/3183/diff/
> 
> 
> Testing
> ---
> 
> Both testsuite originate tests still pass.
> 
> 
> Thanks,
> 
> Matt Jordan
> 
>

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3180: Documenation: Configuration section naming in pjsip.conf.sample needs a little clarification

[Scott Griepentrog]

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

Ship it!


Ship It!


/branches/12/configs/pjsip.conf.sample


s/endpoints/endpoints or aor/


- Scott Griepentrog


On Feb. 5, 2014, 3:10 p.m., rnewton wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3180/
> ---
> 
> (Updated Feb. 5, 2014, 3:10 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> There is some nuance to naming configuration sections. In the long term I'll 
> hope that configuration section names become a bit more arbitrary; in the 
> short term help me make sure this documentation patch clarifies things.
> 
> 
> Diffs
> -
> 
>   /branches/12/configs/pjsip.conf.sample 407338 
> 
> Diff: https://reviewboard.asterisk.org/r/3180/diff/
> 
> 
> Testing
> ---
> 
> Only adding informational text to the pjsip.conf.sample file.
> 
> 
> Thanks,
> 
> rnewton
> 
>

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3158: indications.conf: fix post-stutter dialtone for in, mx and ph, extra missing stutter

[rmudgett]

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

Ship it!


I know that Asterisk will always play the stutter tone on analog ports if there 
are voicemail messages.  I don't know what the code will do if the stutter tone 
is not defined.  Play nothing?

This should go into v1.8, v11, v12, and trunk.

- rmudgett


On Feb. 6, 2014, 11:51 a.m., Tzafrir Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3158/
> ---
> 
> (Updated Feb. 6, 2014, 11:51 a.m.)
> 
> 
> Review request for Asterisk Developers and rmudgett.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> The stutter tone for many (most?) countries in indications.conf is not takes 
> from national/international standards. If there's no alternative definition 
> for a voice mail notification tone, it needs to give a short stutter and then 
> continue with the dial tone.
> 
> In a number of countries it proceeds with a different dial tone. This fixes 
> the definitions of India (in), Mexico (mx) and the Philippines (ph) to 
> proceed with their dial tone, rather than with the us dial tone.
> 
> In addition to that, stutter tones were added to the following zones: Spain 
> (es), Malaysia (my) and Venezuela (ve): I assume that a voicemail 
> notification tone would be useful anywhere. Is that a sane assumption?
> 
> 
> Diffs
> -
> 
>   /trunk/configs/indications.conf.sample 407571 
> 
> Diff: https://reviewboard.asterisk.org/r/3158/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tzafrir Cohen
> 
>

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3192: chan_dahdi: handle DAHDI_EVENT_REMOVED on a pri D-Channel

[rmudgett]

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

Ship it!


Use svn merge to backport the revisions -r394552 and -r394567 for the patch so 
comparing the files between version does not show up unnecessary differences.  
Also you should use the commit message from -r394552 adding a note indicating 
that the change was backported.

You will need to manually close this review when you commit because autoclosing 
of reviews is currently broken.

- rmudgett


On Feb. 6, 2014, 10:29 a.m., Tzafrir Cohen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3192/
> ---
> 
> (Updated Feb. 6, 2014, 10:29 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> This is a request to backport code from 
> https://reviewboard.asterisk.org/r/726/ to the stable branches (1.8 and 11).
> 
> 726 is about handling a DAHDI event called DAHDI_EVENT_REMOVED on the 
> D-channel. It was committed in r394552 (and r394567) which are also included 
> in branch 12. It was followed by a releated and complementing change - 
> r396474 ("chan_dahdi: create channels at run-time" , review 1598). The latter 
> would probably considered as a new feature and not considered to backport.
> 
> The issue it is aimed to fix:
> 
> When we disconnect a DAHDI device[1], userspace devices can no longer read 
> from its channels. DAHDI still keep minimal stubs that answer -ENODEV to each 
> relevant system call. DAHDI also sends to userspace the event 
> DAHDI_EVENT_REMOVE on each channel. Asterisk already knows to remove channels 
> when it gets those events. This deals with analog channels, CAS, and MFC/R2. 
> It also deals with the B-channels of an ISDN span. But the D-channel is left 
> open and keeps flooding the log with the following line:
> 
>   [Feb  6 18:06:22] ERROR[9169] chan_dahdi.c: PRI Span: 2 Read on 95 failed: 
> No such device
> 
> which confusingly comes from libpri, as pri.c:__pri_read() does not pass the 
> errno it gets on.
> 
> Applying the reviewed code fixes the issue. This problem has become even 
> simpler to trigger with the introduction of dahdi_span_assignment, though it 
> has been around earlier on.
> 
> [1] physically disconnect, 'rmmod xpp_usb' on and astribank, and with recent 
> DAHDI versions: 'dahdi_span_assignments remove'
> 
> 
> Diffs
> -
> 
>   /branches/1.8/channels/chan_dahdi.c 407565 
> 
> Diff: https://reviewboard.asterisk.org/r/3192/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Tzafrir Cohen
> 
>

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3193: cli: pjsip show endpoint shows allow/disallow codecs the same - OPTION 2

[Scott Griepentrog]

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

(Updated Feb. 6, 2014, 12:50 p.m.)


Review request for Asterisk Developers.


Changes
---

Completed deprecation flag and added an ifdef'd demonstration of how to use 
this to migrate configuration options in sorcery.

It's my belief that this option is the best way to easily deal with the 
allow/disallow confusion - and get a new feature in the process.


Bugs: ASTERISK-23092
https://issues.asterisk.org/jira/browse/ASTERISK-23092


Repository: Asterisk


Description
---

WAS:

Insert a ! prefix in the display of endpoint disallow value.  Result is:

 disallow  : !(ulaw|alaw)

NOW:

Remove the disallow option from generated lists, while still accepting it from 
a configuration file.

This is OPTION 2 - option 1 is https://reviewboard.asterisk.org/r/3136/


Diffs (updated)
-

  /branches/12/res/res_pjsip_outbound_registration.c 407566 
  /branches/12/res/res_pjsip_endpoint_identifier_ip.c 407566 
  /branches/12/res/res_pjsip_acl.c 407566 
  /branches/12/res/res_pjsip/pjsip_options.c 407566 
  /branches/12/res/res_pjsip/pjsip_configuration.c 407566 
  /branches/12/res/res_pjsip/location.c 407566 
  /branches/12/res/res_pjsip/config_transport.c 407566 
  /branches/12/res/res_pjsip/config_system.c 407566 
  /branches/12/res/res_pjsip/config_global.c 407566 
  /branches/12/res/res_pjsip/config_domain_aliases.c 407566 
  /branches/12/res/res_pjsip/config_auth.c 407566 
  /branches/12/main/sorcery.c 407566 
  /branches/12/main/bucket.c 407566 
  /branches/12/include/asterisk/sorcery.h 407566 

Diff: https://reviewboard.asterisk.org/r/3193/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

Re: [asterisk-dev] [Code Review] 3155: ConfBridge: Correct prompt playback target

[rmudgett]

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



branches/11/apps/confbridge/conf_state_multi_marked.c


This sound needs to be played using a deferred join action on the joining 
marked user: conf_add_post_join_action().  Otherwise, the conference bridge is 
locked until the sound prompt completes playing.  Locking the bridge for that 
duration will prevent others from joining or leaving until the sound completes.


- rmudgett


On Jan. 30, 2014, 4:21 p.m., opticron wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3155/
> ---
> 
> (Updated Jan. 30, 2014, 4:21 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: PQ-1396
> https://issues.asterisk.org/jira/browse/PQ-1396
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Currently, when the first marked user enters the conference that contains 
> waitmarked users, a prompt is played indicating that the user is being placed 
> into the conference. Unfortunately, this prompt is played to the marked user 
> and not the waitmarked users which is not very helpful.
> 
> This patch changes that behavior to play a prompt stating "The conference 
> will now begin" to the entire conference after adding and unmuting the 
> waitmarked users since the design of confbridge is not conducive to playing a 
> prompt to a subset of users in a conference in an asynchronous manner.
> 
> 
> Diffs
> -
> 
>   branches/11/configs/confbridge.conf.sample 406782 
>   branches/11/apps/confbridge/include/confbridge.h 406782 
>   branches/11/apps/confbridge/conf_state_multi_marked.c 406782 
>   branches/11/apps/confbridge/conf_state_empty.c 406782 
>   branches/11/apps/confbridge/conf_config_parser.c 406782 
>   branches/11/apps/app_confbridge.c 406782 
>   branches/11/UPGRADE.txt 406782 
> 
> Diff: https://reviewboard.asterisk.org/r/3155/diff/
> 
> 
> Testing
> ---
> 
> Verified that the prompt is heard by users already in the conference when a 
> marked user enters and that ConfBridge tests pass with modified event 
> expectations.
> 
> 
> Thanks,
> 
> opticron
> 
>

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3182: testsuite: LinkedID Propagation test

[Scott Griepentrog]

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

(Updated Feb. 6, 2014, 12:29 p.m.)


Review request for Asterisk Developers.


Changes
---

Fixed per Matt's comments.


Bugs: ASTERISK-23120
https://issues.asterisk.org/jira/browse/ASTERISK-23120


Repository: testsuite


Description (updated)
---

This monitors bridge enter/exit and CEL events, and builds
a picture of which channels are bridged together, and when
their LinkedID changes, and then verifies that each change
is correct.

Note: The BridgeEnter and CEL 'BRIDGE_ENTER' events can be
in either order - so wait for both to arrive before making
a judgement on the LinkedID.

This test can be added to any other bridging test to add a
layer of LinkedId Propagation checking to it.

This test has now been added to the existing bridge_action
test rather than have a separate test for it.


Diffs (updated)
-

  /asterisk/trunk/tests/bridge/bridge_action/test-config.yaml 4675 
  /asterisk/trunk/lib/python/asterisk/linkedid_check.py PRE-CREATION 

Diff: https://reviewboard.asterisk.org/r/3182/diff/


Testing
---

This test fails spuriously due to what appears to be CEL events being sent on 
BRIDGE_ENTER prior to the LinkedId being updated.  This bug will be fixed along 
with the other changes for 23120.


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

Re: [asterisk-dev] Ast 11 / chan_sip: get_rpid/pai not setting CALLERID(num-pres)

[Pavel Troller]

Hello again,
  please discard this noise. It seems I'm on the track now, som call 
redirecton is involved in the process and it seems the incorrect 
presentation comes from there.
  With regards,
Pavel

> Hello there,
>   I'm trying to solve a problem with calling party data for SIP incoming
> calls, where the data are taken from RPID or PAI headers. They are not
> properly propagated to the CALLERID() function (especially the presentation).
>   The symptoms are looking as follows:
> 
> [Feb  6 16:11:43] WARNING[1023][C-000b]: chan_sip.c:17374 get_pai: 
> Presentation from PAI: 23, 1, 0
> [Feb  6 16:11:43] WARNING[1023][C-000b]: chan_sip.c:8097 sip_new: Copying 
> presentation: 23
> cliscrn:orig pres=allowed_not_screened
> 
> The first line is a result of my added debugging statement to get_pai(),
> added to its very end, and written as
> 
> ast_log(AST_LOG_WARNING,"Presentation from PAI: %x, %d, %x\n",callingpres, 
> do_update, p->owner);
> 
> We can see, that the callingpres value is 0x23, which represents
> AST_PRES_PROHIB_NETWORK_NUMBER (yes, it's a result of my former patch,
> normally there would be AST_PRES_PROHIB_USER_NUMBER_NOT_SCREENED, brcause
> when using PAI/Privacy headers, we cannot get the real screening info, but
> I need this value instead). The values are changed, but, as the third
> value indicates, p->owner is still zero, i.e. we still don't have a
> channel to write these values into.
> 
> The second line is still from chan_sip, from sip_new(), and again my added
> debug statement shows, that the presentation value is still present and
> copied to the newly created channel structure.
> 
> However, the third line is my dialplan debugging output showing, that the
> value of actual presentation known to Asterisk is allowed_not_screened, i.e.
> an initial value of zero. The line reads as
> exten => s,n,Verbose(1,cliscrn:orig pres=${CALLERID(num-pres)})
> 
> I'm out of ideas, where the value can get lost. The most interesting thing
> is, that when the call is bridged to some B-party, the destination channel
> is getting correct values! So, maybe an error in CALLERID() function ? But
> for example for calls from DAHDI or IAX2, it shows always correct values...
> 
> Any hint, where to search for the bug, is appreciated! I'm willing to prepare
> a patch, but for now, I'm stuck here.
> 
> With regards,
>   Pavel
> 
> -- 
> _
> -- 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

-- 
_
-- 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

Re: [asterisk-dev] 302 redirects ocassionally ignored; hypothesis: later queued busy preferred to earlier early media frame

[Richard Mudgett]

On Thu, Feb 6, 2014 at 11:43 AM, Dave WOOLLEY wrote:

> One point of detail.  It is actually the write to the alertpipe, within
> ast_queue_control that does the wakeup.
>

Yes, but there needs to be at least one write to the alertpipe for every
frame in the queue.

Richard
-- 
_
-- 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

Re: [asterisk-dev] res_fax_spandsp segfaults during fax detection - FIXED?

[Michal Rybarik]

  
  
Hi Jaco,

if I understand correctly, your segfault did not happen during in
T38gateway, but while receiving fax to tiff file (ReceiveFax), am I
right?
I haven't checked neither patched this (because my Asterisks are
only relaying faxes, not terminating/originating to/from tiff file),
but if your segfault happen when data are passed to libspandsp, it
should be the same situation as mine was. Code in res_fax allows
slinear/alaw/ulaw frames to be passed to res_fax_spandsp and then to
libspandsp, but libspandsp accepts only slinear. When ulaw/alaw
frames are passed here, bad things can happen.
-- 
Regards,
Michal Rybarik


Dňa 6. 2. 2014 12:07, Jaco Kroon  wrote / napísal(a):

  
  Hi All,
  
  Could this backtrace possibly be related?
  
  #0  process_rx_data (t=0x7fae54c698a8, user_data=0x2,
  data_type=1, field_type=,
  buf=0x7fae11c58cda "cng", len=0) at t38_terminal.c:314
  #1  0x7fae11c22c7d in t38_core_rx_ifp_packet
  (s=0x7fae54c698a8, buf=0x7fae54c8475b "\002", len=1,
  seq_no=) at t38_core.c:459
  #2  0x7fae50ea96c5 in generic_fax_exec
  (chan=chan@entry=0x7fadc4548c18,
  details=details@entry=0x7fad50602c28,
  reserved=reserved@entry=0x7fad50155478, token=) at res_fax.c:1498
  #3  0x7fae50eaea9e in receivefax_exec
  (chan=0x7fadc4548c18, data="" out>) at
  res_fax.c:1932
  #4  0x00530fdd in pbx_exec (c=c@entry=0x7fadc4548c18,
  app=app@entry=0x2ddca60, data=""
  "/tmp/morpheus-1391681512.850.tiff") at pbx.c:1622
  #5  0x0053656f in pbx_extension_helper
  (c=c@entry=0x7fadc4548c18, context=,
  exten=exten@entry=0x7fadc4549ab8 "0123489251",
  priority=priority@entry=6, label=label@entry=0x0,
  callerid=callerid@entry=0x7fadc44757b0 "0126413300",
  action="" found=found@entry=0x7fad838bad60,
  
      combined_find_spawn=combined_find_spawn@entry=1, con=0x0)
  at pbx.c:4922
  #6  0x005404a4 in ast_spawn_extension
  (found=0x7fad838bad60, callerid=0x7fadc44757b0 "0126413300",
  priority=6, exten=0x7fadc4549ab8 "0123489251",
  context=, c=0x7fadc4548c18,
  combined_find_spawn=) at pbx.c:6038
  #7  __ast_pbx_run (c=c@entry=0x7fadc4548c18,
  args=args@entry=0x0) at pbx.c:6513
  #8  0x00541c0b in pbx_thread
  (data="" at pbx.c:6843
  #9  0x00587c5a in dummy_start (data=""
  out>) at utils.c:1162
  #10 0x7fae530f2f3a in start_thread (arg=0x7fad838bb700) at
  pthread_create.c:308
  #11 0x7fae54754dad in clone () at
  ../sysdeps/unix/sysv/linux/x86_64/clone.S:113
  
  Had about 11 of those this morning on asterisk 11.7.0. 
  Codec's that's allowed on SIP though is g729 and gsm only, so
  no ulaw/alaw allowed.  Actually, just double checked,
  ulaw/alaw is (was now) allowed, so someone is possibly trying
  to run in bypass mode, resulting in the t38 gateway instead of
  t38 pass through.  I downgraded to 11.6.0 and hadn't had a
  crash since but I opted to disable ulaw+alaw in any case, just
  to be on the safer side.
  

Kind Regards,
  Jaco Kroon
   



  

On 01/02/2014 06:49, Michal Rybárik wrote:
  
  Hello

Pavel, 

On 01/31/2014 07:59 AM, Pavel Troller wrote: 

  This code will translate non-slinear
frames to slinear, just before they 
are sent to libspandsp for v21detection. With this patch
applied, v21 
detection is done also for RTP (SIP) alaw/ulaw frames, so
maybe SIP/G711 
<->  SIP/T38 gateway will work too. I tested
DAHDI<->  SIP/T38, gateway 
works both ways, voice calls too. Is it better now? :o) 
  
  I fully understand the code, but I'm not trained enough in the
  Asterisk 
  internals to respond to questions, which immediately appeared
  in my head: 
  1) In the original code, the result from
  fax_gateway_detect_v21() is returned. 
  Now, you are returning the original frame. I quickly looked at
  the above 
  routine and it in turn calls fax_gateway_request_t38() and
  returns its 
  result (but not always), and in the fax_gateway_request_t38()
  function 
  they are also returning different things according to results
  of the 
  program flow. So, is it really safe to do this ? Are you sure,
  that the 
  real result is really unneccessary ? 
  2) Are yo

Re: [asterisk-dev] [Code Review] 3110: live_ast: run wrapped programs with exec

[Tzafrir Cohen]

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

(Updated Feb. 6, 2014, 5:52 p.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Repository: Asterisk


Description
---

live_ast can be used as a wrapper script to run asterisk, gdb or valgrind. In 
those cases it runs them and returns the result. It is more useful to use 
'exec' to avoid having another odd process in the chain.


Diffs
-

  /trunk/contrib/scripts/live_ast 405032 

Diff: https://reviewboard.asterisk.org/r/3110/diff/


Testing
---


Thanks,

Tzafrir Cohen

-- 
_
-- 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

Re: [asterisk-dev] 302 redirects ocassionally ignored; hypothesis: later queued busy preferred to earlier early media frame

[Dave WOOLLEY]

David wrote:
> 
> Richard wrote:
> 
> > Without looking into the code, queueing a busy frame to wake up the
> > read thread and look at the call-forward string seems wrong.  The null
> > frame should be queued instead.
> 
> This is the code from branch 11 SVN (branch 12 seems to be the same).
> Whilst it is done as a drop through, part of the logic may be to provide and
> appropriate behaviour if redirects have been disabled at a higher level.
> change_redirecting_information eventually sets the field containing the
> address information that Dial uses to detect the redirection.

Sending a null frame followed by the busy might be a safer way of making the 
change.
-- 
David Woolley
BTS Holdings Plc
Tel: +44 (0)20 8401 9000 Fax: +44 (0)20 8401 9100
http://www.bts.co.uk

BTS Holdings PLC - Registered office: BTS House, Manor Road, Wallington, SM6 
0DD - Registered in England: 1517630

-- 
_
-- 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

[asterisk-dev] [Code Review] 3158: indications.conf: fix post-stutter dialtone for in, mx and ph, extra missing stutter

[Tzafrir Cohen]

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

Review request for Asterisk Developers and rmudgett.


Repository: Asterisk


Description
---

The stutter tone for many (most?) countries in indications.conf is not takes 
from national/international standards. If there's no alternative definition for 
a voice mail notification tone, it needs to give a short stutter and then 
continue with the dial tone.

In a number of countries it proceeds with a different dial tone. This fixes the 
definitions of India (in), Mexico (mx) and the Philippines (ph) to proceed with 
their dial tone, rather than with the us dial tone.

In addition to that, stutter tones were added to the following zones: Spain 
(es), Malaysia (my) and Venezuela (ve): I assume that a voicemail notification 
tone would be useful anywhere. Is that a sane assumption?


Diffs
-

  /trunk/configs/indications.conf.sample 407571 

Diff: https://reviewboard.asterisk.org/r/3158/diff/


Testing
---


Thanks,

Tzafrir Cohen

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3136: cli: pjsip show endpoint shows allow/disallow codecs the same - OPTION 1

[Joshua Colp]

> On Feb. 6, 2014, 3:52 p.m., Joshua Colp wrote:
> > While this works I'm not happy with pushing this down to such a low level. 
> > What if in the future I want to filter something else?
> > 
> > What I'd really like to see is something on top which allows you to 
> > arbitrarily filter anything. If something similar came up in the future 
> > then we'd have an immediate easy solution with no core changes and it would 
> > also allow the information to still exist for cases where you do want to 
> > get it.
> 
> Scott Griepentrog wrote:
> The other option would be to add another parameter to the 
> ast_sorcery_object_field_register() which would indicate a "hidden" option.  
> I can write that up as a separate review and see which one is preferred.  
> Which is now up for review: https://reviewboard.asterisk.org/r/3193/
>

I disagree that that is the only other option. You don't have to consume the 
information provided from sorcery as-is, and you don't have to push filtering 
down to that level. Something that sits between sorcery and the consumer can 
easily do the filtering based on information the consumer provides.


- Joshua


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


On Feb. 6, 2014, 5:40 p.m., Scott Griepentrog wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3136/
> ---
> 
> (Updated Feb. 6, 2014, 5:40 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23092
> https://issues.asterisk.org/jira/browse/ASTERISK-23092
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> WAS:
> 
> Insert a ! prefix in the display of endpoint disallow value.  Result is:
> 
>  disallow  : !(ulaw|alaw)
> 
> NOW:
> 
> Remove the disallow option from generated lists, while still accepting it 
> from a configuration file.
> 
> 
> Diffs
> -
> 
>   /branches/12/res/res_pjsip/pjsip_configuration.c 407196 
>   /branches/12/main/sorcery.c 407196 
>   /branches/12/main/config_options.c 407196 
>   /branches/12/include/asterisk/config_options.h 407196 
> 
> 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

Re: [asterisk-dev] [Code Review] 3136: cli: pjsip show endpoint shows allow/disallow codecs the same - OPTION 1

[Joshua Colp]

> On Feb. 6, 2014, 3:52 p.m., Joshua Colp wrote:
> > While this works I'm not happy with pushing this down to such a low level. 
> > What if in the future I want to filter something else?
> > 
> > What I'd really like to see is something on top which allows you to 
> > arbitrarily filter anything. If something similar came up in the future 
> > then we'd have an immediate easy solution with no core changes and it would 
> > also allow the information to still exist for cases where you do want to 
> > get it.
> 
> Scott Griepentrog wrote:
> The other option would be to add another parameter to the 
> ast_sorcery_object_field_register() which would indicate a "hidden" option.  
> I can write that up as a separate review and see which one is preferred.  
> Which is now up for review: https://reviewboard.asterisk.org/r/3193/
>
> 
> Joshua Colp wrote:
> I disagree that that is the only other option. You don't have to consume 
> the information provided from sorcery as-is, and you don't have to push 
> filtering down to that level. Something that sits between sorcery and the 
> consumer can easily do the filtering based on information the consumer 
> provides.

To go a step further: Whether an object field should be hidden is not a 
property of the object field itself, it is a constraint within the consumer.


- Joshua


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


On Feb. 6, 2014, 5:40 p.m., Scott Griepentrog wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3136/
> ---
> 
> (Updated Feb. 6, 2014, 5:40 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23092
> https://issues.asterisk.org/jira/browse/ASTERISK-23092
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> WAS:
> 
> Insert a ! prefix in the display of endpoint disallow value.  Result is:
> 
>  disallow  : !(ulaw|alaw)
> 
> NOW:
> 
> Remove the disallow option from generated lists, while still accepting it 
> from a configuration file.
> 
> 
> Diffs
> -
> 
>   /branches/12/res/res_pjsip/pjsip_configuration.c 407196 
>   /branches/12/main/sorcery.c 407196 
>   /branches/12/main/config_options.c 407196 
>   /branches/12/include/asterisk/config_options.h 407196 
> 
> 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

Re: [asterisk-dev] 302 redirects ocassionally ignored; hypothesis: later queued busy preferred to earlier early media frame

[Dave WOOLLEY]

Richard wrote:

> Without looking into the code, queueing a busy frame to wake up the read 
> thread and look at the call-forward string seems wrong.  The null frame 
> should be queued instead.

This is the code from branch 11 SVN (branch 12 seems to be the same).  Whilst 
it is done as a drop through, part of the logic may be to provide and 
appropriate behaviour if redirects have been disabled at a higher level.  
change_redirecting_information eventually sets the field containing the address 
information that Dial uses to detect the redirection.

case 302: /* Moved temporarily */
case 305: /* Use Proxy */
if (p->owner) {
struct ast_party_redirecting 
redirecting;
struct 
ast_set_party_redirecting update_redirecting;


ast_party_redirecting_init(&redirecting);
memset(&update_redirecting, 0, 
sizeof(update_redirecting));

change_redirecting_information(p, req, &redirecting,
&update_redirecting, 
TRUE);

ast_channel_set_redirecting(p->owner, &redirecting,
&update_redirecting);

ast_party_redirecting_free(&redirecting);
}
/* Fall through */
case 486: /* Busy here */
case 600: /* Busy everywhere */
case 603: /* Decline */
if (p->owner) {
sip_handle_cc(p, req, 
AST_CC_CCBS);
ast_queue_control(p->owner, 
AST_CONTROL_BUSY);

One point of detail.  It is actually the write to the alertpipe, within 
ast_queue_control that does the wakeup.

-- 
David Woolley
BTS Holdings Plc
Tel: +44 (0)20 8401 9000 Fax: +44 (0)20 8401 9100
http://www.bts.co.uk 

BTS Holdings PLC - Registered office: BTS House, Manor Road, Wallington, SM6 
0DD - Registered in England: 1517630

-- 
_
-- 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

[asterisk-dev] [Code Review] 3193: cli: pjsip show endpoint shows allow/disallow codecs the same - OPTION 2

[Scott Griepentrog]

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

Review request for Asterisk Developers.


Bugs: ASTERISK-23092
https://issues.asterisk.org/jira/browse/ASTERISK-23092


Repository: Asterisk


Description
---

WAS:

Insert a ! prefix in the display of endpoint disallow value.  Result is:

 disallow  : !(ulaw|alaw)

NOW:

Remove the disallow option from generated lists, while still accepting it from 
a configuration file.

This is OPTION 2 - option 1 is https://reviewboard.asterisk.org/r/3136/


Diffs
-

  /branches/12/res/res_pjsip_outbound_registration.c 407566 
  /branches/12/res/res_pjsip_endpoint_identifier_ip.c 407566 
  /branches/12/res/res_pjsip_acl.c 407566 
  /branches/12/res/res_pjsip/pjsip_options.c 407566 
  /branches/12/res/res_pjsip/pjsip_configuration.c 407566 
  /branches/12/res/res_pjsip/location.c 407566 
  /branches/12/res/res_pjsip/config_transport.c 407566 
  /branches/12/res/res_pjsip/config_system.c 407566 
  /branches/12/res/res_pjsip/config_global.c 407566 
  /branches/12/res/res_pjsip/config_domain_aliases.c 407566 
  /branches/12/res/res_pjsip/config_auth.c 407566 
  /branches/12/main/sorcery.c 407566 
  /branches/12/main/bucket.c 407566 
  /branches/12/include/asterisk/sorcery.h 407566 

Diff: https://reviewboard.asterisk.org/r/3193/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

Re: [asterisk-dev] [Code Review] 3136: cli: pjsip show endpoint shows allow/disallow codecs the same - OPTION 1

[Scott Griepentrog]

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

(Updated Feb. 6, 2014, 11:40 a.m.)


Review request for Asterisk Developers.


Changes
---

Added link to option 2: https://reviewboard.asterisk.org/r/3193/


Summary (updated)
-

cli: pjsip show endpoint  shows allow/disallow codecs the same - 
OPTION 1


Bugs: ASTERISK-23092
https://issues.asterisk.org/jira/browse/ASTERISK-23092


Repository: Asterisk


Description
---

WAS:

Insert a ! prefix in the display of endpoint disallow value.  Result is:

 disallow  : !(ulaw|alaw)

NOW:

Remove the disallow option from generated lists, while still accepting it from 
a configuration file.


Diffs
-

  /branches/12/res/res_pjsip/pjsip_configuration.c 407196 
  /branches/12/main/sorcery.c 407196 
  /branches/12/main/config_options.c 407196 
  /branches/12/include/asterisk/config_options.h 407196 

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

Re: [asterisk-dev] [Code Review] 3136: cli: pjsip show endpoint shows allow/disallow codecs the same

[Scott Griepentrog]

> On Feb. 6, 2014, 9:52 a.m., Joshua Colp wrote:
> > While this works I'm not happy with pushing this down to such a low level. 
> > What if in the future I want to filter something else?
> > 
> > What I'd really like to see is something on top which allows you to 
> > arbitrarily filter anything. If something similar came up in the future 
> > then we'd have an immediate easy solution with no core changes and it would 
> > also allow the information to still exist for cases where you do want to 
> > get it.

The other option would be to add another parameter to the 
ast_sorcery_object_field_register() which would indicate a "hidden" option.  I 
can write that up as a separate review and see which one is preferred.  Which 
is now up for review: https://reviewboard.asterisk.org/r/3193/


- Scott


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


On Feb. 3, 2014, 2:18 p.m., Scott Griepentrog wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3136/
> ---
> 
> (Updated Feb. 3, 2014, 2:18 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23092
> https://issues.asterisk.org/jira/browse/ASTERISK-23092
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> WAS:
> 
> Insert a ! prefix in the display of endpoint disallow value.  Result is:
> 
>  disallow  : !(ulaw|alaw)
> 
> NOW:
> 
> Remove the disallow option from generated lists, while still accepting it 
> from a configuration file.
> 
> 
> Diffs
> -
> 
>   /branches/12/res/res_pjsip/pjsip_configuration.c 407196 
>   /branches/12/main/sorcery.c 407196 
>   /branches/12/main/config_options.c 407196 
>   /branches/12/include/asterisk/config_options.h 407196 
> 
> 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

Re: [asterisk-dev] [Code Review] 3184: Create sorcery instance registry

[Joshua Colp]

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


Only minor points but I'd really like to see a unit test for this.


branches/12/include/asterisk/sorcery.h


Document that the returned instance has the reference count increased so 
the caller knows they have to unref.



branches/12/main/sorcery.c


Mark this as being safe so if someone looks at it they don't go "say what"


- Joshua Colp


On Feb. 6, 2014, 5 p.m., George Joseph wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3184/
> ---
> 
> (Updated Feb. 6, 2014, 5 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22537
> https://issues.asterisk.org/jira/browse/ASTERISK-22537
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Create sorcery instance registry as a precursor to creating a generic 
> dialplan function that can retrieve parameters from a sorcery-based config 
> file.
> 
> ast_sorcery_init now creates a hashtab as a registry.
> ast_sorcery_open now checks the hashtab for an existing sorcery instance 
> matching the caller's module name.  If it finds one, it bumps the refcount 
> and returns it.  If not, it creates a new sorcery instance, adds it to the 
> hashtab, then returns it.
> ast_sorcery_retrieve_by_module_name is a new function that does a hashtab 
> lookup by module name.  It can be called by the future dialplan function.
> 
> A side effect of this patch is that a module can only have 1 sorcery instance 
> (because it's the key for the hashtab).  res_pjsip/config_system needed a 
> small change to share the main res_pjsip sorcery instance.
> 
> 
> Diffs
> -
> 
>   branches/12/res/res_pjsip/include/res_pjsip_private.h 407566 
>   branches/12/res/res_pjsip/config_system.c 407566 
>   branches/12/res/res_pjsip.c 407566 
>   branches/12/main/sorcery.c 407566 
>   branches/12/include/asterisk/sorcery.h 407566 
> 
> Diff: https://reviewboard.asterisk.org/r/3184/diff/
> 
> 
> Testing
> ---
> 
> Made sure that users of sorcery (mostly res_pjsip) continued to load their 
> configs correctly.
> Made sure there were no ill effects on res_pjsip from config_system sharing 
> the same sorcery instance as the rest of the pjsip infrastructure.
> Made sure that config_system was properly marked as 'not reloadable' and that 
> it was maintaining it's original values when res_pjsip was reloaded.
> 
> 
> Thanks,
> 
> George Joseph
> 
>

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3184: Create sorcery instance registry

[George Joseph]

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

(Updated Feb. 6, 2014, 10 a.m.)


Review request for Asterisk Developers.


Changes
---

Addressed latest issues...
registry containers are now static.
compare and hash functions follow template from wiki.
sorcery registry is now rwlock instead of mutex.
ast_sorcery_unref checks ref count and unregisters instance if no consumers are 
left.


Bugs: ASTERISK-22537
https://issues.asterisk.org/jira/browse/ASTERISK-22537


Repository: Asterisk


Description
---

Create sorcery instance registry as a precursor to creating a generic dialplan 
function that can retrieve parameters from a sorcery-based config file.

ast_sorcery_init now creates a hashtab as a registry.
ast_sorcery_open now checks the hashtab for an existing sorcery instance 
matching the caller's module name.  If it finds one, it bumps the refcount and 
returns it.  If not, it creates a new sorcery instance, adds it to the hashtab, 
then returns it.
ast_sorcery_retrieve_by_module_name is a new function that does a hashtab 
lookup by module name.  It can be called by the future dialplan function.

A side effect of this patch is that a module can only have 1 sorcery instance 
(because it's the key for the hashtab).  res_pjsip/config_system needed a small 
change to share the main res_pjsip sorcery instance.


Diffs (updated)
-

  branches/12/res/res_pjsip/include/res_pjsip_private.h 407566 
  branches/12/res/res_pjsip/config_system.c 407566 
  branches/12/res/res_pjsip.c 407566 
  branches/12/main/sorcery.c 407566 
  branches/12/include/asterisk/sorcery.h 407566 

Diff: https://reviewboard.asterisk.org/r/3184/diff/


Testing
---

Made sure that users of sorcery (mostly res_pjsip) continued to load their 
configs correctly.
Made sure there were no ill effects on res_pjsip from config_system sharing the 
same sorcery instance as the rest of the pjsip infrastructure.
Made sure that config_system was properly marked as 'not reloadable' and that 
it was maintaining it's original values when res_pjsip was reloaded.


Thanks,

George Joseph

-- 
_
-- 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

Re: [asterisk-dev] 302 redirects ocassionally ignored; hypothesis: later queued busy preferred to earlier early media frame

[Richard Mudgett]

On Thu, Feb 6, 2014 at 10:23 AM, Dave WOOLLEY wrote:

> We have a situation where about 1% of incoming 302 responses result in the
> call failing as busy, rather than redirecting.  We are using a heavily
> patched 1.6.1.0, however we have a theory for the mechanism that still
> seems to be valid on Asterisk 12.  Could anyone verify the analysis and
> comment on our current proposed solution, or suggests alternatives?
>
> We are making calls, actually via an ApplianX gateway and iSDX, that can
> end up with a 302 redirect to the voicemail number, if there is no answer.
>  We have Asterisk configured for promiscuous redirects.  Most of the time
>  these work.  However, of the order of 1% of the times, the 302 is received
> (sip debug), and recognized, RDNIS is logged (back port), but the Dial
> operation ends up failing with a busy status.  The interim response before
> the 302 is a 180 with SDP.
>
> Our understanding of what happens is that redirects are passed from the
> channel driver to Dial by setting a call forward string in the channel
> structure, then pushing AST_CONTROL_BUSY into the channel's queue, and
> writing to the alertpipe to wake things up.
>
> When woken up, Dial checks for the presence of the forwarding string, and
> goes into the redirect processing.  If it doesn't find it, it calls
> ast_read.  Amongst other things, ast_read first tries to read from the
> queue, then, if there is nothing there, reads from the channel driver,
> adding everything after the first frame retrieved onto the queue.
>
> If everything arrives from the channel driver, this is the correct order
> of processing, however, if something is pushed directly onto the queue,
> like the AST_CONTROL_BUSY, it will have precedence over frames that were
> available earlier from the channel driver.
>
> Our current hypothesis is that an early media frame arrives on the RTP
> socket.  This causes the ast_waitfor_n in Dial to wake up.  Just after it
> passes the check for a redirection (c->call_forward), the 302 arrives and
> sets the redirection and queues the AST_CONTROL_BUSY.  When Dial reaches
> the ast_read, it reads the AST_CONTROL_BUSY without having seen the
> redirection address and aborts the call.
>
> The simplest approach to this would be to delay do_transfer until after
> the ast_read.  Are there any reasons why this might not be safe?
>

Without looking into the code, queueing a busy frame to wake up the read
thread and look at the call-forward string seems wrong.  The null frame
should be queued instead.

Richard
-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3182: testsuite: LinkedID Propagation test

[Matt Jordan]

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



/asterisk/trunk/tests/bridge/linkedid_propagation/bridge_action.py


Although I'm sure the code this was pulled from used three single quotes to 
denote a pydoc comment, we've since gone with three double quotes in the 
testsuite library modules.

Yes, either are okay - but for the sake of appeasing the gods of 
consistency (who are wont to be filled with wrath), I'd use """ to denote pydoc 
comments from here on out.



/asterisk/trunk/tests/bridge/linkedid_propagation/bridge_action.py


I didn't write this :-)

Or... maybe I did?

If so, and multiple tests need to use this logic, there's no need to copy 
it. In fact, having duplicate copies of what drives the same test isn't needed. 
There are two ways to handle this:

(1) Add the linkedid propagation verification to the existing test. This is 
probably preferable, if what drives the test is exactly the same as another 
test.

(2) If there are differences between the Asterisk actions that drive the 
test, then refactor the common code into a shared module. You have two choices 
on where to put that shared module:
(a) In lib/python/asterisk
(b) In a Python module in a base directory. The YAML for defining pluggable 
modules lets you specify search paths for modules, which you can then specify 
to the location of the module.



/asterisk/trunk/tests/bridge/linkedid_propagation/bridge_action.py


Pedantic: pydoc comments typically don't have a space between the start of 
the comment and the first word of the comment.

So:

"""Foo

Instead of:

""" Foo

Granted, I wrote this before I had bothered to look at that... so if you 
remove this file, feel free to disregard these comments here. If you decide to 
refactor or keep this, then by all means clean up my mistakes :-)



/asterisk/trunk/tests/bridge/linkedid_propagation/configs/ast1/extensions.conf


Formatting (probably tabs/spaces)



/asterisk/trunk/tests/bridge/linkedid_propagation/linkedid_check.py


Pydoc: """ instead of ''' for consistency



/asterisk/trunk/tests/bridge/linkedid_propagation/linkedid_check.py


For Pydoc comments, the summary goes on the same line as the start of the 
comment. So:

"""Foo

Instead of:

"""
Foo



/asterisk/trunk/tests/bridge/linkedid_propagation/test-config.yaml


I think this should just be added as a check in the existing test that 
verifies the BridgeAction AMI action.



/asterisk/trunk/tests/bridge/linkedid_propagation/test-config.yaml


I don't think we need a tag specifically for linkedid checking.

Tags are only useful when you have a number of tests that all verify the 
same thing. Until we have a number of tests that verify linkedids, this doesn't 
really add any value (and even then, how often will we want to run the test 
suite only verifying linkedids?)


- Matt Jordan


On Feb. 4, 2014, 4:16 p.m., Scott Griepentrog wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3182/
> ---
> 
> (Updated Feb. 4, 2014, 4:16 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23120
> https://issues.asterisk.org/jira/browse/ASTERISK-23120
> 
> 
> Repository: testsuite
> 
> 
> Description
> ---
> 
> This monitors bridge enter/exit and CEL events, and builds
> a picture of which channels are bridged together, and when
> their LinkedID changes, and then verifies that each change
> is correct.
> 
> Note: The BridgeEnter and CEL 'BRIDGE_ENTER' events can be
> in either order - so wait for both to arrive before making
> a judgement on the LinkedID.
> 
> This test can be added to any other bridging test to add a
> layer of LinkedId Propagation checking to it.
> 
> 
> Diffs
> -
> 
>   /asterisk/trunk/tests/bridge/tests.yaml 4671 
>   /asterisk/trunk/tests/bridge/linkedid_propagation/test-config.yaml 
> PRE-CREATION 
>   /asterisk/trunk/tests/bridge/linkedid_propagation/linkedid_check.py 
> PRE-CREATION 
>   
> /asterisk/trunk/tests/bridge/linkedid_propagation/configs/ast1/extensions.conf
>  PRE-CREATION 
>   /asterisk/trunk/tests/bridge/linkedid_propag

Re: [asterisk-dev] [Code Review] 3174: chan_iax2: Block unnecessary control frames to/from the wire.

[rmudgett]

> On Feb. 5, 2014, 5:06 p.m., Matt Jordan wrote:
> > /branches/1.8/channels/chan_iax2.c, lines 1289-1293
> > 
> >
> > This shouldn't be needed if you have a 'default' label that disallows 
> > the frame.
> 
> rmudgett wrote:
> You cannot have a default case in the switch and still have the compiler 
> complain if an enum case is missing.  I have a comment at the switch 
> statement to that effect.
> 
> Matt Jordan wrote:
> Why wouldn't we want the compiler to complain if we fail to handle a 
> control frame here?

That's right we want the compiler to complain.  Adding a default to the switch 
statement prevents the compiler complaint because all cases are then handled.


- rmudgett


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


On Feb. 5, 2014, 6:13 p.m., rmudgett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3174/
> ---
> 
> (Updated Feb. 5, 2014, 6:13 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: AST-1302
> https://issues.asterisk.org/jira/browse/AST-1302
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Establishing an IAX2 call between Asterisk v1.4 and v1.8 (or later) results 
> in an unexpected call disconnect.  The problem happens because newer values 
> in the enum ast_control_frame_type are not consistent between the branch 
> versions of Asterisk.
> 
> For example:
> 1) v1.4 calls v1.8 (or later) using IAX2
> 2) v1.8 answers and sends a connected line update control frame. (on v1.8 
> AST_CONTROL_CONNECTED_LINE = 22)
> 3) v1.4 receives the control frame as an end-of-q (on v1.4 
> AST_CONTROL_END_OF_Q = 22)
> 4) v1.4 disconnects the call once the receive queue becomes empty.
> 
> Several things are done by this patch to fix the problem and attempt to 
> prevent it from happening again in the future:
> * Added a warning at the definition of enum ast_control_frame_type about how 
> to add new control frame values.
> 
> * Made block sending and receiving control frames that have no reason to go 
> over the wire.
> 
> * Extended the connectedline iax.conf parameter to also include the 
> redirecting information updates.
> 
> * Updated the connectedline iax.conf parameter documentation to include a 
> notice that the parameter must be "no" when the peer is an Asterisk v1.4 
> instance.
> 
> 
> Diffs
> -
> 
>   /branches/1.8/include/asterisk/frame.h 407562 
>   /branches/1.8/configs/iax.conf.sample 407562 
>   /branches/1.8/channels/chan_iax2.c 407562 
> 
> Diff: https://reviewboard.asterisk.org/r/3174/diff/
> 
> 
> Testing
> ---
> 
> Without the patch the IAX2 call gets disconnected when the call initially 
> connects.
> With the patch the call stays up.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-- 
_
-- 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

[asterisk-dev] [Code Review] 3192: chan_dahdi: handle DAHDI_EVENT_REMOVED on a pri D-Channel

[Tzafrir Cohen]

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

Review request for Asterisk Developers.


Repository: Asterisk


Description
---

This is a request to backport code from https://reviewboard.asterisk.org/r/726/ 
to the stable branches (1.8 and 11).

726 is about handling a DAHDI event called DAHDI_EVENT_REMOVED on the 
D-channel. It was committed in r394552 (and r394567) which are also included in 
branch 12. It was followed by a releated and complementing change - r396474 
("chan_dahdi: create channels at run-time" , review 1598). The latter would 
probably considered as a new feature and not considered to backport.

The issue it is aimed to fix:

When we disconnect a DAHDI device[1], userspace devices can no longer read from 
its channels. DAHDI still keep minimal stubs that answer -ENODEV to each 
relevant system call. DAHDI also sends to userspace the event 
DAHDI_EVENT_REMOVE on each channel. Asterisk already knows to remove channels 
when it gets those events. This deals with analog channels, CAS, and MFC/R2. It 
also deals with the B-channels of an ISDN span. But the D-channel is left open 
and keeps flooding the log with the following line:

  [Feb  6 18:06:22] ERROR[9169] chan_dahdi.c: PRI Span: 2 Read on 95 failed: No 
such device

which confusingly comes from libpri, as pri.c:__pri_read() does not pass the 
errno it gets on.

Applying the reviewed code fixes the issue. This problem has become even 
simpler to trigger with the introduction of dahdi_span_assignment, though it 
has been around earlier on.

[1] physically disconnect, 'rmmod xpp_usb' on and astribank, and with recent 
DAHDI versions: 'dahdi_span_assignments remove'


Diffs
-

  /branches/1.8/channels/chan_dahdi.c 407565 

Diff: https://reviewboard.asterisk.org/r/3192/diff/


Testing
---


Thanks,

Tzafrir Cohen

-- 
_
-- 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

[asterisk-dev] 302 redirects ocassionally ignored; hypothesis: later queued busy preferred to earlier early media frame

[Dave WOOLLEY]

We have a situation where about 1% of incoming 302 responses result in the call 
failing as busy, rather than redirecting.  We are using a heavily patched 
1.6.1.0, however we have a theory for the mechanism that still seems to be 
valid on Asterisk 12.  Could anyone verify the analysis and comment on our 
current proposed solution, or suggests alternatives?

We are making calls, actually via an ApplianX gateway and iSDX, that can end up 
with a 302 redirect to the voicemail number, if there is no answer.  We have 
Asterisk configured for promiscuous redirects.  Most of the time  these work.  
However, of the order of 1% of the times, the 302 is received (sip debug), and 
recognized, RDNIS is logged (back port), but the Dial operation ends up failing 
with a busy status.  The interim response before the 302 is a 180 with SDP.

Our understanding of what happens is that redirects are passed from the channel 
driver to Dial by setting a call forward string in the channel structure, then 
pushing AST_CONTROL_BUSY into the channel's queue, and writing to the alertpipe 
to wake things up.

When woken up, Dial checks for the presence of the forwarding string, and goes 
into the redirect processing.  If it doesn't find it, it calls ast_read.  
Amongst other things, ast_read first tries to read from the queue, then, if 
there is nothing there, reads from the channel driver, adding everything after 
the first frame retrieved onto the queue.

If everything arrives from the channel driver, this is the correct order of 
processing, however, if something is pushed directly onto the queue, like the 
AST_CONTROL_BUSY, it will have precedence over frames that were available 
earlier from the channel driver.

Our current hypothesis is that an early media frame arrives on the RTP socket.  
This causes the ast_waitfor_n in Dial to wake up.  Just after it passes the 
check for a redirection (c->call_forward), the 302 arrives and sets the 
redirection and queues the AST_CONTROL_BUSY.  When Dial reaches the ast_read, 
it reads the AST_CONTROL_BUSY without having seen the redirection address and 
aborts the call.

The simplest approach to this would be to delay do_transfer until after the 
ast_read.  Are there any reasons why this might not be safe?

-- 
David Woolley
BTS Holdings Plc
Tel: +44 (0)20 8401 9000 Fax: +44 (0)20 8401 9100
http://www.bts.co.uk 


BTS Holdings PLC - Registered office: BTS House, Manor Road, Wallington, SM6 
0DD - Registered in England: 1517630

-- 
_
-- 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

[asterisk-dev] Ast 11 / chan_sip: get_rpid/pai not setting CALLERID(num-pres)

[Pavel Troller]

Hello there,
  I'm trying to solve a problem with calling party data for SIP incoming
calls, where the data are taken from RPID or PAI headers. They are not
properly propagated to the CALLERID() function (especially the presentation).
  The symptoms are looking as follows:

[Feb  6 16:11:43] WARNING[1023][C-000b]: chan_sip.c:17374 get_pai: 
Presentation from PAI: 23, 1, 0
[Feb  6 16:11:43] WARNING[1023][C-000b]: chan_sip.c:8097 sip_new: Copying 
presentation: 23
cliscrn:orig pres=allowed_not_screened

The first line is a result of my added debugging statement to get_pai(),
added to its very end, and written as

ast_log(AST_LOG_WARNING,"Presentation from PAI: %x, %d, %x\n",callingpres, 
do_update, p->owner);

We can see, that the callingpres value is 0x23, which represents
AST_PRES_PROHIB_NETWORK_NUMBER (yes, it's a result of my former patch,
normally there would be AST_PRES_PROHIB_USER_NUMBER_NOT_SCREENED, brcause
when using PAI/Privacy headers, we cannot get the real screening info, but
I need this value instead). The values are changed, but, as the third
value indicates, p->owner is still zero, i.e. we still don't have a
channel to write these values into.

The second line is still from chan_sip, from sip_new(), and again my added
debug statement shows, that the presentation value is still present and
copied to the newly created channel structure.

However, the third line is my dialplan debugging output showing, that the
value of actual presentation known to Asterisk is allowed_not_screened, i.e.
an initial value of zero. The line reads as
exten => s,n,Verbose(1,cliscrn:orig pres=${CALLERID(num-pres)})

I'm out of ideas, where the value can get lost. The most interesting thing
is, that when the call is bridged to some B-party, the destination channel
is getting correct values! So, maybe an error in CALLERID() function ? But
for example for calls from DAHDI or IAX2, it shows always correct values...

Any hint, where to search for the bug, is appreciated! I'm willing to prepare
a patch, but for now, I'm stuck here.

With regards,
  Pavel

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3136: cli: pjsip show endpoint shows allow/disallow codecs the same

[Joshua Colp]

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


While this works I'm not happy with pushing this down to such a low level. What 
if in the future I want to filter something else?

What I'd really like to see is something on top which allows you to arbitrarily 
filter anything. If something similar came up in the future then we'd have an 
immediate easy solution with no core changes and it would also allow the 
information to still exist for cases where you do want to get it.

- Joshua Colp


On Feb. 3, 2014, 8:18 p.m., Scott Griepentrog wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3136/
> ---
> 
> (Updated Feb. 3, 2014, 8:18 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23092
> https://issues.asterisk.org/jira/browse/ASTERISK-23092
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> WAS:
> 
> Insert a ! prefix in the display of endpoint disallow value.  Result is:
> 
>  disallow  : !(ulaw|alaw)
> 
> NOW:
> 
> Remove the disallow option from generated lists, while still accepting it 
> from a configuration file.
> 
> 
> Diffs
> -
> 
>   /branches/12/res/res_pjsip/pjsip_configuration.c 407196 
>   /branches/12/main/sorcery.c 407196 
>   /branches/12/main/config_options.c 407196 
>   /branches/12/include/asterisk/config_options.h 407196 
> 
> 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

Re: [asterisk-dev] [Code Review] 3186: AMI Security Events: document the events; add optional IEs to the events

[Joshua Colp]

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

Ship it!


Ship It!

- Joshua Colp


On Feb. 5, 2014, 10:51 p.m., Matt Jordan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3186/
> ---
> 
> (Updated Feb. 5, 2014, 10:51 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> This patch:
> (1) Adds optional IEs to the AMI events. The optional fields are documented 
> as not being required.
> (2) Adds documentation for all of the Security Events.
> 
> 
> Diffs
> -
> 
>   /branches/12/main/security_events.c 407401 
>   /branches/12/UPGRADE.txt 407402 
>   /branches/12/CHANGES 407402 
> 
> Diff: https://reviewboard.asterisk.org/r/3186/diff/
> 
> 
> Testing
> ---
> 
> Tests that use the AMI events in the testsuite continue to pass.
> 
> Tested documentation through the CLI.
> 
> 
> Thanks,
> 
> Matt Jordan
> 
>

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3183: ARI: pass channel variables into originate as opposed to assigning after originate

[Corey Farrell]

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



/branches/12/res/ari/resource_channels.c


I feel the following prototype would be better for the caller:
static int json_to_ast_variables(struct ast_json *src, struct ast_variable 
**dest);
Return 0 or -1 for success/failure.

This way the caller can tell between a failure and an empty json_variables 
object.



/branches/12/res/ari/resource_channels.c


If we actually failed to create variables that were requested, we should 
abort the Originate.  Half-success is worse than total failure.


- Corey Farrell


On Feb. 6, 2014, 10:05 a.m., Matt Jordan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3183/
> ---
> 
> (Updated Feb. 6, 2014, 10:05 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> This patch tweaks the behaviour of POST /channels with channel variables such 
> that the variables are passed into the pbx.c routines that perform the 
> origination. This allows the variables to be assigned to the newly created 
> channels immediately upon their construction, as opposed to be assigned after 
> the originate has completed.
> 
> The upshot of this is that the variables are available on the channels if 
> they execute in the dialplan, as opposed to only being available once the 
> channels are answered.
> 
> 
> Diffs
> -
> 
>   /branches/12/res/ari/resource_channels.c 407562 
> 
> Diff: https://reviewboard.asterisk.org/r/3183/diff/
> 
> 
> Testing
> ---
> 
> Both testsuite originate tests still pass.
> 
> 
> Thanks,
> 
> Matt Jordan
> 
>

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3186: AMI Security Events: document the events; add optional IEs to the events

[Leif Madsen]

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

Ship it!


Other than the s/, security/, 'security'/ in the CHANGES file, this looks good 
to me!

- Leif Madsen


On Feb. 5, 2014, 10:51 p.m., Matt Jordan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3186/
> ---
> 
> (Updated Feb. 5, 2014, 10:51 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> This patch:
> (1) Adds optional IEs to the AMI events. The optional fields are documented 
> as not being required.
> (2) Adds documentation for all of the Security Events.
> 
> 
> Diffs
> -
> 
>   /branches/12/main/security_events.c 407401 
>   /branches/12/UPGRADE.txt 407402 
>   /branches/12/CHANGES 407402 
> 
> Diff: https://reviewboard.asterisk.org/r/3186/diff/
> 
> 
> Testing
> ---
> 
> Tests that use the AMI events in the testsuite continue to pass.
> 
> Tested documentation through the CLI.
> 
> 
> Thanks,
> 
> Matt Jordan
> 
>

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3183: ARI: pass channel variables into originate as opposed to assigning after originate

[Matt Jordan]

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

(Updated Feb. 6, 2014, 9:05 a.m.)


Review request for Asterisk Developers.


Changes
---

Added a WARNING if variable construction fails.


Repository: Asterisk


Description
---

This patch tweaks the behaviour of POST /channels with channel variables such 
that the variables are passed into the pbx.c routines that perform the 
origination. This allows the variables to be assigned to the newly created 
channels immediately upon their construction, as opposed to be assigned after 
the originate has completed.

The upshot of this is that the variables are available on the channels if they 
execute in the dialplan, as opposed to only being available once the channels 
are answered.


Diffs (updated)
-

  /branches/12/res/ari/resource_channels.c 407562 

Diff: https://reviewboard.asterisk.org/r/3183/diff/


Testing
---

Both testsuite originate tests still pass.


Thanks,

Matt Jordan

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3172: testsuite: chan_sip Record-Route test

[Matt Jordan]

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

Ship it!


Hooray for tests!


/asterisk/trunk/tests/channels/SIP/route/test-config.yaml


Pedantic: go ahead and specify the full Asterisk version of '1.8.0.0' here.


- Matt Jordan


On Feb. 3, 2014, 6:41 a.m., Corey Farrell wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3172/
> ---
> 
> (Updated Feb. 3, 2014, 6:41 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22582
> https://issues.asterisk.org/jira/browse/ASTERISK-22582
> 
> 
> Repository: testsuite
> 
> 
> Description
> ---
> 
> This adds a basic test of chan_sip Record-Route handling.
> 
> * strict and loose routes
> * inbound and outbound calls
> * verifies expected values for Record-Route and Route headers
> * verifies expected method URL's
> 
> 
> Diffs
> -
> 
>   /asterisk/trunk/tests/channels/SIP/tests.yaml 4663 
>   /asterisk/trunk/tests/channels/SIP/route/test-config.yaml PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/route/sipp/send-invite.xml PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/route/sipp/recv-invite.xml PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/route/configs/ast1/sip.conf PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/route/configs/ast1/extensions.conf 
> PRE-CREATION 
> 
> Diff: https://reviewboard.asterisk.org/r/3172/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Corey Farrell
> 
>

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3183: ARI: pass channel variables into originate as opposed to assigning after originate

[Matt Jordan]

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

(Updated Feb. 6, 2014, 8:51 a.m.)


Review request for Asterisk Developers.


Changes
---

Addressed Josh's finding.


Repository: Asterisk


Description
---

This patch tweaks the behaviour of POST /channels with channel variables such 
that the variables are passed into the pbx.c routines that perform the 
origination. This allows the variables to be assigned to the newly created 
channels immediately upon their construction, as opposed to be assigned after 
the originate has completed.

The upshot of this is that the variables are available on the channels if they 
execute in the dialplan, as opposed to only being available once the channels 
are answered.


Diffs (updated)
-

  /branches/12/res/ari/resource_channels.c 407562 

Diff: https://reviewboard.asterisk.org/r/3183/diff/


Testing
---

Both testsuite originate tests still pass.


Thanks,

Matt Jordan

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3155: ConfBridge: Correct prompt playback target

[Matt Jordan]

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

Ship it!


Ship It!

- Matt Jordan


On Jan. 30, 2014, 4:21 p.m., opticron wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3155/
> ---
> 
> (Updated Jan. 30, 2014, 4:21 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: PQ-1396
> https://issues.asterisk.org/jira/browse/PQ-1396
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Currently, when the first marked user enters the conference that contains 
> waitmarked users, a prompt is played indicating that the user is being placed 
> into the conference. Unfortunately, this prompt is played to the marked user 
> and not the waitmarked users which is not very helpful.
> 
> This patch changes that behavior to play a prompt stating "The conference 
> will now begin" to the entire conference after adding and unmuting the 
> waitmarked users since the design of confbridge is not conducive to playing a 
> prompt to a subset of users in a conference in an asynchronous manner.
> 
> 
> Diffs
> -
> 
>   branches/11/configs/confbridge.conf.sample 406782 
>   branches/11/apps/confbridge/include/confbridge.h 406782 
>   branches/11/apps/confbridge/conf_state_multi_marked.c 406782 
>   branches/11/apps/confbridge/conf_state_empty.c 406782 
>   branches/11/apps/confbridge/conf_config_parser.c 406782 
>   branches/11/apps/app_confbridge.c 406782 
>   branches/11/UPGRADE.txt 406782 
> 
> Diff: https://reviewboard.asterisk.org/r/3155/diff/
> 
> 
> Testing
> ---
> 
> Verified that the prompt is heard by users already in the conference when a 
> marked user enters and that ConfBridge tests pass with modified event 
> expectations.
> 
> 
> Thanks,
> 
> opticron
> 
>

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3174: chan_iax2: Block unnecessary control frames to/from the wire.

[Matt Jordan]

> On Feb. 5, 2014, 5:06 p.m., Matt Jordan wrote:
> > /branches/1.8/channels/chan_iax2.c, lines 1289-1293
> > 
> >
> > This shouldn't be needed if you have a 'default' label that disallows 
> > the frame.
> 
> rmudgett wrote:
> You cannot have a default case in the switch and still have the compiler 
> complain if an enum case is missing.  I have a comment at the switch 
> statement to that effect.

Why wouldn't we want the compiler to complain if we fail to handle a control 
frame here?


- Matt


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


On Feb. 5, 2014, 6:13 p.m., rmudgett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3174/
> ---
> 
> (Updated Feb. 5, 2014, 6:13 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: AST-1302
> https://issues.asterisk.org/jira/browse/AST-1302
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Establishing an IAX2 call between Asterisk v1.4 and v1.8 (or later) results 
> in an unexpected call disconnect.  The problem happens because newer values 
> in the enum ast_control_frame_type are not consistent between the branch 
> versions of Asterisk.
> 
> For example:
> 1) v1.4 calls v1.8 (or later) using IAX2
> 2) v1.8 answers and sends a connected line update control frame. (on v1.8 
> AST_CONTROL_CONNECTED_LINE = 22)
> 3) v1.4 receives the control frame as an end-of-q (on v1.4 
> AST_CONTROL_END_OF_Q = 22)
> 4) v1.4 disconnects the call once the receive queue becomes empty.
> 
> Several things are done by this patch to fix the problem and attempt to 
> prevent it from happening again in the future:
> * Added a warning at the definition of enum ast_control_frame_type about how 
> to add new control frame values.
> 
> * Made block sending and receiving control frames that have no reason to go 
> over the wire.
> 
> * Extended the connectedline iax.conf parameter to also include the 
> redirecting information updates.
> 
> * Updated the connectedline iax.conf parameter documentation to include a 
> notice that the parameter must be "no" when the peer is an Asterisk v1.4 
> instance.
> 
> 
> Diffs
> -
> 
>   /branches/1.8/include/asterisk/frame.h 407562 
>   /branches/1.8/configs/iax.conf.sample 407562 
>   /branches/1.8/channels/chan_iax2.c 407562 
> 
> Diff: https://reviewboard.asterisk.org/r/3174/diff/
> 
> 
> Testing
> ---
> 
> Without the patch the IAX2 call gets disconnected when the call initially 
> connects.
> With the patch the call stays up.
> 
> 
> Thanks,
> 
> rmudgett
> 
>

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3183: ARI: pass channel variables into originate as opposed to assigning after originate

[Joshua Colp]

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

Ship it!


Besides my error comment this looks good.


/branches/12/res/ari/resource_channels.c


I'd throw out an error here if we got no variables.


- Joshua Colp


On Feb. 6, 2014, 2:51 p.m., Matt Jordan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3183/
> ---
> 
> (Updated Feb. 6, 2014, 2:51 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> This patch tweaks the behaviour of POST /channels with channel variables such 
> that the variables are passed into the pbx.c routines that perform the 
> origination. This allows the variables to be assigned to the newly created 
> channels immediately upon their construction, as opposed to be assigned after 
> the originate has completed.
> 
> The upshot of this is that the variables are available on the channels if 
> they execute in the dialplan, as opposed to only being available once the 
> channels are answered.
> 
> 
> Diffs
> -
> 
>   /branches/12/res/ari/resource_channels.c 407562 
> 
> Diff: https://reviewboard.asterisk.org/r/3183/diff/
> 
> 
> Testing
> ---
> 
> Both testsuite originate tests still pass.
> 
> 
> Thanks,
> 
> Matt Jordan
> 
>

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3143: pjsip_configuration: in ast_sip_auth_array_init, change assert(auths->names == NULL) to ast_sip_auth_array_destroy(auths)

[George Joseph]

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

(Updated Feb. 6, 2014, 2:50 p.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Bugs: ASTERISK-23168
https://issues.asterisk.org/jira/browse/ASTERISK-23168


Repository: Asterisk


Description
---

Currently, ast_sip_auth_array_init asserts that auths->names is NULL 
effectively barfing if the array being initialized already contained auth 
names.  This works OK except where the sorcery object is derived from a 
template and the object tries to override the auth parameter.

Example:

[template](!)
type=registration
outbound_auth=someauth

[somereg](template)
type=registration
outbound_auth=someOTHERauth

Causes...
ERROR[29423]: res_pjsip/pjsip_configuration.c:236 ast_sip_auth_array_init: 
FRACK!, Failed assertion auths->names == NULL (0)
ERROR[29423]: res_pjsip/pjsip_configuration.c:237 ast_sip_auth_array_init: 
FRACK!, Failed assertion !auths->num (0)

This patch simply tests auths->names and if not null, calls 
ast_sip_auth_array_destroy before initializing the array with the new values.


Diffs
-

  branches/12/res/res_pjsip/pjsip_configuration.c 405922 

Diff: https://reviewboard.asterisk.org/r/3143/diff/


Testing
---


Thanks,

George Joseph

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3183: ARI: pass channel variables into originate as opposed to assigning after originate

[Matt Jordan]

> On Feb. 6, 2014, 6:58 a.m., Joshua Colp wrote:
> > /branches/12/res/ari/resource_channels.c, lines 704-705
> > 
> >
> > Should this actually just completely fail this? Otherwise you get a 
> > list which doesn't contain all of your JSON variables. Which, if you 
> > experienced it would cause you to scratch your head and spend time 
> > wondering what is going on.

Works for me. Fixed.


- Matt


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


On Feb. 5, 2014, 10:41 a.m., Matt Jordan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3183/
> ---
> 
> (Updated Feb. 5, 2014, 10:41 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> This patch tweaks the behaviour of POST /channels with channel variables such 
> that the variables are passed into the pbx.c routines that perform the 
> origination. This allows the variables to be assigned to the newly created 
> channels immediately upon their construction, as opposed to be assigned after 
> the originate has completed.
> 
> The upshot of this is that the variables are available on the channels if 
> they execute in the dialplan, as opposed to only being available once the 
> channels are answered.
> 
> 
> Diffs
> -
> 
>   /branches/12/res/ari/resource_channels.c 407401 
> 
> Diff: https://reviewboard.asterisk.org/r/3183/diff/
> 
> 
> Testing
> ---
> 
> Both testsuite originate tests still pass.
> 
> 
> Thanks,
> 
> Matt Jordan
> 
>

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3188: format_wav: enhancing log message "Not a wav file" to be clear on what is supported

[rnewton]

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

(Updated Feb. 6, 2014, 1:29 p.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Bugs: ASTERISK-22310
https://issues.asterisk.org/jira/browse/ASTERISK-22310


Repository: Asterisk


Description
---

>From Jim's report:

"The following error message lies. Many things are WAV files which do not 
contain 1 in the format field.

if (ltohs(format) != 1) {
ast_log(LOG_WARNING, "Not a wav file %d\n", ltohs(format));

Given how unusual the .WAV vs. .wav distinction is I'd go for: "Not a supported 
wav file format (%d). Only PCM encoded versions are supported with a lower-case 
'.wav' extension."

Which would be helpful."


The message I've suggested in the patch is:

"Not a supported wav file format (%d). Only PCM encoded, 16 bit, mono, 8kHz 
files are supported with a lowercase '.wav' extension.\n"

Please let me know:

1) Is the message content accurate according to format_wav.c ?
2) Is the message more understandable and less ambiguous than the original?


Diffs
-

  /branches/12/formats/format_wav.c 407338 

Diff: https://reviewboard.asterisk.org/r/3188/diff/


Testing
---


Thanks,

rnewton

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3180: Documenation: Configuration section naming in pjsip.conf.sample needs a little clarification

[Joshua Colp]

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

Ship it!


Ship It!

- Joshua Colp


On Feb. 5, 2014, 9:10 p.m., rnewton wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3180/
> ---
> 
> (Updated Feb. 5, 2014, 9:10 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> There is some nuance to naming configuration sections. In the long term I'll 
> hope that configuration section names become a bit more arbitrary; in the 
> short term help me make sure this documentation patch clarifies things.
> 
> 
> Diffs
> -
> 
>   /branches/12/configs/pjsip.conf.sample 407338 
> 
> Diff: https://reviewboard.asterisk.org/r/3180/diff/
> 
> 
> Testing
> ---
> 
> Only adding informational text to the pjsip.conf.sample file.
> 
> 
> Thanks,
> 
> rnewton
> 
>

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3184: Create sorcery instance registry

[Joshua Colp]

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



branches/12/main/sorcery.c


If you create a zero sized array at the end you can allocate the memory you 
need with the main structure alloc.



branches/12/main/sorcery.c


These two should be static. Go ahead and fix wizards with this.



branches/12/main/sorcery.c


All new ao2 comparison and hashing functions should use these templates:

https://wiki.asterisk.org/wiki/pages/viewpage.action?pageId=25919686



branches/12/main/sorcery.c


Go ahead and make this protected by a read/write lock. Sorcery instances 
are not something that really come and go, they are generally created once at 
startup.



branches/12/main/sorcery.c


This function will never get called now. The ao2 container has a reference 
to the sorcery instance itself. You could do something where in 
ast_sorcery_unref if it is the last reference it is unlinked from the instances 
container.


- Joshua Colp


On Feb. 5, 2014, 8:48 p.m., George Joseph wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3184/
> ---
> 
> (Updated Feb. 5, 2014, 8:48 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-22537
> https://issues.asterisk.org/jira/browse/ASTERISK-22537
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Create sorcery instance registry as a precursor to creating a generic 
> dialplan function that can retrieve parameters from a sorcery-based config 
> file.
> 
> ast_sorcery_init now creates a hashtab as a registry.
> ast_sorcery_open now checks the hashtab for an existing sorcery instance 
> matching the caller's module name.  If it finds one, it bumps the refcount 
> and returns it.  If not, it creates a new sorcery instance, adds it to the 
> hashtab, then returns it.
> ast_sorcery_retrieve_by_module_name is a new function that does a hashtab 
> lookup by module name.  It can be called by the future dialplan function.
> 
> A side effect of this patch is that a module can only have 1 sorcery instance 
> (because it's the key for the hashtab).  res_pjsip/config_system needed a 
> small change to share the main res_pjsip sorcery instance.
> 
> 
> Diffs
> -
> 
>   branches/12/res/res_pjsip/include/res_pjsip_private.h 407454 
>   branches/12/res/res_pjsip/config_system.c 407454 
>   branches/12/res/res_pjsip.c 407454 
>   branches/12/main/sorcery.c 407454 
>   branches/12/include/asterisk/sorcery.h 407454 
> 
> Diff: https://reviewboard.asterisk.org/r/3184/diff/
> 
> 
> Testing
> ---
> 
> Made sure that users of sorcery (mostly res_pjsip) continued to load their 
> configs correctly.
> Made sure there were no ill effects on res_pjsip from config_system sharing 
> the same sorcery instance as the rest of the pjsip infrastructure.
> Made sure that config_system was properly marked as 'not reloadable' and that 
> it was maintaining it's original values when res_pjsip was reloaded.
> 
> 
> Thanks,
> 
> George Joseph
> 
>

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3183: ARI: pass channel variables into originate as opposed to assigning after originate

[Joshua Colp]

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



/branches/12/res/ari/resource_channels.c


Should this actually just completely fail this? Otherwise you get a list 
which doesn't contain all of your JSON variables. Which, if you experienced it 
would cause you to scratch your head and spend time wondering what is going on.


- Joshua Colp


On Feb. 5, 2014, 4:41 p.m., Matt Jordan wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3183/
> ---
> 
> (Updated Feb. 5, 2014, 4:41 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> This patch tweaks the behaviour of POST /channels with channel variables such 
> that the variables are passed into the pbx.c routines that perform the 
> origination. This allows the variables to be assigned to the newly created 
> channels immediately upon their construction, as opposed to be assigned after 
> the originate has completed.
> 
> The upshot of this is that the variables are available on the channels if 
> they execute in the dialplan, as opposed to only being available once the 
> channels are answered.
> 
> 
> Diffs
> -
> 
>   /branches/12/res/ari/resource_channels.c 407401 
> 
> Diff: https://reviewboard.asterisk.org/r/3183/diff/
> 
> 
> Testing
> ---
> 
> Both testsuite originate tests still pass.
> 
> 
> Thanks,
> 
> Matt Jordan
> 
>

-- 
_
-- 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

Re: [asterisk-dev] res_fax_spandsp segfaults during fax detection - FIXED?

[Jaco Kroon]

  
  
Hi All,

Could this backtrace possibly be related?

#0  process_rx_data (t=0x7fae54c698a8, user_data=0x2,
data_type=1, field_type=,
buf=0x7fae11c58cda "cng", len=0) at t38_terminal.c:314
#1  0x7fae11c22c7d in t38_core_rx_ifp_packet
(s=0x7fae54c698a8, buf=0x7fae54c8475b "\002", len=1,
seq_no=) at t38_core.c:459
#2  0x7fae50ea96c5 in generic_fax_exec
(chan=chan@entry=0x7fadc4548c18,
details=details@entry=0x7fad50602c28,
reserved=reserved@entry=0x7fad50155478, token=) at res_fax.c:1498
#3  0x7fae50eaea9e in receivefax_exec (chan=0x7fadc4548c18,
data="" out>) at res_fax.c:1932
#4  0x00530fdd in pbx_exec (c=c@entry=0x7fadc4548c18,
app=app@entry=0x2ddca60, data=""
"/tmp/morpheus-1391681512.850.tiff") at pbx.c:1622
#5  0x0053656f in pbx_extension_helper
(c=c@entry=0x7fadc4548c18, context=,
exten=exten@entry=0x7fadc4549ab8 "0123489251",
priority=priority@entry=6, label=label@entry=0x0,
callerid=callerid@entry=0x7fadc44757b0 "0126413300",
action="" found=found@entry=0x7fad838bad60, 
    combined_find_spawn=combined_find_spawn@entry=1, con=0x0) at
pbx.c:4922
#6  0x005404a4 in ast_spawn_extension
(found=0x7fad838bad60, callerid=0x7fadc44757b0 "0126413300",
priority=6, exten=0x7fadc4549ab8 "0123489251",
context=, c=0x7fadc4548c18,
combined_find_spawn=) at pbx.c:6038
#7  __ast_pbx_run (c=c@entry=0x7fadc4548c18,
args=args@entry=0x0) at pbx.c:6513
#8  0x00541c0b in pbx_thread
(data="" at pbx.c:6843
#9  0x00587c5a in dummy_start (data=""
out>) at utils.c:1162
#10 0x7fae530f2f3a in start_thread (arg=0x7fad838bb700) at
pthread_create.c:308
#11 0x7fae54754dad in clone () at
../sysdeps/unix/sysv/linux/x86_64/clone.S:113

Had about 11 of those this morning on asterisk 11.7.0.  Codec's
that's allowed on SIP though is g729 and gsm only, so no
ulaw/alaw allowed.  Actually, just double checked, ulaw/alaw is
(was now) allowed, so someone is possibly trying to run in
bypass mode, resulting in the t38 gateway instead of t38 pass
through.  I downgraded to 11.6.0 and hadn't had a crash since
but I opted to disable ulaw+alaw in any case, just to be on the
safer side.

  
  Kind Regards,
Jaco Kroon


  
  
  

  
  On 01/02/2014 06:49, Michal Rybárik wrote:

Hello
  Pavel,
  
  
  On 01/31/2014 07:59 AM, Pavel Troller wrote:
  
  
This code will translate non-slinear
  frames to slinear, just before they
  
  are sent to libspandsp for v21detection. With this patch
  applied, v21
  
  detection is done also for RTP (SIP) alaw/ulaw frames, so
  maybe SIP/G711
  
  <->  SIP/T38 gateway will work too. I tested
  DAHDI<->  SIP/T38, gateway
  
  works both ways, voice calls too. Is it better now? :o)
  

I fully understand the code, but I'm not trained enough in the
Asterisk

internals to respond to questions, which immediately appeared in
my head:

1) In the original code, the result from
fax_gateway_detect_v21() is returned.

Now, you are returning the original frame. I quickly looked at
the above

routine and it in turn calls fax_gateway_request_t38() and
returns its

result (but not always), and in the fax_gateway_request_t38()
function

they are also returning different things according to results of
the

program flow. So, is it really safe to do this ? Are you sure,
that the

real result is really unneccessary ?

2) Are you sure, that ast_translate() will always allocate a new
buffer for

tmpframe ? Is it written somewhere ? Isn't it possible that it
will just

reallocate the buffer for the original frame to increase its
size and return

its pointer, so by doing ast_frfree() you would just deallocate
the same

buffer, thus making big troubles ? You would find it by checking
that

tmpframe != f...

   As you can see, I'm very careful, or maybe even a bit
conservative, with

patching things, unless I really DEEPLY understand, how they are
going...

So, I believe, that you really studied the code enough to be
sure, that

you can reall