[asterisk-dev] [Code Review]: Add manager event for early media

[jrose]

> On Dec. 9, 2011, 8:44 a.m., jrose wrote:
> > Truth be told, progress isn't really a state... it's just an indication.  Usually I think it's just used for Early Media, but I don't really know all the implications behind it, so it might be proper to call it an indication event.  But you seem to have already talked with Russell about this a while back going on the conversation in here and came to the conclusion that this wouldn't work for some reason.
> > 
> > The only thing I'm at all worried about at this point is that we come back later and decide that this really should have been a different kind of event and have to make an option to keep this behavior instead of just discarding it outright.  Even then it wouldn't be that horribly big of a deal, and I generally agree with having an ami event for tracking this, so you can go ahead and commit this.
> 
> Russell Bryant wrote:
>     I don't remember the discussion oej references (we probably did talk about it, I just don't remember).  
>     
>     FWIW, just looking at this review, I still agree with my past self that this is not a channel state, so using the NewState event is wrong.
> 
> rmudgett wrote:
>     Progress is not a state and I agree that reporting it as a state is wrong.  Q,931 specifically states that the PROGRESS message does not change the call state.
> 
> Olle E Johansson wrote:
>     http://markmail.org/message/32gn7rgupnbw3jcd
>     
>     Now we're back to where we started, running in circles. I started this discussion and Russell and I agreed that this was the way forward. Now we're back. Instead of denying, come up with proposals. We need some sort of event here.
> 
> Olle E Johansson wrote:
>     Notice that I clearly said in the start message that this is not a call state, but...
>     http://markmail.org/message/32gn7rgupnbw3jcd#query:+page:1+mid:zfr4bn5mz3x4xcn2+state:results
>
> 
> jrose wrote:
>     As far as I could tell from that conversation, it looked like Russell was actually either unsure whether sending it as a NewState Event or creating a new event type for this sort of thing is more appropriate.  Of the emails you linked, the end of the conversation seemed to be pushing more towards a Progress event (though I personally think Indication with some kind of type indicator Progress would be the most workable).
>     
>     Since there is a general lack of a clear answer here though, I just want to know what, if any, major advantage there is here in making this a NewState event as opposed to making it a new type of event... and as long as this is just sending AMI messages for now without reference to anything involving the actual state of the channel, I'm not seeing any immediately jumping out at me.
> 
> Olle E Johansson wrote:
>     Quoting Russell: "Agreed, the NewState event seems to make sense (which pretty much implies that
>     we have to add a channel state)." I can't parse it any other way than an agreement that this is the way
>     forward, even though we both agreed it was not an ISDN event - but it makes sense from the AMI developer's
>     point of view, as confirmed by Nicolas Guidino in the same thread.
>     
>     We implemented this in two applications that has been running since April - when this conversation happened.
>     We found no family of events we could create a new event type for. In SIP, a progress is mostly used as a replacement
>     for ringing - so in practice it belongs in newstate.

That would be a very positive quote in favor of using a NewState event if he hadn't sent another email afterwards calling that previous sentiment into question.

Keep in mind that many of these states that you are associating with Progress also send indications.  I personally think it would actually be more appropriate to add indication events to those.  Sending those indications doesn't explicitly change the state of the call either.


- jrose


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


On Sept. 19, 2011, 6:43 a.m., Olle E Johansson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1187/
> -----------------------------------------------------------
> 
> (Updated Sept. 19, 2011, 6:43 a.m.)
> 
> 
> Review request for Asterisk Developers and Russell Bryant.
> 
> 
> Summary
> -------
> 
> Add a manager event for early media. Using the existing Newstate event with a "faked" state called "progress". 
> 
> In the long run, this is not the right thing to do, but after discussion with Russell on asterisk-dev it was concluded that it was the easiest way forward.
> 
> 
> This addresses bug 19135.
>     https://issues.asterisk.org/jira/browse/19135
> 
> 
> Diffs
> -----
> 
>   /team/oej/bring-in-the-early-media-trunk/apps/app_dial.c 332755 
>   /team/oej/bring-in-the-early-media-trunk/main/pbx.c 332755 
> 
> Diff: https://reviewboard.asterisk.org/r/1187/diff
> 
> 
> Testing
> -------
> 
> Tested many calls in the 1.4 version of the same patch. 
> 
> One-legged calls using the progress() dialplan app, two legged calls using dial().
> 
> 
> Thanks,
> 
> Olle E
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20111213/117ddeaf/attachment-0001.htm>