> On May 20, 2014, 11 a.m., Joshua Colp wrote:
> > /branches/12/res/parking/parking_bridge_features.c, lines 58-82
> > <https://reviewboard.asterisk.org/r/3485/diff/6/?file=58663#file58663line58>
> >
> >     What value does this add over just using transfer_channel_data directly?

Probably nothing. In an earlier version of the patch this was used for tracking 
a mutex/condition so that I could block the return of the main transfer 
function and I just never thought to clean it up. I'll take care of this in a 
while.


> On May 20, 2014, 11 a.m., Joshua Colp wrote:
> > /branches/12/res/res_pjsip_refer.c, line 519
> > <https://reviewboard.asterisk.org/r/3485/diff/6/?file=58664#file58664line519>
> >
> >     Can you further elaborate on this? Why did you add this? What problem 
> > was there? What does it solve?

Ok, so now that I'm looking through this again, I see that this approach is 
totally wrong, but here's the basic problem...

the transfer_channel_cb applies a subscription here:
                /* We also will need to detect if the transferee enters a 
bridge. This is currently the only reliable way to
                 * detect if the transfer target has answered the call
                 */
                refer->progress->bridge_sub = 
stasis_subscribe(ast_bridge_topic_all(), refer_progress_bridge, 
refer->progress);


This subscription listens for bridge messages and if the channel bridge matches 
the channel being tracked, then a 200 notify is pushed. Obvious in the case of 
parking this is bad since the channel is bridged before the announcement 
completes and this will end up terminating the call before the announcement is 
over.

My silly ham-fisted fix for this essentially made refer_progress_bridge just 
kill itself... so if I were going to continue on that approach, I'd have to 
remove refer_progress_bridge entirely. At the moment, I'm thinking the thing to 
do is just to add a reference to the transfer_channel_data so that it can also 
track whether or not completed has been set.


- Jonathan


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


On May 19, 2014, 1:25 p.m., Jonathan Rose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3485/
> -----------------------------------------------------------
> 
> (Updated May 19, 2014, 1:25 p.m.)
> 
> 
> Review request for Asterisk Developers, Matt Jordan and Mark Michelson.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> If a PJSIP endpoint attempts to blind transfer to a parking extension, there 
> is an override to the normal transfer logic that can make things act a little 
> weird. We noticed that this would leave various phones hanging on transfer 
> screens without progressing. When the transfer was considered successful, 
> PJSIP deferred the actual action of sending the 200 notify and the actual 
> trigger for that happening never occurs when the transfer is to a parking 
> extension.
> 
> In order to handle this, the bridge function that handles blind transfers now 
> returns a different value if a call was parked and if the channel driver 
> needs to react differently in this case, it can.  In the case of PJSIP, we 
> respond to transfers to park by immediately sending the notify with 200 OK 
> sip frag instead of deferring the action.
> 
> 
> Diffs
> -----
> 
>   /branches/12/res/res_pjsip_refer.c 413303 
>   /branches/12/res/parking/parking_bridge_features.c 413303 
>   /branches/12/res/parking/parking_applications.c 413303 
>   /branches/12/main/parking.c 413303 
>   /branches/12/main/bridge_basic.c 413303 
>   /branches/12/main/bridge.c 413303 
>   /branches/12/include/asterisk/parking.h 413303 
>   /branches/12/include/asterisk/bridge.h 413303 
>   /branches/12/channels/sig_analog.c 413303 
>   /branches/12/channels/chan_sip.c 413303 
>   /branches/12/channels/chan_mgcp.c 413303 
>   /branches/12/channels/chan_dahdi.c 413303 
> 
> Diff: https://reviewboard.asterisk.org/r/3485/diff/
> 
> 
> Testing
> -------
> 
> Before patch:
> * Blind transfer on Polycom SPIP: Phone is on the blind transfer screen until 
> it either manually hangs up or 60 seconds pass and Asterisk terminates the 
> session.
> 
> After the patch:
> * Blind transfer on Polycom SPIP: Phone immediately leaves the blind transfer 
> screen and goes back to idle mode.
> 
> 
> Thanks,
> 
> Jonathan Rose
> 
>

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

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

Reply via email to