Re: [asterisk-dev] Asterisk MoS and RTCP data [pardon the directness and language :-)]

[Matthew Jordan]

On Thu, Nov 6, 2014 at 8:30 AM, Nir Simionovich
 wrote:
> Hi All,
>
> So, if there is one thing I really like about PJSIP and WebRTC (specifically
> with mobile) is the ability to produce meaningful MoS scoring for calls in
> real time. Now, Asterisk doesn't have that capability, at least, not during
> the actual call - but only after.

Well, you can get real-time RTCP statistics over AMI - the events of
which in 12+ do contain the channel information (thanks to Olle and
Jaco Kroon's patches). So you can correlate quality with channels
(albeit in an external system).

> In itself, not an issue - it's good enough.
>
> If you were to look up the calculation for how to get your MoS, the
> following is the most common
> algorithm that I've found:
>
> effectiveLatency = rttMs + averageJitterMs * 2 + 10
> R = 93.2 - (effectiveLatency/40)
> R = R - (fractionLost * 2.5)
> MOS = 1 + (0.035)*R+(0.07)*R*(R-60)*(100-R)
>
> My primary question is this: I can't find what rttMs in Asterisk terminology
> is. There is an rtt value,
> but its value is several orders of magnitude than I would expect. So, I
> thought it may be in microSec, not miliSec - but that doesn't make much
> sense either.

Where are you getting the RTT from? It *may* differ, depending on
where you read it from.

In AMI (in 12+, at least), the RTT is in seconds. The following
RTCPReceived event is taken from the log of the hep/rtcp-sender test
in the Asterisk Test Suite (and is documented on the wiki here
https://wiki.asterisk.org/wiki/display/AST/Asterisk+13+ManagerEvent_RTCPReceived):

Event: RTCPReceived
Privilege: reporting,all
Channel: PJSIP/ast2-
ChannelState: 6
ChannelStateDesc: Up
CallerIDNum: 1000
CallerIDName: 
ConnectedLineNum: 
ConnectedLineName: 
AccountCode:
Context: default
Exten:
Priority: 1
Uniqueid: 1415331776.5
To: 127.0.1.1:0
From: 127.0.0.1:11007
RTT: 0.0333
SSRC: 0x39f81562
PT: 200(SR)
ReportCount: 1
SentNTP: 1415331786.17547983142912
SentRTP: 80160
SentPackets: 501
SentOctets: 80160
Report0SourceSSRC: 0x6ed0a094
Report0FractionLost: 0
Report0CumulativeLost: 0
Report0HighestSequence: 59911
Report0SequenceNumberCycles: 0
Report0IAJitter: 0
Report0LSR: 3192234347
Report0DLSR: 5.

The RTT is calculated as:
  current_time - received last SR time - received last delay since last SR time

This matches what RFC 3550 recommends:

{quote}
 Let SSRC_r denote the receiver issuing this receiver report.
  Source SSRC_n can compute the round-trip propagation delay to
  SSRC_r by recording the time A when this reception report block is
  received.  It calculates the total round-trip time A-LSR using the
  last SR timestamp (LSR) field, and then subtracting this field to
  leave the round-trip propagation delay as (A - LSR - DLSR).
{quote}


> So, on one hand I have a shit load of information, on the other hand, it is
> formatted in a very hard way to manage.
>
> Any idea how we can make this better? On another notion, I think that adding
> RTCP reading capabilities to the channels module in ARI may probe extremely
> useful.
>

You're in luck! That's actually possible already using the CHANNEL
function. Try:

http://localhost:8088/ari/channels/12345/variable?variable=CHANNEL(rtcp,rtt)

(There's a few variants of RTT to retrieve - you may want to play
around with a few of them)

See:

https://wiki.asterisk.org/wiki/display/AST/Asterisk+13+Function_CHANNEL

(Although I admit we *still* need to tweak the formatting of that
documentation. It's a hard one to get right.)

-- 
Matthew Jordan
Digium, Inc. | Engineering Manager
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
Check us out at: http://digium.com & http://asterisk.org

-- 
_
-- 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 14 - Remote URI Playback

[Matthew Jordan]

On Thu, Nov 6, 2014 at 6:46 PM, Joshua Colp  wrote:
>>
>> I think this is the way to do this in the long run. There's some major
>> technical hurdles to overcome with this, however:
>>
>> (1) A remote file stream doesn't have the ability to do a 'parallel
>> fetch' the way it is proposed on the wiki page. For video streams,
>> that means Asterisk would (finally) have to understand media container
>> formats, which is a *very* large project.
>> (2) There are the obvious technical issues dealing with buffering,
>> overrun of the network download, and other off nominal kinds of
>> things. I think those are reasonably well understood, or at least can
>> be thought through.
>> (3) The largest concern I have is that I can't envision the API for
>> this. Currently, Asterisk channels support two file streams - one for
>> audio, one for video. Those streams are opaque objects managed by the
>> file API (file.h). If this fits behind that API, then there are some
>> serious things to work through with how that API works. If it is a
>> separate type of stream and a different object, then we can't take
>> advantage of the remote stream in all of the existing applications,
>> which would be unfortunate.
>>
>> I think - for the purposes of this project only - I'd go with the
>> following philosophy: Make the media cache/HTTP playback blocking for
>> now, but put the hooks in for an asynchronous access to the retrieved
>> bytes for future expansion. That way the larger questions above can be
>> dealt with separately, but the information is available for what that
>> occurs.
>
>
> I hesitate to even say put the asynchronous hooks in because without scoping
> out how the core would look and behave chances are (speaking from
> experience) that what you do will be inadequate or just end up being thrown
> out.

I don't think I'd go too crazy here, as the call to perform a
'streaming' playback from a remote source should block - that's what
the dialplan applications (and other APIs, for that matter) expect.
The "asynchronous"-ish part is being able to take data returned from
the cURL request, convert them to frames, and put them on the channel
during execution of curl_exec. That's actually pretty easy, since
curl_exec periodically calls a callback function with the data
retrieved. Something could insert itself into the callback, take the
data, and - instead of saving it to a file - turn that data into
frames (buffering the remaining), and queue it up on the channel.

That's about the extent that I was thinking.

>
>>
>> To do this correctly, I think we'll need a sorcery wizard that accepts
>> push configuration/objects. We had previously talked about this with
>> respect to a push configuration model for PJSIP, but this actually
>> takes it one step further with a push wizard for buckets. Since
>> buckets sits on top of sorcery, it feels like this is theoretically
>> possible... but I'm not sure yet how it would play out completely.
>> Josh may want to comment on this part.
>
>
> It's all such a thin layer that you can do whatever you want. No matter
> whether it is sorcery or bucket, the implementation is only called into when
> the resource is requested - and ultimately the caller doesn't care where it
> comes from. This being said there's nothing in the sorcery core acting as a
> cache or place for this stuff - that's up to the wizards. As well in bucket
> what wizard is used is determined based on the URI scheme - and there can be
> only one impl for each scheme.
>

Yup. I'm not entirely sure how this would work yet - I think I have to
think about it some more. Ideally, we would have:
 * ARI resources that receive HTTP requests with data, e.g., /sounds
could have a PUT operation that, as BJ pointed out, pushes some JSON
metadata + a sound file to the server.
 * An API in the media cache that allows for updating item in the
cache. The ARI resources can then call this to push data to the cache
and add/update items manually.

As I write this, I think sorcery wizards may end up playing a smaller
role here than I originally thought. The actual sorcery wizard
implementation would still hide behind the media cache API - there's
really no reason for ARI (which provides the HTTP API for clients) to
use anything else.

I'll think about this some more and update the wiki page. I think
getting a prototype put together for the media cache would go a long
way, as it will start to flesh out these ideas.

-- 
Matthew Jordan
Digium, Inc. | Engineering Manager
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
Check us out at: http://digium.com & http://asterisk.org

-- 
_
-- 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] func_jitterbuffer handling of masquerades

[Joshua Colp]

Matthew Jordan wrote:

 (Conserve the bytes)



This gets further weird with ConfBridge (unfortunately), which uses
func_jitterbuffer to put a jitterbuffer on each channel that joins.
This works great unless they get masqueraded out of the ConfBridge.
Should the jitterbuffer travel with them at that point? Probably not.


Just as a slight note: ConfBridge does not remove the jitterbuffer when 
the channel leaves it. Same for denoise. >_>


--
Joshua Colp
Digium, Inc. | Senior Software Developer
445 Jan Davis Drive NW - Huntsville, AL 35806 - US
Check us out at: www.digium.com & www.asterisk.org

--
_
-- 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 14 - Remote URI Playback

[Joshua Colp]

Matthew Jordan wrote:





I think this is the way to do this in the long run. There's some major
technical hurdles to overcome with this, however:

(1) A remote file stream doesn't have the ability to do a 'parallel
fetch' the way it is proposed on the wiki page. For video streams,
that means Asterisk would (finally) have to understand media container
formats, which is a *very* large project.
(2) There are the obvious technical issues dealing with buffering,
overrun of the network download, and other off nominal kinds of
things. I think those are reasonably well understood, or at least can
be thought through.
(3) The largest concern I have is that I can't envision the API for
this. Currently, Asterisk channels support two file streams - one for
audio, one for video. Those streams are opaque objects managed by the
file API (file.h). If this fits behind that API, then there are some
serious things to work through with how that API works. If it is a
separate type of stream and a different object, then we can't take
advantage of the remote stream in all of the existing applications,
which would be unfortunate.

I think - for the purposes of this project only - I'd go with the
following philosophy: Make the media cache/HTTP playback blocking for
now, but put the hooks in for an asynchronous access to the retrieved
bytes for future expansion. That way the larger questions above can be
dealt with separately, but the information is available for what that
occurs.


I hesitate to even say put the asynchronous hooks in because without 
scoping out how the core would look and behave chances are (speaking 
from experience) that what you do will be inadequate or just end up 
being thrown out.






To do this correctly, I think we'll need a sorcery wizard that accepts
push configuration/objects. We had previously talked about this with
respect to a push configuration model for PJSIP, but this actually
takes it one step further with a push wizard for buckets. Since
buckets sits on top of sorcery, it feels like this is theoretically
possible... but I'm not sure yet how it would play out completely.
Josh may want to comment on this part.


It's all such a thin layer that you can do whatever you want. No matter 
whether it is sorcery or bucket, the implementation is only called into 
when the resource is requested - and ultimately the caller doesn't care 
where it comes from. This being said there's nothing in the sorcery core 
acting as a cache or place for this stuff - that's up to the wizards. As 
well in bucket what wizard is used is determined based on the URI scheme 
- and there can be only one impl for each scheme.


Cheers,

--
Joshua Colp
Digium, Inc. | Senior Software Developer
445 Jan Davis Drive NW - Huntsville, AL 35806 - US
Check us out at: www.digium.com & www.asterisk.org

--
_
-- 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 14 - Remote URI Playback

[BJ Weschke]

On 11/6/14, 4:04 PM, Matthew Jordan wrote:



  eg -
Playback(http://myserver.com/monkeys.wav&http://myserver.com/can.wav&http://myserver.com/act.wav&http://myserver.com/like.wav&http://myserver.com/weasels.wav)
<--- On an empty HTTP Media cache, the previous app invocation would
probably sound pretty bad to the first caller going through this workflow.
:-)

  Also, I think the inability to use & in a URI for playback really limits
the usefulness of this change. I totally understand why the typical URI
decode doesn't work, but perhaps a combination of a URI encoded & with an
HTML entity representation is a suitable alternative?  eg - (%26amp; == & in
a URI in Playback and do that pattern replacement in the URI before any
other URI decoding/encoding operations. Ya, I know, it's a hack, but not
allowing multiple parameters in a loaded queryString URL is way too
restricting IMHO).

So I just replied to this part on Johan's e-mail - do you think we can
skip supporting an '&' in a resource? (How many media files are going
to be named tt-monkeys&weasels anyway?)




 Yes. I think that's perfectly reasonable.


  Well, kind of. I think you're still envisioning using CURL behind the
scenes using the input provided in the JSON body of the PUT to /media_cache
to go and grab the resource from the remote server. If you go that way, I
think not only should we handle custom headers, but it's probably also not
unreasonable to provide a way to do basic/digest authentication for the GET
call as well. However, instead of that, I had envisioned being able to do a
PUT to /media_cache as a multipart MIME request where one part is the JSON
descriptor and the second part is the binary resource itself you're looking
to place into HTTP Media cache. The advantage of doing things this way is
that if you're running call control via some sort of API, that API will know
for certain when files/resources are ready to be played back and you don't
run the risk of the awkward blocking silence scenario that you have above.
However, when you do it this way, the URI description/parameter itself
doesn't make too much sense because it's not really where the resource came
from. I guess there's also a question as to whether or not we follow the
true REST practice with using POST for a brand new resource and PUT for
updates to existing resources.
I'd prefer that approach actually. Pushing media to the server is
preferable to having Asterisk attempt to pull it.

To do this correctly, I think we'll need a sorcery wizard that accepts
push configuration/objects. We had previously talked about this with
respect to a push configuration model for PJSIP, but this actually
takes it one step further with a push wizard for buckets. Since
buckets sits on top of sorcery, it feels like this is theoretically
possible... but I'm not sure yet how it would play out completely.
Josh may want to comment on this part.

 I look forward to his commentary. :-)


  As for the timestamps for deciding whether the local cache is dirty, I
don't think we should try to reinvent the wheel here. We should stick what's
already well established for stuff like this and use the entity tag (Etag)
response header stored and then use the "If-None-Match" request header
approach. Google does a much better job of explaining it than I can here:
https://developers.google.com/web/fundamentals/performance/optimizing-content-efficiency/http-caching


Agreed, as commented on Johan's e-mail. E-Tags for the win!

I'll update the wiki from the conversation here shortly.




--
_
-- 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] 4156: Test Suite: Allow setting snaplen & buffer_size for packet capturing

[jbigelow]

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

Review request for Asterisk Developers.


Repository: testsuite


Description
---

This allows setting the snaplen and buffer_size for packet capturing. The 
defaults set are the same as the defaults that yappcap.PcapLive sets.


Diffs
-

  /asterisk/trunk/lib/python/pcap_listener.py 5913 
  /asterisk/trunk/lib/python/asterisk/test_case.py 5913 
  /asterisk/trunk/lib/python/asterisk/pcap.py 5913 

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


Testing
---

Used the options to set a lower snaplen and a higher buffer successfully.


Thanks,

jbigelow

-- 
_
-- 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 14 - Remote URI Playback

[Matthew Jordan]

On Wed, Nov 5, 2014 at 1:45 AM, BJ Weschke  wrote:
> On 11/4/14, 3:40 PM, Matthew Jordan wrote:
>>
>> On Tue, Nov 4, 2014 at 12:57 PM, BJ Weschke  wrote:
>>>
>>>   Matt -
>>>
>>>   This is a pretty neat idea, indeed, but I've got some
>>> questions/thoughts on
>>> implementation. :-)   Apologies if all of this was already
>>> considered/accounted for already..
>>>
>>>   1) Does the entire file need to be downloaded and in place on the HTTP
>>> Media Cache before you can call an ast_openstream on it? This could cause
>>> some problems with larger files not sitting on a fat pipe local to the
>>> Asterisk instance.
>>
>> It does need to be completely on the local file system, which would be
>> a problem for extremely large files and/or slow network connections.
>>
>> The ability to do an 'asynchronous' version of this is not really
>> present. The filestream code in the core of Asterisk doesn't have
>> anything present that would allow it to buffer the file partially
>> before playing back with some expected max size. If we went down that
>> road, it'd almost be a completely separate filestream concept from
>> what we have today, which is pretty non-trivial.
>>
>> I don't think I have a good solution for really large files just yet.
>> There's some ways to do this using cURL (where we get back a chunk of
>> binary data, buffer it, and immediately start turning it into frames
>> for a channel) - but that feels like it would need a lot of work,
>> since we'd be essentially creating a new remote filestream type.
>
>  I know there's going to be a large population of Asterisk users that will
> want the simplicity of just specifying a URI for playback and expecting
> "sorcery" to happen. A decent number of them may even be OK with what may be
> a sub-second awkward silence to the caller on the line while things like the
> servicing thread synchronously queues the URI resource into the local HTTP
> media cache before playback. That's probably going to be an acceptable
> experience for a decent number of functional use cases. However, I think one
> somewhat common use case where this wouldn't go so well would be a list of
> URI resources that weren't already in HTTP media cache since they'd be
> fetched serially in-line at the time where playback really should be
> starting and block the channel with silence until the resource is set in
> media cache.

I think this is the way to do this in the long run. There's some major
technical hurdles to overcome with this, however:

(1) A remote file stream doesn't have the ability to do a 'parallel
fetch' the way it is proposed on the wiki page. For video streams,
that means Asterisk would (finally) have to understand media container
formats, which is a *very* large project.
(2) There are the obvious technical issues dealing with buffering,
overrun of the network download, and other off nominal kinds of
things. I think those are reasonably well understood, or at least can
be thought through.
(3) The largest concern I have is that I can't envision the API for
this. Currently, Asterisk channels support two file streams - one for
audio, one for video. Those streams are opaque objects managed by the
file API (file.h). If this fits behind that API, then there are some
serious things to work through with how that API works. If it is a
separate type of stream and a different object, then we can't take
advantage of the remote stream in all of the existing applications,
which would be unfortunate.

I think - for the purposes of this project only - I'd go with the
following philosophy: Make the media cache/HTTP playback blocking for
now, but put the hooks in for an asynchronous access to the retrieved
bytes for future expansion. That way the larger questions above can be
dealt with separately, but the information is available for what that
occurs.

I do think that means punting on using func_curl for this however, as
it just wasn't designed for this feature. That's not a big deal, since
it's pretty easy to use libcurl directly and - in the callback
function that receives the bytes from the remote HTTP server - make
the ast_channel object available for 'usage' in the future.

(And by the way: if someone was very interested in this part of the
project, I'd be thrilled if they wanted to go take a look at it. I
just think defining all of those APIs is outside the scope of getting
remote playback working first.)

>  eg -
> Playback(http://myserver.com/monkeys.wav&http://myserver.com/can.wav&http://myserver.com/act.wav&http://myserver.com/like.wav&http://myserver.com/weasels.wav)
> <--- On an empty HTTP Media cache, the previous app invocation would
> probably sound pretty bad to the first caller going through this workflow.
> :-)
>
>  Also, I think the inability to use & in a URI for playback really limits
> the usefulness of this change. I totally understand why the typical URI
> decode doesn't work, but perhaps a combination of a URI encoded & with an
> HTML entity representation is a suit

Re: [asterisk-dev] Asterisk 14 - Remote URI Playback

[Matthew Jordan]

On Wed, Nov 5, 2014 at 3:58 AM, Johan Wilfer  wrote:
> Den 2014-11-05 08:45, BJ Weschke skrev:
>>
>>
>>   As for the timestamps for deciding whether the local cache is dirty, I
>> don't think we should try to reinvent the wheel here. We should stick
>> what's already well established for stuff like this and use the entity
>> tag (Etag) response header stored and then use the "If-None-Match"
>> request header approach. Google does a much better job of explaining it
>> than I can here:
>>
>> https://developers.google.com/web/fundamentals/performance/optimizing-content-efficiency/http-caching
>>
>
> +1 This was my thought as well. We should stick to the HTTP standard, and
> not have our own way of doing caching. In HTTP you have two models:
>  1. Timestamps (header "Expires")
>  2. E-tag for validation of the resource (header "If-None-Match").

Yup, E-tag is the way to go here. I'll punt on my suggestions that I
have on the wiki and re-write it to note that. Thanks!

> I would also like to add that we really should have a way of using & in an
> url. Wouldn't this be a much more common use-case than to play in parallel?
> Maybe the parallel payback can be enabled/disabled by a extra flag in the
> dialplan? This way you would lose the ability to have an amp in the url if
> you would like to do parallel playback but not otherwise. I think that would
> be acceptable...

Consider a sound resource named "stuff&things". This is technically a
valid resource name, so long as the "&" is URI encoded:

http://myserver.com/sounds/stuff%26things

If this was a query of some sorts, then my URI my look something like this:

http://myserver.com/sounds?media=yes&file=stuff%26things

This is where things break down in Asterisk dialplan (keeping in mind
that for ARI, you could put all of this into a JSON blob and not
care.) If we allowed '&' to be URI encoded in the Playback dialplan
application, this would look like:

same => n,Playback(http://myserver.com/sounds?media=yes%26file=stuff%26things)

And when Asterisk decoded the URI, it would turn this into:

http://myserver.com/sounds?media=yes&file=stuff&things

Which is very, very wrong.

This has nothing to do with playing sounds in parallel, but with
Asterisk's nomenclature for delineating sequential sounds to playback
(that is, playlists). The '&' is already a reserved keyword for that,
so we have to escape it in some fashion before passing it to the
dialplan applications.

Now, as BJ pointed out, we could escape things in *different* ways -
but I'm concerned that doing that would make things more complicated
and prone to breaking. As an API usage, it also makes it hard to know
how to pass data to Asterisk - the more custom things are, the harder
it is to know what to do. URI escaping is common; custom Asterisk-y
escaping is ... not.

All of this is a long way of saying that I'd be happy if we supported
'&' in a URI, but only at the expense of *not* supporting a URI
escaped '&' in a resource in that URI.

>
> Overall I must say that this is very simple and cool idea and a nice way to
> do scalability. Thank you! :-)

Well, keep in mind this is just a proposal. It's no promise that it
will happen. We'll need assistance to have this feature actually show
up in Asterisk 14!

-- 
Matthew Jordan
Digium, Inc. | Engineering Manager
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
Check us out at: http://digium.com & http://asterisk.org

-- 
_
-- 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] 4101: Channel Originate/Continue via ARI support for labels in dialplan is incomplete

[Scott Griepentrog]

> On Nov. 5, 2014, 11:43 a.m., Joshua Colp wrote:
> > Matt brought it up that this is actually a backwards incompatible change - 
> > specifically changing priority into a string from an integer. What about 
> > having label as a separate argument that is optional? If present it's 
> > treated as a label and if not then the priority is used as normal. This 
> > allows labels to be used with no backwards incompatible changes and also 
> > makes it a bit more explicit from a developer side of what they want.
> 
> greenfieldtech wrote:
> Wow, that was actually my initial idea almost 4 weeks ago - the only 
> issue was that I was feeling it was truly unclean. If you look into Asterisk 
> docs, priority and labels and normally mixed. For example, if you use Goto in 
> dialplan - you can do Goto(context,exten,priority) or 
> Goto(context,exten,label). I personally feel it should work exactly the same, 
> I see no point in having two methodologies. In addition, what should be used 
> if both are defined? which has the stronger priority here? I feel this will 
> also bring much confusion into the mix. Again, I can easily revert my 
> original code work, but in my opinion it is very much confusing.
> 
> Anyone else would like to share their thought?

I agree that it would be easier to include this patch if it did not alter the 
existing API, only add an additional optional field.


- Scott


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


On Nov. 5, 2014, 8:16 a.m., greenfieldtech wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4101/
> ---
> 
> (Updated Nov. 5, 2014, 8:16 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24412
> https://issues.asterisk.org/jira/browse/ASTERISK-24412
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> This patch changes the current behavior of ARI, to allow channel 
> originate/continue requests to be performed with labels as the priority, not 
> only integer values.
> 
> 
> Diffs
> -
> 
>   /trunk/rest-api/api-docs/channels.json 425359 
>   /trunk/res/res_ari_channels.c 425359 
>   /trunk/res/ari/resource_channels.c 425359 
>   /trunk/res/ari/resource_channels.h 425359 
> 
> Diff: https://reviewboard.asterisk.org/r/4101/diff/
> 
> 
> Testing
> ---
> 
> Testing was performed by testing the following scenarios:
> 1. Originating a call to a numeric priority - works
> 2. Originating a call to a null priority - works
> 3. Originating a call to a label - works
> 4. Continue a call to a label - not tested yet
> 
> 
> Thanks,
> 
> greenfieldtech
> 
>

-- 
_
-- 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] func_jitterbuffer handling of masquerades

[Matthew Jordan]

On Wed, Nov 5, 2014 at 12:24 PM, Joshua Colp  wrote:
> Corey Farrell wrote:
>
> 
>
>>
>> The change I am proposing is that we always have an active JB after
>> masquerade if either side had one before the masquerade.  So in
>> scenario 1 and 2 listed above this would cause the only active
>> jitterbuffer to remain active after a masquerade.  For situations
>> where both channels have active jitterbuffer, we would always prefer
>> the jitterbuffer settings from clonechan.
>
>
> I'm not sure I agree with that. Local channels aside (as they always
> complicate things) for the moment if I have two channels:
>
> PJSIP/alice
> PJSIP/bob
>
> Following assumptions:
>
> PJSIP/alice has had a jitterbuffer placed on her.
>
> Scenario:
>
> PJSIP/bob masquerades into PJSIP/alice to take her place.
>
> As a deployer would I expect PJSIP/bob to have a jitterbuffer then? No. I
> placed it on PJSIP/alice. Why should it be on PJSIP/bob after this? I don't
> know or care that a masquerade happened. If it is on PJSIP/bob though - how
> do I know a masquerade has happened so I can get rid of it since I don't
> want it there?
>
> I can understand why when Local channels are involved it can make things
> easier but I don't think the resulting behavior would be what people would
> expect or want, and allowing some method to control it confuses people.
>
> That's my feelings about this.
>
> What do others think?
>

I think this scenario is a lot easier in Asterisk 12+.

But I also think Josh is right. Unfortunately, in Asterisk 11-, you
don't really know whether or not the channel being masqueraded into
another is representative of a new path of communication to the same
device or a different device. If it is the same device, you want it to
inherit; if not, you don't. Witness the craziness of the
'AUDIOHOOK_INHERIT' function.

This gets further weird with ConfBridge (unfortunately), which uses
func_jitterbuffer to put a jitterbuffer on each channel that joins.
This works great unless they get masqueraded out of the ConfBridge.
Should the jitterbuffer travel with them at that point? Probably not.

I think when a masquerade occurs, a jitterbuffer must:
 * Assume that it should be destroyed
 * Inform the channel that it was on that it has left the building
 * Re-sync the RTP engine appropriately so that the timestamps/SSRC
are as correct as it can

Matt

-- 
Matthew Jordan
Digium, Inc. | Engineering Manager
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
Check us out at: http://digium.com & http://asterisk.org

-- 
_
-- 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] 4135: Resolve race condition that can result in ARI apps not being notified of transfers

[opticron]

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



/branches/12/include/asterisk/stasis_bridges.h


This is missing documentation on the return value as are other 
documentation blocks.



/branches/12/main/bridge.c


Ahem.



/branches/12/main/bridge.c


This message needs to be removed.



/branches/12/main/stasis_bridges.c


This function only eats the transfer_message ref on failure. Callers don't 
appear to expect their reference to be consumed.



/branches/12/main/stasis_bridges.c


Using RAII_VAR here would be appropriate since the 5 applicable return 
paths require its cleanup.


- opticron


On Nov. 4, 2014, 3:44 p.m., Mark Michelson wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4135/
> ---
> 
> (Updated Nov. 4, 2014, 3:44 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> During blind transfer testing, it was noticed that tests were failing 
> occasionally because the ARI blind transfer event was not being sent. After 
> investigating, I detected a race condition in the blind transfer code. When 
> blind transferring a single channel, the actual transfer operation (i.e. 
> removing the transferee from the bridge and directing them to the proper 
> dialplan location) is queued onto the transferee bridge channel. After 
> queuing the transfer operation, the blind transfer Stasis message is 
> published. At the time of publication, snapshots of the channels and bridge 
> involved are created. The ARI subscriber to the blind transfer Stasis message 
> then attempts to determine if the bridge or any of the involved channels are 
> subscribed to by ARI applications. If so, then the blind transfer message is 
> sent to the applications. The way that the ARI blind transfer message handler 
> works is to first see if the transferer channel is subscribed to. If not, 
> then iterate over all the ch
 annel IDs in the bridge snapshot and determine if any of those are subscribed 
to. In the test we were running, the lone transferee channel was subscribed to, 
so an ARI event should have been sent to our application. Occasionally, though, 
the bridge snapshot did not have any channels IDs on it at all. Why?
> 
> The problem is that since the blind transfer operation is handled by a 
> separate thread, it is possible that the transfer will have completed and the 
> channels removed from the bridge before we publish the blind transfer Stasis 
> message. Since the blind transfer has completed, the bridge on which the 
> transfer occurred no longer has any channels on it, so the resulting bridge 
> snapshot has no channels on it. Through investigation of the code, I found 
> that attended transfers can have this issue too for the case where a 
> transferee is transferred to an application.
> 
> To fix this problem, I thought of four possible fixes:
> 1) Let the thread that actually performs the blind transfer publish the 
> Stasis message.
> 2) Get the bridge snapshot before queuing the blind transfer operation.
> 3) Publish the blind transfer Stasis message before queuing the blind 
> transfer operation.
> 4) Hold the bridge lock while queuing the blind transfer operation and 
> publishing the blind transfer Stasis message.
> 
> Option 1 is clearly the "best" option, but it also is made nearly impossible 
> due to the way that bridge channel operations are queued. Bridge channel 
> operations use frames, which require that their payload be copyable using 
> memcpy(). Including any sort of pointer is a no-no. So I would be forced to 
> come up with some inane method of representing multiple channels and bridges 
> in a frame in order to do this.
> 
> Option 2 is slightly more workable. Currently, there are functions to publish 
> blind and attended transfers that require bridges and channels, not 
> snapshots. Changing the API to accommodate snapshots is possible, but it is 
> more widespread than I would like, and it changes the API. It also runs the 
> slight risk of publishing "stale" data, though that is not likely.
> 
> Option 3 is easiest to implement, but it runs the (very slight) risk that we 
> could publish that a blind transfer happened successfully when in fact the 
> attempt to queue the blind transfer operation failed. I almost went with 
> this, but I felt like the possibility of lyin

Re: [asterisk-dev] [Code Review] 4123: app_agent_pool: Make agent alert interruptable by DTMF.

[rmudgett]

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

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


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 427493


Bugs: ASTERISK-24257 and ASTERISK-24447
https://issues.asterisk.org/jira/browse/ASTERISK-24257
https://issues.asterisk.org/jira/browse/ASTERISK-24447


Repository: Asterisk


Description
---

This patch can be broken into two logical changes.  The first is to allow the 
DTMF feature hooks to collect digits while passing frames from the bridge 
(ASTERISK-24447).  The secod is to allow an agent pool agent to interrupt the 
alerting playback file with DTMF (ASTERISK-24257).  The agent interrupting the 
alerting playback builds on the ASTERISK-24447 part because it knows what digit 
interrupted the playback and needs to be able to pass that digit to the DTMF 
hook digit collection code.

* Made collecting DTMF digits for the DTMF feature hooks not block receiving 
frames from the bridge. (Changes in bridge_channel.c and bridge_channel.h)

* Made collecting DTMF digits possible by other bridge hooks if there is a need.

* Made agent able to interrupt the alerting beep playback with DTMF.  Any digit 
can interrupt if the call does not need to be acknowledged.  Only the first 
digit of the acknowledgement can interrupt if the call needs to be 
acknowledged. (Changes in app_agent_pool.c)


Diffs
-

  /branches/12/main/bridge_channel.c 427333 
  /branches/12/include/asterisk/bridge_channel.h 427333 
  /branches/12/apps/app_agent_pool.c 427333 

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


Testing
---

With the patch
1) Agent can interrupt the beep playback with any digit if an acknowledgement 
is not required.
2) Agent can interrupt the beep playback with the first digit of the configured 
acknowledgement.  Other digits are ignored.  The acknowledgement could be one 
or more digits.
3) Agent can still wait for the playback to end.

4) Collecting DTMF digits for the DTMF feature hooks does not block receiving 
frames from the bridge.  Tested by using ControlPlayback application through a 
local channel chain that had other DTMF feature hooks starting with '*'.  When 
I pressed the '*', the rewinding of the playback was delayed by the digit 
collection interdigit timeout time but the audio was not discarded during the 
interdigit timeout time.


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] 4146: res_pjsip: If an endpoint is associated with the dialog place it on the messag early

[Joshua Colp]

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

(Updated Nov. 6, 2014, 12:20 p.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 427490


Bugs: AST-1459
https://issues.asterisk.org/jira/browse/AST-1459


Repository: Asterisk


Description
---

Currently when distributing a message the code finds the dialog related to it 
and if present uses the information to send the message to the right serializer 
and to place the endpoint on the message. For the endpoint its presence is 
checked in the endpoint lookup step and placed into the expected endpoint spot. 
This causes problems as the dialog handling will occur before the endpoint 
lookup step, causing the endpoint to get lost and never placed on the message.

This change removes the endpoint lookup step requirement and places the 
endpoint in the expected spot in the distribution step. If the endpoint lookup 
step is actually invoked it just immediately returns if an endpoint is already 
present and does nothing.


Diffs
-

  /branches/12/res/res_pjsip/pjsip_distributor.c 427112 

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


Testing
---

Added some code to determine if endpoint is present in the NAT handling code. 
Without the patch no endpoint would be present on the 200 OK response to an 
outgoing re-INVITE. With the patch an endpoint is present on the 200 OK 
response.


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

[asterisk-dev] [Code Review] 4155: PJSIP: Allow contact rewriting to fall back for reliable transports

[opticron]

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

Review request for Asterisk Developers and Joshua Colp.


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


Repository: Asterisk


Description
---

This modifies contact rewriting to revert to the original contact URI for a 
dialog when persistent transports shut down.

Some calls can enter a condition where their contact URI from the initial 
incoming invite was rewritten for a reliable transport, but that transport has 
been shut down due to inactivity. Such reliable transports can not be 
re-established since the remote host was never listening on the associated 
port. In cases such as these, it is useful to be able to fall back to the 
original contact URI since it might be accessible and allow the call to 
continue normally.


Diffs
-

  branches/12/res/res_pjsip_nat.c 425222 

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


Testing
---

Verified that this allowed the call to operate normally despite the original 
reliable connection being torn down where this situation was experienced.


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

[asterisk-dev] Asterisk MoS and RTCP data [pardon the directness and language :-)]

[Nir Simionovich]

Hi All,

So, if there is one thing I really like about PJSIP and WebRTC
(specifically with mobile) is the ability to produce meaningful MoS scoring
for calls in real time. Now, Asterisk doesn't have that capability, at
least, not during the actual call - but only after.

In itself, not an issue - it's good enough.

If you were to look up the calculation for how to get your MoS, the
following is the most common
algorithm that I've found:

effectiveLatency = rttMs + averageJitterMs * 2 + 10
R = 93.2 - (effectiveLatency/40)
R = R - (fractionLost * 2.5)
MOS = 1 + (0.035)*R+(0.07)*R*(R-60)*(100-R)

My primary question is this: I can't find what rttMs in Asterisk
terminology is. There is an rtt value,
but its value is several orders of magnitude than I would expect. So, I
thought it may be in microSec, not miliSec - but that doesn't make much
sense either.

So, on one hand I have a shit load of information, on the other hand, it is
formatted in a very hard way to manage.

Any idea how we can make this better? On another notion, I think that
adding RTCP reading capabilities to the channels module in ARI may probe
extremely useful.

Nir S
-- 
_
-- 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] 4101: Channel Originate/Continue via ARI support for labels in dialplan is incomplete

[greenfieldtech]

> On Nov. 5, 2014, 5:43 p.m., Joshua Colp wrote:
> > Matt brought it up that this is actually a backwards incompatible change - 
> > specifically changing priority into a string from an integer. What about 
> > having label as a separate argument that is optional? If present it's 
> > treated as a label and if not then the priority is used as normal. This 
> > allows labels to be used with no backwards incompatible changes and also 
> > makes it a bit more explicit from a developer side of what they want.

Wow, that was actually my initial idea almost 4 weeks ago - the only issue was 
that I was feeling it was truly unclean. If you look into Asterisk docs, 
priority and labels and normally mixed. For example, if you use Goto in 
dialplan - you can do Goto(context,exten,priority) or 
Goto(context,exten,label). I personally feel it should work exactly the same, I 
see no point in having two methodologies. In addition, what should be used if 
both are defined? which has the stronger priority here? I feel this will also 
bring much confusion into the mix. Again, I can easily revert my original code 
work, but in my opinion it is very much confusing.

Anyone else would like to share their thought?


- greenfieldtech


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


On Nov. 5, 2014, 2:16 p.m., greenfieldtech wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4101/
> ---
> 
> (Updated Nov. 5, 2014, 2:16 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24412
> https://issues.asterisk.org/jira/browse/ASTERISK-24412
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> This patch changes the current behavior of ARI, to allow channel 
> originate/continue requests to be performed with labels as the priority, not 
> only integer values.
> 
> 
> Diffs
> -
> 
>   /trunk/rest-api/api-docs/channels.json 425359 
>   /trunk/res/res_ari_channels.c 425359 
>   /trunk/res/ari/resource_channels.c 425359 
>   /trunk/res/ari/resource_channels.h 425359 
> 
> Diff: https://reviewboard.asterisk.org/r/4101/diff/
> 
> 
> Testing
> ---
> 
> Testing was performed by testing the following scenarios:
> 1. Originating a call to a numeric priority - works
> 2. Originating a call to a null priority - works
> 3. Originating a call to a label - works
> 4. Continue a call to a label - not tested yet
> 
> 
> Thanks,
> 
> greenfieldtech
> 
>

-- 
_
-- 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] 4149: main/file.c: fix possible extra ast_module_unref to format modules

[Corey Farrell]

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

(Updated Nov. 6, 2014, 6:10 a.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 427464


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


Repository: Asterisk


Description
---

fn_wrapper only adds a reference to the format's module if the file was able to 
be opened.  If not this causes an unmatched ast_module_unref in 
filestream_destructor.


Diffs
-

  /branches/11/main/file.c 427255 

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


Testing
---

Verified the issue and fix with tests/apps/voicemail/play_message + r4141.


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

[asterisk-dev] [Code Review] 4153: manager: HTTP connections leak references

[Corey Farrell]

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

Review request for Asterisk Developers.


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


Repository: Asterisk


Description
---

Fix reference leak that happens if (session && !blastaway).


Diffs
-

  /branches/11/main/manager.c 427380 

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


Testing
---

tests/manager/config/basic against 13.


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] 4151: Fix compiler error when using ./configure --enable-dev-mode --enable-coverage

[Corey Farrell]

> On Nov. 6, 2014, 6:04 a.m., wdoekes wrote:
> > Isn't the better fix to disable coverage for the shadow compilation?

Probably.  I have updated code but I won't have time to test until this 
weekend.  I'll update the review once I've had a chance to do some builds with 
it.


- Corey


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


On Nov. 6, 2014, 5:06 a.m., Corey Farrell wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4151/
> ---
> 
> (Updated Nov. 6, 2014, 5:06 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24502
> https://issues.asterisk.org/jira/browse/ASTERISK-24502
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> When coverage and dev-mode are enabled with DONT_OPTIMIZE, it causes build 
> failure.  The double compilation does a 'shadow' build of each file with 
> output to /dev/null.  Unfortunately when coverage is enabled, GCC tries 
> writing to /dev/null.gcno (at least some versions do).  This prevents the 
> build from proceeding.
> 
> The simple fix is to simply prevent COMPILE_DOUBLE when coverage is enabled, 
> that is what I'm proposing here.  The other option would be to use a real 
> output location instead of /dev/null, delete the file immediately after 
> building.  I'm not sure that is needed, so I've proposed the simpler fix.
> 
> 
> Diffs
> -
> 
>   /branches/11/Makefile.rules 427380 
> 
> Diff: https://reviewboard.asterisk.org/r/4151/diff/
> 
> 
> Testing
> ---
> 
> Build with these options now works in Linux Mint 17 (gcc 4.8.2-19ubuntu1).
> 
> 
> 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] 4152: chan_console: Fix reference leaks to pvt

[Corey Farrell]

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

(Updated Nov. 6, 2014, 6:06 a.m.)


Review request for Asterisk Developers.


Changes
---

Add missing braces.


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


Repository: Asterisk


Description
---

Fix a bunch of calls to get_active_pvt where the reference is never released.

Leaks found by Bamboo 
https://bamboo.asterisk.org/bamboo/artifact/AST-ATSRC1/C632TE/build-2/Test-Suite-Logs/apps/queues/queue_baseline/run_1/ast1/var/log/asterisk/refs.txt


Diffs (updated)
-

  /branches/11/channels/chan_console.c 427380 

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


Testing
---

No.  Visually inspected changes, they are straight forward.


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] 4151: Fix compiler error when using ./configure --enable-dev-mode --enable-coverage

[wdoekes]

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


Isn't the better fix to disable coverage for the shadow compilation?

- wdoekes


On Nov. 6, 2014, 10:06 a.m., Corey Farrell wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4151/
> ---
> 
> (Updated Nov. 6, 2014, 10:06 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24502
> https://issues.asterisk.org/jira/browse/ASTERISK-24502
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> When coverage and dev-mode are enabled with DONT_OPTIMIZE, it causes build 
> failure.  The double compilation does a 'shadow' build of each file with 
> output to /dev/null.  Unfortunately when coverage is enabled, GCC tries 
> writing to /dev/null.gcno (at least some versions do).  This prevents the 
> build from proceeding.
> 
> The simple fix is to simply prevent COMPILE_DOUBLE when coverage is enabled, 
> that is what I'm proposing here.  The other option would be to use a real 
> output location instead of /dev/null, delete the file immediately after 
> building.  I'm not sure that is needed, so I've proposed the simpler fix.
> 
> 
> Diffs
> -
> 
>   /branches/11/Makefile.rules 427380 
> 
> Diff: https://reviewboard.asterisk.org/r/4151/diff/
> 
> 
> Testing
> ---
> 
> Build with these options now works in Linux Mint 17 (gcc 4.8.2-19ubuntu1).
> 
> 
> 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] 4152: chan_console: Fix reference leaks to pvt

[wdoekes]

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

Ship it!


Minor nits.


/branches/11/channels/chan_console.c


Please add the missing braces here. It looks awkward now.



/branches/11/channels/chan_console.c


Idem.



/branches/11/channels/chan_console.c


Idem.



/branches/11/channels/chan_console.c


Idem.


- wdoekes


On Nov. 6, 2014, 10:52 a.m., Corey Farrell wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4152/
> ---
> 
> (Updated Nov. 6, 2014, 10:52 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24504
> https://issues.asterisk.org/jira/browse/ASTERISK-24504
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Fix a bunch of calls to get_active_pvt where the reference is never released.
> 
> Leaks found by Bamboo 
> https://bamboo.asterisk.org/bamboo/artifact/AST-ATSRC1/C632TE/build-2/Test-Suite-Logs/apps/queues/queue_baseline/run_1/ast1/var/log/asterisk/refs.txt
> 
> 
> Diffs
> -
> 
>   /branches/11/channels/chan_console.c 427380 
> 
> Diff: https://reviewboard.asterisk.org/r/4152/diff/
> 
> 
> Testing
> ---
> 
> No.  Visually inspected changes, they are straight forward.
> 
> 
> 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

[asterisk-dev] [Code Review] 4152: chan_console: Fix reference leaks to pvt

[Corey Farrell]

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

Review request for Asterisk Developers.


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


Repository: Asterisk


Description
---

Fix a bunch of calls to get_active_pvt where the reference is never released.

Leaks found by Bamboo 
https://bamboo.asterisk.org/bamboo/artifact/AST-ATSRC1/C632TE/build-2/Test-Suite-Logs/apps/queues/queue_baseline/run_1/ast1/var/log/asterisk/refs.txt


Diffs
-

  /branches/11/channels/chan_console.c 427380 

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


Testing
---

No.  Visually inspected changes, they are straight forward.


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

[asterisk-dev] [Code Review] 4151: Fix compiler error when using ./configure --enable-dev-mode --enable-coverage

[Corey Farrell]

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

Review request for Asterisk Developers.


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


Repository: Asterisk


Description
---

When coverage and dev-mode are enabled with DONT_OPTIMIZE, it causes build 
failure.  The double compilation does a 'shadow' build of each file with output 
to /dev/null.  Unfortunately when coverage is enabled, GCC tries writing to 
/dev/null.gcno (at least some versions do).  This prevents the build from 
proceeding.

The simple fix is to simply prevent COMPILE_DOUBLE when coverage is enabled, 
that is what I'm proposing here.  The other option would be to use a real 
output location instead of /dev/null, delete the file immediately after 
building.  I'm not sure that is needed, so I've proposed the simpler fix.


Diffs
-

  /branches/11/Makefile.rules 427380 

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


Testing
---

Build with these options now works in Linux Mint 17 (gcc 4.8.2-19ubuntu1).


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] 4150: res_hep: fix major leak that occurs when config is missing or enabled=no

[Corey Farrell]

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

(Updated Nov. 6, 2014, 3:22 a.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 427400


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


Repository: Asterisk


Description
---

Add missing unref to hepv3_send_packet.


Diffs
-

  /branches/13/res/res_hep.c 427298 

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


Testing
---

Tested by Zane Conkle.


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] 4114: Prevent stringfields from accumulating unused memory

[Corey Farrell]

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

(Updated Nov. 6, 2014, 3:05 a.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 427380


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


Repository: Asterisk


Description
---

Any time a stringfield is blanked it currently prevents any currently allocated 
memory from being freed.  If a stringfield is repeatedly set to blank then set 
to a non-blank value, it causes new pools to be continuously allocated and 
never freed.

I'm unsure if the loop can be optimized, maybe the break can be re-added to the 
original location on the condition that ptr == __ast_string_field_empty?


Diffs
-

  /branches/11/main/utils.c 427111 
  /branches/11/include/asterisk/stringfields.h 427111 

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


Testing
---

Manual test using 
https://github.com/elessard1/asterisk-lab/blob/master/examples/lab_stringfields_leak.c
 to verify that old pools are now freed.

Full testsuite against Asterisk 13.


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