[asterisk-dev] [Code Review] res_sip and res_sip_session design review

[jcolp]

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


res_sip_session --

Structures:

1. How does a session extension persist data on a session? mod_data in pjsip_inv_session?
2. How does a session extension know the life time of the session? Does it know based on the messages? (Should we have common logic which then calls into callbacks in the extension)?
3. I'm not a huge fan of using "extension" in the way you have, I understand why but it has other meanings in our system.
4. The SDP stuff isn't just for INVITEs - it's for 200 OKs, 180s, 183s, etc.
5. If the SDP stuff was extended more we could have the complete logic for each type of media (audio, video, T.38 udptl) separated out into different modules... would make organization cleaner. 

Common SIP methods:

1. Your ast_sip_session_get_identity is missing the rdata
2. How does something calling ast_sip_session_send_reinvite know the result?

- jcolp


On Dec. 20, 2012, 1:17 p.m., Mark Michelson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2251/
> -----------------------------------------------------------
> 
> (Updated Dec. 20, 2012, 1:17 p.m.)
> 
> 
> Review request for Asterisk Developers, Matt Jordan and jcolp.
> 
> 
> Summary
> -------
> 
> This is a proposal for a res_sip and res_sip_session API for use in the new SIP channel driver. The pages are located here:
> 
> https://wiki.asterisk.org/wiki/display/AST/res_sip+design
> https://wiki.asterisk.org/wiki/display/AST/res_sip_session+design
> 
> Please let me know what you think of these.
> 
> There are a few things that are not here and that probably should
> * A struct called ast_sip_endpoint is referenced in a few places, but it is not defined. This is because a SIP endpoint is more-or-less defined by the DAL, which is currently under development by Mr. Joshua Colp. Once endpoint configuration and related structures are defined, they can be added in to these pages.
> * There are no functions in res_sip_session for iterating over SDP media streams or attributes, nor are there any functions for aiding in creating SDPs. These likely should exist, but I have not placed them here now since I have difficulty seeing what parameters will be necessary nor what they might return.
> 
> 
> Diffs
> -----
> 
> 
> Diff: https://reviewboard.asterisk.org/r/2251/diff
> 
> 
> Testing
> -------
> 
> The wiki page renders properly.
> 
> 
> Thanks,
> 
> Mark
> 
>

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