[asterisk-dev] [Code Review] 3485: pjsip: Fix a bug where transferring to a parking extension causes calls to hang

[Matt Jordan]

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



/branches/12/main/bridge.c
<https://reviewboard.asterisk.org/r/3485/#comment21843>

    The 'completed flag'



/branches/12/res/res_pjsip_refer.c
<https://reviewboard.asterisk.org/r/3485/#comment21846>

    Is there any reason why the transfer_channel_data struct wasn't just added to the refer_progress struct?



/branches/12/res/res_pjsip_refer.c
<https://reviewboard.asterisk.org/r/3485/#comment21845>

    (1) Don't leave comments with your name. That's about equivalent to putting in issue nubmers.
    
    (2) From the perspective of SIP, the subscription is still correct: we need to know when the channel is re-bridged to send the 200 OK. If the transfer goes into something other than a bridge, the subscription should get terminated when the channel is hung up.



/branches/12/res/res_pjsip_refer.c
<https://reviewboard.asterisk.org/r/3485/#comment21847>

    This struct is the same as refer_progress_bridge_data. Why do we have two identical definitions?



/branches/12/res/res_pjsip_refer.c
<https://reviewboard.asterisk.org/r/3485/#comment21848>

    This should see if the subscription is still active on the bridge and, if so, remove the subscription.



/branches/12/res/res_pjsip_refer.c
<https://reviewboard.asterisk.org/r/3485/#comment21849>

    This feels very messy. We just created a wrapper struct for the user_data_wrapper and the refer_data->progress, bumped their ref counts ... then cleaned up and destroyed them.
    
    And now we're doing it again. I think there's a way to combine the logic here.


- Matt Jordan


On May 21, 2014, 11:29 a.m., Jonathan Rose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3485/
> -----------------------------------------------------------
> 
> (Updated May 21, 2014, 11:29 a.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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140521/61564df4/attachment-0001.html>