[asterisk-dev] [Code Review] Pimp my SIP: Direct Media Support

[Mark Michelson]

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

(Updated March 14, 2013, 12:21 p.m.)


Review request for Asterisk Developers and Joshua Colp.


Changes
-------

Addresses findings.


Summary
-------

This adds direct media support to the new SIP work in Asterisk.

There are four direct media options, all applicable to endpoints:

1) direct_media: Boolean to indicate if direct media is enabled. Defaults to "yes".
2) direct_media_method: Indicator of which method to use when requesting direct media. Settable to "invite" (alternatively "reinvite") or "update". Defaults to "invite".
3) direct_media_glare_mitigation: Allows for direct media only to be sent on certain call directions in order to prevent glare situations. Defaults to "none".
4) disable_direct_media_on_nat: Boolean to indicate if direct media should be disabled if a media NAT is detected.

These more-or-less align with the direct media options for chan_sip, except that they are separated out into different options instead of falling into a single "directmedia" option.

The most difficult part of this patch was that the pjsip_inv_session code does not maintain any sort of INVITE state machine once the initial INVITE transaction has terminated. In other words, if we were to receive or send a reinvite, the underlying pjsip_inv_session layer would not take this into account when attempting to do things like end a session.

In order to keep things simple, there are certain operations we will not do if there is currently a reinvite transaction in progress. These things are
1) Send any sort of session refresh request (INVITE or UPDATE)
2) End a session

In the case of 1) it may be possible given the current state of the INVITE transaction to send an UPDATE. However, the benefit of being able to do this is slim, and the maintenance of such code is much more difficult than just waiting for the INVITE transaction to terminate before sending the UPDATE.

In the case of 2) it may be possible to send a CANCEL for the reinvite and then send a BYE. However, the pjsip_inv_session API does not provide a method to create a CANCEL request. It would be possible to create a CANCEL at the dialog level; however, bypassing the pjsip_inv_session layer is generally ill-advised.

The big API changes here are to remove ast_sip_session_send_reinvite() and replace it with ast_sip_session_refresh(), which allows for the session refresh method to be specified. The other big change is to add ast_sip_session_send_request_with_cb(), which allows for a callback to be called when the request's response is received.


This addresses bug ASTERISK-21072.
    https://issues.asterisk.org/jira/browse/ASTERISK-21072


Diffs (updated)
-----

  /team/group/pimp_my_sip/channels/chan_gulp.c 383058 
  /team/group/pimp_my_sip/include/asterisk/res_sip.h 383058 
  /team/group/pimp_my_sip/include/asterisk/res_sip_session.h 383058 
  /team/group/pimp_my_sip/res/res_sip/sip_configuration.c 383058 
  /team/group/pimp_my_sip/res/res_sip_sdp_audio.c 383058 
  /team/group/pimp_my_sip/res/res_sip_session.c 383058 
  /team/group/pimp_my_sip/res/res_sip_session.exports.in 383058 

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


Testing
-------

I tested the individual direct media options both with phones and with SIPp scenarios that simulated situations such as non-response to a reinvite or glare. In all cases, behavior was as I would expect. Direct media will have a formal battery of testsuite tests added at a later date.

You will notice in chan_gulp.c that there are a couple of stray direct_media settings for the outbound endpoint to enable direct media and to use UPDATE for session refreshes. This was leftover from my testing since the pimp_sip_location work has not been merged yet.


Thanks,

Mark

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