[asterisk-dev] [Code Review] 2794: bridge_native_rtp: Fix bugs caused by HOLD/UNHOLD/UPDATE_RTP_PEER

[Joshua Colp]

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

Ship it!


Besides my minor comments below looks fine.


/trunk/bridges/bridge_native_rtp.c
<https://reviewboard.asterisk.org/r/2794/#comment18589>

    Not a huge fan of "requestor" here but I haven't come up with an alternative.



/trunk/bridges/bridge_native_rtp.c
<https://reviewboard.asterisk.org/r/2794/#comment18590>

    Add a comment here explaining why this works and why it is not operating on the opposite channel.



/trunk/bridges/bridge_native_rtp.c
<https://reviewboard.asterisk.org/r/2794/#comment18587>

    I don't think this is really beneficial.



/trunk/bridges/bridge_native_rtp.c
<https://reviewboard.asterisk.org/r/2794/#comment18586>

    I don't think this is really beneficial.


- Joshua Colp


On Aug. 23, 2013, 6:01 p.m., jrose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2794/
> -----------------------------------------------------------
> 
> (Updated Aug. 23, 2013, 6:01 p.m.)
> 
> 
> Review request for Asterisk Developers, Joshua Colp, Matt Jordan, Mark Michelson, and rmudgett.
> 
> 
> Bugs: ASTERISK-22217
>     https://issues.asterisk.org/jira/browse/ASTERISK-22217
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Issuing hold/unhold would lead to odd behavior. Between two chan_sip devices, a hold could cause an endless chain of updates while with pjsip a similar chain would begin but then end somewhat randomly. This patch fixes that by no longer tweaking the RTP glue on both sides of the call for every HOLD/UNHOLD/UPDATE_RTP_PEER frame.
> 
> Overall this restores holding behavior with chan_sip to how it worked with 11 (which includes some bugs I've noticed). chan_pjsip seems to be working as well, but it doesn't have the same bugs chan_sip exhibited with 11 (chan_sip does), which I guess is a good thing.
> 
> This patch doesn't fully address the test failures noted in the bug report. Asterisk 12 will needs a new version of the test with Asterisk 12 specific sipp scenarios to deal with that.
> 
> 
> Diffs
> -----
> 
>   /trunk/bridges/bridge_native_rtp.c 397424 
> 
> Diff: https://reviewboard.asterisk.org/r/2794/diff/
> 
> 
> Testing
> -------
> 
> Polycom to Polycom holds and unholds from both sides with directmedia using chan_sip
> Polycom to Polycom holds and unholds from both sides with directmedia using chan_pjsip
> Polycom to JITSI softphone same scenario (pjsip)
> Polycom to Digium phone same scenario (pjsip)
> 
> 
> Thanks,
> 
> jrose
> 
>

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